From 81920e599b6931aa04c2b0d1670178aef7bd5d25 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Fri, 11 Oct 2024 08:04:15 +0100 Subject: [PATCH] fix(SAML): log underlying error if SAML response validation fails (#8721) # Which Problems Are Solved If SAML response validation in crewjam/saml fails, a generic "Authentication failed" error is thrown. This makes it challenging to determine the actual cause, since there are a variety of reasons response validation may fail. # How the Problems Are Solved Add a log statement if we receive a response validation error from crewjam/saml that logs the internal `InvalidResponseError.PrivateErr` error from crewjam/saml to stdout. We continue to return a generic error message to the client to prevent leaking data. Verified by running `go test -v ./internal/idp/providers/saml` in verbose mode, which output the following line for the "response_invalid" test case: ``` time="2024-10-03T14:53:10+01:00" level=info msg="invalid SAML response details" caller="/Users/sdouglas/Documents/thirdparty-repos/zitadel/internal/idp/providers/saml/session.go:72" error="cannot parse base64: illegal base64 data at input byte 2" ``` # Additional Changes None # Additional Context - closes #8717 --------- Co-authored-by: Stuart Douglas --- internal/idp/providers/saml/session.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/idp/providers/saml/session.go b/internal/idp/providers/saml/session.go index f3d44fc36..09de7163b 100644 --- a/internal/idp/providers/saml/session.go +++ b/internal/idp/providers/saml/session.go @@ -3,11 +3,13 @@ package saml import ( "bytes" "context" + "errors" "net/http" "net/url" "github.com/crewjam/saml" "github.com/crewjam/saml/samlsp" + "github.com/zitadel/logging" "github.com/zitadel/zitadel/internal/idp" "github.com/zitadel/zitadel/internal/zerrors" @@ -67,6 +69,10 @@ func (s *Session) FetchUser(ctx context.Context) (user idp.User, err error) { s.Assertion, err = s.ServiceProvider.ServiceProvider.ParseResponse(s.Request, []string{s.RequestID}) if err != nil { + invalidRespErr := new(saml.InvalidResponseError) + if errors.As(err, &invalidRespErr) { + logging.WithError(invalidRespErr.PrivateErr).Info("invalid SAML response details") + } return nil, zerrors.ThrowInvalidArgument(err, "SAML-nuo0vphhh9", "Errors.Intent.ResponseInvalid") }