Repository navigation
fix(auth): SAML endpoints refuse every request until assertions are verified (#866) - #868
Merged
Merged
Conversation
The SAML assertion consumer service read the email out of whatever XML it was posted and opened a session for that account. It checked no signature, issuer, audience, validity window or InResponseTo, and it was mounted on every deployment whether or not SAML was configured. SAML2ACS and SAML2InitiateLogin now redirect to /login?error=provider_not_configured&provider=saml2, which the login screen already explains, and create nothing. The code that turned an assertion into a user, a group-mapped role and a session is removed rather than left unreachable; real SAML support comes back through a maintained library that verifies signed assertions. The metadata endpoint is unchanged. SAML sign-in was never proven end to end, so no working flow is lost. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
The README listed SAML2 among the SSO options, and the endpoint references said the ACS redirects with a token. Both now say SAML is turned off and what its endpoints answer. The pricing and self-hosting plan tables are left for the owner. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
This was referenced Oct 2, 2026
fix(auth): master does not compile — a merge put the old SAML ACS body back without its imports
#886
Closed
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 #866
SAML sign-in is turned off until assertions are verified.
POST /api/v1/auth/saml2/acsandGET /api/v1/auth/saml2/loginnow redirect to/login?error=provider_not_configured&provider=saml2and create no user and no session. The login screen already explains that code in both languages.GET /api/v1/auth/saml2/metadatais unchanged.The code that turned a posted assertion into a user, a group-mapped role and a session (
provisionSAML2User,applyGroupRoleMapping, the hand-rolled assertion structs) is removed, not left unreachable. SAML should come back through a maintained library that verifies signed assertions, which needs an ADR and a dependency decision. Nothing in that code is worth keeping for the rewrite, and git history holds it.SAML sign-in was never proven end to end (ROADMAP module 2), so no working flow is lost.
Verification
saml2_handler_test.go: a well-formed success response is refused, an empty POST is refused, and the login redirects even with SAML configured. Each check asserts the redirect and that no session cookie is set. Against master's handler the same tests fail, because the request reaches the database.Interaction with #803
Both PRs touch
SAML2ACS. Merged together in a scratch worktree, the one conflict is insaml2_handler.go, and the resolution is this PR's version (#803 only changed the line this PR deletes). The merged tree runs 79 ok, 0 FAIL. Whichever lands second needs that one-line resolution.Docs
API_COMPLETE_ENDPOINTS.mdandENDPOINTS.mdnow give what the two endpoints answer.API_SECURITY_GUIDE.mdandSAML_OAUTH2_INTEGRATION.mdcarry a "turned off" note.Honest remainders
docs/PRICING.md(line 29) anddocs/SELF_HOSTING.md(lines 119, 187) still list "SSO / SAML" in the plan tables. Pricing wording is the owner's call, so they are left as they are.WantAssertionsSigned="false". It is harmless while the ACS refuses everything, and it should change with the real implementation.