fix(account): silent re-auth on user info error instead of forcing login#8785
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Account Center’s re-authentication behavior when /api/my-account fails: it now attempts a silent OIDC re-auth (prompt=none) first, and only forces an interactive login (prompt=login) if the authorization server returns error=login_required. This better distinguishes “stale local tokens” from “no valid server-side OIDC session,” addressing issue #8657 while keeping the stale-state cleanup flow intact.
Changes:
- Add a two-phase re-auth flow in
App.tsx: try silent auth onuserInfoError, then fall back to explicit login whenerror=login_requiredis observed on the next load. - Add a changeset documenting the behavioral change for
@logto/account.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/account/src/App.tsx | Switch userInfoError handling to attempt Prompt.None first, with a login_required URL-detected fallback to Prompt.Login. |
| .changeset/silent-account-reauth.md | Patch changeset describing silent re-auth + fallback behavior and rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9dd196f to
5c62523
Compare
COMPARE TO
|
| Name | Diff |
|---|---|
| .changeset/silent-account-reauth.md | 📈 +591 Bytes |
| packages/account/jest.config.ts | 📈 +49 Bytes |
| packages/account/package.json | 📈 +89 Bytes |
| packages/account/src/App.test.tsx | 📈 +5.27 KB |
| packages/account/src/App.tsx | 📈 +1.08 KB |
| packages/account/src/Callback.test.tsx | 📈 +1.42 KB |
| packages/account/src/jest.setup.ts | 📈 +1.02 KB |
| packages/account/src/use-auth-redirect.ts | 📈 +3.86 KB |
| pnpm-lock.yaml | 📈 +849 Bytes |
|
Thanks for the detailed follow-up and the implementation. I think the direction makes sense for the clarified user-switch scenario: Account Center may have stale tokens for User A while the browser already has a valid OIDC session for User B, and trying Could you update the PR / issue wording to focus on that scenario rather than ordinary access-token expiry? With It would also be great to add test coverage for the new two-step flow, especially:
If adding those tests is inconvenient in this PR, I can also take care of the test coverage separately. |
|
Thanks for the review! I'll update both the PR and the issue wording, and add the tests as well. |
d6bc888 to
0e9a787
Compare
Use prompt=none to let OIDC silently re-authenticate via the existing session cookie when /api/my-account returns a 401 (typically due to an expired access token or a revoked token from a user switch). Only fall back to prompt=login when the provider answers with login_required, which signals that no valid session exists. This preserves the stale-state cleanup invariant from logto-io#8313 / logto-io#8554 / via the code callback), while removing the visible login screen that previously appeared on every access token expiry. Refs logto-io#8657 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refs logto-io#8657 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Share extraParams between the silent-failed and unauthenticated branches in the first effect, and split the second effect's guard into two early returns, to bring App.tsx back under the 300-line limit after the silent re-auth changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
App.test.tsx - userInfoError while authenticated triggers signIn with Prompt.None - ?error=login_required falls back to signIn with Prompt.Login - both branches are no-ops when accountCenterSettings.enabled === false - unauthenticated landing still kicks off a regular sign-in - auth callback in flight (?code=) skips the redirect effects Callback.test.tsx - mount clears stale tokens + verification record (preserves the cleanup invariant the silent-success callback path depends on) - error UI renders when useHandleSignInCallback reports an error Test infrastructure (account package only): - Expose Main from App.tsx for component-level testing - jest.setup.ts: polyfill structuredClone (jsdom <22) and window.matchMedia - jest.config.ts: stub non-module scss imports via identity-obj-proxy Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…edirect Move the two auth-redirect useEffect blocks out of App.tsx's Main component into a dedicated useAuthRedirect hook. App.tsx now passes only the URL-derived flags (isInCallback, isSilentAuthFailed); the hook owns the SDK and PageContext reads, the redirect URI, and the silent re-auth two-phase flow. Side effects: - App.tsx loses ~50 lines, bringing it back under the max-lines limit without needing a file-level eslint-disable. - App.test.tsx keeps working unchanged because it renders <Main /> and the hook composes naturally; the existing mocks for @logto/react cover both call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0e9a787 to
eef6f01
Compare
wangsijie
left a comment
There was a problem hiding this comment.
Overall LGTM, please take a look at Copilot's comments
- useAuthRedirect: guard the silent Prompt.None redirect with a ref so it fires at most once per page load (userInfoError stays truthy until navigation, so an unguarded effect re-dispatched signIn and kept overwriting the saved route on every dependency change). - useAuthRedirect: document that a disabled tenant short-circuits both effects and intentionally leaves ?error=login_required in the URL (harmless: App.tsx renders the disabled UI, no further redirect). - jest.setup: replace the JSON-roundtrip structuredClone shim with the faithful @ungap/structured-clone polyfill so tests don't mask cloning bugs (jest-environment-jsdom@29 does not expose structuredClone). - App.test: keep literal @logto/react enum mocks (jest.requireActual pulls in @logto/client → @logto/js which fails to resolve under the monorepo pnpm/jest layout) and explain why in a comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@copilot review |
|
Thanks! |
Summary
Fixes #8657.
When two users share a browser and switch accounts through an external Logto-backed app, the Account Center forces the second user through a login screen even though their OIDC session is already valid.
User-switch scenario. User A signs in via an external app that uses Logto as its IdP and visits the Account Center, which writes A's tokens to
localStorageon the Logto origin. A signs out (end_session), destroying A's OIDC session and revoking A's session-bound refresh token — but A'slocalStorageentries remain on the browser. User B signs in via the same external app on the same browser, which creates a fresh OIDC session for B. When B follows a link to the Account Center, the SDK reads A's stale tokens, hits/api/my-account, gets401, and the refresh attempt fails too (A's refresh token was revoked with A's session).userInfoErrorfires, and today's code redirects withprompt=login— even though B has a valid OIDC session cookie on the Logto origin.This PR distinguishes "OIDC session still valid, local token state stale" from "OIDC session gone" by delegating the decision to the authorization server: first try
prompt=none, and only fall back toprompt=loginwhen the provider answerserror=login_required.Implementation
@logto/react'ssignIn()performs a browser redirect (window.location.assignto/oidc/auth), so a naïvetry { await signIn({ prompt: Prompt.None }) } catch { signIn({ prompt: Prompt.Login }) }does not work — the page navigates away before the Promise settles. The OIDC error response (login_required) is only observable as a URL query parameter on the next page load.The fix is a two-phase flow in
App.tsx:userInfoError(while authenticated), redirect withprompt=noneso the OIDC provider attempts silent re-authentication via the session cookie.error=login_requiredin the URL parameters and fall back tosignIn({ prompt: Prompt.Login }).Callback.tsx'sclearAllTokens()+clearVerificationRecord()still run on the silent-success branch (the callback delivers a freshcodefor the actually-authenticated user), so the stale-state cleanup invariant from #8313 / #8554 / #8590 is preserved.Behavior on the target scenarios
login_requiredfallback)accountCenterSettings.enabled === false)Testing
svhd/logto:latest: with the fix, User B is silently re-authenticated and lands on the Account Center without a login screen; A'slocalStorageis cleared by the callback.prompt=nonereturnserror=login_required; the fallback path renders the sign-in screen as before.App.test.tsx(userInfoErrorwhile authenticated →Prompt.None;error=login_required→Prompt.Loginfallback;accountCenterSettings.enabled === falseregression for both branches; regular unauthenticated sign-in still fires; auth callback in flight skips the redirects) andCallback.test.tsx(mount-timeclearAllTokens()+clearVerificationRecord()invariant; error UI on callback failure).Checklist
.changeset