fix: stop auth=success login loop on cross-app cookie conflict#1960
Open
emlimlf wants to merge 3 commits into
Open
fix: stop auth=success login loop on cross-app cookie conflict#1960emlimlf wants to merge 3 commits into
emlimlf wants to merge 3 commits into
Conversation
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Auth0 login/callback flow to prevent an infinite silent-login redirect loop (and repeated auth=success query growth) when cross-app cookie name conflicts cause JWT validation to fail.
Changes:
- In
callback.ts, treat additional silent-auth failure cases as non-fatal redirects and strip staleauthparams before redirecting. - Ensure successful callback redirects don’t accumulate repeated
auth=successparams across redirect cycles. - Preserve the
silentLoginAttemptedguard flag across silent login redirects so the client can stop retrying.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
frontend/server/api/auth/callback.ts |
Cleans auth query handling for silent failures and success redirects to prevent redirect loops / param accumulation. |
frontend/composables/useAuth.ts |
Avoids resetting the silent-attempt flag during silent logins so retry prevention works. |
frontend/app/plugins/auth.client.ts |
Strips auth from the client redirectTo path before triggering silent login (protected file per hook). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+203
to
+210
| const successUrl = new URL(redirectTo, 'https://placeholder.example'); | ||
| successUrl.searchParams.delete('auth'); | ||
| const cleanRedirectTo = | ||
| successUrl.pathname + (successUrl.search || '') + (successUrl.hash || ''); | ||
| const finalRedirect = | ||
| redirectTo === '/' | ||
| !cleanRedirectTo || cleanRedirectTo === '/' | ||
| ? '/?auth=success' | ||
| : `${redirectTo}${redirectTo.includes('?') ? '&' : '?'}auth=success`; | ||
| : `${cleanRedirectTo}${cleanRedirectTo.includes('?') ? '&' : '?'}auth=success`; |
…app conflict LFXV2-2255 Signed-off-by: Efren Lim <elim@linuxfoundation.org>
…auth LFXV2-2255 Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Comment on lines
224
to
+228
| // Clean up all auth cookies on unrecoverable error | ||
| deleteCookie(event, 'auth_pkce'); | ||
| deleteCookie(event, 'auth_redirect_to'); | ||
| deleteCookie(event, 'auth_oidc_token'); | ||
| deleteCookie(event, 'auth_refresh_token'); | ||
| deleteCookie(event, 'insights_oidc_token'); | ||
| deleteCookie(event, 'insights_refresh_token'); |
Comment on lines
+36
to
+39
| // auth_pkce and auth_redirect_to are short-lived flow cookies — no explicit domain set, | ||
| // so plain deleteCookie is fine here. | ||
| deleteCookie(event, 'auth_pkce'); | ||
| deleteCookie(event, 'auth_redirect_to'); |
Comment on lines
87
to
90
| const logoutHandler = async () => { | ||
| document.cookie = 'auth_oidc_token=; Path=/; Max-Age=0; SameSite=None; Secure'; | ||
| document.cookie = 'insights_oidc_token=; Path=/; Max-Age=0; SameSite=None; Secure'; | ||
| await logout(); | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
?auth=success&auth=success&...URL accumulation when a user is logged intocrowdfunding.linuxfoundation.org(which shares cookie names with insights) and then visits insightscallback.ts: Strip any pre-existing?authparam fromredirectTobefore appendingauth=success, preventing accumulation across redirect cyclescallback.ts: On silent auth failure (login_required,interaction_required,consent_required), redirect back cleanly without appendingauth=success— the user isn't authenticated so the flag is misleading and perpetuates the loopuseAuth.ts+auth.client.ts: Don't resetsilentLoginAttemptedflag insidelogin()for silent calls — the flag was being cleared before the redirect completed, so the guard never fired on the next page loadRoot cause
Both apps use the same auth cookie names (
auth_oidc_token,auth_refresh_token). If crowdfunding sets these with a parent-domain scope (.linuxfoundation.org), they bleed into insights requests. Insights then fails JWT validation (different signing secret), returnsshouldAttemptSilentLogin: true, and triggers a silent re-auth — which previously looped forever.Test plan
crowdfunding.linuxfoundation.org, then navigate to insights — URL should not accumulate?auth=successparams; page settles as unauthenticated after one silent login attempt/?auth=successexactly onceauth_oidc_tokencookie and refreshing no longer throwsAuthentication error: consent_requiredNotes
frontend/app/plugins/auth.client.tsis a protected file — code owner review required before merge🤖 Generated with Claude Code