-
Notifications
You must be signed in to change notification settings - Fork 63
fix(widget): persist identified session tokens to survive page disruptions #332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9d7acd6
3f194b5
a8713d3
234f65c
8bf8570
8511040
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,9 @@ import { | |
| persistAnonymousToken, | ||
| readPersistedToken, | ||
| clearPersistedToken, | ||
| persistIdentifiedToken, | ||
| readPersistedIdentifiedToken, | ||
| clearIdentifiedToken, | ||
| } from '@/lib/client/widget-auth' | ||
| import { sendToHost } from '@/lib/client/widget-bridge' | ||
| import { widgetQueryKeys } from '@/lib/client/hooks/use-widget-vote' | ||
|
|
@@ -142,6 +145,31 @@ export function WidgetAuthProvider({ | |
|
|
||
| const p = (async (): Promise<boolean> => { | ||
| try { | ||
| // 0. Try a persisted identified token first — it may have survived a | ||
| // page interaction (tab switch, focus loss, browser memory pressure) | ||
| // that cleared the in-memory token but not localStorage. | ||
| const persistedIdent = readPersistedIdentifiedToken() | ||
| if (persistedIdent) { | ||
| try { | ||
| const res = await fetch('/api/widget/session', { | ||
| headers: { Authorization: `Bearer ${persistedIdent}` }, | ||
| }) | ||
| if (res.ok) { | ||
| const body = await res.json() | ||
| if (body.data?.user) { | ||
| storeToken(persistedIdent) | ||
| persistIdentifiedToken(persistedIdent) | ||
| setUser(body.data.user) | ||
| return true | ||
| } | ||
| // Identified token is stale or demoted — drop it. | ||
| clearIdentifiedToken() | ||
| } | ||
| } catch { | ||
| // Server unreachable — fall through. | ||
| } | ||
| } | ||
|
|
||
| // 1. Prefer a persisted anonymous token — validate it server-side | ||
| // before adopting (it may have expired or been merged away). | ||
| const persisted = readPersistedToken() | ||
|
|
@@ -214,9 +242,11 @@ export function WidgetAuthProvider({ | |
| (result: { sessionToken: string; user: WidgetUser; votedPostIds?: string[] }) => { | ||
| storeToken(result.sessionToken) | ||
| // Any anonymous session was merged into this identified user server-side, | ||
| // so drop its persisted token. Identified tokens are never persisted — | ||
| // they're re-established via SDK identify / portal passthrough on load. | ||
| // so drop its persisted token. Persist the identified token so it survives | ||
| // page interactions (e.g. the user navigates away and returns to the | ||
| // widget tab) — the server validates it as a Bearer on next use. | ||
| clearPersistedToken() | ||
| persistIdentifiedToken(result.sessionToken) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Persisting the identified token here makes it survive a reload, but the existing Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already addressed — |
||
| setUser(result.user) | ||
| if (result.votedPostIds) { | ||
| queryClient.setQueryData( | ||
|
|
@@ -287,17 +317,17 @@ export function WidgetAuthProvider({ | |
| } | ||
| }, [portalSessionToken, portalUser, storeToken]) | ||
|
|
||
| // Restore a persisted anonymous session on mount so a returning visitor's | ||
| // conversation is visible immediately, without waiting for a write. Skipped | ||
| // when a portal session is present (it takes precedence) or one is already | ||
| // ready. Restore-only: never mints — the first write lazily mints if nothing | ||
| // valid was restored. | ||
| // Restore a persisted session (anonymous or identified) on mount so a | ||
| // returning visitor's conversation is visible immediately, without waiting | ||
| // for a write. Skipped when a portal session is present (it takes | ||
| // precedence) or one is already ready. Restore-only: never mints — the | ||
| // first write lazily mints if nothing valid was restored. | ||
| const restoreAttemptedRef = useRef(false) | ||
| useEffect(() => { | ||
| if (restoreAttemptedRef.current) return | ||
| if (portalSessionToken || sessionReadyRef.current) return | ||
| restoreAttemptedRef.current = true | ||
| if (!readPersistedToken()) return | ||
| if (!readPersistedToken() && !readPersistedIdentifiedToken()) return | ||
| void acquireSession(false) | ||
| }, [portalSessionToken, acquireSession]) | ||
|
|
||
|
|
@@ -354,6 +384,12 @@ export function WidgetAuthProvider({ | |
| async function handleAnonymousIdentify() { | ||
| // Don't eagerly create anonymous session — it will be created lazily | ||
| // on first write action (vote, comment, post) via ensureSessionThen. | ||
| // Clear both the in-memory token and any persisted identified token so | ||
| // a previously-identified visitor who now loads the host app logged | ||
| // out cannot have their old identity restored by an in-flight or | ||
| // completed token restore and continue writing as the prior user. | ||
| clearWidgetToken() | ||
| sessionReadyRef.current = false | ||
| setUser(null) | ||
| sendToHost({ type: 'quackback:identify-result', success: true, user: null }) | ||
| sendToHost({ type: 'quackback:auth-change', user: null }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| "widget.home.form.errorEmail": "Could not verify your email. Please try again.", | ||
| "widget.home.form.errorSession": "Could not create session. Please try again.", | ||
| "widget.home.form.errorNetwork": "Network error. Please try again.", | ||
| "widget.home.form.errorReauthRequired": "Your session has expired. Please sign in again to submit feedback.", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Adding a new English message id without entries in the supported non-English catalogs violates the locale parity test ( Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already addressed — |
||
| "widget.home.similar.heading": "Similar ideas", | ||
| "widget.home.popular.heading": "Popular ideas", | ||
| "widget.home.popular.search.placeholder": "Search ideas...", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This identified-token restore only runs from
acquireSession(), but the mount restore effect below still returns unlessreadPersistedToken()finds an anonymous token. In the main new case where SSO persisted only an identified token, the provider never restores it on mount, sosessionVersionstays on the anonymous baseline and permission-gated submit/vote UI can remain disabled or route to login before the submit fallback can repair it.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already addressed — the mount useEffect at line 330 now checks both
readPersistedToken()ANDreadPersistedIdentifiedToken()before returning early. This was fixed in commit 8bf8570.