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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 43e6aae4..75be8d67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,76 @@ 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. 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). +- **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. + +### 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 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. +- **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..c6ba01f0 100755 --- a/generate_management_sdk.py +++ b/generate_management_sdk.py @@ -645,6 +645,59 @@ 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]) -> 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"❌ {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"❌ {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 + + # ============================================================================= # VALIDATION # ============================================================================= @@ -799,6 +852,10 @@ 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 + if not apply_spec_errata(config): + return False # 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 04fb09d0..7ab8870a 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, KindeTokenException from ..middleware.framework_middleware import FrameworkMiddleware import os import uuid @@ -139,7 +140,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 @@ -182,17 +182,32 @@ 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: + return HTMLResponse("Authentication failed: Missing authorization code", status_code=400) 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: + # 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: - 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 + # 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=500) request.session['user_id'] = user_id @@ -206,23 +221,28 @@ 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') + # 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 + 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=await self._oauth.logout()) + 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 2d69a15a..5731fa13 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, KindeTokenException 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,26 @@ 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 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: - return f"Authentication failed: {str(e)}", 400 + # Anything else is a server-side fault, not a bad request + logger.error("OAuth callback failed: %s", type(e).__name__) + return "Authentication failed", 500 + + # 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 @@ -266,8 +279,16 @@ def callback(): def logout(): """Logout the user and redirect to Kinde logout page.""" user_id = 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 + 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() - logout_url = self._run_async(self._oauth.logout(user_id)) return redirect(logout_url) # Register route @@ -288,7 +309,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_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..aab6a037 100644 --- a/kinde_sdk/auth/base_auth.py +++ b/kinde_sdk/auth/base_auth.py @@ -1,5 +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 @@ -11,7 +13,6 @@ class BaseAuth: def __init__(self): self._logger = logging.getLogger("kinde_sdk") - self._logger.setLevel(logging.INFO) self._framework = None self._session_manager = UserSession() @@ -50,6 +51,58 @@ 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 _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 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. + # Use the normalised origin, so stray whitespace or a trailing slash in the + # configuration can't end up in the API URL. + hosts = [ + 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 {} + issuer = claims.get("iss") if isinstance(claims, dict) else None + if issuer is not None: + issuer_origin = BaseAuth._https_origin(issuer) + for host in hosts: + if 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): """ Create an authenticated API client for the current user. @@ -79,8 +132,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 an HTTPS 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 0c2d19d1..c65a7afe 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 @@ -12,7 +13,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, @@ -20,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, @@ -60,6 +79,10 @@ def __init__( # Validate required configurations if not self.client_id: raise KindeConfigurationException("Client ID is required.") + # 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() @@ -84,9 +107,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 +216,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 +240,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() @@ -486,9 +505,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 @@ -515,14 +539,30 @@ 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") + + # 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") + # Get code verifier for PKCE code_verifier = None stored_code_verifier = self._session_manager.storage_manager.get("user:code_verifier") @@ -538,11 +578,42 @@ 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. + # 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 not id_token: + self._logger.error("Token response has no ID token") + raise KindeLoginException("Missing ID token") + try: + # 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") # Store tokens user_info = { "client_id": self.client_id, - "client_secret": self.client_secret, "token_url": self.token_url, "redirect_uri": self.redirect_uri, } @@ -565,13 +636,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 @@ -603,12 +667,10 @@ 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}") + 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/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/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 3411fc39..6da099ce 100644 --- a/kinde_sdk/auth/token_manager.py +++ b/kinde_sdk/auth/token_manager.py @@ -2,9 +2,11 @@ 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 + class TokenManager: _instances = {} _lock = threading.Lock() # Add a lock for thread safety @@ -33,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): @@ -112,7 +118,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,11 +157,16 @@ 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() 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): @@ -218,7 +229,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..ccf4e5a3 100644 --- a/kinde_sdk/auth/user_session.py +++ b/kinde_sdk/auth/user_session.py @@ -5,6 +5,13 @@ 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]] = {} + # 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 self.lock = threading.Lock() # Add a lock for thread safety @@ -13,18 +20,25 @@ 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) 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"], - user_info.get("client_secret"), # May be None for PKCE flow + user_id, + user_info["client_id"], + client_secret, user_info["token_url"] ) # 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, @@ -33,21 +47,33 @@ 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) # 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) 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 @@ -82,18 +108,24 @@ 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, + # 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, user_info.get("client_id"), - user_info.get("client_secret"), + self.client_secrets.get(user_info.get("client_id")), user_info.get("token_url") ) # 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/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..ce41578a 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 @@ -113,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}" @@ -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 @@ -216,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}" @@ -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 @@ -269,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}" @@ -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 @@ -323,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}" @@ -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 @@ -387,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}" @@ -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 @@ -474,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}" @@ -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/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/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/kinde_sdk/core/storage/storage_manager.py b/kinde_sdk/core/storage/storage_manager.py index c40d4f8b..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,9 +7,11 @@ from .storage_factory import StorageFactory from .storage_interface import StorageInterface +logger = logging.getLogger(__name__) + 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 +75,28 @@ 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 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 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..4ba67643 --- /dev/null +++ b/testv2/testv2_auth/test_account_api_host.py @@ -0,0 +1,140 @@ +""" +Account API calls (force_api) must target the configured Kinde domain, not the +generated client's placeholder host or an unverified token's issuer. +""" +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" + + +@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_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://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({}) + + 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_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"}, {"iss": "https://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({}) + + 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 + + +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" + + +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 diff --git a/testv2/testv2_auth/test_feature_flags.py b/testv2/testv2_auth/test_feature_flags.py index 970a6ff3..90b2f82a 100644 --- a/testv2/testv2_auth/test_feature_flags.py +++ b/testv2/testv2_auth/test_feature_flags.py @@ -238,4 +238,65 @@ 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" + + +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_log_redaction.py b/testv2/testv2_auth/test_log_redaction.py new file mode 100644 index 00000000..813a34df --- /dev/null +++ b/testv2/testv2_auth/test_log_redaction.py @@ -0,0 +1,485 @@ +""" +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}", + iss=HOST, aud=[CLIENT_ID], exp=int(time.time()) + 3600) + + 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 + + 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") + assert token_manager_post.call_args.kwargs["data"]["token"] == tokens.access + 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.""" + oauth._session_manager.user_sessions.clear() + TokenManager.reset_instances() + + +# --------------------------------------------------------------------------- +# 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"]) + 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 + + 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") + 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 + 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") + 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 + 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_oauth_callback_security.py b/testv2/testv2_auth/test_oauth_callback_security.py new file mode 100644 index 00000000..95e59f28 --- /dev/null +++ b/testv2/testv2_auth/test_oauth_callback_security.py @@ -0,0 +1,421 @@ +""" +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), + * 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); +the Flask / FastAPI tests drive the real ``/login`` and ``/callback`` routes. +Only the network calls (token endpoint, userinfo) are mocked. +""" +import asyncio +import tempfile +import time +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", + "exp": int(time.time()) + 3600} + payload.update(claims) + # 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") + + +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_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) + 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 + + @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 +# --------------------------------------------------------------------------- + +@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_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"])) + r = flask_client.get(f"/callback?code=c&state={q['state']}") + 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 +# --------------------------------------------------------------------------- + +@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") + + 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_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"]) + 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 + + 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 diff --git a/testv2/testv2_auth/test_permissions.py b/testv2/testv2_auth/test_permissions.py index 5b8854b4..bf7730a4 100644 --- a/testv2/testv2_auth/test_permissions.py +++ b/testv2/testv2_auth/test_permissions.py @@ -183,4 +183,62 @@ 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"]} + +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_auth/test_session_secrets.py b/testv2/testv2_auth/test_session_secrets.py new file mode 100644 index 00000000..31c168fe --- /dev/null +++ b/testv2/testv2_auth/test_session_secrets.py @@ -0,0 +1,157 @@ +import asyncio +import time +import unittest +from unittest.mock import AsyncMock, patch +from urllib.parse import parse_qs, urlparse + +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() + 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")): + return OAuth( + client_id="secret_test_client", + client_secret=client_secret, + redirect_uri="http://localhost/callback", + host="https://example.kinde.com", + ) + + def _token_data(self): + return { + "access_token": jwt.encode({"sub": "user_1"}, "key", algorithm="HS256"), + "refresh_token": "refresh", + "expires_in": 3600, + } + + def _complete_login(self, user_id="user_1"): + # 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()) + 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, "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={})): + 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"]) + 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") + + 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_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 _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", { + "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") + # 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()) + self.assertNotIn("super_secret", repr(StorageManager().get("user_1"))) + + +if __name__ == "__main__": + unittest.main() 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 diff --git a/testv2/testv2_auth/test_user_session.py b/testv2/testv2_auth/test_user_session.py index fc9dfb4a..99d81937 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) @@ -429,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_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_helpers.py b/testv2/testv2_core/test_helpers.py index 8e54da3c..a424f384 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): @@ -167,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.""" @@ -280,7 +283,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): @@ -305,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.""" @@ -366,7 +371,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): @@ -393,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.""" @@ -456,7 +463,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): @@ -483,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.""" @@ -545,7 +554,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 +571,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): @@ -586,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.""" @@ -713,7 +725,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 +742,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): @@ -754,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.""" @@ -904,7 +919,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 +1162,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): 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..8b01f919 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.""" @@ -161,5 +192,49 @@ 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) + + 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 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..ec9f646e --- /dev/null +++ b/testv2/testv2_management/test_feature_flag_value_types.py @@ -0,0 +1,64 @@ +"""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) + + 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 + + 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