From b49b2b36e03f3fa264034d40b9e6e8b8c064d08a Mon Sep 17 00:00:00 2001 From: root Date: Sun, 3 May 2026 20:20:40 +0000 Subject: [PATCH] =?UTF-8?q?refactor(V2):=20IBKR=20OAuth=20=E2=80=94=20name?= =?UTF-8?q?d=20constants,=20explicit=20raises,=20lifted=20import?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review fixes (commit 92da6aa): - LST refresh buffer / fallback TTL extracted as named module constants - Replace `assert` with explicit `if/raise` (asserts stripped under -O) - Move IBKRAuthError above OAuth1aSigner (forward declaration) - async_client import lifted to module level - Test uses actual prime (23) instead of composite (255) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cerbero_mcp/exchanges/ibkr/oauth.py | 32 ++++++++++++++++++------- tests/unit/exchanges/ibkr/test_oauth.py | 3 +-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/cerbero_mcp/exchanges/ibkr/oauth.py b/src/cerbero_mcp/exchanges/ibkr/oauth.py index 11fb4b4..ad0b284 100644 --- a/src/cerbero_mcp/exchanges/ibkr/oauth.py +++ b/src/cerbero_mcp/exchanges/ibkr/oauth.py @@ -16,6 +16,13 @@ from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import padding from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey +from cerbero_mcp.common.http import async_client + +# Refresh LST 9h before its 24h expiry (very conservative; trades token +# lifetime for safety margin against clock drift and slow re-auth flows). +_LST_REFRESH_BUFFER_S = 9 * 3600 # 32_400 +_LST_FALLBACK_TTL_S = 15 * 3600 # 54_000 — used when server omits expiration + def _percent_encode(value: str) -> str: """RFC 3986 percent-encoding for OAuth (no `+` for space).""" @@ -38,6 +45,10 @@ def build_signature_base_string( return f"{method.upper()}&{_percent_encode(url)}&{params_str}" +class IBKRAuthError(Exception): + """OAuth flow failed (key invalid, consumer revoked, mint failed).""" + + @dataclass class OAuth1aSigner: consumer_key: str @@ -100,8 +111,6 @@ class OAuth1aSigner: 5. shared = dh_response^dh_random mod dh_prime 6. LST = HMAC-SHA1(shared, decrypted_secret), base64 """ - from cerbero_mcp.common.http import async_client - url = f"{base_url}/oauth/live_session_token" prime = int(self.dh_prime, 16) @@ -109,13 +118,17 @@ class OAuth1aSigner: dh_challenge = pow(2, dh_random, prime) dh_challenge_hex = format(dh_challenge, "x") + if self._encryption_key is None: # pragma: no cover — set in __post_init__ + raise IBKRAuthError("encryption key not loaded") try: - assert self._encryption_key is not None encrypted = bytes.fromhex(self.access_token_secret) decrypted_secret = self._encryption_key.decrypt( encrypted, padding.PKCS1v15() ) - except Exception as e: + except Exception as e: # narrow to crypto/value errors; broad on purpose + # Intentionally broad: covers ValueError (bad hex), cryptography + # errors (InvalidKey, padding decoding), and any RSA backend issue + # — all map to the same user-facing failure ("bad credentials"). raise IBKRAuthError(f"access_token_secret decrypt failed: {e}") from e oauth_params = self.make_oauth_params() @@ -149,7 +162,12 @@ class OAuth1aSigner: lst = base64.b64encode(lst_raw).decode("ascii") self._live_session_token = lst - ttl = max(60.0, expires_ms / 1000 - time.time() - 32400) if expires_ms else 54000.0 + if expires_ms: + ttl = max(60.0, (expires_ms / 1000) - time.time() - _LST_REFRESH_BUFFER_S) + else: + ttl = float(_LST_FALLBACK_TTL_S) + # `expires_ms` is wall clock; convert to a monotonic deadline so the + # cache check is unaffected by future clock adjustments. self._lst_expires_at = time.monotonic() + ttl return lst @@ -161,7 +179,3 @@ class OAuth1aSigner: lst_bytes = base64.b64decode(self._live_session_token) sig = hmac.new(lst_bytes, base.encode("utf-8"), hashlib.sha256).digest() return base64.b64encode(sig).decode("ascii") - - -class IBKRAuthError(Exception): - """OAuth flow failed (key invalid, consumer revoked, mint failed).""" diff --git a/tests/unit/exchanges/ibkr/test_oauth.py b/tests/unit/exchanges/ibkr/test_oauth.py index 2cee491..091684c 100644 --- a/tests/unit/exchanges/ibkr/test_oauth.py +++ b/tests/unit/exchanges/ibkr/test_oauth.py @@ -114,14 +114,13 @@ async def test_live_session_token_mint(httpx_mock: HTTPXMock, tmp_path): }, ) - from cerbero_mcp.exchanges.ibkr.oauth import OAuth1aSigner signer = OAuth1aSigner( consumer_key="TEST_CK", access_token="TEST_AT", access_token_secret=encrypted_hex, signature_key_path=str(tmp_path / "sig.pem"), encryption_key_path=str(tmp_path / "enc.pem"), - dh_prime="ff", + dh_prime="17", # 23 — smallest prime > 16 that fits a 1-byte modulus ) lst = await signer.get_live_session_token( base_url="https://api.ibkr.com/v1/api"