Return users to the page they asked for after OIDC login - #572
Draft
somethingnew2-0 wants to merge 1 commit into
Draft
Return users to the page they asked for after OIDC login#572somethingnew2-0 wants to merge 1 commit into
somethingnew2-0 wants to merge 1 commit into
Conversation
Closes #566. A signed-out user following a link to a group page should land on that group page once the IdP is done with them, not on the home page. Two gaps kept that from happening. The `next` captured on the login bounce was `request.url.path`, dropping the query string. The SPA keeps list filters, search terms, sort order and pagination there, so a shared deep link arrived stripped of most of its meaning. And an unauthenticated `/api/*` request was answered with the same 307 to the IdP that a browser navigation gets. `fetch` can't complete an interactive login: it follows the redirect into the IdP's cross-origin HTML, fails CORS, and the SPA renders a generic error. A user whose session expired mid-session was stuck there with no way back short of a manual reload. Requests that aren't a top-level navigation now get a 401 carrying `login_url`, and the client escalates to a real navigation with its own location as `next`. Navigation is detected from `Sec-Fetch-Mode` (falling back to sniffing `Accept` for clients that omit it) rather than from the path, so a browser pointed at `/api/docs` still gets the login bounce. `next` also now rejects paths under `/oidc/`, which would otherwise let a crafted link log the user straight back out or loop the login. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #566.
A signed-out user following a link to a group page should land on that group page once the IdP is done with them, not on the home page. Two gaps kept that from happening.
The query string was dropped. The
nextcaptured on the login bounce wasrequest.url.path. The SPA keeps list filters, search terms, sort order and pagination in the query string, so that's the shareable part of a deep link — without it the user arrives at an unfiltered page.XHR was bounced to the IdP. An unauthenticated
/api/*request got the same 307 a browser navigation gets.fetchcan't complete an interactive login: it follows the redirect into the IdP's cross-origin HTML, fails CORS, and the SPA surfaces a generic "unexpected error". Someone whose session expired while the app was open was stuck there with no way back short of a manual reload. Requests that aren't a top-level navigation now get a401carrying alogin_urlextension member, and the client escalates to a real navigation with its own location (query string and fragment included) asnext.Navigation is detected from
Sec-Fetch-Mode, falling back to sniffingAcceptfor clients that omit it, rather than from the request path. Path-based detection would have been simpler but would have broken browsing to/api/docson deployments that enable it.While in here,
nextnow also rejects paths under/oidc/— a crafted link could otherwise return the user to/oidc/logout(undoing the login they just completed) or/oidc/login(looping). Third-party hosts were already rejected.Note this is partly pre-existing behaviour the reporter hasn't seen yet: the
nextround trip itself landed with the FastAPI migration (#425) and isn't in a release, so on v1.6.1 the redirect always goes to/. This PR closes the remaining gaps on top of it.🤖 Generated with Claude Code