Fix/mobile oauth redirect#60
Conversation
Enhance pre-commit checks and add contact_messages to schema
Implement email update functionality with verification process
Refine deployment triggers and health checks for staging and production
Fix health check URL for staging deployment
Add Brevo API transport with Gmail SMTP fallback for email
Fix environment variable handling and set defaults for Brevo API
Implement guest session management, audit logging, and WebSocket chat
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extends the OAuth login flow to support optional custom redirect URLs. Clients supply a ChangesOAuth State Custom Redirect URL Support
Sequence DiagramsequenceDiagram
participant Client
participant google_login
participant create_oauth_state
participant JWTToken as OAuth State (JWT)
participant GoogleOAuth as Google OAuth
participant google_callback
participant decode_oauth_state
participant FrontendURL as Redirect Target
Client->>google_login: GET /auth/google?return_url=mobile://deep-link
google_login->>create_oauth_state: create_oauth_state(return_url)
create_oauth_state->>JWTToken: encode return_url in JWT
JWTToken-->>google_login: state param
google_login-->>GoogleOAuth: redirect to Google with state
GoogleOAuth->>google_callback: callback with state & authorization_code
google_callback->>decode_oauth_state: decode_oauth_state(state)
decode_oauth_state-->>google_callback: OAuthStatePayload(return_url)
alt return_url present
google_callback->>FrontendURL: redirect to return_url?access_token=...
else return_url absent
google_callback->>FrontendURL: redirect to default URL?access_token=...
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/v1/endpoints/auth.py`:
- Around line 419-420: The redirect assembly currently concatenates a query
string using base_redirect and urlencode (seen in base_redirect,
oauth_ctx.return_url, settings.FRONTEND_AUTH_CALLBACK_URL, app_access_token, and
redirect_url), which breaks if base_redirect already contains query params;
instead parse base_redirect with a URL parser (e.g., urllib.parse.urlparse /
parse_qs / urlencode), merge or update its query parameters to include
access_token, rebuild the URL with urlunparse, and assign that to redirect_url
so existing params are preserved and the access_token is appended safely.
In `@app/services/oauth_state.py`:
- Around line 53-54: The current code unconditionally stores client-supplied
return_url into payload["return_url"], enabling open-redirect/token
exfiltration; change this to validate the sanitized return_url against a trusted
whitelist before storing. Parse return_url (e.g., urllib.parse.urlparse), ensure
scheme is http/https, and verify host/netloc (or full origin) is in a configured
TRUSTED_REDIRECTS/ALLOWED_CALLBACKS set or matches allowed patterns; only then
assign payload["return_url"] = validated_return_url[:500], otherwise omit it or
set a safe default. Apply this check where return_url is handled (the variable
named return_url and payload map in oauth_state.py).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3652cae-e2f9-4914-9def-de4f163edb83
📒 Files selected for processing (2)
app/api/v1/endpoints/auth.pyapp/services/oauth_state.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/conftest.py`:
- Line 4: Replace the developer-specific hardcoded DATABASE_URL in
tests/conftest.py (the os.environ.setdefault("DATABASE_URL", ... ) call) with a
non-personal, generic test default or remove the default to require CI/dev
injection; for example use an in-memory SQLite URL or a clearly generic Postgres
placeholder (e.g. "postgresql+asyncpg://user:pass@localhost:5432/testdb") so
tests don't rely on personal credentials and CI must supply the real
DATABASE_URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1acf6b46-8a70-4dd3-993d-09c3c0696e1c
📒 Files selected for processing (4)
app/api/v1/endpoints/auth.pyapp/services/oauth_state.pytests/conftest.pytests/test_oauth_state.py
Description
This PR fixes the Google OAuth mobile authentication flow for the Clinsight mobile app.
The backend previously handled authentication using a web-only OAuth flow and ignored the provided mobile redirect_uri / return_url. As a result, after Google authentication, the callback returned backend JSON responses instead of redirecting back to the mobile app deep link.
This update adds support for mobile OAuth redirects by properly handling the mobile return URL and redirecting authenticated users back to the app using the configured deep link scheme.
Related Issue (Link to issue ticket)
Motivation and Context
The mobile app could not complete openAuthSessionAsync because the backend callback endpoint returned JSON instead of redirecting to the mobile deep link with authentication tokens.This change ensures the OAuth flow works correctly for mobile authentication by redirecting users back to:
clinsight://auth/google?access_token=...
after successful authentication.
How Has This Been Tested?- Ran linting and formatting checks using:
uv run ruff check . --fix
Screenshots (if appropriate - Postman, etc):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes / Security
Tests