sdks/implementations/python/src/stack_auth/_jwt.py (2)

50-53: ⚠️ Potential issue | 🟠 Major

Deduplicate forced JWKS refresh on missing kid to prevent fetch amplification.

Line 52 and Line 103 unconditionally call force=True; with concurrent misses, each request can trigger another upstream JWKS call. Gate forced refresh once per observed cache snapshot.

💡 Proposed fix

 class AsyncJWKSFetcher:
@@
     def __init__(self, jwks_url: str, http_client: httpx.AsyncClient) -> None:
@@
         self._fetch_lock = asyncio.Lock()
+        self._refresh_lock = asyncio.Lock()
@@
     async def get_signing_key(self, kid: str) -> Any:
@@
         if key_data is None:
-            # Force-refresh once for potential key rotation
-            jwks = await self._fetch_jwks(force=True)
+            observed_cache_time = self._cache_time
+            async with self._refresh_lock:
+                if self._cache_time == observed_cache_time:
+                    await self._fetch_jwks(force=True)
+            jwks = await self._fetch_jwks()
             key_data = _find_key(jwks, kid)
@@
 class SyncJWKSFetcher:
@@
     def __init__(self, jwks_url: str, http_client: httpx.Client) -> None:
@@
         self._fetch_lock = threading.Lock()
+        self._refresh_lock = threading.Lock()
@@
     def get_signing_key(self, kid: str) -> Any:
@@
         if key_data is None:
-            jwks = self._fetch_jwks(force=True)
+            observed_cache_time = self._cache_time
+            with self._refresh_lock:
+                if self._cache_time == observed_cache_time:
+                    self._fetch_jwks(force=True)
+            jwks = self._fetch_jwks()
             key_data = _find_key(jwks, kid)

Also applies to: 102-104

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_jwt.py` around lines 50 - 53, The
code unconditionally calls self._fetch_jwks(force=True) on a missing key, which
can cause N concurrent forced refreshes; change the logic in the JWT lookup (the
block that checks key_data and uses _find_key and _fetch_jwks) to ensure the
forced JWKS refresh runs at most once per observed cache snapshot: read the
current cache/version token, if key_data is None atomically check a
refreshed-flag or compare the cache token and only call _fetch_jwks(force=True)
when this request is the first to observe the stale snapshot, then re-run
_find_key against the refreshed jwks; keep references to _find_key, _fetch_jwks,
key_data and kid to locate the code.

---

73-75: ⚠️ Potential issue | 🟠 Major

Validate JWKS response shape before caching, and harden key iteration.

Line 73 and Line 124 cache unvalidated JSON, while Line 132 assumes each entry supports .get. Malformed JWKS can raise AttributeError and bypass the intended unauthenticated fallback path.

💡 Proposed fix

     async def _fetch_jwks(self, force: bool = False) -> dict[str, Any]:
@@
-            self._cache = response.json()
+            data = response.json()
+            if not isinstance(data, dict) or not isinstance(data.get("keys"), list):
+                raise ValueError("Invalid JWKS response format")
+            self._cache = data
             self._cache_time = time.monotonic()
             return self._cache  # type: ignore[return-value]
@@
     def _fetch_jwks(self, force: bool = False) -> dict[str, Any]:
@@
-            self._cache = response.json()
+            data = response.json()
+            if not isinstance(data, dict) or not isinstance(data.get("keys"), list):
+                raise ValueError("Invalid JWKS response format")
+            self._cache = data
             self._cache_time = time.monotonic()
             return self._cache  # type: ignore[return-value]
@@
 def _find_key(jwks: dict[str, Any], kid: str) -> dict[str, Any] | None:
     """Find a key by kid in a JWKS key set."""
-    for key in jwks.get("keys", []):
-        if key.get("kid") == kid:
+    keys = jwks.get("keys")
+    if not isinstance(keys, list):
+        return None
+    for key in keys:
+        if isinstance(key, dict) and key.get("kid") == kid:
             return key  # type: ignore[no-any-return]
     return None

Also applies to: 124-126, 129-133

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_jwt.py` around lines 73 - 75, The
code currently assigns response.json() directly to self._cache and later
iterates assuming each entry has .get, which can raise AttributeError on
malformed JWKS; instead, validate the JWKS shape before caching: ensure
response.json() is a dict with a "keys" key whose value is a list, and that each
item in that list is a dict (use isinstance checks). Update the assignment sites
that set self._cache (the lines with "self._cache = response.json()") to only
set and set self._cache_time when validation passes; if validation fails, do not
cache and fall back to the unauthenticated path. Also harden the key iteration
code that currently calls item.get(...) to first confirm item is a dict and use
item.get safely (or skip non-dict items), and add an explanatory debug/error log
when skipping invalid shapes.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

sdks/implementations/python/src/stack_auth/_client.py (1)

120-138: ⚠️ Potential issue | 🟠 Major

Support HTTP-date Retry-After values.

This helper still only accepts numeric seconds. A valid HTTP-date header falls back to exponential backoff here, so 429 retries can run earlier than the server requested.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_client.py` around lines 120 -
138, In _get_retry_delay, support HTTP-date Retry-After values in addition to
numeric seconds: when reading response.headers.get("Retry-After") inside
_get_retry_delay (and in the x-stack-actual-status 429 branch) try parsing the
header first as a float and return that; if float parsing fails, parse it as an
HTTP-date (e.g., using a timezone-aware parser like
email.utils.parsedate_to_datetime or equivalent), compute max(0, (parsed_date -
now).total_seconds()), and return that; ensure exceptions fall through to the
exponential backoff fallback and that the computed delay is non-negative.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

sdks/implementations/python/src/stack_auth/_client.py (1)

260-264: Minor: Potential race in lazy client initialization.

In async contexts, multiple coroutines calling _get_client() simultaneously could both see _client is None and create separate AsyncClient instances. The impact is minor (just wasted resources; functionality is correct), but if you want to guard against this:

Optional fix using asyncio.Lock

 class AsyncAPIClient(BaseAPIClient[httpx.AsyncClient]):
     """Asynchronous HTTP client using :class:`httpx.AsyncClient`."""
+    
+    def __init__(self, **kwargs: Any) -> None:
+        super().__init__(**kwargs)
+        self._client_lock = asyncio.Lock()

-    def _get_client(self) -> httpx.AsyncClient:
+    async def _get_client(self) -> httpx.AsyncClient:
         """Return the underlying httpx.AsyncClient, creating it lazily if needed."""
-        if self._client is None:
-            self._client = httpx.AsyncClient(timeout=httpx.Timeout(30.0))
+        if self._client is None:
+            async with self._client_lock:
+                if self._client is None:
+                    self._client = httpx.AsyncClient(timeout=httpx.Timeout(30.0))
         return self._client

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_client.py` around lines 260 -
264, The lazy initialization in _get_client can race in async code; make it safe
by adding an asyncio.Lock instance (e.g. self._client_lock = asyncio.Lock()) in
the class __init__, change _get_client to an async def that acquires the lock
(await self._client_lock) and uses a double-checked pattern to create
self._client only if still None, then release the lock and return the client;
update callers to await _get_client() or alternatively use an async helper
get_client that wraps this behavior so only one AsyncClient is ever created
concurrently.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

sdks/implementations/python/src/stack_auth/_token_store.py (2)

19-19: Optional: Import from collections.abc for modern Python.

Since Python 3.10+ is required, Awaitable and Callable should be imported from collections.abc rather than typing per PEP 585.

♻️ Suggested change

-from typing import Any, Awaitable, Callable, Literal, Union
+from collections.abc import Awaitable, Callable
+from typing import Any, Literal, Union

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_token_store.py` at line 19,
Update the imports to use the modern collections.abc locations for
runtime/generic types: replace importing Awaitable and Callable from typing with
importing them from collections.abc while keeping Any, Literal, Union from
typing; modify the top-level import line that currently mentions Awaitable and
Callable so it imports Awaitable and Callable from collections.abc and the other
types remain from typing to satisfy PEP 585 and Python 3.10+ expectations (refer
to symbols Awaitable, Callable in _token_store.py).

---

222-222: Optional: Use X | Y union syntax for type annotations.

With Python 3.10+ as the minimum version, the modern union syntax is preferred.

♻️ Suggested change

-TokenStoreInit = Union[Literal["memory"], dict, RequestLike, None]
+TokenStoreInit = Literal["memory"] | dict | RequestLike | None

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_token_store.py` at line 222, The
type alias TokenStoreInit currently uses typing.Union; update it to the modern
Python 3.10+ union syntax by replacing "TokenStoreInit =
Union[Literal[\"memory\"], dict, RequestLike, None]" with "TokenStoreInit =
Literal[\"memory\"] | dict | RequestLike | None", and remove the now-unused
Union import if it isn't referenced elsewhere in the module; keep Literal and
RequestLike imports as needed.

sdks/implementations/python/src/stack_auth/_token_store.py (2)

392-397: Returning (True, None) when access_token is missing could be misleading.

If the server returns HTTP 200 but the response lacks an access_token field (or it's None), the function returns (True, None). While the caller handles this correctly by checking was_valid and new_access is not None, the True suggests the refresh was "valid" when it arguably wasn't.

Consider treating a missing access_token as a failure:

♻️ Suggested refinement

         if resp.status_code == 200:
             data = resp.json()
             if not isinstance(data, dict):
                 return (False, None)
-            return (True, data.get("access_token"))
+            access_token = data.get("access_token")
+            if not isinstance(access_token, str):
+                return (False, None)
+            return (True, access_token)
         return (False, None)

Also applies to lines 425-430 (async variant).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_token_store.py` around lines 392
- 397, The code currently returns (True, None) when resp.status_code == 200 but
data.get("access_token") is missing; change the logic so that a missing or None
access_token is treated as a failure and the function returns (False, None)
instead of (True, None). Update the synchronous branch that inspects
resp.status_code/resp.json() and the analogous asynchronous branch (the async
variant handling resp.status_code and data.get("access_token")) to check that
access_token is present before returning True, and return False when it's
absent.

---

19-20: Consider moving type-only imports to TYPE_CHECKING block.

Since from __future__ import annotations is active, Awaitable and Callable are only needed for static type checking and can be moved to a TYPE_CHECKING block to avoid unnecessary runtime imports.

♻️ Suggested refactor

+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from collections.abc import Awaitable, Callable
-from collections.abc import Awaitable, Callable
 from typing import Any, Literal

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_token_store.py` around lines 19 -
20, Move the type-only imports into a TYPE_CHECKING guard: add "from typing
import TYPE_CHECKING" to the top and wrap the Awaitable and Callable imports in
an "if TYPE_CHECKING:" block so they are only imported for static type checking
(leave Any and Literal as-is if they are used at runtime). This targets the
current import lines referencing Awaitable and Callable (and works with the
existing "from __future__ import annotations") to avoid unnecessary runtime
imports.

sdks/implementations/python/src/stack_auth/_token_store.py (1)

19-20: Consider moving type-only imports into a TYPE_CHECKING block.

Static analysis (Ruff TC003) flags Awaitable and Callable as imports used only for type annotations. Moving them into a TYPE_CHECKING block avoids importing them at runtime.

♻️ Suggested refactor

+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from collections.abc import Awaitable, Callable
-from collections.abc import Awaitable, Callable
 from typing import Any, Literal

Then update the type annotations on lines 303 and 338 to use string quotes if needed (though Python 3.10+ with from __future__ import annotations already defers evaluation, so this may work as-is).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_token_store.py` around lines 19 -
20, Move the runtime imports of Awaitable and Callable into a TYPE_CHECKING
block to avoid importing them at runtime: import TYPE_CHECKING from typing, wrap
"from collections.abc import Awaitable, Callable" inside "if TYPE_CHECKING:
...". Then update the annotations in this module that reference Awaitable and
Callable to either remain as normal (if using PEP 563 / from __future__ import
annotations) or convert them to forward-ref string literals so they are only
used for type checking; ensure references to Awaitable and Callable in the
module's type annotations (the places using those types) are adjusted
accordingly.

sdks/implementations/python/src/stack_auth/_client.py (1)

90-97: Extract x-stack-actual-status parsing into a shared helper to prevent drift.

The same parsing/fallback logic appears in _parse_response, SyncAPIClient.request, and AsyncAPIClient.request. Centralizing it in one helper will reduce maintenance risk and keep sync/async behavior locked together.

♻️ Proposed refactor

 class BaseAPIClient(Generic[HttpxClientT]):
@@
+    `@staticmethod`
+    def _get_actual_status(response: httpx.Response) -> int:
+        actual_status_header = response.headers.get("x-stack-actual-status")
+        if actual_status_header:
+            try:
+                return int(actual_status_header)
+            except ValueError:
+                return response.status_code
+        return response.status_code
+
     def _parse_response(self, response: httpx.Response) -> tuple[int, dict[str, Any] | None]:
@@
-        actual_status_header = response.headers.get("x-stack-actual-status")
-        if actual_status_header:
-            try:
-                actual_status = int(actual_status_header)
-            except ValueError:
-                actual_status = response.status_code
-        else:
-            actual_status = response.status_code
+        actual_status = self._get_actual_status(response)
@@
-                actual_status_hdr = resp.headers.get("x-stack-actual-status")
-                try:
-                    actual_status = int(actual_status_hdr) if actual_status_hdr else resp.status_code
-                except ValueError:
-                    actual_status = resp.status_code
+                actual_status = self._get_actual_status(resp)
@@
-                actual_status_hdr = resp.headers.get("x-stack-actual-status")
-                try:
-                    actual_status = int(actual_status_hdr) if actual_status_hdr else resp.status_code
-                except ValueError:
-                    actual_status = resp.status_code
+                actual_status = self._get_actual_status(resp)

Also applies to: 210-215, 312-317

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sdks/implementations/python/src/stack_auth/_client.py` around lines 90 - 97,
Extract the x-stack-actual-status parsing/fallback logic into a single helper
(e.g., _parse_actual_status(headers) or parse_actual_status) and replace the
duplicated blocks in _parse_response, SyncAPIClient.request, and
AsyncAPIClient.request with calls to that helper; the helper should accept
response.headers (or headers mapping), try to read and int() the
"x-stack-actual-status" header, and on ValueError or missing header return
response.status_code (or accept a fallback status argument) so both sync and
async code paths use the same logic.