fix: Preserve OIDC params across SAML session regeneration#631
Merged
H2CK merged 4 commits intoMar 27, 2026
Merged
Conversation
…eration When a user authenticates via a SAML backend, Nextcloud regenerates the PHP session (as protection against session fixation attacks). This destroys the OIDC authorization parameters (client_id, state, redirect_uri, etc.) that were stored in the session before the login redirect. After SAML login, PageController::index() reads null values and Redirect.vue redirects to /apps/oidc/authorize without parameters, causing: ClientMapper::getByIdentifier(): Argument H2CK#1 must be of type string, null given. Fix: Pass all OIDC parameters as URL query parameters in the redirect_url so they survive session regeneration. The session fallback is kept for backwards compatibility with non-SAML login flows. Changes: - LoginRedirectorController: Include params in linkToRoute() call - PageController: Read from request first, fall back to session - templates/main.php: Pass params as data-* attributes on the DOM - Redirect.vue: Read data-* attributes and append to authorize URL Fixes H2CK#628
H2CK
requested changes
Mar 24, 2026
Owner
H2CK
left a comment
H2CK
left a comment
There was a problem hiding this comment.
Please update the code according to the requested changes per file.
- Wrap URL parameters in array_filter() to omit null values - Use all-or-nothing approach for URL vs session parameters in PageController to avoid mixing data from different sources Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
H2CK
reviewed
Mar 25, 2026
Owner
H2CK
left a comment
H2CK
left a comment
There was a problem hiding this comment.
Changes are ok for me. How about the open tasks in the test plan, e.g. PKCE? Are you able to test it?
The last task about consent flow will fail (according to my tests). Necessary changes are already described in #630 in
https://github.com/H2CK/oidc/pull/630/changes#diff-fbdcb468be0d9575dfd4c1dd9f35ebd94c74a355dc7807e6927374b947866148
I included those changes with this PR, then it worked.
…for missing OAuth params
H2CK
requested changes
Mar 25, 2026
Owner
H2CK
left a comment
H2CK
left a comment
There was a problem hiding this comment.
Revert the code LoginRedirectController from line 226 on. Otherwise previous bugs will be opened up again.
Revert the granular per-parameter fallback and guard clause as requested in review — these changes reintroduced previously fixed bugs. The original session fallback (triggered only when client_id is empty) is restored. All scenarios tested successfully: - SAML backend login (first attempt, no retry needed) - Direct Nextcloud login - PKCE flow (code_challenge preserved across SAML redirect) - Consent flow (ConsentController now passes all params via URL)
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
2 participants
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
client_id,state,redirect_uri, etc.) are lost when SAML login regenerates the PHP sessionProblem
When Nextcloud acts as OIDC Identity Provider and authenticates users via SAML:
authorize()stores OIDC params in session, redirects to loginPageController::index()readsnullfrom sessionRedirect.vueredirects to/apps/oidc/authorizewithout parametersClientMapper::getByIdentifier()throws:Argument #1 must be of type string, null givenSecond login attempt always works because the SAML session is already established.
Solution
Pass OIDC authorization parameters as URL query parameters through the redirect chain, so they survive session regeneration. This does not weaken session fixation protection — the session is still regenerated, we just avoid storing routing data in it.
Files changed:
LoginRedirectorController.php— Include params inlinkToRoute()callPageController.php— Read from request first, fall back to sessiontemplates/main.php— Pass params asdata-*attributes on the DOM elementRedirect.vue— Readdata-*attributes and append to authorize URLTest plan
code_challenge) — parameters must be preservedFixes #628