Feat/mobile auth - #779
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughMobile login now uses a non-cacheable HTML handoff before opening the app. Native logout now performs IdP logout, clears the server session, and uses configurable callback schemes. Backend tests cover callback state, scheme validation, and stale handoff data. ChangesMobile authentication flows
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/mobile.md`:
- Around line 112-124: Update the later “Logout keeps the IdP session alive”
section to describe RP-initiated browser logout through OIDCLogoutView as the
nominal mobile flow, and identify /mobile/auth/logout/ with MobileLogoutView as
the local-session fallback. Remove the outdated claim that nativeLogout() skips
/logout/ and that IdP logout is only a follow-up.
In `@Makefile`:
- Around line 787-792: Replace the git commit-count default in Makefile’s
MOBILE_VERSION_CODE definition with a required CI-managed persistent
per-application release sequence, failing release builds when it is unset while
preserving explicit overrides. Update docs/env.md at the documented
environment-variable entry to remove the commit-count monotonicity claim and
describe the required persistent source.
In `@src/backend/core/templates/core/mobile_handoff.html`:
- Line 10: Update the root <html> element in the mobile handoff template to
include lang="fr", preserving the existing document structure.
In `@src/frontend/android/app/build.gradle`:
- Around line 15-20: Update the signing configuration setup around signingValue
and hasSigningConfig to resolve and require all four signing values—keystore
path, store password, key alias, and key password—before treating signing as
configured. Keep release signing unattached for partial credentials, and update
the release guard to report the specific missing signing keys rather than only
detecting a missing keystore path.
In `@src/frontend/src/features/native/auth.ts`:
- Around line 103-107: Update the logout request flow in the try block to store
the fetch response, validate response.ok, and throw when the server returns a
non-2xx status before clearing local cookies or reloading. Preserve the existing
warning/error path so failed logout responses do not report successful
completion.
🪄 Autofix
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 Plus
Run ID: 349cb1a6-8f44-455e-8d7a-5c12833f5042
📒 Files selected for processing (23)
Makefiledeploy/env/frontend.defaultsdocs/env.mddocs/mobile.mdsrc/backend/core/api/viewsets/mobile_auth.pysrc/backend/core/authentication/urls.pysrc/backend/core/authentication/views.pysrc/backend/core/templates/core/mobile_handoff.htmlsrc/backend/core/tests/authentication/test_logout.pysrc/backend/core/tests/authentication/test_mobile_auth.pysrc/frontend/android/.gitignoresrc/frontend/android/app/build.gradlesrc/frontend/android/app/src/main/AndroidManifest.xmlsrc/frontend/android/app/src/main/res/values/strings.xmlsrc/frontend/capacitor.config.tssrc/frontend/ios/.gitignoresrc/frontend/ios/App/App.xcodeproj/project.pbxprojsrc/frontend/ios/App/App/Info.plistsrc/frontend/scripts/generate-ios-xcconfig.mjssrc/frontend/src/features/native/auth.tssrc/frontend/src/features/native/sso-invariants.test.tssrc/frontend/src/vite-env.d.tssrc/frontend/vite.config.ts
4dcb866 to
9adf176
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@src/backend/core/authentication/views.py`:
- Around line 203-207: Bind the mobile logout marker to the generated OIDC
logout state in the flow that stores MOBILE_LOGOUT_SESSION_KEY, preserving the
scheme and timestamp. In the callback logic consuming MOBILE_LOGOUT_SESSION_KEY,
pop and use it only when the callback state matches the stored state, rather
than for any valid oidc_states entry. Add a regression test covering an earlier
valid web state followed by a later mobile logout state, ensuring the mobile
callback redirects to its scheme logout URL.
🪄 Autofix
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 Plus
Run ID: 1999d85a-828e-41b4-8c50-2ea9eb73750d
📒 Files selected for processing (3)
docs/mobile.mdsrc/backend/core/authentication/views.pysrc/backend/core/tests/authentication/test_logout.py
A staging and a production build must be installable side by side on one device, and two apps claiming the same OIDC deep-link scheme would make Android ask the user which one receives the login callback, mid-flow. The app id, the displayed name and the callback scheme therefore become per-environment (MOBILE_APP_ID / MOBILE_APP_NAME / MOBILE_AUTH_SCHEME).
9adf176 to
5c7aea7
Compare
ProConnect ignores prompt=login, so preserving the IdP session locked mobile users into the same identity forever. The logout endpoint now accepts a mobile_scheme and ends the RP-initiated round-trip on a new logout-callback view that deep-links back to the app, so the system browser — which holds both the Django session handed over at login and the IdP SSO cookie — terminates both sessions. Then, Proconnect login page's Content Security Policy blocks the direct redirect: Chrome enforces its form-action on the whole redirect chain of the credential form submission, and "*" only matches network schemes — so our network mobile scheme violates it and the user stays stuck on the identity provider during logout workflow. The callback now serves a page that ends the form chain on a network mobile scheme, then hands off to the app from our own page, outside the IdP policy: automatically via script (iOS interception, unchanged) with a button as the always-working fallback.
5c7aea7 to
3b888a3
Compare
Purpose
Adapt auth workflow for mobile flow
Summary by CodeRabbit
New Features
Bug Fixes
Documentation