Repository navigation
fix(auth): make token refresh and session restore work #210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a6c72aa
fb6e515
017e0a7
fb1f1e6
a702f7a
4377420
6a5aaf7
02992c5
5ca63bb
ae0af59
9b27aca
173404f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,7 +120,13 @@ def _initialize_framework(self) -> None: | |
| self._storage = StorageFactory.create_storage({"type": self.framework}) | ||
|
|
||
| # Initialize storage manager with the framework-specific storage | ||
| self._storage_manager.initialize(config={"type": self.framework, "device_id": self._framework.get_name()}, storage=self._storage) | ||
| # device_id must be passed as an argument: inside config it is ignored and a random | ||
| # per-process id is used, so sessions can't be found after a restart or on another worker | ||
| self._storage_manager.initialize( | ||
| config={"type": self.framework}, | ||
| device_id=self._framework.get_name(), | ||
| storage=self._storage, | ||
| ) | ||
|
Comment on lines
120
to
+129
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| def _initialize_null_framework(self, storage_config: Dict[str, Any]) -> None: | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import time | ||
| import requests | ||
| from kinde_sdk.core.exceptions import KindeTokenPersistenceException | ||
| import threading | ||
| import logging | ||
| from typing import Any, Dict, Optional | ||
|
|
@@ -33,9 +34,13 @@ def __init__(self, user_id, client_id, client_secret, token_url): | |
| self.client_secret = client_secret | ||
| self.token_url = token_url | ||
| self.tokens = {} # Store tokens (access/refresh) | ||
| self.lock = threading.Lock() # Add a lock for thread safety | ||
| # Reentrant: get_access_token() holds the lock while a refresh calls set_tokens() | ||
| self.lock = threading.RLock() | ||
| self.redirect_uri = None # Initialize the redirect_uri attribute | ||
| self.force_api = False # Initialize force_api setting | ||
| self.on_tokens_refreshed = None # Called after a successful refresh, e.g. to persist tokens | ||
| self.load_persisted_tokens = None # Returns the saved tokens, which another process may have refreshed | ||
| self._save_pending = False # A refresh succeeded but saving its tokens failed | ||
| self.initialized = True | ||
|
|
||
| def set_force_api(self, force_api: bool): | ||
|
|
@@ -124,18 +129,84 @@ def get_access_token(self): | |
| with self.lock: | ||
| if not self.tokens or "access_token" not in self.tokens: | ||
| raise ValueError("No access token available") | ||
|
|
||
|
|
||
| # Save tokens from an earlier refresh before deciding whether to refresh | ||
| # again. Refresh tokens rotate, so storage must not keep the old one: if | ||
| # this save fails and a refresh below fails too, a restart could not | ||
| # recover the session. A failed save keeps the refreshed tokens in memory. | ||
| save_error = None | ||
| if self._save_pending: | ||
| try: | ||
| self._save_refreshed_tokens() | ||
| except KindeTokenPersistenceException as e: | ||
| save_error = e | ||
|
|
||
| # Check if token is expired | ||
| # if time.time() >= self.tokens["expires_at"]: | ||
| if time.time() >= self.tokens.get("expires_at", 0): | ||
| # Another worker may already have refreshed and saved newer tokens | ||
| self._adopt_newer_persisted_tokens() | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| if time.time() < self.tokens.get("expires_at", 0): | ||
| return self.tokens["access_token"] | ||
|
|
||
| # Try to refresh token if available | ||
| if "refresh_token" in self.tokens: | ||
| return self.refresh_access_token() | ||
| try: | ||
| return self.refresh_access_token() | ||
| except requests.RequestException as e: | ||
| self._log_refresh_failure(e) | ||
| raise | ||
| else: | ||
| raise ValueError("Access token expired and no refresh token available") | ||
|
|
||
|
|
||
| if save_error is not None: | ||
| raise save_error | ||
|
Comment on lines
+162
to
+163
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| return self.tokens["access_token"] | ||
|
|
||
| def _save_refreshed_tokens(self) -> None: | ||
| """Save the tokens from a refresh. If saving fails, the refreshed tokens are kept | ||
| and the save is retried on the next call, without refreshing again.""" | ||
| if self.on_tokens_refreshed: | ||
| try: | ||
| self.on_tokens_refreshed() | ||
| except Exception as e: | ||
| logging.getLogger(__name__).warning( | ||
| "Refreshed tokens could not be saved: %s", type(e).__name__ | ||
| ) | ||
| raise KindeTokenPersistenceException( | ||
| "Refreshed tokens could not be saved", access_token=self.tokens["access_token"] | ||
| ) from e | ||
| self._save_pending = False | ||
|
|
||
| def _adopt_newer_persisted_tokens(self) -> None: | ||
| """Use tokens that another process refreshed and saved, so this process | ||
| doesn't refresh with a refresh token Kinde has already rotated out.""" | ||
| if not self.load_persisted_tokens: | ||
| return | ||
| try: | ||
| persisted = self.load_persisted_tokens() | ||
| except Exception as e: | ||
| logging.getLogger(__name__).warning("Saved tokens could not be read: %s", type(e).__name__) | ||
| return | ||
| if (persisted and persisted.get("access_token") | ||
| and persisted.get("expires_at", 0) > self.tokens.get("expires_at", 0)): | ||
| self.tokens = dict(persisted) | ||
| self._save_pending = False # The saved tokens are newer than any unsaved ones | ||
|
|
||
| def _log_refresh_failure(self, error: Exception) -> None: | ||
| """Log why a refresh failed, without logging any token values.""" | ||
| response = getattr(error, "response", None) | ||
| if response is None: | ||
| logging.getLogger(__name__).warning("Token refresh failed: %s", type(error).__name__) | ||
| return | ||
| try: | ||
| error_code = response.json().get("error") | ||
| except Exception: | ||
| error_code = None | ||
| logging.getLogger(__name__).warning( | ||
| "Token refresh failed: HTTP %s (%s)", response.status_code, error_code or "no error code" | ||
| ) | ||
|
|
||
| def refresh_access_token(self): | ||
| """ Use the refresh token to get a new access token. """ | ||
| if "refresh_token" not in self.tokens: | ||
|
|
@@ -154,8 +225,12 @@ def refresh_access_token(self): | |
| response = requests.post(self.token_url, data=data) | ||
| response.raise_for_status() | ||
| token_data = response.json() | ||
|
|
||
| self.set_tokens(token_data) | ||
| # Save here, not in get_access_token(), so direct callers also persist the rotated refresh token | ||
| with self.lock: | ||
| self._save_pending = True | ||
| self._save_refreshed_tokens() | ||
| return self.tokens["access_token"] | ||
|
|
||
| def get_id_token(self): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.