From 7e539fbf8afc8751b61dd5d60d203abab5fb6b3c Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:30:23 +0200 Subject: [PATCH 01/20] fix: stop leaking secrets through logs and session storage - Stop persisting client_secret in each user's session. OAuth now keeps it in an in-process UserSession.client_secrets map keyed by client_id, so sessions restored from storage still get the secret for refresh/revoke. - Remove log lines that emitted the token-exchange payload (incl. client_secret, code, verifier) and response body, the OAuth state at WARNING, the whole FastAPI session at WARNING, user details at INFO, and session values in the framework storage debug logs. - Drop logging.getLogger("kinde_sdk").setLevel(INFO) calls; a library should not set its own log level. - Log normal framework registration at DEBUG instead of WARNING. - Add a 30s REQUEST_TIMEOUT to all requests calls that had no timeout. --- kinde_fastapi/framework/fastapi_framework.py | 1 - kinde_flask/storage/flask_storage_factory.py | 2 +- kinde_sdk/auth/base_auth.py | 1 - kinde_sdk/auth/oauth.py | 21 +++----- kinde_sdk/auth/portals.py | 1 - kinde_sdk/auth/token_manager.py | 8 +-- kinde_sdk/auth/user_session.py | 8 ++- kinde_sdk/core/framework/framework_factory.py | 4 +- kinde_sdk/core/helpers.py | 18 ++++--- .../core/storage/framework_aware_storage.py | 6 +-- testv2/testv2_auth/test_session_secrets.py | 50 +++++++++++++++++++ testv2/testv2_core/test_helpers.py | 31 ++++++++---- 12 files changed, 105 insertions(+), 46 deletions(-) create mode 100644 testv2/testv2_auth/test_session_secrets.py diff --git a/kinde_fastapi/framework/fastapi_framework.py b/kinde_fastapi/framework/fastapi_framework.py index 04fb09d0..3d45c165 100644 --- a/kinde_fastapi/framework/fastapi_framework.py +++ b/kinde_fastapi/framework/fastapi_framework.py @@ -139,7 +139,6 @@ async def login(request: Request): login_options['invitation_code'] = invitation_code url = await self._oauth.login(login_options) - self._logger.warning(f"[Login] Session is: {request.session}") return RedirectResponse(url=url) # Callback route diff --git a/kinde_flask/storage/flask_storage_factory.py b/kinde_flask/storage/flask_storage_factory.py index 78ab8d99..7ecc62b4 100644 --- a/kinde_flask/storage/flask_storage_factory.py +++ b/kinde_flask/storage/flask_storage_factory.py @@ -27,7 +27,7 @@ def get(self, key: str) -> Optional[Dict]: session = self._get_session() if session is not None: value = session.get(key) - logger.debug(f"Getting key '{key}' from session: {value}") + logger.debug(f"Getting key '{key}' from session") return value return None diff --git a/kinde_sdk/auth/base_auth.py b/kinde_sdk/auth/base_auth.py index 51e492ac..8f98ec47 100644 --- a/kinde_sdk/auth/base_auth.py +++ b/kinde_sdk/auth/base_auth.py @@ -11,7 +11,6 @@ class BaseAuth: def __init__(self): self._logger = logging.getLogger("kinde_sdk") - self._logger.setLevel(logging.INFO) self._framework = None self._session_manager = UserSession() diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index 0c2d19d1..5a9c28d2 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -12,7 +12,7 @@ from .config_loader import load_config from .enums import IssuerRouteTypes, PromptTypes from .login_options import LoginOptions -from kinde_sdk.core.helpers import generate_random_string, generate_pkce_pair, get_user_details as helper_get_user_details, get_user_details_sync +from kinde_sdk.core.helpers import REQUEST_TIMEOUT, generate_random_string, generate_pkce_pair, get_user_details as helper_get_user_details, get_user_details_sync from kinde_sdk.core.exceptions import ( KindeConfigurationException, KindeLoginException, @@ -60,6 +60,7 @@ def __init__( # Validate required configurations if not self.client_id: raise KindeConfigurationException("Client ID is required.") + UserSession.client_secrets[self.client_id] = self.client_secret # Initialize API endpoints self._set_api_endpoints() @@ -84,9 +85,7 @@ def __init__( self._session_manager = UserSession() - # Logging settings self._logger = logging.getLogger("kinde_sdk") - self._logger.setLevel(logging.INFO) # Authentication properties self.verify_ssl = True @@ -195,9 +194,7 @@ def get_user_info(self) -> Dict[str, Any]: token_manager=token_manager, logger=self._logger ) - - # Get claims from token manager - self._logger.info(f"Get the claims from the token manager {user_details}") + return user_details def _set_api_endpoints(self): @@ -221,7 +218,7 @@ def _fetch_openid_configuration(self): openid_config_url = f"{self.host}/.well-known/openid-configuration" # Make the request - response = requests.get(openid_config_url) + response = requests.get(openid_config_url, timeout=REQUEST_TIMEOUT) if response.status_code == 200: config = response.json() @@ -518,9 +515,8 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = # Verify state if provided if state: stored_state = self._session_manager.storage_manager.get("user:state") - self._logger.warning(f"stored_state: {stored_state}, state: {state}") if not stored_state or state != stored_state.get("value"): - self._logger.error(f"State mismatch: received {state}, stored {stored_state}") + self._logger.error("State mismatch in OAuth callback") raise KindeLoginException("Invalid state parameter") # Get code verifier for PKCE @@ -542,7 +538,6 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = # Store tokens user_info = { "client_id": self.client_id, - "client_secret": self.client_secret, "token_url": self.token_url, "redirect_uri": self.redirect_uri, } @@ -603,10 +598,8 @@ async def exchange_code_for_tokens(self, code: str, code_verifier: Optional[str] if code_verifier: data["code_verifier"] = code_verifier - self._logger.debug(f"[Exchange code for tokens] [{self.token_url}] [{data}]") - - response = requests.post(self.token_url, data=data) - self._logger.debug(f"[Exchange code for tokens] [{response.status_code}] [{response.text}]") + response = requests.post(self.token_url, data=data, timeout=REQUEST_TIMEOUT) + self._logger.debug(f"[Exchange code for tokens] [{response.status_code}]") if response.status_code != 200: raise KindeTokenException(f"Token exchange failed: {response.text}") diff --git a/kinde_sdk/auth/portals.py b/kinde_sdk/auth/portals.py index f61509d8..e0fdfe48 100644 --- a/kinde_sdk/auth/portals.py +++ b/kinde_sdk/auth/portals.py @@ -19,7 +19,6 @@ class PortalPage(str, Enum): class Portals: def __init__(self): self._logger = logging.getLogger("kinde_sdk") - self._logger.setLevel(logging.INFO) self._framework = None self._session_manager = UserSession() diff --git a/kinde_sdk/auth/token_manager.py b/kinde_sdk/auth/token_manager.py index 3411fc39..37d99b3e 100644 --- a/kinde_sdk/auth/token_manager.py +++ b/kinde_sdk/auth/token_manager.py @@ -5,6 +5,8 @@ from typing import Any, Dict, Optional import jwt +from kinde_sdk.core.helpers import REQUEST_TIMEOUT + class TokenManager: _instances = {} _lock = threading.Lock() # Add a lock for thread safety @@ -112,7 +114,7 @@ async def exchange_code_for_token(self, code: str, code_verifier: Optional[str] if code_verifier: data["code_verifier"] = code_verifier - response = requests.post(self.token_url, data=data) + response = requests.post(self.token_url, data=data, timeout=REQUEST_TIMEOUT) response.raise_for_status() token_data = response.json() @@ -151,7 +153,7 @@ def refresh_access_token(self): if self.client_secret: data["client_secret"] = self.client_secret - response = requests.post(self.token_url, data=data) + response = requests.post(self.token_url, data=data, timeout=REQUEST_TIMEOUT) response.raise_for_status() token_data = response.json() @@ -218,7 +220,7 @@ def revoke_token(self): data["client_secret"] = self.client_secret try: - response = requests.post(revoke_url, data=data) + response = requests.post(revoke_url, data=data, timeout=REQUEST_TIMEOUT) response.raise_for_status() except Exception: pass # Best effort revocation diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index 19d9ca07..a62610a0 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -5,6 +5,10 @@ from kinde_sdk.core.storage.storage_manager import StorageManager class UserSession: + # Keyed by client_id and populated by OAuth. Kept in process memory so the + # secret is never written to session storage (which may be a browser cookie). + client_secrets: Dict[str, Optional[str]] = {} + def __init__(self): self.user_sessions = {} # Store user-specific session data self.lock = threading.Lock() # Add a lock for thread safety @@ -18,7 +22,7 @@ def set_user_data(self, user_id: str, user_info: Dict[str, Any], token_data: Dic token_manager = TokenManager( user_id, user_info["client_id"], - user_info.get("client_secret"), # May be None for PKCE flow + self.client_secrets.get(user_info["client_id"]), # May be None for PKCE flow user_info["token_url"] ) @@ -87,7 +91,7 @@ def _load_from_storage(self, user_id: str) -> bool: token_manager = TokenManager( user_id, user_info.get("client_id"), - user_info.get("client_secret"), + self.client_secrets.get(user_info.get("client_id")), user_info.get("token_url") ) diff --git a/kinde_sdk/core/framework/framework_factory.py b/kinde_sdk/core/framework/framework_factory.py index 9c49bb43..58798828 100644 --- a/kinde_sdk/core/framework/framework_factory.py +++ b/kinde_sdk/core/framework/framework_factory.py @@ -39,7 +39,7 @@ def _discover_frameworks(cls) -> None: try: # Import the package importlib.import_module(name) - logger.warning(f"Successfully imported framework package: {name}") + logger.debug(f"Successfully imported framework package: {name}") except ImportError as e: logger.warning(f"Failed to import framework package {name}: {str(e)}") except Exception as e: @@ -59,7 +59,7 @@ def register_framework(cls, name: str, framework_class: Type[FrameworkInterface] """ with cls._lock: cls._frameworks[name] = framework_class - logger.warning(f"Registered framework: {name}") + logger.debug(f"Registered framework: {name}") @classmethod def get_framework_instance(cls) -> Optional[FrameworkInterface]: diff --git a/kinde_sdk/core/helpers.py b/kinde_sdk/core/helpers.py index b7a99996..4b1e1ca5 100644 --- a/kinde_sdk/core/helpers.py +++ b/kinde_sdk/core/helpers.py @@ -11,6 +11,8 @@ logger = logging.getLogger("kinde_sdk") +REQUEST_TIMEOUT = 30 # seconds + def generate_random_string(length: int = 32) -> str: """ Generate a random string of specified length. @@ -96,7 +98,7 @@ async def get_user_details(userinfo_url: str, token_manager, logger) -> Dict[str } # Make the request to userinfo endpoint - response = requests.get(userinfo_url, headers=headers) + response = requests.get(userinfo_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Return user profile data @@ -198,7 +200,7 @@ def get_user_organizations(api_url: str, token_manager, logger) -> List[Dict[str # Make the request to organizations endpoint orgs_url = f"{api_url}/user/organizations" - response = requests.get(orgs_url, headers=headers) + response = requests.get(orgs_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Return organizations data @@ -252,7 +254,7 @@ def get_organization_details(api_url: str, org_code: str, token_manager, logger) # Make the request to organization details endpoint org_url = f"{api_url}/organization/{org_code}" - response = requests.get(org_url, headers=headers) + response = requests.get(org_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Return organization data @@ -305,7 +307,7 @@ def get_organization_users(api_url: str, org_code: str, token_manager, logger) - # Make the request to organization users endpoint users_url = f"{api_url}/organization/{org_code}/users" - response = requests.get(users_url, headers=headers) + response = requests.get(users_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Return users data @@ -364,7 +366,7 @@ def get_user_permissions(api_url: str, token_manager, org_code: Optional[str] = permissions_url = f"{api_url}/user/permissions" # Make the request to permissions endpoint - response = requests.get(permissions_url, headers=headers) + response = requests.get(permissions_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Return permissions data @@ -455,7 +457,7 @@ def get_user_roles(api_url: str, token_manager, org_code: Optional[str] = None, roles_url = f"{api_url}/user/roles" # Make the request to roles endpoint - response = requests.get(roles_url, headers=headers) + response = requests.get(roles_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Return roles data @@ -544,7 +546,7 @@ def get_flag_value(api_url: str, flag_code: str, default_value: Any, token_manag flag_url = f"{api_url}/feature-flags/{flag_code}" # Make the request to flag endpoint - response = requests.get(flag_url, headers=headers) + response = requests.get(flag_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() # Parse the response @@ -723,7 +725,7 @@ def get_user_details_sync(userinfo_url: str, token_manager, logger) -> Dict[str, "Authorization": f"Bearer {access_token}", "Accept": "application/json" } - response = requests.get(userinfo_url, headers=headers) + response = requests.get(userinfo_url, headers=headers, timeout=REQUEST_TIMEOUT) response.raise_for_status() return response.json() except RuntimeError: diff --git a/kinde_sdk/core/storage/framework_aware_storage.py b/kinde_sdk/core/storage/framework_aware_storage.py index e0832acf..b3e8cea7 100644 --- a/kinde_sdk/core/storage/framework_aware_storage.py +++ b/kinde_sdk/core/storage/framework_aware_storage.py @@ -51,7 +51,7 @@ def get(self, key: str) -> Optional[Dict]: session = self._get_session() if session is not None: value = session.get(key) - self._logger.debug(f"Getting key '{key}' from session: {value}") + self._logger.debug(f"Getting key '{key}' from session") return value return None @@ -65,7 +65,7 @@ def set(self, key: str, value: Dict) -> None: """ session = self._get_session() if session is not None: - self._logger.debug(f"Setting key '{key}' in session with value: {value}") + self._logger.debug(f"Setting key '{key}' in session") session[key] = value # Mark session as modified for Flask if hasattr(session, 'modified'): @@ -97,7 +97,7 @@ def set_flat(self, value: str) -> None: """ session = self._get_session() if session is not None: - self._logger.debug(f"Setting flat data in session: {value}") + self._logger.debug("Setting flat data in session") session["_flat_data"] = value # Mark session as modified for Flask if hasattr(session, 'modified'): diff --git a/testv2/testv2_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py new file mode 100644 index 00000000..7194cdaa --- /dev/null +++ b/testv2/testv2_auth/test_session_secrets.py @@ -0,0 +1,50 @@ +import asyncio +import unittest +from unittest.mock import AsyncMock, patch + +import jwt + +from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.auth.token_manager import TokenManager +from kinde_sdk.auth.user_session import UserSession +from kinde_sdk.core.storage.storage_manager import StorageManager + + +class TestClientSecretNotPersisted(unittest.TestCase): + def setUp(self): + StorageManager().initialize({"type": "memory"}) + TokenManager.reset_instances() + with patch("requests.get", side_effect=Exception("offline")): + self.oauth = OAuth( + client_id="secret_test_client", + client_secret="super_secret", + redirect_uri="http://localhost/callback", + host="https://example.kinde.com", + ) + + def tearDown(self): + StorageManager().initialize({"type": "memory"}) + TokenManager.reset_instances() + + def test_session_storage_has_no_client_secret_but_restored_token_manager_does(self): + token_data = { + "access_token": jwt.encode({"sub": "user_1"}, "key", algorithm="HS256"), + "refresh_token": "refresh", + "expires_in": 3600, + } + with patch.object(OAuth, "exchange_code_for_tokens", AsyncMock(return_value=token_data)), \ + patch("kinde_sdk.auth.oauth.helper_get_user_details", AsyncMock(return_value={})): + asyncio.run(self.oauth.handle_redirect("code", "user_1")) + + stored = StorageManager().get("user_1") + self.assertNotIn("client_secret", stored["user_info"]) + self.assertNotIn("super_secret", repr(stored)) + + # Simulate a fresh process restoring the session from storage. + TokenManager.reset_instances() + token_manager = UserSession().get_token_manager("user_1") + self.assertEqual(token_manager.client_secret, "super_secret") + + +if __name__ == "__main__": + unittest.main() diff --git a/testv2/testv2_core/test_helpers.py b/testv2/testv2_core/test_helpers.py index 8e54da3c..060a58eb 100644 --- a/testv2/testv2_core/test_helpers.py +++ b/testv2/testv2_core/test_helpers.py @@ -11,6 +11,7 @@ # Import the functions to test from kinde_sdk.core.helpers import ( + REQUEST_TIMEOUT, generate_random_string, base64_url_encode, generate_pkce_pair, @@ -142,7 +143,8 @@ def test_get_user_details(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_user_details_token_error(self): @@ -280,7 +282,8 @@ def test_get_user_organizations(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_user_organizations_token_error(self): @@ -366,7 +369,8 @@ def test_get_organization_details(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_organization_details_token_error(self): @@ -456,7 +460,8 @@ def test_get_organization_users(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_organization_users_token_error(self): @@ -545,7 +550,8 @@ def test_get_user_permissions(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) # Reset mock and test with org_code @@ -561,7 +567,8 @@ def test_get_user_permissions(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_user_permissions_token_error(self): @@ -713,7 +720,8 @@ def test_get_user_roles(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) # Reset mock and test with org_code @@ -729,7 +737,8 @@ def test_get_user_roles(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_user_roles_token_error(self): @@ -904,7 +913,8 @@ def test_get_flag_value(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_flag_value_request_error(self): @@ -1146,7 +1156,8 @@ def test_get_user_details_sync_with_event_loop(self): headers={ "Authorization": "Bearer test_access_token", "Accept": "application/json" - } + }, + timeout=REQUEST_TIMEOUT ) def test_get_user_details_sync_token_error(self): From 445f44c8d96d34a0a2876bc19242813d149b2871 Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:03:52 +0200 Subject: [PATCH 02/20] fix(auth): require OAuth callback state and validate ID token nonce handle_redirect only checked `state` when the callback carried one, so a callback with the state parameter stripped skipped validation entirely and went straight to token exchange (login CSRF / code injection). The nonce stored at login was also never compared with the ID token. - Always require state: reject the callback unless a state was stored for a login started by the SDK in this session and the request presents the same value. Callbacks with no login in progress (nothing stored) are rejected before token exchange, with or without a state parameter. - After token exchange, when a nonce was stored and the response has an ID token, reject a missing or mismatched `nonce` claim. - Consume stored state and nonce as soon as state validates, so they are single-use even if exchange or nonce validation fails. A rejected callback leaves the pending state in place, so a forged request cannot clear it to make a later callback pass. - Stop logging raw state values at warning level. Adds testv2/testv2_auth/test_oauth_callback_security.py covering the standalone, Flask and FastAPI callback paths. --- kinde_sdk/auth/oauth.py | 45 ++- .../test_oauth_callback_security.py | 335 ++++++++++++++++++ 2 files changed, 365 insertions(+), 15 deletions(-) create mode 100644 testv2/testv2_auth/test_oauth_callback_security.py diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index 0c2d19d1..97109ac7 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -2,6 +2,7 @@ import requests import logging import time +import jwt from typing import Any, Dict, Optional from urllib.parse import urlencode @@ -515,14 +516,24 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = Returns: Dict with user and token information """ - # Verify state if provided - if state: - stored_state = self._session_manager.storage_manager.get("user:state") - self._logger.warning(f"stored_state: {stored_state}, state: {state}") - if not stored_state or state != stored_state.get("value"): - self._logger.error(f"State mismatch: received {state}, stored {stored_state}") - raise KindeLoginException("Invalid state parameter") - + storage = self._session_manager.storage_manager + + # Verify state. A callback is only accepted for a login started by this + # SDK in this session: a state must have been stored, and the callback + # must present the same value. Missing state is rejected, not skipped. + stored_state = storage.get("user:state") + expected_state = stored_state.get("value") if stored_state else None + if not expected_state or state != expected_state: + self._logger.error("State mismatch in OAuth callback") + raise KindeLoginException("Invalid state parameter") + + # State and nonce are single-use: consume them once state has been + # validated, so they can't be replayed even if the rest of the flow fails. + stored_nonce = storage.get("user:nonce") + expected_nonce = stored_nonce.get("value") if stored_nonce else None + storage.delete("user:state") + storage.delete("user:nonce") + # Get code verifier for PKCE code_verifier = None stored_code_verifier = self._session_manager.storage_manager.get("user:code_verifier") @@ -538,6 +549,17 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = except Exception as e: self._logger.error(f"Token exchange failed: {str(e)}") raise KindeTokenException(f"Failed to exchange code for tokens: {str(e)}") from e + + # Verify the ID token nonce matches the one sent in the authorization request + id_token = token_data.get("id_token") + if expected_nonce and id_token: + try: + id_token_claims = jwt.decode(id_token, options={"verify_signature": False}) + except jwt.PyJWTError as e: + raise KindeLoginException("Invalid ID token") from e + if id_token_claims.get("nonce") != expected_nonce: + self._logger.error("Nonce mismatch in ID token") + raise KindeLoginException("Invalid nonce in ID token") # Store tokens user_info = { @@ -565,13 +587,6 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = logger=self._logger ) - # Clean up state - if state: - self._session_manager.storage_manager.delete("user:state") - - # Clean up nonce - self._session_manager.storage_manager.delete("user:nonce") - return { "tokens": token_data, "user": user_details diff --git a/testv2/testv2_auth/test_oauth_callback_security.py b/testv2/testv2_auth/test_oauth_callback_security.py new file mode 100644 index 00000000..4db929b8 --- /dev/null +++ b/testv2/testv2_auth/test_oauth_callback_security.py @@ -0,0 +1,335 @@ +""" +Security regression tests for the OAuth callback (``OAuth.handle_redirect``). + +Covers: + * state is always required: a callback is rejected unless a login was + started by the SDK in this session (state stored) and the callback + presents the same state (login CSRF / authorization code injection), + * the ID token ``nonce`` claim must match the nonce sent in the + authorization request, + * stored state / nonce are single-use and cleared after the callback. + +The standalone tests use the real in-memory storage path (null framework); +the Flask / FastAPI tests drive the real ``/login`` and ``/callback`` routes. +Only the network calls (token endpoint, userinfo) are mocked. +""" +import asyncio +import tempfile +from unittest.mock import patch, MagicMock, AsyncMock +from urllib.parse import urlparse, parse_qs + +import jwt +import pytest + +from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.core.exceptions import KindeLoginException +from kinde_sdk.core.framework.framework_factory import FrameworkFactory +from kinde_sdk.core.storage.storage_manager import StorageManager + +CLIENT_ID = "test_client_id" +REDIRECT_URI = "http://localhost/callback" + + +def _id_token(**claims): + payload = {"sub": "kp_user", "aud": CLIENT_ID, "iss": "https://example.kinde.com"} + payload.update(claims) + # Signature is irrelevant here: the SDK decodes the ID token without + # verifying the signature (token comes straight from the token endpoint). + return jwt.encode(payload, "test-secret-key-of-sufficient-length-123", algorithm="HS256") + + +def _token_response(id_token=None, include_id_token=True): + resp = MagicMock() + resp.status_code = 200 + body = { + "access_token": jwt.encode({"sub": "kp_user"}, "test-secret-key-of-sufficient-length-123", algorithm="HS256"), + "token_type": "bearer", + "expires_in": 3600, + } + if include_id_token: + body["id_token"] = id_token + resp.json.return_value = body + resp.text = "ok" + return resp + + +def _query(url): + return {k: v[0] for k, v in parse_qs(urlparse(url).query).items()} + + +@pytest.fixture +def isolated_singletons(): + """Isolate the process-wide StorageManager / FrameworkFactory singletons.""" + sm = StorageManager() + saved = (sm._storage, sm._device_id, sm._storage_type) + with patch.object(FrameworkFactory, "_framework_instance", None): + yield + sm._storage, sm._device_id, sm._storage_type = saved + + +@pytest.fixture +def standalone_oauth(isolated_singletons): + oauth = OAuth( + client_id=CLIENT_ID, + client_secret="test_client_secret", + redirect_uri=REDIRECT_URI, + host="https://example.kinde.com", + ) + return oauth + + +@pytest.fixture +def network(): + """Mock the token endpoint and userinfo lookup.""" + with patch("kinde_sdk.auth.oauth.requests.post") as post, \ + patch("kinde_sdk.auth.oauth.helper_get_user_details", new=AsyncMock(return_value={"id": "kp_user"})): + yield post + + +def _storage(): + return StorageManager() + + +# --------------------------------------------------------------------------- +# Standalone (null framework, real memory storage) +# --------------------------------------------------------------------------- + +class TestCallbackStateStandalone: + + def test_callback_without_state_rejected_when_state_stored(self, standalone_oauth, network): + """(a) A callback with no `state` param must not reach token exchange.""" + asyncio.run(standalone_oauth.login()) + assert _storage().get("user:state") is not None + + network.return_value = _token_response(_id_token(nonce="whatever")) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("attacker_code", "victim", None)) + network.assert_not_called() + + def test_callback_with_empty_state_rejected_when_state_stored(self, standalone_oauth, network): + asyncio.run(standalone_oauth.login()) + network.return_value = _token_response(_id_token(nonce="whatever")) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("attacker_code", "victim", "")) + network.assert_not_called() + + def test_callback_with_wrong_state_rejected(self, standalone_oauth, network): + asyncio.run(standalone_oauth.login()) + network.return_value = _token_response(_id_token(nonce="whatever")) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "victim", "not-the-state")) + network.assert_not_called() + + def test_state_is_single_use(self, standalone_oauth, network): + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + assert _storage().get("user:state") is None + assert _storage().get("user:nonce") is None + assert _storage().get("user:code_verifier") is None + + network.reset_mock() + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + network.assert_not_called() + + def test_forged_callback_cannot_clear_pending_state(self, standalone_oauth, network): + """A rejected callback must not consume the pending state; otherwise an + attacker could send one forged callback to clear it and a second, + state-less one that would then be accepted.""" + q = _query(asyncio.run(standalone_oauth.login())) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "victim", "not-the-state")) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("attacker_code", "victim", None)) + network.assert_not_called() + + # The victim's genuine callback still completes. + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + asyncio.run(standalone_oauth.handle_redirect("code", "victim", q["state"])) + assert network.call_count == 1 + assert _storage().get("user:state") is None + assert _storage().get("user:nonce") is None + + +class TestCallbackNoLoginInProgressStandalone: + """Nothing stored (no SDK-started login in this session / already logged in).""" + + @pytest.mark.parametrize("state", [None, "", "attacker-state"]) + def test_callback_rejected_when_no_login_in_progress(self, standalone_oauth, network, state): + assert _storage().get("user:state") is None + network.return_value = _token_response(_id_token(nonce="x")) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("attacker_code", "victim", state)) + network.assert_not_called() + assert standalone_oauth._session_manager.get_token_manager("victim") is None + + def test_callback_rejected_after_completed_login(self, standalone_oauth, network): + """Once a login has completed, a further (forged) callback is rejected + and does not replace the established session.""" + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + asyncio.run(standalone_oauth.handle_redirect("code", "victim", q["state"])) + network.reset_mock() + + for state in (None, q["state"]): + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("attacker_code", "victim", state)) + network.assert_not_called() + + +class TestCallbackNonceStandalone: + + def test_valid_state_and_nonce_accepted(self, standalone_oauth, network): + q = _query(asyncio.run(standalone_oauth.login())) + verifier = _storage().get("user:code_verifier")["value"] + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + + result = asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + + assert result["user"] == {"id": "kp_user"} + sent = network.call_args.kwargs["data"] + assert sent["code"] == "code" + assert sent["code_verifier"] == verifier + + def test_id_token_with_wrong_nonce_rejected(self, standalone_oauth, network): + """(b) An ID token whose nonce doesn't match the stored nonce is rejected.""" + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(_id_token(nonce="attacker-nonce")) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + # No session must have been established for the user + assert standalone_oauth._session_manager.get_token_manager("u1") is None + assert _storage().get("user:nonce") is None + + def test_id_token_without_nonce_rejected(self, standalone_oauth, network): + """(b) An ID token with no nonce claim is rejected when a nonce was sent.""" + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(_id_token()) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + assert standalone_oauth._session_manager.get_token_manager("u1") is None + + def test_undecodable_id_token_rejected(self, standalone_oauth, network): + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response("not-a-jwt") + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + + def test_token_response_without_id_token_still_accepted(self, standalone_oauth, network): + """Nonce binds the ID token only; non-OIDC responses (no id_token) are unaffected.""" + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(include_id_token=False) + result = asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + assert "id_token" not in result["tokens"] + + +# --------------------------------------------------------------------------- +# Flask routes +# --------------------------------------------------------------------------- + +@pytest.fixture +def flask_client(isolated_singletons, monkeypatch): + from flask import Flask + monkeypatch.setenv("SECRET_KEY", "test-secret") + monkeypatch.setenv("SESSION_FILE_DIR", tempfile.mkdtemp(prefix="kinde_cb_test_")) + app = Flask(__name__) + OAuth( + client_id=CLIENT_ID, + client_secret="test_client_secret", + redirect_uri=REDIRECT_URI, + host="https://example.kinde.com", + framework="flask", + app=app, + ) + app.config["TESTING"] = True + return app.test_client() + + +class TestFlaskCallback: + + def test_callback_without_state_rejected(self, flask_client, network): + r = flask_client.get("/login") + assert r.status_code == 302 + network.return_value = _token_response(_id_token(nonce="x")) + + r = flask_client.get("/callback?code=attacker_code") + + assert r.status_code == 400 + network.assert_not_called() + + @pytest.mark.parametrize("qs", ["code=attacker_code", "code=attacker_code&state=attacker-state"]) + def test_callback_rejected_when_no_login_in_progress(self, flask_client, network, qs): + network.return_value = _token_response(_id_token(nonce="x")) + r = flask_client.get(f"/callback?{qs}") + assert r.status_code == 400 + network.assert_not_called() + + def test_callback_wrong_nonce_rejected(self, flask_client, network): + q = _query(flask_client.get("/login").headers["Location"]) + network.return_value = _token_response(_id_token(nonce="attacker-nonce")) + r = flask_client.get(f"/callback?code=c&state={q['state']}") + assert r.status_code == 400 + + def test_callback_happy_path(self, flask_client, network): + q = _query(flask_client.get("/login").headers["Location"]) + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + r = flask_client.get(f"/callback?code=c&state={q['state']}") + assert r.status_code == 302 + assert network.call_count == 1 + + +# --------------------------------------------------------------------------- +# FastAPI routes +# --------------------------------------------------------------------------- + +@pytest.fixture +def fastapi_client(isolated_singletons): + from fastapi import FastAPI + from fastapi.testclient import TestClient + from starlette.middleware.sessions import SessionMiddleware + app = FastAPI() + OAuth( + client_id=CLIENT_ID, + client_secret="test_client_secret", + redirect_uri=REDIRECT_URI, + host="https://example.kinde.com", + framework="fastapi", + app=app, + ) + app.add_middleware(SessionMiddleware, secret_key="test-secret") + return TestClient(app, raise_server_exceptions=False, follow_redirects=False) + + +class TestFastAPICallback: + + def test_callback_without_state_rejected(self, fastapi_client, network): + r = fastapi_client.get("/login") + assert r.status_code in (302, 307) + network.return_value = _token_response(_id_token(nonce="x")) + + r = fastapi_client.get("/callback?code=attacker_code") + + # FastAPI route re-raises SDK exceptions (pre-existing behaviour) -> 500 + assert r.status_code >= 400 + network.assert_not_called() + + @pytest.mark.parametrize("qs", ["code=attacker_code", "code=attacker_code&state=attacker-state"]) + def test_callback_rejected_when_no_login_in_progress(self, fastapi_client, network, qs): + network.return_value = _token_response(_id_token(nonce="x")) + r = fastapi_client.get(f"/callback?{qs}") + assert r.status_code >= 400 + network.assert_not_called() + + def test_callback_wrong_nonce_rejected(self, fastapi_client, network): + q = _query(fastapi_client.get("/login").headers["location"]) + network.return_value = _token_response(_id_token(nonce="attacker-nonce")) + r = fastapi_client.get(f"/callback?code=c&state={q['state']}") + assert r.status_code >= 400 + + def test_callback_happy_path(self, fastapi_client, network): + q = _query(fastapi_client.get("/login").headers["location"]) + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + r = fastapi_client.get(f"/callback?code=c&state={q['state']}") + assert r.status_code in (302, 307) + assert network.call_count == 1 From ad0c2eee3db9e6e9f7af8ad5f02263e1bc3b7f27 Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:39:35 +0200 Subject: [PATCH 03/20] test: disable GC around SmartOAuth timing loops The TestSmartOAuthPerformance tests measure wall time over a few milliseconds. If an automatic full (gen-2) garbage collection is triggered inside the timed region it adds ~50-110 ms for the current test-suite heap, failing test_method_delegation_performance (< 10 ms) regardless of SmartOAuth's delegation cost. Whether that happens depends only on the allocation history of earlier tests, so adding unrelated tests can flip it (seen on Python 3.10 CI). Pause automatic GC only around each timed region and restore it in a finally block (re-enabling only if it was enabled). Limits are unchanged. --- .../test_smart_oauth_integration.py | 50 ++++++++++++++----- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/testv2/testv2_auth/test_smart_oauth_integration.py b/testv2/testv2_auth/test_smart_oauth_integration.py index 3302f2a9..5bcf3f80 100644 --- a/testv2/testv2_auth/test_smart_oauth_integration.py +++ b/testv2/testv2_auth/test_smart_oauth_integration.py @@ -202,27 +202,43 @@ class TestSmartOAuthPerformance: def test_initialization_performance(self): """Test that SmartOAuth initialization is fast.""" + import gc import time with patch('kinde_sdk.auth.smart_oauth.OAuth'), \ patch('kinde_sdk.auth.smart_oauth.AsyncOAuth'): - start_time = time.time() - smart_oauth = SmartOAuth(client_id="test") - end_time = time.time() + # Keep automatic (full) GC pauses out of the timed region + gc_was_enabled = gc.isenabled() + gc.disable() + try: + start_time = time.time() + smart_oauth = SmartOAuth(client_id="test") + end_time = time.time() + finally: + if gc_was_enabled: + gc.enable() # Initialization should be fast (< 100ms) assert (end_time - start_time) < 0.1 def test_method_delegation_performance(self, smart_oauth, mock_sync_oauth): """Test that method delegation is fast.""" + import gc import time - # Test sync method performance - start_time = time.time() - for _ in range(100): - smart_oauth.is_authenticated() - end_time = time.time() + # Test sync method performance (automatic GC paused so a full + # collection can't land in the timed region) + gc_was_enabled = gc.isenabled() + gc.disable() + try: + start_time = time.time() + for _ in range(100): + smart_oauth.is_authenticated() + end_time = time.time() + finally: + if gc_was_enabled: + gc.enable() # 100 calls should be fast (< 10ms) assert (end_time - start_time) < 0.01 @@ -230,13 +246,21 @@ def test_method_delegation_performance(self, smart_oauth, mock_sync_oauth): @pytest.mark.asyncio async def test_async_method_performance(self, smart_oauth, mock_async_oauth): """Test that async method delegation is fast.""" + import gc import time - # Test async method performance - start_time = time.time() - for _ in range(10): - await smart_oauth.get_user_info_async() - end_time = time.time() + # Test async method performance (automatic GC paused so a full + # collection can't land in the timed region) + gc_was_enabled = gc.isenabled() + gc.disable() + try: + start_time = time.time() + for _ in range(10): + await smart_oauth.get_user_info_async() + end_time = time.time() + finally: + if gc_was_enabled: + gc.enable() # 10 async calls should be fast (< 100ms) assert (end_time - start_time) < 0.1 From 3fe16646b72f51dc85a4c6b3cebba6e623dafe60 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Tue, 29 Sep 2026 06:51:30 +0200 Subject: [PATCH 04/20] fix(auth): keep configured client secret and strip legacy secrets from sessions - Only register a client secret when one is configured, so a secretless OAuth instance for the same client_id can't clear it. - Drop client_secret from user_info when saving a session and when loading sessions written by older SDK versions. - test_session_secrets: start the login through the SDK and pass the stored state on the callback; add tests for both fixes. - test_user_session: expect client_secret to be absent from stored and restored user_info. Co-Authored-By: Claude Opus 5.5 --- kinde_sdk/auth/oauth.py | 4 +- kinde_sdk/auth/user_session.py | 10 ++-- testv2/testv2_auth/test_session_secrets.py | 60 ++++++++++++++++++---- testv2/testv2_auth/test_user_session.py | 13 +++-- 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index 5a9c28d2..c00bbb47 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -60,7 +60,9 @@ def __init__( # Validate required configurations if not self.client_id: raise KindeConfigurationException("Client ID is required.") - UserSession.client_secrets[self.client_id] = self.client_secret + # Don't let a secretless (PKCE) instance for the same client_id wipe a configured secret + if self.client_secret: + UserSession.client_secrets[self.client_id] = self.client_secret # Initialize API endpoints self._set_api_endpoints() diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index a62610a0..d785c9b7 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -50,8 +50,10 @@ def _save_to_storage(self, user_id: str): if session_data: # We need to serialize the session data # Token manager can't be directly serialized + # Never persist client_secret, even if a caller passed it in user_info + user_info = {k: v for k, v in session_data["user_info"].items() if k != "client_secret"} serialized_data = { - "user_info": session_data["user_info"], + "user_info": user_info, "tokens": session_data["token_manager"].tokens, } # Store with user: prefix to make it user-specific but device-independent @@ -86,8 +88,10 @@ def _load_from_storage(self, user_id: str) -> bool: "token_url" not in user_info or "access_token" not in tokens): return False - - + + # Sessions saved by older SDK versions may still contain client_secret; drop it + user_info = {k: v for k, v in user_info.items() if k != "client_secret"} + token_manager = TokenManager( user_id, user_info.get("client_id"), diff --git a/testv2/testv2_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py index 7194cdaa..b464f59c 100644 --- a/testv2/testv2_auth/test_session_secrets.py +++ b/testv2/testv2_auth/test_session_secrets.py @@ -1,6 +1,7 @@ import asyncio import unittest from unittest.mock import AsyncMock, patch +from urllib.parse import parse_qs, urlparse import jwt @@ -14,27 +15,41 @@ class TestClientSecretNotPersisted(unittest.TestCase): def setUp(self): StorageManager().initialize({"type": "memory"}) TokenManager.reset_instances() + UserSession.client_secrets.clear() + self.oauth = self._make_oauth(client_secret="super_secret") + + def tearDown(self): + StorageManager().initialize({"type": "memory"}) + TokenManager.reset_instances() + UserSession.client_secrets.clear() + + def _make_oauth(self, client_secret=None): with patch("requests.get", side_effect=Exception("offline")): - self.oauth = OAuth( + return OAuth( client_id="secret_test_client", - client_secret="super_secret", + client_secret=client_secret, redirect_uri="http://localhost/callback", host="https://example.kinde.com", ) - def tearDown(self): - StorageManager().initialize({"type": "memory"}) - TokenManager.reset_instances() - - def test_session_storage_has_no_client_secret_but_restored_token_manager_does(self): - token_data = { + def _token_data(self): + return { "access_token": jwt.encode({"sub": "user_1"}, "key", algorithm="HS256"), "refresh_token": "refresh", "expires_in": 3600, } - with patch.object(OAuth, "exchange_code_for_tokens", AsyncMock(return_value=token_data)), \ + + def _complete_login(self, user_id="user_1"): + # Start the login through the SDK so a state is stored, then return it + # on the callback like Kinde would. + login_url = asyncio.run(self.oauth.login()) + state = parse_qs(urlparse(login_url).query)["state"][0] + with patch.object(OAuth, "exchange_code_for_tokens", AsyncMock(return_value=self._token_data())), \ patch("kinde_sdk.auth.oauth.helper_get_user_details", AsyncMock(return_value={})): - asyncio.run(self.oauth.handle_redirect("code", "user_1")) + asyncio.run(self.oauth.handle_redirect("code", user_id, state)) + + def test_session_storage_has_no_client_secret_but_restored_token_manager_does(self): + self._complete_login() stored = StorageManager().get("user_1") self.assertNotIn("client_secret", stored["user_info"]) @@ -45,6 +60,31 @@ def test_session_storage_has_no_client_secret_but_restored_token_manager_does(se token_manager = UserSession().get_token_manager("user_1") self.assertEqual(token_manager.client_secret, "super_secret") + def test_secretless_oauth_instance_does_not_clear_configured_secret(self): + self._make_oauth(client_secret=None) + + self.assertEqual(UserSession.client_secrets.get("secret_test_client"), "super_secret") + + def test_legacy_session_secret_is_dropped_on_load_and_not_saved_again(self): + # A session written by an older SDK version, with client_secret in user_info + StorageManager().setItems("user_1", { + "user_info": { + "client_id": "secret_test_client", + "client_secret": "super_secret", + "token_url": "https://example.kinde.com/oauth2/token", + }, + "tokens": self._token_data(), + }) + + session = UserSession() + user_info = session.get_user_data("user_1") + self.assertNotIn("client_secret", user_info) + self.assertEqual(session.get_token_manager("user_1").client_secret, "super_secret") + + # Saving the session again must not write the secret back + session.set_user_data("user_1", {**user_info, "client_secret": "super_secret"}, self._token_data()) + self.assertNotIn("super_secret", repr(StorageManager().get("user_1"))) + if __name__ == "__main__": unittest.main() diff --git a/testv2/testv2_auth/test_user_session.py b/testv2/testv2_auth/test_user_session.py index fc9dfb4a..00425726 100644 --- a/testv2/testv2_auth/test_user_session.py +++ b/testv2/testv2_auth/test_user_session.py @@ -94,7 +94,9 @@ def test_set_user_data(self): # Check storage dict reflects the data self.assertIn(self.user_id, self.storage_dict) stored_data = self.storage_dict[self.user_id] - self.assertEqual(stored_data["user_info"], self.user_info) + # client_secret is never written to session storage + expected_stored_info = {k: v for k, v in self.user_info.items() if k != "client_secret"} + self.assertEqual(stored_data["user_info"], expected_stored_info) self.assertIn("tokens", stored_data) # Verify tokens @@ -152,12 +154,13 @@ def test_get_user_data_from_storage(self): "user_info": self.user_info, "tokens": token_manager.tokens, } - + # Get user data retrieved_user_info = self.user_session.get_user_data(self.user_id) - - # Check user info - self.assertEqual(retrieved_user_info, self.user_info) + + # Check user info (a client_secret left in older stored sessions is dropped on load) + expected_info = {k: v for k, v in self.user_info.items() if k != "client_secret"} + self.assertEqual(retrieved_user_info, expected_info) # Verify storage.get was called self.mock_storage_manager.get.assert_called_with(self.user_id) From 514f84072e3d62f9c0185a29501256f41b838289 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Thu, 1 Oct 2026 06:18:38 +0200 Subject: [PATCH 05/20] fix(auth): always validate the ID token nonce; return 400 from the FastAPI callback Review follow-up for #209. - Reject the callback when no nonce was stored for the login (before the code is redeemed) and when the token response has no ID token. Both previously skipped the nonce check, so a swapped code from an authorization request without `openid` could still establish a session. - FastAPI /callback: reject a missing `code` with 400 before handle_redirect, which would otherwise consume the pending state, nonce and code verifier; return 400 for KindeLoginException instead of 500, matching Flask. Removes the "State not found" branch, which nothing raises. - Tests: the no-ID-token test now expects a rejection; add tests for a missing stored nonce and a missing code (Flask and FastAPI); FastAPI route tests assert 400 exactly; test_session_secrets returns an ID token with the login's nonce. Co-Authored-By: Claude Opus 5.5 --- kinde_fastapi/framework/fastapi_framework.py | 12 ++-- kinde_sdk/auth/oauth.py | 34 ++++++++---- .../test_oauth_callback_security.py | 55 +++++++++++++++---- testv2/testv2_auth/test_session_secrets.py | 13 +++-- 4 files changed, 85 insertions(+), 29 deletions(-) diff --git a/kinde_fastapi/framework/fastapi_framework.py b/kinde_fastapi/framework/fastapi_framework.py index 3d45c165..8f0a078a 100644 --- a/kinde_fastapi/framework/fastapi_framework.py +++ b/kinde_fastapi/framework/fastapi_framework.py @@ -3,6 +3,7 @@ from fastapi.responses import RedirectResponse, HTMLResponse from kinde_sdk.core.framework.framework_interface import FrameworkInterface from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.core.exceptions import KindeLoginException from ..middleware.framework_middleware import FrameworkMiddleware import os import uuid @@ -183,15 +184,18 @@ async def callback(request: Request, code: Optional[str] = None, state: Optional except Exception as ex: return HTMLResponse(f"Error parsing reauth state: {str(ex)}", status_code=400) + # Reject before handle_redirect, which would consume the pending state + if not code: + return HTMLResponse("Authentication failed: Missing authorization code", status_code=400) + user_id = request.session.get('user_id') or str(uuid.uuid4()) try: assert self._oauth is not None await self._oauth.handle_redirect(code, user_id, state) - except Exception as e: - if "State not found" in str(e): - return HTMLResponse("Error: State not found. Please check Kinde Python SDK documentation.\n" + str(e), status_code=500) - raise e + except KindeLoginException as e: + # Missing, forged or stale callbacks are client errors, as in the Flask route + return HTMLResponse(f"Authentication failed: {e}", status_code=400) request.session['user_id'] = user_id diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index 12f1496a..e87a6e9c 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -526,10 +526,16 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = self._logger.error("State mismatch in OAuth callback") raise KindeLoginException("Invalid state parameter") - # State and nonce are single-use: consume them once state has been - # validated, so they can't be replayed even if the rest of the flow fails. + # Every SDK-started login stores a nonce. Without it the ID token can't be + # bound to this login, so reject before the code is redeemed. stored_nonce = storage.get("user:nonce") expected_nonce = stored_nonce.get("value") if stored_nonce else None + if not expected_nonce: + self._logger.error("No nonce stored for OAuth callback") + raise KindeLoginException("Missing nonce for this login") + + # State and nonce are single-use: consume them once they have been + # validated, so they can't be replayed even if the rest of the flow fails. storage.delete("user:state") storage.delete("user:nonce") @@ -549,16 +555,22 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = self._logger.error(f"Token exchange failed: {str(e)}") raise KindeTokenException(f"Failed to exchange code for tokens: {str(e)}") from e - # Verify the ID token nonce matches the one sent in the authorization request + # Verify the ID token nonce matches the one sent in the authorization request. + # A response without an ID token is rejected too: otherwise a swapped code + # from an authorization request without `openid` would skip this check. id_token = token_data.get("id_token") - if expected_nonce and id_token: - try: - id_token_claims = jwt.decode(id_token, options={"verify_signature": False}) - except jwt.PyJWTError as e: - raise KindeLoginException("Invalid ID token") from e - if id_token_claims.get("nonce") != expected_nonce: - self._logger.error("Nonce mismatch in ID token") - raise KindeLoginException("Invalid nonce in ID token") + if not id_token: + self._logger.error("Token response has no ID token") + raise KindeLoginException("Missing ID token") + try: + # Signature not verified, as elsewhere in the SDK: the token comes + # directly from the token endpoint over TLS + id_token_claims = jwt.decode(id_token, options={"verify_signature": False}) + except jwt.PyJWTError as e: + raise KindeLoginException("Invalid ID token") from e + if id_token_claims.get("nonce") != expected_nonce: + self._logger.error("Nonce mismatch in ID token") + raise KindeLoginException("Invalid nonce in ID token") # Store tokens user_info = { diff --git a/testv2/testv2_auth/test_oauth_callback_security.py b/testv2/testv2_auth/test_oauth_callback_security.py index 4db929b8..7f339f69 100644 --- a/testv2/testv2_auth/test_oauth_callback_security.py +++ b/testv2/testv2_auth/test_oauth_callback_security.py @@ -5,8 +5,9 @@ * state is always required: a callback is rejected unless a login was started by the SDK in this session (state stored) and the callback presents the same state (login CSRF / authorization code injection), - * the ID token ``nonce`` claim must match the nonce sent in the - authorization request, + * a nonce must have been stored for the login, the token response must + contain an ID token, and its ``nonce`` claim must match the nonce sent + in the authorization request, * stored state / nonce are single-use and cleared after the callback. The standalone tests use the real in-memory storage path (null framework); @@ -216,12 +217,24 @@ def test_undecodable_id_token_rejected(self, standalone_oauth, network): with pytest.raises(KindeLoginException): asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) - def test_token_response_without_id_token_still_accepted(self, standalone_oauth, network): - """Nonce binds the ID token only; non-OIDC responses (no id_token) are unaffected.""" + def test_token_response_without_id_token_rejected(self, standalone_oauth, network): + """Without an ID token the nonce can't be checked, so a swapped code from an + authorization request without `openid` must not establish a session.""" q = _query(asyncio.run(standalone_oauth.login())) network.return_value = _token_response(include_id_token=False) - result = asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) - assert "id_token" not in result["tokens"] + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + assert standalone_oauth._session_manager.get_token_manager("u1") is None + + def test_missing_stored_nonce_rejected_before_token_exchange(self, standalone_oauth, network): + """Losing the stored nonce must not turn nonce validation off.""" + q = _query(asyncio.run(standalone_oauth.login())) + _storage().delete("user:nonce") + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + network.assert_not_called() + assert standalone_oauth._session_manager.get_token_manager("u1") is None # --------------------------------------------------------------------------- @@ -271,6 +284,17 @@ def test_callback_wrong_nonce_rejected(self, flask_client, network): r = flask_client.get(f"/callback?code=c&state={q['state']}") assert r.status_code == 400 + def test_callback_without_code_rejected_and_keeps_pending_login(self, flask_client, network): + q = _query(flask_client.get("/login").headers["Location"]) + r = flask_client.get(f"/callback?state={q['state']}") + assert r.status_code == 400 + network.assert_not_called() + + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + r = flask_client.get(f"/callback?code=c&state={q['state']}") + assert r.status_code == 302 + assert network.call_args.kwargs["data"].get("code_verifier") + def test_callback_happy_path(self, flask_client, network): q = _query(flask_client.get("/login").headers["Location"]) network.return_value = _token_response(_id_token(nonce=q["nonce"])) @@ -310,22 +334,33 @@ def test_callback_without_state_rejected(self, fastapi_client, network): r = fastapi_client.get("/callback?code=attacker_code") - # FastAPI route re-raises SDK exceptions (pre-existing behaviour) -> 500 - assert r.status_code >= 400 + assert r.status_code == 400 network.assert_not_called() @pytest.mark.parametrize("qs", ["code=attacker_code", "code=attacker_code&state=attacker-state"]) def test_callback_rejected_when_no_login_in_progress(self, fastapi_client, network, qs): network.return_value = _token_response(_id_token(nonce="x")) r = fastapi_client.get(f"/callback?{qs}") - assert r.status_code >= 400 + assert r.status_code == 400 network.assert_not_called() def test_callback_wrong_nonce_rejected(self, fastapi_client, network): q = _query(fastapi_client.get("/login").headers["location"]) network.return_value = _token_response(_id_token(nonce="attacker-nonce")) r = fastapi_client.get(f"/callback?code=c&state={q['state']}") - assert r.status_code >= 400 + assert r.status_code == 400 + + def test_callback_without_code_rejected_and_keeps_pending_login(self, fastapi_client, network): + q = _query(fastapi_client.get("/login").headers["location"]) + r = fastapi_client.get(f"/callback?state={q['state']}") + assert r.status_code == 400 + network.assert_not_called() + + # State, nonce and code verifier weren't consumed: the genuine callback still completes + network.return_value = _token_response(_id_token(nonce=q["nonce"])) + r = fastapi_client.get(f"/callback?code=c&state={q['state']}") + assert r.status_code in (302, 307) + assert network.call_args.kwargs["data"].get("code_verifier") def test_callback_happy_path(self, fastapi_client, network): q = _query(fastapi_client.get("/login").headers["location"]) diff --git a/testv2/testv2_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py index b464f59c..528d80a6 100644 --- a/testv2/testv2_auth/test_session_secrets.py +++ b/testv2/testv2_auth/test_session_secrets.py @@ -40,11 +40,16 @@ def _token_data(self): } def _complete_login(self, user_id="user_1"): - # Start the login through the SDK so a state is stored, then return it - # on the callback like Kinde would. + # Start the login through the SDK so a state and nonce are stored, then + # return them on the callback and in the ID token like Kinde would. login_url = asyncio.run(self.oauth.login()) - state = parse_qs(urlparse(login_url).query)["state"][0] - with patch.object(OAuth, "exchange_code_for_tokens", AsyncMock(return_value=self._token_data())), \ + query = parse_qs(urlparse(login_url).query) + state, nonce = query["state"][0], query["nonce"][0] + token_data = { + **self._token_data(), + "id_token": jwt.encode({"sub": "user_1", "nonce": nonce}, "key", algorithm="HS256"), + } + with patch.object(OAuth, "exchange_code_for_tokens", AsyncMock(return_value=token_data)), \ patch("kinde_sdk.auth.oauth.helper_get_user_details", AsyncMock(return_value={})): asyncio.run(self.oauth.handle_redirect("code", user_id, state)) From 70b2cb84a7ac2144034983599abca74a510c1121 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Tue, 6 Oct 2026 16:15:10 +0200 Subject: [PATCH 06/20] fix(auth): stop leaking token errors and keep sessions across workers Callback and Account API failures no longer include response bodies. Refreshed tokens are written back to the session, and the storage device ID is read from the session so a restart or another worker stays signed in. Management API feature-flag values accept bool and int, which the API returns but the spec declares as strings. Co-authored-by: Cursor --- CHANGELOG.md | 60 +++ README.md | 44 +- generate_management_sdk.py | 47 ++ .../examples/management_token_example.py | 14 +- kinde_fastapi/framework/fastapi_framework.py | 30 +- kinde_flask/framework/flask_framework.py | 25 +- kinde_sdk/auth/base_auth.py | 26 +- kinde_sdk/auth/feature_flags.py | 32 +- kinde_sdk/auth/oauth.py | 29 +- kinde_sdk/auth/permissions.py | 13 +- kinde_sdk/auth/roles.py | 2 +- kinde_sdk/auth/token_manager.py | 13 +- kinde_sdk/auth/user_session.py | 6 + kinde_sdk/core/session_management.py | 22 +- kinde_sdk/core/storage/storage_manager.py | 30 +- ...ture_flags_response_feature_flags_value.py | 6 +- testv2/testv2_auth/test_account_api_host.py | 61 +++ testv2/testv2_auth/test_feature_flags.py | 51 ++- testv2/testv2_auth/test_log_redaction.py | 403 ++++++++++++++++++ testv2/testv2_auth/test_logout_url.py | 48 +++ testv2/testv2_auth/test_permissions.py | 47 +- testv2/testv2_auth/test_user_session.py | 39 +- .../test_kinde_session_management.py | 26 ++ testv2/testv2_core/test_storage_manager.py | 47 +- .../test_feature_flag_value_types.py | 49 +++ 25 files changed, 1084 insertions(+), 86 deletions(-) create mode 100644 testv2/testv2_auth/test_account_api_host.py create mode 100644 testv2/testv2_auth/test_log_redaction.py create mode 100644 testv2/testv2_auth/test_logout_url.py create mode 100644 testv2/testv2_management/test_feature_flag_value_types.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 43e6aae4..2b129048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,67 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security +- **Secrets in session storage**: `client_secret` is no longer written to session storage + (which may be a browser cookie); it is kept in process memory. Legacy sessions that still + contain it are cleaned on load and save. +- **Secrets in logs**: the SDK no longer logs the token-exchange payload or response, the OAuth + `state`, session contents, user details or storage values, and no longer raises its own + loggers to INFO. +- **Token endpoint errors**: a failed code exchange now reports only the HTTP status and the + standard OAuth `error` code. The response body is no longer included in exceptions, logs, or + the Flask / FastAPI callback responses, which now return a generic HTTP 400. +- **OAuth callback validation**: `state` is always required and must match the login started + in the same session; the ID token `nonce` is verified. Both are single-use. +- **FastAPI**: the callback no longer forwards the consumed `state` to the post-login URL, and + logout now clears the user's server-side tokens (previously only the session cookie was + cleared). +- **Flask**: the session is bound to a user only after a successful callback. +- **Account API errors**: the permissions, roles and feature flag helpers log only the + exception type and HTTP status when an Account API call fails, not the response body. + +### Changed +- Callbacks without a `state` parameter, and token responses without an ID token (e.g. custom + scopes that omit `openid`), are now rejected. + ### Fixed +- **Token refresh deadlock**: `TokenManager.get_access_token()` hung forever when the access + token had expired and a refresh token was available (non-reentrant lock re-acquired in + `set_tokens`). +- **Account API host**: permission, role and feature flag lookups with `force_api=True` were sent + to the generated client's placeholder host (`your_kinde_subdomain.kinde.com`) and silently + returned empty results. They now use the issuer of the user's access token, falling back to + `KINDE_HOST`. +- **Account API permissions**: with `force_api=True`, `get_permission()` never granted anything + because the API's `{id, name, key}` objects were compared with the key string; + `get_permissions()` now returns keys, as in token mode. +- **Account API feature flags**: with `force_api=True`, flags were read from a non-existent + `data.flags` attribute and always came back empty; the `data.feature_flags` list is now parsed. +- **Management API feature flags**: `environments_api.get_environement_feature_flags()` and + `organizations_api.get_organization_feature_flags()` raised a `ValidationError` for any + boolean or integer flag, because the published spec declares every flag value as a string. + Values are now `bool`, `int` or `str`, and the generator re-applies the correction + (`SPEC_ERRATA` in `generate_management_sdk.py`) until the spec is fixed upstream. +- **Signed out after a restart or on another worker**: session data is keyed by a device ID that + was generated per process and never saved (there is no request at startup), so after a restart, + or with several workers (e.g. `gunicorn -w 4`), the tokens in a user's session couldn't be found. + The device ID is now stored in the user's session and read from there. Sessions created + before this fix will need to sign in once more. +- **Refreshed tokens persisted**: tokens from a refresh were only kept in process memory, so with + server-side or cookie sessions, a restart or another worker kept using (and refreshing with) the + old ones. `UserSession` now writes them back to session storage after every refresh. +- **StorageManager.reset() deadlock**: `reset()` held the class lock while calling `initialize()`, + which takes the same non-reentrant lock; the lock is now reentrant. +- **KindeSessionManagement**: the standalone-only guard never fired, because constructing the + `NullFramework` singleton always marks it initialized; it now raises inside Flask / FastAPI + apps as documented, and requires a standalone `OAuth` client to exist. +- **Feature flag codes**: `get_all_flags()` in token mode returned flags with an empty `code`. +- **Logout redirect**: the logout URL now includes the `redirect` parameter that Kinde's + `/logout` endpoint reads (`redirect_uri` is still sent). The target is + `post_logout_redirect_uri`, then the new `KINDE_POST_LOGOUT_REDIRECT_URI` environment + variable, then `KINDE_REDIRECT_URI` as before, so the built-in `/logout` routes can return + users to your home page instead of the callback. +- Previously unbounded `requests` calls now use a 30 second timeout. - **CI**: Hardened `renovate.json` so Renovate can no longer re-propose bumping the Python `<3.12` `django` pin in `requirements.txt` to Django 6.x (Django 6.0+ requires Python 3.12+). Django is a test-only dependency for this SDK (no runtime usage), and the diff --git a/README.md b/README.md index cc680c50..46bae05c 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,8 @@ Create a `.env` file with your Kinde credentials: KINDE_CLIENT_ID=your_client_id KINDE_CLIENT_SECRET=your_client_secret KINDE_REDIRECT_URI=http://localhost:8000/callback -KINDE_DOMAIN=your_kinde_domain +KINDE_POST_LOGOUT_REDIRECT_URI=http://localhost:8000 +KINDE_HOST=https://your_kinde_domain.kinde.com ``` #### Available Routes @@ -141,7 +142,8 @@ Create a `.env` file with your Kinde credentials: KINDE_CLIENT_ID=your_client_id KINDE_CLIENT_SECRET=your_client_secret KINDE_REDIRECT_URI=http://localhost:5000/callback -KINDE_DOMAIN=your_kinde_domain +KINDE_POST_LOGOUT_REDIRECT_URI=http://localhost:5000 +KINDE_HOST=https://your_kinde_domain.kinde.com ``` #### Available Routes @@ -179,11 +181,41 @@ def protected_route(): For both FastAPI and Flask integrations: 1. Always use HTTPS in production -2. Use a secure session secret key -3. Implement proper state parameter validation +2. Use a secure session secret key (`SECRET_KEY` for Flask, the session middleware key for FastAPI) +3. Use **server-side sessions** (see below) 4. Handle OAuth errors appropriately -5. Implement proper session management -6. Consider implementing CSRF protection +5. Consider implementing CSRF protection for your own state-changing routes + +**What the SDK keeps in the session.** After login the session holds the user's access token, +refresh token and ID token (plus their decoded claims) so the user stays signed in across +requests. During a login it also holds the OAuth `state`, the `nonce` and the PKCE +`code_verifier` until the callback consumes them. The `client_secret` is never written to the +session: it is held in process memory only. + +Because those values are bearer credentials, keep the session data on the server: + +- **Flask**: the integration configures Flask-Session with `SESSION_TYPE=filesystem` by default. + Set `SESSION_FILE_DIR` to a private directory, or use `redis` in multi-instance deployments. +- **FastAPI**: Starlette's `SessionMiddleware` stores the whole session in a signed (not + encrypted) cookie, so the tokens would travel in the cookie. Prefer a server-side session + middleware that keeps only a session ID in the cookie. + +**Logout.** The logout URL carries the ID token as the standard OIDC `id_token_hint` query +parameter, so it can appear in browser history and in the access logs of the identity +provider. Access and refresh tokens are never put in URLs. + +**Callback validation.** The `/callback` route only accepts a callback for a login started by +this SDK in the same session: the `state` parameter is required and must match, and the ID +token's `nonce` must match the one sent with the authorization request. Both are single-use. +As a consequence: + +- callbacks without a `state` parameter are rejected with HTTP 400; +- logins must request the `openid` scope (the SDK default) so that an ID token is returned. + +**Errors and logs.** Callback failures return a generic HTTP 400 message; details such as the +token endpoint's response body are never echoed to the browser or written to the logs. The SDK +does not log secrets, authorization codes or tokens at any log level, and it does not change +the level of your loggers. # Kinde Management API Module diff --git a/generate_management_sdk.py b/generate_management_sdk.py index 9167211d..ff7c0e2a 100755 --- a/generate_management_sdk.py +++ b/generate_management_sdk.py @@ -645,6 +645,50 @@ def add_custom_imports(config: Dict[str, Any]): print("✓ Added custom imports to __init__.py") +# ============================================================================= +# SPEC ERRATA +# ============================================================================= + +# Places where the published spec disagrees with what the API returns. +# Each entry: (model file, text the generator emits, corrected text). +SPEC_ERRATA = [ + # Flag values are typed by "type" (str, int or bool), but the spec declares them as strings, + # so boolean and integer flags failed validation. + ( + "models/get_organization_feature_flags_response_feature_flags_value.py", + " value: Optional[StrictStr] = None", + " value: Optional[Union[StrictBool, StrictInt, StrictStr]] = None", + ), + ( + "models/get_organization_feature_flags_response_feature_flags_value.py", + "from pydantic import BaseModel, ConfigDict, StrictStr, field_validator\n" + "from typing import Any, ClassVar, Dict, List, Optional\n", + "from pydantic import BaseModel, ConfigDict, StrictBool, StrictInt, StrictStr, field_validator\n" + "from typing import Any, ClassVar, Dict, List, Optional, Union\n", + ), +] + + +def apply_spec_errata(config: Dict[str, Any]): + """Correct generated models where the spec doesn't match the API.""" + print_section("Applying Spec Errata") + + output_dir = Path(config["output_dir"]) + for relative_path, generated, corrected in SPEC_ERRATA: + path = output_dir / relative_path + if not path.exists(): + print(f"⚠️ Warning: {relative_path} not found") + continue + content = path.read_text(encoding="utf-8") + if corrected in content: + continue + if generated not in content: + print(f"⚠️ Warning: {relative_path} changed upstream; review the erratum") + continue + path.write_text(content.replace(generated, corrected), encoding="utf-8") + print(f"✓ Patched {relative_path}") + + # ============================================================================= # VALIDATION # ============================================================================= @@ -799,6 +843,9 @@ def generate_single_sdk(skip_tests: bool, no_diff: bool) -> bool: # Step 3: Add custom imports add_custom_imports(config) + + # Step 3b: Correct models where the published spec is wrong + apply_spec_errata(config) # Step 4: Validate if not validate_generation(config): diff --git a/kinde_fastapi/examples/management_token_example.py b/kinde_fastapi/examples/management_token_example.py index ebb8bf09..6aac62d0 100644 --- a/kinde_fastapi/examples/management_token_example.py +++ b/kinde_fastapi/examples/management_token_example.py @@ -26,8 +26,8 @@ # Extract, introspect, and validate management token from header def get_management_token(credentials: HTTPAuthorizationCredentials = Depends(security)) -> ManagementTokenManager: logger.debug("Starting token validation") + # Never log the bearer token, not even a prefix bearer_token = credentials.credentials - logger.debug(f"Received bearer token (first 20 chars): {bearer_token[:20]}...") # SDK config from env domain = os.getenv("KINDE_HOST", "https://app.kinde.com") @@ -53,10 +53,10 @@ def get_management_token(credentials: HTTPAuthorizationCredentials = Depends(sec client_id=client_id, client_secret=client_secret ) - logger.debug(f"ManagementTokenManager instantiated {bearer_token}") + logger.debug("ManagementTokenManager instantiated") introspection_result = token_manager.validate_and_set_via_introspection(bearer_token) - logger.debug(f"Introspection result: {introspection_result}") + logger.debug(f"Introspection active: {bool(introspection_result.get('active'))}") access_token = token_manager.get_access_token() if not access_token: @@ -74,10 +74,10 @@ def get_management_token(credentials: HTTPAuthorizationCredentials = Depends(sec headers={"WWW-Authenticate": "Bearer"} ) except Exception as e: - logger.error(f"Exception in token validation: {str(e)}", exc_info=True) + logger.error(f"Exception in token validation: {type(e).__name__}") raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail=f"Token introspection failed: {str(e)}", + detail="Token introspection failed", headers={"WWW-Authenticate": "Bearer"} ) @@ -107,10 +107,10 @@ async def get_users(token_manager: ManagementTokenManager = Depends(get_manageme "users": users_response.users if users_response.users else [] # In production, filter sensitive data } except Exception as e: - logger.error(f"Error in get_users: {str(e)}", exc_info=True) + logger.error(f"Error in get_users: {type(e).__name__}") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=f"Failed to fetch users: {str(e)}" + detail="Failed to fetch users" ) from e # Run the app diff --git a/kinde_fastapi/framework/fastapi_framework.py b/kinde_fastapi/framework/fastapi_framework.py index 8f0a078a..b6cc7920 100644 --- a/kinde_fastapi/framework/fastapi_framework.py +++ b/kinde_fastapi/framework/fastapi_framework.py @@ -182,7 +182,8 @@ async def callback(request: Request, code: Optional[str] = None, state: Optional return RedirectResponse(login_url) except Exception as ex: - return HTMLResponse(f"Error parsing reauth state: {str(ex)}", status_code=400) + self._logger.warning("Invalid reauth state: %s", type(ex).__name__) + return HTMLResponse("Error parsing reauth state", status_code=400) # Reject before handle_redirect, which would consume the pending state if not code: @@ -190,12 +191,18 @@ async def callback(request: Request, code: Optional[str] = None, state: Optional user_id = request.session.get('user_id') or str(uuid.uuid4()) + # Exception messages can carry token-endpoint details, so they are + # logged by type only and never returned to the browser try: assert self._oauth is not None await self._oauth.handle_redirect(code, user_id, state) - except KindeLoginException as e: + except KindeLoginException: # Missing, forged or stale callbacks are client errors, as in the Flask route - return HTMLResponse(f"Authentication failed: {e}", status_code=400) + self._logger.warning("OAuth callback rejected") + return HTMLResponse("Authentication failed: invalid or expired login request", status_code=400) + except Exception as e: + self._logger.error("OAuth callback failed: %s", type(e).__name__) + return HTMLResponse("Authentication failed", status_code=400) request.session['user_id'] = user_id @@ -209,23 +216,18 @@ async def callback(request: Request, code: Optional[str] = None, state: Optional if not post_login_redirect.startswith('http'): post_login_redirect = str(request.base_url).rstrip('/') + post_login_redirect - parsed = urlparse(post_login_redirect) - if state: - query_dict = parse_qs(parsed.query) - query_dict['state'] = [state] - new_query = urlencode(query_dict, doseq=True) - redirect_url = urlunparse((parsed.scheme, parsed.netloc, parsed.path, parsed.params, new_query, parsed.fragment)) - else: - redirect_url = post_login_redirect - - return RedirectResponse(redirect_url) + # The consumed OAuth state is not forwarded: it would only end up in + # browser history, Referer headers and access logs + return RedirectResponse(post_login_redirect) # Logout route @self.app.get("/logout") async def logout(request: Request): """Logout the user and redirect to Kinde logout page.""" + # Pass the user_id so the SDK also drops the server-side tokens + user_id = request.session.get('user_id') request.session.clear() - return RedirectResponse(url=await self._oauth.logout()) + return RedirectResponse(url=await self._oauth.logout(user_id)) # Register route @self.app.get("/register") diff --git a/kinde_flask/framework/flask_framework.py b/kinde_flask/framework/flask_framework.py index 2d69a15a..43d2a36e 100644 --- a/kinde_flask/framework/flask_framework.py +++ b/kinde_flask/framework/flask_framework.py @@ -3,6 +3,7 @@ from flask_session import Session from kinde_sdk.core.framework.framework_interface import FrameworkInterface from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.core.exceptions import KindeLoginException from ..middleware.framework_middleware import FrameworkMiddleware import os import uuid @@ -229,7 +230,8 @@ def callback(): return redirect(login_url) except Exception as ex: - return f"Error parsing reauth state: {str(ex)}", 400 + logger.warning("Invalid reauth state: %s", type(ex).__name__) + return "Error parsing reauth state", 400 post_login_redirect = session.pop('post_login_redirect_url', None) if post_login_redirect: @@ -244,15 +246,21 @@ def callback(): if not code: return "Authentication failed: Missing authorization code", 400 - # Get or generate user_id - user_id = session.get('user_id', str(uuid.uuid4())) - session['user_id'] = user_id - - # Handle async call to handle_redirect + user_id = session.get('user_id') or str(uuid.uuid4()) + + # Exception messages can carry token-endpoint details, so they are + # logged by type only and never returned to the browser try: self._run_async(self._oauth.handle_redirect(code, user_id, state)) + except KindeLoginException: + logger.warning("OAuth callback rejected") + return "Authentication failed: invalid or expired login request", 400 except Exception as e: - return f"Authentication failed: {str(e)}", 400 + logger.error("OAuth callback failed: %s", type(e).__name__) + return "Authentication failed", 400 + + # Only bind the session to a user once the callback has succeeded + session['user_id'] = user_id if not post_login_redirect.startswith('http'): # Use url_root to get just the scheme and host without the current path @@ -288,7 +296,8 @@ def get_user(): return self._oauth.get_user_info() except Exception as e: - return f"Failed to get user info: {str(e)}", 400 + logger.error("Failed to get user info: %s", type(e).__name__) + return "Failed to get user info", 400 def can_auto_detect(self) -> bool: """ diff --git a/kinde_sdk/auth/base_auth.py b/kinde_sdk/auth/base_auth.py index 8f98ec47..5b6e1355 100644 --- a/kinde_sdk/auth/base_auth.py +++ b/kinde_sdk/auth/base_auth.py @@ -1,5 +1,6 @@ from typing import Optional, Any import logging +import os from kinde_sdk.core.framework.framework_factory import FrameworkFactory from kinde_sdk.auth.user_session import UserSession @@ -49,6 +50,23 @@ def _get_force_api_setting(self) -> bool: return False return token_manager.get_force_api() + @staticmethod + def _describe_api_error(error: Exception) -> str: + """Exception type and HTTP status only: API exception messages include the response body.""" + status = getattr(error, "status", None) + return f"{type(error).__name__} (HTTP {status})" if status else type(error).__name__ + + @staticmethod + def _get_account_api_host(token_manager) -> Optional[str]: + """ + The Account API lives on the Kinde domain that issued the user's token, + which also covers custom domains; KINDE_HOST is the fallback. + """ + claims = token_manager.get_claims() if hasattr(token_manager, "get_claims") else {} + issuer = claims.get("iss") if isinstance(claims, dict) else None + host = issuer if isinstance(issuer, str) and issuer.startswith("https://") else os.getenv("KINDE_HOST") + return host.rstrip("/") if host else None + def _create_authenticated_api_client(self, api_class): """ Create an authenticated API client for the current user. @@ -78,8 +96,12 @@ def _create_authenticated_api_client(self, api_class): from kinde_sdk.frontend.configuration import Configuration from kinde_sdk.frontend.api_client import ApiClient - # Create configuration with the access token - config = Configuration() + host = self._get_account_api_host(token_manager) + if not host: + self._logger.error("Cannot determine the Kinde host for Account API calls") + return None + + config = Configuration(host=host) config.access_token = access_token # Create API client with the configuration diff --git a/kinde_sdk/auth/feature_flags.py b/kinde_sdk/auth/feature_flags.py index 5b1897a3..a7568a0f 100644 --- a/kinde_sdk/auth/feature_flags.py +++ b/kinde_sdk/auth/feature_flags.py @@ -169,10 +169,32 @@ async def get_all_flags( feature_flags = claims.get("feature_flags", {}) return { - code: self._parse_flag_value(flag_data) + code: self._parse_flag_value({**flag_data, "code": code}) for code, flag_data in feature_flags.items() } + @staticmethod + def _flags_from_api_response(response: Any) -> Dict[str, Dict[str, Any]]: + """ + Convert the Account API's ``data.feature_flags`` list of {key, type, value} + into the token claim shape: {code: {"t": "s"|"b"|"i", "v": value}}. + """ + data = getattr(response, "data", None) + items = getattr(data, "feature_flags", None) or [] + flags: Dict[str, Dict[str, Any]] = {} + for item in items: + if isinstance(item, dict): + key, flag_type, value = item.get("key"), item.get("type"), item.get("value") + else: + key, flag_type, value = getattr(item, "key", None), getattr(item, "type", None), getattr(item, "value", None) + if not key: + continue + # oneOf wrappers keep the real value in actual_instance + value = getattr(value, "actual_instance", value) + type_code = {"string": "s", "boolean": "b", "integer": "i"}.get(str(flag_type).lower(), str(flag_type or "")[:1].lower()) + flags[key] = {"t": type_code, "v": value} + return flags + async def _call_account_api(self, flag_code: Optional[str] = None) -> Dict[str, Any]: """ Calls the Kinde Account API to get feature flags. @@ -189,13 +211,11 @@ async def _call_account_api(self, flag_code: Optional[str] = None) -> Dict[str, except Exception as e: # Log error and return empty result if hasattr(self, '_logger'): - self._logger.error(f"Failed to fetch feature flags from API: {str(e)}") + self._logger.error("Failed to fetch feature flags from API: %s", self._describe_api_error(e)) return {} - flags = {} - if response and response.data and hasattr(response.data, "flags"): - flags = response.data.flags or {} - + flags = self._flags_from_api_response(response) + if flag_code is not None: return flags.get(flag_code, {}) return flags diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index e87a6e9c..64f6fb48 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -21,6 +21,24 @@ KindeRetrieveException, ) + +def _describe_token_error(response: requests.Response) -> str: + """Summarise a failed token response without echoing its body. + + Only the status code and the standard OAuth ``error`` code (RFC 6749 5.2) + are kept; the raw body and ``error_description`` may reflect request data. + """ + message = f"Token exchange failed with status {response.status_code}" + try: + body = response.json() + except ValueError: + return message + error = body.get("error") if isinstance(body, dict) else None + if isinstance(error, str) and error.replace("_", "").isalnum() and len(error) <= 64: + message += f" ({error})" + return message + + class OAuth: def __init__( self, @@ -486,9 +504,14 @@ async def logout(self, user_id: Optional[str] = None, logout_options: Dict[str, "client_id": self.client_id, } - # Add redirect URI - redirect_uri = logout_options.get("post_logout_redirect_uri", self.redirect_uri) + redirect_uri = ( + logout_options.get("post_logout_redirect_uri") + or os.getenv("KINDE_POST_LOGOUT_REDIRECT_URI") + or self.redirect_uri + ) if redirect_uri: + # Kinde's logout endpoint reads `redirect`; `redirect_uri` is kept for compatibility + params["redirect"] = redirect_uri params["redirect_uri"] = redirect_uri # Add state if provided @@ -631,7 +654,7 @@ async def exchange_code_for_tokens(self, code: str, code_verifier: Optional[str] response = requests.post(self.token_url, data=data, timeout=REQUEST_TIMEOUT) self._logger.debug(f"[Exchange code for tokens] [{response.status_code}]") if response.status_code != 200: - raise KindeTokenException(f"Token exchange failed: {response.text}") + raise KindeTokenException(_describe_token_error(response)) return response.json() diff --git a/kinde_sdk/auth/permissions.py b/kinde_sdk/auth/permissions.py index 14505bb0..8a411317 100644 --- a/kinde_sdk/auth/permissions.py +++ b/kinde_sdk/auth/permissions.py @@ -97,6 +97,14 @@ async def get_permissions( "permissions": permissions } + @staticmethod + def _permission_key(permission: Any) -> Optional[str]: + if isinstance(permission, str): + return permission + if isinstance(permission, dict): + return permission.get("key") + return getattr(permission, "key", None) + async def _call_account_api(self, permission_key: Optional[str] = None) -> Dict[str, Any]: """ Calls the Kinde Account API to get permissions. @@ -115,7 +123,7 @@ async def _call_account_api(self, permission_key: Optional[str] = None) -> Dict[ except Exception as e: # Log error and return empty result if hasattr(self, '_logger'): - self._logger.error(f"Failed to fetch permissions from API: {str(e)}") + self._logger.error("Failed to fetch permissions from API: %s", self._describe_api_error(e)) if permission_key is None: return {"orgCode": None, "permissions": []} return {"permissionKey": permission_key, "orgCode": None, "isGranted": False} @@ -128,6 +136,9 @@ async def _call_account_api(self, permission_key: Optional[str] = None) -> Dict[ else: permissions = getattr(response, "permissions", []) or [] org_code = getattr(response, "org_code", None) + + # The Account API returns {id, name, key} objects; expose keys, as the token claim does + permissions = [key for key in (self._permission_key(p) for p in permissions) if key] if permission_key is None: return { diff --git a/kinde_sdk/auth/roles.py b/kinde_sdk/auth/roles.py index 3fc0699b..98644448 100644 --- a/kinde_sdk/auth/roles.py +++ b/kinde_sdk/auth/roles.py @@ -154,7 +154,7 @@ async def _call_account_api(self, role_key: Optional[str] = None) -> Dict[str, A response = roles_api.get_user_roles() except Exception as e: - self._logger.error(f"Failed to fetch roles from API: {str(e)}") + self._logger.error("Failed to fetch roles from API: %s", self._describe_api_error(e)) if role_key is None: return { "orgCode": None, diff --git a/kinde_sdk/auth/token_manager.py b/kinde_sdk/auth/token_manager.py index 37d99b3e..6da099ce 100644 --- a/kinde_sdk/auth/token_manager.py +++ b/kinde_sdk/auth/token_manager.py @@ -2,7 +2,7 @@ import requests import threading import logging -from typing import Any, Dict, Optional +from typing import Any, Callable, Dict, Optional import jwt from kinde_sdk.core.helpers import REQUEST_TIMEOUT @@ -35,9 +35,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 + # Re-entrant: get_access_token holds it while refreshing, and the refresh + # path re-acquires it in set_tokens + self.lock = threading.RLock() self.redirect_uri = None # Initialize the redirect_uri attribute self.force_api = False # Initialize force_api setting + # Set by UserSession so refreshed tokens are written back to session storage + self.on_tokens_refreshed: Optional[Callable[[], None]] = None self.initialized = True def set_force_api(self, force_api: bool): @@ -158,6 +162,11 @@ def refresh_access_token(self): token_data = response.json() self.set_tokens(token_data) + if self.on_tokens_refreshed: + try: + self.on_tokens_refreshed() + except Exception as e: + logging.warning("Failed to persist refreshed tokens: %s", type(e).__name__) return self.tokens["access_token"] def get_id_token(self): diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index d785c9b7..9add22c8 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -29,6 +29,7 @@ def set_user_data(self, user_id: str, user_info: Dict[str, Any], token_data: Dic # Set redirect URI if available if "redirect_uri" in user_info: token_manager.set_redirect_uri(user_info["redirect_uri"]) + self._persist_refreshes(user_id, token_manager) self.user_sessions[user_id] = { "user_info": user_info, @@ -44,6 +45,10 @@ def set_user_data(self, user_id: str, user_info: Dict[str, Any], token_data: Dic # Save to persistent storage self._save_to_storage(user_id) + def _persist_refreshes(self, user_id: str, token_manager: TokenManager) -> None: + """Write tokens back to storage after a refresh, so other workers and restarts see them.""" + token_manager.on_tokens_refreshed = lambda: self._save_to_storage(user_id) + def _save_to_storage(self, user_id: str): """Save session data to storage.""" session_data = self.user_sessions.get(user_id) @@ -102,6 +107,7 @@ def _load_from_storage(self, user_id: str) -> bool: # Set redirect URI if available if "redirect_uri" in user_info: token_manager.set_redirect_uri(user_info["redirect_uri"]) + self._persist_refreshes(user_id, token_manager) # Set tokens token_manager.tokens = tokens diff --git a/kinde_sdk/core/session_management.py b/kinde_sdk/core/session_management.py index e3f6490e..e8e9ef4c 100644 --- a/kinde_sdk/core/session_management.py +++ b/kinde_sdk/core/session_management.py @@ -63,17 +63,17 @@ def _get_null_framework(self) -> Optional['NullFramework']: # Check if it's a NullFramework instance if current_framework and isinstance(current_framework, NullFramework): return current_framework - - # If not found in FrameworkFactory, try to get it from the NullFramework singleton - # This handles the case where OAuth creates NullFramework directly - try: - null_framework = NullFramework() - # Check if this is actually being used (has been initialized) - if hasattr(null_framework, '_initialized') and null_framework._initialized: - return null_framework - except Exception: - pass - + + # A web framework owns the session + if current_framework is not None: + return None + + # Standalone OAuth creates the NullFramework singleton directly. Constructing it + # always marks it initialized, so require that an OAuth client is attached. + null_framework = NullFramework() + if null_framework is not None and getattr(null_framework, "_oauth", None) is not None: + return null_framework + return None except Exception as e: diff --git a/kinde_sdk/core/storage/storage_manager.py b/kinde_sdk/core/storage/storage_manager.py index c40d4f8b..2470741b 100644 --- a/kinde_sdk/core/storage/storage_manager.py +++ b/kinde_sdk/core/storage/storage_manager.py @@ -8,7 +8,7 @@ class StorageManager: _instance = None - _lock = threading.Lock() # Lock for thread safety + _lock = threading.RLock() # Reentrant: reset() calls initialize() while holding it def __new__(cls): with cls._lock: @@ -72,18 +72,26 @@ def get_device_id(self) -> str: Returns: str: The current device ID """ + # Framework storage is the user's session, so the ID stored there is the + # one every process and worker must use; the in-process ID is only a + # fallback (memory storage, or no request in context). + storage = self._storage + if storage is not None: + stored_device = storage.get("_device_id") + if isinstance(stored_device, dict) and stored_device.get("value"): + return stored_device["value"] + with self._lock: if not self._device_id: - # Try to load from storage - stored_device = self.get("_device_id") - if stored_device and "value" in stored_device: - self._device_id = stored_device["value"] - else: - # Generate a new device ID - self._device_id = str(uuid.uuid4()) - self.setItems("_device_id", {"value": self._device_id, "timestamp": time.time()}) - - return self._device_id + self._device_id = str(uuid.uuid4()) + device_id = self._device_id + + if storage is not None: + try: + storage.set("_device_id", {"value": device_id, "timestamp": time.time()}) + except Exception: + pass + return device_id @property def storage(self) -> Optional[StorageInterface]: diff --git a/kinde_sdk/management/models/get_organization_feature_flags_response_feature_flags_value.py b/kinde_sdk/management/models/get_organization_feature_flags_response_feature_flags_value.py index 885d4109..ac405ea5 100644 --- a/kinde_sdk/management/models/get_organization_feature_flags_response_feature_flags_value.py +++ b/kinde_sdk/management/models/get_organization_feature_flags_response_feature_flags_value.py @@ -18,8 +18,8 @@ import re # noqa: F401 import json -from pydantic import BaseModel, ConfigDict, StrictStr, field_validator -from typing import Any, ClassVar, Dict, List, Optional +from pydantic import BaseModel, ConfigDict, StrictBool, StrictInt, StrictStr, field_validator +from typing import Any, ClassVar, Dict, List, Optional, Union from typing import Optional, Set from typing_extensions import Self @@ -28,7 +28,7 @@ class GetOrganizationFeatureFlagsResponseFeatureFlagsValue(BaseModel): GetOrganizationFeatureFlagsResponseFeatureFlagsValue """ # noqa: E501 type: Optional[StrictStr] = None - value: Optional[StrictStr] = None + value: Optional[Union[StrictBool, StrictInt, StrictStr]] = None __properties: ClassVar[List[str]] = ["type", "value"] @field_validator('type') diff --git a/testv2/testv2_auth/test_account_api_host.py b/testv2/testv2_auth/test_account_api_host.py new file mode 100644 index 00000000..f2f0afde --- /dev/null +++ b/testv2/testv2_auth/test_account_api_host.py @@ -0,0 +1,61 @@ +""" +Account API calls (force_api) must target the Kinde domain, not the +generated client's placeholder host. +""" +from unittest.mock import MagicMock + +from kinde_sdk.auth.base_auth import BaseAuth +from kinde_sdk.frontend.api.permissions_api import PermissionsApi + +PLACEHOLDER = "your_kinde_subdomain" + + +def _auth_with_claims(claims): + auth = BaseAuth() + token_manager = MagicMock() + token_manager.get_access_token.return_value = "access-token" + token_manager.get_claims.return_value = claims + auth._get_token_manager = lambda: token_manager + return auth + + +def test_host_comes_from_token_issuer(monkeypatch): + monkeypatch.setenv("KINDE_HOST", "https://fallback.kinde.com") + auth = _auth_with_claims({"iss": "https://auth.example.com/"}) + + api = auth._create_authenticated_api_client(PermissionsApi) + + assert api.api_client.configuration.host == "https://auth.example.com" + assert api.api_client.configuration.access_token == "access-token" + + +def test_host_falls_back_to_kinde_host(monkeypatch): + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") + auth = _auth_with_claims({}) + + api = auth._create_authenticated_api_client(PermissionsApi) + + assert api.api_client.configuration.host == "https://acme.kinde.com" + + +def test_non_https_issuer_is_ignored(monkeypatch): + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") + auth = _auth_with_claims({"iss": "http://attacker.example"}) + + api = auth._create_authenticated_api_client(PermissionsApi) + + assert api.api_client.configuration.host == "https://acme.kinde.com" + + +def test_no_client_without_a_host(monkeypatch): + monkeypatch.delenv("KINDE_HOST", raising=False) + auth = _auth_with_claims({}) + + assert auth._create_authenticated_api_client(PermissionsApi) is None + + +def test_never_uses_placeholder_host(monkeypatch): + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") + for claims in ({}, {"iss": "https://acme.kinde.com"}): + api = _auth_with_claims(claims)._create_authenticated_api_client(PermissionsApi) + assert PLACEHOLDER not in api.api_client.configuration.host diff --git a/testv2/testv2_auth/test_feature_flags.py b/testv2/testv2_auth/test_feature_flags.py index 970a6ff3..58eaa53b 100644 --- a/testv2/testv2_auth/test_feature_flags.py +++ b/testv2/testv2_auth/test_feature_flags.py @@ -238,4 +238,53 @@ async def test_get_feature_flag_force_api_false_no_feature_flags(self, mock_sess assert isinstance(result, FeatureFlag) assert result.code == "test" assert result.type == "unknown" - assert result.value == None \ No newline at end of file + assert result.value == None + +class TestFeatureFlagsRealShapes: + """Token claims and Account API responses as Kinde actually returns them.""" + + @pytest.mark.asyncio + async def test_get_all_flags_keeps_codes_from_token_claim_keys(self, mock_framework_factory, mock_session_manager, mock_token_manager): + # Real tokens don't repeat the code inside each flag + mock_token_manager.get_claims.return_value = {"feature_flags": {"theme": {"t": "s", "v": "pink"}}} + with patch.object(feature_flags, "_session_manager", mock_session_manager): + flags = await feature_flags.get_all_flags() + assert flags["theme"].code == "theme" + assert flags["theme"].value == "pink" + + @pytest.mark.asyncio + async def test_account_api_response_is_parsed(self): + from kinde_sdk.frontend.models import ( + GetFeatureFlagsResponse, + GetFeatureFlagsResponseData, + GetFeatureFlagsResponseDataFeatureFlagsInner, + GetFeatureFlagsResponseDataFeatureFlagsInnerValue, + ) + + def flag(key, flag_type, value): + return GetFeatureFlagsResponseDataFeatureFlagsInner( + id=f"flag_{key}", name=key, key=key, type=flag_type, + value=GetFeatureFlagsResponseDataFeatureFlagsInnerValue(value), + ) + + response = GetFeatureFlagsResponse(data=GetFeatureFlagsResponseData(feature_flags=[ + flag("theme", "string", "pink"), + flag("is_dark_mode", "boolean", True), + flag("competitions_limit", "integer", 5), + ])) + api = Mock() + api.get_feature_flags.return_value = response + + with patch.object(feature_flags, "_create_authenticated_api_client", return_value=api), \ + patch.object(feature_flags, "_get_force_api_setting", return_value=True): + all_flags = await feature_flags.get_all_flags() + dark_mode = await feature_flags.get_flag("is_dark_mode", default_value=False) + missing = await feature_flags.get_flag("nope", default_value="fallback") + + assert {code: (f.type, f.value) for code, f in all_flags.items()} == { + "theme": ("string", "pink"), + "is_dark_mode": ("boolean", True), + "competitions_limit": ("integer", 5), + } + assert dark_mode.value is True + assert missing.value == "fallback" diff --git a/testv2/testv2_auth/test_log_redaction.py b/testv2/testv2_auth/test_log_redaction.py new file mode 100644 index 00000000..2f19c823 --- /dev/null +++ b/testv2/testv2_auth/test_log_redaction.py @@ -0,0 +1,403 @@ +""" +Regression tests: secrets must not reach logs or HTTP responses. + +A full login -> callback -> refresh -> logout flow is driven for the +standalone client and for the real Flask / FastAPI routes with every SDK +logger at DEBUG. Afterwards no log output (including formatted exceptions) +and no HTTP response may contain the client secret, the authorization code, +the PKCE verifier, the OAuth state / nonce or any token value. + +Failed token exchanges must not echo the token endpoint's response body, +either in the raised exception or in the callback's HTTP response. +""" +import asyncio +import logging +import tempfile +import threading +import time +from unittest.mock import AsyncMock, MagicMock, patch +from urllib.parse import parse_qs, urlparse + +import jwt +import pytest + +from kinde_sdk.auth.api_options import ApiOptions +from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.auth.token_manager import TokenManager +from kinde_sdk.core.exceptions import KindeTokenException +from kinde_sdk.core.framework.framework_factory import FrameworkFactory +from kinde_sdk.core.storage.storage_manager import StorageManager + +CLIENT_ID = "redaction_client_id" +CLIENT_SECRET = "SECRET-client-secret-9f2c" +AUTH_CODE = "SECRET-auth-code-71bd" +REDIRECT_URI = "http://localhost/callback" +HOST = "https://example.kinde.com" +JWT_KEY = "test-secret-key-of-sufficient-length-123" +# Something a token endpoint might reflect back in an error body +REFLECTED = "SECRET-reflected-body-4e1a" + + +def _jwt(**claims): + return jwt.encode(claims, JWT_KEY, algorithm="HS256") + + +class Tokens: + def __init__(self, nonce, tag="1"): + self.access = _jwt(sub="kp_user", tag=f"access-{tag}") + self.refresh = f"SECRET-refresh-token-{tag}" + self.id = _jwt(sub="kp_user", nonce=nonce, tag=f"id-{tag}") + + def response(self, expires_in=3600): + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = { + "access_token": self.access, + "refresh_token": self.refresh, + "id_token": self.id, + "token_type": "bearer", + "expires_in": expires_in, + } + resp.text = str(resp.json.return_value) + return resp + + def values(self): + return [self.access, self.refresh, self.id] + + +def _error_response(status=400): + resp = MagicMock() + resp.status_code = status + resp.json.return_value = {"error": "invalid_grant", "error_description": f"bad code {REFLECTED}"} + resp.text = f'{{"error":"invalid_grant","error_description":"bad code {REFLECTED}"}}' + return resp + + +def _query(url): + return {k: v[0] for k, v in parse_qs(urlparse(url).query).items()} + + +def _assert_absent(haystack, secrets, where): + for secret in secrets: + assert secret and secret not in haystack, f"secret leaked into {where}: {secret[:12]}..." + + +@pytest.fixture +def isolated(monkeypatch): + """Isolate the process-wide singletons the SDK relies on.""" + sm = StorageManager() + saved = (sm._storage, sm._device_id, sm._storage_type) + TokenManager.reset_instances() + with patch.object(FrameworkFactory, "_framework_instance", None): + yield + sm._storage, sm._device_id, sm._storage_type = saved + TokenManager.reset_instances() + + +class SdkLogs: + """DEBUG output of the SDK's own loggers, exceptions included. + + Third-party access logs (e.g. the test client's) are left out: they log + the callback URL, which carries the code and state by OAuth design. + """ + + def __init__(self, caplog): + self._caplog = caplog + + @property + def text(self): + formatter = logging.Formatter("%(name)s %(message)s") + return "\n".join( + formatter.format(record) + for record in self._caplog.records + if record.name.startswith("kinde") or record.name == "root" + ) + + +@pytest.fixture +def debug_logs(caplog): + caplog.set_level(logging.DEBUG) + return SdkLogs(caplog) + + +@pytest.fixture +def network(): + """Token endpoint (exchange vs refresh / revoke) and userinfo lookup. + + Both SDK modules share the global ``requests.post``, so a single mock + routes on the grant type. + """ + exchange, token_manager_post = MagicMock(), MagicMock() + + def route(url, data=None, **kwargs): + if url.endswith("/oauth2/token") and data and data.get("grant_type") == "authorization_code": + return exchange(url, data=data, **kwargs) + return token_manager_post(url, data=data, **kwargs) + + with patch("requests.post", side_effect=route), \ + patch("kinde_sdk.auth.oauth.helper_get_user_details", new=AsyncMock(return_value={"id": "kp_user"})): + yield exchange, token_manager_post + + +# --------------------------------------------------------------------------- +# Standalone client +# --------------------------------------------------------------------------- + +class TestStandaloneFlowLogs: + + def test_full_flow_logs_no_secrets(self, isolated, debug_logs, network): + exchange, token_manager_post = network + oauth = OAuth(client_id=CLIENT_ID, client_secret=CLIENT_SECRET, redirect_uri=REDIRECT_URI, host=HOST) + + q = _query(asyncio.run(oauth.login())) + verifier = StorageManager().get("user:code_verifier")["value"] + first = Tokens(q["nonce"], "1") + second = Tokens(q["nonce"], "2") + # Expired right away, so the next access goes through the refresh path + exchange.return_value = first.response(expires_in=0) + token_manager_post.return_value = second.response() + + asyncio.run(oauth.handle_redirect(AUTH_CODE, "u1", q["state"])) + assert oauth._session_manager.get_token_manager("u1").get_access_token() == second.access + asyncio.run(oauth.logout("u1")) + + _assert_absent( + debug_logs.text, + [CLIENT_SECRET, AUTH_CODE, verifier, q["state"], q["nonce"], *first.values(), *second.values()], + "logs", + ) + + def test_failed_exchange_does_not_echo_response_body(self, isolated, debug_logs, network): + exchange, _ = network + oauth = OAuth(client_id=CLIENT_ID, client_secret=CLIENT_SECRET, redirect_uri=REDIRECT_URI, host=HOST) + q = _query(asyncio.run(oauth.login())) + exchange.return_value = _error_response() + + with pytest.raises(KindeTokenException) as exc_info: + asyncio.run(oauth.handle_redirect(AUTH_CODE, "u1", q["state"])) + + message = str(exc_info.value) + assert "400" in message and "invalid_grant" in message + _assert_absent(message, [REFLECTED, AUTH_CODE, CLIENT_SECRET], "exception message") + _assert_absent(debug_logs.text, [REFLECTED, AUTH_CODE, CLIENT_SECRET], "logs") + + def test_failed_exchange_with_non_json_body(self, isolated, network): + exchange, _ = network + oauth = OAuth(client_id=CLIENT_ID, client_secret=CLIENT_SECRET, redirect_uri=REDIRECT_URI, host=HOST) + q = _query(asyncio.run(oauth.login())) + resp = MagicMock(status_code=502, text=f"{REFLECTED}") + resp.json.side_effect = ValueError("not json") + exchange.return_value = resp + + with pytest.raises(KindeTokenException) as exc_info: + asyncio.run(oauth.handle_redirect(AUTH_CODE, "u1", q["state"])) + assert "502" in str(exc_info.value) + assert REFLECTED not in str(exc_info.value) + + def test_untrusted_error_code_is_dropped(self, isolated, network): + exchange, _ = network + oauth = OAuth(client_id=CLIENT_ID, client_secret=CLIENT_SECRET, redirect_uri=REDIRECT_URI, host=HOST) + q = _query(asyncio.run(oauth.login())) + resp = _error_response() + resp.json.return_value = {"error": f""} + exchange.return_value = resp + + with pytest.raises(KindeTokenException) as exc_info: + asyncio.run(oauth.handle_redirect(AUTH_CODE, "u1", q["state"])) + assert REFLECTED not in str(exc_info.value) + + +class TestExpiredTokenRefresh: + + def test_get_access_token_refreshes_without_deadlock(self, isolated, network): + """get_access_token holds the lock while refreshing; set_tokens re-acquires it.""" + _, token_manager_post = network + fresh = Tokens("n", "fresh") + token_manager_post.return_value = fresh.response() + tm = TokenManager("deadlock-user", CLIENT_ID, None, f"{HOST}/oauth2/token") + tm.tokens = {"access_token": "old", "refresh_token": "r", "expires_at": time.time() - 1} + + result = {} + worker = threading.Thread(target=lambda: result.setdefault("token", tm.get_access_token()), daemon=True) + worker.start() + worker.join(5) + + assert not worker.is_alive(), "get_access_token deadlocked while refreshing" + assert result["token"] == fresh.access + + +# --------------------------------------------------------------------------- +# Flask routes +# --------------------------------------------------------------------------- + +@pytest.fixture +def flask_app(isolated, monkeypatch): + from flask import Flask + monkeypatch.setenv("SECRET_KEY", "test-secret") + monkeypatch.setenv("SESSION_FILE_DIR", tempfile.mkdtemp(prefix="kinde_redaction_test_")) + app = Flask(__name__) + oauth = OAuth( + client_id=CLIENT_ID, client_secret=CLIENT_SECRET, redirect_uri=REDIRECT_URI, + host=HOST, framework="flask", app=app, + ) + app.config["TESTING"] = True + return app, oauth + + +class TestFlaskRoutes: + + def test_full_flow_logs_and_responses_have_no_secrets(self, flask_app, debug_logs, network): + exchange, token_manager_post = network + app, oauth = flask_app + client = app.test_client() + + q = _query(client.get("/login").headers["Location"]) + tokens = Tokens(q["nonce"]) + exchange.return_value = tokens.response() + token_manager_post.return_value = MagicMock(status_code=200) + + callback = client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + logout = client.get("/logout") + + assert callback.status_code == 302 + assert logout.status_code == 302 + _assert_absent(debug_logs.text, [CLIENT_SECRET, AUTH_CODE, q["state"], q["nonce"], *tokens.values()], "logs") + _assert_absent(callback.get_data(as_text=True), [CLIENT_SECRET, *tokens.values()], "callback body") + _assert_absent(callback.headers["Location"], [CLIENT_SECRET, AUTH_CODE, q["state"]], "callback redirect") + # OIDC logout carries only the ID token, as id_token_hint + assert _query(logout.headers["Location"]).get("id_token_hint") == tokens.id + _assert_absent(logout.headers["Location"], [CLIENT_SECRET, tokens.access, tokens.refresh], "logout redirect") + + def test_failed_exchange_returns_generic_error(self, flask_app, debug_logs, network): + exchange, _ = network + app, _ = flask_app + client = app.test_client() + q = _query(client.get("/login").headers["Location"]) + exchange.return_value = _error_response() + + r = client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + + assert r.status_code == 400 + body = r.get_data(as_text=True) + _assert_absent(body, [REFLECTED, AUTH_CODE, CLIENT_SECRET, "invalid_grant"], "response body") + _assert_absent(debug_logs.text, [REFLECTED, AUTH_CODE, CLIENT_SECRET], "logs") + + def test_failed_callback_does_not_bind_session_user(self, flask_app, network): + exchange, _ = network + app, _ = flask_app + client = app.test_client() + q = _query(client.get("/login").headers["Location"]) + exchange.return_value = _error_response() + + client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + + with client.session_transaction() as session: + assert "user_id" not in session + + +# --------------------------------------------------------------------------- +# FastAPI routes +# --------------------------------------------------------------------------- + +@pytest.fixture +def fastapi_app(isolated): + from fastapi import FastAPI + from fastapi.testclient import TestClient + from starlette.middleware.sessions import SessionMiddleware + app = FastAPI() + oauth = OAuth( + client_id=CLIENT_ID, client_secret=CLIENT_SECRET, redirect_uri=REDIRECT_URI, + host=HOST, framework="fastapi", app=app, + ) + app.add_middleware(SessionMiddleware, secret_key="test-secret") + return TestClient(app, raise_server_exceptions=False, follow_redirects=False), oauth + + +class TestFastAPIRoutes: + + def test_full_flow_logs_and_responses_have_no_secrets(self, fastapi_app, debug_logs, network): + exchange, token_manager_post = network + client, oauth = fastapi_app + + q = _query(client.get("/login").headers["location"]) + tokens = Tokens(q["nonce"]) + exchange.return_value = tokens.response() + token_manager_post.return_value = MagicMock(status_code=200) + + callback = client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + assert callback.status_code in (302, 307) + assert len(oauth._session_manager.user_sessions) == 1 + + logout = client.get("/logout") + assert logout.status_code in (302, 307) + + _assert_absent(debug_logs.text, [CLIENT_SECRET, AUTH_CODE, q["state"], q["nonce"], *tokens.values()], "logs") + _assert_absent(callback.text, [CLIENT_SECRET, *tokens.values()], "callback body") + # The consumed state is not forwarded to the post-login page + _assert_absent(callback.headers["location"], [AUTH_CODE, q["state"]], "callback redirect") + assert _query(logout.headers["location"]).get("id_token_hint") == tokens.id + _assert_absent(logout.headers["location"], [CLIENT_SECRET, tokens.access, tokens.refresh], "logout redirect") + + def test_logout_clears_server_side_tokens(self, fastapi_app, network): + exchange, token_manager_post = network + client, oauth = fastapi_app + q = _query(client.get("/login").headers["location"]) + exchange.return_value = Tokens(q["nonce"]).response() + token_manager_post.return_value = MagicMock(status_code=200) + client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + assert oauth._session_manager.user_sessions + + client.get("/logout") + + assert oauth._session_manager.user_sessions == {} + # Best-effort revocation of the access token was attempted + assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + + def test_failed_exchange_returns_400_without_body_echo(self, fastapi_app, debug_logs, network): + exchange, _ = network + client, _ = fastapi_app + q = _query(client.get("/login").headers["location"]) + exchange.return_value = _error_response() + + r = client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + + assert r.status_code == 400 + _assert_absent(r.text, [REFLECTED, AUTH_CODE, CLIENT_SECRET, "invalid_grant"], "response body") + _assert_absent(debug_logs.text, [REFLECTED, AUTH_CODE, CLIENT_SECRET], "logs") + + +# --------------------------------------------------------------------------- +# Account API helpers +# --------------------------------------------------------------------------- + +class TestAccountApiErrorLogs: + """A failed Account API call logs the exception type and status, never the response body.""" + + @pytest.fixture + def api_error(self): + from kinde_sdk.frontend.exceptions import ApiException + error = ApiException(status=401, reason="Unauthorized") + error.body = f'{{"errors":[{{"detail":"{REFLECTED}"}}]}}' + return error + + @pytest.mark.parametrize("helper_name, api_path, method, call", [ + ("permissions", "kinde_sdk.auth.permissions.PermissionsApi", "get_user_permissions", + lambda h: h.get_permission("read:x", ApiOptions(force_api=True))), + ("roles", "kinde_sdk.auth.roles.RolesApi", "get_user_roles", + lambda h: h.get_roles(ApiOptions(force_api=True))), + ("feature_flags", "kinde_sdk.auth.feature_flags.FeatureFlagsApi", "get_feature_flags", + lambda h: h.get_all_flags(ApiOptions(force_api=True))), + ]) + def test_body_not_logged(self, debug_logs, api_error, helper_name, api_path, method, call): + import importlib + helper = getattr(importlib.import_module("kinde_sdk.auth"), helper_name) + api = MagicMock() + getattr(api, method).side_effect = api_error + with patch.object(type(helper), "_create_authenticated_api_client", return_value=api), \ + patch.object(type(helper), "_get_token_manager", return_value=MagicMock()): + try: + asyncio.run(call(helper)) + except Exception: + pass + assert "ApiException (HTTP 401)" in debug_logs.text + assert REFLECTED not in debug_logs.text diff --git a/testv2/testv2_auth/test_logout_url.py b/testv2/testv2_auth/test_logout_url.py new file mode 100644 index 00000000..c1b4936d --- /dev/null +++ b/testv2/testv2_auth/test_logout_url.py @@ -0,0 +1,48 @@ +"""Logout URL parameters understood by Kinde's /logout endpoint.""" +import asyncio +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse + +import pytest + +from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.core.framework.framework_factory import FrameworkFactory +from kinde_sdk.core.storage.storage_manager import StorageManager + + +@pytest.fixture +def oauth(): + sm = StorageManager() + saved = (sm._storage, sm._device_id, sm._storage_type) + with patch.object(FrameworkFactory, "_framework_instance", None): + yield OAuth( + client_id="logout_client", + redirect_uri="http://localhost/callback", + host="https://example.kinde.com", + ) + sm._storage, sm._device_id, sm._storage_type = saved + + +def _params(url): + return {k: v[0] for k, v in parse_qs(urlparse(url).query).items()} + + +def test_explicit_post_logout_redirect(oauth, monkeypatch): + monkeypatch.setenv("KINDE_POST_LOGOUT_REDIRECT_URI", "http://localhost/from-env") + url = asyncio.run(oauth.logout(logout_options={"post_logout_redirect_uri": "http://localhost/bye"})) + + params = _params(url) + assert params["redirect"] == "http://localhost/bye" + assert params["redirect_uri"] == "http://localhost/bye" + + +def test_post_logout_redirect_from_env(oauth, monkeypatch): + monkeypatch.setenv("KINDE_POST_LOGOUT_REDIRECT_URI", "http://localhost/") + assert _params(asyncio.run(oauth.logout()))["redirect"] == "http://localhost/" + + +def test_falls_back_to_redirect_uri(oauth, monkeypatch): + monkeypatch.delenv("KINDE_POST_LOGOUT_REDIRECT_URI", raising=False) + params = _params(asyncio.run(oauth.logout())) + assert params["redirect"] == "http://localhost/callback" + assert params["client_id"] == "logout_client" diff --git a/testv2/testv2_auth/test_permissions.py b/testv2/testv2_auth/test_permissions.py index 5b8854b4..ab4197ce 100644 --- a/testv2/testv2_auth/test_permissions.py +++ b/testv2/testv2_auth/test_permissions.py @@ -183,4 +183,49 @@ async def test_get_permission_force_api_false_no_permissions(self, mock_framewor result = await permissions.get_permission("create:todos", options) assert result["permissionKey"] == "create:todos" assert result["orgCode"] == "org_123" - assert result["isGranted"] is False \ No newline at end of file + assert result["isGranted"] is False + + +class TestPermissionsAccountApiResponse: + """_call_account_api against the real generated response models.""" + + @staticmethod + def _api_returning(*keys): + from kinde_sdk.frontend.models import ( + GetUserPermissionsResponse, + GetUserPermissionsResponseData, + GetUserPermissionsResponseDataPermissionsInner, + ) + response = GetUserPermissionsResponse( + data=GetUserPermissionsResponseData( + org_code="org_123", + permissions=[ + GetUserPermissionsResponseDataPermissionsInner(id=f"perm_{i}", name=key.title(), key=key) + for i, key in enumerate(keys) + ], + ) + ) + api = Mock() + api.get_user_permissions.return_value = response + return api + + @pytest.mark.asyncio + async def test_granted_permission_is_found_by_key(self): + api = self._api_returning("create:todos", "read:todos") + with patch.object(permissions, "_create_authenticated_api_client", return_value=api): + result = await permissions._call_account_api("create:todos") + assert result == {"permissionKey": "create:todos", "orgCode": "org_123", "isGranted": True} + + @pytest.mark.asyncio + async def test_missing_permission_is_not_granted(self): + api = self._api_returning("read:todos") + with patch.object(permissions, "_create_authenticated_api_client", return_value=api): + result = await permissions._call_account_api("delete:todos") + assert result["isGranted"] is False + + @pytest.mark.asyncio + async def test_all_permissions_are_returned_as_keys(self): + api = self._api_returning("create:todos", "read:todos") + with patch.object(permissions, "_create_authenticated_api_client", return_value=api): + result = await permissions._call_account_api() + assert result == {"orgCode": "org_123", "permissions": ["create:todos", "read:todos"]} \ No newline at end of file diff --git a/testv2/testv2_auth/test_user_session.py b/testv2/testv2_auth/test_user_session.py index 00425726..99d81937 100644 --- a/testv2/testv2_auth/test_user_session.py +++ b/testv2/testv2_auth/test_user_session.py @@ -432,4 +432,41 @@ def test_cleanup_expired_sessions_edge_cases(self): if __name__ == "__main__": - pytest.main(["-xvs", __file__]) \ No newline at end of file + pytest.main(["-xvs", __file__]) + +class TestRefreshedTokensPersisted(unittest.TestCase): + """A refresh must reach session storage, or other workers and restarts keep the old tokens.""" + + setUp = TestUserSession.setUp + + def _refresh_response(self): + response = MagicMock() + response.json.return_value = {"access_token": "refreshed_access", "refresh_token": "rotated_refresh", + "expires_in": 3600} + return response + + def test_refresh_after_sign_in_is_saved(self): + self.user_session.set_user_data(self.user_id, self.user_info, self.token_data) + token_manager = self.user_session.get_token_manager(self.user_id) + with patch("kinde_sdk.auth.token_manager.requests.post", return_value=self._refresh_response()): + token_manager.refresh_access_token() + stored = self.storage_dict[self.user_id]["tokens"] + self.assertEqual(stored["access_token"], "refreshed_access") + self.assertEqual(stored["refresh_token"], "rotated_refresh") + + def test_refresh_after_loading_from_storage_is_saved(self): + self.user_session.set_user_data(self.user_id, self.user_info, self.token_data) + TokenManager._instances = {} + restarted = UserSession() + token_manager = restarted.get_token_manager(self.user_id) + with patch("kinde_sdk.auth.token_manager.requests.post", return_value=self._refresh_response()): + token_manager.refresh_access_token() + self.assertEqual(self.storage_dict[self.user_id]["tokens"]["access_token"], "refreshed_access") + self.assertNotIn("client_secret", self.storage_dict[self.user_id]["user_info"]) + + def test_storage_failure_does_not_break_refresh(self): + self.user_session.set_user_data(self.user_id, self.user_info, self.token_data) + token_manager = self.user_session.get_token_manager(self.user_id) + self.mock_storage_manager.setItems.side_effect = RuntimeError("storage down") + with patch("kinde_sdk.auth.token_manager.requests.post", return_value=self._refresh_response()): + self.assertEqual(token_manager.refresh_access_token(), "refreshed_access") diff --git a/testv2/testv2_core/test_kinde_session_management.py b/testv2/testv2_core/test_kinde_session_management.py index 421af84b..da879e37 100644 --- a/testv2/testv2_core/test_kinde_session_management.py +++ b/testv2/testv2_core/test_kinde_session_management.py @@ -258,3 +258,29 @@ def test_multiple_session_management_instances(self): if __name__ == '__main__': unittest.main() + + +class TestKindeSessionManagementUnpatched(unittest.TestCase): + """The standalone-only guard, without mocking NullFramework away.""" + + def setUp(self): + FrameworkFactory._framework_instance = None + NullFramework._instance = None + + tearDown = setUp + + def test_raises_when_a_web_framework_is_active(self): + mock_framework = MagicMock() + mock_framework.get_name.return_value = "flask" + with patch.object(FrameworkFactory, "get_framework_instance", return_value=mock_framework): + with self.assertRaises(RuntimeError): + KindeSessionManagement() + + def test_raises_when_no_oauth_client_exists(self): + NullFramework() # constructed by some other code path, but no OAuth attached + with self.assertRaises(RuntimeError): + KindeSessionManagement() + + def test_available_after_standalone_oauth(self): + OAuth(client_id="test_client_id", redirect_uri="http://localhost/callback", framework=None) + self.assertFalse(KindeSessionManagement().is_user_logged_in()) diff --git a/testv2/testv2_core/test_storage_manager.py b/testv2/testv2_core/test_storage_manager.py index 93376f69..27108447 100644 --- a/testv2/testv2_core/test_storage_manager.py +++ b/testv2/testv2_core/test_storage_manager.py @@ -80,23 +80,46 @@ def test_get(self): """Test get method.""" test_key = "test_key" test_value = {"value": "test_value"} - self.mock_storage.get.return_value = test_value + namespaced = f"device:{self.test_device_id}:{test_key}" + self.mock_storage.get.side_effect = lambda key: test_value if key == namespaced else None result = self.storage_manager.get(test_key) self.assertEqual(result, test_value) - self.mock_storage.get.assert_called_once_with(f"device:{self.test_device_id}:{test_key}") + self.mock_storage.get.assert_called_with(namespaced) def test_setItems(self): """Test setItems method.""" test_key = "test_key" test_value = {"value": "test_value"} + self.mock_storage.get.return_value = None self.storage_manager.setItems(test_key, test_value) - self.mock_storage.set.assert_called_once_with( + self.mock_storage.set.assert_called_with( f"device:{self.test_device_id}:{test_key}", test_value ) + def test_device_id_comes_from_the_session(self): + """Another process or worker must find the keys the signing-in process wrote.""" + session_device_id = str(uuid.uuid4()) + self.mock_storage.get.side_effect = ( + lambda key: {"value": session_device_id} if key == "_device_id" else None + ) + + self.storage_manager.setItems("user_1", {"tokens": {}}) + + self.mock_storage.set.assert_called_once_with(f"device:{session_device_id}:user_1", {"tokens": {}}) + self.assertNotEqual(session_device_id, self.test_device_id) + + def test_device_id_is_pinned_in_a_new_session(self): + self.mock_storage.get.return_value = None + + self.assertEqual(self.storage_manager.get_device_id(), self.test_device_id) + + key, value = self.mock_storage.set.call_args.args + self.assertEqual(key, "_device_id") + self.assertEqual(value["value"], self.test_device_id) + def test_set(self): """Test set method for flat access token.""" test_token = "test_access_token" @@ -149,11 +172,19 @@ def worker(): self.assertTrue(all(r == results[0] for r in results)) def test_reset(self): - """Test reset method.""" - #self.storage_manager.reset() - #self.assertIsNone(self.storage_manager._storage) - #self.assertIsNone(self.storage_manager._device_id) - #self.assertEqual(self.storage_manager._storage_type, "memory") + """reset() re-initializes fresh memory storage without deadlocking on its own lock.""" + self.storage_manager.initialize({"type": "memory"}, device_id="before-reset") + self.storage_manager.setItems("key", {"value": 1}) + + worker = threading.Thread(target=self.storage_manager.reset, daemon=True) + worker.start() + worker.join(timeout=5) + self.assertFalse(worker.is_alive(), "reset() deadlocked") + + self.assertEqual(self.storage_manager._storage_type, "memory") + self.assertIsNotNone(self.storage_manager._storage) + self.assertNotEqual(self.storage_manager.get_device_id(), "before-reset") + self.assertIsNone(self.storage_manager.get("key")) def test_auto_initialize(self): """Test auto-initialization when storage is None.""" diff --git a/testv2/testv2_management/test_feature_flag_value_types.py b/testv2/testv2_management/test_feature_flag_value_types.py new file mode 100644 index 00000000..ea7e037b --- /dev/null +++ b/testv2/testv2_management/test_feature_flag_value_types.py @@ -0,0 +1,49 @@ +"""Feature flag values come back typed (bool, int or str), not always as strings.""" +import pytest + +from kinde_sdk.management.models.get_environment_feature_flags_response import GetEnvironmentFeatureFlagsResponse +from kinde_sdk.management.models.get_organization_feature_flags_response import GetOrganizationFeatureFlagsResponse + + +API_RESPONSE = { + "code": "OK", + "message": "Success", + "feature_flags": { + "dark_mode": {"type": "bool", "value": True}, + "social_media": {"type": "bool", "value": False}, + "max_projects": {"type": "int", "value": 5}, + "theme": {"type": "str", "value": "dark"}, + }, +} + + +@pytest.mark.parametrize("model", [GetEnvironmentFeatureFlagsResponse, GetOrganizationFeatureFlagsResponse]) +def test_typed_flag_values_parse(model): + flags = model.from_dict(API_RESPONSE).feature_flags + assert flags["dark_mode"].value is True + assert flags["social_media"].value is False + assert flags["max_projects"].value == 5 + assert flags["theme"].value == "dark" + assert model.from_dict(API_RESPONSE).to_dict() == API_RESPONSE + + +def test_generator_erratum_patches_generated_model(tmp_path): + import generate_management_sdk as generator + + target = tmp_path / "models" / "get_organization_feature_flags_response_feature_flags_value.py" + target.parent.mkdir() + generated = ( + "from pydantic import BaseModel, ConfigDict, StrictStr, field_validator\n" + "from typing import Any, ClassVar, Dict, List, Optional\n" + "class M(BaseModel):\n" + " value: Optional[StrictStr] = None\n" + ) + target.write_text(generated) + + generator.apply_spec_errata({"output_dir": str(tmp_path)}) + patched = target.read_text() + assert "Optional[Union[StrictBool, StrictInt, StrictStr]]" in patched + assert "StrictBool, StrictInt, StrictStr, field_validator" in patched + + generator.apply_spec_errata({"output_dir": str(tmp_path)}) + assert target.read_text() == patched From ddab53f7b53c80b55a1d995cadf9028dd5b8ec6d Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Thu, 8 Oct 2026 06:23:36 +0200 Subject: [PATCH 07/20] fix(auth): only call the Account API over HTTPS The token issuer was already required to be https, but the KINDE_HOST fallback was not checked. The client sends the user's access token, so refuse to create it for a plain HTTP host. --- kinde_sdk/auth/base_auth.py | 7 +++++-- testv2/testv2_auth/test_account_api_host.py | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/kinde_sdk/auth/base_auth.py b/kinde_sdk/auth/base_auth.py index 5b6e1355..834e271f 100644 --- a/kinde_sdk/auth/base_auth.py +++ b/kinde_sdk/auth/base_auth.py @@ -65,7 +65,10 @@ def _get_account_api_host(token_manager) -> Optional[str]: claims = token_manager.get_claims() if hasattr(token_manager, "get_claims") else {} issuer = claims.get("iss") if isinstance(claims, dict) else None host = issuer if isinstance(issuer, str) and issuer.startswith("https://") else os.getenv("KINDE_HOST") - return host.rstrip("/") if host else None + # The client sends the user's access token, so never use a plain HTTP host + if not host or not host.startswith("https://"): + return None + return host.rstrip("/") def _create_authenticated_api_client(self, api_class): """ @@ -98,7 +101,7 @@ def _create_authenticated_api_client(self, api_class): host = self._get_account_api_host(token_manager) if not host: - self._logger.error("Cannot determine the Kinde host for Account API calls") + self._logger.error("Cannot determine an HTTPS Kinde host for Account API calls") return None config = Configuration(host=host) diff --git a/testv2/testv2_auth/test_account_api_host.py b/testv2/testv2_auth/test_account_api_host.py index f2f0afde..276cbdfb 100644 --- a/testv2/testv2_auth/test_account_api_host.py +++ b/testv2/testv2_auth/test_account_api_host.py @@ -47,6 +47,13 @@ def test_non_https_issuer_is_ignored(monkeypatch): assert api.api_client.configuration.host == "https://acme.kinde.com" +def test_http_kinde_host_fallback_is_refused(monkeypatch): + # The client would send the access token, so it must not be created for a plain HTTP host + monkeypatch.setenv("KINDE_HOST", "http://acme.kinde.com") + for claims in ({}, {"iss": "http://acme.kinde.com"}): + assert _auth_with_claims(claims)._create_authenticated_api_client(PermissionsApi) is None + + def test_no_client_without_a_host(monkeypatch): monkeypatch.delenv("KINDE_HOST", raising=False) auth = _auth_with_claims({}) From 6bf860b658de5ffd79bcb50f95add4d566d5674b Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Fri, 9 Oct 2026 05:59:19 +0200 Subject: [PATCH 08/20] test: cover the remaining lines changed in this PR - Session reads and writes in FrameworkAwareStorage, including that values are not logged. - Permission keys given as strings, dicts and objects. - Account API feature flags given as dicts, and flags without a key. - Device ID with no storage configured, and when the session write fails. --- testv2/testv2_auth/test_feature_flags.py | 12 ++++++ testv2/testv2_auth/test_permissions.py | 15 ++++++- .../test_framework_aware_storage.py | 39 +++++++++++++++++++ testv2/testv2_core/test_storage_manager.py | 28 +++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 testv2/testv2_core/test_framework_aware_storage.py diff --git a/testv2/testv2_auth/test_feature_flags.py b/testv2/testv2_auth/test_feature_flags.py index 58eaa53b..90b2f82a 100644 --- a/testv2/testv2_auth/test_feature_flags.py +++ b/testv2/testv2_auth/test_feature_flags.py @@ -288,3 +288,15 @@ def flag(key, flag_type, value): } assert dark_mode.value is True assert missing.value == "fallback" + + +class TestFlagsFromApiResponse: + def test_dict_items_are_parsed_and_items_without_a_key_are_skipped(self): + response = Mock() + response.data = Mock(feature_flags=[ + {"key": "theme", "type": "string", "value": "pink"}, + {"type": "boolean", "value": True}, + {"key": "", "type": "integer", "value": 1}, + ]) + + assert feature_flags._flags_from_api_response(response) == {"theme": {"t": "s", "v": "pink"}} diff --git a/testv2/testv2_auth/test_permissions.py b/testv2/testv2_auth/test_permissions.py index ab4197ce..bf7730a4 100644 --- a/testv2/testv2_auth/test_permissions.py +++ b/testv2/testv2_auth/test_permissions.py @@ -228,4 +228,17 @@ async def test_all_permissions_are_returned_as_keys(self): api = self._api_returning("create:todos", "read:todos") with patch.object(permissions, "_create_authenticated_api_client", return_value=api): result = await permissions._call_account_api() - assert result == {"orgCode": "org_123", "permissions": ["create:todos", "read:todos"]} \ No newline at end of file + assert result == {"orgCode": "org_123", "permissions": ["create:todos", "read:todos"]} + +class TestPermissionKey: + """Account API permissions can be model objects, dicts or plain key strings.""" + + @pytest.mark.parametrize("permission, expected", [ + ("read:todos", "read:todos"), + ({"id": "perm_1", "key": "read:todos"}, "read:todos"), + (Mock(key="read:todos"), "read:todos"), + ({"id": "perm_1"}, None), + (None, None), + ]) + def test_permission_key(self, permission, expected): + assert permissions._permission_key(permission) == expected diff --git a/testv2/testv2_core/test_framework_aware_storage.py b/testv2/testv2_core/test_framework_aware_storage.py new file mode 100644 index 00000000..887f88cd --- /dev/null +++ b/testv2/testv2_core/test_framework_aware_storage.py @@ -0,0 +1,39 @@ +import unittest +from types import SimpleNamespace +from unittest.mock import patch + +from kinde_sdk.core.storage.framework_aware_storage import FrameworkAwareStorage + +LOGGER = "kinde_sdk.core.storage.framework_aware_storage" + + +class TestFrameworkAwareStorage(unittest.TestCase): + def setUp(self): + # A FastAPI-style request: the session is a dict on the request + self.request = SimpleNamespace(session={}) + patcher = patch(f"{LOGGER}.FrameworkContext.get_request", return_value=self.request) + patcher.start() + self.addCleanup(patcher.stop) + self.storage = FrameworkAwareStorage() + + def test_values_round_trip_through_the_session_without_being_logged(self): + with self.assertLogs(LOGGER, level="DEBUG") as logs: + self.storage.set("user_1", {"access_token": "secret-token"}) + self.assertEqual(self.storage.get("user_1"), {"access_token": "secret-token"}) + self.storage.set_flat("secret-flat-value") + + self.assertEqual(self.request.session["_flat_data"], "secret-flat-value") + output = "\n".join(logs.output) + self.assertIn("Setting key 'user_1' in session", output) + self.assertIn("Getting key 'user_1' from session", output) + self.assertNotIn("secret", output) + + def test_without_a_request_nothing_is_read_or_written(self): + with patch(f"{LOGGER}.FrameworkContext.get_request", return_value=None): + self.storage.set("user_1", {"access_token": "token"}) + self.assertIsNone(self.storage.get("user_1")) + self.assertEqual(self.request.session, {}) + + +if __name__ == "__main__": + unittest.main() diff --git a/testv2/testv2_core/test_storage_manager.py b/testv2/testv2_core/test_storage_manager.py index 27108447..daf11f17 100644 --- a/testv2/testv2_core/test_storage_manager.py +++ b/testv2/testv2_core/test_storage_manager.py @@ -192,5 +192,33 @@ def test_auto_initialize(self): self.storage_manager.get("test_key") self.assertIsNotNone(self.storage_manager._storage) + +class TestDeviceIdFallbacks(unittest.TestCase): + def setUp(self): + StorageManager._instance = None + self.storage_manager = StorageManager() + + def tearDown(self): + StorageManager._instance = None + + def test_without_storage_an_in_process_id_is_generated_once(self): + self.assertIsNone(self.storage_manager._storage) + + device_id = self.storage_manager.get_device_id() + + self.assertEqual(str(uuid.UUID(device_id)), device_id) + self.assertEqual(self.storage_manager.get_device_id(), device_id) + + def test_failed_device_id_write_still_returns_the_id(self): + storage = MagicMock(spec=StorageInterface) + storage.get.return_value = None + storage.set.side_effect = OSError("session not writable") + self.storage_manager._storage = storage + + device_id = self.storage_manager.get_device_id() + + self.assertTrue(device_id) + self.assertEqual(self.storage_manager.get_device_id(), device_id) + if __name__ == "__main__": unittest.main() \ No newline at end of file From 222b6a955e284516e490532cc1c22fd87f941311 Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sat, 10 Oct 2026 08:02:12 +0200 Subject: [PATCH 09/20] fix(storage): log a failed device ID write instead of ignoring it --- kinde_sdk/core/storage/storage_manager.py | 9 +++++++-- testv2/testv2_core/test_storage_manager.py | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/kinde_sdk/core/storage/storage_manager.py b/kinde_sdk/core/storage/storage_manager.py index 2470741b..a0553e37 100644 --- a/kinde_sdk/core/storage/storage_manager.py +++ b/kinde_sdk/core/storage/storage_manager.py @@ -1,4 +1,5 @@ # core/storage/storage_manager.py +import logging import threading import uuid import time @@ -6,6 +7,8 @@ from .storage_factory import StorageFactory from .storage_interface import StorageInterface +logger = logging.getLogger(__name__) + class StorageManager: _instance = None _lock = threading.RLock() # Reentrant: reset() calls initialize() while holding it @@ -89,8 +92,10 @@ def get_device_id(self) -> str: if storage is not None: try: storage.set("_device_id", {"value": device_id, "timestamp": time.time()}) - except Exception: - pass + except Exception as e: + # Not fatal: the in-process ID is still returned, but it won't be + # shared with other workers until a write succeeds. + logger.warning("Failed to persist device ID to storage: %s", type(e).__name__) return device_id @property diff --git a/testv2/testv2_core/test_storage_manager.py b/testv2/testv2_core/test_storage_manager.py index daf11f17..8b01f919 100644 --- a/testv2/testv2_core/test_storage_manager.py +++ b/testv2/testv2_core/test_storage_manager.py @@ -220,5 +220,21 @@ def test_failed_device_id_write_still_returns_the_id(self): self.assertTrue(device_id) self.assertEqual(self.storage_manager.get_device_id(), device_id) + def test_failed_device_id_write_is_logged_without_details(self): + storage = MagicMock(spec=StorageInterface) + storage.get.return_value = None + storage.set.side_effect = OSError("session not writable: secret-detail") + self.storage_manager._storage = storage + + with self.assertLogs("kinde_sdk.core.storage.storage_manager", level="WARNING") as logs: + device_id = self.storage_manager.get_device_id() + + self.assertTrue(device_id) + output = "\n".join(logs.output) + self.assertIn("Failed to persist device ID", output) + self.assertIn("OSError", output) + self.assertNotIn("secret-detail", output) + self.assertNotIn(device_id, output) + if __name__ == "__main__": unittest.main() \ No newline at end of file From f724922a80bf7c4020aaab8d1263c213af8a4b5b Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sat, 10 Oct 2026 08:02:41 +0200 Subject: [PATCH 10/20] fix(auth): only use the token issuer as Account API host when it matches the configured host --- kinde_sdk/auth/base_auth.py | 44 ++++++++++-- kinde_sdk/auth/oauth.py | 1 + kinde_sdk/auth/user_session.py | 3 + testv2/testv2_auth/test_account_api_host.py | 77 ++++++++++++++++++--- 4 files changed, 109 insertions(+), 16 deletions(-) diff --git a/kinde_sdk/auth/base_auth.py b/kinde_sdk/auth/base_auth.py index 834e271f..9f314e52 100644 --- a/kinde_sdk/auth/base_auth.py +++ b/kinde_sdk/auth/base_auth.py @@ -1,6 +1,7 @@ from typing import Optional, Any import logging import os +from urllib.parse import urlsplit from kinde_sdk.core.framework.framework_factory import FrameworkFactory from kinde_sdk.auth.user_session import UserSession @@ -56,19 +57,48 @@ def _describe_api_error(error: Exception) -> str: status = getattr(error, "status", None) return f"{type(error).__name__} (HTTP {status})" if status else type(error).__name__ + @staticmethod + def _https_origin(url: Any) -> Optional[str]: + """Lower-cased https://host[:port] of a bare https URL, or None.""" + if not isinstance(url, str): + return None + try: + parts = urlsplit(url.strip()) + parts.port # raises ValueError for an invalid port + except ValueError: + return None + if (parts.scheme.lower() != "https" or not parts.hostname or parts.username + or parts.password or parts.path not in ("", "/") or parts.query or parts.fragment): + return None + return f"https://{parts.netloc.lower()}" + @staticmethod def _get_account_api_host(token_manager) -> Optional[str]: """ - The Account API lives on the Kinde domain that issued the user's token, - which also covers custom domains; KINDE_HOST is the fallback. + The Account API lives on the configured Kinde host (the OAuth client's + host, then KINDE_HOST). The token's iss claim is unverified, so it can + only select one of those hosts; the access token is never sent to a host + that comes from the token alone. """ + client_id = getattr(token_manager, "client_id", None) + client_host = UserSession.client_hosts.get(client_id) if isinstance(client_id, str) else None + # The client sends the user's access token, so never use a plain HTTP host + hosts = [ + host.rstrip("/") for host in (client_host, os.getenv("KINDE_HOST")) + if BaseAuth._https_origin(host) + ] + claims = token_manager.get_claims() if hasattr(token_manager, "get_claims") else {} issuer = claims.get("iss") if isinstance(claims, dict) else None - host = issuer if isinstance(issuer, str) and issuer.startswith("https://") else os.getenv("KINDE_HOST") - # The client sends the user's access token, so never use a plain HTTP host - if not host or not host.startswith("https://"): - return None - return host.rstrip("/") + if issuer is not None: + issuer_origin = BaseAuth._https_origin(issuer) + for host in hosts: + if BaseAuth._https_origin(host) == issuer_origin: + return host + logging.getLogger("kinde_sdk").warning( + "Token issuer does not match the configured Kinde host; using the configured host" + ) + return hosts[0] if hosts else None def _create_authenticated_api_client(self, api_class): """ diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index 64f6fb48..192d0a17 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -82,6 +82,7 @@ def __init__( # Don't let a secretless (PKCE) instance for the same client_id wipe a configured secret if self.client_secret: UserSession.client_secrets[self.client_id] = self.client_secret + UserSession.client_hosts[self.client_id] = self.host # Initialize API endpoints self._set_api_endpoints() diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index 9add22c8..99b66c8b 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -8,6 +8,9 @@ class UserSession: # Keyed by client_id and populated by OAuth. Kept in process memory so the # secret is never written to session storage (which may be a browser cookie). client_secrets: Dict[str, Optional[str]] = {} + # Configured Kinde host per client_id, also set by OAuth. The Account API + # host is checked against it rather than trusted from a token's iss claim. + client_hosts: Dict[str, str] = {} def __init__(self): self.user_sessions = {} # Store user-specific session data diff --git a/testv2/testv2_auth/test_account_api_host.py b/testv2/testv2_auth/test_account_api_host.py index 276cbdfb..b1ab6bbc 100644 --- a/testv2/testv2_auth/test_account_api_host.py +++ b/testv2/testv2_auth/test_account_api_host.py @@ -1,34 +1,93 @@ """ -Account API calls (force_api) must target the Kinde domain, not the -generated client's placeholder host. +Account API calls (force_api) must target the configured Kinde domain, not the +generated client's placeholder host or an unverified token's issuer. """ -from unittest.mock import MagicMock +import logging +from unittest.mock import MagicMock, patch + +import pytest from kinde_sdk.auth.base_auth import BaseAuth +from kinde_sdk.auth.oauth import OAuth +from kinde_sdk.auth.user_session import UserSession from kinde_sdk.frontend.api.permissions_api import PermissionsApi PLACEHOLDER = "your_kinde_subdomain" -def _auth_with_claims(claims): +@pytest.fixture(autouse=True) +def _no_registered_client_hosts(monkeypatch): + monkeypatch.setattr(UserSession, "client_hosts", {}) + + +def _auth_with_claims(claims, client_id=None): auth = BaseAuth() token_manager = MagicMock() + token_manager.client_id = client_id token_manager.get_access_token.return_value = "access-token" token_manager.get_claims.return_value = claims auth._get_token_manager = lambda: token_manager return auth -def test_host_comes_from_token_issuer(monkeypatch): - monkeypatch.setenv("KINDE_HOST", "https://fallback.kinde.com") - auth = _auth_with_claims({"iss": "https://auth.example.com/"}) +def test_matching_issuer_is_used(monkeypatch): + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") + auth = _auth_with_claims({"iss": "https://ACME.kinde.com/"}) api = auth._create_authenticated_api_client(PermissionsApi) - assert api.api_client.configuration.host == "https://auth.example.com" + assert api.api_client.configuration.host == "https://acme.kinde.com" assert api.api_client.configuration.access_token == "access-token" +def test_issuer_selects_the_oauth_client_host(monkeypatch): + # e.g. a custom domain passed as OAuth(host=...) while KINDE_HOST is the kinde.com domain + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") + UserSession.client_hosts["client_1"] = "https://auth.example.com/" + auth = _auth_with_claims({"iss": "https://auth.example.com"}, client_id="client_1") + + api = auth._create_authenticated_api_client(PermissionsApi) + + assert api.api_client.configuration.host == "https://auth.example.com" + + +@pytest.mark.parametrize("issuer", [ + "https://attacker.example", + "https://acme.kinde.com.attacker.example", + "https://acme.kinde.com@attacker.example", + "https://acme.kinde.com:8443", + "https://acme.kinde.com/evil", +]) +def test_mismatched_issuer_is_ignored(monkeypatch, caplog, issuer): + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") + auth = _auth_with_claims({"iss": issuer}) + + with caplog.at_level(logging.WARNING, logger="kinde_sdk"): + api = auth._create_authenticated_api_client(PermissionsApi) + + assert api.api_client.configuration.host == "https://acme.kinde.com" + assert "does not match the configured Kinde host" in caplog.text + assert "access-token" not in caplog.text + + +def test_mismatched_issuer_without_a_configured_host_is_refused(monkeypatch): + monkeypatch.delenv("KINDE_HOST", raising=False) + auth = _auth_with_claims({"iss": "https://attacker.example"}) + + assert auth._create_authenticated_api_client(PermissionsApi) is None + + +def test_oauth_registers_its_configured_host(monkeypatch): + monkeypatch.delenv("KINDE_HOST", raising=False) + with patch("requests.get", side_effect=Exception("offline")): + OAuth(client_id="client_1", redirect_uri="http://localhost/callback", host="https://acme.kinde.com") + auth = _auth_with_claims({"iss": "https://acme.kinde.com"}, client_id="client_1") + + api = auth._create_authenticated_api_client(PermissionsApi) + + assert api.api_client.configuration.host == "https://acme.kinde.com" + + def test_host_falls_back_to_kinde_host(monkeypatch): monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com") auth = _auth_with_claims({}) @@ -50,7 +109,7 @@ def test_non_https_issuer_is_ignored(monkeypatch): def test_http_kinde_host_fallback_is_refused(monkeypatch): # The client would send the access token, so it must not be created for a plain HTTP host monkeypatch.setenv("KINDE_HOST", "http://acme.kinde.com") - for claims in ({}, {"iss": "http://acme.kinde.com"}): + for claims in ({}, {"iss": "http://acme.kinde.com"}, {"iss": "https://acme.kinde.com"}): assert _auth_with_claims(claims)._create_authenticated_api_client(PermissionsApi) is None From 05643112ffd385ea9245df52a46a0474f154a2d9 Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sat, 10 Oct 2026 08:04:57 +0200 Subject: [PATCH 11/20] fix(fastapi): return 500, not 400, for unexpected callback errors --- kinde_fastapi/framework/fastapi_framework.py | 9 +++++++-- testv2/testv2_auth/test_oauth_callback_security.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/kinde_fastapi/framework/fastapi_framework.py b/kinde_fastapi/framework/fastapi_framework.py index b6cc7920..39dfab07 100644 --- a/kinde_fastapi/framework/fastapi_framework.py +++ b/kinde_fastapi/framework/fastapi_framework.py @@ -3,7 +3,7 @@ from fastapi.responses import RedirectResponse, HTMLResponse from kinde_sdk.core.framework.framework_interface import FrameworkInterface from kinde_sdk.auth.oauth import OAuth -from kinde_sdk.core.exceptions import KindeLoginException +from kinde_sdk.core.exceptions import KindeLoginException, KindeTokenException from ..middleware.framework_middleware import FrameworkMiddleware import os import uuid @@ -200,9 +200,14 @@ async def callback(request: Request, code: Optional[str] = None, state: Optional # Missing, forged or stale callbacks are client errors, as in the Flask route self._logger.warning("OAuth callback rejected") return HTMLResponse("Authentication failed: invalid or expired login request", status_code=400) + except KindeTokenException: + # Kinde rejected the authorization code; the message is already logged by type and status + self._logger.warning("OAuth callback token exchange failed") + return HTMLResponse("Authentication failed", status_code=400) except Exception as e: + # Anything else is a server-side fault, not a bad request self._logger.error("OAuth callback failed: %s", type(e).__name__) - return HTMLResponse("Authentication failed", status_code=400) + return HTMLResponse("Authentication failed", status_code=500) request.session['user_id'] = user_id diff --git a/testv2/testv2_auth/test_oauth_callback_security.py b/testv2/testv2_auth/test_oauth_callback_security.py index 7f339f69..e0799eef 100644 --- a/testv2/testv2_auth/test_oauth_callback_security.py +++ b/testv2/testv2_auth/test_oauth_callback_security.py @@ -368,3 +368,15 @@ def test_callback_happy_path(self, fastapi_client, network): r = fastapi_client.get(f"/callback?code=c&state={q['state']}") assert r.status_code in (302, 307) assert network.call_count == 1 + + def test_unexpected_callback_error_returns_500_without_details(self, fastapi_client, network): + q = _query(fastapi_client.get("/login").headers["location"]) + failure = AsyncMock(side_effect=RuntimeError("internal-detail-do-not-show")) + + with patch.object(OAuth, "handle_redirect", failure): + r = fastapi_client.get(f"/callback?code=c&state={q['state']}") + + assert failure.await_count == 1 + assert r.status_code == 500 + assert "internal-detail-do-not-show" not in r.text + assert "RuntimeError" not in r.text From e1c1b73cb776a385c4490e3ab7f8f4b1265d8a6b Mon Sep 17 00:00:00 2001 From: Koman Rudden <1030623+KomanRudden@users.noreply.github.com> Date: Sat, 10 Oct 2026 08:14:31 +0200 Subject: [PATCH 12/20] fix(flask): return 500, not 400, for unexpected callback errors --- kinde_flask/framework/flask_framework.py | 9 +++++++-- .../testv2_auth/test_oauth_callback_security.py | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/kinde_flask/framework/flask_framework.py b/kinde_flask/framework/flask_framework.py index 43d2a36e..15f90d9c 100644 --- a/kinde_flask/framework/flask_framework.py +++ b/kinde_flask/framework/flask_framework.py @@ -3,7 +3,7 @@ from flask_session import Session from kinde_sdk.core.framework.framework_interface import FrameworkInterface from kinde_sdk.auth.oauth import OAuth -from kinde_sdk.core.exceptions import KindeLoginException +from kinde_sdk.core.exceptions import KindeLoginException, KindeTokenException from ..middleware.framework_middleware import FrameworkMiddleware import os import uuid @@ -255,9 +255,14 @@ def callback(): except KindeLoginException: logger.warning("OAuth callback rejected") return "Authentication failed: invalid or expired login request", 400 + except KindeTokenException: + # Kinde rejected the authorization code; the message is already logged by type and status + logger.warning("OAuth callback token exchange failed") + return "Authentication failed", 400 except Exception as e: + # Anything else is a server-side fault, not a bad request logger.error("OAuth callback failed: %s", type(e).__name__) - return "Authentication failed", 400 + return "Authentication failed", 500 # Only bind the session to a user once the callback has succeeded session['user_id'] = user_id diff --git a/testv2/testv2_auth/test_oauth_callback_security.py b/testv2/testv2_auth/test_oauth_callback_security.py index e0799eef..dd72bcc6 100644 --- a/testv2/testv2_auth/test_oauth_callback_security.py +++ b/testv2/testv2_auth/test_oauth_callback_security.py @@ -302,6 +302,21 @@ def test_callback_happy_path(self, flask_client, network): assert r.status_code == 302 assert network.call_count == 1 + def test_unexpected_callback_error_returns_500_without_details(self, flask_client, network): + q = _query(flask_client.get("/login").headers["Location"]) + failure = AsyncMock(side_effect=RuntimeError("internal-detail-do-not-show")) + + with patch.object(OAuth, "handle_redirect", failure): + r = flask_client.get(f"/callback?code=c&state={q['state']}") + + assert failure.await_count == 1 + assert r.status_code == 500 + body = r.get_data(as_text=True) + assert "internal-detail-do-not-show" not in body + assert "RuntimeError" not in body + with flask_client.session_transaction() as session: + assert "user_id" not in session + # --------------------------------------------------------------------------- # FastAPI routes From d7d64c73e1ebf8433179c77e8261c779fd911016 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Mon, 19 Oct 2026 06:47:51 +0200 Subject: [PATCH 13/20] fix(auth): check the ID token issuer, audience and expiry; clean old sessions on load - The ID token's signature still isn't verified, but its issuer must now match the configured Kinde host, its audience must include the client ID, and it must not be expired, as OpenID Connect requires. - When an old session still contains client_secret, write the cleaned session back as soon as it's loaded, instead of waiting for the next save. --- CHANGELOG.md | 4 ++- kinde_sdk/auth/oauth.py | 21 +++++++++++-- kinde_sdk/auth/user_session.py | 7 +++-- testv2/testv2_auth/test_log_redaction.py | 3 +- .../test_oauth_callback_security.py | 30 +++++++++++++++++-- testv2/testv2_auth/test_session_secrets.py | 9 +++++- 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b129048..d8254bdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 standard OAuth `error` code. The response body is no longer included in exceptions, logs, or the Flask / FastAPI callback responses, which now return a generic HTTP 400. - **OAuth callback validation**: `state` is always required and must match the login started - in the same session; the ID token `nonce` is verified. Both are single-use. + in the same session; the ID token `nonce` is verified. Both are single-use. The ID token's + issuer must match the configured Kinde host, its audience must include the client ID, and it + must not be expired. - **FastAPI**: the callback no longer forwards the consumed `state` to the post-login URL, and logout now clears the user's server-side tokens (previously only the session cookie was cleared). diff --git a/kinde_sdk/auth/oauth.py b/kinde_sdk/auth/oauth.py index 192d0a17..c65a7afe 100644 --- a/kinde_sdk/auth/oauth.py +++ b/kinde_sdk/auth/oauth.py @@ -587,11 +587,26 @@ async def handle_redirect(self, code: str, user_id: str, state: Optional[str] = self._logger.error("Token response has no ID token") raise KindeLoginException("Missing ID token") try: - # Signature not verified, as elsewhere in the SDK: the token comes - # directly from the token endpoint over TLS - id_token_claims = jwt.decode(id_token, options={"verify_signature": False}) + # The signature isn't verified (the token comes directly from the token + # endpoint over TLS), but OpenID Connect still requires the audience and + # expiry to be checked, and the issuer below + id_token_claims = jwt.decode( + id_token, + options={ + "verify_signature": False, + "verify_aud": True, + "verify_exp": True, + "require": ["iss", "aud", "exp"], + }, + audience=self.client_id, + leeway=60, + ) except jwt.PyJWTError as e: + self._logger.error("Invalid ID token: %s", type(e).__name__) raise KindeLoginException("Invalid ID token") from e + if str(id_token_claims["iss"]).rstrip("/") != str(self.host).rstrip("/"): + self._logger.error("ID token issuer does not match the configured Kinde host") + raise KindeLoginException("Invalid ID token issuer") if id_token_claims.get("nonce") != expected_nonce: self._logger.error("Nonce mismatch in ID token") raise KindeLoginException("Invalid nonce in ID token") diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index 99b66c8b..ac0f9c2a 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -97,8 +97,11 @@ def _load_from_storage(self, user_id: str) -> bool: "access_token" not in tokens): return False - # Sessions saved by older SDK versions may still contain client_secret; drop it - user_info = {k: v for k, v in user_info.items() if k != "client_secret"} + # Sessions saved by older SDK versions may still contain client_secret. Drop it, + # and write the cleaned record back so it doesn't stay in the stored session + if "client_secret" in user_info: + user_info = {k: v for k, v in user_info.items() if k != "client_secret"} + self.storage_manager.setItems(user_id, {**session_data, "user_info": user_info}) token_manager = TokenManager( user_id, diff --git a/testv2/testv2_auth/test_log_redaction.py b/testv2/testv2_auth/test_log_redaction.py index 2f19c823..ff6b9206 100644 --- a/testv2/testv2_auth/test_log_redaction.py +++ b/testv2/testv2_auth/test_log_redaction.py @@ -46,7 +46,8 @@ class Tokens: def __init__(self, nonce, tag="1"): self.access = _jwt(sub="kp_user", tag=f"access-{tag}") self.refresh = f"SECRET-refresh-token-{tag}" - self.id = _jwt(sub="kp_user", nonce=nonce, tag=f"id-{tag}") + self.id = _jwt(sub="kp_user", nonce=nonce, tag=f"id-{tag}", + iss=HOST, aud=[CLIENT_ID], exp=int(time.time()) + 3600) def response(self, expires_in=3600): resp = MagicMock() diff --git a/testv2/testv2_auth/test_oauth_callback_security.py b/testv2/testv2_auth/test_oauth_callback_security.py index dd72bcc6..95e59f28 100644 --- a/testv2/testv2_auth/test_oauth_callback_security.py +++ b/testv2/testv2_auth/test_oauth_callback_security.py @@ -16,6 +16,7 @@ """ import asyncio import tempfile +import time from unittest.mock import patch, MagicMock, AsyncMock from urllib.parse import urlparse, parse_qs @@ -32,10 +33,13 @@ def _id_token(**claims): - payload = {"sub": "kp_user", "aud": CLIENT_ID, "iss": "https://example.kinde.com"} + payload = {"sub": "kp_user", "aud": CLIENT_ID, "iss": "https://example.kinde.com", + "exp": int(time.time()) + 3600} payload.update(claims) - # Signature is irrelevant here: the SDK decodes the ID token without - # verifying the signature (token comes straight from the token endpoint). + # A claim passed as None is left out of the token + payload = {k: v for k, v in payload.items() if v is not None} + # Signature is irrelevant here: the SDK doesn't verify the ID token's + # signature (the token comes straight from the token endpoint). return jwt.encode(payload, "test-secret-key-of-sufficient-length-123", algorithm="HS256") @@ -236,6 +240,26 @@ def test_missing_stored_nonce_rejected_before_token_exchange(self, standalone_oa network.assert_not_called() assert standalone_oauth._session_manager.get_token_manager("u1") is None + @pytest.mark.parametrize("claims", [ + {"iss": "https://attacker.example"}, + {"aud": "another_client"}, + {"exp": int(time.time()) - 3600}, + {"exp": None}, + ], ids=["wrong-issuer", "wrong-audience", "expired", "no-expiry"]) + def test_id_token_with_bad_issuer_audience_or_expiry_rejected(self, standalone_oauth, network, claims): + """The signature isn't checked, but the issuer, audience and expiry are.""" + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(_id_token(nonce=q["nonce"], **claims)) + with pytest.raises(KindeLoginException): + asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + assert standalone_oauth._session_manager.get_token_manager("u1") is None + + def test_id_token_issuer_with_trailing_slash_accepted(self, standalone_oauth, network): + q = _query(asyncio.run(standalone_oauth.login())) + network.return_value = _token_response(_id_token(nonce=q["nonce"], iss="https://example.kinde.com/")) + result = asyncio.run(standalone_oauth.handle_redirect("code", "u1", q["state"])) + assert result["user"] == {"id": "kp_user"} + # --------------------------------------------------------------------------- # Flask routes diff --git a/testv2/testv2_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py index 528d80a6..8cc9ff2c 100644 --- a/testv2/testv2_auth/test_session_secrets.py +++ b/testv2/testv2_auth/test_session_secrets.py @@ -1,4 +1,5 @@ import asyncio +import time import unittest from unittest.mock import AsyncMock, patch from urllib.parse import parse_qs, urlparse @@ -47,7 +48,10 @@ def _complete_login(self, user_id="user_1"): state, nonce = query["state"][0], query["nonce"][0] token_data = { **self._token_data(), - "id_token": jwt.encode({"sub": "user_1", "nonce": nonce}, "key", algorithm="HS256"), + "id_token": jwt.encode({ + "sub": "user_1", "nonce": nonce, "iss": "https://example.kinde.com", + "aud": ["secret_test_client"], "exp": int(time.time()) + 3600, + }, "key", algorithm="HS256"), } with patch.object(OAuth, "exchange_code_for_tokens", AsyncMock(return_value=token_data)), \ patch("kinde_sdk.auth.oauth.helper_get_user_details", AsyncMock(return_value={})): @@ -85,6 +89,9 @@ def test_legacy_session_secret_is_dropped_on_load_and_not_saved_again(self): user_info = session.get_user_data("user_1") self.assertNotIn("client_secret", user_info) self.assertEqual(session.get_token_manager("user_1").client_secret, "super_secret") + # Loading alone cleans the stored session, without waiting for a later save + self.assertNotIn("super_secret", repr(StorageManager().get("user_1"))) + self.assertIn("access_token", StorageManager().get("user_1")["tokens"]) # Saving the session again must not write the secret back session.set_user_data("user_1", {**user_info, "client_secret": "super_secret"}, self._token_data()) From 100274d4eab72430e897dccb465fc2e55a265395 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Fri, 30 Oct 2026 06:35:37 +0200 Subject: [PATCH 14/20] fix: log out before clearing the session; redact legacy helper errors - Flask and FastAPI /logout now call OAuth.logout() before clearing the session. After a restart or on another worker, the SDK reads the user's tokens from the session to revoke them and to add the id_token_hint; clearing it first skipped both. - The legacy Account API helpers in kinde_sdk.core.helpers log only the HTTP status and the standard OAuth error code, not error_description. - Use the normalised origin for the configured Account API host, so whitespace or a trailing slash in KINDE_HOST can't reach the API URL. - CHANGELOG: describe the current Account API host behaviour and the two fixes above. --- CHANGELOG.md | 11 +++++- kinde_fastapi/framework/fastapi_framework.py | 6 ++- kinde_flask/framework/flask_framework.py | 5 ++- kinde_sdk/auth/base_auth.py | 11 ++++-- kinde_sdk/core/helpers.py | 12 +++--- testv2/testv2_auth/test_account_api_host.py | 8 ++++ testv2/testv2_auth/test_log_redaction.py | 39 ++++++++++++++++++++ testv2/testv2_core/test_helpers.py | 18 ++++++--- 8 files changed, 90 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8254bdd..75be8d67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 logout now clears the user's server-side tokens (previously only the session cookie was cleared). - **Flask**: the session is bound to a user only after a successful callback. +- **Logout after a restart or on another worker**: the Flask and FastAPI `/logout` routes cleared + the session before logging out, so the SDK couldn't find the user's tokens there: the access + token wasn't revoked and the logout URL had no `id_token_hint`. They now log out first. +- **Legacy Account API helpers**: the helper functions in `kinde_sdk.core.helpers` now log only + the HTTP status and the standard OAuth `error` code when a request fails, not the response's + `error_description`. - **Account API errors**: the permissions, roles and feature flag helpers log only the exception type and HTTP status when an Account API call fails, not the response body. @@ -38,8 +44,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `set_tokens`). - **Account API host**: permission, role and feature flag lookups with `force_api=True` were sent to the generated client's placeholder host (`your_kinde_subdomain.kinde.com`) and silently - returned empty results. They now use the issuer of the user's access token, falling back to - `KINDE_HOST`. + returned empty results. They now use the configured Kinde host (`OAuth(host=...)` or + `KINDE_HOST`). The access token's `iss` is used only when it exactly matches a configured + host; otherwise the configured host is used and a warning is logged. - **Account API permissions**: with `force_api=True`, `get_permission()` never granted anything because the API's `{id, name, key}` objects were compared with the key string; `get_permissions()` now returns keys, as in token mode. diff --git a/kinde_fastapi/framework/fastapi_framework.py b/kinde_fastapi/framework/fastapi_framework.py index 39dfab07..d40d8c70 100644 --- a/kinde_fastapi/framework/fastapi_framework.py +++ b/kinde_fastapi/framework/fastapi_framework.py @@ -231,8 +231,12 @@ async def logout(request: Request): """Logout the user and redirect to Kinde logout page.""" # Pass the user_id so the SDK also drops the server-side tokens user_id = request.session.get('user_id') + # Log out before clearing the session: after a restart, or on another worker, + # the SDK reads the user's tokens from the session to revoke them and to build + # the logout URL + logout_url = await self._oauth.logout(user_id) request.session.clear() - return RedirectResponse(url=await self._oauth.logout(user_id)) + return RedirectResponse(url=logout_url) # Register route @self.app.get("/register") diff --git a/kinde_flask/framework/flask_framework.py b/kinde_flask/framework/flask_framework.py index 15f90d9c..cb895bd6 100644 --- a/kinde_flask/framework/flask_framework.py +++ b/kinde_flask/framework/flask_framework.py @@ -279,8 +279,11 @@ def callback(): def logout(): """Logout the user and redirect to Kinde logout page.""" user_id = session.get('user_id') - session.clear() + # Log out before clearing the session: after a restart, or on another worker, + # the SDK reads the user's tokens from the session to revoke them and to build + # the logout URL logout_url = self._run_async(self._oauth.logout(user_id)) + session.clear() return redirect(logout_url) # Register route diff --git a/kinde_sdk/auth/base_auth.py b/kinde_sdk/auth/base_auth.py index 9f314e52..aab6a037 100644 --- a/kinde_sdk/auth/base_auth.py +++ b/kinde_sdk/auth/base_auth.py @@ -82,10 +82,13 @@ def _get_account_api_host(token_manager) -> Optional[str]: """ client_id = getattr(token_manager, "client_id", None) client_host = UserSession.client_hosts.get(client_id) if isinstance(client_id, str) else None - # The client sends the user's access token, so never use a plain HTTP host + # The client sends the user's access token, so never use a plain HTTP host. + # Use the normalised origin, so stray whitespace or a trailing slash in the + # configuration can't end up in the API URL. hosts = [ - host.rstrip("/") for host in (client_host, os.getenv("KINDE_HOST")) - if BaseAuth._https_origin(host) + origin for origin in ( + BaseAuth._https_origin(host) for host in (client_host, os.getenv("KINDE_HOST")) + ) if origin ] claims = token_manager.get_claims() if hasattr(token_manager, "get_claims") else {} @@ -93,7 +96,7 @@ def _get_account_api_host(token_manager) -> Optional[str]: if issuer is not None: issuer_origin = BaseAuth._https_origin(issuer) for host in hosts: - if BaseAuth._https_origin(host) == issuer_origin: + if host == issuer_origin: return host logging.getLogger("kinde_sdk").warning( "Token issuer does not match the configured Kinde host; using the configured host" diff --git a/kinde_sdk/core/helpers.py b/kinde_sdk/core/helpers.py index 4b1e1ca5..ce41578a 100644 --- a/kinde_sdk/core/helpers.py +++ b/kinde_sdk/core/helpers.py @@ -115,7 +115,7 @@ async def get_user_details(userinfo_url: str, token_manager, logger) -> Dict[str if hasattr(e, 'response') and e.response is not None: try: error_data = e.response.json() - error_message = f"User details retrieval failed: {error_data.get('error_description', error_data.get('error', 'Unknown error'))}" + error_message = f"User details retrieval failed with status code: {e.response.status_code} ({error_data.get('error') or 'no error code'})" except Exception: error_message = f"User details retrieval failed with status code: {e.response.status_code}" @@ -218,7 +218,7 @@ def get_user_organizations(api_url: str, token_manager, logger) -> List[Dict[str if hasattr(e, 'response') and e.response is not None: try: error_data = e.response.json() - error_message = f"Organizations retrieval failed: {error_data.get('error_description', error_data.get('error', 'Unknown error'))}" + error_message = f"Organizations retrieval failed with status code: {e.response.status_code} ({error_data.get('error') or 'no error code'})" except Exception: error_message = f"Organizations retrieval failed with status code: {e.response.status_code}" @@ -271,7 +271,7 @@ def get_organization_details(api_url: str, org_code: str, token_manager, logger) if hasattr(e, 'response') and e.response is not None: try: error_data = e.response.json() - error_message = f"Organization details retrieval failed: {error_data.get('error_description', error_data.get('error', 'Unknown error'))}" + error_message = f"Organization details retrieval failed with status code: {e.response.status_code} ({error_data.get('error') or 'no error code'})" except Exception: error_message = f"Organization details retrieval failed with status code: {e.response.status_code}" @@ -325,7 +325,7 @@ def get_organization_users(api_url: str, org_code: str, token_manager, logger) - if hasattr(e, 'response') and e.response is not None: try: error_data = e.response.json() - error_message = f"Organization users retrieval failed: {error_data.get('error_description', error_data.get('error', 'Unknown error'))}" + error_message = f"Organization users retrieval failed with status code: {e.response.status_code} ({error_data.get('error') or 'no error code'})" except Exception: error_message = f"Organization users retrieval failed with status code: {e.response.status_code}" @@ -389,7 +389,7 @@ def get_user_permissions(api_url: str, token_manager, org_code: Optional[str] = if hasattr(e, 'response') and e.response is not None: try: error_data = e.response.json() - error_message = f"User permissions retrieval failed: {error_data.get('error_description', error_data.get('error', 'Unknown error'))}" + error_message = f"User permissions retrieval failed with status code: {e.response.status_code} ({error_data.get('error') or 'no error code'})" except Exception: error_message = f"User permissions retrieval failed with status code: {e.response.status_code}" @@ -476,7 +476,7 @@ def get_user_roles(api_url: str, token_manager, org_code: Optional[str] = None, if hasattr(e, 'response') and e.response is not None: try: error_data = e.response.json() - error_message = f"User roles retrieval failed: {error_data.get('error_description', error_data.get('error', 'Unknown error'))}" + error_message = f"User roles retrieval failed with status code: {e.response.status_code} ({error_data.get('error') or 'no error code'})" except Exception: error_message = f"User roles retrieval failed with status code: {e.response.status_code}" diff --git a/testv2/testv2_auth/test_account_api_host.py b/testv2/testv2_auth/test_account_api_host.py index b1ab6bbc..12c1358a 100644 --- a/testv2/testv2_auth/test_account_api_host.py +++ b/testv2/testv2_auth/test_account_api_host.py @@ -125,3 +125,11 @@ def test_never_uses_placeholder_host(monkeypatch): for claims in ({}, {"iss": "https://acme.kinde.com"}): api = _auth_with_claims(claims)._create_authenticated_api_client(PermissionsApi) assert PLACEHOLDER not in api.api_client.configuration.host + + +def test_configured_host_is_normalised(monkeypatch): + # Stray whitespace, a trailing slash or upper case must not reach the API URL + monkeypatch.setenv("KINDE_HOST", " https://Acme.Kinde.com/ ") + for claims in ({}, {"iss": "https://acme.kinde.com"}): + api = _auth_with_claims(claims)._create_authenticated_api_client(PermissionsApi) + assert api.api_client.configuration.host == "https://acme.kinde.com" diff --git a/testv2/testv2_auth/test_log_redaction.py b/testv2/testv2_auth/test_log_redaction.py index ff6b9206..6e1ea215 100644 --- a/testv2/testv2_auth/test_log_redaction.py +++ b/testv2/testv2_auth/test_log_redaction.py @@ -295,6 +295,30 @@ def test_failed_callback_does_not_bind_session_user(self, flask_app, network): with client.session_transaction() as session: assert "user_id" not in session + def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, flask_app, network): + exchange, token_manager_post = network + app, oauth = flask_app + client = app.test_client() + q = _query(client.get("/login").headers["Location"]) + tokens = Tokens(q["nonce"]) + exchange.return_value = tokens.response() + token_manager_post.return_value = MagicMock(status_code=200) + client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + _forget_in_memory_sessions(oauth) + + logout = client.get("/logout") + + assert _query(logout.headers["Location"]).get("id_token_hint") == tokens.id + assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + with client.session_transaction() as session: + assert "user_id" not in session + + +def _forget_in_memory_sessions(oauth): + """Simulate a restarted process or another worker: only the session has the tokens.""" + oauth._session_manager.user_sessions.clear() + TokenManager.reset_instances() + # --------------------------------------------------------------------------- # FastAPI routes @@ -354,6 +378,21 @@ def test_logout_clears_server_side_tokens(self, fastapi_app, network): # Best-effort revocation of the access token was attempted assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, fastapi_app, network): + exchange, token_manager_post = network + client, oauth = fastapi_app + q = _query(client.get("/login").headers["location"]) + tokens = Tokens(q["nonce"]) + exchange.return_value = tokens.response() + token_manager_post.return_value = MagicMock(status_code=200) + client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + _forget_in_memory_sessions(oauth) + + logout = client.get("/logout") + + assert _query(logout.headers["location"]).get("id_token_hint") == tokens.id + assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + def test_failed_exchange_returns_400_without_body_echo(self, fastapi_app, debug_logs, network): exchange, _ = network client, _ = fastapi_app diff --git a/testv2/testv2_core/test_helpers.py b/testv2/testv2_core/test_helpers.py index 060a58eb..a424f384 100644 --- a/testv2/testv2_core/test_helpers.py +++ b/testv2/testv2_core/test_helpers.py @@ -169,13 +169,14 @@ def test_get_user_details_request_error_with_json(self): # Create a mock response with JSON error data with patch('kinde_sdk.core.helpers.requests.get') as mock_get: mock_response = MagicMock() + mock_response.status_code = 401 mock_response.json.return_value = {"error": "invalid_token", "error_description": "Token is invalid"} mock_get.side_effect = RequestException(response=mock_response) with self.assertRaises(RequestException): asyncio.run(get_user_details(userinfo_url, token_manager, logger)) - logger.error.assert_called_with("User details retrieval failed: Token is invalid") + logger.error.assert_called_with("User details retrieval failed with status code: 401 (invalid_token)") def test_get_user_details_request_error_without_json(self): """Test get_user_details handles request errors without JSON response.""" @@ -308,13 +309,14 @@ def test_get_user_organizations_request_error_with_json(self): # Create a mock response with JSON error data with patch('kinde_sdk.core.helpers.requests.get') as mock_get: mock_response = MagicMock() + mock_response.status_code = 403 mock_response.json.return_value = {"error": "forbidden", "error_description": "Access denied"} mock_get.side_effect = RequestException(response=mock_response) with self.assertRaises(RequestException): get_user_organizations(api_url, token_manager, logger) - logger.error.assert_called_with("Organizations retrieval failed: Access denied") + logger.error.assert_called_with("Organizations retrieval failed with status code: 403 (forbidden)") def test_get_user_organizations_request_error_without_json(self): """Test get_user_organizations handles request errors without JSON response.""" @@ -397,13 +399,14 @@ def test_get_organization_details_request_error_with_json(self): # Create a mock response with JSON error data with patch('kinde_sdk.core.helpers.requests.get') as mock_get: mock_response = MagicMock() + mock_response.status_code = 404 mock_response.json.return_value = {"error": "not_found", "error_description": "Organization not found"} mock_get.side_effect = RequestException(response=mock_response) with self.assertRaises(RequestException): get_organization_details(api_url, org_code, token_manager, logger) - logger.error.assert_called_with("Organization details retrieval failed: Organization not found") + logger.error.assert_called_with("Organization details retrieval failed with status code: 404 (not_found)") def test_get_organization_details_request_error_without_json(self): """Test get_organization_details handles request errors without JSON response.""" @@ -488,13 +491,14 @@ def test_get_organization_users_request_error_with_json(self): # Create a mock response with JSON error data with patch('kinde_sdk.core.helpers.requests.get') as mock_get: mock_response = MagicMock() + mock_response.status_code = 403 mock_response.json.return_value = {"error": "forbidden", "error_description": "Access denied"} mock_get.side_effect = RequestException(response=mock_response) with self.assertRaises(RequestException): get_organization_users(api_url, org_code, token_manager, logger) - logger.error.assert_called_with("Organization users retrieval failed: Access denied") + logger.error.assert_called_with("Organization users retrieval failed with status code: 403 (forbidden)") def test_get_organization_users_request_error_without_json(self): """Test get_organization_users handles request errors without JSON response.""" @@ -593,13 +597,14 @@ def test_get_user_permissions_request_error_with_json(self): # Create a mock response with JSON error data with patch('kinde_sdk.core.helpers.requests.get') as mock_get: mock_response = MagicMock() + mock_response.status_code = 403 mock_response.json.return_value = {"error": "forbidden", "error_description": "Access denied"} mock_get.side_effect = RequestException(response=mock_response) with self.assertRaises(RequestException): get_user_permissions(api_url, token_manager, logger=logger) - logger.error.assert_called_with("User permissions retrieval failed: Access denied") + logger.error.assert_called_with("User permissions retrieval failed with status code: 403 (forbidden)") def test_get_user_permissions_request_error_without_json(self): """Test get_user_permissions handles request errors without JSON response.""" @@ -763,13 +768,14 @@ def test_get_user_roles_request_error_with_json(self): # Create a mock response with JSON error data with patch('kinde_sdk.core.helpers.requests.get') as mock_get: mock_response = MagicMock() + mock_response.status_code = 403 mock_response.json.return_value = {"error": "forbidden", "error_description": "Access denied"} mock_get.side_effect = RequestException(response=mock_response) with self.assertRaises(RequestException): get_user_roles(api_url, token_manager, logger=logger) - logger.error.assert_called_with("User roles retrieval failed: Access denied") + logger.error.assert_called_with("User roles retrieval failed with status code: 403 (forbidden)") def test_get_user_roles_request_error_without_json(self): """Test get_user_roles handles request errors without JSON response.""" From ee69be8af624af0719a8ffc8f889e9f3ac25c905 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Fri, 30 Oct 2026 07:03:45 +0200 Subject: [PATCH 15/20] test: refuse an Account API host with an invalid port --- testv2/testv2_auth/test_account_api_host.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/testv2/testv2_auth/test_account_api_host.py b/testv2/testv2_auth/test_account_api_host.py index 12c1358a..4ba67643 100644 --- a/testv2/testv2_auth/test_account_api_host.py +++ b/testv2/testv2_auth/test_account_api_host.py @@ -133,3 +133,8 @@ def test_configured_host_is_normalised(monkeypatch): for claims in ({}, {"iss": "https://acme.kinde.com"}): api = _auth_with_claims(claims)._create_authenticated_api_client(PermissionsApi) assert api.api_client.configuration.host == "https://acme.kinde.com" + + +def test_host_with_an_invalid_port_is_refused(monkeypatch): + monkeypatch.setenv("KINDE_HOST", "https://acme.kinde.com:not-a-port") + assert _auth_with_claims({})._create_authenticated_api_client(PermissionsApi) is None From 4e1ceb705ec7ba9fd84a72e76350054ac6d64c1d Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Tue, 3 Nov 2026 05:48:30 +0200 Subject: [PATCH 16/20] fix(auth): keep a client secret passed to set_user_data() in memory The token manager gets the client secret from the in-memory map that an OAuth client fills in. Code that calls UserSession.set_user_data() directly, with client_secret in user_info and no OAuth client, lost the secret, so refresh and revoke were sent without it. Keep that secret in memory (it's still never written to session storage), without replacing a secret an OAuth client has already configured. --- kinde_sdk/auth/user_session.py | 5 +++++ testv2/testv2_auth/test_session_secrets.py | 24 ++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index ac0f9c2a..7011ed40 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -20,6 +20,11 @@ def __init__(self): def set_user_data(self, user_id: str, user_info: Dict[str, Any], token_data: Dict[str, Any]): """Store user session details and associate tokens.""" with self.lock: # Acquire the lock + # A secret passed in user_info (e.g. without an OAuth client) is kept in + # memory only; it's still never written to session storage + if user_info.get("client_secret") and not self.client_secrets.get(user_info["client_id"]): + self.client_secrets[user_info["client_id"]] = user_info["client_secret"] + if user_id not in self.user_sessions: # Create new token manager token_manager = TokenManager( diff --git a/testv2/testv2_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py index 8cc9ff2c..80695921 100644 --- a/testv2/testv2_auth/test_session_secrets.py +++ b/testv2/testv2_auth/test_session_secrets.py @@ -74,6 +74,30 @@ def test_secretless_oauth_instance_does_not_clear_configured_secret(self): self.assertEqual(UserSession.client_secrets.get("secret_test_client"), "super_secret") + def test_secret_passed_to_set_user_data_is_used_but_not_stored(self): + # Using UserSession directly, without an OAuth client registering the secret + UserSession.client_secrets.clear() + user_info = { + "client_id": "direct_client", + "client_secret": "direct_secret", + "token_url": "https://example.kinde.com/oauth2/token", + } + + session = UserSession() + session.set_user_data("user_2", user_info, self._token_data()) + + self.assertEqual(session.get_token_manager("user_2").client_secret, "direct_secret") + self.assertNotIn("direct_secret", repr(StorageManager().get("user_2"))) + + def test_secret_in_user_info_does_not_replace_the_configured_secret(self): + UserSession().set_user_data("user_3", { + "client_id": "secret_test_client", + "client_secret": "other_secret", + "token_url": "https://example.kinde.com/oauth2/token", + }, self._token_data()) + + self.assertEqual(UserSession.client_secrets["secret_test_client"], "super_secret") + def test_legacy_session_secret_is_dropped_on_load_and_not_saved_again(self): # A session written by an older SDK version, with client_secret in user_info StorageManager().setItems("user_1", { From b515150e4a2880752dab80945c2b743ccf03c018 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Tue, 3 Nov 2026 06:52:43 +0200 Subject: [PATCH 17/20] test: check that logout revokes the access token The logout tests only checked that the revoke request went to /oauth2/revoke. Also check that the token sent is the access token. --- testv2/testv2_auth/test_log_redaction.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/testv2/testv2_auth/test_log_redaction.py b/testv2/testv2_auth/test_log_redaction.py index 6e1ea215..3945bcc9 100644 --- a/testv2/testv2_auth/test_log_redaction.py +++ b/testv2/testv2_auth/test_log_redaction.py @@ -310,6 +310,7 @@ def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, flask_ assert _query(logout.headers["Location"]).get("id_token_hint") == tokens.id assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + assert token_manager_post.call_args.kwargs["data"]["token"] == tokens.access with client.session_transaction() as session: assert "user_id" not in session @@ -367,7 +368,8 @@ def test_logout_clears_server_side_tokens(self, fastapi_app, network): exchange, token_manager_post = network client, oauth = fastapi_app q = _query(client.get("/login").headers["location"]) - exchange.return_value = Tokens(q["nonce"]).response() + tokens = Tokens(q["nonce"]) + exchange.return_value = tokens.response() token_manager_post.return_value = MagicMock(status_code=200) client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") assert oauth._session_manager.user_sessions @@ -377,6 +379,7 @@ def test_logout_clears_server_side_tokens(self, fastapi_app, network): assert oauth._session_manager.user_sessions == {} # Best-effort revocation of the access token was attempted assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + assert token_manager_post.call_args.kwargs["data"]["token"] == tokens.access def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, fastapi_app, network): exchange, token_manager_post = network @@ -392,6 +395,7 @@ def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, fastap assert _query(logout.headers["location"]).get("id_token_hint") == tokens.id assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") + assert token_manager_post.call_args.kwargs["data"]["token"] == tokens.access def test_failed_exchange_returns_400_without_body_echo(self, fastapi_app, debug_logs, network): exchange, _ = network From 9619f94b9b24d5be3b37c8eadcfa74637e6ac389 Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Wed, 4 Nov 2026 08:06:04 +0200 Subject: [PATCH 18/20] fix(auth): keep a client secret passed to set_user_data() on that session only A secret passed in user_info was written into the shared client_secrets map, so later sessions for the same client without a secret picked it up. Give it to that session's token manager instead. Updating an existing session now also gives its manager a new secret (or the configured one for its client) when it has none, while keeping a secret it already has. --- kinde_sdk/auth/user_session.py | 23 ++++++++++------ testv2/testv2_auth/test_session_secrets.py | 31 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/kinde_sdk/auth/user_session.py b/kinde_sdk/auth/user_session.py index 7011ed40..ccf4e5a3 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -20,17 +20,18 @@ def __init__(self): def set_user_data(self, user_id: str, user_info: Dict[str, Any], token_data: Dict[str, Any]): """Store user session details and associate tokens.""" with self.lock: # Acquire the lock - # A secret passed in user_info (e.g. without an OAuth client) is kept in - # memory only; it's still never written to session storage - if user_info.get("client_secret") and not self.client_secrets.get(user_info["client_id"]): - self.client_secrets[user_info["client_id"]] = user_info["client_secret"] + # A secret passed in user_info (e.g. without an OAuth client) belongs to this + # session's token manager only. It isn't shared with other sessions for the + # same client, and it's never written to session storage. + # May be None for PKCE flow. + client_secret = user_info.get("client_secret") or self.client_secrets.get(user_info["client_id"]) if user_id not in self.user_sessions: # Create new token manager token_manager = TokenManager( - user_id, - user_info["client_id"], - self.client_secrets.get(user_info["client_id"]), # May be None for PKCE flow + user_id, + user_info["client_id"], + client_secret, user_info["token_url"] ) @@ -46,7 +47,13 @@ def set_user_data(self, user_id: str, user_info: Dict[str, Any], token_data: Dic else: # Update existing user info self.user_sessions[user_id]["user_info"] = user_info - + + # The manager may already exist (one per user), so give it the secret if it + # has none, but keep a secret it already has + token_manager = self.user_sessions[user_id]["token_manager"] + if not token_manager.client_secret and client_secret: + token_manager.client_secret = client_secret + # Set tokens in token manager self.user_sessions[user_id]["token_manager"].set_tokens(token_data) diff --git a/testv2/testv2_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py index 80695921..31c168fe 100644 --- a/testv2/testv2_auth/test_session_secrets.py +++ b/testv2/testv2_auth/test_session_secrets.py @@ -98,6 +98,37 @@ def test_secret_in_user_info_does_not_replace_the_configured_secret(self): self.assertEqual(UserSession.client_secrets["secret_test_client"], "super_secret") + def _direct_user_info(self, client_secret=None): + user_info = {"client_id": "direct_client", "token_url": "https://example.kinde.com/oauth2/token"} + if client_secret: + user_info["client_secret"] = client_secret + return user_info + + def test_secret_passed_to_set_user_data_is_not_shared_with_other_sessions(self): + session = UserSession() + session.set_user_data("user_2", self._direct_user_info("direct_secret"), self._token_data()) + session.set_user_data("user_4", self._direct_user_info(), self._token_data()) + + self.assertNotIn("direct_client", UserSession.client_secrets) + self.assertIsNone(session.get_token_manager("user_4").client_secret) + + def test_updating_a_session_gives_its_manager_a_new_secret_but_keeps_an_existing_one(self): + session = UserSession() + session.set_user_data("user_5", self._direct_user_info(), self._token_data()) + session.set_user_data("user_5", self._direct_user_info("first_secret"), self._token_data()) + self.assertEqual(session.get_token_manager("user_5").client_secret, "first_secret") + + session.set_user_data("user_5", self._direct_user_info("second_secret"), self._token_data()) + self.assertEqual(session.get_token_manager("user_5").client_secret, "first_secret") + + def test_updating_a_session_without_a_secret_uses_the_configured_one(self): + session = UserSession() + session.set_user_data("user_6", self._direct_user_info(), self._token_data()) + UserSession.client_secrets["direct_client"] = "configured_secret" + + session.set_user_data("user_6", self._direct_user_info(), self._token_data()) + self.assertEqual(session.get_token_manager("user_6").client_secret, "configured_secret") + def test_legacy_session_secret_is_dropped_on_load_and_not_saved_again(self): # A session written by an older SDK version, with client_secret in user_info StorageManager().setItems("user_1", { From 07a60c88221e86f9c7aa6ed866b047452637929d Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Wed, 4 Nov 2026 08:06:09 +0200 Subject: [PATCH 19/20] chore: ignore local env files, .DS_Store, Wrangler state and workspaces --- .gitignore | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 7c481a92..14634117 100644 --- a/.gitignore +++ b/.gitignore @@ -76,4 +76,14 @@ docs/_build/ .env .env.* !.env.example -!.env.sample \ No newline at end of file +!.env.sample +*.env + +# macOS +.DS_Store + +# Cloudflare Wrangler local state +.wrangler/ + +# Personal editor workspaces +*.code-workspace From 93075065d6e544e06440f817e38628579113214e Mon Sep 17 00:00:00 2001 From: Koman Rudden Date: Thu, 5 Nov 2026 06:28:05 +0200 Subject: [PATCH 20/20] fix(auth): sign out even if SDK logout fails; fail generation when a spec fix can't be applied --- generate_management_sdk.py | 20 +++++++--- kinde_fastapi/framework/fastapi_framework.py | 8 +++- kinde_flask/framework/flask_framework.py | 7 +++- testv2/testv2_auth/test_log_redaction.py | 38 +++++++++++++++++++ .../test_feature_flag_value_types.py | 19 +++++++++- 5 files changed, 83 insertions(+), 9 deletions(-) diff --git a/generate_management_sdk.py b/generate_management_sdk.py index ff7c0e2a..c6ba01f0 100755 --- a/generate_management_sdk.py +++ b/generate_management_sdk.py @@ -669,24 +669,33 @@ def add_custom_imports(config: Dict[str, Any]): ] -def apply_spec_errata(config: Dict[str, Any]): - """Correct generated models where the spec doesn't match the API.""" +def apply_spec_errata(config: Dict[str, Any]) -> bool: + """Correct generated models where the spec doesn't match the API. + + Returns False if a correction can't be applied, so generation fails instead of + shipping the bug the correction fixes. + """ print_section("Applying Spec Errata") output_dir = Path(config["output_dir"]) + ok = True for relative_path, generated, corrected in SPEC_ERRATA: path = output_dir / relative_path if not path.exists(): - print(f"⚠️ Warning: {relative_path} not found") + print(f"❌ {relative_path} not found; update SPEC_ERRATA") + ok = False continue content = path.read_text(encoding="utf-8") if corrected in content: continue if generated not in content: - print(f"⚠️ Warning: {relative_path} changed upstream; review the erratum") + print(f"❌ {relative_path} changed upstream; update the entry in SPEC_ERRATA, " + "or remove it if the spec is now correct") + ok = False continue path.write_text(content.replace(generated, corrected), encoding="utf-8") print(f"✓ Patched {relative_path}") + return ok # ============================================================================= @@ -845,7 +854,8 @@ def generate_single_sdk(skip_tests: bool, no_diff: bool) -> bool: add_custom_imports(config) # Step 3b: Correct models where the published spec is wrong - apply_spec_errata(config) + if not apply_spec_errata(config): + return False # Step 4: Validate if not validate_generation(config): diff --git a/kinde_fastapi/framework/fastapi_framework.py b/kinde_fastapi/framework/fastapi_framework.py index d40d8c70..7ab8870a 100644 --- a/kinde_fastapi/framework/fastapi_framework.py +++ b/kinde_fastapi/framework/fastapi_framework.py @@ -234,7 +234,13 @@ async def logout(request: Request): # Log out before clearing the session: after a restart, or on another worker, # the SDK reads the user's tokens from the session to revoke them and to build # the logout URL - logout_url = await self._oauth.logout(user_id) + try: + logout_url = await self._oauth.logout(user_id) + except Exception as e: + # Still sign the user out here and at Kinde. Returning a response (not + # raising) is what saves the cleared session to the cookie + self._logger.error("Logout failed: %s", type(e).__name__) + logout_url = await self._oauth.logout() request.session.clear() return RedirectResponse(url=logout_url) diff --git a/kinde_flask/framework/flask_framework.py b/kinde_flask/framework/flask_framework.py index cb895bd6..5731fa13 100644 --- a/kinde_flask/framework/flask_framework.py +++ b/kinde_flask/framework/flask_framework.py @@ -282,7 +282,12 @@ def logout(): # Log out before clearing the session: after a restart, or on another worker, # the SDK reads the user's tokens from the session to revoke them and to build # the logout URL - logout_url = self._run_async(self._oauth.logout(user_id)) + try: + logout_url = self._run_async(self._oauth.logout(user_id)) + except Exception as e: + # Still sign the user out here and at Kinde + logger.error("Logout failed: %s", type(e).__name__) + logout_url = self._run_async(self._oauth.logout()) session.clear() return redirect(logout_url) diff --git a/testv2/testv2_auth/test_log_redaction.py b/testv2/testv2_auth/test_log_redaction.py index 3945bcc9..813a34df 100644 --- a/testv2/testv2_auth/test_log_redaction.py +++ b/testv2/testv2_auth/test_log_redaction.py @@ -314,6 +314,22 @@ def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, flask_ with client.session_transaction() as session: assert "user_id" not in session + def test_logout_still_signs_out_when_sdk_logout_fails(self, flask_app, network): + exchange, token_manager_post = network + app, oauth = flask_app + client = app.test_client() + q = _query(client.get("/login").headers["Location"]) + exchange.return_value = Tokens(q["nonce"]).response() + client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + + with patch.object(oauth._session_manager, "logout", side_effect=OSError("storage down")): + logout = client.get("/logout") + + assert logout.status_code == 302 + assert logout.headers["Location"].startswith(f"{HOST}/logout") + with client.session_transaction() as session: + assert "user_id" not in session + def _forget_in_memory_sessions(oauth): """Simulate a restarted process or another worker: only the session has the tokens.""" @@ -397,6 +413,28 @@ def test_logout_after_restart_still_revokes_and_sends_id_token_hint(self, fastap assert token_manager_post.call_args.args[0].endswith("/oauth2/revoke") assert token_manager_post.call_args.kwargs["data"]["token"] == tokens.access + def test_logout_still_signs_out_when_sdk_logout_fails(self, fastapi_app, network): + exchange, _ = network + client, oauth = fastapi_app + from fastapi import Request + + @client.app.get("/whoami") + async def whoami(request: Request): + return {"user_id": request.session.get("user_id")} + + q = _query(client.get("/login").headers["location"]) + exchange.return_value = Tokens(q["nonce"]).response() + client.get(f"/callback?code={AUTH_CODE}&state={q['state']}") + assert client.get("/whoami").json()["user_id"] + + with patch.object(oauth._session_manager, "logout", side_effect=OSError("storage down")): + logout = client.get("/logout") + + assert logout.status_code in (302, 307) + assert logout.headers["location"].startswith(f"{HOST}/logout") + # The cleared session reached the cookie, so the user is signed out + assert client.get("/whoami").json()["user_id"] is None + def test_failed_exchange_returns_400_without_body_echo(self, fastapi_app, debug_logs, network): exchange, _ = network client, _ = fastapi_app diff --git a/testv2/testv2_management/test_feature_flag_value_types.py b/testv2/testv2_management/test_feature_flag_value_types.py index ea7e037b..ec9f646e 100644 --- a/testv2/testv2_management/test_feature_flag_value_types.py +++ b/testv2/testv2_management/test_feature_flag_value_types.py @@ -40,10 +40,25 @@ def test_generator_erratum_patches_generated_model(tmp_path): ) target.write_text(generated) - generator.apply_spec_errata({"output_dir": str(tmp_path)}) + assert generator.apply_spec_errata({"output_dir": str(tmp_path)}) patched = target.read_text() assert "Optional[Union[StrictBool, StrictInt, StrictStr]]" in patched assert "StrictBool, StrictInt, StrictStr, field_validator" in patched - generator.apply_spec_errata({"output_dir": str(tmp_path)}) + assert generator.apply_spec_errata({"output_dir": str(tmp_path)}) assert target.read_text() == patched + + +@pytest.mark.parametrize("generated", [ + None, # the model file is missing + "from pydantic import BaseModel\nclass M(BaseModel):\n value: Optional[StrictStr] = Field(default=None)\n", +]) +def test_generator_fails_when_erratum_cannot_be_applied(tmp_path, generated): + import generate_management_sdk as generator + + if generated is not None: + target = tmp_path / "models" / "get_organization_feature_flags_response_feature_flags_value.py" + target.parent.mkdir() + target.write_text(generated) + + assert generator.apply_spec_errata({"output_dir": str(tmp_path)}) is False