Feat/role based protection #113
Conversation
WalkthroughAdds a YAML/JSON-driven route protection system, framework-agnostic middleware with Flask and FastAPI adapters, OAuth integration for runtime route checks, management client user endpoint parameterization, and a new Flask example demonstrating route-protection usage and diagnostics. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as User Request
participant App as Flask App
participant Middleware as FlaskRouteProtectionMiddleware
participant OAuth as OAuth Instance
participant Engine as RouteProtectionEngine
Client->>App: HTTP Request
App->>Middleware: before_request hook
Middleware->>Middleware: extract path & method
Middleware->>OAuth: check_route_access(path, method)
OAuth->>Engine: validate_route_access(path, method)
Engine->>Engine: match rule / check roles & permissions
Engine-->>OAuth: {allowed, reason, details}
OAuth-->>Middleware: boolean / structured result
alt allowed
Middleware-->>App: continue request
App-->>Client: handler response
else denied
Middleware-->>Client: 403 response (error handler)
end
sequenceDiagram
participant Init as App Startup
participant OAuth as OAuth Class
participant Engine as RouteProtectionEngine
Init->>OAuth: __init__(route_protection_file=config.yaml)
OAuth->>Engine: instantiate RouteProtectionEngine(config.yaml)
Engine->>Engine: load_route_config()
Engine-->>OAuth: ready (or logged warning on failure)
Note over OAuth,Engine: Runtime checks call Engine.validate_route_access(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_sdk/auth/route_middleware.py (1)
1-352: Add comprehensive unit tests for the middlewareThis critical security component should have extensive test coverage including edge cases, error scenarios, and framework-specific behaviors.
Would you like me to generate comprehensive unit tests for the route protection middleware, covering:
- Path pattern matching edge cases
- Framework-specific integration tests
- Error handling scenarios
- Async/sync execution contexts
- OAuth client interaction mocking
🧹 Nitpick comments (22)
kinde_sdk/management/management_client.py (2)
474-482: Remove unused manual querystring assembly
param_serializealready handlesquery_params. This block is dead code and risks future duplication.Apply:
- # FIXED: Use param_serialize to properly construct the full URL with host - # Handle query parameters by appending them to the path - final_path = formatted_path - if query_params and http_method in ('GET', 'DELETE'): - query_string = '&'.join([f"{k}={v}" for k, v in query_params.items() if v is not None]) - if query_string: - separator = '&' if '?' in final_path else '?' - final_path = f"{final_path}{separator}{query_string}"
487-493: Avoid shadowingmethodfrom outer scopeSmall readability nit; don’t shadow the
http_methodname.Apply:
- method, url, header_params, body, post_params = self.api_client.param_serialize( + _, url, header_params, body, post_params = self.api_client.param_serialize(docs/ROUTE_PROTECTION.md (1)
251-267: Prefer sync helper in Flask to avoid per-request event loopIn Flask “Manual Protection”, use
check_route_accessinstead ofasyncio.run.Apply:
-@app.before_request -def check_access(): - if oauth.is_route_protection_enabled(): - # Check if current route is protected - result = asyncio.run(oauth.validate_route_access(request.path, request.method)) - if not result["allowed"]: - return {"error": "Access Denied"}, 403 - return None +@app.before_request +def check_access(): + if oauth.is_route_protection_enabled(): + if not oauth.check_route_access(request.path, request.method): + return {"error": "Access Denied"}, 403 + return Nonekinde_sdk/auth/route_protection.py (3)
312-340: Reduce N network calls by batching role checksUse
get_roles()once and check membership locally to cut latency.Example:
- try: - # Check each required role - for role_key in required_roles: - role_result = await self._roles_client.get_role(role_key, options) - if role_result and role_result.get("isGranted", False): - return { - "allowed": True, - "reason": f"User has required role: {role_key}" - } + try: + roles_data = await self._roles_client.get_roles(options) + have = {r.get("key") for r in roles_data.get("roles", []) if isinstance(r, dict)} + for role_key in required_roles: + if role_key in have: + return {"allowed": True, "reason": f"User has required role: {role_key}"}
348-376: Reduce N network calls by batching permission checksSame optimization for permissions.
Example:
- try: - # Check each required permission - for permission_key in required_permissions: - permission_result = await self._permissions_client.get_permission(permission_key, options) - if permission_result and permission_result.get("isGranted", False): - return { - "allowed": True, - "reason": f"User has required permission: {permission_key}" - } + try: + perms_data = await self._permissions_client.get_permissions(options) + have = set(perms_data.get("permissions", [])) + for permission_key in required_permissions: + if permission_key in have: + return {"allowed": True, "reason": f"User has required permission: {permission_key}"}
59-61: Remove unused module-level logger
logger = logging.getLogger(__name__)is unused; rely onself._logger.-logger = logging.getLogger(__name__) -kinde_sdk/auth/__init__.py (2)
15-21: Make optional imports resilient to partial availability.A single failed attribute import from route_middleware disables all route-protection exports. Import the module and getattr() each symbol so missing framework-specific middleware doesn’t turn off the whole feature set.
- from .route_protection import RouteProtectionEngine, route_protection - from .route_middleware import ( - RouteProtectionMiddleware, - FlaskRouteProtectionMiddleware, - FastAPIRouteProtectionMiddleware, - create_route_protection_middleware - ) - ROUTE_PROTECTION_AVAILABLE = True - _route_protection_components = [ - "RouteProtectionEngine", "route_protection", - "RouteProtectionMiddleware", "create_route_protection_middleware" - ] - # Add framework-specific middleware if available - if FlaskRouteProtectionMiddleware: - _route_protection_components.append("FlaskRouteProtectionMiddleware") - if FastAPIRouteProtectionMiddleware: - _route_protection_components.append("FastAPIRouteProtectionMiddleware") + from .route_protection import RouteProtectionEngine, route_protection + from . import route_middleware as _rm + ROUTE_PROTECTION_AVAILABLE = True + RouteProtectionMiddleware = getattr(_rm, "RouteProtectionMiddleware", None) + FlaskRouteProtectionMiddleware = getattr(_rm, "FlaskRouteProtectionMiddleware", None) + FastAPIRouteProtectionMiddleware = getattr(_rm, "FastAPIRouteProtectionMiddleware", None) + create_route_protection_middleware = getattr(_rm, "create_route_protection_middleware", None) + _route_protection_components = ["RouteProtectionEngine", "route_protection"] + if RouteProtectionMiddleware: + _route_protection_components.append("RouteProtectionMiddleware") + if create_route_protection_middleware: + _route_protection_components.append("create_route_protection_middleware") + if FlaskRouteProtectionMiddleware: + _route_protection_components.append("FlaskRouteProtectionMiddleware") + if FastAPIRouteProtectionMiddleware: + _route_protection_components.append("FastAPIRouteProtectionMiddleware")
36-39: Adopt iterable unpacking for all.Minor style/perf improvement per Ruff RUF005.
-__all__ = [ - "OAuth", "TokenManager", "UserSession", "permissions", "ApiOptions", - "claims", "feature_flags", "portals", "tokens", "roles" -] + _route_protection_components +__all__ = [ + "OAuth", "TokenManager", "UserSession", "permissions", "ApiOptions", + "claims", "feature_flags", "portals", "tokens", "roles", + *_route_protection_components, +]kinde_sdk/auth/oauth.py (3)
106-118: Use logging.exception and tighten the catch during engine init.Keep the graceful fallback but log the traceback for easier diagnosis.
- if route_protection_file and ROUTE_PROTECTION_AVAILABLE: + if route_protection_file and ROUTE_PROTECTION_AVAILABLE: try: self._route_protection = RouteProtectionEngine(route_protection_file) self._logger.info(f"Route protection enabled with config: {route_protection_file}") - except Exception as e: - self._logger.error(f"Failed to initialize route protection: {e}") + except Exception: + self._logger.exception("Failed to initialize route protection")
815-819: Drop the blanket try/except or at least log._get_framework silently swallows all exceptions, making debugging harder.
- try: - return getattr(self, '_framework', None) - except Exception: - return None + return getattr(self, "_framework", None)
148-157: Docstrings mention a request parameter that doesn’t exist.is_authenticated/get_user_info docstrings still reference a request arg. Update to avoid confusion.
examples/flask_route_protection_example.py (5)
15-21: Resolve config path relative to this file to prevent CWD surprises.-# Initialize Kinde OAuth with route protection -oauth = OAuth( +# Initialize Kinde OAuth with route protection +CONFIG_PATH = os.path.join(os.path.dirname(__file__), "route_protection_config.yaml") +oauth = OAuth( framework="flask", app=app, - route_protection_file="route_protection_config.yaml" # Enable route protection + route_protection_file=CONFIG_PATH # Enable route protection )
130-138: Return a real timestamp.Use UTC now to avoid stale metadata in health checks.
+from datetime import datetime @@ return jsonify({ "status": "healthy", "route_protection": oauth.is_route_protection_enabled(), - "timestamp": "2024-01-01T00:00:00Z" + "timestamp": datetime.utcnow().isoformat() + "Z" })
257-265: Silence unused arg warning in error handlers.Prefix with underscore to satisfy linters without changing Flask’s signature.
-@app.errorhandler(403) -def forbidden(error): +@app.errorhandler(403) +def forbidden(_error):
266-274: Same here for 404 handler.-@app.errorhandler(404) -def not_found(error): +@app.errorhandler(404) +def not_found(_error):
285-289: Do not ship with debug=True; gate by env and allow PORT override.Prevents accidental exposure of Werkzeug debugger in real environments.
- app.run(debug=True, port=5000) + debug = os.getenv("FLASK_DEBUG", "0") == "1" + port = int(os.getenv("PORT", "5000")) + app.run(debug=debug, port=port)kinde_sdk/auth/route_middleware.py (6)
200-202: Remove unused parameters from the error handler methodThe
reasonandkwargsparameters are not used in the Flask error handler implementation.- def _default_error_handler(self, reason: str, **kwargs) -> Any: + def _default_error_handler(self, reason: str, **kwargs) -> Any: # pylint: disable=unused-argument """Default Flask error handler returns JSON response.""" return flask_jsonify({"error": "Access Denied"}), 403Or alternatively, use the reason in the response:
def _default_error_handler(self, reason: str, **kwargs) -> Any: """Default Flask error handler returns JSON response.""" - return flask_jsonify({"error": "Access Denied"}), 403 + return flask_jsonify({"error": "Access Denied", "reason": reason}), 403
261-266: Remove unused parameters from the FastAPI error handlerSimilar to the Flask handler, the
reasonandkwargsparameters are not used.- def _default_error_handler(self, reason: str, **kwargs) -> JSONResponse: + def _default_error_handler(self, reason: str, **kwargs) -> JSONResponse: # pylint: disable=unused-argument """Default FastAPI error handler.""" return JSONResponse( status_code=403, - content={"error": "Access Denied"} + content={"error": "Access Denied", "reason": reason} )
277-279: Remove unnecessary init_subclass methodThe
__init_subclass__method is defined but doesn't add any functionality beyond calling super(). It should be removed.- # Monkey patch the method for this instance - def __init_subclass__(cls, **kwargs): - super().__init_subclass__(**kwargs)
340-342: Improve error message definitionMove the error message to a constant or the exception class for better maintainability.
+ FASTAPI_APP_REQUIRED_ERROR = "FastAPI middleware requires 'app' parameter" + elif framework == "fastapi" and FastAPIRouteProtectionMiddleware: if not app: - raise ValueError("FastAPI middleware requires 'app' parameter") + raise ValueError(FASTAPI_APP_REQUIRED_ERROR)
172-173: Consider more robust path normalizationThe current path normalization only handles trailing slashes. Consider normalizing multiple slashes and handling edge cases.
# Normalize paths - path = path.rstrip('/') - pattern = pattern.rstrip('/') + import re + # Normalize paths: remove trailing slashes and collapse multiple slashes + path = re.sub(r'/+', '/', path.rstrip('/')) + pattern = re.sub(r'/+', '/', pattern.rstrip('/'))
100-140: Consider adding request context to logs for better debuggingThe logging statements could benefit from including request ID or correlation ID for better traceability in production environments.
Would you like me to help implement a request context logging pattern that includes correlation IDs for better traceability?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
examples/route_protection_config.yamlis excluded by!**/*.yaml
📒 Files selected for processing (7)
docs/ROUTE_PROTECTION.md(1 hunks)examples/flask_route_protection_example.py(1 hunks)kinde_sdk/auth/__init__.py(1 hunks)kinde_sdk/auth/oauth.py(4 hunks)kinde_sdk/auth/route_middleware.py(1 hunks)kinde_sdk/auth/route_protection.py(1 hunks)kinde_sdk/management/management_client.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-03T09:38:46.175Z
Learnt from: brettchaldecott
PR: kinde-oss/kinde-python-sdk#83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
Applied to files:
kinde_sdk/auth/oauth.pyexamples/flask_route_protection_example.py
🧬 Code graph analysis (1)
kinde_sdk/auth/route_protection.py (4)
kinde_sdk/auth/config_loader.py (1)
load_config(6-26)kinde_sdk/auth/roles.py (2)
Roles(8-230)get_role(9-82)kinde_sdk/auth/permissions.py (2)
Permissions(8-143)get_permission(9-55)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)
🪛 Ruff (0.12.2)
kinde_sdk/auth/route_protection.py
111-111: Abstract raise to an inner function
(TRY301)
111-111: Avoid specifying long messages outside the exception class
(TRY003)
133-133: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
134-134: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
134-134: Avoid specifying long messages outside the exception class
(TRY003)
135-135: Do not catch blind exception: Exception
(BLE001)
136-136: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
137-137: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
137-137: Avoid specifying long messages outside the exception class
(TRY003)
336-339: Consider moving this statement to an else block
(TRY300)
341-341: Do not catch blind exception: Exception
(BLE001)
342-342: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
345-345: Use explicit conversion flag
Replace with conversion flag
(RUF010)
372-375: Consider moving this statement to an else block
(TRY300)
377-377: Do not catch blind exception: Exception
(BLE001)
378-378: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
381-381: Use explicit conversion flag
Replace with conversion flag
(RUF010)
kinde_sdk/auth/oauth.py
112-112: Do not catch blind exception: Exception
(BLE001)
113-113: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
667-667: Avoid specifying long messages outside the exception class
(TRY003)
667-667: Use explicit conversion flag
Replace with conversion flag
(RUF010)
675-675: Unused method argument: request
(ARG002)
718-719: try-except-pass detected, consider logging the exception
(S110)
718-718: Do not catch blind exception: Exception
(BLE001)
740-740: Redefinition of unused asyncio from line 5
Remove definition: asyncio
(F811)
745-745: Do not catch blind exception: Exception
(BLE001)
746-746: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
811-811: Do not catch blind exception: Exception
(BLE001)
818-818: Do not catch blind exception: Exception
(BLE001)
kinde_sdk/auth/route_middleware.py
134-134: Do not catch blind exception: Exception
(BLE001)
135-135: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
137-137: Use explicit conversion flag
Replace with conversion flag
(RUF010)
200-200: Unused method argument: reason
(ARG002)
200-200: Unused method argument: kwargs
(ARG002)
218-218: Do not catch blind exception: Exception
(BLE001)
219-219: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
220-220: Use explicit conversion flag
Replace with conversion flag
(RUF010)
261-261: Unused method argument: reason
(ARG002)
261-261: Unused method argument: kwargs
(ARG002)
341-341: Avoid specifying long messages outside the exception class
(TRY003)
examples/flask_route_protection_example.py
258-258: Unused function argument: error
(ARG001)
267-267: Unused function argument: error
(ARG001)
289-289: Use of debug=True in Flask app detected
(S201)
kinde_sdk/auth/__init__.py
36-39: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
kinde_sdk/management/management_client.py
430-430: Function definition does not bind loop variable api_method
(B023)
🪛 LanguageTool
docs/ROUTE_PROTECTION.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...etup. ## Table of Contents - Overview - Quick Start - [Configurat...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ... - Overview - Quick Start - Configuration - [Framew...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ck Start](#quick-start) - Configuration - [Framework Integration](#framework-integr...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...#configuration) - Framework Integration - API Reference - [Exampl...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...#framework-integration) - API Reference - Examples - [Best Practices](...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...I Reference](#api-reference) - Examples - Best Practices - [Trou...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... Examples - Best Practices - Troubleshooting ## O...
(QB_NEW_EN)
[grammar] ~160-~160: There might be a mistake here.
Context: ...s: | Pattern | Description | Examples | |---------|-------------|----------| | `...
(QB_NEW_EN)
[grammar] ~161-~161: There might be a mistake here.
Context: ...s | |---------|-------------|----------| | /admin | Exact match | Matches only ...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ... | Exact match | Matches only /admin | | /admin/* | Wildcard match | Matches ...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...n/users, /admin/settings/view, etc. | | /api/v1/*` | Prefix wildcard | Matche...
(QB_NEW_EN)
[grammar] ~354-~354: There might be a mistake here.
Context: ..... other parameters ) ``` Parameters: - route_protection_file: Path to YAML/JSON configuration file ...
(QB_NEW_EN)
[grammar] ~690-~690: There might be a mistake here.
Context: ...oute Protection Not Working Symptoms: - Routes that should be protected are acce...
(QB_NEW_EN)
[grammar] ~718-~718: There might be a mistake here.
Context: ... 403 Forbidden Unexpectedly Symptoms: - Users with correct roles still getting d...
(QB_NEW_EN)
[grammar] ~741-~741: There might be a mistake here.
Context: ...onfiguration File Not Found Symptoms: - Error messages about missing configurati...
(QB_NEW_EN)
[grammar] ~742-~742: There might be a mistake here.
Context: ...essages about missing configuration file - Route protection not initializing **Sol...
(QB_NEW_EN)
[grammar] ~766-~766: There might be a mistake here.
Context: ...missions Not Found in Token Symptoms: - Users have roles in Kinde admin but rout...
(QB_NEW_EN)
[grammar] ~770-~770: There might be a mistake here.
Context: ...permissions being returned Solutions: 1. Check Kinde Admin Panel: - Applicat...
(QB_NEW_EN)
[grammar] ~771-~771: There might be a mistake here.
Context: ...lutions:** 1. Check Kinde Admin Panel: - Applications → Your App → Token customiz...
(QB_NEW_EN)
[grammar] ~772-~772: There might be a mistake here.
Context: ...cations → Your App → Token customization - Ensure "Include roles in tokens" is enab...
(QB_NEW_EN)
[grammar] ~802-~802: There might be a mistake here.
Context: ... 5. Pattern Matching Issues Symptoms: - Route patterns not matching expected pat...
(QB_NEW_EN)
[grammar] ~803-~803: There might be a mistake here.
Context: ...ute patterns not matching expected paths - Wildcard patterns not working correctly ...
(QB_NEW_EN)
[grammar] ~895-~895: There might be a mistake here.
Context: ...e documentation or contact support with: - Your configuration file - Debug logs sho...
(QB_NEW_EN)
[grammar] ~896-~896: There might be a mistake here.
Context: ... support with: - Your configuration file - Debug logs showing the issue - Steps to ...
(QB_NEW_EN)
[grammar] ~897-~897: There might be a mistake here.
Context: ...tion file - Debug logs showing the issue - Steps to reproduce the problem - Expecte...
(QB_NEW_EN)
[grammar] ~898-~898: There might be a mistake here.
Context: ...e issue - Steps to reproduce the problem - Expected vs actual behavior
(QB_NEW_EN)
[grammar] ~899-~899: There might be a mistake here.
Context: ...he problem - Expected vs actual behavior
(QB_NEW_EN)
🪛 ast-grep (0.38.6)
examples/flask_route_protection_example.py
[warning] 288-288: Detected Flask app with debug=True. Do not deploy to production with this flag enabled as it will leak sensitive information. Instead, consider using Flask configuration variables or setting 'debug' using system environment variables.
Context: app.run(debug=True, port=5000)
Note: [CWE-489] Active Debug Code. [REFERENCES]
- https://labs.detectify.com/2015/10/02/how-patreon-got-hacked-publicly-exposed-werkzeug-debugger/
(debug-enabled-python)
🪛 GitHub Actions: CI
kinde_sdk/management/management_client.py
[error] 1-1: Docstring mismatch: expected 'Get a user by ID' in get_user() docstring, but actual docstring reads 'Get user by ID (wrapper for backward compatibility). Bug #89 Fix: The actual API endpoint is GET /api/v1/user?id={user_id} but this wrapper maintains the original get_user(user_id) signature.' Failing test: testv2/testv2_management/test_management_client.py::TestManagementClient::test_method_docstrings (pytest step).
🔇 Additional comments (6)
kinde_sdk/management/management_client.py (2)
31-35: LGTM: users.get endpoint path fixUsing
/api/v1/userresolves Bug #89 and aligns with the wrapper.
413-431: Resolved: Query parameteridis correct
The UsersApi documentation and examples forget_user_datause?id=<user_id>, so the wrapper’s use ofkwargs['id'] = user_idis accurate.kinde_sdk/auth/oauth.py (2)
749-765: API shape is clear and helpful.list_protected_routes exposure is a good DX addition and keeps the engine encapsulated.
775-798: Targeted route introspection looks good.get_route_info provides useful visibility for debugging and UI guards.
kinde_sdk/auth/route_middleware.py (2)
124-126: Drop the suggestion to removeawait.validate_route_accessis defined asasync definkinde_sdk/auth/oauth.py(lines 671–676), so awaiting it is correct and required.Likely an incorrect or invalid review comment.
282-298: Simplify the FastAPI middleware's request info extractionThe current approach of temporarily overriding methods is complex and error-prone. The middleware already has access to the correct method implementation.
async def dispatch(self, request: StarletteRequest, call_next): """FastAPI middleware dispatch method.""" - # Temporarily override the method for this request - original_get_request_info = self.protection_middleware._get_request_info - self.protection_middleware._get_request_info = lambda req: self._get_request_info_for_middleware(req) - - try: - # Check route protection - error_response = await self.protection_middleware.validate_request_access(request) - if error_response: - return error_response - - # Continue to next middleware/route handler - response = await call_next(request) - return response - - finally: - # Restore original method - self.protection_middleware._get_request_info = original_get_request_info + # Check route protection + error_response = await self.protection_middleware.validate_request_access(request) + if error_response: + return error_response + + # Continue to next middleware/route handler + response = await call_next(request) + return responseAnd ensure the concrete implementation properly handles the request as shown in the previous comment.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
kinde_sdk/auth/route_protection.py (1)
248-248: Use typing.Tuple to retain Python 3.8 compatibility.
tuple[...]is 3.9+. Switch toTuple[...]and import it.Apply:
-from typing import Dict, List, Any, Optional +from typing import Dict, List, Any, Optional, Tuple @@ - def _find_matching_route(self, path: str, method: str) -> tuple[Optional[str], Dict[str, Any]]: + def _find_matching_route(self, path: str, method: str) -> Tuple[Optional[str], Dict[str, Any]]:Verify declared Python support:
#!/bin/bash set -euo pipefail # Show files that may declare Python versions fd -a 'pyproject.toml' 'setup.cfg' 'setup.py' 2>/dev/null || true # Inspect python_requires and Trove classifiers rg -n -C1 -e 'python_requires' -e 'Programming Language :: Python :: ' -- pyproject.toml setup.cfg setup.py 2>/dev/null || trueAlso applies to: 52-52
🧹 Nitpick comments (4)
kinde_sdk/auth/route_protection.py (1)
259-271: Prefer most-specific match (exact over wildcard) to avoid surprising hits.Optional: first scan exact matches, then wildcard/regex, keeping current order per group.
If desired, I can provide a targeted diff to implement two-pass matching.
kinde_sdk/auth/oauth.py (2)
695-705: Docstring fix: behavior when not configured (no exception).The code returns allowed=True when not configured; it doesn’t raise. Update the docstring.
Apply:
- - Raises: - ValueError: If route protection is not configured + + Note: + If route protection is not configured, returns allowed=True with an explanatory reason.
737-755: Synchronous wrapper inside running event loop.You already detect a running loop and raise RuntimeError, but then catch-all swallows it and returns False. Consider letting the RuntimeError propagate or returning a deterministic True/False with a warning, to avoid silent denials in FastAPI contexts.
I can provide a small diff to re-raise only the specific RuntimeError while still logging others.
kinde_sdk/auth/route_middleware.py (1)
304-323: Simplify FastAPI dispatch; remove monkey-patch.No need to patch _get_request_info per request; the concrete middleware already routes correctly.
Apply:
- async def dispatch(self, request: StarletteRequest, call_next): - """FastAPI middleware dispatch method.""" - # Temporarily override the method for this request - original_get_request_info = self.protection_middleware._get_request_info - self.protection_middleware._get_request_info = lambda req: self._get_request_info_for_middleware(req) - - try: - # Check route protection - error_response = await self.protection_middleware.validate_request_access(request) - if error_response: - return error_response - - # Continue to next middleware/route handler - response = await call_next(request) - return response - - finally: - # Restore original method - self.protection_middleware._get_request_info = original_get_request_info + async def dispatch(self, request: StarletteRequest, call_next): + """FastAPI middleware dispatch method.""" + error_response = await self.protection_middleware.validate_request_access(request) + if error_response: + return error_response + return await call_next(request)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
kinde_sdk/auth/oauth.py(4 hunks)kinde_sdk/auth/route_middleware.py(1 hunks)kinde_sdk/auth/route_protection.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-03T09:38:46.175Z
Learnt from: brettchaldecott
PR: kinde-oss/kinde-python-sdk#83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
Applied to files:
kinde_sdk/auth/oauth.py
🧬 Code graph analysis (3)
kinde_sdk/auth/route_middleware.py (2)
kinde_sdk/auth/oauth.py (2)
is_route_protection_enabled(773-780)validate_route_access(671-718)kinde_sdk/auth/route_protection.py (1)
validate_route_access(142-246)
kinde_sdk/auth/oauth.py (7)
kinde_sdk/auth/route_protection.py (4)
RouteProtectionEngine(62-434)validate_route_access(142-246)list_protected_routes(413-434)get_route_info(388-411)kinde_sdk/auth/base_auth.py (2)
_get_token_manager(24-39)_get_framework(18-22)kinde_sdk/auth/token_manager.py (1)
get_force_api(45-47)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_fastapi/framework/fastapi_framework.py (1)
get_user_id(96-106)kinde_flask/framework/flask_framework.py (1)
get_user_id(103-113)kinde_sdk/auth/user_session.py (1)
get_token_manager(119-127)
kinde_sdk/auth/route_protection.py (5)
kinde_sdk/auth/config_loader.py (1)
load_config(6-26)kinde_sdk/auth/roles.py (2)
Roles(8-230)get_role(9-82)kinde_sdk/auth/permissions.py (2)
Permissions(8-143)get_permission(9-55)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_sdk/auth/oauth.py (1)
validate_route_access(671-718)
🪛 Ruff (0.12.2)
kinde_sdk/auth/route_middleware.py
204-204: Unused method argument: reason
(ARG002)
204-204: Unused method argument: kwargs
(ARG002)
235-235: Do not catch blind exception: Exception
(BLE001)
236-236: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
237-237: Use explicit conversion flag
Replace with conversion flag
(RUF010)
285-285: Unused method argument: reason
(ARG002)
285-285: Unused method argument: kwargs
(ARG002)
365-365: Avoid specifying long messages outside the exception class
(TRY003)
kinde_sdk/auth/oauth.py
112-112: Do not catch blind exception: Exception
(BLE001)
113-113: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
667-667: Avoid specifying long messages outside the exception class
(TRY003)
667-667: Use explicit conversion flag
Replace with conversion flag
(RUF010)
675-675: Unused method argument: request
(ARG002)
714-714: Do not catch blind exception: Exception
(BLE001)
742-745: Abstract raise to an inner function
(TRY301)
742-745: Avoid specifying long messages outside the exception class
(TRY003)
818-818: Do not catch blind exception: Exception
(BLE001)
825-825: Do not catch blind exception: Exception
(BLE001)
kinde_sdk/auth/route_protection.py
111-111: Abstract raise to an inner function
(TRY301)
111-111: Avoid specifying long messages outside the exception class
(TRY003)
134-136: Avoid specifying long messages outside the exception class
(TRY003)
140-140: Avoid specifying long messages outside the exception class
(TRY003)
340-343: Consider moving this statement to an else block
(TRY300)
376-379: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (1)
kinde_sdk/auth/route_protection.py (1)
201-238: Confirm OR semantics for roles/permissions.Current logic grants access if any required role OR any required permission matches. If “all-of” is expected, this needs adjustment or a config flag (e.g., match: "any" | "all").
I can add support for both with a non-breaking default.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_sdk/management/management_client.py (1)
580-603: Create actual backward-compatibility aliases in management_client.py
Replace thepassinside the BC loop (around line 582) with asetattrso old names point to the new methods:-for method_name, new_name in [...]: - pass # These methods will be created dynamically +for method_name, new_name in [...]: + setattr(ManagementClient, method_name, getattr(ManagementClient, new_name))This ensures calls to legacy method names resolve to their current implementations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
kinde_sdk/management/management_client.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
kinde_sdk/management/management_client.py (3)
kinde_sdk/management/api_client.py (1)
param_serialize(142-248)kinde_sdk/auth/token_manager.py (1)
get_access_token(122-137)kinde_sdk/management/management_token_manager.py (1)
get_access_token(223-231)
🔇 Additional comments (4)
kinde_sdk/management/management_client.py (4)
396-402: LGTM! Flexible endpoint definition handlingThe code now correctly handles both 2-tuple and 3-tuple endpoint definitions, allowing for optional query parameters. This is a clean and backward-compatible implementation.
447-457: Well-structured query/body parameter separationThe logic correctly separates query parameters from body parameters based on the HTTP method and the declared
query_keys. For GET/DELETE requests, all kwargs become query parameters, while for other methods, only keys inquery_keysare extracted as query parameters with the rest becoming the JSON body.
479-483: Good practice: Ensuring proper content headersThe code correctly sets Content-Type and Accept headers for non-GET/DELETE requests, ensuring proper JSON communication.
31-34: Confirmed API endpoint changes align with official specs.
The GET, PATCH, and DELETE operations against/api/v1/user?id={id}match the Kinde Management API documentation. No further changes required.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
docs/ROUTE_PROTECTION.md (1)
160-165: Clarify how to match base path vs. subpaths (align docs with engine semantics).You already note that
/admin/*does not match/adminor/adminXYZ. Add one explicit sentence showing how to match both base and subpaths to prevent misconfig.Apply:
| `/admin/*` | Wildcard match | Matches `/admin/users`, `/admin/settings/view`, etc. (does not match `/admin` or `/adminXYZ`) | | `/api/v1/*` | Prefix wildcard | Matches `/api/v1/users`, `/api/v1/organizations`, etc. | + +Note: To match both the base path and its subpaths, include two entries: one for the base (e.g., `/admin`) and one wildcard (e.g., `/admin/*`).kinde_sdk/auth/route_protection.py (1)
302-307: Wildcard matches base path (security/behavior mismatch with docs).
/prefix/*currently matches/prefixviarequest_path == pattern_prefix. Docs say it must not. This can unintentionally allow or deny the base path.Apply:
- if route_pattern.endswith('/*'): - pattern_prefix = route_pattern[:-2] # Remove /* - # Ensure we're matching a path segment boundary - return request_path == pattern_prefix or request_path.startswith(pattern_prefix + '/') + if route_pattern.endswith('/*'): + prefix = route_pattern[:-2] # Remove /* + # Match only subpaths with a segment boundary, not the base path itself + return request_path.startswith(prefix + '/')kinde_sdk/auth/route_middleware.py (2)
219-237: Don’t leak exception details; log traceback and return generic error.Use
logger.exceptionand avoid returning raw exception text to clients.Apply:
- return asyncio.run(self.validate_request_access(flask_request)) - except Exception as e: - self._logger.error(f"Error in Flask route protection: {e}") - return self.error_handler(f"Route protection error: {str(e)}") + return asyncio.run(self.validate_request_access(flask_request)) + except Exception: + self._logger.exception("Error in Flask route protection") + return self.error_handler("Route protection error")
174-177: Root path skip-patterns never match (‘/’ normalized to empty).Unconditionally rstripping slashes turns “/” into “”, breaking explicit root skip rules.
Apply:
- # Normalize paths - path = path.rstrip('/') - pattern = pattern.rstrip('/') + # Normalize paths but preserve root '/' + if path != '/': + path = path.rstrip('/') + if pattern != '/': + pattern = pattern.rstrip('/')
🧹 Nitpick comments (5)
docs/ROUTE_PROTECTION.md (1)
166-173: Document default fallback when no rules match or no config is loaded.The engine defaults to using
settings.default_allow(true if unset). Make this explicit in “Access Control Logic” to avoid accidental open configs in production.Apply:
- **No Rules**: Falls back to `default_allow` setting + - **No Rules / No Match**: Falls back to `settings.default_allow` (defaults to `true` if not set). For production, set `default_allow: false`.kinde_sdk/auth/route_protection.py (2)
117-123: Add light config validation to prevent silent misconfigurations.Validate field types and values (methods uppercase strings, roles/permissions lists of strings, public bool). Fail fast on invalid types.
Example:
for route_name, route_config in routes_config.items(): + if not isinstance(route_config, dict): + raise ValueError(f"Route '{route_name}' must be a mapping") if "path" not in route_config: raise ValueError(f"Route '{route_name}' missing required 'path' field") # Set defaults route_config.setdefault("methods", ["GET"]) + # Normalize and validate + route_config["methods"] = [str(m).upper() for m in route_config["methods"]] + for key in ("roles", "permissions"): + if key in route_config and route_config[key] is not None: + if not isinstance(route_config[key], list) or not all(isinstance(x, str) for x in route_config[key]): + raise ValueError(f"Route '{route_name}': '{key}' must be a list of strings") + if "public" in route_config and not isinstance(route_config["public"], bool): + raise ValueError(f"Route '{route_name}': 'public' must be boolean")Also applies to: 130-133
321-356: Avoid sequential API calls for multiple roles/permissions (latency).When many roles/permissions are listed, you call the API per item. Consider fetching all once (get_roles/get_permissions) or parallelize with
asyncio.gather.Apply (roles example):
- for role_key in required_roles: - role_result = await self._roles_client.get_role(role_key, options) - if role_result and role_result.get("isGranted", False): - return { - "allowed": True, - "reason": f"User has required role: {role_key}" - } + # Fast path: fetch all roles once if more than one required + if len(required_roles) > 1 and hasattr(self._roles_client, "get_roles"): + data = await self._roles_client.get_roles(options) + granted = {r.get("key") for r in data.get("roles", [])} + for role_key in required_roles: + if role_key in granted: + return {"allowed": True, "reason": f"User has required role: {role_key}"} + else: + for role_key in required_roles: + role_result = await self._roles_client.get_role(role_key, options) + if role_result and role_result.get("isGranted", False): + return {"allowed": True, "reason": f"User has required role: {role_key}"}Do similarly for permissions using
get_permissions.kinde_sdk/auth/route_middleware.py (2)
304-323: Remove per-request monkey patching; use the Concrete middleware’s implementation.
ConcreteProtectionMiddleware._get_request_infoalready forwards correctly; the monkey patch adds brittleness without benefit.Apply:
- # Temporarily override the method for this request - original_get_request_info = self.protection_middleware._get_request_info - self.protection_middleware._get_request_info = lambda req: self._get_request_info_for_middleware(req) - - try: - # Check route protection - error_response = await self.protection_middleware.validate_request_access(request) - if error_response: - return error_response - - # Continue to next middleware/route handler - response = await call_next(request) - return response - - finally: - # Restore original method - self.protection_middleware._get_request_info = original_get_request_info + # Check route protection + error_response = await self.protection_middleware.validate_request_access(request) + if error_response: + return error_response + # Continue to next middleware/route handler + return await call_next(request)Then remove the unused
_get_request_info_for_middlewaremethod.
363-372: Factory returns a dict for FastAPI; either add the middleware or return the class consistently.Requiring
appbut not callingapp.add_middlewareis surprising. Consider adding it directly and returning the instance, or drop theapprequirement and return the class plus kwargs.Apply (add directly):
- elif framework == "fastapi" and FastAPIRouteProtectionMiddleware: - if not app: - raise ValueError("FastAPI middleware requires 'app' parameter") - # Return class and kwargs for FastAPI's app.add_middleware() - return { - "middleware_class": FastAPIRouteProtectionMiddleware, - "oauth_client": oauth_client, - **kwargs - } + elif framework == "fastapi" and FastAPIRouteProtectionMiddleware: + if not app: + raise ValueError("FastAPI middleware requires 'app' parameter") + app.add_middleware(FastAPIRouteProtectionMiddleware, oauth_client=oauth_client, **kwargs) + return FastAPIRouteProtectionMiddlewareUpdate docs if you keep the dict-return variant.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/ROUTE_PROTECTION.md(1 hunks)kinde_sdk/auth/route_middleware.py(1 hunks)kinde_sdk/auth/route_protection.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
kinde_sdk/auth/route_middleware.py (1)
kinde_sdk/auth/oauth.py (2)
is_route_protection_enabled(773-780)validate_route_access(671-718)
kinde_sdk/auth/route_protection.py (5)
kinde_sdk/auth/config_loader.py (1)
load_config(6-26)kinde_sdk/auth/roles.py (2)
Roles(8-230)get_role(9-82)kinde_sdk/auth/permissions.py (2)
Permissions(8-143)get_permission(9-55)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_sdk/auth/oauth.py (2)
validate_route_access(671-718)get_route_info(782-804)
🪛 Ruff (0.12.2)
kinde_sdk/auth/route_middleware.py
204-204: Unused method argument: reason
(ARG002)
204-204: Unused method argument: kwargs
(ARG002)
235-235: Do not catch blind exception: Exception
(BLE001)
236-236: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
237-237: Use explicit conversion flag
Replace with conversion flag
(RUF010)
285-285: Unused method argument: reason
(ARG002)
285-285: Unused method argument: kwargs
(ARG002)
365-365: Avoid specifying long messages outside the exception class
(TRY003)
kinde_sdk/auth/route_protection.py
114-114: Abstract raise to an inner function
(TRY301)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
137-139: Avoid specifying long messages outside the exception class
(TRY003)
143-143: Avoid specifying long messages outside the exception class
(TRY003)
345-348: Consider moving this statement to an else block
(TRY300)
381-384: Consider moving this statement to an else block
(TRY300)
🪛 LanguageTool
docs/ROUTE_PROTECTION.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...etup. ## Table of Contents - Overview - Quick Start - [Configurat...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ... - Overview - Quick Start - Configuration - [Framew...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ck Start](#quick-start) - Configuration - [Framework Integration](#framework-integr...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...#configuration) - Framework Integration - API Reference - [Exampl...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...#framework-integration) - API Reference - Examples - [Best Practices](...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...I Reference](#api-reference) - Examples - Best Practices - [Trou...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... Examples - Best Practices - Troubleshooting ## O...
(QB_NEW_EN)
[grammar] ~160-~160: There might be a mistake here.
Context: ...s: | Pattern | Description | Examples | |---------|-------------|----------| | `...
(QB_NEW_EN)
[grammar] ~161-~161: There might be a mistake here.
Context: ...s | |---------|-------------|----------| | /admin | Exact match | Matches only ...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ... | Exact match | Matches only /admin | | /admin/* | Wildcard match | Matches ...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...oes not match /admin or /adminXYZ) | | /api/v1/* | Prefix wildcard | Matche...
(QB_NEW_EN)
[grammar] ~354-~354: There might be a mistake here.
Context: ..... other parameters ) ``` Parameters: - route_protection_file: Path to YAML/JSON configuration file ...
(QB_NEW_EN)
[grammar] ~690-~690: There might be a mistake here.
Context: ...oute Protection Not Working Symptoms: - Routes that should be protected are acce...
(QB_NEW_EN)
[grammar] ~718-~718: There might be a mistake here.
Context: ... 403 Forbidden Unexpectedly Symptoms: - Users with correct roles still getting d...
(QB_NEW_EN)
[grammar] ~741-~741: There might be a mistake here.
Context: ...onfiguration File Not Found Symptoms: - Error messages about missing configurati...
(QB_NEW_EN)
[grammar] ~742-~742: There might be a mistake here.
Context: ...essages about missing configuration file - Route protection not initializing **Sol...
(QB_NEW_EN)
[grammar] ~770-~770: There might be a mistake here.
Context: ...permissions being returned Solutions: 1. Check Kinde Admin Panel: - Applicat...
(QB_NEW_EN)
[grammar] ~771-~771: There might be a mistake here.
Context: ...lutions:** 1. Check Kinde Admin Panel: - Applications → Your App → Token customiz...
(QB_NEW_EN)
[grammar] ~772-~772: There might be a mistake here.
Context: ...cations → Your App → Token customization - Ensure "Include roles in tokens" is enab...
(QB_NEW_EN)
[grammar] ~802-~802: There might be a mistake here.
Context: ... 5. Pattern Matching Issues Symptoms: - Route patterns not matching expected pat...
(QB_NEW_EN)
[grammar] ~803-~803: There might be a mistake here.
Context: ...ute patterns not matching expected paths - Wildcard patterns not working correctly ...
(QB_NEW_EN)
[grammar] ~895-~895: There might be a mistake here.
Context: ...e documentation or contact support with: - Your configuration file - Debug logs sho...
(QB_NEW_EN)
[grammar] ~896-~896: There might be a mistake here.
Context: ... support with: - Your configuration file - Debug logs showing the issue - Steps to ...
(QB_NEW_EN)
[grammar] ~897-~897: There might be a mistake here.
Context: ...tion file - Debug logs showing the issue - Steps to reproduce the problem - Expecte...
(QB_NEW_EN)
[grammar] ~898-~898: There might be a mistake here.
Context: ...e issue - Steps to reproduce the problem - Expected vs actual behavior
(QB_NEW_EN)
[grammar] ~899-~899: There might be a mistake here.
Context: ...he problem - Expected vs actual behavior
(QB_NEW_EN)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
kinde_sdk/auth/route_middleware.py (1)
184-189: Do not let wildcard patterns match the base path (security/consistency).According to the docstring, "/public/*" should match children like "/public/info" but not "/public" itself. Current equality check allows skipping protection on the base path too.
Apply:
- # Wildcard match - if pattern.endswith('/*'): - pattern_prefix = pattern[:-2] # Remove /* - # Ensure we're matching a path segment boundary - return path == pattern_prefix or path.startswith(pattern_prefix + '/') + # Wildcard match + if pattern.endswith('/*'): + pattern_prefix = pattern[:-2] # Remove /* + # Match only child paths (do not match the base path) + return path.startswith(pattern_prefix + '/')To confirm intended semantics in docs:
#!/bin/bash rg -n -C2 '/public/\*' docs/ROUTE_PROTECTION.md || true rg -n -C2 'wildcard' docs/ROUTE_PROTECTION.md || true
🧹 Nitpick comments (5)
kinde_sdk/auth/route_middleware.py (5)
134-141: Avoid returning raw exception text to clients; keep details in logs.Prevent leakage of internal errors via error_handler message; logger.exception already preserves traceback.
- except (AttributeError, KeyError, TypeError) as e: + except (AttributeError, KeyError, TypeError) as e: self._logger.exception("Error during route protection validation") # Fail closed - deny access if validation fails - return self.error_handler(f"Route protection validation failed: {e!s}", path=path, method=method) - except Exception as e: + return self.error_handler("Route protection validation failed", path=path, method=method) - except Exception as e: + except Exception: self._logger.exception("Unexpected error during route protection validation") - return self.error_handler(f"Route protection validation failed: {e!s}", path=path, method=method) + return self.error_handler("Route protection validation failed", path=path, method=method)
206-209: Silence ARG002: mark unused parameters in default error handlers.These handlers ignore reason/kwargs by design; prefix with underscores to placate linters without changing behavior.
- def _default_error_handler(self, reason: str, **kwargs) -> Any: + def _default_error_handler(self, _reason: str, **_kwargs) -> Any: """Default Flask error handler returns JSON response.""" return flask_jsonify({"error": "Access Denied"}), 403- def _default_error_handler(self, reason: str, **kwargs) -> JSONResponse: + def _default_error_handler(self, _reason: str, **_kwargs) -> JSONResponse: """Default FastAPI error handler.""" return JSONResponse( status_code=403, content={"error": "Access Denied"} )Also applies to: 287-293
298-325: Remove per-request monkey patching; use the bound implementation directly.ConcreteProtectionMiddleware already defers _get_request_info to the outer class, so the patch is redundant and adds risk.
- # Override the RouteProtectionMiddleware method to work with FastAPI request objects - def _get_request_info_for_middleware(self, request: StarletteRequest) -> Tuple[str, str]: - return str(request.url.path), request.method - - # Monkey patch the method for this instance - def __init_subclass__(cls, **kwargs): - super().__init_subclass__(**kwargs) - async def dispatch(self, request: StarletteRequest, call_next): """FastAPI middleware dispatch method.""" - # Temporarily override the method for this request - original_get_request_info = self.protection_middleware._get_request_info - self.protection_middleware._get_request_info = lambda req: self._get_request_info_for_middleware(req) - - try: - # Check route protection - error_response = await self.protection_middleware.validate_request_access(request) - if error_response: - return error_response - - # Continue to next middleware/route handler - response = await call_next(request) - return response - - finally: - # Restore original method - self.protection_middleware._get_request_info = original_get_request_info + # Check route protection + error_response = await self.protection_middleware.validate_request_access(request) + if error_response: + return error_response + # Continue to next middleware/route handler + return await call_next(request)
129-133: Enrich denial log with matched rule; avoid f-strings in logs.More actionable logs without extra verbosity.
- if not result.get("allowed", False): - reason = result.get("reason", "Access denied") - self._logger.info(f"Access denied for {method} {path}: {reason}") + if not result.get("allowed", False): + reason = result.get("reason", "Access denied") + matched_rule = result.get("matched_rule") + self._logger.info( + "Access denied for %s %s: %s%s", + method, + path, + reason, + f" [rule={matched_rule}]" if matched_rule else "" + ) return self.error_handler(reason, path=path, method=method, result=result)
365-374: Factory: use the provided FastAPI app or drop the requirement.The function requires app but doesn’t use it; either install the middleware or return the config without enforcing app. Suggest supporting both for convenience.
- elif framework == "fastapi" and FastAPIRouteProtectionMiddleware: - if not app: - raise ValueError("FastAPI middleware requires 'app' parameter") - # Return class and kwargs for FastAPI's app.add_middleware() - return { - "middleware_class": FastAPIRouteProtectionMiddleware, - "oauth_client": oauth_client, - **kwargs - } + elif framework == "fastapi" and FastAPIRouteProtectionMiddleware: + if app is None: + logger.warning("FastAPI app not provided; returning middleware class and kwargs") + return { + "middleware_class": FastAPIRouteProtectionMiddleware, + "oauth_client": oauth_client, + **kwargs + } + # Install middleware immediately for convenience + app.add_middleware(FastAPIRouteProtectionMiddleware, oauth_client=oauth_client, **kwargs) + return None
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
kinde_sdk/auth/route_middleware.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
kinde_sdk/auth/route_middleware.py (2)
kinde_sdk/auth/oauth.py (2)
is_route_protection_enabled(773-780)validate_route_access(671-718)kinde_sdk/auth/route_protection.py (1)
validate_route_access(145-249)
🪛 Ruff (0.12.2)
kinde_sdk/auth/route_middleware.py
206-206: Unused method argument: reason
(ARG002)
206-206: Unused method argument: kwargs
(ARG002)
287-287: Unused method argument: reason
(ARG002)
287-287: Unused method argument: kwargs
(ARG002)
367-367: Avoid specifying long messages outside the exception class
(TRY003)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
kinde_sdk/auth/route_protection.py (1)
140-143: Narrow exception handling to preserve root cause.Catching all Exceptions hides non-config errors; catch ValueError from loader/validation.
- except Exception as e: - # Parsing or validation error from load_config - self._logger.exception("Failed to load route protection config") - raise ValueError("Failed to load route protection configuration") from e + except ValueError as e: + # Parsing or validation error from load_config + self._logger.exception("Failed to load route protection config") + raise ValueError("Failed to load route protection configuration") from e
🧹 Nitpick comments (5)
kinde_sdk/auth/route_protection.py (1)
50-50: Remove unused imports/variables.
reand module-levelloggerare unused.-import re import logging @@ -logger = logging.getLogger(__name__)Also applies to: 59-59
docs/ROUTE_PROTECTION.md (1)
872-885: Fix top-level await in example.Wrap in an async main or use asyncio.run to avoid SyntaxError in sync contexts.
import asyncio async def main(): for path, method in test_routes: result = await oauth.validate_route_access(path, method) status = "✅ ALLOWED" if result["allowed"] else "❌ DENIED" print(f"{status} {method} {path} - {result['reason']}") asyncio.run(main())kinde_sdk/auth/route_middleware.py (3)
306-325: Remove per-request monkey patching; use the concrete middleware directly.Monkey patching risks concurrency issues and is unnecessary here.
- async def dispatch(self, request: StarletteRequest, call_next): - """FastAPI middleware dispatch method.""" - # Temporarily override the method for this request - original_get_request_info = self.protection_middleware._get_request_info - self.protection_middleware._get_request_info = lambda req: self._get_request_info_for_middleware(req) - - try: - # Check route protection - error_response = await self.protection_middleware.validate_request_access(request) - if error_response: - return error_response - - # Continue to next middleware/route handler - response = await call_next(request) - return response - - finally: - # Restore original method - self.protection_middleware._get_request_info = original_get_request_info + async def dispatch(self, request: StarletteRequest, call_next): + """FastAPI middleware dispatch method.""" + error_response = await self.protection_middleware.validate_request_access(request) + if error_response: + return error_response + return await call_next(request)
47-47: Drop unused import.
Unionisn’t used.-from typing import Optional, Any, Dict, Callable, List, Union, Tuple +from typing import Optional, Any, Dict, Callable, List, Tuple
206-209: Silence unused-arg warnings in default error handlers.Rename to underscore-prefixed params.
- def _default_error_handler(self, reason: str, **kwargs) -> Any: + def _default_error_handler(self, _reason: str, **_kwargs) -> Any: @@ - def _default_error_handler(self, reason: str, **kwargs) -> JSONResponse: + def _default_error_handler(self, _reason: str, **_kwargs) -> JSONResponse:Also applies to: 287-292
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/ROUTE_PROTECTION.md(1 hunks)kinde_sdk/auth/route_middleware.py(1 hunks)kinde_sdk/auth/route_protection.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
kinde_sdk/auth/route_protection.py (5)
kinde_sdk/auth/config_loader.py (1)
load_config(6-26)kinde_sdk/auth/roles.py (2)
Roles(8-230)get_role(9-82)kinde_sdk/auth/permissions.py (2)
Permissions(8-143)get_permission(9-55)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_sdk/auth/oauth.py (2)
validate_route_access(671-718)get_route_info(782-804)
kinde_sdk/auth/route_middleware.py (2)
kinde_sdk/auth/oauth.py (2)
is_route_protection_enabled(773-780)validate_route_access(671-718)kinde_sdk/auth/route_protection.py (1)
validate_route_access(145-249)
🪛 Ruff (0.12.2)
kinde_sdk/auth/route_protection.py
114-114: Abstract raise to an inner function
(TRY301)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
137-139: Avoid specifying long messages outside the exception class
(TRY003)
143-143: Avoid specifying long messages outside the exception class
(TRY003)
336-339: Consider moving this statement to an else block
(TRY300)
372-375: Consider moving this statement to an else block
(TRY300)
kinde_sdk/auth/route_middleware.py
206-206: Unused method argument: reason
(ARG002)
206-206: Unused method argument: kwargs
(ARG002)
287-287: Unused method argument: reason
(ARG002)
287-287: Unused method argument: kwargs
(ARG002)
367-367: Avoid specifying long messages outside the exception class
(TRY003)
🪛 LanguageTool
docs/ROUTE_PROTECTION.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...etup. ## Table of Contents - Overview - Quick Start - [Configurat...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ... - Overview - Quick Start - Configuration - [Framew...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...ck Start](#quick-start) - Configuration - [Framework Integration](#framework-integr...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...#configuration) - Framework Integration - API Reference - [Exampl...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...#framework-integration) - API Reference - Examples - [Best Practices](...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...I Reference](#api-reference) - Examples - Best Practices - [Trou...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ... Examples - Best Practices - Troubleshooting ## O...
(QB_NEW_EN)
[grammar] ~160-~160: There might be a mistake here.
Context: ...s: | Pattern | Description | Examples | |---------|-------------|----------| | `...
(QB_NEW_EN)
[grammar] ~161-~161: There might be a mistake here.
Context: ...s | |---------|-------------|----------| | /admin | Exact match | Matches only ...
(QB_NEW_EN)
[grammar] ~162-~162: There might be a mistake here.
Context: ... | Exact match | Matches only /admin | | /admin/* | Wildcard match | Matches ...
(QB_NEW_EN)
[grammar] ~163-~163: There might be a mistake here.
Context: ...oes not match /admin or /adminXYZ) | | /api/v1/* | Prefix wildcard | Matche...
(QB_NEW_EN)
[grammar] ~354-~354: There might be a mistake here.
Context: ..... other parameters ) ``` Parameters: - route_protection_file: Path to YAML/JSON configuration file ...
(QB_NEW_EN)
[grammar] ~692-~692: There might be a mistake here.
Context: ...oute Protection Not Working Symptoms: - Routes that should be protected are acce...
(QB_NEW_EN)
[grammar] ~720-~720: There might be a mistake here.
Context: ... 403 Forbidden Unexpectedly Symptoms: - Users with correct roles still getting d...
(QB_NEW_EN)
[grammar] ~743-~743: There might be a mistake here.
Context: ...onfiguration File Not Found Symptoms: - Error messages about missing configurati...
(QB_NEW_EN)
[grammar] ~744-~744: There might be a mistake here.
Context: ...essages about missing configuration file - Route protection not initializing **Sol...
(QB_NEW_EN)
[grammar] ~768-~768: There might be a mistake here.
Context: ...missions Not Found in Token Symptoms: - Users have roles in Kinde admin but rout...
(QB_NEW_EN)
[grammar] ~772-~772: There might be a mistake here.
Context: ...permissions being returned Solutions: 1. Check Kinde Admin Panel: - Applicat...
(QB_NEW_EN)
[grammar] ~773-~773: There might be a mistake here.
Context: ...lutions:** 1. Check Kinde Admin Panel: - Applications → Your App → Token customiz...
(QB_NEW_EN)
[grammar] ~774-~774: There might be a mistake here.
Context: ...cations → Your App → Token customization - Ensure "Include roles in tokens" is enab...
(QB_NEW_EN)
[grammar] ~804-~804: There might be a mistake here.
Context: ... 5. Pattern Matching Issues Symptoms: - Route patterns not matching expected pat...
(QB_NEW_EN)
[grammar] ~805-~805: There might be a mistake here.
Context: ...ute patterns not matching expected paths - Wildcard patterns not working correctly ...
(QB_NEW_EN)
[grammar] ~897-~897: There might be a mistake here.
Context: ...e documentation or contact support with: - Your configuration file - Debug logs sho...
(QB_NEW_EN)
[grammar] ~898-~898: There might be a mistake here.
Context: ... support with: - Your configuration file - Debug logs showing the issue - Steps to ...
(QB_NEW_EN)
[grammar] ~899-~899: There might be a mistake here.
Context: ...tion file - Debug logs showing the issue - Steps to reproduce the problem - Expecte...
(QB_NEW_EN)
[grammar] ~900-~900: There might be a mistake here.
Context: ...e issue - Steps to reproduce the problem - Expected vs actual behavior
(QB_NEW_EN)
[grammar] ~901-~901: There might be a mistake here.
Context: ...he problem - Expected vs actual behavior
(QB_NEW_EN)
🔇 Additional comments (3)
docs/ROUTE_PROTECTION.md (3)
160-165: Confirm engine matches documented wildcard semantics.Docs say
/admin/*does not match/admin. Engine currently does (see comment in route_protection.py). Align behavior and add a quick test example if possible.
772-787: No changes needed: OAuth constructor supportsforce_apiparameter
ConfirmedOAuth.__init__signature includesforce_api: bool = False, so the docs are accurate.
392-415: No action needed—OAuth APIs are correctly exported.Both
check_route_access(line 720) andget_route_protection_info(line 756) are implemented onOAuthin kinde_sdk/auth/oauth.py, andOAuthis included in__all__in kinde_sdk/auth/init.py, making these methods available when importing viafrom kinde_sdk.auth import OAuth.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kinde_sdk/auth/oauth.py(4 hunks)kinde_sdk/auth/route_protection.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: brettchaldecott
Repo: kinde-oss/kinde-python-sdk PR: 83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
📚 Learning: 2025-07-03T09:38:46.175Z
Learnt from: brettchaldecott
Repo: kinde-oss/kinde-python-sdk PR: 83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
Applied to files:
kinde_sdk/auth/oauth.py
🧬 Code graph analysis (2)
kinde_sdk/auth/oauth.py (5)
kinde_sdk/auth/route_protection.py (4)
RouteProtectionEngine(62-447)validate_route_access(161-265)list_protected_routes(426-447)get_route_info(401-424)kinde_sdk/auth/base_auth.py (2)
_get_token_manager(24-39)_get_framework(18-22)kinde_sdk/auth/token_manager.py (1)
get_force_api(45-47)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_sdk/auth/user_session.py (1)
get_token_manager(119-127)
kinde_sdk/auth/route_protection.py (5)
kinde_sdk/auth/config_loader.py (1)
load_config(6-26)kinde_sdk/auth/roles.py (2)
Roles(8-230)get_role(9-82)kinde_sdk/auth/permissions.py (2)
Permissions(8-143)get_permission(9-55)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_sdk/auth/oauth.py (2)
validate_route_access(671-718)get_route_info(782-804)
🪛 Ruff (0.14.3)
kinde_sdk/auth/oauth.py
667-667: Avoid specifying long messages outside the exception class
(TRY003)
667-667: Use explicit conversion flag
Replace with conversion flag
(RUF010)
675-675: Unused method argument: request
(ARG002)
714-714: Do not catch blind exception: Exception
(BLE001)
742-745: Abstract raise to an inner function
(TRY301)
742-745: Avoid specifying long messages outside the exception class
(TRY003)
818-818: Do not catch blind exception: Exception
(BLE001)
825-825: Do not catch blind exception: Exception
(BLE001)
kinde_sdk/auth/route_protection.py
112-112: Prefer TypeError exception for invalid type
(TRY004)
112-112: Abstract raise to an inner function
(TRY301)
112-112: Avoid specifying long messages outside the exception class
(TRY003)
118-118: Abstract raise to an inner function
(TRY301)
118-118: Avoid specifying long messages outside the exception class
(TRY003)
120-120: Prefer TypeError exception for invalid type
(TRY004)
120-120: Abstract raise to an inner function
(TRY301)
120-120: Avoid specifying long messages outside the exception class
(TRY003)
125-125: Abstract raise to an inner function
(TRY301)
125-125: Avoid specifying long messages outside the exception class
(TRY003)
153-155: Avoid specifying long messages outside the exception class
(TRY003)
159-159: Avoid specifying long messages outside the exception class
(TRY003)
353-356: Consider moving this statement to an else block
(TRY300)
389-392: Consider moving this statement to an else block
(TRY300)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
kinde_sdk/auth/oauth.py (1)
667-667: Optional: Simplify f-string conversion.The
str(e)is redundant since f-strings automatically convert to string.Apply this diff if desired:
- raise ValueError(f"Failed to retrieve tokens: {str(e)}") from e + raise ValueError(f"Failed to retrieve tokens: {e}") from e
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kinde_sdk/auth/oauth.py(4 hunks)kinde_sdk/auth/route_protection.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: brettchaldecott
Repo: kinde-oss/kinde-python-sdk PR: 83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
📚 Learning: 2025-07-03T09:38:46.175Z
Learnt from: brettchaldecott
Repo: kinde-oss/kinde-python-sdk PR: 83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
Applied to files:
kinde_sdk/auth/oauth.py
🧬 Code graph analysis (2)
kinde_sdk/auth/oauth.py (3)
kinde_sdk/auth/route_protection.py (4)
RouteProtectionEngine(62-452)validate_route_access(166-270)list_protected_routes(431-452)get_route_info(406-429)kinde_sdk/auth/token_manager.py (1)
get_force_api(45-47)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)
kinde_sdk/auth/route_protection.py (5)
kinde_sdk/auth/config_loader.py (1)
load_config(6-26)kinde_sdk/auth/roles.py (2)
Roles(8-230)get_role(9-82)kinde_sdk/auth/permissions.py (2)
Permissions(8-143)get_permission(9-55)kinde_sdk/auth/api_options.py (1)
ApiOptions(1-3)kinde_sdk/auth/oauth.py (2)
validate_route_access(671-718)get_route_info(784-806)
🪛 Ruff (0.14.3)
kinde_sdk/auth/oauth.py
667-667: Avoid specifying long messages outside the exception class
(TRY003)
667-667: Use explicit conversion flag
Replace with conversion flag
(RUF010)
675-675: Unused method argument: request
(ARG002)
714-714: Do not catch blind exception: Exception
(BLE001)
744-747: Abstract raise to an inner function
(TRY301)
744-747: Avoid specifying long messages outside the exception class
(TRY003)
820-820: Do not catch blind exception: Exception
(BLE001)
827-827: Do not catch blind exception: Exception
(BLE001)
kinde_sdk/auth/route_protection.py
109-109: Prefer TypeError exception for invalid type
(TRY004)
109-109: Abstract raise to an inner function
(TRY301)
109-109: Avoid specifying long messages outside the exception class
(TRY003)
117-117: Prefer TypeError exception for invalid type
(TRY004)
117-117: Abstract raise to an inner function
(TRY301)
117-117: Avoid specifying long messages outside the exception class
(TRY003)
123-123: Abstract raise to an inner function
(TRY301)
123-123: Avoid specifying long messages outside the exception class
(TRY003)
125-125: Prefer TypeError exception for invalid type
(TRY004)
125-125: Abstract raise to an inner function
(TRY301)
125-125: Avoid specifying long messages outside the exception class
(TRY003)
130-130: Abstract raise to an inner function
(TRY301)
130-130: Avoid specifying long messages outside the exception class
(TRY003)
158-160: Avoid specifying long messages outside the exception class
(TRY003)
164-164: Avoid specifying long messages outside the exception class
(TRY003)
358-361: Consider moving this statement to an else block
(TRY300)
394-397: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (10)
kinde_sdk/auth/route_protection.py (5)
87-165: LGTM—comprehensive config validation with proper error handling.The config loading logic correctly addresses all past review concerns: guards against None/empty config, validates settings and routes as mappings, normalizes singleton strings to lists, and chains exceptions properly with full stack traces.
298-332: LGTM—path matching is secure and well-implemented.The wildcard semantics are correct:
"/admin/*"matches"/admin/users"but not"/admin"or"/adminX", ensuring proper path boundary enforcement. Root path normalization preserves"/"to allow exact-root matching.
166-270: LGTM—validation logic is comprehensive and well-structured.Access control flow is sound: handles unconfigured routes with default_allow, supports public routes, enforces role/permission requirements with clear reasons, and gracefully degrades on errors.
334-404: LGTM—role and permission checks are secure and defensive.Both methods correctly fail closed on errors, log exceptions with full context, and support the "any of" semantics (first granted role/permission grants access).
406-452: LGTM—utility methods provide clean introspection API.These methods enable runtime inspection of route protection configuration, useful for debugging and documentation purposes.
kinde_sdk/auth/oauth.py (5)
24-30: LGTM—graceful integration with proper backward compatibility.The optional import pattern with the
ROUTE_PROTECTION_AVAILABLEflag ensures the SDK remains functional even if route protection dependencies are missing. Initialization doesn't raise exceptions, allowing users to checkis_route_protection_enabled()to verify setup.Also applies to: 45-45, 106-118
671-718: LGTM—validates route access with proper force_api propagation.The method correctly detects the force_api setting from the token manager and passes it to the route protection engine via ApiOptions. Defensive error handling ensures route checks proceed even if force_api detection fails.
720-757: LGTM—synchronous wrapper with correct event loop guard.The event loop detection is now properly implemented: it catches only the RuntimeError from
get_running_loop(), then raises a descriptive error if a loop is running, preventing theasyncio.run()crash in async contexts like FastAPI.
758-806: LGTM—utility methods provide clean public API.These methods expose route protection functionality through the OAuth class with proper null checks, maintaining a consistent interface regardless of whether route protection is configured.
808-828: LGTM—internal helpers are appropriately defensive.These private methods safely navigate the framework/session hierarchy to retrieve the token manager, returning None on any failure. The broad exception handling is intentional—route protection checks should degrade gracefully rather than crash.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
kinde_sdk/auth/__init__.py (1)
37-40: Consider using iterable unpacking for cleaner syntax.The list concatenation works but iterable unpacking is more Pythonic and efficient.
Apply this diff:
__all__ = [ "OAuth", "TokenManager", "UserSession", "permissions", "ApiOptions", - "claims", "async_claims", "feature_flags", "portals", "tokens", "roles" -] + _route_protection_components + "claims", "async_claims", "feature_flags", "portals", "tokens", "roles", + *_route_protection_components +]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kinde_sdk/auth/__init__.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: brettchaldecott
Repo: kinde-oss/kinde-python-sdk PR: 83
File: kinde_flask/examples/example_app.py:25-44
Timestamp: 2025-07-03T09:38:46.175Z
Learning: The kinde_flask framework automatically creates and registers standard OAuth routes (/login, /logout, /callback, /register, /user) at runtime when the OAuth instance is initialized with a Flask app. These routes do not need to be manually defined in application code.
🧬 Code graph analysis (1)
kinde_sdk/auth/__init__.py (2)
kinde_sdk/auth/route_protection.py (1)
RouteProtectionEngine(62-452)kinde_sdk/auth/route_middleware.py (4)
RouteProtectionMiddleware(53-190)FlaskRouteProtectionMiddleware(197-239)FastAPIRouteProtectionMiddleware(252-324)create_route_protection_middleware(332-377)
🪛 Ruff (0.14.3)
kinde_sdk/auth/__init__.py
37-40: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
🔇 Additional comments (2)
kinde_sdk/auth/__init__.py (2)
7-7: LGTM: async_claims import is properly added.The import is clean and correctly included in the public API exports.
28-31: The conditional checks are necessary and serve a valid purpose.The verification confirms that in
route_middleware.py, bothFlaskRouteProtectionMiddlewareandFastAPIRouteProtectionMiddlewareare conditionally set toNonewhen their respective framework dependencies are unavailable (lines 243 and 328). This means they can legitimately beNoneafter import, making the truthiness checks in lines 28-31 necessary to avoid appendingNoneto the_route_protection_componentslist.The original review comment was based on an incorrect assumption about how these variables are defined.
Likely an incorrect or invalid review comment.
1 participant
Route Protection System - Adds declarative, configuration-driven route protection based on user roles and permissions.
Core Engine (kinde_sdk/auth/route_protection.py) - Loads YAML/JSON config files, validates user access
Framework Middleware (kinde_sdk/auth/route_middleware.py) - Flask/FastAPI middleware for automatic protection
OAuth Integration - Added route_protection_file parameter and validation methods to OAuth class
Files Added
Files Modified