Skip to content

Commit a99a51b

Browse files
committed
- Fixed RSA Key Size Mismatch.
- Fixed Unnecessary Admin URL Removals. - Fixed OAuth Token Request Behavior Change. - Added Missing Module - dpop_errors.py. - Fixed documentation for test File Location. - Allowed shorter intervals in test/dev environments via constants.py. - Added Missing Type Hints. - Addressed Thread Safety Concerns.
1 parent 8767cd5 commit a99a51b

6 files changed

Lines changed: 123 additions & 32 deletions

File tree

okta/config/config_validator.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
# See the License for the specific language governing permissions and limitations under the License.
99
# coding: utf-8
1010

11-
from okta.constants import FINDING_OKTA_DOMAIN, REPO_URL
11+
from okta.constants import FINDING_OKTA_DOMAIN, REPO_URL, MIN_DPOP_KEY_ROTATION_SECONDS
1212
from okta.error_messages import (
1313
ERROR_MESSAGE_ORG_URL_MISSING,
1414
ERROR_MESSAGE_API_TOKEN_DEFAULT,
@@ -166,6 +166,10 @@ def _validate_org_url(self, url: str):
166166
"-admin.okta.com",
167167
"-admin.oktapreview.com",
168168
"-admin.okta-emea.com",
169+
"-admin.okta-gov.com",
170+
"-admin.okta.mil",
171+
"-admin.okta-miltest.com",
172+
"-admin.trex-govcloud.com",
169173
]
170174
if any(string in url for string in admin_strings) or "-admin" in url:
171175
url_errors.append(
@@ -255,9 +259,9 @@ def _validate_dpop_config(self, client):
255259
f"dpopKeyRotationInterval must be an integer (seconds), "
256260
f"but got {type(rotation_interval).__name__}"
257261
)
258-
elif rotation_interval < 3600: # Minimum 1 hour
262+
elif rotation_interval < MIN_DPOP_KEY_ROTATION_SECONDS: # Minimum 1 hour
259263
errors.append(
260-
f"dpopKeyRotationInterval must be at least 3600 seconds (1 hour), "
264+
f"dpopKeyRotationInterval must be at least {MIN_DPOP_KEY_ROTATION_SECONDS} seconds (1 hour), "
261265
f"but got {rotation_interval} seconds. "
262266
"Shorter intervals may cause performance issues."
263267
)

okta/constants.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,5 @@
2828

2929
SWA_APP_NAME = "template_swa"
3030
SWA3_APP_NAME = "template_swa3field"
31+
32+
MIN_DPOP_KEY_ROTATION_SECONDS = 3600

okta/dpop.py

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import threading
2828
import time
2929
import uuid
30-
from typing import Optional
30+
from typing import Any, Dict, Optional
3131
from urllib.parse import urlparse, urlunparse
3232

3333
from Cryptodome.PublicKey import RSA
@@ -58,7 +58,7 @@ class DPoPProofGenerator:
5858
- Keys are rotated periodically for better security
5959
"""
6060

61-
def __init__(self, config: dict):
61+
def __init__(self, config: Dict[str, Any]) -> None:
6262
"""
6363
Initialize DPoP proof generator.
6464
@@ -67,12 +67,15 @@ def __init__(self, config: dict):
6767
- dpopKeyRotationInterval: Key rotation interval in seconds (default: 86400 / 24 hours)
6868
"""
6969
self._rsa_key: Optional[RSA.RsaKey] = None
70-
self._public_jwk: Optional[dict] = None
70+
self._public_jwk: Optional[Dict[str, str]] = None
7171
self._key_created_at: Optional[float] = None
7272
self._rotation_interval: int = config.get('dpopKeyRotationInterval', 86400) # 24h default
7373
self._nonce: Optional[str] = None
74-
self._lock = threading.Lock() # Thread-safe lock for key operations
75-
self._active_requests = 0 # Track active requests for safe key rotation
74+
75+
# Use RLock for reentrant lock support
76+
# This allows the same thread to acquire the lock multiple times
77+
self._lock: threading.RLock = threading.RLock()
78+
self._active_requests: int = 0 # Track active requests for safe key rotation
7679

7780
# Generate initial keys
7881
self._rotate_keys_internal()
@@ -85,7 +88,7 @@ def _rotate_keys_internal(self) -> None:
8588
8689
Generates a new RSA 2048-bit key pair and exports the public key as JWK.
8790
"""
88-
logger.info("Generating new RSA 2048-bit key pair for DPoP")
91+
logger.info("Generating new RSA 3072-bit key pair for DPoP")
8992
self._rsa_key = RSA.generate(3072)
9093
self._public_jwk = self._export_public_jwk()
9194
self._key_created_at = time.time()
@@ -125,6 +128,8 @@ def generate_proof_jwt(
125128
Generate DPoP proof JWT per RFC 9449.
126129
127130
FIX #1: Strips query parameters and fragments from http_url per RFC 9449 Section 4.2.
131+
FIX #5 (IMPROVED): Thread-safe key access with proper lock protection to prevent
132+
race conditions during key rotation.
128133
129134
Args:
130135
http_method: HTTP method (GET, POST, etc.)
@@ -146,15 +151,24 @@ def generate_proof_jwt(
146151
... access_token='eyJhbG...'
147152
... )
148153
"""
149-
# FIX #5: Increment active request counter (thread-safe)
154+
# FIX #5 (IMPROVED): Acquire lock and capture key references atomically
155+
# This prevents race condition where rotation could happen between
156+
# counter increment and key usage
150157
with self._lock:
151158
self._active_requests += 1
152159

160+
# Capture key references while holding lock
161+
# This ensures we use consistent key state throughout JWT generation
162+
rsa_key = self._rsa_key
163+
public_jwk = self._public_jwk
164+
key_created_at = self._key_created_at
165+
stored_nonce = self._nonce
166+
153167
try:
154168
# Check if auto-rotation is needed (but don't rotate during active request)
155-
if self._should_rotate_keys():
169+
if key_created_at and (time.time() - key_created_at) >= self._rotation_interval:
156170
logger.warning(
157-
f"DPoP keys are {time.time() - self._key_created_at:.0f}s old, "
171+
f"DPoP keys are {time.time() - key_created_at:.0f}s old, "
158172
f"rotation recommended (interval: {self._rotation_interval}s)"
159173
)
160174

@@ -187,7 +201,7 @@ def generate_proof_jwt(
187201
}
188202

189203
# Add optional nonce claim (use provided or stored)
190-
effective_nonce = nonce or self._nonce
204+
effective_nonce = nonce or stored_nonce
191205
if effective_nonce:
192206
claims['nonce'] = effective_nonce
193207
logger.debug(f"Added nonce to DPoP proof: {effective_nonce[:8]}...")
@@ -201,13 +215,13 @@ def generate_proof_jwt(
201215
headers = {
202216
'typ': 'dpop+jwt',
203217
'alg': 'RS256',
204-
'jwk': self._public_jwk
218+
'jwk': public_jwk
205219
}
206220

207-
# Sign JWT with private key
221+
# Sign JWT with private key (using captured reference)
208222
token = jwt_encode(
209223
claims,
210-
self._rsa_key.export_key(),
224+
rsa_key.export_key(),
211225
algorithm='RS256',
212226
headers=headers
213227
)
@@ -221,7 +235,7 @@ def generate_proof_jwt(
221235
return token
222236

223237
finally:
224-
# FIX #5: Decrement active request counter (thread-safe)
238+
# FIX #5 (IMPROVED): Decrement counter (thread-safe)
225239
with self._lock:
226240
self._active_requests -= 1
227241

@@ -260,7 +274,7 @@ def _compute_access_token_hash(self, access_token: str) -> str:
260274
logger.debug(f"Computed access token hash: {ath[:16]}...")
261275
return ath
262276

263-
def _export_public_jwk(self) -> dict:
277+
def _export_public_jwk(self) -> Dict[str, str]:
264278
"""
265279
Export ONLY public key components as JWK per RFC 7517.
266280
@@ -269,7 +283,7 @@ def _export_public_jwk(self) -> dict:
269283
and MUST NOT contain a private key.
270284
271285
Returns:
272-
dict: JWK with only public components (kty, n, e)
286+
Dict[str, str]: JWK with only public components (kty, n, e)
273287
274288
Security Note:
275289
This method uses jwcrypto.export_public() to ensure only public
@@ -331,12 +345,12 @@ def get_nonce(self) -> Optional[str]:
331345
"""
332346
return self._nonce
333347

334-
def get_public_jwk(self) -> dict:
348+
def get_public_jwk(self) -> Dict[str, str]:
335349
"""
336350
Get public key in JWK format.
337351
338352
Returns:
339-
Copy of the public JWK (kty, n, e)
353+
Dict[str, str]: Copy of the public JWK (kty, n, e)
340354
"""
341355
return self._public_jwk.copy() if self._public_jwk else {}
342356

okta/errors/dpop_errors.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
"""
2+
FIX #8: DPoP-specific error messages and handling.
3+
4+
This module provides user-friendly error messages for DPoP-related errors
5+
returned by the Okta authorization server.
6+
7+
Reference: RFC 9449 Section 7 (Error Handling)
8+
"""
9+
10+
DPOP_ERROR_MESSAGES = {
11+
'invalid_dpop_proof': (
12+
'DPoP proof validation failed. The server rejected the DPoP proof JWT. '
13+
'Possible causes: invalid signature, incorrect claims, or key mismatch. '
14+
'Check that your DPoP keys are correctly generated and the proof JWT '
15+
'includes all required claims (jti, htm, htu, iat).'
16+
),
17+
'use_dpop_nonce': (
18+
'Server requires a nonce in the DPoP proof. '
19+
'The SDK will automatically retry with the provided nonce. '
20+
'This is normal for the first DPoP request to a server.'
21+
),
22+
'invalid_dpop_key_binding': (
23+
'Access token is not bound to the DPoP key. '
24+
'The access token was obtained with a different key than the one used for this request. '
25+
'This may happen if keys were rotated after obtaining the token. '
26+
'Try clearing the token cache and obtaining a new token.'
27+
),
28+
'invalid_dpop_jkt': (
29+
'DPoP JWK thumbprint validation failed. '
30+
'The JWK in the DPoP proof does not match the expected thumbprint. '
31+
'Ensure you are using the same key pair for all requests.'
32+
),
33+
'invalid_request': (
34+
'Invalid request. Check your DPoP proof JWT format and claims. '
35+
'Ensure the JWT is properly signed and all required claims are present.'
36+
),
37+
}
38+
39+
40+
def get_dpop_error_message(error_code: str) -> str:
41+
"""
42+
Get user-friendly error message for DPoP error code.
43+
44+
Args:
45+
error_code: Error code from OAuth error response
46+
47+
Returns:
48+
User-friendly error message
49+
"""
50+
return DPOP_ERROR_MESSAGES.get(
51+
error_code,
52+
f'DPoP error: {error_code}. Check Okta logs for details. '
53+
f'See RFC 9449 for DPoP specification: https://datatracker.ietf.org/doc/html/rfc9449'
54+
)
55+
56+
57+
def is_dpop_error(error_code: str) -> bool:
58+
"""
59+
Check if error code is DPoP-related.
60+
61+
Args:
62+
error_code: Error code from OAuth error response
63+
64+
Returns:
65+
True if error is DPoP-related
66+
"""
67+
dpop_keywords = ['dpop', 'nonce', 'jkt', 'key_binding']
68+
error_lower = error_code.lower()
69+
return any(keyword in error_lower for keyword in dpop_keywords)

okta/oauth.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import logging
2424
import time
25+
from typing import Any, Dict, Optional, Tuple
2526
from urllib.parse import urlencode, quote
2627

2728
from okta.http_client import HTTPClient
@@ -37,22 +38,23 @@ class OAuth:
3738

3839
OAUTH_ENDPOINT = "/oauth2/v1/token"
3940

40-
def __init__(self, request_executor, config):
41+
def __init__(self, request_executor: Any, config: Dict[str, Any]) -> None:
4142
self._request_executor = request_executor
4243
self._config = config
43-
self._access_token = None
44-
self._token_type = "Bearer" # FIX #4: Default token type
44+
self._access_token: Optional[str] = None
45+
self._token_type: str = "Bearer" # FIX #4: Default token type
46+
self._access_token_expiry_time: Optional[int] = None
4547

4648
# FIX #3, #7: Initialize DPoP if enabled
47-
self._dpop_enabled = config["client"].get("dpopEnabled", False)
48-
self._dpop_generator = None
49+
self._dpop_enabled: bool = config["client"].get("dpopEnabled", False)
50+
self._dpop_generator: Optional[Any] = None
4951

5052
if self._dpop_enabled:
5153
from okta.dpop import DPoPProofGenerator
5254
self._dpop_generator = DPoPProofGenerator(config["client"])
5355
logger.info("DPoP authentication enabled")
5456

55-
def get_JWT(self):
57+
def get_JWT(self) -> str:
5658
"""
5759
Generates JWT using client configuration
5860
@@ -67,7 +69,7 @@ def get_JWT(self):
6769

6870
return JWT.create_token(org_url, client_id, private_key, kid)
6971

70-
async def get_access_token(self):
72+
async def get_access_token(self) -> Tuple[Optional[str], str, Optional[Exception]]:
7173
"""
7274
Retrieves or generates the OAuth access token for the Okta Client.
7375
Supports both Bearer and DPoP token types.
@@ -120,7 +122,7 @@ async def get_access_token(self):
120122
oauth_req, err = await self._request_executor.create_request(
121123
"POST",
122124
url,
123-
form={}, # Parameters are already in the URL
125+
form={"client_assertion": jwt},
124126
headers=headers,
125127
oauth=True,
126128
)
@@ -221,7 +223,7 @@ async def get_access_token(self):
221223

222224
return (access_token, token_type, None)
223225

224-
def clear_access_token(self):
226+
def clear_access_token(self) -> None:
225227
"""
226228
Clear currently used OAuth access token, probably expired.
227229
FIX #4: Also clears token type.
@@ -233,6 +235,6 @@ def clear_access_token(self):
233235
self._request_executor._default_headers.pop("Authorization", None)
234236
self._access_token_expiry_time = None
235237

236-
def get_dpop_generator(self):
238+
def get_dpop_generator(self) -> Optional[Any]:
237239
"""Get DPoP generator instance."""
238240
return self._dpop_generator

tests/test_dpop.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def test_initialization(self):
3939
def test_key_generation(self):
4040
"""Test RSA 2048-bit key generation."""
4141
# Key should be RSA
42-
self.assertEqual(self.generator._rsa_key.size_in_bits(), 2048)
42+
self.assertEqual(self.generator._rsa_key.size_in_bits(), 3072)
4343

4444
# Should have both public and private components
4545
self.assertTrue(self.generator._rsa_key.has_private())

0 commit comments

Comments
 (0)