Skip to content

fix(auth): restore the fail-closed SAML handler so master compiles (#886) - #888

Merged
alex-dembele merged 1 commit into
masterfrom
fix/886-saml-handler-build
Oct 2, 2026
Merged

alex-dembele merged 1 commit into
masterfrom
fix/886-saml-handler-build

Conversation

@alex-dembele

Copy link
Copy Markdown
Member

Closes #886

master does not compile: saml2_handler.go uses base64, xml, strings, log, domain and gorm without importing them.

The merge e86df5cd (master into the #866 branch, merged via #868) kept #866's trimmed imports and doc comment, but brought back #803's old SAML2ACS body, along with provisionSAML2User and applyGroupRoleMapping.

Do not fix this by adding the imports. That body opens a session for whatever email an unsigned XML names, which is the bypass #866 closed. This PR restores 069b3db8's version of the file, the one #866 intended, where both SAML entry points refuse every request. It is one file: 183 lines removed and the one-line refusal back.

Verification

On the branch:

go build ./...                      → ok
go vet ./... && go test ./... -race → exit 0, 80 packages ok, 0 FAIL

Those packages include #866's own tests, TestSAML2ACS_RefusesAWellFormedSuccessResponse and TestSAML2ACS_RefusesAnEmptyPost, which could not compile on master.

Not done

  • I did not check how a broken build reached master: CI was red on 14f26e8e. Worth a look at branch protection.

)

The merge e86df5c brought the old SAML2ACS body, provisionSAML2User
and applyGroupRoleMapping back from #803, while keeping #866's trimmed
imports. master stopped compiling.

Adding the imports would have compiled the hand-rolled ACS back in: it
reads the email out of any posted XML, with no signature check, and opens
a session. This restores #866's version of the file instead, where both
SAML entry points refuse every request. No other file is touched.

Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:backend Go, /internal, /pkg priority:P0-critical Production broken or exposed — work now tier:0-trust Trust: security, isolation, evidence integrity type:security Security defect or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(auth): master does not compile — a merge put the old SAML ACS body back without its imports

1 participant