[feat] Sign in on mobile, and keep the session alive (7/12) - #5686
[feat] Sign in on mobile, and keep the session alive (7/12)#5686ardaerzin wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughMobile authentication now uses SuperTokens for password sign-in and session refresh. The mobile app provides an authentication page, retries project requests after session refresh, and routes desktop authentication paths to mobile authentication. ChangesMobile authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MobileApp
participant SignInScreen
participant SuperTokens
participant ProjectContext
participant API
MobileApp->>SuperTokens: initialize session and email-password recipes
MobileApp->>SignInScreen: render authentication page
SignInScreen->>SuperTokens: submit email and password
SuperTokens-->>SignInScreen: return authentication outcome
SignInScreen->>ProjectContext: invalidate project queries
ProjectContext->>API: request projects
API-->>ProjectContext: return unauthenticated response
ProjectContext->>SuperTokens: refresh session
SuperTokens-->>ProjectContext: return refresh result
ProjectContext->>API: retry project request once
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/mobile/src/features/auth/SignInScreen.tsx (1)
50-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd accessible names to the email and password fields.
The inputs have no
<label>and rely only onplaceholder. Screen readers lose the field name once the user starts typing. Addaria-label(or a visually-hidden<label>) for each input.♿ Proposed fix
<input type="email" autoComplete="email" required placeholder="Email" + aria-label="Email" value={email} onChange={(event) => setEmail(event.target.value)} className="border-border bg-background rounded-md border px-3 py-2 text-sm" /> <input type="password" autoComplete="current-password" required placeholder="Password" + aria-label="Password" value={password} onChange={(event) => setPassword(event.target.value)} className="border-border bg-background rounded-md border px-3 py-2 text-sm" />web/mobile/src/lib/auth.ts (1)
29-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the OIDC env-key list between mobile and desktop.
web/mobile/src/lib/auth.tsduplicates the OAuth client-id keys used byweb/oss/src/lib/helpers/dynamicEnv.ts::getEffectiveAuthConfig. KeepOIDC_CLIENT_ID_KEYSin a shared module and import it from both apps so added providers updategetEmailSignInMode()and desktop auth config together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fbd43ccb-1630-4578-bd25-65688080d262
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
web/mobile/package.jsonweb/mobile/src/features/app/AppProviders.tsxweb/mobile/src/features/auth/SignInScreen.tsxweb/mobile/src/features/context/states/SignedOutNotice.tsxweb/mobile/src/lib/auth.tsweb/mobile/src/lib/context.tsweb/mobile/src/pages/auth.tsxweb/packages/agenta-shared/src/utils/mobileGate/index.tsweb/packages/agenta-shared/tests/unit/mobileGate.test.ts
| /** Desktop URL → mobile equivalent (design.md "Gate and routing"). */ | ||
| export function mapDesktopToMobile(pathname: string, search: string): string { | ||
| // Mobile sign-in (auth-lite): /auth/callback never reaches here (exception). | ||
| if (/^\/auth(\/|$)/.test(pathname)) return "/m/auth" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Password-reset token is lost when routing mobile /auth/reset-password links. mapDesktopToMobile discards the search string for every /auth* path, and the test suite only verifies this with an empty search, so the realistic ?token=... case from SuperTokens' reset-password email link is never exercised and the resulting data loss goes unnoticed.
web/packages/agenta-shared/src/utils/mobileGate/index.ts#L74-L77: preservetoken/tenantIdwhen mapping/auth/reset-passwordto a mobile destination, or exclude it from mobile mapping like/auth/callback.web/packages/agenta-shared/tests/unit/mobileGate.test.ts#L90-L93: add a case assertingmapDesktopToMobile("/auth/reset-password", "?token=abc")preserves the token once the implementation is fixed.
📍 Affects 2 files
web/packages/agenta-shared/src/utils/mobileGate/index.ts#L74-L77(this comment)web/packages/agenta-shared/tests/unit/mobileGate.test.ts#L90-L93
b43b6c7 to
42873d9
Compare
fe228cc to
54e141c
Compare
42873d9 to
83782a0
Compare
54e141c to
08c72cb
Compare
83782a0 to
4822eb6
Compare
08c72cb to
d23caf8
Compare
4822eb6 to
7a90b8c
Compare
d23caf8 to
74b3f55
Compare
|
Real, fixed, and worse than described — thank you.
Preserving the query would have moved the token to a page that cannot use it. The fix follows the precedent already in this file: an OAuth callback is passed through because bouncing it drops the one-time code. A one-time Two tests added, and I verified they fail without the change: Worth noting the blast radius was bounded: the gate is off unless a deployment sets |
|
@coderabbitai review |
✅ Action performedReview finished.
|
7a90b8c to
2a51cb5
Compare
74b3f55 to
7d6b07f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
2a51cb5 to
e55669f
Compare
7d6b07f to
478248f
Compare
|
Already fixed on this branch. The finding is right about Two tests cover it ( |
/auth leaves DESKTOP_EXCEPTIONS now that /m/auth exists: mobile devices navigating desktop /auth land on the mobile sign-in via the deep-link map. /auth/callback stays a desktop exception — the OAuth/SSO redirect landing must complete on the desktop app (mobile has no SSO). The web/mobile middleware carries only the reverse-gate subset (none of the changed functions), so its verbatim copy needs no mirror; the desktop middlewares import from @agenta/shared and pick this up without edits.
An expired access token was indistinguishable from signed-out: fetchProjects
returned {kind:"unauthenticated"} on the first 401 even when a valid refresh
token cookie was sitting right there. Add a headless supertokens-web-js
client (same 0.16.x the desktop's supertokens-auth-react wraps, appInfo
mirroring web/oss/src/config/appInfo.ts) and have fetchProjects try one
Session.attemptRefreshingSession() + retry before the verdict stands.
/m/auth: thin page shell + SignInScreen with a raw email/password form against the shared SuperTokens backend (EmailPassword recipe joins the headless init). The effective email mode mirrors the desktop's getEffectiveAuthConfig — otp and disabled deployments get a notice instead of the form, and SSO/social is signposted to the desktop app. On success the cached unauthenticated projects verdict is invalidated and the root context resolver takes over. SignedOutNotice now links to /m/auth.
e55669f to
e9ce407
Compare
478248f to
823f0e5
Compare
1 participant
Context
The mobile app assumed you were already signed in. If you were not, you got a blank screen: no sign-in route, and no attempt to use the session you may already have.
Changes
/mgets a raw email sign-in page and a refresh attempt before it concludes you are signed out. The refresh matters because the access token has a short life: a phone returning from the lock screen often holds a valid refresh token and an expired access token, and without this it would show a sign-out screen to someone who is signed in.The interceptor is installed at provider scope rather than per-request, so every caller gets the retry rather than only the ones that remembered to ask.
The desktop's sign-in page learns to route a mobile device to
/m/auth(in@agenta/shared, alongside the gate), so a phone that starts at the desktop sign-in ends up in the right app.Tests / notes
What to QA
/msigned out. You get the sign-in page rather than a blank screen./min the same browser. You are already signed in./mopen past the access-token lifetime, then return to the tab. It refreshes rather than showing you a sign-out screen.