feat: enhance null framework for standalone OAuth usage #122
Conversation
- Refactor OAuth initialization to use null framework for standalone usage - Transform NullFramework into singleton pattern with session management - Add thread-safe session handling with context variables support - Update auth module exports for better framework integration - Add comprehensive HTTP OAuth server example - Include comprehensive test suite for null framework functionality This enables standalone OAuth usage without requiring a web framework, making the SDK more flexible for various application architectures.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Remove outdated comments about framework checks that are no longer needed after implementing null framework support for standalone usage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Can we rename NullFramework to something along the lines of |
- Add KindeSessionManagement class providing user-friendly session management - Wrap NullFramework functionality behind intuitive API for standalone usage - Include methods for user ID management, login status checks, and session info - Add comprehensive test suites for session management functionality - Update module exports to include new session management class - Enhance HTTP OAuth server example with improved session handling This provides a clean interface for managing user sessions in standalone applications without requiring web framework integration.
|
Warning Rate limit exceeded@brettchaldecott has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a standalone HTTP OAuth example, a thread-safe NullFramework with context-local user_id, a KindeSessionManagement API for standalone session handling, OAuth null-framework initialization, package export updates, and extensive tests for session and framework behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant S as HTTPServer
participant H as OAuthHTTPRequestHandler
participant M as SimpleOAuthManager
participant O as AsyncOAuth (SDK)
participant F as NullFramework/Session
B->>S: GET /login
S->>H: route request
H->>M: create_session()
M-->>H: session_id
H->>M: generate_login_url(session_id)
M->>O: request auth URL
O->>F: ensure standalone/session context
O-->>M: auth URL
M-->>H: auth URL
H-->>B: 302 Redirect
B->>S: GET /callback?code=...&state=...
S->>H: route request
H->>M: handle_callback(session_id, code, state)
M->>O: exchange code
O->>F: update session/user_id
O-->>M: tokens + user info
M-->>H: render result
H-->>B: HTML page
sequenceDiagram
autonumber
participant App as Standalone App
participant O as OAuth
participant F as NullFramework
participant SM as StorageManager
App->>O: Initialize without framework
O->>O: _initialize_null_framework(storage_config)
O->>F: get/create singleton
O->>F: set_oauth(self)
O->>SM: create memory storage
O-->>App: OAuth initialized (standalone)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
kinde_sdk/auth/oauth.py (4)
120-125: Bug: device_id passed inside config is ignored by StorageManager.initializeStorageManager.initialize expects device_id as a separate argument; embedding it in config has no effect. Pass device_id explicitly.
Apply:
- self._storage_manager.initialize(config={"type": self.framework, "device_id": self._framework.get_name()}, storage=self._storage) + self._storage_manager.initialize( + config={"type": self.framework}, + device_id=self._framework.get_name(), + storage=self._storage, + )
205-242: Add timeouts (and SSL verify) to OpenID configuration fetchExternal calls without timeouts risk hanging threads. Also honor verify setting.
Apply:
- response = requests.get(openid_config_url) + # Use a conservative timeout; allow verify to be toggled + response = requests.get(openid_config_url, timeout=10)Optionally move verify/proxy initialization earlier in init so you can also pass verify=self.verify_ssl and proxies=self.proxy here. I can propose that patch if desired.
596-600: Remove sensitive logging (secrets/PII leakage)Request/response logs include auth code, client_secret, and token payloads. This is a critical leak.
Apply:
- 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=10) + # Avoid logging sensitive request/response contents + self._logger.debug(f"[Exchange code for tokens] [{self.token_url}] [{response.status_code}]")Additionally consider passing verify=self.verify_ssl and proxies=self.proxy if available.
509-515: Enforce robust state validation (prevent CSRF/mix-ups)Currently state is only checked if provided, and the raw value is logged. Enforce presence and validation when a stored state exists; avoid logging sensitive state.
Apply:
- if state: - stored_state = self._session_manager.storage_manager.get("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") + stored_state = self._session_manager.storage_manager.get("state") + if stored_state: + if not state or state != stored_state.get("value"): + self._logger.error("State mismatch or missing state parameter") + raise KindeLoginException("Invalid or missing state parameter")
🧹 Nitpick comments (29)
kinde_sdk/core/framework/null_framework.py (1)
17-26: Singleton is fine; accept minor polishThread-safe double-checked locking looks correct. Consider accepting args safely in new to future-proof.
Apply:
- def __new__(cls): + def __new__(cls, *args, **kwargs): """Implement singleton pattern."""kinde_sdk/auth/__init__.py (1)
13-13: Sort all to satisfy Ruff (RUF022)Keep all sorted to pass lint.
Apply:
-__all__ = ["OAuth", "TokenManager", "UserSession", "permissions", "ApiOptions", "claims", "async_claims", "feature_flags", "portals", "tokens", "roles"] +__all__ = [ + "ApiOptions", + "OAuth", + "TokenManager", + "UserSession", + "async_claims", + "claims", + "feature_flags", + "permissions", + "portals", + "roles", + "tokens", +]kinde_sdk/auth/oauth.py (2)
202-203: Don’t log full user details at INFOThis is PII. Use debug without payload.
Apply:
- self._logger.info(f"Get the claims from the token manager {user_details}") + self._logger.debug("Fetched user details successfully")
323-331: Avoid global state/nonce/code_verifier keys (concurrency-safety)Single keys (“state”, “nonce”, “code_verifier”) collide if multiple auth flows run concurrently. Key by state to isolate sessions.
Example patch:
- self._session_manager.storage_manager.setItems("state", {"value": state}) + self._session_manager.storage_manager.setItems(f"oauth:{state}:state", {"value": state}) ... - self._session_manager.storage_manager.setItems("code_verifier", {"value": code_verifier}) + self._session_manager.storage_manager.setItems(f"oauth:{state}:code_verifier", {"value": code_verifier})And in handle_redirect:
- if state: - stored_state = self._session_manager.storage_manager.get("state") + if state: + stored_state = self._session_manager.storage_manager.get(f"oauth:{state}:state") ... - stored_code_verifier = self._session_manager.storage_manager.get("code_verifier") + stored_code_verifier = self._session_manager.storage_manager.get(f"oauth:{state}:code_verifier") ... - self._session_manager.storage_manager.delete("code_verifier") + self._session_manager.storage_manager.delete(f"oauth:{state}:code_verifier")This makes multi-user flows safer. Based on learnings
Also applies to: 517-524
testv2/testv2_core/test_null_framework.py (5)
1-1: Shebang without execute bit (optional).Either make this file executable in git or drop the shebang to satisfy linters.
-#!/usr/bin/env python3 +# (shebang removed; this file is run by the test runner)
120-134: Avoid catching broad Exception in tests.Catching all exceptions can mask real failures. Narrow the exception or at least log via logger.exception.
- except Exception as e: - errors.append(f"Thread {thread_id}: {e}") + except AssertionError as e: + errors.append(f"Thread {thread_id}: {e}") + except Exception: + import traceback + errors.append(f"Thread {thread_id}: {traceback.format_exc()}")
162-169: Unused parameter in async worker.Rename to underscore to silence linters.
- async def async_worker(worker_id: int, user_id: str): + async def async_worker(_: int, user_id: str):
447-465: Remove f-prefix on constant strings.These f-strings have no placeholders; switch to plain strings.
- print(f"\n{'='*60}") - print(f"Test Summary:") + print("\n" + "=" * 60) + print("Test Summary:") @@ - if result.failures: - print(f"\nFailures:") + if result.failures: + print("\nFailures:") @@ - if result.errors: - print(f"\nErrors:") + if result.errors: + print("\nErrors:")
226-259: Docstring vs. behavior in context isolation (nit).The test confirms isolation by showing a copied context inherits values but later writes in the new context don’t affect the original. Consider rewording the comments to avoid confusion.
testv2/testv2_core/test_kinde_session_management.py (3)
14-16: Avoid sys.path mutation in tests.Prefer relying on installed package/editable install, or configure test runner PYTHONPATH. Path hacking is brittle.
45-50: Silence unused variable and hardcoded secret warnings (tests).
- The local
oauthvariable is only used to trigger NullFramework init. Rename to_oauthto satisfy linters.- Add
# noqa: S106to client_secret argument lines to avoid false positives in test code.- oauth = OAuth( + _oauth = OAuth( client_id="test_client_id", - client_secret="test_client_secret", + client_secret="test_client_secret", # noqa: S106 redirect_uri="http://localhost:5000/callback", framework=None # This will trigger NullFramework usage )(Apply similarly to other occurrences in this file.)
Also applies to: 104-109, 138-143, 162-167, 210-214, 233-238
23-25: Duplicate test coverage with test_kinde_session_management_fixed.py.This file appears functionally identical to test_kinde_session_management_fixed.py. Consolidate to a single module to avoid duplicate execution and maintenance.
testv2/testv2_core/test_kinde_session_management_fixed.py (1)
1-261: Duplicate test module – consolidate.Content substantially duplicates test_kinde_session_management.py. Remove one or merge differences to reduce runtime and maintenance.
kinde_sdk/core/session_management.py (4)
12-16: Remove unused module-level logger.You use self._logger everywhere; drop the module-level logger to avoid confusion.
-if TYPE_CHECKING: - from kinde_sdk.core.framework.null_framework import NullFramework - -logger = logging.getLogger(__name__) +if TYPE_CHECKING: + from kinde_sdk.core.framework.null_framework import NullFramework
74-81: Don’t swallow exceptions silently in discovery path.Log at debug when the NullFramework singleton path fails to aid diagnosis.
- except Exception: - pass + except Exception as e: + self._logger.debug(f"NullFramework singleton check failed: {e}")
148-168: Tighten return type and minor typing polish.Return a typed dict for clarity.
- def get_session_info(self) -> dict: + from typing import Dict, Any # at top of module + def get_session_info(self) -> Dict[str, Any]:
170-177: Avoid PII in repr (optional).Printing raw user_id can leak PII in logs. Consider masking or omitting it.
- return f"KindeSessionManagement(user_id={user_id}, status={status})" + masked = (user_id[:3] + "…") if user_id else None + return f"KindeSessionManagement(user_id={masked}, status={status})"examples/simple_http_oauth_server.py (12)
1-1: Shebang vs. execute bit (optional).Either make the file executable (chmod +x) or remove the shebang to silence linters.
-#!/usr/bin/env python3 +# (shebang removed; run with `python examples/simple_http_oauth_server.py`)
224-227: Use logger.exception for unexpected errors.Capture traceback automatically.
- except Exception as e: - logger.error(f"Error handling request: {e}") + except Exception: + logger.exception("Error handling request") self._send_error_response(500, f"Internal server error: {str(e)}")
271-286: Prefer logger.exception and remove unused param warnings.
- Use logger.exception for tracebacks.
- If
paramsisn’t used, rename to_params.- def _handle_login(self, params): + def _handle_login(self, _params): @@ - except Exception as e: - self._send_error_response(500, f"Login failed: {str(e)}") + except Exception: + logger.exception("Login failed") + self._send_error_response(500, "Login failed")
287-302: Same as above for register handler.- def _handle_register(self, params): + def _handle_register(self, _params): @@ - except Exception as e: - self._send_error_response(500, f"Registration failed: {str(e)}") + except Exception: + logger.exception("Registration failed") + self._send_error_response(500, "Registration failed")
357-359: Use logger.exception and avoid echoing exception text to clients.Return a generic message; log the details server-side.
- except Exception as e: - self._send_error_response(400, f"Authentication failed: {str(e)}") + except Exception: + logger.exception("Authentication failed") + self._send_error_response(400, "Authentication failed")
405-407: Use logger.exception for user info errors.- except Exception as e: - self._send_error_response(500, f"Failed to get user info: {str(e)}") + except Exception: + logger.exception("Failed to get user info") + self._send_error_response(500, "Failed to get user info")
425-427: Use logger.exception for logout errors.- except Exception as e: - self._send_error_response(500, f"Logout failed: {str(e)}") + except Exception: + logger.exception("Logout failed") + self._send_error_response(500, "Logout failed")
279-283: Harden session cookie attributes.Add SameSite=Lax to reduce CSRF risk (HttpOnly already set).
- self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') + self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax') @@ - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') + self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax') @@ - self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0') + self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0; SameSite=Lax')Also applies to: 295-298, 420-423
472-475: Fix logger formatting in log_message.Use parameterized logging to avoid formatting overhead and satisfy linters.
- logger.info(f"{self.address_string()} - {format % args}") + logger.info("%s - " + format, self.address_string(), *args)
506-508: Silence unused variable.Rename to
_oauthor inline.- oauth = get_oauth_manager() + _oauth = get_oauth_manager()
513-526: Remove f-prefix on constant strings.- print(f"🌐 Server running at http://localhost:5000") + print("🌐 Server running at http://localhost:5000")
510-512: Consider ThreadingHTTPServer for responsiveness (optional).Threaded server avoids blocking on long-running requests in examples.
- httpd = HTTPServer(server_address, OAuthHTTPRequestHandler) + from http.server import ThreadingHTTPServer + httpd = ThreadingHTTPServer(server_address, OAuthHTTPRequestHandler)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
examples/simple_http_oauth_server.py(1 hunks)kinde_sdk/__init__.py(2 hunks)kinde_sdk/auth/__init__.py(1 hunks)kinde_sdk/auth/oauth.py(2 hunks)kinde_sdk/core/__init__.py(2 hunks)kinde_sdk/core/framework/null_framework.py(4 hunks)kinde_sdk/core/session_management.py(1 hunks)testv2/testv2_core/test_kinde_session_management.py(1 hunks)testv2/testv2_core/test_kinde_session_management_fixed.py(1 hunks)testv2/testv2_core/test_null_framework.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
testv2/testv2_core/test_kinde_session_management.py (4)
kinde_sdk/core/session_management.py (6)
KindeSessionManagement(18-177)get_user_id(83-93)is_user_logged_in(135-146)set_user_id(95-117)get_session_info(148-168)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (1)
NullFramework(10-128)kinde_sdk/core/framework/framework_factory.py (1)
FrameworkFactory(15-120)kinde_sdk/auth/oauth.py (1)
OAuth(24-658)
kinde_sdk/auth/oauth.py (3)
kinde_sdk/core/framework/null_framework.py (4)
NullFramework(10-128)set_oauth(112-119)start(54-59)get_name(36-43)kinde_sdk/core/storage/storage_factory.py (2)
StorageFactory(10-68)create_storage(26-68)kinde_sdk/core/storage/storage_manager.py (2)
initialize(30-66)storage(89-100)
kinde_sdk/core/framework/null_framework.py (3)
kinde_fastapi/framework/fastapi_framework.py (3)
get_user_id(96-106)set_oauth(108-115)can_auto_detect(234-245)kinde_flask/framework/flask_framework.py (3)
get_user_id(103-113)set_oauth(115-122)can_auto_detect(251-262)kinde_sdk/core/framework/framework_interface.py (2)
get_user_id(68-75)can_auto_detect(78-85)
kinde_sdk/core/__init__.py (1)
kinde_sdk/core/session_management.py (1)
KindeSessionManagement(18-177)
kinde_sdk/core/session_management.py (2)
kinde_sdk/core/framework/null_framework.py (1)
NullFramework(10-128)kinde_sdk/core/framework/framework_factory.py (2)
FrameworkFactory(15-120)get_framework_instance(65-73)
testv2/testv2_core/test_kinde_session_management_fixed.py (4)
kinde_sdk/core/session_management.py (6)
KindeSessionManagement(18-177)get_user_id(83-93)is_user_logged_in(135-146)set_user_id(95-117)get_session_info(148-168)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (1)
NullFramework(10-128)kinde_sdk/core/framework/framework_factory.py (1)
FrameworkFactory(15-120)kinde_sdk/auth/oauth.py (1)
OAuth(24-658)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(363-399)register(401-442)handle_redirect(496-568)is_authenticated(148-168)get_user_info(170-203)logout(444-494)
testv2/testv2_core/test_null_framework.py (2)
kinde_sdk/core/framework/null_framework.py (12)
NullFramework(10-128)get_name(36-43)get_description(45-52)start(54-59)stop(61-66)get_app(68-75)get_request(77-84)can_auto_detect(121-128)get_user_id(86-93)set_user_id(95-104)clear_user_id(106-110)set_oauth(112-119)kinde_sdk/auth/oauth.py (3)
OAuth(24-658)login(363-399)is_authenticated(148-168)
kinde_sdk/__init__.py (1)
kinde_sdk/core/session_management.py (1)
KindeSessionManagement(18-177)
🪛 Ruff (0.13.1)
kinde_sdk/auth/__init__.py
13-13: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
testv2/testv2_core/test_kinde_session_management.py
45-45: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
47-47: Possible hardcoded password assigned to argument: "client_secret"
(S106)
104-104: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
106-106: Possible hardcoded password assigned to argument: "client_secret"
(S106)
138-138: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
140-140: Possible hardcoded password assigned to argument: "client_secret"
(S106)
162-162: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
164-164: Possible hardcoded password assigned to argument: "client_secret"
(S106)
211-211: Possible hardcoded password assigned to argument: "client_secret"
(S106)
233-233: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
235-235: Possible hardcoded password assigned to argument: "client_secret"
(S106)
kinde_sdk/core/session_management.py
42-47: Avoid specifying long messages outside the exception class
(TRY003)
74-75: try-except-pass detected, consider logging the exception
(S110)
74-74: Do not catch blind exception: Exception
(BLE001)
77-77: Consider moving this statement to an else block
(TRY300)
79-79: Do not catch blind exception: Exception
(BLE001)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
111-111: Avoid specifying long messages outside the exception class
(TRY003)
114-114: Avoid specifying long messages outside the exception class
(TRY003)
130-130: Avoid specifying long messages outside the exception class
(TRY003)
143-143: Avoid specifying long messages outside the exception class
(TRY003)
159-159: Avoid specifying long messages outside the exception class
(TRY003)
testv2/testv2_core/test_kinde_session_management_fixed.py
45-45: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
47-47: Possible hardcoded password assigned to argument: "client_secret"
(S106)
104-104: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
106-106: Possible hardcoded password assigned to argument: "client_secret"
(S106)
138-138: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
140-140: Possible hardcoded password assigned to argument: "client_secret"
(S106)
162-162: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
164-164: Possible hardcoded password assigned to argument: "client_secret"
(S106)
211-211: Possible hardcoded password assigned to argument: "client_secret"
(S106)
233-233: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
235-235: Possible hardcoded password assigned to argument: "client_secret"
(S106)
examples/simple_http_oauth_server.py
1-1: Shebang is present but file is not executable
(EXE001)
87-87: Avoid specifying long messages outside the exception class
(TRY003)
163-163: Do not catch blind exception: Exception
(BLE001)
164-164: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
224-224: Do not catch blind exception: Exception
(BLE001)
225-225: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
226-226: Use explicit conversion flag
Replace with conversion flag
(RUF010)
271-271: Unused method argument: params
(ARG002)
284-284: Do not catch blind exception: Exception
(BLE001)
285-285: Use explicit conversion flag
Replace with conversion flag
(RUF010)
287-287: Unused method argument: params
(ARG002)
300-300: Do not catch blind exception: Exception
(BLE001)
301-301: Use explicit conversion flag
Replace with conversion flag
(RUF010)
357-357: Do not catch blind exception: Exception
(BLE001)
358-358: Use explicit conversion flag
Replace with conversion flag
(RUF010)
360-360: Unused method argument: params
(ARG002)
405-405: Do not catch blind exception: Exception
(BLE001)
406-406: Use explicit conversion flag
Replace with conversion flag
(RUF010)
408-408: Unused method argument: params
(ARG002)
425-425: Do not catch blind exception: Exception
(BLE001)
426-426: Use explicit conversion flag
Replace with conversion flag
(RUF010)
506-506: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
513-513: f-string without any placeholders
Remove extraneous f prefix
(F541)
533-533: Do not catch blind exception: Exception
(BLE001)
testv2/testv2_core/test_null_framework.py
1-1: Shebang is present but file is not executable
(EXE001)
132-132: Do not catch blind exception: Exception
(BLE001)
162-162: Unused function argument: worker_id
(ARG001)
448-448: f-string without any placeholders
Remove extraneous f prefix
(F541)
455-455: f-string without any placeholders
Remove extraneous f prefix
(F541)
460-460: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (7)
kinde_sdk/core/framework/null_framework.py (3)
30-35: ContextVar choice: confirm cross-thread expectationsContextVar values don’t propagate across threads; each thread gets its own default unless explicitly set. Confirm this matches how the standalone HTTP example handles per-request/session user_id.
86-110: Session accessors look goodThe get/set/clear user_id APIs are straightforward and match the interface. No issues.
121-128: Explicit non-detectable frameworkReturning False is appropriate here.
kinde_sdk/core/__init__.py (1)
4-4: Top-level core export looks goodPublicly exposing KindeSessionManagement via core is consistent with the PR goals.
Also applies to: 13-13
kinde_sdk/auth/oauth.py (1)
83-85: Automatic null framework init: LGTMReasonable default for standalone usage when no framework is supplied.
kinde_sdk/__init__.py (1)
26-27: Verify packaging metadata version bump
No pyproject.toml, setup.cfg or setup.py found in the repo root; ensure your packaging metadata exists and its version is bumped to match__version__ = "2.1.1"(minor semver bump).testv2/testv2_core/test_kinde_session_management_fixed.py (1)
233-238: Repeat of unused variable and S106 items.Same fixes as suggested in the other test module apply here.
Add HTML escaping to error message output in the OAuth server example to prevent potential XSS vulnerabilities when displaying error content.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
examples/simple_http_oauth_server.py (9)
272-276: Silence unused-parameter warnings for handlers.The
paramsargument isn’t used in these handlers. Rename to_paramsto satisfy linters.Apply this diff:
- def _handle_login(self, params): + def _handle_login(self, _params): @@ - def _handle_register(self, params): + def _handle_register(self, _params): @@ - def _handle_user(self, params): + def _handle_user(self, _params): @@ - def _handle_logout(self, params): + def _handle_logout(self, _params):Also applies to: 288-294, 361-366, 409-415
280-284: Harden session cookies (SameSite).Add SameSite=Lax to reduce CSRF risk while keeping OAuth redirects working on top-level GET.
Apply this diff:
self.send_response(302) self.send_header('Location', login_url) - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') + self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax') self.end_headers() @@ self.send_response(302) self.send_header('Location', register_url) - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') + self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax') self.end_headers() @@ self.send_response(302) self.send_header('Location', logout_url) - self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0') + self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0; SameSite=Lax') self.end_headers()Note: For production over HTTPS, also add the Secure attribute.
Also applies to: 296-300, 421-424
225-227: Don’t leak exceptions to clients; preferlogger.exceptionfor tracebacks.Return generic messages to the browser; log stack traces server-side. Addresses BLE001/TRY400/RUF010.
Apply this diff:
- except Exception as e: - logger.error(f"Error handling request: {e}") - self._send_error_response(500, f"Internal server error: {str(e)}") + except Exception: + logger.exception("Error handling request") + self._send_error_response(500, "Internal server error")- except Exception as e: - self._send_error_response(500, f"Login failed: {str(e)}") + except Exception: + logger.exception("Login failed") + self._send_error_response(500, "Login failed")- except Exception as e: - self._send_error_response(500, f"Registration failed: {str(e)}") + except Exception: + logger.exception("Registration failed") + self._send_error_response(500, "Registration failed")- except Exception as e: - self._send_error_response(400, f"Authentication failed: {str(e)}") + except Exception: + logger.exception("Authentication failed") + self._send_error_response(400, "Authentication failed")- except Exception as e: - self._send_error_response(500, f"Failed to get user info: {str(e)}") + except Exception: + logger.exception("Failed to get user info") + self._send_error_response(500, "Failed to get user info")- except Exception as e: - self._send_error_response(500, f"Logout failed: {str(e)}") + except Exception: + logger.exception("Logout failed") + self._send_error_response(500, "Logout failed")- except Exception as e: - print(f"❌ Server error: {e}") + except Exception: + logger.exception("Server error") + print("❌ Server error")Also applies to: 285-287, 301-303, 358-360, 406-408, 426-428, 534-535
229-271: Avoid naming page contenthtmlto prevent future module shadowing.Minor rename for clarity.
Apply this diff:
def _handle_home(self): """Handle home page.""" - html = """ + page_html = """ <!DOCTYPE html> <html> <head> <title>Kinde OAuth Test Server</title> @@ </html> """ - self._send_html_response(html) + self._send_html_response(page_html)
433-441: Usehttp.cookies.SimpleCookieto parse cookies robustly.Current split logic fails with quoted values or extra spaces. Use the stdlib parser.
Apply this diff:
def _get_session_id_from_cookie(self) -> Optional[str]: """Extract session ID from cookie.""" - cookie_header = self.headers.get('Cookie', '') - for cookie in cookie_header.split(';'): - cookie = cookie.strip() - if cookie.startswith('session_id='): - return cookie.split('=', 1)[1] - return None + cookie_header = self.headers.get('Cookie', '') + if not cookie_header: + return None + try: + import http.cookies as cookies + jar = cookies.SimpleCookie() + jar.load(cookie_header) + if 'session_id' in jar: + return jar['session_id'].value + except Exception: + logger.debug("Failed to parse Cookie header", exc_info=True) + return None
442-448: Rename_send_html_responseparameter to avoid module shadowing and clarify purpose.Prevents confusion with the
htmlmodule name.Apply this diff:
- def _send_html_response(self, html: str, status_code: int = 200): + def _send_html_response(self, content: str, status_code: int = 200): """Send HTML response.""" self.send_response(status_code) self.send_header('Content-Type', 'text/html; charset=utf-8') self.end_headers() - self.wfile.write(html.encode('utf-8')) + self.wfile.write(content.encode('utf-8'))
1-1: Shebang vs. execution bit (EXE001).Either remove the shebang or set the file executable. Since this is run via
python file.py, removing is simplest.Apply this diff:
-#!/usr/bin/env python3
505-509: Remove unused variable assignment.
oauthisn’t used; just call the initializer for side effects.Apply this diff:
- oauth = get_oauth_manager() + get_oauth_manager() print("✅ OAuth manager initialized successfully")
514-514: Drop unnecessary f-prefix.No interpolations present.
Apply this diff:
- print(f"🌐 Server running at http://localhost:5000") + print("🌐 Server running at http://localhost:5000")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(363-399)register(401-442)handle_redirect(496-568)is_authenticated(148-168)get_user_info(170-203)logout(444-494)
🪛 Ruff (0.13.1)
examples/simple_http_oauth_server.py
1-1: Shebang is present but file is not executable
(EXE001)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
164-164: Do not catch blind exception: Exception
(BLE001)
165-165: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
225-225: Do not catch blind exception: Exception
(BLE001)
226-226: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
227-227: Use explicit conversion flag
Replace with conversion flag
(RUF010)
272-272: Unused method argument: params
(ARG002)
285-285: Do not catch blind exception: Exception
(BLE001)
286-286: Use explicit conversion flag
Replace with conversion flag
(RUF010)
288-288: Unused method argument: params
(ARG002)
301-301: Do not catch blind exception: Exception
(BLE001)
302-302: Use explicit conversion flag
Replace with conversion flag
(RUF010)
358-358: Do not catch blind exception: Exception
(BLE001)
359-359: Use explicit conversion flag
Replace with conversion flag
(RUF010)
361-361: Unused method argument: params
(ARG002)
406-406: Do not catch blind exception: Exception
(BLE001)
407-407: Use explicit conversion flag
Replace with conversion flag
(RUF010)
409-409: Unused method argument: params
(ARG002)
426-426: Do not catch blind exception: Exception
(BLE001)
427-427: Use explicit conversion flag
Replace with conversion flag
(RUF010)
465-465: Local variable html referenced before assignment
(F823)
507-507: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
514-514: f-string without any placeholders
Remove extraneous f prefix
(F541)
534-534: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
examples/simple_http_oauth_server.py (1)
465-466: HTML escaping implemented — good catch from earlier review.Message is escaped before embedding, addressing the previous security concern.
- Fix null framework storage initialization with proper storage config handling - Use explicit device_id parameter in storage manager initialization - Add parameter masking in OAuth server logs to prevent sensitive data exposure - Ensure storage type defaults to 'memory' when config is not provided These changes improve both functionality and security for standalone OAuth usage.
There was a problem hiding this comment.
Actionable comments posted: 0
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/oauth.py (1)
120-125: Bug: device_id passed inside config is ignored; pass via parameterStorageManager.initialize ignores device_id embedded in config, so a random device_id will be generated. Pass device_id as its own argument.
Apply:
- # Initialize storage manager with the framework-specific storage - self._storage_manager.initialize(config={"type": self.framework, "device_id": self._framework.get_name()}, storage=self._storage) + # Initialize storage manager with the framework-specific storage + self._storage_manager.initialize( + config={"type": self.framework}, + device_id=self._framework.get_name(), + storage=self._storage, + )
🧹 Nitpick comments (9)
kinde_sdk/auth/oauth.py (1)
516-519: Mask sensitive values in state validation logsAvoid logging raw OAuth state values.
Apply:
- self._logger.warning(f"stored_state: {stored_state}, state: {state}") + self._logger.warning("Validating OAuth state (values redacted)") @@ - self._logger.error(f"State mismatch: received {state}, stored {stored_state}") + self._logger.error("State mismatch during OAuth callback (values redacted)")examples/simple_http_oauth_server.py (8)
1-1: Shebang: either remove or make file executableSince this script isn’t marked executable in the repo, prefer removing the shebang.
Apply:
-#!/usr/bin/env python3
235-276: Avoid shadowing thehtmlmodule; use a clearer variable nameRename the local html string to page_html to prevent accidental shadowing.
Apply:
- html = """ + page_html = """ @@ - self._send_html_response(html) + self._send_html_response(page_html)
197-228: Remove unused params argument and update call sitesThe params argument isn’t used in these handlers; simplify signatures and callers.
Apply:
@@ - elif path == '/login': - self._handle_login(params) + elif path == '/login': + self._handle_login() @@ - elif path == '/register': - self._handle_register(params) + elif path == '/register': + self._handle_register() @@ - elif path == '/user': - self._handle_user(params) + elif path == '/user': + self._handle_user() @@ - elif path == '/logout': - self._handle_logout(params) + elif path == '/logout': + self._handle_logout()-def _handle_login(self, params): +def _handle_login(self):-def _handle_register(self, params): +def _handle_register(self):-def _handle_user(self, params): +def _handle_user(self):-def _handle_logout(self, params): +def _handle_logout(self):Also applies to: 278-289, 294-306, 367-379, 415-421
231-234: Prefer logger.exception for unexpected errorsLogs the traceback and avoids f-strings in logging.
Apply:
- logger.error(f"Error handling request: {e}") + logger.exception("Error handling request")
479-482: Use logger formatting instead of building the string yourselfLet logging handle interpolation.
Apply:
- logger.info(f"{self.address_string()} - {format % args}") + logger.info("%s - " + format, self.address_string(), *args)
513-515: Remove unused variableThe oauth variable isn’t used.
Apply:
- oauth = get_oauth_manager() + get_oauth_manager() print("✅ OAuth manager initialized successfully")
520-520: Remove unnecessary f-stringNo formatting placeholders present.
Apply:
- print(f"🌐 Server running at http://localhost:5000") + print("🌐 Server running at http://localhost:5000")
286-289: Add SameSite to session cookie for basic CSRF protectionEven for a local example, SameSite=Lax is a safe default.
Apply:
- self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') + self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax')- self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') + self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax')- self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0') + self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0; SameSite=Lax')Also applies to: 302-305, 427-430
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/simple_http_oauth_server.py(1 hunks)kinde_sdk/auth/oauth.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
kinde_sdk/auth/oauth.py (3)
kinde_sdk/core/framework/null_framework.py (4)
NullFramework(10-128)set_oauth(112-119)start(54-59)get_name(36-43)kinde_sdk/core/storage/storage_manager.py (4)
storage_type(103-110)get(140-154)initialize(30-66)storage(89-100)kinde_sdk/core/storage/storage_factory.py (2)
StorageFactory(10-68)create_storage(26-68)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(368-404)register(406-447)handle_redirect(501-573)is_authenticated(153-173)get_user_info(175-208)logout(449-499)
🪛 Ruff (0.13.1)
examples/simple_http_oauth_server.py
1-1: Shebang is present but file is not executable
(EXE001)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
164-164: Do not catch blind exception: Exception
(BLE001)
165-165: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
231-231: Do not catch blind exception: Exception
(BLE001)
232-232: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
233-233: Use explicit conversion flag
Replace with conversion flag
(RUF010)
278-278: Unused method argument: params
(ARG002)
291-291: Do not catch blind exception: Exception
(BLE001)
292-292: Use explicit conversion flag
Replace with conversion flag
(RUF010)
294-294: Unused method argument: params
(ARG002)
307-307: Do not catch blind exception: Exception
(BLE001)
308-308: Use explicit conversion flag
Replace with conversion flag
(RUF010)
364-364: Do not catch blind exception: Exception
(BLE001)
365-365: Use explicit conversion flag
Replace with conversion flag
(RUF010)
367-367: Unused method argument: params
(ARG002)
412-412: Do not catch blind exception: Exception
(BLE001)
413-413: Use explicit conversion flag
Replace with conversion flag
(RUF010)
415-415: Unused method argument: params
(ARG002)
432-432: Do not catch blind exception: Exception
(BLE001)
433-433: Use explicit conversion flag
Replace with conversion flag
(RUF010)
471-471: Local variable html referenced before assignment
(F823)
513-513: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
520-520: f-string without any placeholders
Remove extraneous f prefix
(F541)
540-540: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (6)
kinde_sdk/auth/oauth.py (2)
83-85: Good: seamless standalone init via NullFrameworkUsing the null framework when no framework is provided simplifies standalone OAuth usage.
126-152: LGTM: fixed null-framework storage init (type and device_id)The initialization now uses the real storage type and passes device_id correctly to the manager.
examples/simple_http_oauth_server.py (4)
205-215: Nice: sensitive query params masked in logsRedacting code/state in request logs prevents secret leakage.
329-362: Escape user_info before embedding in HTML (XSS risk) and avoidhtmlname shadowingEscape JSON before injecting; rename page var to avoid shadowing.
Apply:
- user_info = result['user'] - html = f""" + user_info = result['user'] + safe_user_info_json = html.escape(json.dumps(user_info, indent=2)) + page_html = f""" @@ - <pre>{json.dumps(user_info, indent=2)}</pre> + <pre>{safe_user_info_json}</pre> @@ - """ - self._send_html_response(html) + """ + self._send_html_response(page_html)
385-411: Escape user_info on user page and avoidhtmlname shadowingSame XSS and shadowing issue here.
Apply:
- html = f""" + safe_user_info_json = html.escape(json.dumps(user_info, indent=2)) + page_html = f""" @@ - <pre>{json.dumps(user_info, indent=2)}</pre> + <pre>{safe_user_info_json}</pre> @@ - """ - self._send_html_response(html) + """ + self._send_html_response(page_html)
455-477: Blocker:htmlvariable shadows module; causes UnboundLocalError and breaks escapingRename the local variable and pre-escape message.
Apply:
- def _send_error_response(self, status_code: int, message: str): + def _send_error_response(self, status_code: int, message: str): """Send error response.""" - html = f""" + escaped_message = html.escape(message) + page_html = f""" @@ - <div class="error"> - <p>{html.escape(message)}</p> + <div class="error"> + <p>{escaped_message}</p> </div> @@ - """ - self._send_html_response(html, status_code) + """ + self._send_html_response(page_html, status_code)
Add HTML escaping to JSON user information displayed in OAuth server example to prevent potential XSS vulnerabilities when rendering user data in HTML pages. Also rename html variable to avoid conflict with imported html module.
Separate HTML escaping logic in error response handler and rename html variable to page_html to avoid naming conflicts with the imported html module, improving code clarity and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
examples/simple_http_oauth_server.py (9)
286-289: Harden session cookies: add SameSite and conditional Secure.Set SameSite=Lax to mitigate CSRF and add Secure when redirect_uri is HTTPS. Also preserve HttpOnly and Path.
@@ - self.send_response(302) - self.send_header('Location', login_url) - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') - self.end_headers() + self.send_response(302) + self.send_header('Location', login_url) + cookie_attrs = 'HttpOnly; Path=/; SameSite=Lax' + try: + if get_oauth_manager().redirect_uri.startswith('https://'): + cookie_attrs += '; Secure' + except Exception: + pass + self.send_header('Set-Cookie', f'session_id={session_id}; {cookie_attrs}') + self.end_headers() @@ - self.send_response(302) - self.send_header('Location', register_url) - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/') - self.end_headers() + self.send_response(302) + self.send_header('Location', register_url) + cookie_attrs = 'HttpOnly; Path=/; SameSite=Lax' + try: + if get_oauth_manager().redirect_uri.startswith('https://'): + cookie_attrs += '; Secure' + except Exception: + pass + self.send_header('Set-Cookie', f'session_id={session_id}; {cookie_attrs}') + self.end_headers() @@ - self.send_response(302) - self.send_header('Location', logout_url) - self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0') - self.end_headers() + self.send_response(302) + self.send_header('Location', logout_url) + cookie_attrs = 'HttpOnly; Path=/; Max-Age=0; SameSite=Lax' + try: + if get_oauth_manager().redirect_uri.startswith('https://'): + cookie_attrs += '; Secure' + except Exception: + pass + self.send_header('Set-Cookie', f'session_id=; {cookie_attrs}') + self.end_headers()Also applies to: 302-305, 429-432
441-448: Use SimpleCookie to robustly parse cookies.Manual parsing can break with spaces/quoting. Use http.cookies.SimpleCookie.
@@ -from http.server import HTTPServer, BaseHTTPRequestHandler +from http.server import HTTPServer, BaseHTTPRequestHandler +from http.cookies import SimpleCookie @@ def _get_session_id_from_cookie(self) -> Optional[str]: """Extract session ID from cookie.""" - cookie_header = self.headers.get('Cookie', '') - for cookie in cookie_header.split(';'): - cookie = cookie.strip() - if cookie.startswith('session_id='): - return cookie.split('=', 1)[1] - return None + cookie_header = self.headers.get('Cookie', '') + if not cookie_header: + return None + cookie = SimpleCookie() + cookie.load(cookie_header) + morsel = cookie.get('session_id') + return morsel.value if morsel else NoneAlso applies to: 44-47
231-234: Don’t leak internal errors to clients; log stack traces.Switch to logger.exception and return generic messages.
@@ - except Exception as e: - logger.error(f"Error handling request: {e}") - self._send_error_response(500, f"Internal server error: {str(e)}") + except Exception: + logger.exception("Error handling request") + self._send_error_response(500, "Internal server error") @@ - except Exception as e: - self._send_error_response(500, f"Login failed: {str(e)}") + except Exception: + logger.exception("Login failed") + self._send_error_response(500, "Login failed") @@ - except Exception as e: - self._send_error_response(500, f"Registration failed: {str(e)}") + except Exception: + logger.exception("Registration failed") + self._send_error_response(500, "Registration failed") @@ - except Exception as e: - self._send_error_response(400, f"Authentication failed: {str(e)}") + except Exception: + logger.exception("Authentication failed") + self._send_error_response(400, "Authentication failed") @@ - except Exception as e: - self._send_error_response(500, f"Failed to get user info: {str(e)}") + except Exception: + logger.exception("Failed to get user info") + self._send_error_response(500, "Failed to get user info") @@ - except Exception as e: - self._send_error_response(500, f"Logout failed: {str(e)}") + except Exception: + logger.exception("Logout failed") + self._send_error_response(500, "Logout failed")Also applies to: 291-293, 307-309, 365-366, 414-416, 434-436
210-213: Minor: simplify dict check with get().Streamline masking logic.
- for k in ("code", "state"): - if k in masked_params and masked_params[k]: - masked_params[k] = "[redacted]" + for k in ("code", "state"): + if masked_params.get(k): + masked_params[k] = "[redacted]"
235-276: Avoid shadowing the imported html module.Rename local html variable to reduce confusion.
- html = """ + page_html = """ @@ - """ - self._send_html_response(html) + """ + self._send_html_response(page_html)
521-521: Optional: use ThreadingHTTPServer for concurrent requests.Improves UX under multiple parallel requests. Safe with context-local session management.
-from http.server import HTTPServer, BaseHTTPRequestHandler +from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler @@ - httpd = HTTPServer(server_address, OAuthHTTPRequestHandler) + httpd = ThreadingHTTPServer(server_address, OAuthHTTPRequestHandler)Also applies to: 44-47
278-278: Silence unused-argument warnings.Rename params to _params.
-def _handle_login(self, params): +def _handle_login(self, _params): @@ -def _handle_register(self, params): +def _handle_register(self, _params): @@ -def _handle_user(self, params): +def _handle_user(self, _params): @@ -def _handle_logout(self, params): +def _handle_logout(self, _params):Also applies to: 294-294, 368-368, 417-417
516-518: Tidy: remove unused variable and extraneous f-string.- oauth = get_oauth_manager() - print("✅ OAuth manager initialized successfully") + get_oauth_manager() + print("✅ OAuth manager initialized successfully") @@ - print(f"🌐 Server running at http://localhost:5000") + print("🌐 Server running at http://localhost:5000")Also applies to: 523-523
1-1: Shebang: either make the file executable or drop the shebang.Script isn’t executable in VCS per lint. Either chmod +x or remove the shebang line.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(368-404)register(406-447)handle_redirect(501-573)is_authenticated(153-173)get_user_info(175-208)logout(449-499)
🪛 Ruff (0.13.1)
examples/simple_http_oauth_server.py
1-1: Shebang is present but file is not executable
(EXE001)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
164-164: Do not catch blind exception: Exception
(BLE001)
165-165: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
231-231: Do not catch blind exception: Exception
(BLE001)
232-232: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
233-233: Use explicit conversion flag
Replace with conversion flag
(RUF010)
278-278: Unused method argument: params
(ARG002)
291-291: Do not catch blind exception: Exception
(BLE001)
292-292: Use explicit conversion flag
Replace with conversion flag
(RUF010)
294-294: Unused method argument: params
(ARG002)
307-307: Do not catch blind exception: Exception
(BLE001)
308-308: Use explicit conversion flag
Replace with conversion flag
(RUF010)
365-365: Do not catch blind exception: Exception
(BLE001)
366-366: Use explicit conversion flag
Replace with conversion flag
(RUF010)
368-368: Unused method argument: params
(ARG002)
414-414: Do not catch blind exception: Exception
(BLE001)
415-415: Use explicit conversion flag
Replace with conversion flag
(RUF010)
417-417: Unused method argument: params
(ARG002)
434-434: Do not catch blind exception: Exception
(BLE001)
435-435: Use explicit conversion flag
Replace with conversion flag
(RUF010)
516-516: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
523-523: f-string without any placeholders
Remove extraneous f prefix
(F541)
543-543: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
examples/simple_http_oauth_server.py (1)
65-66: Imports are valid: AsyncOAuth and KindeSessionManagement are exported in kinde_sdk/init.py.
- Add SameSite=Lax attribute to session cookies for CSRF protection - Rename unused parameters to _params to indicate intentional non-usage - Improve overall security posture of the OAuth server example These changes help prevent cross-site request forgery attacks and improve code clarity by properly handling unused parameters.
- Replace detailed error messages in user responses with generic messages - Use logger.exception() for proper error logging with stack traces - Remove unused exception variables to clean up code - Prevent sensitive error information from being exposed to users This enhances security by avoiding information disclosure through error messages while maintaining proper logging for debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/simple_http_oauth_server.py (1)
1-70: LGTM: Clean setup and comprehensive documentation.The module header, imports, and environment loading logic are well-structured. The docstring provides clear usage instructions, and the dotenv fallback logic (parent directory → current directory) handles common project layouts.
Optional: The shebang is present but the file isn't executable. If you want users to run it directly, add execute permissions:
chmod +x examples/simple_http_oauth_server.py
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(368-404)register(406-447)handle_redirect(501-573)is_authenticated(153-173)get_user_info(175-208)logout(449-499)
🪛 Ruff (0.13.3)
examples/simple_http_oauth_server.py
1-1: Shebang is present but file is not executable
(EXE001)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
521-521: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
528-528: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (4)
examples/simple_http_oauth_server.py (4)
72-180: LGTM: Well-designed manager for standalone OAuth.The
SimpleOAuthManagerclass demonstrates clean integration ofKindeSessionManagementwith the SDK's standalone mode (framework=None). The consistent pattern of settinguser_idbefore each SDK operation is correct and aligns with the new session management API.Key strengths:
- Proper initialization with configuration validation
- Thread-safe session management via
KindeSessionManagement- Graceful error handling in
get_user_info(line 164)- Clear separation between session lifecycle and OAuth operations
183-191: LGTM: Appropriate singleton pattern for the example.The lazy initialization pattern is suitable for this demo server.
194-489: LGTM: All previous security issues resolved.The request handler implementation correctly addresses all security concerns from previous reviews:
✅ Lines 208-214: Sensitive OAuth parameters (
code,state) are masked before logging
✅ Lines 333, 389: User info is HTML-escaped before embedding in responses
✅ Line 464: Error messages are escaped before embedding
✅ Lines 334, 390, 465: Variable naming avoids shadowing thehtmlmoduleCookie security is appropriate for a localhost example (HttpOnly, SameSite=Lax). The routing logic and error handling are clean and straightforward.
492-550: LGTM: Robust startup validation.The main function properly validates environment configuration before starting the server. Line 521's
oauth = get_oauth_manager()assignment is intentional—it validates that OAuth initialization succeeds before binding the server socket, providing fail-fast behavior with clear error messages.
Rename html variable to page_html in _handle_home method for consistency with other methods and to avoid naming conflicts with the imported html module.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
examples/simple_http_oauth_server.py (8)
208-214: Simplify masking using dict.get (RUF019).Minor cleanup; avoids redundant key check.
- masked_params = dict(params) - for k in ("code", "state"): - if k in masked_params and masked_params[k]: - masked_params[k] = "[redacted]" + masked_params = dict(params) + for k in ("code", "state"): + if masked_params.get(k): + masked_params[k] = "[redacted]"
286-289: Add Secure to cookies when appropriate.Harden cookies if running over HTTPS; keeps localhost working.
@@ - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax') + cookie_attrs = "HttpOnly; Path=/; SameSite=Lax" + try: + # Add Secure if redirect target is HTTPS (likely production) + if oauth.redirect_uri and oauth.redirect_uri.startswith("https://"): + cookie_attrs += "; Secure" + except Exception: + pass + self.send_header('Set-Cookie', f'session_id={session_id}; {cookie_attrs}') @@ - self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Path=/; SameSite=Lax') + cookie_attrs = "HttpOnly; Path=/; SameSite=Lax" + try: + if oauth.redirect_uri and oauth.redirect_uri.startswith("https://"): + cookie_attrs += "; Secure" + except Exception: + pass + self.send_header('Set-Cookie', f'session_id={session_id}; {cookie_attrs}') @@ - self.send_header('Set-Cookie', 'session_id=; HttpOnly; Path=/; Max-Age=0; SameSite=Lax') + cookie_attrs = "HttpOnly; Path=/; Max-Age=0; SameSite=Lax" + try: + if oauth.redirect_uri and oauth.redirect_uri.startswith("https://"): + cookie_attrs += "; Secure" + except Exception: + pass + self.send_header('Set-Cookie', f'session_id=; {cookie_attrs}')Also applies to: 303-306, 433-436
446-453: Use SimpleCookie for robust cookie parsing.Handles spaces/quoting reliably.
- def _get_session_id_from_cookie(self) -> Optional[str]: - """Extract session ID from cookie.""" - cookie_header = self.headers.get('Cookie', '') - for cookie in cookie_header.split(';'): - cookie = cookie.strip() - if cookie.startswith('session_id='): - return cookie.split('=', 1)[1] - return None + def _get_session_id_from_cookie(self) -> Optional[str]: + """Extract session ID from cookie.""" + from http.cookies import SimpleCookie + cookie_header = self.headers.get('Cookie') + if not cookie_header: + return None + c = SimpleCookie() + c.load(cookie_header) + morsel = c.get('session_id') + return morsel.value if morsel else None
455-461: Avoid parameter name shadowing of the html module.Rename for clarity and to prevent future regressions.
- def _send_html_response(self, html: str, status_code: int = 200): + def _send_html_response(self, page_html: str, status_code: int = 200): """Send HTML response.""" self.send_response(status_code) self.send_header('Content-Type', 'text/html; charset=utf-8') self.end_headers() - self.wfile.write(html.encode('utf-8')) + self.wfile.write(page_html.encode('utf-8'))
521-523: Remove unused variable (F841).Call the initializer without assigning.
- oauth = get_oauth_manager() + get_oauth_manager() print("✅ OAuth manager initialized successfully")
528-528: Drop unnecessary f-string (F541).No interpolation needed.
- print(f"🌐 Server running at http://localhost:5000") + print("🌐 Server running at http://localhost:5000")
176-180: Avoid logging raw session IDs.Reduce exposure; log a short prefix.
- logger.info(f"Generated logout URL and cleared session {session_id}") + logger.info("Generated logout URL and cleared session %s...", session_id[:8])
1-1: Shebang vs file mode (EXE001). The file is currently non-executable (mode 100644). Either:
- Make it executable:
git update-index --chmod=+x examples/simple_http_oauth_server.py- Or drop the shebang:
-#!/usr/bin/env python3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(368-404)register(406-447)handle_redirect(501-573)is_authenticated(153-173)get_user_info(175-208)logout(449-499)
🪛 Ruff (0.13.3)
examples/simple_http_oauth_server.py
1-1: Shebang is present but file is not executable
(EXE001)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
521-521: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
528-528: f-string without any placeholders
Remove extraneous f prefix
(F541)
- Replace manual cookie parsing with proper http.cookies.SimpleCookie - Add robust error handling for malformed cookie headers - Make the OAuth server script executable for direct execution - Add null check for empty cookie headers This improves reliability when handling cookies and makes the server easier to run directly from the command line.
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 (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (3)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)
🪛 Ruff (0.13.3)
examples/simple_http_oauth_server.py
88-88: Avoid specifying long messages outside the exception class
(TRY003)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
457-457: Do not catch blind exception: Exception
(BLE001)
527-527: Local variable oauth is assigned to but never used
Remove assignment to unused variable oauth
(F841)
534-534: f-string without any placeholders
Remove extraneous f prefix
(F541)
Remove unused oauth variable assignment in main function initialization test to clean up code and eliminate unused variable warning.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/simple_http_oauth_server.py (1)
534-534: Remove unnecessary f-string prefix.The print statement has no placeholders, so the
fprefix is unnecessary.Apply this diff:
- print(f"🌐 Server running at http://localhost:5000") + print("🌐 Server running at http://localhost:5000")
🧹 Nitpick comments (2)
examples/simple_http_oauth_server.py (2)
88-88: Consider extracting the error message (optional refinement).Ruff TRY003 suggests avoiding long messages directly in exception calls. For an example file, the current approach is clear and acceptable. If you prefer strict adherence to the guideline, you could define a constant message or move it to a custom exception class.
210-212: Simplify withdict.get()(optional refinement).The explicit key check can be simplified using
dict.get().Apply this diff:
- for k in ("code", "state"): - if k in masked_params and masked_params[k]: - masked_params[k] = "[redacted]" + for k in ("code", "state"): + if masked_params.get(k): + masked_params[k] = "[redacted]"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(368-404)register(406-447)handle_redirect(501-573)is_authenticated(153-173)get_user_info(175-208)logout(449-499)
🪛 Ruff (0.13.3)
examples/simple_http_oauth_server.py
88-88: Avoid specifying long messages outside the exception class
(TRY003)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
457-457: Do not catch blind exception: Exception
(BLE001)
534-534: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (3)
examples/simple_http_oauth_server.py (3)
208-214: LGTM! Sensitive parameters are properly masked.The masking of
codeandstatebefore logging prevents sensitive OAuth values from appearing in logs. This addresses the security concern from previous reviews.
333-333: LGTM! HTML injection is properly prevented.All user-controlled content is correctly escaped using
html.escape()before being embedded in HTML responses, and the variable naming avoids shadowing thehtmlmodule. This resolves the security issues from previous reviews.Also applies to: 389-389, 470-470
457-457: Broad exception handling is acceptable here.While Ruff BLE001 flags the broad
Exceptioncatch, this is appropriate for a helper method that should gracefully returnNoneon any cookie parsing error, preventing the request handler from crashing on malformed cookies.
Replace f-string with regular string for server startup message since no variable interpolation is needed, improving code simplicity.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
examples/simple_http_oauth_server.py (3)
88-88: Consider extracting the exception message.The static analysis tool flags this as TRY003 (long message in exception). While not critical, extracting to a constant or shortening would align with best practices.
Apply this diff to shorten the message:
- raise KindeConfigurationException("KINDE_CLIENT_ID environment variable is required") + raise KindeConfigurationException("KINDE_CLIENT_ID is required")
210-212: Consider using dict.get() for cleaner code.The static analysis tool suggests RUF019 - using
dict.get()instead of membership check followed by access.Apply this diff:
masked_params = dict(params) for k in ("code", "state"): - if k in masked_params and masked_params[k]: - masked_params[k] = "[redacted]" + if masked_params.get(k): + masked_params[k] = "[redacted]"
286-288: Note: Cookie security for production use.The cookies use
HttpOnlyandSameSite=Lax, which is good. However, for production deployments over HTTPS, add theSecureflag to prevent cookie transmission over insecure connections. The current configuration is appropriate for localhost development.For production:
self.send_header('Set-Cookie', f'session_id={session_id}; HttpOnly; Secure; Path=/; SameSite=Lax')
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_http_oauth_server.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/simple_http_oauth_server.py (4)
kinde_sdk/auth/async_oauth.py (1)
AsyncOAuth(13-99)kinde_sdk/core/session_management.py (3)
KindeSessionManagement(18-177)set_user_id(95-117)clear_user_id(119-133)kinde_sdk/core/framework/null_framework.py (2)
set_user_id(95-104)clear_user_id(106-110)kinde_sdk/auth/oauth.py (6)
login(368-404)register(406-447)handle_redirect(501-573)is_authenticated(153-173)get_user_info(175-208)logout(449-499)
🪛 Ruff (0.13.3)
examples/simple_http_oauth_server.py
88-88: Avoid specifying long messages outside the exception class
(TRY003)
211-211: Unnecessary key check before dictionary access
Replace with dict.get
(RUF019)
457-457: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (7)
examples/simple_http_oauth_server.py (7)
37-63: LGTM!The import structure is clean, and the dotenv loading gracefully handles both the ImportError case and multiple .env file locations.
80-98: Verify if KINDE_CLIENT_SECRET validation is needed.The code loads
client_secretfrom the environment but doesn't validate it as required. While public clients can operate without a client secret, confidential clients and M2M flows typically require it. Consider whether to:
- Validate
client_secretas required for this example (recommended for clarity)- Document when it's optional vs. required
- Validate it conditionally based on OAuth flow type
For a comprehensive example demonstrating OAuth best practices, validating the client secret would prevent common configuration errors.
208-214: LGTM!The sensitive parameter masking for OAuth credentials is correctly implemented.
333-354: LGTM!The user info JSON is properly escaped before embedding in HTML, preventing injection attacks. The variable naming (page_html) avoids shadowing the html module.
446-459: LGTM!The defensive exception handling in cookie parsing is appropriate. The blind Exception catch (flagged by BLE001) is intentional here - if cookie parsing fails for any reason, the method safely returns None, which is the correct behavior for this use case.
498-557: LGTM!The main function provides excellent developer experience with:
- Clear environment validation
- Helpful setup instructions when configuration is missing
- Configuration summary at startup
- Graceful shutdown handling
72-181: Excellent demonstration of the new standalone OAuth pattern.The
SimpleOAuthManagercorrectly demonstrates how to use the SDK in standalone mode:
- Initialize
AsyncOAuthwithframework=Noneto useNullFramework- Create
KindeSessionManagementinstance for session control- Call
session_mgmt.set_user_id()before each SDK operation- Use standard SDK methods (
login(),register(),handle_redirect(), etc.)This is a valuable reference implementation for users wanting to integrate Kinde OAuth without a web framework.
Use dict.get() method instead of explicit membership and truthiness checks for cleaner and more pythonic code in parameter masking.
Make KINDE_CLIENT_ID error message more concise by removing redundant reference to environment variable in the message text.
2 participants
Explain your changes
This enables standalone OAuth usage without requiring a web framework, making the SDK more flexible for various application architectures.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.