Conversation
Flask: - Initialize Flask-Session extension with Session(app) - Without this, SESSION_TYPE config was ignored and OAuth tokens were lost due to cookie size limits (~4KB) FastAPI: - Use OAuth instead of SmartOAuth to match README - Remove Management API dependency from example - Remove custom /login /logout routes that returned JSON - Update README with correct run instructions Fixes: authentication always returning false after login Refs: #129
Load .env relative to the example script's directory so it is found regardless of the working directory. Remove nest_asyncio (incompatible with uvloop) and use asyncio.new_event_loop() consistently in all Flask route handlers.
WalkthroughFastAPI examples moved to .env-based config, replaced SmartOAuth with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant FlaskApp
participant OAuthHandler
participant SessionStore
User->>Browser: Click "Login"
Browser->>FlaskApp: GET /login
FlaskApp->>OAuthHandler: _run_async -> generate auth URL & redirect
OAuthHandler-->>Browser: Redirect to provider
Browser->>FlaskApp: GET /callback
FlaskApp->>OAuthHandler: _run_async -> exchange code for tokens
OAuthHandler-->>FlaskApp: Return tokens and user info
FlaskApp->>SessionStore: Persist server-side session using user:* keys
FlaskApp-->>Browser: Redirect to protected resource
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_flask/framework/flask_framework.py (1)
207-214:⚠️ Potential issue | 🟡 MinorCallback route doesn't wrap
loop.close()infinally, unlike all other routes.If
handle_redirectraises an unexpected exception not caught by theexcept(e.g.,BaseExceptionsubclass) or if additional code is added later beforeloop.close(), the event loop leaks. All other route handlers in this file correctly usetry/finally. Apply the same pattern here for consistency and safety.Proposed fix
try: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) - loop.run_until_complete(self._oauth.handle_redirect(code, user_id, state)) - loop.close() - except Exception as e: - return f"Authentication failed: {str(e)}", 400 + try: + loop.run_until_complete(self._oauth.handle_redirect(code, user_id, state)) + except Exception as e: + return f"Authentication failed: {str(e)}", 400 + finally: + loop.close()
🤖 Fix all issues with AI agents
In `@kinde_fastapi/examples/example_app.py`:
- Around line 114-126: The protected_route currently returns a 200 JSON error
body when kinde_oauth.is_authenticated() is false; change this so
unauthenticated requests either send a RedirectResponse to "/login" or raise
fastapi.HTTPException(status_code=401). Locate protected_route and replace the
current return {"error":..., "redirect": "/login"} with either a
RedirectResponse("/login") (and add the fastapi.responses import) for automatic
redirection, or raise HTTPException(status_code=401) for a proper unauthorized
response, keeping user = kinde_oauth.get_user_info() only in the authenticated
branch.
- Around line 70-100: The HTML response interpolates unescaped user-controlled
values (user.get('email'), user.get('given_name'), user.get('family_name')) and
exception text (str(e)) which enables XSS; update the response generation in the
block that runs when kinde_oauth.is_authenticated() and in the except block to
escape these values (e.g., call html.escape on each interpolated value) or
switch to a templating engine (e.g., Jinja2) that auto-escapes, ensuring values
from kinde_oauth.get_user_info() and the exception message passed to
logger.error/returned HTML are properly escaped before insertion.
In `@kinde_fastapi/examples/README.md`:
- Around line 18-23: The fenced .env snippet containing the lines
KINDE_CLIENT_ID, KINDE_CLIENT_SECRET, KINDE_REDIRECT_URI, and KINDE_HOST should
include a language identifier to satisfy MD040; update the opening fence from
``` to ```env (or ```bash/```ini) so the block becomes a recognized env code
block.
In `@kinde_flask/framework/flask_framework.py`:
- Line 40: The hardcoded default for SESSION_FILE_DIR uses a predictable /tmp
path; change the logic around self.app.config['SESSION_FILE_DIR'] to fallback to
a securely created temporary directory when SESSION_FILE_DIR env var is not set:
call tempfile.mkdtemp() (or create a project-local subdir) to generate the
directory, ensure it exists with restrictive permissions (e.g., 0700), and then
assign that path to self.app.config['SESSION_FILE_DIR'] while preserving the
existing os.getenv override behavior so operators can still supply
SESSION_FILE_DIR.
- Line 37: The current assignment self.app.config['SECRET_KEY'] =
os.getenv('SECRET_KEY', 'your-secret-key') uses a predictable default; change it
to require a real secret by reading secret = os.getenv('SECRET_KEY') and if
secret is missing raise a clear error (e.g., RuntimeError("SECRET_KEY
environment variable must be set for production")), otherwise set
self.app.config['SECRET_KEY'] = secret; alternatively, if you prefer an
automatic non-guessable fallback, generate a secure value with
secrets.token_urlsafe() and use that instead—update the code paths around the
os.getenv('SECRET_KEY') use and the assignment to self.app.config['SECRET_KEY']
accordingly.
🧹 Nitpick comments (2)
kinde_fastapi/examples/example_app.py (2)
66-66: Unusedrequestparameter.
request: Requestis declared but never referenced in the function body (Ruff ARG001). Either remove it or, if the session middleware requires it to be declared, add a comment explaining why.
90-91: Uselogging.exceptionto capture the traceback.
logger.error(f"Error getting user info: {e}")discards the stack trace. Per Ruff TRY400, preferlogger.exception(...)which automatically attaches the traceback at ERROR level.- logger.error(f"Error getting user info: {e}") + logger.exception("Error getting user info")
KomanRudden
changed the title
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_flask/framework/flask_framework.py (1)
226-233:⚠️ Potential issue | 🟠 MajorEvent loop not closed on exception path — inconsistent with other routes.
Unlike the
login,logout, andregisterroutes which all usetry/finallyto guaranteeloop.close(), the callback route leaks the event loop whenhandle_redirectraises an exception. Also,asyncio.set_event_loop(loop)is called here but not in the other routes — this inconsistency should be resolved (preferably by removing it, since it's unnecessary forrun_until_complete).Proposed fix
# Handle async call to handle_redirect - try: - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) + loop = asyncio.new_event_loop() + try: loop.run_until_complete(self._oauth.handle_redirect(code, user_id, state)) - loop.close() except Exception as e: return f"Authentication failed: {str(e)}", 400 + finally: + loop.close()
🤖 Fix all issues with AI agents
In `@kinde_fastapi/examples/example_app.py`:
- Around line 121-134: The protected_route calls kinde_oauth.get_user_info()
without error handling; wrap the call in a try/except like the home route to
catch exceptions from get_user_info() and handle them gracefully (e.g., log the
error and return RedirectResponse("/login") or an appropriate HTTP response) so
a thrown exception doesn't surface as a raw 500; update protected_route to call
kinde_oauth.is_authenticated(), then try: user = kinde_oauth.get_user_info()
except Exception as e: (log e) and return RedirectResponse("/login") before
returning the JSON with user.get('email').
🧹 Nitpick comments (3)
kinde_flask/framework/flask_framework.py (1)
270-278: Unnecessaryasyncio.set_event_loop(loop)call.Same as the callback route —
set_event_loopisn't needed forrun_until_completeand isn't used in the other routes. Remove it for consistency.Proposed fix
loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) try: login_url = loop.run_until_complete(self._oauth.login()) return redirect(login_url) finally: loop.close()kinde_fastapi/examples/example_app.py (2)
96-98: Consider usinglogger.exceptioninstead oflogger.errorto capture the traceback.
logger.exception()automatically includes the stack trace, which is more useful for debugging OAuth/API errors.Proposed fix
- logger.error(f"Error getting user info: {e}") + logger.exception("Error getting user info")
66-118: Remove unusedrequestparameter.The
html.escape()on all user-controlled values and the exception message properly addresses XSS concerns. However, therequest: Requestparameter on line 67 is never referenced in the function body and can be safely removed.Proposed fix
`@app.get`("/", response_class=HTMLResponse) -async def home(request: Request): +async def home():
…/kinde-python-sdk into fix/fastapi-flask-sdk-example
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@testv2/testv2_framework/test_flask_framework.py`:
- Around line 291-299: The test's try/except is ineffective because Flask's test
client returns a response rather than raising; update the block that calls
framework.app.test_client() to capture the response (response =
client.get('/login')) and assert response.status_code == 500 to verify the route
raised an exception, and then keep the existing assertion
mock_loop.close.assert_called() to ensure the loop was closed; reference the
call to framework.app.test_client(), the GET to '/login', and mock_loop.close in
your changes.
- Around line 176-188: The tearDown currently removes SESSION_FILE_DIR from
os.environ before reading it, so the temp directory created in setUp is never
removed; modify tearDown (and/or setUp) to capture the session dir path
(SESSION_FILE_DIR) into a local variable (e.g., session_dir =
os.environ.get('SESSION_FILE_DIR') or store it on self during setUp) before
deleting environment keys, then use that saved path to remove the directory with
shutil.rmtree if it exists; ensure references to self.original_env restoration
and deletion of env keys remain but operate after or without clobbering the
saved session_dir.
- Around line 82-90: The test test_session_type_from_environment currently sets
SESSION_TYPE to the same default ('filesystem') so it doesn't prove the
environment override is read; update the test to set os.environ['SESSION_TYPE']
to a distinct value (e.g., 'null' or 'redis') before instantiating
FlaskFramework and assert that framework.app.config['SESSION_TYPE'] equals that
distinct value, keeping the
`@patch`('kinde_flask.framework.flask_framework.Session') usage and test name
unchanged so the override path in FlaskFramework is exercised.
- Around line 393-424: The tearDown is a no-op causing a temp session directory
created by instantiating FlaskFramework in test_secret_key_length to leak;
modify tests to track and remove that directory by (1) updating tearDown to
remove any created temp session dir (detect framework.session_dir or the path
returned by tempfile.mkdtemp) and clear os.environ changes, and (2) in
test_secret_key_length either mock tempfile.mkdtemp/Session like
test_session_directory_permissions or record framework.app.config usage then
delete the created test_temp_dir after the assertion; ensure you reference
FlaskFramework, tearDown, test_secret_key_length, tempfile.mkdtemp and Session
when locating the code to change.
🧹 Nitpick comments (2)
testv2/testv2_framework/test_flask_framework.py (2)
109-113: Preferself.assert*over bareassertin unittest tests.Bare
assertstatements are silently skipped underpython -Oand produce less informative failure messages than unittest's assertion methods.Proposed fix
- assert mock_chmod.call_count >= 1 - first_call = mock_chmod.call_args_list[0] - assert first_call == call(test_temp_dir, 0o700) + self.assertGreaterEqual(mock_chmod.call_count, 1) + first_call = mock_chmod.call_args_list[0] + self.assertEqual(first_call, call(test_temp_dir, 0o700))The same applies to lines 409-411.
37-54: InconsistentSessionmocking — some tests trigger real Flask-Session initialization.This test (and
test_secret_key_from_environment,test_session_file_dir_from_environment,test_secret_key_length) don't mockSession, soFlaskFramework()creates real server-side session infrastructure. Other tests in the same file do mock it. This creates filesystem side effects and couples tests to Flask-Session being installed.Consider adding
@patch('kinde_flask.framework.flask_framework.Session')consistently to all tests that instantiateFlaskFramework(), or extract it into a shared helper/setUp.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@testv2/testv2_framework/test_flask_framework.py`:
- Around line 26-34: tearDown currently only restores envs in self.original_env
and removes self.test_session_dir, so temp dirs created by FlaskFramework via
tempfile.mkdtemp and any SESSION_FILE_DIR set by tests are leaked and env vars
added during tests (e.g., SECRET_KEY) are not removed; fix by (1) expanding
tearDown in test_flask_framework.py to remove any framework-created session dir:
inspect framework.app.config.get('SESSION_FILE_DIR') (or track the path returned
by tempfile.mkdtemp when FlaskFramework() is instantiated) and if present and
exists, shutil.rmtree it, and (2) ensure teardown also deletes any env vars that
tests added by comparing original_env to os.environ and removing keys not
present in original_env; alternatively, in each affected test that instantiates
FlaskFramework(), mock tempfile.mkdtemp and os.chmod to avoid creating real
tempdirs (target tempfile.mkdtemp and os.chmod calls used during FlaskFramework
initialization).
🧹 Nitpick comments (3)
testv2/testv2_framework/test_flask_framework.py (3)
109-111: Useself.assert*instead of bareassertfor consistency and better failure messages.Bare
assertstatements don't produce informative failure messages in unittest and are inconsistent with the rest of the test file.Proposed fix
- assert mock_chmod.call_count >= 1 - first_call = mock_chmod.call_args_list[0] - assert first_call == call(test_temp_dir, 0o700) + self.assertGreaterEqual(mock_chmod.call_count, 1) + first_call = mock_chmod.call_args_list[0] + self.assertEqual(first_call, call(test_temp_dir, 0o700))
188-214: Consider extracting the repeated event-loop mock setup into a helper.The three route tests (
test_login_route_event_loop_management,test_logout_route_event_loop_management,test_register_route_event_loop_management) share near-identical mock setup and assertions. A small helper could reduce duplication.Not blocking — the tests are correct as-is.
Also applies to: 216-245, 247-273
382-393:setUpdoesn't save original env vars before clearing them.Unlike the other test classes,
TestFlaskFrameworkSecurity.setUpdeletes env vars without saving originals, andtearDownonly deletes — it never restores. If these vars were set in the outer environment before the test run, they'll be permanently lost for subsequent test classes.Proposed fix
def setUp(self): """Set up test fixtures.""" - # Clear env vars + self.original_env = {} for key in ['SECRET_KEY', 'SESSION_TYPE', 'SESSION_FILE_DIR']: if key in os.environ: + self.original_env[key] = os.environ[key] del os.environ[key] def tearDown(self): """Clean up after tests.""" for key in ['SECRET_KEY', 'SESSION_TYPE', 'SESSION_FILE_DIR']: if key in os.environ: del os.environ[key] + for key, value in self.original_env.items(): + os.environ[key] = value
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_flask/framework/flask_framework.py (1)
235-241:⚠️ Potential issue | 🟠 MajorEvent loop is leaked when
handle_redirectraises.Lines 236–239 don't use a
try/finallyblock, unlike every other route in this file. Ifself._oauth.handle_redirect(...)throws,loop.close()on line 239 is skipped because control jumps directly to theexcepton line 240, leaking the event loop.Proposed fix — wrap in try/finally like the other routes
try: loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) - loop.run_until_complete(self._oauth.handle_redirect(code, user_id, state)) - loop.close() + try: + loop.run_until_complete(self._oauth.handle_redirect(code, user_id, state)) + finally: + loop.close() except Exception as e: return f"Authentication failed: {str(e)}", 400
🧹 Nitpick comments (2)
kinde_flask/framework/flask_framework.py (2)
236-237: Inconsistent use ofasyncio.set_event_loop()across routes.The
callback(line 237) andget_user(line 281) routes callasyncio.set_event_loop(loop), butlogin,logout, andregisterdo not. If the OAuth internals ever callasyncio.get_event_loop()(rather than relying on the loop passed torun_until_complete), those routes will break. Either always set it or never set it — consistency avoids subtle async bugs.Option: extract a small helper to reduce duplication and ensure consistency
def _run_async(self, coro): """Run an async coroutine in a new event loop, then close it.""" loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) try: return loop.run_until_complete(coro) finally: loop.close()Then each route becomes e.g.
login_url = self._run_async(self._oauth.login(login_options)).Also applies to: 280-281
50-59: Temporary session directories accumulate across restarts.
tempfile.mkdtemp()creates a new directory on every application init. After restarts, old session directories (and their files) are orphaned in the system temp folder. This is fine for development (the warning steers users toward setting the env var), but worth noting for anyone running this withoutSESSION_FILE_DIRset for an extended period.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_flask/framework/flask_framework.py (1)
205-206:⚠️ Potential issue | 🟡 Minor
os.getenv("KINDE_REDIRECT_URI")can beNone, causingAttributeErroron Line 206.If
KINDE_REDIRECT_URIis unset,redirect_url.replace(...)throws anAttributeError. The outerexcept Exceptioncatches it, but the user gets a confusing "Error parsing reauth state" message instead of a clear configuration error.Suggested guard
redirect_url = os.getenv("KINDE_REDIRECT_URI") + if not redirect_url: + return "KINDE_REDIRECT_URI environment variable is not set", 500 base_url = redirect_url.replace("/callback", "")
🤖 Fix all issues with AI agents
In `@kinde_flask/framework/flask_framework.py`:
- Around line 148-167: The failing CI is due to asyncio.set_event_loop rejecting
Mock objects returned by asyncio.new_event_loop in tests; update the _run_async
helper to avoid calling asyncio.set_event_loop (keep creating a fresh loop with
asyncio.new_event_loop, run loop.run_until_complete(coro) and then close it) so
tests that mock new_event_loop don't need an AbstractEventLoop spec; if you must
keep set_event_loop instead, update tests to return a mock that is an instance
of asyncio.AbstractEventLoop (e.g., MagicMock(spec=asyncio.AbstractEventLoop))
so asyncio.set_event_loop accepts it.
- Line 3: The package imports flask_session (from flask_session import Session)
in kinde_flask/framework/flask_framework.py but the dependency is missing from
the published package; update kinde_flask/pyproject.toml by adding
flask-session>=0.5.0 to the dependencies array so installations include this
runtime dependency (add the exact package name "flask-session" with the version
specifier to the dependencies list).
🧹 Nitpick comments (1)
kinde_flask/framework/flask_framework.py (1)
50-59: Temp session directory leaks on every app restart.Each
FlaskFramework()instantiation creates a newmkdtempdirectory. On repeated restarts (e.g., dev with auto-reload), orphaned directories accumulate in/tmp. Consider using a deterministic path (e.g.,os.path.join(tempfile.gettempdir(), 'kinde_flask_sessions')) as the fallback so restarts reuse the same directory and existing sessions survive.Suggested approach
- session_file_dir = tempfile.mkdtemp(prefix='kinde_flask_sessions_') - os.chmod(session_file_dir, 0o700) + session_file_dir = os.path.join(tempfile.gettempdir(), 'kinde_flask_sessions') + os.makedirs(session_file_dir, mode=0o700, exist_ok=True)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@testv2/testv2_framework/test_flask_framework.py`:
- Around line 135-137: Replace the bare asserts in the test method in
test_flask_framework.py with unittest assertion methods: change "assert
mock_chmod.call_count >= 1" to "self.assertGreaterEqual(mock_chmod.call_count,
1)" and change "assert first_call == call(test_temp_dir, 0o700)" to
"self.assertEqual(first_call, call(test_temp_dir, 0o700))"; apply the same
replacements at the other occurrence (the block around lines 508–510) so the
test uses self.assert* for consistent failure reporting.
🧹 Nitpick comments (4)
testv2/testv2_framework/test_flask_framework.py (4)
14-56: Extract a shared base test class to eliminate duplicated setUp/tearDown.The setUp/tearDown logic is copy-pasted across all four test classes with only minor variations (whether env vars are cleared or pre-set). A base class would centralize framework tracking, env-var snapshot/restore, and temp-dir cleanup.
♻️ Sketch of a shared base class
class FlaskFrameworkTestBase(unittest.TestCase): """Shared setUp/tearDown for FlaskFramework tests.""" # Subclasses override to customise env before each test clear_env_keys = ('SECRET_KEY', 'SESSION_TYPE', 'SESSION_FILE_DIR') preset_env: dict = {} # e.g. {'SECRET_KEY': 'test_secret_key'} create_test_session_dir = False # True when tests need self.test_session_dir def setUp(self): self.original_env = dict(os.environ) for key in self.clear_env_keys: os.environ.pop(key, None) for key, value in self.preset_env.items(): os.environ[key] = value self.test_session_dir = ( tempfile.mkdtemp(prefix='test_flask_sessions_') if self.create_test_session_dir else None ) self.created_frameworks = [] def tearDown(self): session_dir = os.environ.get('SESSION_FILE_DIR') for fw in self.created_frameworks: if hasattr(fw, 'app') and fw.app: d = fw.app.config.get('SESSION_FILE_DIR') if d and os.path.exists(d): shutil.rmtree(d, ignore_errors=True) if session_dir and os.path.exists(session_dir): shutil.rmtree(session_dir, ignore_errors=True) if self.test_session_dir and os.path.exists(self.test_session_dir): shutil.rmtree(self.test_session_dir, ignore_errors=True) # Restore env for key in set(os.environ) - set(self.original_env): del os.environ[key] os.environ.update(self.original_env)Also applies to: 189-236, 342-386, 459-494
96-104: Unusedmock_session(andmock_set_event_loop) parameters are intentional intercepts — consider prefixing with_for clarity.These mocks exist solely to prevent
Session()(orasyncio.set_event_loop) from executing real code. Prefixing them with_(e.g.,_mock_session) signals this intent and silences linter warnings (Ruff ARG002).Also applies to: 107-114, 178-183, 240-260, 264-287, 291-311, 316-336, 422-428, 499-510
496-510:frameworkon line 504 is not added toself.created_frameworks.Since
mkdtempandchmodare mocked here, no real directory is created, so this is not a resource leak. However, for consistency with every other test in this file, either add it toself.created_frameworksor assign to_to indicate the variable is intentionally unused.- framework = FlaskFramework() + _framework = FlaskFramework()
30-56:shutil.rmtreeerrors are silently swallowed — considerignore_errors=Trueinstead of bareexcept.
shutil.rmtree(path, ignore_errors=True)achieves the same "don't fail on cleanup" behavior without triggering Ruff S110/BLE001 warnings and is more idiomatic.♻️ Example (apply in all tearDown methods)
- try: - shutil.rmtree(session_dir) - except Exception: - pass # Ignore cleanup errors + shutil.rmtree(session_dir, ignore_errors=True)Also applies to: 204-236, 354-386, 472-494
|
@CodeRabbit resume |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
… quality - Extract BaseFlaskFrameworkTest base class with common setUp/tearDown logic to eliminate ~100 lines of duplicated code across 4 test classes - Replace try/except blocks with idiomatic shutil.rmtree(ignore_errors=True) - Prefix unused mock parameters with _ to signal intentional non-use and silence linter warnings (18 parameters updated) - Fix framework tracking in test_session_directory_permissions by renaming to _framework (mocks prevent real directory creation)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@kinde_fastapi/examples/example_app.py`:
- Around line 136-138: Remove the unused exception variable in the except clause
inside the protected route that logs and redirects (currently written as "except
Exception as e:"); change it to "except Exception:" so Ruff won't flag F841—keep
the call to logger.exception("Error getting user info in protected route") and
the subsequent return RedirectResponse("/login") unchanged.
🧹 Nitpick comments (2)
kinde_fastapi/examples/example_app.py (1)
67-67: Sync OAuth calls insideasync defhandlers will block the event loop.
kinde_oauth.is_authenticated()andkinde_oauth.get_user_info()are synchronous methods (per the SDK'sbase_oauth.py/smart_oauth.py). Calling them fromasync defroute handlers blocks the FastAPI event loop. For an example app with light traffic this is unlikely to cause issues, but it sets a pattern users will copy.Consider either making the route handlers plain
def(FastAPI will run them in a threadpool automatically) or keepingasync defand wrapping the sync calls inasyncio.to_thread().Simplest fix: use plain def
-async def home(): +def home():-async def protected_route(): +def protected_route():Also applies to: 122-122
testv2/testv2_framework/test_flask_framework.py (1)
104-105: List comprehension variable shadows the importedcallfromunittest.mock.The loop variable
callin the comprehension shadows thecallimported on line 5. While it works here becauseunittest.mock.callisn't needed inside the comprehension, it's a readability trap and would cause a subtle bug if the comprehension body ever neededmock.call. Same issue on lines 174–175.Suggested rename
- warning_calls = [call for call in mock_logger.warning.call_args_list - if 'SECRET_KEY' in str(call)] + warning_calls = [c for c in mock_logger.warning.call_args_list + if 'SECRET_KEY' in str(c)]Apply the same rename at lines 174–175.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@kinde_fastapi/examples/example_app.py`:
- Line 67: The async FastAPI route handler home() is calling synchronous methods
is_authenticated() and get_user_info() (which uses requests) and thus blocks the
event loop; either convert the route to a synchronous handler (change async def
home to def home) so FastAPI runs it in a threadpool, or switch to the
async-friendly OAuth API: replace the sync OAuth usage with
SmartOAuth/AsyncOAuth and call await oauth.get_user_info_async() and await
oauth.is_authenticated_async() (or equivalent async methods) inside home();
update imports and any instantiation (e.g., replace OAuth with
SmartOAuth/AsyncOAuth) and ensure all calls inside home() are await'ed if you
choose the async approach.
| # Example home route | ||
| @app.get("/", response_class=HTMLResponse) | ||
| async def home(request: Request): | ||
| async def home(): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t py -name "*.py" | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 300
🏁 Script executed:
# Find the OAuth class definition
rg -n "class OAuth" --type=py -A 5Repository: kinde-oss/kinde-python-sdk
Length of output: 1245
🏁 Script executed:
# Look for is_authenticated and get_user_info methods
rg -n "def is_authenticated|async def is_authenticated" --type=py -B 2 -A 5Repository: kinde-oss/kinde-python-sdk
Length of output: 4207
🏁 Script executed:
# Check the example_app.py file
find . -name "example_app.py" -type fRepository: kinde-oss/kinde-python-sdk
Length of output: 147
🏁 Script executed:
cat -n ./kinde_fastapi/examples/example_app.py | sed -n '60,75p'Repository: kinde-oss/kinde-python-sdk
Length of output: 639
🏁 Script executed:
cat -n ./kinde_fastapi/examples/example_app.py | sed -n '115,130p'Repository: kinde-oss/kinde-python-sdk
Length of output: 575
🏁 Script executed:
# Check the entire structure of oauth.py to understand the class better
cat -n kinde_sdk/auth/oauth.py | head -100Repository: kinde-oss/kinde-python-sdk
Length of output: 4493
🏁 Script executed:
# Look for get_user_info method
rg -n "def get_user_info" --type=py -B 2 -A 8Repository: kinde-oss/kinde-python-sdk
Length of output: 4979
🏁 Script executed:
# Check what _initialize_framework does
cat -n kinde_sdk/auth/oauth.py | sed -n '96,150p'Repository: kinde-oss/kinde-python-sdk
Length of output: 2775
🏁 Script executed:
# Look for SmartOAuth class and when it's used
rg -n "class SmartOAuth\|SmartOAuth(" --type=py -B 2 -A 3Repository: kinde-oss/kinde-python-sdk
Length of output: 178
🏁 Script executed:
# Check FrameworkFactory to see what it returns for fastapi
rg -n "fastapi\|SmartOAuth" kinde_sdk/core/framework/ --type=py -B 2 -A 5Repository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
# Search for SmartOAuth class definition
rg -n "class SmartOAuth" --type=pyRepository: kinde-oss/kinde-python-sdk
Length of output: 131
🏁 Script executed:
# Search for where SmartOAuth is instantiated
rg -n "SmartOAuth" --type=pyRepository: kinde-oss/kinde-python-sdk
Length of output: 7399
🏁 Script executed:
# Check the FrameworkFactory implementation
cat -n kinde_sdk/core/framework/framework_factory.py | head -80Repository: kinde-oss/kinde-python-sdk
Length of output: 3413
🏁 Script executed:
# Look at the complete OAuth class to see if it returns SmartOAuth somewhere
rg -n "SmartOAuth\|return " kinde_sdk/auth/oauth.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
# Check how the example_app imports OAuth
head -n 20 kinde_fastapi/examples/example_app.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 768
🏁 Script executed:
# Look at the SmartOAuth implementation to understand its behavior
cat -n kinde_sdk/auth/smart_oauth.py | head -120Repository: kinde-oss/kinde-python-sdk
Length of output: 5543
🏁 Script executed:
# Check what the OAuth.is_authenticated actually does - does it perform I/O?
cat -n kinde_sdk/auth/oauth.py | sed -n '152,170p'Repository: kinde-oss/kinde-python-sdk
Length of output: 951
🏁 Script executed:
# Check what get_user_info does
cat -n kinde_sdk/auth/oauth.py | sed -n '171,200p'Repository: kinde-oss/kinde-python-sdk
Length of output: 1459
🏁 Script executed:
# Check the user_session.py to understand what happens in is_authenticated and get_user_info
cat -n kinde_sdk/auth/user_session.py | sed -n '1,50p'Repository: kinde-oss/kinde-python-sdk
Length of output: 2524
🏁 Script executed:
# Look at the actual implementation in user_session
cat -n kinde_sdk/auth/user_session.py | sed -n '130,170p'Repository: kinde-oss/kinde-python-sdk
Length of output: 1939
🏁 Script executed:
# Check what get_user_details_sync does - this is called in get_user_info
rg -n "def get_user_details_sync" --type=py -A 20Repository: kinde-oss/kinde-python-sdk
Length of output: 1439
🏁 Script executed:
# Check token_manager.get_access_token() implementation
rg -n "def get_access_token" --type=py -A 15Repository: kinde-oss/kinde-python-sdk
Length of output: 2981
🏁 Script executed:
# Get the full implementation of get_user_details_sync
cat -n kinde_sdk/core/helpers.py | sed -n '698,780p'Repository: kinde-oss/kinde-python-sdk
Length of output: 1658
🏁 Script executed:
# Check what refresh_access_token does in token_manager
cat -n kinde_sdk/auth/token_manager.py | sed -n '138,180p'Repository: kinde-oss/kinde-python-sdk
Length of output: 1944
🏁 Script executed:
# Check if there's any documentation or guidance about using OAuth with FastAPI
find . -name "README*" -o -name "*.md" | xargs grep -l "FastAPI\|async" 2>/dev/null | head -5Repository: kinde-oss/kinde-python-sdk
Length of output: 154
🏁 Script executed:
# Look at the kinde_fastapi package to see if there's any wrapper or guidance
ls -la kinde_fastapi/Repository: kinde-oss/kinde-python-sdk
Length of output: 539
🏁 Script executed:
# Check if there's a __init__.py or documentation in kinde_fastapi
cat kinde_fastapi/__init__.pyRepository: kinde-oss/kinde-python-sdk
Length of output: 540
🏁 Script executed:
# Check the ASYNC_SYNC_GUIDE for guidance on using with FastAPI
cat ASYNC_SYNC_GUIDE.md | head -100Repository: kinde-oss/kinde-python-sdk
Length of output: 2628
🏁 Script executed:
# Check what OAuth class actually returns when instantiated with framework="fastapi"
# Look to see if there's any wrapper or factory behavior
rg -n "def __new__\|return SmartOAuth\|return AsyncOAuth" kinde_sdk/auth/oauth.py --type=pyRepository: kinde-oss/kinde-python-sdk
Length of output: 52
🏁 Script executed:
# Double-check what is actually imported/used in the example
grep -n "from\|import\|OAuth" kinde_fastapi/examples/example_app.py | head -20Repository: kinde-oss/kinde-python-sdk
Length of output: 893
🏁 Script executed:
# Check the FastAPIFramework to see if it does any special wrapping
cat -n kinde_fastapi/framework/fastapi_framework.py | head -80Repository: kinde-oss/kinde-python-sdk
Length of output: 2965
🏁 Script executed:
# Verify one more time: what does OAuth() constructor actually return?
# Is there any __new__ method or factory pattern in OAuth class?
cat -n kinde_sdk/auth/oauth.py | sed -n '20,50p'Repository: kinde-oss/kinde-python-sdk
Length of output: 1401
Use async-safe methods for async FastAPI handlers — or switch to def routes.
The example calls synchronous is_authenticated() and get_user_info() inside async def route handlers. The get_user_info() method makes blocking HTTP requests via the synchronous requests library, which blocks the event loop and degrades concurrency.
Per the ASYNC_SYNC_GUIDE, for FastAPI use either:
SmartOAuthwithawait oauth.get_user_info_async()in async handlersAsyncOAuthwith async-only methods- Plain
defroutes (notasync def) to let FastAPI run them in a thread pool
Currently the example uses the sync OAuth class, which is intended for Flask, not FastAPI.
Also applies to: 122-122
🤖 Prompt for AI Agents
In `@kinde_fastapi/examples/example_app.py` at line 67, The async FastAPI route
handler home() is calling synchronous methods is_authenticated() and
get_user_info() (which uses requests) and thus blocks the event loop; either
convert the route to a synchronous handler (change async def home to def home)
so FastAPI runs it in a threadpool, or switch to the async-friendly OAuth API:
replace the sync OAuth usage with SmartOAuth/AsyncOAuth and call await
oauth.get_user_info_async() and await oauth.is_authenticated_async() (or
equivalent async methods) inside home(); update imports and any instantiation
(e.g., replace OAuth with SmartOAuth/AsyncOAuth) and ensure all calls inside
home() are await'ed if you choose the async approach.
| # Example home route | ||
| @app.get("/", response_class=HTMLResponse) | ||
| async def home(request: Request): | ||
| async def home(): |
There was a problem hiding this comment.
I'm going to echo coderabbit: home() is declared async def but calls synchronous methods kinde_oauth.is_authenticated() and kinde_oauth.get_user_info(). Tracing through the SDK source:
OAuth.get_user_info()->get_user_details_sync()inkinde_sdk/core/helpers.py-> callsrequests.get()(blocking I/O)OAuth.is_authenticated()-> synchronous session lookup (fast, but still sync)
My thought: Change async def home() to def home(). FastAPI automatically runs plain def route handlers in a thread pool, which safely accommodates the synchronous requests library calls. No other code changes required - just remove the async keyword.
| @app.get("/logout") | ||
| async def logout(): | ||
| @app.get("/protected") | ||
| async def protected_route(): |
There was a problem hiding this comment.
Same issue as home() - protected_route() is async def but calls synchronous kinde_oauth.is_authenticated() and kinde_oauth.get_user_info() which block the event loop.
Change to def protected_route() for the same reason as the home route.
| # Create a secure temporary directory with restrictive permissions (0700) | ||
| session_file_dir = tempfile.mkdtemp(prefix='kinde_flask_sessions_') | ||
| os.chmod(session_file_dir, 0o700) | ||
| logger.warning( | ||
| f"SESSION_FILE_DIR not set. Using temporary directory: {session_file_dir}. " |
There was a problem hiding this comment.
Temp session directories accumulate across restarts. tempfile.mkdtemp() creates a new unique directory on every FlaskFramework() instantiation. In development with auto-reload (--reload), each restart orphans the previous session directory (and its files) in /tmp.
Consider using a deterministic fallback path instead, so restarts reuse the same directory and existing sessions survive:
session_file_dir = os.path.join(tempfile.gettempdir(), 'kinde_flask_sessions')
os.makedirs(session_file_dir, mode=0o700, exist_ok=True)This preserves the security posture (0o700 permissions) while avoiding directory proliferation. The warning message steering users toward setting SESSION_FILE_DIR for production is good - this just improves the dev experience.
3 participants
fix(flask,fastapi): updated examples as reported in Bug 129 (#129)
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.