diff --git a/connector/connector.go b/connector/connector.go index f95c62e4a2..81094b4362 100644 --- a/connector/connector.go +++ b/connector/connector.go @@ -135,3 +135,26 @@ type LogoutCallbackConnector interface { // return nil. HandleLogoutCallback(ctx context.Context, r *http.Request) error } + +// StatefulLogoutCallbackConnector is an optional capability for connectors +// whose logout flow needs server-side correlation state to be carried from +// the outgoing logout request to the inbound logout response. The server +// persists the opaque state alongside the logout session and hands it back +// on the callback, allowing the connector to enforce one-shot, replay-proof +// checks (e.g. SAML's InResponseTo). +// +// Connectors that don't need correlation state should implement the simpler +// LogoutCallbackConnector instead. The server prefers this interface over +// LogoutCallbackConnector when both are implemented. +type StatefulLogoutCallbackConnector interface { + // LogoutURLWithState returns the upstream provider's logout URL plus an + // opaque connector-specific state to be persisted by the server and + // passed back to HandleLogoutCallbackWithState. Returning empty url means + // upstream logout is not available; in that case state must be nil. + LogoutURLWithState(ctx context.Context, connectorData []byte, postLogoutRedirectURI string) (logoutURL string, state []byte, err error) + + // HandleLogoutCallbackWithState validates the upstream provider's logout + // response received in the callback request. state is the value returned + // by the matching LogoutURLWithState call. + HandleLogoutCallbackWithState(ctx context.Context, r *http.Request, state []byte) error +} diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 8ef434b62a..e0b6de7605 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -3,14 +3,21 @@ package saml import ( "bytes" + "compress/flate" "context" + "crypto" + "crypto/rand" + "crypto/rsa" "crypto/x509" "encoding/base64" "encoding/json" "encoding/pem" "encoding/xml" "fmt" + "io" "log/slog" + "net/http" + "net/url" "os" "strings" "sync" @@ -48,6 +55,14 @@ const ( // allowed clock drift for timestamp validation allowedClockDrift = time.Duration(30) * time.Second + + // Default RSA algorithm for SAML HTTP-Redirect query-string signatures (SP logout). + defaultRedirectSigAlg = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" + + // Keep SLO responses small enough to validate and inflate safely. A normal + // LogoutResponse is only a few kilobytes. + maxSAMLLogoutResponseSize = 1 << 20 + maxSAMLLogoutPOSTBodySize = 2 << 20 ) var ( @@ -84,6 +99,20 @@ type Config struct { InsecureSkipSignatureValidation bool `json:"insecureSkipSignatureValidation"` + // SLOURL is the IdP's Single Logout Service URL (HTTP-Redirect binding). + // If empty, SLO is not available for this connector. + SLOURL string `json:"sloURL"` + + // SLOSigningKey and SLOSigningKeyData are a PEM-encoded RSA private key used to + // sign SP-initiated SAML LogoutRequests on the HTTP-Redirect binding (SigAlg + + // Signature query parameters per SAML 2.0 Bindings §3.4.4). Optional: when both + // are empty, LogoutRequest is sent unsigned. + // + // This is not the same as ca/caData: those are public certificates for verifying + // the IdP; signing requires the SP's own key pair registered with the IdP. + SLOSigningKey string `json:"sloSigningKey"` + SLOSigningKeyData []byte `json:"sloSigningKeyData"` + // Assertion attribute names to lookup various claims with. UsernameAttr string `json:"usernameAttr"` EmailAttr string `json:"emailAttr"` @@ -164,6 +193,47 @@ func (c *Config) openConnector(logger *slog.Logger) (*provider, error) { logger: logger, nameIDPolicyFormat: c.NameIDPolicyFormat, + + sloURL: c.SLOURL, + } + + if c.SLOSigningKey != "" && len(c.SLOSigningKeyData) > 0 { + return nil, errors.New("saml: specify at most one of sloSigningKey and sloSigningKeyData") + } + if c.SLOURL != "" && c.EntityIssuer == "" { + // Single Logout Profile (§4.4.4.1) requires on every LogoutRequest; + // without entityIssuer we have no SP entityID to populate it with, and + // most production IdPs (Keycloak, ADFS, Okta, ...) reject Issuer-less + // LogoutRequests outright. + return nil, errors.New("saml: entityIssuer is required when sloURL is set") + } + if c.SLOSigningKey != "" || len(c.SLOSigningKeyData) > 0 { + if c.SLOURL == "" { + return nil, errors.New("saml: sloSigningKey or sloSigningKeyData requires sloURL") + } + var keyPEM []byte + if c.SLOSigningKey != "" { + data, err := os.ReadFile(c.SLOSigningKey) + if err != nil { + return nil, fmt.Errorf("saml: read sloSigningKey: %v", err) + } + keyPEM = data + } else { + keyPEM = c.SLOSigningKeyData + } + sloKey, err := parseRSAPrivateKeyPEM(keyPEM) + if err != nil { + return nil, fmt.Errorf("saml: parse sloSigningKey: %v", err) + } + p.sloSignKey = sloKey + } else if c.SLOURL != "" { + // Per SAML 2.0 Profiles §4.4.3.4 / §4.4.4.1, LogoutRequests sent over + // HTTP-Redirect or HTTP-POST MUST be signed. We allow the unsigned + // configuration (some test setups need it), but warn loudly so the + // operator notices before the IdP rejects every logout. + logger.Warn("saml: sloURL configured without sloSigningKey/sloSigningKeyData; " + + "LogoutRequest will be sent unsigned, which violates SAML 2.0 Profiles §4.4.3.4 " + + "and is rejected by most production IdPs") } if p.nameIDPolicyFormat == "" { @@ -228,13 +298,45 @@ func (c *Config) openConnector(logger *slog.Logger) (*provider, error) { return nil, errors.New("no certificates found in ca data") } p.validator = dsig.NewDefaultValidationContext(certStore{certs}) + p.certs = certs } return p, nil } +// parseRSAPrivateKeyPEM loads the first RSA private key from PEM data (PKCS#1 or PKCS#8). +func parseRSAPrivateKeyPEM(pemData []byte) (*rsa.PrivateKey, error) { + for len(pemData) > 0 { + block, rest := pem.Decode(pemData) + if block == nil { + break + } + switch block.Type { + case "RSA PRIVATE KEY": + k, err := x509.ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + return nil, err + } + return k, nil + case "PRIVATE KEY": + k, err := x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, err + } + rsaK, ok := k.(*rsa.PrivateKey) + if !ok { + return nil, fmt.Errorf("private key is %T, want RSA", k) + } + return rsaK, nil + } + pemData = rest + } + return nil, errors.New("no RSA private key found in PEM data") +} + var ( - _ connector.SAMLConnector = (*provider)(nil) - _ connector.RefreshConnector = (*provider)(nil) + _ connector.SAMLConnector = (*provider)(nil) + _ connector.RefreshConnector = (*provider)(nil) + _ connector.StatefulLogoutCallbackConnector = (*provider)(nil) ) type provider struct { @@ -246,6 +348,9 @@ type provider struct { // If nil, don't do signature validation. validator *dsig.ValidationContext + // Stored separately for HTTP-Redirect binding signature verification, + // which uses raw RSA/ECDSA over query string rather than XML digital signatures. + certs []*x509.Certificate // Attribute mappings usernameAttr string @@ -259,12 +364,17 @@ type provider struct { nameIDPolicyFormat string + sloURL string + + // If non-nil, SP-initiated LogoutRequests use HTTP-Redirect query signing. + sloSignKey *rsa.PrivateKey + logger *slog.Logger } -// cachedIdentity stores the identity from SAML assertion for refresh token support. -// Since SAML has no native refresh mechanism, we cache the identity obtained during -// the initial authentication and return it on subsequent refresh requests. +// cachedIdentity stores the identity from SAML assertion for refresh token support +// and SLO (Single Logout). The NameID/NameIDFormat/SessionIndex fields are used +// to build a SAML LogoutRequest when the user logs out. type cachedIdentity struct { UserID string `json:"userId"` Username string `json:"username"` @@ -272,10 +382,15 @@ type cachedIdentity struct { Email string `json:"email"` EmailVerified bool `json:"emailVerified"` Groups []string `json:"groups,omitempty"` + NameID string `json:"nameId,omitempty"` + NameIDFormat string `json:"nameIdFormat,omitempty"` + SessionIndex string `json:"sessionIndex,omitempty"` } -// marshalCachedIdentity serializes the identity into ConnectorData for refresh token support. -func marshalCachedIdentity(ident connector.Identity) (connector.Identity, error) { +// marshalCachedIdentity serializes the identity along with SAML-specific SLO +// fields into ConnectorData. The nameIDFormat and sessionIdx parameters come +// from the parsed SAML assertion and are needed to construct a LogoutRequest. +func marshalCachedIdentity(ident connector.Identity, nameIDFormat, sessionIdx string) (connector.Identity, error) { ci := cachedIdentity{ UserID: ident.UserID, Username: ident.Username, @@ -283,6 +398,9 @@ func marshalCachedIdentity(ident connector.Identity) (connector.Identity, error) Email: ident.Email, EmailVerified: ident.EmailVerified, Groups: ident.Groups, + NameID: ident.UserID, + NameIDFormat: nameIDFormat, + SessionIndex: sessionIdx, } connectorData, err := json.Marshal(ci) if err != nil { @@ -407,15 +525,22 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str } } + var nameIDFormat string switch { case subject.NameID != nil: if ident.UserID = subject.NameID.Value; ident.UserID == "" { return ident, fmt.Errorf("element NameID does not contain a value") } + nameIDFormat = subject.NameID.Format default: return ident, fmt.Errorf("subject does not contain an NameID element") } + var sessionIdx string + if len(assertion.AuthnStatements) > 0 { + sessionIdx = assertion.AuthnStatements[0].SessionIndex + } + // After verifying the assertion, map data in the attribute statements to // various user info. attributes := assertion.AttributeStatement @@ -442,7 +567,7 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str if len(p.allowedGroups) == 0 && (!s.Groups || p.groupsAttr == "") { // Groups not requested or not configured. We're done. - return marshalCachedIdentity(ident) + return marshalCachedIdentity(ident, nameIDFormat, sessionIdx) } if len(p.allowedGroups) > 0 && (!s.Groups || p.groupsAttr == "") { @@ -468,7 +593,7 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str if len(p.allowedGroups) == 0 { // No allowed groups set, just return the ident - return marshalCachedIdentity(ident) + return marshalCachedIdentity(ident, nameIDFormat, sessionIdx) } // Look for membership in one of the allowed groups @@ -484,7 +609,7 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str } // Otherwise, we're good - return marshalCachedIdentity(ident) + return marshalCachedIdentity(ident, nameIDFormat, sessionIdx) } // Refresh implements connector.RefreshConnector. @@ -711,3 +836,428 @@ func before(now, notBefore time.Time) bool { func after(now, notOnOrAfter time.Time) bool { return now.After(notOnOrAfter.Add(allowedClockDrift)) } + +// newRequestID generates a random ID suitable for SAML request IDs. +func newRequestID() (string, error) { + buf := make([]byte, 16) + if _, err := io.ReadFull(rand.Reader, buf); err != nil { + return "", fmt.Errorf("crypto/rand failed: %v", err) + } + return fmt.Sprintf("_%x", buf), nil +} + +// sloCallbackURLFromRequest reconstructs the absolute URL of this HTTP request (SLO callback). +// Used to validate LogoutResponse Destination per SAML 2.0 Bindings §3.4.5.2. When Host is +// missing (e.g. some unit tests), returns empty and Destination checking is skipped. +func sloCallbackURLFromRequest(r *http.Request) string { + host := r.Host + if h := r.Header.Get("X-Forwarded-Host"); h != "" { + if i := strings.IndexByte(h, ','); i >= 0 { + h = strings.TrimSpace(h[:i]) + } else { + h = strings.TrimSpace(h) + } + host = h + } + if host == "" { + return "" + } + scheme := "http" + if r.TLS != nil { + scheme = "https" + } + if p := r.Header.Get("X-Forwarded-Proto"); p != "" { + if i := strings.IndexByte(p, ','); i >= 0 { + p = strings.TrimSpace(p[:i]) + } else { + p = strings.TrimSpace(p) + } + scheme = p + } + return scheme + "://" + host + r.URL.Path +} + +// sloURLsMatch compares two URLs ignoring trailing slashes, with case-insensitive +// scheme/host comparison per RFC 3986 §3.1/§3.2.2. Default-port normalization +// (e.g. https://x → https://x:443) is intentionally NOT performed: SAML IdPs +// typically echo the exact Destination they received, and pretending two URLs +// are equal when an operator wrote them differently in their config tends to +// hide misconfigurations rather than fix them. +func sloURLsMatch(a, b string) bool { + pa, errA := url.Parse(strings.TrimSpace(a)) + pb, errB := url.Parse(strings.TrimSpace(b)) + if errA != nil || errB != nil { + return strings.TrimSuffix(strings.TrimSpace(a), "/") == strings.TrimSuffix(strings.TrimSpace(b), "/") + } + if !strings.EqualFold(pa.Scheme, pb.Scheme) { + return false + } + if !strings.EqualFold(pa.Host, pb.Host) { + return false + } + return strings.TrimSuffix(pa.Path, "/") == strings.TrimSuffix(pb.Path, "/") && + pa.RawQuery == pb.RawQuery +} + +// LogoutURLWithState builds a SAML LogoutRequest and returns the IdP's SLO +// endpoint URL with the request encoded using HTTP-Redirect binding +// (deflate + base64). The second return value is the outgoing LogoutRequest +// ID, which the server persists in storage.LogoutState.ConnectorState and +// hands back to HandleLogoutCallbackWithState so InResponseTo can be matched +// against a server-side, one-shot value (defeats replay of captured +// LogoutResponses). +// +// postLogoutRedirectURI is Dex's own /logout/callback URL; SAML doesn't carry +// it in the request (the IdP knows where to send the LogoutResponse via its +// configured SP metadata), so it is intentionally ignored here. +// +// See: https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf §3.4 +// and https://docs.oasis-open.org/security/saml/v2.0/saml-profiles-2.0-os.pdf §4.4. +func (p *provider) LogoutURLWithState(_ context.Context, connectorData []byte, _ string) (string, []byte, error) { + if p.sloURL == "" { + return "", nil, nil + } + + var ci cachedIdentity + if len(connectorData) > 0 { + if err := json.Unmarshal(connectorData, &ci); err != nil { + return "", nil, fmt.Errorf("saml: failed to unmarshal connector data for logout: %v", err) + } + } + + if ci.NameID == "" { + return "", nil, nil + } + + reqID, err := newRequestID() + if err != nil { + return "", nil, fmt.Errorf("saml: %v", err) + } + + req := &logoutRequest{ + ID: reqID, + IssueInstant: xmlTime(p.now()), + Destination: p.sloURL, + Issuer: &issuer{Issuer: p.entityIssuer}, // §4.4.4.1: Issuer is REQUIRED + NameID: nameID{ + Format: ci.NameIDFormat, + Value: ci.NameID, + }, + } + if ci.SessionIndex != "" { + req.SessionIndex = []sessionIndex{{Value: ci.SessionIndex}} + } + + data, err := xml.Marshal(req) + if err != nil { + return "", nil, fmt.Errorf("saml: failed to marshal LogoutRequest: %v", err) + } + + // HTTP-Redirect binding: deflate then base64-encode. + var buf bytes.Buffer + fw, err := flate.NewWriter(&buf, flate.DefaultCompression) + if err != nil { + return "", nil, fmt.Errorf("saml: failed to create deflate writer: %v", err) + } + if _, err := fw.Write(data); err != nil { + return "", nil, fmt.Errorf("saml: failed to deflate LogoutRequest: %v", err) + } + if err := fw.Close(); err != nil { + return "", nil, fmt.Errorf("saml: failed to close deflate writer: %v", err) + } + + encoded := base64.StdEncoding.EncodeToString(buf.Bytes()) + + u, err := url.Parse(p.sloURL) + if err != nil { + return "", nil, fmt.Errorf("saml: failed to parse SLO URL: %v", err) + } + + // We do not emit RelayState. SAML 2.0 Bindings §3.4.3 limits it to 80 + // bytes and Dex correlates the SLO flow through the session cookie + + // server-side LogoutState; InResponseTo is matched against the request + // ID we hand back to the server below. + samlReqEscaped := url.QueryEscape(encoded) + baseQuery := "SAMLRequest=" + samlReqEscaped + + if p.sloSignKey != nil { + sigAlgEscaped := url.QueryEscape(defaultRedirectSigAlg) + signedContent := baseQuery + "&SigAlg=" + sigAlgEscaped + h := crypto.SHA256.New() + h.Write([]byte(signedContent)) + sig, err := rsa.SignPKCS1v15(rand.Reader, p.sloSignKey, crypto.SHA256, h.Sum(nil)) + if err != nil { + return "", nil, fmt.Errorf("saml: sign LogoutRequest redirect binding: %v", err) + } + sigB64 := base64.StdEncoding.EncodeToString(sig) + u.RawQuery = signedContent + "&Signature=" + url.QueryEscape(sigB64) + } else { + u.RawQuery = baseQuery + } + + return u.String(), []byte(reqID), nil +} + +// HandleLogoutCallbackWithState validates the IdP's LogoutResponse received +// after an SP-initiated logout redirect. The response arrives as a +// SAMLResponse parameter via either GET query (HTTP-Redirect binding: +// deflated + base64) or POST form (HTTP-POST binding: base64 only). +// +// state is the value previously returned by LogoutURLWithState — for SAML, +// the bytes of the outgoing LogoutRequest ID. The server retrieves it from +// storage.LogoutState.ConnectorState; using a server-side, single-use value +// (instead of an IdP-echoed RelayState) makes InResponseTo replay-resistant +// even when the LogoutResponse is HTTP-POST and RelayState isn't covered by +// the signature. +func (p *provider) HandleLogoutCallbackWithState(_ context.Context, r *http.Request, state []byte) error { + if len(state) == 0 { + return fmt.Errorf("saml slo: missing server-side LogoutRequest ID") + } + + var samlResponse string + switch r.Method { + case http.MethodGet: + samlResponse = r.URL.Query().Get("SAMLResponse") + case http.MethodPost: + body, err := io.ReadAll(io.LimitReader(r.Body, maxSAMLLogoutPOSTBodySize+1)) + if err != nil { + return fmt.Errorf("saml slo: failed to read form: %w", err) + } + if len(body) > maxSAMLLogoutPOSTBodySize { + return fmt.Errorf("saml slo: POST body exceeds %d bytes", maxSAMLLogoutPOSTBodySize) + } + form, err := url.ParseQuery(string(body)) + if err != nil { + return fmt.Errorf("saml slo: failed to parse form: %w", err) + } + samlResponse = form.Get("SAMLResponse") + default: + return fmt.Errorf("saml slo: unsupported HTTP method %q", r.Method) + } + + if samlResponse == "" { + return fmt.Errorf("saml slo: missing SAMLResponse parameter") + } + if len(samlResponse) > base64.StdEncoding.EncodedLen(maxSAMLLogoutResponseSize+1024) { + return fmt.Errorf("saml slo: encoded SAMLResponse is too large") + } + + if r.Method == http.MethodGet && len(p.certs) > 0 { + if err := p.validateRedirectSignature(r, "SAMLResponse"); err != nil { + return fmt.Errorf("saml slo: %v", err) + } + } + + compressed, err := base64.StdEncoding.DecodeString(samlResponse) + if err != nil { + return fmt.Errorf("saml slo: failed to decode SAMLResponse: %v", err) + } + + // Per SAML 2.0 Bindings: + // §3.4 HTTP-Redirect: SAMLResponse MUST be DEFLATE-encoded, then base64. + // §3.5 HTTP-POST: SAMLResponse is base64 of the raw XML, no DEFLATE. + // Mixing the two would let a malformed response slip through one path while + // pretending to satisfy the other, so we treat the binding strictly. + var rawResp []byte + if r.Method == http.MethodGet { + reader := flate.NewReader(bytes.NewReader(compressed)) + rawResp, err = io.ReadAll(io.LimitReader(reader, maxSAMLLogoutResponseSize+1)) + closeErr := reader.Close() + if err != nil { + return fmt.Errorf("saml slo: failed to inflate SAMLResponse (HTTP-Redirect binding requires DEFLATE): %w", err) + } + if closeErr != nil { + return fmt.Errorf("saml slo: failed to close SAMLResponse inflater: %w", closeErr) + } + } else { + rawResp = compressed + } + if len(rawResp) > maxSAMLLogoutResponseSize { + return fmt.Errorf("saml slo: SAMLResponse exceeds %d bytes after decoding", maxSAMLLogoutResponseSize) + } + + byteReader := bytes.NewReader(rawResp) + if xrvErr := xrv.Validate(byteReader); xrvErr != nil { + return fmt.Errorf("saml slo: %w", xrvErr) + } + + if r.Method == http.MethodPost && p.validator != nil { + if _, err := p.validateSignature(rawResp); err != nil { + return fmt.Errorf("saml slo: %v", err) + } + } + + var resp logoutResponse + if err := xml.Unmarshal(rawResp, &resp); err != nil { + return fmt.Errorf("saml slo: failed to unmarshal LogoutResponse: %v", err) + } + + if resp.ID == "" { + return fmt.Errorf("saml slo: LogoutResponse is missing required ID attribute") + } + if !resp.Version.present { + return fmt.Errorf("saml slo: LogoutResponse is missing required Version attribute") + } + if resp.Status == nil { + return fmt.Errorf("saml slo: LogoutResponse is missing required Status element") + } + if err := p.validateStatus(resp.Status); err != nil { + return fmt.Errorf("saml slo: %v", err) + } + + // SAML Profiles §4.4.4.2 requires Issuer in LogoutResponse. + if resp.Issuer == nil || resp.Issuer.Issuer == "" { + return fmt.Errorf("saml slo: LogoutResponse is missing required Issuer element") + } + if p.ssoIssuer != "" && resp.Issuer.Issuer != p.ssoIssuer { + return fmt.Errorf("saml slo: expected Issuer value %q, got %q", p.ssoIssuer, resp.Issuer.Issuer) + } + + issueInstant := time.Time(resp.IssueInstant) + if issueInstant.IsZero() { + return fmt.Errorf("saml slo: LogoutResponse is missing required IssueInstant attribute") + } + now := p.now() + if before(now, issueInstant) { + return fmt.Errorf("saml slo: LogoutResponse IssueInstant %s is in the future (now: %s)", issueInstant, now) + } + const maxAge = 5 * time.Minute + if now.After(issueInstant.Add(maxAge + allowedClockDrift)) { + return fmt.Errorf("saml slo: LogoutResponse IssueInstant %s is too old (now: %s)", issueInstant, now) + } + + if resp.Destination != "" { + if recv := sloCallbackURLFromRequest(r); recv != "" { + if !sloURLsMatch(resp.Destination, recv) { + return fmt.Errorf("saml slo: expected Destination %q, callback URL was %q", resp.Destination, recv) + } + } + } + + // Match InResponseTo against the one-shot request ID the server kept in + // LogoutState.ConnectorState. + expectedReqID := string(state) + if resp.InResponseTo != expectedReqID { + return fmt.Errorf("saml slo: InResponseTo mismatch: expected %q, got %q", expectedReqID, resp.InResponseTo) + } + + return nil +} + +// redirectSigAlgToHash maps XML Signature algorithm URIs used in SAML HTTP-Redirect +// binding to Go crypto.Hash values. Only RSA algorithms are supported. +// See: https://www.w3.org/TR/xmldsig-core1/#sec-AlgID +var redirectSigAlgToHash = map[string]crypto.Hash{ + "http://www.w3.org/2000/09/xmldsig#rsa-sha1": crypto.SHA1, + "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256": crypto.SHA256, + "http://www.w3.org/2001/04/xmldsig-more#rsa-sha384": crypto.SHA384, + "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512": crypto.SHA512, +} + +// rawQueryParam extracts the raw (still URL-encoded) value of a query parameter +// from a raw query string. This is needed for SAML HTTP-Redirect binding signature +// validation, which signs over the URL-encoded parameter values. +func rawQueryParam(rawQuery, key string) (string, bool) { + prefix := key + "=" + for rawQuery != "" { + var pair string + if i := strings.IndexByte(rawQuery, '&'); i >= 0 { + pair, rawQuery = rawQuery[:i], rawQuery[i+1:] + } else { + pair, rawQuery = rawQuery, "" + } + if strings.HasPrefix(pair, prefix) { + return pair[len(prefix):], true + } + } + return "", false +} + +// validateRedirectSignature verifies the query-string signature used in SAML +// HTTP-Redirect binding. Unlike HTTP-POST where the signature is embedded in +// the XML (), HTTP-Redirect carries it as Signature and SigAlg +// query parameters. The signed content is reconstructed per SAML 2.0 Bindings +// Section 3.4.4.1: SAMLRequest or SAMLResponse, then optional RelayState, then +// SigAlg (using the original URL-encoded values). +func (p *provider) validateRedirectSignature(r *http.Request, samlMsgParam string) error { + rawQuery := r.URL.RawQuery + + sigEncoded, ok := rawQueryParam(rawQuery, "Signature") + if !ok || sigEncoded == "" { + return fmt.Errorf("missing Signature query parameter") + } + + sigAlgEncoded, ok := rawQueryParam(rawQuery, "SigAlg") + if !ok || sigAlgEncoded == "" { + return fmt.Errorf("missing SigAlg query parameter") + } + + sigAlg, err := url.QueryUnescape(sigAlgEncoded) + if err != nil { + return fmt.Errorf("failed to decode SigAlg: %v", err) + } + + hashAlg, ok := redirectSigAlgToHash[sigAlg] + if !ok { + return fmt.Errorf("unsupported signature algorithm: %s", sigAlg) + } + + // Reconstruct the signed content in the spec-mandated order. + var parts []string + if v, ok := rawQueryParam(rawQuery, samlMsgParam); ok { + parts = append(parts, samlMsgParam+"="+v) + } + if v, ok := rawQueryParam(rawQuery, "RelayState"); ok { + parts = append(parts, "RelayState="+v) + } + parts = append(parts, "SigAlg="+sigAlgEncoded) + signedContent := strings.Join(parts, "&") + + sigB64, err := url.QueryUnescape(sigEncoded) + if err != nil { + return fmt.Errorf("failed to URL-decode Signature: %v", err) + } + sig, err := base64.StdEncoding.DecodeString(sigB64) + if err != nil { + return fmt.Errorf("failed to base64-decode Signature: %v", err) + } + + h := hashAlg.New() + h.Write([]byte(signedContent)) + hashed := h.Sum(nil) + + for _, cert := range p.certs { + rsaPub, ok := cert.PublicKey.(*rsa.PublicKey) + if !ok { + continue + } + if rsa.VerifyPKCS1v15(rsaPub, hashAlg, hashed, sig) == nil { + return nil + } + } + + return fmt.Errorf("redirect binding signature validation failed") +} + +// validateSignature validates the XML digital signature of the given raw XML. +func (p *provider) validateSignature(rawXML []byte) ([]byte, error) { + if p.validator == nil { + return nil, fmt.Errorf("signature validation unavailable (no validator configured)") + } + + doc := etree.NewDocument() + if err := doc.ReadFromBytes(rawXML); err != nil { + return nil, fmt.Errorf("failed to parse XML: %v", err) + } + + root := doc.Root() + if root == nil { + return nil, fmt.Errorf("empty XML document") + } + + if _, err := p.validator.Validate(root); err != nil { + return nil, fmt.Errorf("signature validation failed: %v", err) + } + + return rawXML, nil +} diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index 3eba5cf878..89e276d616 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -1,24 +1,54 @@ package saml import ( + "bytes" + "compress/flate" "context" + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/tls" "crypto/x509" "encoding/base64" "encoding/json" "encoding/pem" + "encoding/xml" "errors" + "fmt" + "io" "log/slog" + "net/http" + "net/http/httptest" + "net/url" "os" "sort" + "strings" "testing" "time" + "github.com/beevik/etree" "github.com/kylelemons/godebug/pretty" dsig "github.com/russellhaering/goxmldsig" "github.com/dexidp/dex/connector" ) +func redirectBindingEncode(t *testing.T, xmlPayload string) string { + t.Helper() + var buf bytes.Buffer + fw, err := flate.NewWriter(&buf, flate.DefaultCompression) + if err != nil { + t.Fatalf("deflate writer: %v", err) + } + if _, err := fw.Write([]byte(xmlPayload)); err != nil { + t.Fatalf("deflate write: %v", err) + } + if err := fw.Close(); err != nil { + t.Fatalf("deflate close: %v", err) + } + return base64.StdEncoding.EncodeToString(buf.Bytes()) +} + // responseTest maps a SAML 2.0 response object to a set of expected values. // // Tests are defined in the "testdata" directory and are self-signed using xmlsec1. @@ -916,3 +946,1082 @@ func TestSAMLRefresh(t *testing.T) { } }) } + +func TestHandlePOSTPopulatesSLOFields(t *testing.T) { + c := Config{ + CA: "testdata/ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + GroupsAttr: "groups", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + } + + conn, err := c.openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + now, err := time.Parse(timeFormat, "2017-04-04T04:34:59.330Z") + if err != nil { + t.Fatal(err) + } + conn.now = func() time.Time { return now } + + resp, err := os.ReadFile("testdata/good-resp.xml") + if err != nil { + t.Fatal(err) + } + samlResp := base64.StdEncoding.EncodeToString(resp) + + scopes := connector.Scopes{OfflineAccess: true, Groups: true} + ident, err := conn.HandlePOST(scopes, samlResp, "6zmm5mguyebwvajyf2sdwwcw6m") + if err != nil { + t.Fatalf("HandlePOST failed: %v", err) + } + + if len(ident.ConnectorData) == 0 { + t.Fatal("expected ConnectorData to be set") + } + + var ci cachedIdentity + if err := json.Unmarshal(ident.ConnectorData, &ci); err != nil { + t.Fatalf("failed to unmarshal ConnectorData: %v", err) + } + + if ci.NameID == "" { + t.Error("expected NameID to be populated in ConnectorData") + } + if ci.NameID != ident.UserID { + t.Errorf("NameID should match UserID: got %q, want %q", ci.NameID, ident.UserID) + } + if ci.NameIDFormat != "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" { + t.Errorf("unexpected NameIDFormat: %q", ci.NameIDFormat) + } + if ci.SessionIndex != "6zmm5mguyebwvajyf2sdwwcw6m" { + t.Errorf("unexpected SessionIndex: got %q, want %q", ci.SessionIndex, "6zmm5mguyebwvajyf2sdwwcw6m") + } +} + +// decodeSAMLRequest decodes a SAMLRequest query parameter value +// (base64 → inflate → XML) into a logoutRequest struct. +func decodeSAMLRequest(t *testing.T, encoded string) logoutRequest { + t.Helper() + compressed, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatalf("failed to base64 decode SAMLRequest: %v", err) + } + inflated, err := io.ReadAll(flate.NewReader(bytes.NewReader(compressed))) + if err != nil { + t.Fatalf("failed to inflate SAMLRequest: %v", err) + } + var req logoutRequest + if err := xml.Unmarshal(inflated, &req); err != nil { + t.Fatalf("failed to unmarshal LogoutRequest: %v", err) + } + return req +} + +const logoutRequestID = "_req456" + +// successLogoutResponseXML returns a minimal SAML LogoutResponse with Success status. +const successLogoutResponseXML = ` + https://idp.example.com + + + +` + +const failedLogoutResponseXML = ` + https://idp.example.com + + + Logout failed + +` + +func TestLogoutURL(t *testing.T) { + connNoSLO, err := (&Config{ + CA: "testdata/ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + connSLO, err := (&Config{ + CA: "testdata/ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + SLOURL: "http://idp.example.com/slo", + EntityIssuer: "http://127.0.0.1:5556/dex", + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + t.Run("SLONotConfigured", func(t *testing.T) { + connData, _ := json.Marshal(cachedIdentity{ + NameID: "user@example.com", + NameIDFormat: nameIDFormatEmailAddress, + }) + + u, state, err := connNoSLO.LogoutURLWithState(context.Background(), connData, "https://app.example.com/done") + if err != nil { + t.Fatalf("LogoutURL error: %v", err) + } + if u != "" { + t.Errorf("expected empty URL when SLO not configured, got %q", u) + } + if state != nil { + t.Errorf("expected nil state when SLO not configured, got %q", state) + } + }) + + t.Run("EmptyNameID", func(t *testing.T) { + connData, _ := json.Marshal(cachedIdentity{}) + + u, state, err := connSLO.LogoutURLWithState(context.Background(), connData, "https://app.example.com/done") + if err != nil { + t.Fatalf("LogoutURL error: %v", err) + } + if u != "" { + t.Errorf("expected empty URL when NameID is empty, got %q", u) + } + if state != nil { + t.Errorf("expected nil state when NameID is empty, got %q", state) + } + }) + + t.Run("NilConnectorData", func(t *testing.T) { + u, state, err := connSLO.LogoutURLWithState(context.Background(), nil, "https://app.example.com/done") + if err != nil { + t.Fatalf("LogoutURL error: %v", err) + } + if u != "" { + t.Errorf("expected empty URL with nil connector data, got %q", u) + } + if state != nil { + t.Errorf("expected nil state with nil connector data, got %q", state) + } + }) + + t.Run("ValidLogoutRequest", func(t *testing.T) { + connData, _ := json.Marshal(cachedIdentity{ + NameID: "user@example.com", + NameIDFormat: nameIDFormatEmailAddress, + SessionIndex: "session-abc-123", + }) + + u, state, err := connSLO.LogoutURLWithState(context.Background(), connData, "https://app.example.com/done") + if err != nil { + t.Fatalf("LogoutURL error: %v", err) + } + if u == "" { + t.Fatal("expected non-empty URL") + } + + parsed, err := url.Parse(u) + if err != nil { + t.Fatalf("failed to parse returned URL: %v", err) + } + + if parsed.Host != "idp.example.com" { + t.Errorf("unexpected host: %q", parsed.Host) + } + if parsed.Path != "/slo" { + t.Errorf("unexpected path: %q", parsed.Path) + } + req := decodeSAMLRequest(t, parsed.Query().Get("SAMLRequest")) + + // State is the bytes of the outgoing LogoutRequest ID; the server + // persists it in storage.LogoutState.ConnectorState so InResponseTo + // can be matched in HandleLogoutCallback. + if string(state) != req.ID { + t.Errorf("expected state to be request ID %q, got %q", req.ID, state) + } + // We intentionally do not emit RelayState (correlation is server-side). + if rs := parsed.Query().Get("RelayState"); rs != "" { + t.Errorf("expected no RelayState, got %q", rs) + } + + if req.NameID.Value != "user@example.com" { + t.Errorf("NameID mismatch: got %q", req.NameID.Value) + } + if req.NameID.Format != nameIDFormatEmailAddress { + t.Errorf("NameID Format mismatch: got %q", req.NameID.Format) + } + if req.Destination != "http://idp.example.com/slo" { + t.Errorf("Destination mismatch: got %q", req.Destination) + } + if req.Issuer == nil || req.Issuer.Issuer != "http://127.0.0.1:5556/dex" { + t.Errorf("Issuer mismatch: %+v", req.Issuer) + } + if len(req.SessionIndex) != 1 || req.SessionIndex[0].Value != "session-abc-123" { + t.Errorf("SessionIndex mismatch: %+v", req.SessionIndex) + } + if req.ID == "" { + t.Error("expected non-empty request ID") + } + }) + + t.Run("NoSessionIndex", func(t *testing.T) { + connData, _ := json.Marshal(cachedIdentity{ + NameID: "user@example.com", + NameIDFormat: nameIDFormatEmailAddress, + }) + + u, state, err := connSLO.LogoutURLWithState(context.Background(), connData, "") + if err != nil { + t.Fatalf("LogoutURL error: %v", err) + } + + parsed, err := url.Parse(u) + if err != nil { + t.Fatalf("failed to parse URL: %v", err) + } + + req := decodeSAMLRequest(t, parsed.Query().Get("SAMLRequest")) + + if string(state) != req.ID { + t.Errorf("expected state to be request ID %q, got %q", req.ID, state) + } + if len(req.SessionIndex) != 0 { + t.Errorf("expected no SessionIndex, got %+v", req.SessionIndex) + } + }) +} + +func TestSLOSigningKeyConfigErrors(t *testing.T) { + logger := slog.New(slog.DiscardHandler) + base := Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + } + t.Run("BothKeyAndData", func(t *testing.T) { + c := base + c.SLOSigningKey = "testdata/ca.key" + c.SLOSigningKeyData = []byte("dummy") + _, err := c.Open("saml", logger) + if err == nil { + t.Fatal("expected error when both sloSigningKey and sloSigningKeyData are set") + } + }) + t.Run("KeyWithoutSLOURL", func(t *testing.T) { + c := base + c.SLOSigningKey = "testdata/ca.key" + _, err := c.Open("saml", logger) + if err == nil { + t.Fatal("expected error when sloSigningKey is set without sloURL") + } + }) + t.Run("SLOURLWithoutEntityIssuer", func(t *testing.T) { + // Profile §4.4.4.1: MUST be present in LogoutRequest, so + // entityIssuer must be configured whenever SLO is enabled. + c := base + c.SLOURL = "http://idp.example.com/slo" + _, err := c.Open("saml", logger) + if err == nil { + t.Fatal("expected error when sloURL is set without entityIssuer") + } + }) +} + +func TestLogoutURLRedirectSigning(t *testing.T) { + keyPEM, err := os.ReadFile("testdata/ca.key") + if err != nil { + t.Fatal(err) + } + conn, err := (&Config{ + CA: "testdata/ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + SLOURL: "http://idp.example.com/slo", + SLOSigningKeyData: keyPEM, + EntityIssuer: "http://127.0.0.1:5556/dex", + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + connData, _ := json.Marshal(cachedIdentity{ + NameID: "user@example.com", + NameIDFormat: nameIDFormatEmailAddress, + }) + logoutURL, state, err := conn.LogoutURLWithState(context.Background(), connData, "https://app.example.com/done") + if err != nil { + t.Fatal(err) + } + if len(state) == 0 { + t.Fatal("expected non-empty state (LogoutRequest ID)") + } + parsed, err := url.Parse(logoutURL) + if err != nil { + t.Fatal(err) + } + if parsed.Query().Get("Signature") == "" { + t.Fatal("expected Signature query parameter") + } + if parsed.Query().Get("SigAlg") == "" { + t.Fatal("expected SigAlg query parameter") + } + req := httptest.NewRequest(http.MethodGet, logoutURL, nil) + if err := conn.validateRedirectSignature(req, "SAMLRequest"); err != nil { + t.Fatalf("signed LogoutURL failed verification: %v", err) + } + t.Run("TamperedQueryRejected", func(t *testing.T) { + bad := strings.Replace(logoutURL, "SAMLRequest=", "SAMLRequest=AAAA", 1) + badReq := httptest.NewRequest(http.MethodGet, bad, nil) + if err := conn.validateRedirectSignature(badReq, "SAMLRequest"); err == nil { + t.Error("expected verification error for tampered SAMLRequest") + } + }) +} + +func TestHandleLogoutCallback(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + // Match the IssueInstant in successLogoutResponseXML / failedLogoutResponseXML. + respTime, _ := time.Parse(timeFormat, "2024-01-01T00:00:00Z") + conn.now = func() time.Time { return respTime } + + // successLogoutResponseXML's InResponseTo is "_req456"; pass matching state. + successState := []byte(logoutRequestID) + + t.Run("EmptySAMLResponse", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/logout/callback", nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err == nil { + t.Error("expected error for empty SAMLResponse") + } + }) + + t.Run("ValidLogoutResponse", func(t *testing.T) { + encoded := redirectBindingEncode(t, successLogoutResponseXML) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err != nil { + t.Errorf("expected no error for valid response, got: %v", err) + } + }) + + t.Run("FailedStatus", func(t *testing.T) { + encoded := redirectBindingEncode(t, failedLogoutResponseXML) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err == nil { + t.Error("expected error for failed status") + } + }) + + t.Run("InvalidBase64", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse=not-valid-base64!!!", nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err == nil { + t.Error("expected error for invalid base64") + } + }) + + t.Run("GETRequiresDeflate", func(t *testing.T) { + // HTTP-Redirect uses DEFLATE; raw base64(XML) without DEFLATE must be rejected. + encoded := base64.StdEncoding.EncodeToString([]byte(successLogoutResponseXML)) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err == nil { + t.Error("expected error for non-deflated GET SAMLResponse") + } + }) + + t.Run("InvalidXML", func(t *testing.T) { + encoded := redirectBindingEncode(t, "not xml at all") + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err == nil { + t.Error("expected error for invalid XML") + } + }) + + t.Run("POSTBinding", func(t *testing.T) { + encoded := base64.StdEncoding.EncodeToString([]byte(successLogoutResponseXML)) + form := url.Values{"SAMLResponse": {encoded}} + req := httptest.NewRequest(http.MethodPost, "/logout/callback", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err != nil { + t.Errorf("expected no error for POST binding, got: %v", err) + } + }) + + t.Run("DeflatedResponse", func(t *testing.T) { + encoded := redirectBindingEncode(t, successLogoutResponseXML) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, successState); err != nil { + t.Errorf("expected no error for deflated response, got: %v", err) + } + }) +} + +func TestHandleLogoutCallbackRequiredFields(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + respTime, _ := time.Parse(timeFormat, "2024-01-01T00:00:00Z") + conn.now = func() time.Time { return respTime } + + tests := []struct { + name string + xml string + }{ + { + name: "ID", + xml: strings.Replace(successLogoutResponseXML, + ` ID="_resp123"`, "", 1), + }, + { + name: "Version", + xml: strings.Replace(successLogoutResponseXML, + ` Version="2.0"`, "", 1), + }, + { + name: "IssueInstant", + xml: strings.Replace(successLogoutResponseXML, + ` IssueInstant="2024-01-01T00:00:00Z"`, "", 1), + }, + { + name: "Issuer", + xml: strings.Replace(successLogoutResponseXML, + "\n\thttps://idp.example.com", "", 1), + }, + { + name: "Status", + xml: strings.Replace(successLogoutResponseXML, + "\n\t\n\t\t\n\t", "", 1), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + encoded := redirectBindingEncode(t, tc.xml) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Errorf("expected missing %s to be rejected", tc.name) + } + }) + } +} + +func TestHandleLogoutCallbackSizeLimits(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + t.Run("InflatedResponse", func(t *testing.T) { + encoded := redirectBindingEncode(t, strings.Repeat(" ", maxSAMLLogoutResponseSize+1)) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)) + if err == nil || !strings.Contains(err.Error(), "after decoding") { + t.Fatalf("expected inflated size error, got %v", err) + } + }) + + t.Run("POSTBody", func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/logout/callback", + strings.NewReader(strings.Repeat("A", maxSAMLLogoutPOSTBodySize+1))) + err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)) + if err == nil || !strings.Contains(err.Error(), "POST body exceeds") { + t.Fatalf("expected POST body size error, got %v", err) + } + }) +} + +func TestHandleLogoutCallbackIssuerValidation(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + SSOIssuer: "https://correct-idp.example.com", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + t.Run("MatchingIssuer", func(t *testing.T) { + xml := fmt.Sprintf(` + https://correct-idp.example.com + + `, time.Now().UTC().Format(timeFormat), logoutRequestID) + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected no error, got: %v", err) + } + }) + + t.Run("MismatchedIssuer", func(t *testing.T) { + xml := fmt.Sprintf(` + https://evil-idp.example.com + + `, time.Now().UTC().Format(timeFormat)) + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for mismatched issuer") + } + }) + + t.Run("MissingIssuer", func(t *testing.T) { + // Profile §4.4.4.2 — missing Issuer must be rejected when ssoIssuer is configured. + xml := fmt.Sprintf(` + + `, time.Now().UTC().Format(timeFormat)) + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for missing Issuer when ssoIssuer is configured") + } + }) +} + +func TestHandleLogoutCallbackDestination(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + inst := time.Now().UTC().Format(timeFormat) + makeResp := func(dest string) string { + destAttr := "" + if dest != "" { + destAttr = fmt.Sprintf(` Destination="%s"`, dest) + } + return fmt.Sprintf(` + https://idp.example.com + + `, inst, logoutRequestID, destAttr) + } + + t.Run("MatchingAbsoluteURL", func(t *testing.T) { + dest := "https://dex.example.com/logout/callback" + enc := redirectBindingEncode(t, makeResp(dest)) + u := "https://dex.example.com/logout/callback?SAMLResponse=" + url.QueryEscape(enc) + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected nil, got %v", err) + } + }) + + t.Run("TrailingSlashEquivalence", func(t *testing.T) { + dest := "https://dex.example.com/logout/callback/" + enc := redirectBindingEncode(t, makeResp(dest)) + u := "https://dex.example.com/logout/callback?SAMLResponse=" + url.QueryEscape(enc) + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected nil, got %v", err) + } + }) + + t.Run("CaseInsensitiveSchemeHost", func(t *testing.T) { + // RFC 3986 mandates case-insensitive comparison for scheme and host. + dest := "HTTPS://Dex.Example.COM/logout/callback" + enc := redirectBindingEncode(t, makeResp(dest)) + u := "https://dex.example.com/logout/callback?SAMLResponse=" + url.QueryEscape(enc) + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected nil for case-only scheme/host difference, got %v", err) + } + }) + + t.Run("MismatchedDestination", func(t *testing.T) { + enc := redirectBindingEncode(t, makeResp("https://evil.example.com/callback")) + u := "https://dex.example.com/logout/callback?SAMLResponse=" + url.QueryEscape(enc) + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for wrong Destination") + } + }) + + t.Run("XForwardedProtoAndHost", func(t *testing.T) { + dest := "https://public.example.com/dex/logout/callback" + enc := redirectBindingEncode(t, makeResp(dest)) + u := "http://10.0.0.5/dex/logout/callback?SAMLResponse=" + url.QueryEscape(enc) + req := httptest.NewRequest(http.MethodGet, u, nil) + req.Header.Set("X-Forwarded-Proto", "https") + req.Header.Set("X-Forwarded-Host", "public.example.com") + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected nil, got %v", err) + } + }) +} + +func TestHandleLogoutCallbackIssueInstantFreshness(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + t.Run("FreshResponse", func(t *testing.T) { + conn.now = func() time.Time { return time.Now() } + xml := fmt.Sprintf(` + https://idp.example.com + + `, time.Now().UTC().Format(timeFormat), logoutRequestID) + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected no error for fresh response, got: %v", err) + } + }) + + t.Run("StaleResponse", func(t *testing.T) { + conn.now = func() time.Time { return time.Now() } + stale := time.Now().Add(-10 * time.Minute).UTC().Format(timeFormat) + xml := fmt.Sprintf(` + https://idp.example.com + + `, stale, logoutRequestID) + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for stale IssueInstant") + } + }) + + t.Run("FutureResponse", func(t *testing.T) { + conn.now = func() time.Time { return time.Now() } + future := time.Now().Add(5 * time.Minute).UTC().Format(timeFormat) + xml := fmt.Sprintf(` + https://idp.example.com + + `, future, logoutRequestID) + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for future IssueInstant") + } + }) +} + +func TestHandleLogoutCallbackInResponseTo(t *testing.T) { + conn, err := (&Config{ + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + InsecureSkipSignatureValidation: true, + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + conn.now = func() time.Time { return time.Now() } + + makeResponse := func(inResponseTo string) string { + return fmt.Sprintf(` + https://idp.example.com + + `, time.Now().UTC().Format(timeFormat), inResponseTo) + } + + // state is what server-side LogoutState.ConnectorState carries: the + // outgoing LogoutRequest ID that LogoutURL produced for this user. + t.Run("MatchingID", func(t *testing.T) { + xml := makeResponse("_abc123") + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte("_abc123")); err != nil { + t.Errorf("expected no error, got: %v", err) + } + }) + + t.Run("MismatchedID", func(t *testing.T) { + xml := makeResponse("_wrong") + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte("_abc123")); err == nil { + t.Error("expected error for InResponseTo mismatch") + } + }) + + t.Run("MissingStateRejected", func(t *testing.T) { + xml := makeResponse("_anything") + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, nil); err == nil { + t.Error("expected error when server-side request ID is missing") + } + }) + + t.Run("RelayStateIgnored", func(t *testing.T) { + // Replay defense: a captured LogoutResponse with an attacker-supplied + // RelayState must NOT be accepted just because RelayState matches — + // only server-side state counts. + xml := makeResponse("_attacker_chosen") + encoded := redirectBindingEncode(t, xml) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded)+ + "&RelayState="+url.QueryEscape("_attacker_chosen"), + nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte("_real_request_id")); err == nil { + t.Error("expected error: RelayState must not satisfy InResponseTo check") + } + }) +} + +// signXMLDocument signs an etree document using the test CA key/cert pair +// and returns the resulting XML bytes. +func signXMLDocument(t *testing.T, doc *etree.Document) []byte { + t.Helper() + tlsCert, err := tls.LoadX509KeyPair("testdata/ca.crt", "testdata/ca.key") + if err != nil { + t.Fatalf("failed to load test key pair: %v", err) + } + keyStore := dsig.TLSCertKeyStore(tlsCert) + sigCtx := dsig.NewDefaultSigningContext(keyStore) + + signed, err := sigCtx.SignEnveloped(doc.Root()) + if err != nil { + t.Fatalf("failed to sign XML: %v", err) + } + + signedDoc := etree.NewDocument() + signedDoc.SetRoot(signed) + out, err := signedDoc.WriteToBytes() + if err != nil { + t.Fatalf("failed to serialize signed XML: %v", err) + } + return out +} + +func postSAMLResponse(encoded string) *http.Request { + form := url.Values{"SAMLResponse": {encoded}} + req := httptest.NewRequest(http.MethodPost, "/logout/callback", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +func TestHandleLogoutCallbackPOSTSignatureValidation(t *testing.T) { + conn, err := (&Config{ + CA: "testdata/ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + respTime, _ := time.Parse(timeFormat, "2024-01-01T00:00:00Z") + conn.now = func() time.Time { return respTime } + + t.Run("ValidSignature", func(t *testing.T) { + doc := etree.NewDocument() + if err := doc.ReadFromString(successLogoutResponseXML); err != nil { + t.Fatal(err) + } + signedXML := signXMLDocument(t, doc) + encoded := base64.StdEncoding.EncodeToString(signedXML) + + if err := conn.HandleLogoutCallbackWithState(context.Background(), postSAMLResponse(encoded), []byte(logoutRequestID)); err != nil { + t.Errorf("expected no error for validly signed response, got: %v", err) + } + }) + + t.Run("InvalidSignature", func(t *testing.T) { + encoded := base64.StdEncoding.EncodeToString([]byte(successLogoutResponseXML)) + if err := conn.HandleLogoutCallbackWithState(context.Background(), postSAMLResponse(encoded), []byte(logoutRequestID)); err == nil { + t.Error("expected error for unsigned response when signature validation is enabled") + } + }) + + t.Run("WrongCA", func(t *testing.T) { + connBadCA, err := (&Config{ + CA: "testdata/bad-ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + doc := etree.NewDocument() + if err := doc.ReadFromString(successLogoutResponseXML); err != nil { + t.Fatal(err) + } + signedXML := signXMLDocument(t, doc) + encoded := base64.StdEncoding.EncodeToString(signedXML) + + if err := connBadCA.HandleLogoutCallbackWithState(context.Background(), postSAMLResponse(encoded), []byte(logoutRequestID)); err == nil { + t.Error("expected error when response is signed with different CA") + } + }) +} + +// signRedirectBinding builds a complete URL for a GET LogoutResponse with +// SAML HTTP-Redirect binding signature. The XML is deflated, base64-encoded, +// and a query-string RSA-SHA256 signature is appended. +func signRedirectBinding(t *testing.T, xmlPayload string, keyFile, certFile string) string { + t.Helper() + + var buf bytes.Buffer + fw, err := flate.NewWriter(&buf, flate.DefaultCompression) + if err != nil { + t.Fatalf("deflate writer: %v", err) + } + if _, err := fw.Write([]byte(xmlPayload)); err != nil { + t.Fatalf("deflate write: %v", err) + } + if err := fw.Close(); err != nil { + t.Fatalf("deflate close: %v", err) + } + samlResp := base64.StdEncoding.EncodeToString(buf.Bytes()) + + sigAlg := "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" + signedContent := "SAMLResponse=" + url.QueryEscape(samlResp) + + "&SigAlg=" + url.QueryEscape(sigAlg) + + tlsCert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + t.Fatalf("load key pair: %v", err) + } + rsaKey, ok := tlsCert.PrivateKey.(*rsa.PrivateKey) + if !ok { + t.Fatal("test key is not RSA") + } + + h := crypto.SHA256.New() + h.Write([]byte(signedContent)) + sig, err := rsa.SignPKCS1v15(rand.Reader, rsaKey, crypto.SHA256, h.Sum(nil)) + if err != nil { + t.Fatalf("sign: %v", err) + } + sigB64 := base64.StdEncoding.EncodeToString(sig) + + return "/logout/callback?" + signedContent + + "&Signature=" + url.QueryEscape(sigB64) +} + +func TestHandleLogoutCallbackRedirectSignatureValidation(t *testing.T) { + conn, err := (&Config{ + CA: "testdata/ca.crt", + UsernameAttr: "Name", + EmailAttr: "email", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + }).openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + respTime, _ := time.Parse(timeFormat, "2024-01-01T00:00:00Z") + conn.now = func() time.Time { return respTime } + + t.Run("ValidSignature", func(t *testing.T) { + u := signRedirectBinding(t, successLogoutResponseXML, "testdata/ca.key", "testdata/ca.crt") + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err != nil { + t.Errorf("expected no error, got: %v", err) + } + }) + + t.Run("MissingSignature", func(t *testing.T) { + var buf bytes.Buffer + fw, _ := flate.NewWriter(&buf, flate.DefaultCompression) + fw.Write([]byte(successLogoutResponseXML)) + fw.Close() + encoded := base64.StdEncoding.EncodeToString(buf.Bytes()) + + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for missing Signature parameter") + } + }) + + t.Run("SignatureCheckedBeforeInflation", func(t *testing.T) { + encoded := redirectBindingEncode(t, strings.Repeat(" ", maxSAMLLogoutResponseSize+1)) + req := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)) + if err == nil || !strings.Contains(err.Error(), "missing Signature") { + t.Fatalf("expected signature error before inflation, got %v", err) + } + }) + + t.Run("WrongCA", func(t *testing.T) { + u := signRedirectBinding(t, successLogoutResponseXML, "testdata/bad-ca.key", "testdata/bad-ca.crt") + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error when signed with wrong CA") + } + }) + + t.Run("TamperedPayload", func(t *testing.T) { + u := signRedirectBinding(t, successLogoutResponseXML, "testdata/ca.key", "testdata/ca.crt") + // Replace part of the SAMLResponse value to simulate tampering. + u = strings.Replace(u, "SAMLResponse=", "SAMLResponse=AAAA", 1) + req := httptest.NewRequest(http.MethodGet, u, nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), req, []byte(logoutRequestID)); err == nil { + t.Error("expected error for tampered payload") + } + }) +} + +func TestSLOEndToEnd(t *testing.T) { + c := Config{ + UsernameAttr: "Name", + EmailAttr: "email", + GroupsAttr: "groups", + RedirectURI: "http://127.0.0.1:5556/dex/callback", + SSOURL: "http://foo.bar/", + SLOURL: "http://idp.example.com/slo", + // EntityIssuer is required when SLOURL is set (Profile §4.4.4.1). + // It also drives audience validation, so it must match good-resp.xml's + // http://127.0.0.1:5556/dex/callback. + EntityIssuer: "http://127.0.0.1:5556/dex/callback", + InsecureSkipSignatureValidation: true, + } + + conn, err := c.openConnector(slog.New(slog.DiscardHandler)) + if err != nil { + t.Fatal(err) + } + + // Step 1: HandlePOST — simulate login, extract ConnectorData + now, err := time.Parse(timeFormat, "2017-04-04T04:34:59.330Z") + if err != nil { + t.Fatal(err) + } + conn.now = func() time.Time { return now } + + resp, err := os.ReadFile("testdata/good-resp.xml") + if err != nil { + t.Fatal(err) + } + samlResp := base64.StdEncoding.EncodeToString(resp) + + scopes := connector.Scopes{OfflineAccess: true, Groups: true} + ident, err := conn.HandlePOST(scopes, samlResp, "6zmm5mguyebwvajyf2sdwwcw6m") + if err != nil { + t.Fatalf("HandlePOST failed: %v", err) + } + + if len(ident.ConnectorData) == 0 { + t.Fatal("expected ConnectorData after HandlePOST") + } + + // Step 2: LogoutURL — build logout redirect URL + connector state from ConnectorData + conn.now = func() time.Time { return time.Now() } + logoutURL, connectorState, err := conn.LogoutURLWithState(context.Background(), ident.ConnectorData, "https://dex.example.com/logout/callback") + if err != nil { + t.Fatalf("LogoutURL failed: %v", err) + } + if logoutURL == "" { + t.Fatal("expected non-empty logout URL") + } + if len(connectorState) == 0 { + t.Fatal("expected non-empty connector state (LogoutRequest ID)") + } + + parsed, err := url.Parse(logoutURL) + if err != nil { + t.Fatalf("failed to parse logout URL: %v", err) + } + + logReq := decodeSAMLRequest(t, parsed.Query().Get("SAMLRequest")) + if logReq.NameID.Value != ident.UserID { + t.Errorf("LogoutRequest NameID should match HandlePOST UserID: got %q, want %q", logReq.NameID.Value, ident.UserID) + } + if logReq.NameID.Format != "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" { + t.Errorf("LogoutRequest NameID format mismatch: got %q", logReq.NameID.Format) + } + if len(logReq.SessionIndex) != 1 || logReq.SessionIndex[0].Value != "6zmm5mguyebwvajyf2sdwwcw6m" { + t.Errorf("LogoutRequest SessionIndex mismatch: %+v", logReq.SessionIndex) + } + // §4.4.4.1: MUST be present. + if logReq.Issuer == nil || logReq.Issuer.Issuer != "http://127.0.0.1:5556/dex/callback" { + t.Errorf("LogoutRequest Issuer mismatch: %+v", logReq.Issuer) + } + + // Connector state must be the outgoing request ID; the server stores it + // in storage.LogoutState.ConnectorState and hands it back below. + if string(connectorState) != logReq.ID { + t.Errorf("connector state should equal request ID %q, got %q", logReq.ID, connectorState) + } + + // Step 3: HandleLogoutCallback — simulate IdP response with matching InResponseTo. + // Note: no RelayState is sent or expected; correlation is purely server-side. + logoutResponseXML := fmt.Sprintf(` + https://idp.example.com + + + +`, time.Now().UTC().Format(timeFormat), logReq.ID) + + encoded := redirectBindingEncode(t, logoutResponseXML) + callbackReq := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(encoded), nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), callbackReq, connectorState); err != nil { + t.Fatalf("HandleLogoutCallback failed: %v", err) + } + + // Step 4: InResponseTo mismatch must be detected even when an attacker + // pre-fills RelayState — only server-side state counts. + badResponseXML := fmt.Sprintf(` + https://idp.example.com + + + +`, time.Now().UTC().Format(timeFormat)) + + badEncoded := redirectBindingEncode(t, badResponseXML) + badCallbackReq := httptest.NewRequest(http.MethodGet, + "/logout/callback?SAMLResponse="+url.QueryEscape(badEncoded)+ + "&RelayState="+url.QueryEscape("_wrong_id"), + nil) + if err := conn.HandleLogoutCallbackWithState(context.Background(), badCallbackReq, connectorState); err == nil { + t.Error("expected error for InResponseTo mismatch") + } +} diff --git a/connector/saml/types.go b/connector/saml/types.go index c8d7e7f3b3..55efd73a64 100644 --- a/connector/saml/types.go +++ b/connector/saml/types.go @@ -27,7 +27,9 @@ func (t *xmlTime) UnmarshalXMLAttr(attr xml.Attr) error { return nil } -type samlVersion struct{} +type samlVersion struct { + present bool +} func (s samlVersion) MarshalXMLAttr(name xml.Name) (xml.Attr, error) { return xml.Attr{ @@ -40,6 +42,7 @@ func (s *samlVersion) UnmarshalXMLAttr(attr xml.Attr) error { if attr.Value != "2.0" { return fmt.Errorf(`saml version expected "2.0" got %q`, attr.Value) } + s.present = true return nil } @@ -80,7 +83,7 @@ type subject struct { type nameID struct { XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion NameID"` - Format string `xml:"Format,omitempty"` + Format string `xml:"Format,attr,omitempty"` Value string `xml:",chardata"` } @@ -191,9 +194,15 @@ type assertion struct { Conditions *conditions `xml:"Conditions"` + AuthnStatements []authnStatement `xml:"AuthnStatement,omitempty"` AttributeStatement *attributeStatement `xml:"AttributeStatement,omitempty"` } +type authnStatement struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion AuthnStatement"` + SessionIndex string `xml:"SessionIndex,attr,omitempty"` +} + type attributeStatement struct { XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:assertion AttributeStatement"` @@ -275,3 +284,34 @@ func (a attribute) String() string { // "groups" = ["engineering", "docs"] return fmt.Sprintf("%q = %q", a.Name, values) } + +type logoutRequest struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol LogoutRequest"` + + ID string `xml:"ID,attr"` + Version samlVersion `xml:"Version,attr"` + IssueInstant xmlTime `xml:"IssueInstant,attr"` + Destination string `xml:"Destination,attr,omitempty"` + + Issuer *issuer `xml:"Issuer,omitempty"` + NameID nameID `xml:"NameID"` + SessionIndex []sessionIndex `xml:"SessionIndex,omitempty"` +} + +type sessionIndex struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol SessionIndex"` + Value string `xml:",chardata"` +} + +type logoutResponse struct { + XMLName xml.Name `xml:"urn:oasis:names:tc:SAML:2.0:protocol LogoutResponse"` + + ID string `xml:"ID,attr"` + InResponseTo string `xml:"InResponseTo,attr,omitempty"` + Version samlVersion `xml:"Version,attr"` + IssueInstant xmlTime `xml:"IssueInstant,attr,omitempty"` + Destination string `xml:"Destination,attr,omitempty"` + + Issuer *issuer `xml:"Issuer,omitempty"` + Status *status `xml:"Status"` +} diff --git a/docs/enhancements/auth-sessions-2026-02-18.md b/docs/enhancements/auth-sessions-2026-02-18.md index 28f5208ccb..7955d598cf 100644 --- a/docs/enhancements/auth-sessions-2026-02-18.md +++ b/docs/enhancements/auth-sessions-2026-02-18.md @@ -1252,7 +1252,8 @@ For OAuth/OIDC/SAML connectors, the user is redirected to upstream IDP and there **CallbackConnector** (OIDC, OAuth, SAML, GitHub, etc.): - Session created after successful callback -- Upstream tokens stored in refresh token's ConnectorData (not in session) +- ConnectorData is copied into the AuthSession for stateful upstream logout; + refresh-token flows continue to store it with the refresh token as well - Identity refresh via RefreshConnector when refresh token is used **PasswordConnector** (LDAP, local passwords): diff --git a/server/logout/logout.go b/server/logout/logout.go index eab88611b6..ba4e5edb14 100644 --- a/server/logout/logout.go +++ b/server/logout/logout.go @@ -1,6 +1,7 @@ package logout import ( + "bytes" "context" "crypto/subtle" "errors" @@ -231,6 +232,17 @@ type idTokenHint struct { sessionID string // "sid", empty for tokens minted before sid existed } +func logoutStatesEqual(a, b *storage.LogoutState) bool { + if a == nil || b == nil { + return a == b + } + return a.PostLogoutRedirectURI == b.PostLogoutRedirectURI && + a.State == b.State && + a.ClientID == b.ClientID && + a.ConnectorID == b.ConnectorID && + bytes.Equal(a.ConnectorState, b.ConnectorState) +} + // matches reports whether the hint describes the given session. A hint carrying a sid // must match it exactly; one without a sid falls back to the subject alone, which is // all a token issued by an older dex can offer. @@ -324,16 +336,83 @@ func (h *Handler) handleLogoutCallback(w http.ResponseWriter, r *http.Request) { ls := session.LogoutState // Let the connector validate the upstream logout response if it supports it. + // Prefer StatefulLogoutCallbackConnector (replays ls.ConnectorState — e.g. + // SAML's outgoing LogoutRequest ID for InResponseTo correlation) and fall + // back to the simpler LogoutCallbackConnector for stateless connectors. + var statefulLogoutErr error if ls.ConnectorID != "" { conn, err := h.Connectors.Get(ctx, ls.ConnectorID) - if err == nil { - if logoutConn, ok := conn.Connector.(connector.LogoutCallbackConnector); ok { - if err := logoutConn.HandleLogoutCallback(ctx, r); err != nil { + if err != nil { + // The upstream connector vanished between the outgoing logout + // redirect and this callback. Without it the response cannot be + // validated, so make the failure visible to the operator. + h.Logger.ErrorContext(ctx, "logout: failed to resolve connector for callback validation", + "connector_id", ls.ConnectorID, "err", err) + if len(ls.ConnectorState) > 0 { + statefulLogoutErr = fmt.Errorf("resolve stateful logout connector: %w", err) + } + } else { + switch logoutConn := conn.Connector.(type) { + case connector.StatefulLogoutCallbackConnector: + if err := logoutConn.HandleLogoutCallbackWithState(ctx, r, ls.ConnectorState); err != nil { h.Logger.ErrorContext(ctx, "logout: upstream logout response validation failed", "connector_id", ls.ConnectorID, "err", err) + statefulLogoutErr = err } + case connector.LogoutCallbackConnector: + if len(ls.ConnectorState) > 0 { + statefulLogoutErr = fmt.Errorf("connector %q no longer supports stateful logout callbacks", ls.ConnectorID) + } else if err := logoutConn.HandleLogoutCallback(ctx, r); err != nil { + h.Logger.ErrorContext(ctx, "logout: upstream logout response validation failed", + "connector_id", ls.ConnectorID, "err", err) + } + default: + if len(ls.ConnectorState) > 0 { + statefulLogoutErr = fmt.Errorf("connector %q does not support logout callbacks", ls.ConnectorID) + } + } + } + } else if len(ls.ConnectorState) > 0 { + statefulLogoutErr = fmt.Errorf("stateful logout has no connector ID") + } + + // Stateful connectors (e.g. SAML) perform cryptographic validation; do not + // complete Dex logout if that fails. Otherwise a forged request carrying a + // valid session cookie could clear the session without a valid IdP response. + if statefulLogoutErr != nil { + // Clear the one-shot correlation state so it cannot be replayed. The + // browser session remains active and can start a fresh logout attempt. + if err := h.Storage.UpdateAuthSession(ctx, session.ID, func(old storage.AuthSession) (storage.AuthSession, error) { + if logoutStatesEqual(old.LogoutState, ls) { + old.LogoutState = nil } + return old, nil + }); err != nil { + h.Logger.ErrorContext(ctx, "logout: failed to clear LogoutState after failed validation", + "connector_id", ls.ConnectorID, "err", err) } + h.renderError(r, w, http.StatusBadRequest, "Upstream logout response validation failed.") + return + } + + // Consume this exact one-shot state before ending the session. A stale + // callback must not complete a newer logout attempt that replaced it. + consumed := false + if err := h.Storage.UpdateAuthSession(ctx, session.ID, func(old storage.AuthSession) (storage.AuthSession, error) { + if logoutStatesEqual(old.LogoutState, ls) { + old.LogoutState = nil + consumed = true + } + return old, nil + }); err != nil { + h.Logger.ErrorContext(ctx, "logout: failed to consume LogoutState", + "connector_id", ls.ConnectorID, "err", err) + h.renderError(r, w, http.StatusInternalServerError, "Failed to complete logout.") + return + } + if !consumed { + h.renderError(r, w, http.StatusBadRequest, "Logout request is no longer current.") + return } // The session actually ends here on the upstream path, so this is where its bound @@ -388,31 +467,39 @@ func (h *Handler) tryUpstreamLogout(ctx context.Context, authSession *storage.Au return "", false } - logoutConn, ok := conn.Connector.(connector.LogoutCallbackConnector) - if !ok { - return "", false - } - - // Store logout parameters in the session. - if err := h.Storage.UpdateAuthSession(ctx, authSession.ID, func(old storage.AuthSession) (storage.AuthSession, error) { - old.LogoutState = &storage.LogoutState{ - PostLogoutRedirectURI: postLogoutRedirectURI, - State: state, - ClientID: clientID, - ConnectorID: connectorID, - } - return old, nil - }); err != nil { - h.Logger.ErrorContext(ctx, "logout: failed to save logout state", "err", err) + // Connectors may implement either the basic LogoutCallbackConnector + // or its stateful variant. Probe for the richer interface first so + // connectors that need server-side correlation state (e.g. SAML's + // LogoutRequest ID for InResponseTo) can hand it back to us. + statefulConn, hasState := conn.Connector.(connector.StatefulLogoutCallbackConnector) + basicConn, hasBasic := conn.Connector.(connector.LogoutCallbackConnector) + if !hasState && !hasBasic { return "", false } callbackURI := h.IssuerURL.AbsURL("/logout/callback") - upstreamURL, err := logoutConn.LogoutURL(ctx, callbackURI) + var ( + upstreamURL string + connectorState []byte + ) + if hasState { + upstreamURL, connectorState, err = statefulConn.LogoutURLWithState( + ctx, + slices.Clone(authSession.ConnectorData), + callbackURI, + ) + } else { + upstreamURL, err = basicConn.LogoutURL(ctx, callbackURI) + } if err != nil { h.Logger.ErrorContext(ctx, "logout: upstream connector error", "err", err) return "", false } + if hasState && upstreamURL != "" && len(connectorState) == 0 { + h.Logger.ErrorContext(ctx, "logout: stateful connector returned no callback state", + "connector_id", connectorID) + return "", false + } if upstreamURL == "" { return "", false } @@ -423,6 +510,20 @@ func (h *Handler) tryUpstreamLogout(ctx context.Context, authSession *storage.Au return "", false } + if err := h.Storage.UpdateAuthSession(ctx, authSession.ID, func(old storage.AuthSession) (storage.AuthSession, error) { + old.LogoutState = &storage.LogoutState{ + PostLogoutRedirectURI: postLogoutRedirectURI, + State: state, + ClientID: clientID, + ConnectorID: connectorID, + ConnectorState: connectorState, + } + return old, nil + }); err != nil { + h.Logger.ErrorContext(ctx, "logout: failed to save logout state", "err", err) + return "", false + } + return u.String(), true } diff --git a/server/server_logout_test.go b/server/server_logout_test.go index 9443e742c4..3c9a013e7d 100644 --- a/server/server_logout_test.go +++ b/server/server_logout_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -15,12 +16,74 @@ import ( "github.com/stretchr/testify/require" + "github.com/dexidp/dex/connector" "github.com/dexidp/dex/server/discovery" "github.com/dexidp/dex/server/internal" "github.com/dexidp/dex/server/tokens" "github.com/dexidp/dex/storage" ) +// fakeStatefulLogoutConnector always reports a validation failure from +// HandleLogoutCallbackWithState. +type fakeStatefulLogoutConnector struct{} + +func (fakeStatefulLogoutConnector) LoginURL(connector.Scopes, string, string) (string, error) { + return "", nil +} + +func (fakeStatefulLogoutConnector) LogoutURLWithState(_ context.Context, _ []byte, _ string) (string, []byte, error) { + return "https://idp.example.com/slo", []byte("_req_id"), nil +} + +func (fakeStatefulLogoutConnector) HandleLogoutCallbackWithState(_ context.Context, _ *http.Request, _ []byte) error { + return errors.New("forced validation failure for tests") +} + +type capturingStatefulLogoutConnector struct { + connectorData *[]byte +} + +func (capturingStatefulLogoutConnector) LoginURL(connector.Scopes, string, string) (string, error) { + return "", nil +} + +func (c capturingStatefulLogoutConnector) LogoutURLWithState(_ context.Context, connectorData []byte, _ string) (string, []byte, error) { + *c.connectorData = append((*c.connectorData)[:0], connectorData...) + return "https://idp.example.com/slo", []byte("_req_id"), nil +} + +func (capturingStatefulLogoutConnector) HandleLogoutCallbackWithState(_ context.Context, _ *http.Request, _ []byte) error { + return nil +} + +type replacingStatefulLogoutConnector struct { + store storage.Storage + sessionID string + callbackErr error +} + +func (replacingStatefulLogoutConnector) LoginURL(connector.Scopes, string, string) (string, error) { + return "", nil +} + +func (replacingStatefulLogoutConnector) LogoutURLWithState(_ context.Context, _ []byte, _ string) (string, []byte, error) { + return "https://idp.example.com/slo", []byte("_new_req_id"), nil +} + +func (c replacingStatefulLogoutConnector) HandleLogoutCallbackWithState(ctx context.Context, _ *http.Request, _ []byte) error { + err := c.store.UpdateAuthSession(ctx, c.sessionID, func(old storage.AuthSession) (storage.AuthSession, error) { + old.LogoutState = &storage.LogoutState{ + ConnectorID: old.ConnectorID, + ConnectorState: []byte("_new_req_id"), + } + return old, nil + }) + if err != nil { + return err + } + return c.callbackErr +} + func TestHandleLogoutNoSessions(t *testing.T) { httpServer, server := newTestServer(t, nil) defer httpServer.Close() @@ -431,6 +494,191 @@ func TestHandleLogoutFromCookie(t *testing.T) { } } +func TestLogoutCallbackStatefulFailureKeepsSessionClearsLogoutState(t *testing.T) { + httpServer, server := newTestServerWithSessions(t, nil) + defer httpServer.Close() + + ctx := t.Context() + sessionID := "test-session" + connectorID := "stateful-fake" + + registerTestConnector(t, server, connectorID, fakeStatefulLogoutConnector{}) + + require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{ + ID: sessionID, + Secret: testSessionSecret(sessionID), + UserID: "test-user", + ConnectorID: connectorID, + CreatedAt: time.Now(), + LastActivity: time.Now(), + LogoutState: &storage.LogoutState{ + ConnectorID: connectorID, + ConnectorState: []byte("_req_id"), + }, + })) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/logout/callback", nil) + req.AddCookie(testSessionCookie(sessionID)) + server.ServeHTTP(rr, req) + + require.Equal(t, http.StatusBadRequest, rr.Code) + + got, err := server.storage.GetAuthSession(ctx, sessionID) + require.NoError(t, err, "session must survive failed stateful logout validation") + require.Nil(t, got.LogoutState, "LogoutState must be cleared after failed validation") + + for _, cookie := range rr.Result().Cookies() { + if cookie.Name == "dex_session" { + require.NotEqual(t, -1, cookie.MaxAge, "session cookie must not be cleared on failure") + } + } +} + +func TestUpstreamLogoutUsesAuthSessionConnectorData(t *testing.T) { + httpServer, server := newTestServerWithSessions(t, nil) + defer httpServer.Close() + + ctx := t.Context() + sessionID := "test-session-connector-data" + connectorID := "stateful-capturing-fake" + wantConnectorData := []byte(`{"nameID":"browser-user","sessionIndex":"browser-session"}`) + var gotConnectorData []byte + registerTestConnector(t, server, connectorID, capturingStatefulLogoutConnector{ + connectorData: &gotConnectorData, + }) + + require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{ + ID: sessionID, + Secret: testSessionSecret(sessionID), + UserID: "test-user", + ConnectorID: connectorID, + ConnectorData: wantConnectorData, + CreatedAt: time.Now(), + LastActivity: time.Now(), + })) + require.NoError(t, server.storage.CreateOfflineSessions(ctx, storage.OfflineSessions{ + UserID: "test-user", + ConnID: connectorID, + ConnectorData: []byte(`{"nameID":"other-device","sessionIndex":"other-session"}`), + Refresh: map[string]*storage.RefreshTokenRef{}, + })) + + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/logout", nil) + req.AddCookie(testSessionCookie(sessionID)) + server.ServeHTTP(rr, req) + + require.Equal(t, http.StatusSeeOther, rr.Code) + require.Equal(t, "https://idp.example.com/slo", rr.Header().Get("Location")) + require.Equal(t, wantConnectorData, gotConnectorData) +} + +func TestLogoutCallbackFailureDoesNotClearNewerLogoutState(t *testing.T) { + httpServer, server := newTestServerWithSessions(t, nil) + defer httpServer.Close() + + ctx := t.Context() + sessionID := "test-session-race" + connectorID := "stateful-replacing-fake" + registerTestConnector(t, server, connectorID, replacingStatefulLogoutConnector{ + store: server.storage, + sessionID: sessionID, + callbackErr: errors.New("forced validation failure after a newer logout started"), + }) + + require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{ + ID: sessionID, + Secret: testSessionSecret(sessionID), + UserID: "test-user", + ConnectorID: connectorID, + CreatedAt: time.Now(), + LastActivity: time.Now(), + LogoutState: &storage.LogoutState{ + ConnectorID: connectorID, + ConnectorState: []byte("_old_req_id"), + }, + })) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/logout/callback", nil) + req.AddCookie(testSessionCookie(sessionID)) + server.ServeHTTP(rr, req) + + require.Equal(t, http.StatusBadRequest, rr.Code) + got, err := server.storage.GetAuthSession(ctx, sessionID) + require.NoError(t, err) + require.NotNil(t, got.LogoutState) + require.Equal(t, []byte("_new_req_id"), got.LogoutState.ConnectorState) +} + +func TestLogoutCallbackSuccessDoesNotConsumeNewerLogoutState(t *testing.T) { + httpServer, server := newTestServerWithSessions(t, nil) + defer httpServer.Close() + + ctx := t.Context() + sessionID := "test-session-success-race" + connectorID := "stateful-success-replacing-fake" + registerTestConnector(t, server, connectorID, replacingStatefulLogoutConnector{ + store: server.storage, + sessionID: sessionID, + }) + + require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{ + ID: sessionID, + Secret: testSessionSecret(sessionID), + UserID: "test-user", + ConnectorID: connectorID, + CreatedAt: time.Now(), + LastActivity: time.Now(), + LogoutState: &storage.LogoutState{ + ConnectorID: connectorID, + ConnectorState: []byte("_old_req_id"), + }, + })) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/logout/callback", nil) + req.AddCookie(testSessionCookie(sessionID)) + server.ServeHTTP(rr, req) + + require.Equal(t, http.StatusBadRequest, rr.Code) + got, err := server.storage.GetAuthSession(ctx, sessionID) + require.NoError(t, err, "stale successful callback must not delete the session") + require.NotNil(t, got.LogoutState) + require.Equal(t, []byte("_new_req_id"), got.LogoutState.ConnectorState) +} + +func TestLogoutCallbackMissingStatefulConnectorFailsClosed(t *testing.T) { + httpServer, server := newTestServerWithSessions(t, nil) + defer httpServer.Close() + + ctx := t.Context() + sessionID := "test-session-missing-connector" + require.NoError(t, server.storage.CreateAuthSession(ctx, storage.AuthSession{ + ID: sessionID, + Secret: testSessionSecret(sessionID), + UserID: "test-user", + ConnectorID: "missing", + CreatedAt: time.Now(), + LastActivity: time.Now(), + LogoutState: &storage.LogoutState{ + ConnectorID: "missing", + ConnectorState: []byte("_req_id"), + }, + })) + + rr := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/logout/callback", nil) + req.AddCookie(testSessionCookie(sessionID)) + server.ServeHTTP(rr, req) + + require.Equal(t, http.StatusBadRequest, rr.Code) + got, err := server.storage.GetAuthSession(ctx, sessionID) + require.NoError(t, err, "session must survive when stateful callback cannot be validated") + require.Nil(t, got.LogoutState) +} + // TestLogoutCallbackWithExpiredSession tests that /logout/callback // returns an error when the session has expired or been deleted. func TestLogoutCallbackWithExpiredSession(t *testing.T) { diff --git a/server/session/session.go b/server/session/session.go index cf45703ae4..541c9296c7 100644 --- a/server/session/session.go +++ b/server/session/session.go @@ -8,6 +8,7 @@ import ( "log/slog" "net" "net/http" + "slices" "time" "github.com/dexidp/dex/server/internal" @@ -234,6 +235,7 @@ func (m *Manager) CreateOrUpdateAuthSession(ctx context.Context, r *http.Request old.IdleExpiry = now.Add(m.Config.ValidIfNotUsedFor) old.IPAddress = remoteIP(r) old.UserAgent = r.UserAgent() + old.ConnectorData = slices.Clone(authReq.ConnectorData) if old.ClientStates == nil { old.ClientStates = make(map[string]*storage.ClientAuthState) } @@ -253,10 +255,11 @@ func (m *Manager) CreateOrUpdateAuthSession(ctx context.Context, r *http.Request } newSession := storage.AuthSession{ - ID: storage.NewID(), - Secret: storage.NewID(), - UserID: userID, - ConnectorID: connectorID, + ID: storage.NewID(), + Secret: storage.NewID(), + UserID: userID, + ConnectorID: connectorID, + ConnectorData: slices.Clone(authReq.ConnectorData), ClientStates: map[string]*storage.ClientAuthState{ authReq.ClientID: clientState, }, diff --git a/server/session/session_test.go b/server/session/session_test.go index 1ce9ede86a..cf67f23e68 100644 --- a/server/session/session_test.go +++ b/server/session/session_test.go @@ -2,12 +2,17 @@ package session import ( "context" + "log/slog" "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/dexidp/dex/server/reqctx" + "github.com/dexidp/dex/storage" + "github.com/dexidp/dex/storage/memory" ) // The address stored on a session is an audit record, so it has to be the @@ -73,3 +78,46 @@ func TestRemoteIP(t *testing.T) { }) } } + +func TestCreateOrUpdateAuthSessionPersistsConnectorData(t *testing.T) { + store := memory.New(slog.New(slog.DiscardHandler)) + now := time.Now().UTC() + manager := Manager{ + Storage: store, + Config: &Config{ + CookieName: "dex_session", + CookieEncryptionKey: []byte("01234567890123456789012345678901"), + AbsoluteLifetime: 24 * time.Hour, + ValidIfNotUsedFor: time.Hour, + }, + Now: func() time.Time { return now }, + Logger: slog.New(slog.DiscardHandler), + } + authReq := storage.AuthRequest{ + ClientID: "client", + ConnectorID: "saml", + ConnectorData: []byte("first-session-index"), + Claims: storage.Claims{ + UserID: "user", + }, + } + + req := httptest.NewRequest("GET", "/", nil) + rec := httptest.NewRecorder() + require.NoError(t, manager.CreateOrUpdateAuthSession(t.Context(), req, rec, authReq, false)) + + sessions, err := store.ListAuthSessions(t.Context()) + require.NoError(t, err) + require.Len(t, sessions, 1) + assert.Equal(t, authReq.ConnectorData, sessions[0].ConnectorData) + + authReq.ConnectorData = []byte("second-session-index") + req = httptest.NewRequest("GET", "/", nil) + req.AddCookie(rec.Result().Cookies()[0]) + rec = httptest.NewRecorder() + require.NoError(t, manager.CreateOrUpdateAuthSession(t.Context(), req, rec, authReq, false)) + + session, err := store.GetAuthSession(t.Context(), sessions[0].ID) + require.NoError(t, err) + assert.Equal(t, authReq.ConnectorData, session.ConnectorData) +} diff --git a/storage/conformance/conformance.go b/storage/conformance/conformance.go index 734b7a2e19..de205f77ef 100644 --- a/storage/conformance/conformance.go +++ b/storage/conformance/conformance.go @@ -59,6 +59,7 @@ func RunTests(t *testing.T, newStorage func(t *testing.T) storage.Storage) { {"DeviceTokenCRUD", testDeviceTokenCRUD}, {"UserIdentityCRUD", testUserIdentityCRUD}, {"AuthSessionCRUD", testAuthSessionCRUD}, + {"AuthSessionLogoutState", testAuthSessionLogoutState}, }) } @@ -1402,10 +1403,11 @@ func testAuthSessionCRUD(t *testing.T, s storage.Storage) { now := time.Now().UTC().Round(time.Millisecond) session := storage.AuthSession{ - ID: storage.NewID(), - Secret: storage.NewID(), - UserID: "user1", - ConnectorID: "conn1", + ID: storage.NewID(), + Secret: storage.NewID(), + UserID: "user1", + ConnectorID: "conn1", + ConnectorData: []byte(`{"nameID":"alice","sessionIndex":"session-1"}`), ClientStates: map[string]*storage.ClientAuthState{ "client1": { AuthenticatedAt: now, @@ -1466,6 +1468,7 @@ func testAuthSessionCRUD(t *testing.T, s storage.Storage) { } old.LastActivity = newNow old.IdleExpiry = newNow.Add(time.Hour) + old.ConnectorData = []byte(`{"nameID":"alice","sessionIndex":"session-2"}`) return old, nil }); err != nil { t.Fatalf("update auth session: %v", err) @@ -1482,6 +1485,9 @@ func testAuthSessionCRUD(t *testing.T, s storage.Storage) { if got.ClientStates["client2"] == nil { t.Fatal("expected client2 state to exist") } + if want := `{"nameID":"alice","sessionIndex":"session-2"}`; string(got.ConnectorData) != want { + t.Errorf("expected connector data %q, got %q", want, got.ConnectorData) + } // The idle timeout has to move with the activity that reset it, or a session // never outlives its first one. if !got.IdleExpiry.UTC().Round(time.Millisecond).Equal(newNow.Add(time.Hour)) { @@ -1506,3 +1512,89 @@ func testAuthSessionCRUD(t *testing.T, s storage.Storage) { _, err = s.GetAuthSession(ctx, session.ID) mustBeErrNotFound(t, "auth session", err) } + +// testAuthSessionLogoutState verifies that storage backends round-trip the +// LogoutState (including the opaque connector-specific ConnectorState used by +// SAML for InResponseTo correlation) and can clear it back to nil. +// +// Backends serialize LogoutState differently (etcd/kubernetes embed a struct, +// while SQL and ent store a JSON blob); without this +// test, missing a field in any of those mirrors would silently break SAML SLO +// at runtime with "No logout in progress." after the upstream redirect. +func testAuthSessionLogoutState(t *testing.T, s storage.Storage) { + ctx := t.Context() + + now := time.Now().UTC().Round(time.Millisecond) + + session := storage.AuthSession{ + ID: storage.NewID(), + Secret: storage.NewID(), + UserID: "user-logout", + ConnectorID: "conn-logout", + ClientStates: map[string]*storage.ClientAuthState{}, + CreatedAt: now, + LastActivity: now, + AbsoluteExpiry: now.Add(24 * time.Hour), + IdleExpiry: now.Add(1 * time.Hour), + } + + if err := s.CreateAuthSession(ctx, session); err != nil { + t.Fatalf("create auth session: %v", err) + } + t.Cleanup(func() { + _ = s.DeleteAuthSession(ctx, session.ID) + }) + + // Initially LogoutState must be nil. + got, err := s.GetAuthSession(ctx, session.ID) + if err != nil { + t.Fatalf("get auth session: %v", err) + } + if got.LogoutState != nil { + t.Fatalf("expected nil LogoutState on fresh session, got %+v", got.LogoutState) + } + + // Set LogoutState with a non-trivial ConnectorState (mimics SAML's + // outgoing LogoutRequest ID used for InResponseTo correlation). + want := &storage.LogoutState{ + PostLogoutRedirectURI: "https://app.example.com/done", + State: "client-state-xyz", + ClientID: "client1", + ConnectorID: session.ConnectorID, + ConnectorState: []byte("_saml_request_id_12345"), + } + if err := s.UpdateAuthSession(ctx, session.ID, func(old storage.AuthSession) (storage.AuthSession, error) { + old.LogoutState = want + return old, nil + }); err != nil { + t.Fatalf("update auth session with LogoutState: %v", err) + } + + got, err = s.GetAuthSession(ctx, session.ID) + if err != nil { + t.Fatalf("get auth session after LogoutState write: %v", err) + } + if got.LogoutState == nil { + t.Fatalf("expected LogoutState to round-trip, got nil") + } + if diff := pretty.Compare(want, got.LogoutState); diff != "" { + t.Errorf("LogoutState did not round-trip: %s", diff) + } + + // Clear LogoutState back to nil; the storage must persist nil, not an + // empty struct (server uses nil to mean "no logout in progress"). + if err := s.UpdateAuthSession(ctx, session.ID, func(old storage.AuthSession) (storage.AuthSession, error) { + old.LogoutState = nil + return old, nil + }); err != nil { + t.Fatalf("update auth session clearing LogoutState: %v", err) + } + + got, err = s.GetAuthSession(ctx, session.ID) + if err != nil { + t.Fatalf("get auth session after LogoutState clear: %v", err) + } + if got.LogoutState != nil { + t.Errorf("expected nil LogoutState after clear, got %+v", got.LogoutState) + } +} diff --git a/storage/ent/client/authsession.go b/storage/ent/client/authsession.go index 8ffa13b746..b3ef50eb9c 100644 --- a/storage/ent/client/authsession.go +++ b/storage/ent/client/authsession.go @@ -29,6 +29,7 @@ func (d *Database) CreateAuthSession(ctx context.Context, session storage.AuthSe SetConnectorID(session.ConnectorID). SetSecret(session.Secret). SetClientStates(encodedStates). + SetConnectorData(session.ConnectorData). SetCreatedAt(session.CreatedAt). SetLastActivity(session.LastActivity). SetIPAddress(session.IPAddress). @@ -108,6 +109,7 @@ func (d *Database) UpdateAuthSession(ctx context.Context, id string, updater fun _, err = tx.AuthSession.UpdateOneID(id). SetClientStates(encodedStates). + SetConnectorData(newSession.ConnectorData). SetLastActivity(newSession.LastActivity). SetIPAddress(newSession.IPAddress). SetUserAgent(newSession.UserAgent). diff --git a/storage/ent/client/types.go b/storage/ent/client/types.go index ce1738a133..36823acab4 100644 --- a/storage/ent/client/types.go +++ b/storage/ent/client/types.go @@ -241,6 +241,7 @@ func toStorageAuthSession(s *db.AuthSession) storage.AuthSession { Secret: s.Secret, UserID: s.UserID, ConnectorID: s.ConnectorID, + ConnectorData: s.ConnectorData, CreatedAt: s.CreatedAt, LastActivity: s.LastActivity, IPAddress: s.IPAddress, diff --git a/storage/ent/db/authsession.go b/storage/ent/db/authsession.go index d332c0f8c3..3d0a68418e 100644 --- a/storage/ent/db/authsession.go +++ b/storage/ent/db/authsession.go @@ -25,6 +25,8 @@ type AuthSession struct { Secret string `json:"secret,omitempty"` // ClientStates holds the value of the "client_states" field. ClientStates []byte `json:"client_states,omitempty"` + // ConnectorData holds the value of the "connector_data" field. + ConnectorData []byte `json:"connector_data,omitempty"` // CreatedAt holds the value of the "created_at" field. CreatedAt time.Time `json:"created_at,omitempty"` // LastActivity holds the value of the "last_activity" field. @@ -47,7 +49,7 @@ func (*AuthSession) scanValues(columns []string) ([]any, error) { values := make([]any, len(columns)) for i := range columns { switch columns[i] { - case authsession.FieldClientStates, authsession.FieldLogoutState: + case authsession.FieldClientStates, authsession.FieldConnectorData, authsession.FieldLogoutState: values[i] = new([]byte) case authsession.FieldID, authsession.FieldUserID, authsession.FieldConnectorID, authsession.FieldSecret, authsession.FieldIPAddress, authsession.FieldUserAgent: values[i] = new(sql.NullString) @@ -98,6 +100,12 @@ func (_m *AuthSession) assignValues(columns []string, values []any) error { } else if value != nil { _m.ClientStates = *value } + case authsession.FieldConnectorData: + if value, ok := values[i].(*[]byte); !ok { + return fmt.Errorf("unexpected type %T for field connector_data", values[i]) + } else if value != nil { + _m.ConnectorData = *value + } case authsession.FieldCreatedAt: if value, ok := values[i].(*sql.NullTime); !ok { return fmt.Errorf("unexpected type %T for field created_at", values[i]) @@ -188,6 +196,9 @@ func (_m *AuthSession) String() string { builder.WriteString("client_states=") builder.WriteString(fmt.Sprintf("%v", _m.ClientStates)) builder.WriteString(", ") + builder.WriteString("connector_data=") + builder.WriteString(fmt.Sprintf("%v", _m.ConnectorData)) + builder.WriteString(", ") builder.WriteString("created_at=") builder.WriteString(_m.CreatedAt.Format(time.ANSIC)) builder.WriteString(", ") diff --git a/storage/ent/db/authsession/authsession.go b/storage/ent/db/authsession/authsession.go index c8d6e9479f..8ccb73898e 100644 --- a/storage/ent/db/authsession/authsession.go +++ b/storage/ent/db/authsession/authsession.go @@ -19,6 +19,8 @@ const ( FieldSecret = "secret" // FieldClientStates holds the string denoting the client_states field in the database. FieldClientStates = "client_states" + // FieldConnectorData holds the string denoting the connector_data field in the database. + FieldConnectorData = "connector_data" // FieldCreatedAt holds the string denoting the created_at field in the database. FieldCreatedAt = "created_at" // FieldLastActivity holds the string denoting the last_activity field in the database. @@ -44,6 +46,7 @@ var Columns = []string{ FieldConnectorID, FieldSecret, FieldClientStates, + FieldConnectorData, FieldCreatedAt, FieldLastActivity, FieldIPAddress, diff --git a/storage/ent/db/authsession/where.go b/storage/ent/db/authsession/where.go index 87698ac33e..b26a776fb8 100644 --- a/storage/ent/db/authsession/where.go +++ b/storage/ent/db/authsession/where.go @@ -84,6 +84,11 @@ func ClientStates(v []byte) predicate.AuthSession { return predicate.AuthSession(sql.FieldEQ(FieldClientStates, v)) } +// ConnectorData applies equality check predicate on the "connector_data" field. It's identical to ConnectorDataEQ. +func ConnectorData(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldEQ(FieldConnectorData, v)) +} + // CreatedAt applies equality check predicate on the "created_at" field. It's identical to CreatedAtEQ. func CreatedAt(v time.Time) predicate.AuthSession { return predicate.AuthSession(sql.FieldEQ(FieldCreatedAt, v)) @@ -354,6 +359,56 @@ func ClientStatesLTE(v []byte) predicate.AuthSession { return predicate.AuthSession(sql.FieldLTE(FieldClientStates, v)) } +// ConnectorDataEQ applies the EQ predicate on the "connector_data" field. +func ConnectorDataEQ(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldEQ(FieldConnectorData, v)) +} + +// ConnectorDataNEQ applies the NEQ predicate on the "connector_data" field. +func ConnectorDataNEQ(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldNEQ(FieldConnectorData, v)) +} + +// ConnectorDataIn applies the In predicate on the "connector_data" field. +func ConnectorDataIn(vs ...[]byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldIn(FieldConnectorData, vs...)) +} + +// ConnectorDataNotIn applies the NotIn predicate on the "connector_data" field. +func ConnectorDataNotIn(vs ...[]byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldNotIn(FieldConnectorData, vs...)) +} + +// ConnectorDataGT applies the GT predicate on the "connector_data" field. +func ConnectorDataGT(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldGT(FieldConnectorData, v)) +} + +// ConnectorDataGTE applies the GTE predicate on the "connector_data" field. +func ConnectorDataGTE(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldGTE(FieldConnectorData, v)) +} + +// ConnectorDataLT applies the LT predicate on the "connector_data" field. +func ConnectorDataLT(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldLT(FieldConnectorData, v)) +} + +// ConnectorDataLTE applies the LTE predicate on the "connector_data" field. +func ConnectorDataLTE(v []byte) predicate.AuthSession { + return predicate.AuthSession(sql.FieldLTE(FieldConnectorData, v)) +} + +// ConnectorDataIsNil applies the IsNil predicate on the "connector_data" field. +func ConnectorDataIsNil() predicate.AuthSession { + return predicate.AuthSession(sql.FieldIsNull(FieldConnectorData)) +} + +// ConnectorDataNotNil applies the NotNil predicate on the "connector_data" field. +func ConnectorDataNotNil() predicate.AuthSession { + return predicate.AuthSession(sql.FieldNotNull(FieldConnectorData)) +} + // CreatedAtEQ applies the EQ predicate on the "created_at" field. func CreatedAtEQ(v time.Time) predicate.AuthSession { return predicate.AuthSession(sql.FieldEQ(FieldCreatedAt, v)) diff --git a/storage/ent/db/authsession_create.go b/storage/ent/db/authsession_create.go index 2e18f6a438..9ff61edf9d 100644 --- a/storage/ent/db/authsession_create.go +++ b/storage/ent/db/authsession_create.go @@ -44,6 +44,12 @@ func (_c *AuthSessionCreate) SetClientStates(v []byte) *AuthSessionCreate { return _c } +// SetConnectorData sets the "connector_data" field. +func (_c *AuthSessionCreate) SetConnectorData(v []byte) *AuthSessionCreate { + _c.mutation.SetConnectorData(v) + return _c +} + // SetCreatedAt sets the "created_at" field. func (_c *AuthSessionCreate) SetCreatedAt(v time.Time) *AuthSessionCreate { _c.mutation.SetCreatedAt(v) @@ -256,6 +262,10 @@ func (_c *AuthSessionCreate) createSpec() (*AuthSession, *sqlgraph.CreateSpec) { _spec.SetField(authsession.FieldClientStates, field.TypeBytes, value) _node.ClientStates = value } + if value, ok := _c.mutation.ConnectorData(); ok { + _spec.SetField(authsession.FieldConnectorData, field.TypeBytes, value) + _node.ConnectorData = value + } if value, ok := _c.mutation.CreatedAt(); ok { _spec.SetField(authsession.FieldCreatedAt, field.TypeTime, value) _node.CreatedAt = value diff --git a/storage/ent/db/authsession_update.go b/storage/ent/db/authsession_update.go index 389a92c977..a22c0cee5e 100644 --- a/storage/ent/db/authsession_update.go +++ b/storage/ent/db/authsession_update.go @@ -76,6 +76,18 @@ func (_u *AuthSessionUpdate) SetClientStates(v []byte) *AuthSessionUpdate { return _u } +// SetConnectorData sets the "connector_data" field. +func (_u *AuthSessionUpdate) SetConnectorData(v []byte) *AuthSessionUpdate { + _u.mutation.SetConnectorData(v) + return _u +} + +// ClearConnectorData clears the value of the "connector_data" field. +func (_u *AuthSessionUpdate) ClearConnectorData() *AuthSessionUpdate { + _u.mutation.ClearConnectorData() + return _u +} + // SetCreatedAt sets the "created_at" field. func (_u *AuthSessionUpdate) SetCreatedAt(v time.Time) *AuthSessionUpdate { _u.mutation.SetCreatedAt(v) @@ -248,6 +260,12 @@ func (_u *AuthSessionUpdate) sqlSave(ctx context.Context) (_node int, err error) if value, ok := _u.mutation.ClientStates(); ok { _spec.SetField(authsession.FieldClientStates, field.TypeBytes, value) } + if value, ok := _u.mutation.ConnectorData(); ok { + _spec.SetField(authsession.FieldConnectorData, field.TypeBytes, value) + } + if _u.mutation.ConnectorDataCleared() { + _spec.ClearField(authsession.FieldConnectorData, field.TypeBytes) + } if value, ok := _u.mutation.CreatedAt(); ok { _spec.SetField(authsession.FieldCreatedAt, field.TypeTime, value) } @@ -340,6 +358,18 @@ func (_u *AuthSessionUpdateOne) SetClientStates(v []byte) *AuthSessionUpdateOne return _u } +// SetConnectorData sets the "connector_data" field. +func (_u *AuthSessionUpdateOne) SetConnectorData(v []byte) *AuthSessionUpdateOne { + _u.mutation.SetConnectorData(v) + return _u +} + +// ClearConnectorData clears the value of the "connector_data" field. +func (_u *AuthSessionUpdateOne) ClearConnectorData() *AuthSessionUpdateOne { + _u.mutation.ClearConnectorData() + return _u +} + // SetCreatedAt sets the "created_at" field. func (_u *AuthSessionUpdateOne) SetCreatedAt(v time.Time) *AuthSessionUpdateOne { _u.mutation.SetCreatedAt(v) @@ -542,6 +572,12 @@ func (_u *AuthSessionUpdateOne) sqlSave(ctx context.Context) (_node *AuthSession if value, ok := _u.mutation.ClientStates(); ok { _spec.SetField(authsession.FieldClientStates, field.TypeBytes, value) } + if value, ok := _u.mutation.ConnectorData(); ok { + _spec.SetField(authsession.FieldConnectorData, field.TypeBytes, value) + } + if _u.mutation.ConnectorDataCleared() { + _spec.ClearField(authsession.FieldConnectorData, field.TypeBytes) + } if value, ok := _u.mutation.CreatedAt(); ok { _spec.SetField(authsession.FieldCreatedAt, field.TypeTime, value) } diff --git a/storage/ent/db/migrate/schema.go b/storage/ent/db/migrate/schema.go index 3a76ba2dbb..493f70e823 100644 --- a/storage/ent/db/migrate/schema.go +++ b/storage/ent/db/migrate/schema.go @@ -77,6 +77,7 @@ var ( {Name: "connector_id", Type: field.TypeString, Size: 2147483647, SchemaType: map[string]string{"mysql": "varchar(384)", "postgres": "text", "sqlite3": "text"}}, {Name: "secret", Type: field.TypeString, Size: 2147483647, SchemaType: map[string]string{"mysql": "varchar(384)", "postgres": "text", "sqlite3": "text"}}, {Name: "client_states", Type: field.TypeBytes}, + {Name: "connector_data", Type: field.TypeBytes, Nullable: true}, {Name: "created_at", Type: field.TypeTime, SchemaType: map[string]string{"mysql": "datetime(3)", "postgres": "timestamptz", "sqlite3": "timestamp"}}, {Name: "last_activity", Type: field.TypeTime, SchemaType: map[string]string{"mysql": "datetime(3)", "postgres": "timestamptz", "sqlite3": "timestamp"}}, {Name: "ip_address", Type: field.TypeString, Size: 2147483647, Default: "", SchemaType: map[string]string{"mysql": "varchar(384)", "postgres": "text", "sqlite3": "text"}}, diff --git a/storage/ent/db/mutation.go b/storage/ent/db/mutation.go index e4edf2fb11..efc1a89def 100644 --- a/storage/ent/db/mutation.go +++ b/storage/ent/db/mutation.go @@ -3221,6 +3221,7 @@ type AuthSessionMutation struct { connector_id *string secret *string client_states *[]byte + connector_data *[]byte created_at *time.Time last_activity *time.Time ip_address *string @@ -3482,6 +3483,55 @@ func (m *AuthSessionMutation) ResetClientStates() { m.client_states = nil } +// SetConnectorData sets the "connector_data" field. +func (m *AuthSessionMutation) SetConnectorData(b []byte) { + m.connector_data = &b +} + +// ConnectorData returns the value of the "connector_data" field in the mutation. +func (m *AuthSessionMutation) ConnectorData() (r []byte, exists bool) { + v := m.connector_data + if v == nil { + return + } + return *v, true +} + +// OldConnectorData returns the old "connector_data" field's value of the AuthSession entity. +// If the AuthSession object wasn't provided to the builder, the object is fetched from the database. +// An error is returned if the mutation operation is not UpdateOne, or the database query fails. +func (m *AuthSessionMutation) OldConnectorData(ctx context.Context) (v []byte, err error) { + if !m.op.Is(OpUpdateOne) { + return v, errors.New("OldConnectorData is only allowed on UpdateOne operations") + } + if m.id == nil || m.oldValue == nil { + return v, errors.New("OldConnectorData requires an ID field in the mutation") + } + oldValue, err := m.oldValue(ctx) + if err != nil { + return v, fmt.Errorf("querying old value for OldConnectorData: %w", err) + } + return oldValue.ConnectorData, nil +} + +// ClearConnectorData clears the value of the "connector_data" field. +func (m *AuthSessionMutation) ClearConnectorData() { + m.connector_data = nil + m.clearedFields[authsession.FieldConnectorData] = struct{}{} +} + +// ConnectorDataCleared returns if the "connector_data" field was cleared in this mutation. +func (m *AuthSessionMutation) ConnectorDataCleared() bool { + _, ok := m.clearedFields[authsession.FieldConnectorData] + return ok +} + +// ResetConnectorData resets all changes to the "connector_data" field. +func (m *AuthSessionMutation) ResetConnectorData() { + m.connector_data = nil + delete(m.clearedFields, authsession.FieldConnectorData) +} + // SetCreatedAt sets the "created_at" field. func (m *AuthSessionMutation) SetCreatedAt(t time.Time) { m.created_at = &t @@ -3781,7 +3831,7 @@ func (m *AuthSessionMutation) Type() string { // order to get all numeric fields that were incremented/decremented, call // AddedFields(). func (m *AuthSessionMutation) Fields() []string { - fields := make([]string, 0, 11) + fields := make([]string, 0, 12) if m.user_id != nil { fields = append(fields, authsession.FieldUserID) } @@ -3794,6 +3844,9 @@ func (m *AuthSessionMutation) Fields() []string { if m.client_states != nil { fields = append(fields, authsession.FieldClientStates) } + if m.connector_data != nil { + fields = append(fields, authsession.FieldConnectorData) + } if m.created_at != nil { fields = append(fields, authsession.FieldCreatedAt) } @@ -3831,6 +3884,8 @@ func (m *AuthSessionMutation) Field(name string) (ent.Value, bool) { return m.Secret() case authsession.FieldClientStates: return m.ClientStates() + case authsession.FieldConnectorData: + return m.ConnectorData() case authsession.FieldCreatedAt: return m.CreatedAt() case authsession.FieldLastActivity: @@ -3862,6 +3917,8 @@ func (m *AuthSessionMutation) OldField(ctx context.Context, name string) (ent.Va return m.OldSecret(ctx) case authsession.FieldClientStates: return m.OldClientStates(ctx) + case authsession.FieldConnectorData: + return m.OldConnectorData(ctx) case authsession.FieldCreatedAt: return m.OldCreatedAt(ctx) case authsession.FieldLastActivity: @@ -3913,6 +3970,13 @@ func (m *AuthSessionMutation) SetField(name string, value ent.Value) error { } m.SetClientStates(v) return nil + case authsession.FieldConnectorData: + v, ok := value.([]byte) + if !ok { + return fmt.Errorf("unexpected type %T for field %s", value, name) + } + m.SetConnectorData(v) + return nil case authsession.FieldCreatedAt: v, ok := value.(time.Time) if !ok { @@ -3992,6 +4056,9 @@ func (m *AuthSessionMutation) AddField(name string, value ent.Value) error { // mutation. func (m *AuthSessionMutation) ClearedFields() []string { var fields []string + if m.FieldCleared(authsession.FieldConnectorData) { + fields = append(fields, authsession.FieldConnectorData) + } if m.FieldCleared(authsession.FieldLogoutState) { fields = append(fields, authsession.FieldLogoutState) } @@ -4009,6 +4076,9 @@ func (m *AuthSessionMutation) FieldCleared(name string) bool { // error if the field is not defined in the schema. func (m *AuthSessionMutation) ClearField(name string) error { switch name { + case authsession.FieldConnectorData: + m.ClearConnectorData() + return nil case authsession.FieldLogoutState: m.ClearLogoutState() return nil @@ -4032,6 +4102,9 @@ func (m *AuthSessionMutation) ResetField(name string) error { case authsession.FieldClientStates: m.ResetClientStates() return nil + case authsession.FieldConnectorData: + m.ResetConnectorData() + return nil case authsession.FieldCreatedAt: m.ResetCreatedAt() return nil diff --git a/storage/ent/db/runtime.go b/storage/ent/db/runtime.go index c53ae1a5ef..ca659c970b 100644 --- a/storage/ent/db/runtime.go +++ b/storage/ent/db/runtime.go @@ -119,11 +119,11 @@ func init() { // authsession.SecretValidator is a validator for the "secret" field. It is called by the builders before save. authsession.SecretValidator = authsessionDescSecret.Validators[0].(func(string) error) // authsessionDescIPAddress is the schema descriptor for ip_address field. - authsessionDescIPAddress := authsessionFields[7].Descriptor() + authsessionDescIPAddress := authsessionFields[8].Descriptor() // authsession.DefaultIPAddress holds the default value on creation for the ip_address field. authsession.DefaultIPAddress = authsessionDescIPAddress.Default.(string) // authsessionDescUserAgent is the schema descriptor for user_agent field. - authsessionDescUserAgent := authsessionFields[8].Descriptor() + authsessionDescUserAgent := authsessionFields[9].Descriptor() // authsession.DefaultUserAgent holds the default value on creation for the user_agent field. authsession.DefaultUserAgent = authsessionDescUserAgent.Default.(string) // authsessionDescID is the schema descriptor for id field. diff --git a/storage/ent/schema/authsession.go b/storage/ent/schema/authsession.go index 07f2ea645b..1678835a69 100644 --- a/storage/ent/schema/authsession.go +++ b/storage/ent/schema/authsession.go @@ -27,6 +27,8 @@ func (AuthSession) Fields() []ent.Field { SchemaType(textSchema). NotEmpty(), field.Bytes("client_states"), + field.Bytes("connector_data"). + Optional(), field.Time("created_at"). SchemaType(timeSchema), field.Time("last_activity"). diff --git a/storage/etcd/types.go b/storage/etcd/types.go index 009ec2885a..9d1a249d0e 100644 --- a/storage/etcd/types.go +++ b/storage/etcd/types.go @@ -334,6 +334,7 @@ type AuthSession struct { UserID string `json:"user_id,omitempty"` ConnectorID string `json:"connector_id,omitempty"` ClientStates map[string]*storage.ClientAuthState `json:"client_states,omitempty"` + ConnectorData []byte `json:"connector_data,omitempty"` CreatedAt time.Time `json:"created_at"` LastActivity time.Time `json:"last_activity"` IPAddress string `json:"ip_address,omitempty"` @@ -350,6 +351,7 @@ func fromStorageAuthSession(s storage.AuthSession) AuthSession { UserID: s.UserID, ConnectorID: s.ConnectorID, ClientStates: s.ClientStates, + ConnectorData: s.ConnectorData, CreatedAt: s.CreatedAt, LastActivity: s.LastActivity, IPAddress: s.IPAddress, @@ -367,6 +369,7 @@ func toStorageAuthSession(s AuthSession) storage.AuthSession { UserID: s.UserID, ConnectorID: s.ConnectorID, ClientStates: s.ClientStates, + ConnectorData: s.ConnectorData, CreatedAt: s.CreatedAt, LastActivity: s.LastActivity, IPAddress: s.IPAddress, diff --git a/storage/kubernetes/types.go b/storage/kubernetes/types.go index 151c8ad112..a3fbe9a673 100644 --- a/storage/kubernetes/types.go +++ b/storage/kubernetes/types.go @@ -1028,6 +1028,7 @@ type AuthSession struct { Nonce string `json:"nonce,omitempty"` Secret string `json:"secret,omitempty"` ClientStates map[string]*storage.ClientAuthState `json:"clientStates,omitempty"` + ConnectorData []byte `json:"connectorData,omitempty"` CreatedAt time.Time `json:"createdAt,omitempty"` LastActivity time.Time `json:"lastActivity,omitempty"` IPAddress string `json:"ipAddress,omitempty"` @@ -1058,6 +1059,7 @@ func (cli *client) fromStorageAuthSession(s storage.AuthSession) AuthSession { UserID: s.UserID, ConnectorID: s.ConnectorID, ClientStates: s.ClientStates, + ConnectorData: s.ConnectorData, CreatedAt: s.CreatedAt, LastActivity: s.LastActivity, IPAddress: s.IPAddress, @@ -1075,6 +1077,7 @@ func toStorageAuthSession(s AuthSession) storage.AuthSession { UserID: s.UserID, ConnectorID: s.ConnectorID, ClientStates: s.ClientStates, + ConnectorData: s.ConnectorData, CreatedAt: s.CreatedAt, LastActivity: s.LastActivity, IPAddress: s.IPAddress, diff --git a/storage/sql/crud.go b/storage/sql/crud.go index 3a82260ff9..e45fdb5b21 100644 --- a/storage/sql/crud.go +++ b/storage/sql/crud.go @@ -996,16 +996,16 @@ func (c *conn) CreateAuthSession(ctx context.Context, s storage.AuthSession) err _, err := c.Exec(` insert into auth_session ( id, secret, user_id, connector_id, - client_states, + client_states, connector_data, created_at, last_activity, ip_address, user_agent, absolute_expiry, idle_expiry, logout_state ) - values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12); + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13); `, s.ID, s.Secret, s.UserID, s.ConnectorID, - encoder(s.ClientStates), + encoder(s.ClientStates), s.ConnectorData, s.CreatedAt, s.LastActivity, s.IPAddress, s.UserAgent, s.AbsoluteExpiry, s.IdleExpiry, @@ -1035,14 +1035,16 @@ func (c *conn) UpdateAuthSession(ctx context.Context, id string, updater func(s update auth_session set client_states = $1, - last_activity = $2, - ip_address = $3, - user_agent = $4, - idle_expiry = $5, - logout_state = $6 - where id = $7; + connector_data = $2, + last_activity = $3, + ip_address = $4, + user_agent = $5, + idle_expiry = $6, + logout_state = $7 + where id = $8; `, encoder(newSession.ClientStates), + newSession.ConnectorData, newSession.LastActivity, newSession.IPAddress, newSession.UserAgent, newSession.IdleExpiry, @@ -1062,7 +1064,7 @@ func (c *conn) GetAuthSession(ctx context.Context, id string) (storage.AuthSessi const authSessionColumns = ` id, secret, user_id, connector_id, - client_states, + client_states, connector_data, created_at, last_activity, ip_address, user_agent, absolute_expiry, idle_expiry, @@ -1081,7 +1083,7 @@ func scanAuthSession(s scanner) (session storage.AuthSession, err error) { var logoutState []byte err = s.Scan( &session.ID, &session.Secret, &session.UserID, &session.ConnectorID, - decoder(&session.ClientStates), + decoder(&session.ClientStates), &session.ConnectorData, &session.CreatedAt, &session.LastActivity, &session.IPAddress, &session.UserAgent, &session.AbsoluteExpiry, &session.IdleExpiry, diff --git a/storage/sql/migrate.go b/storage/sql/migrate.go index 1b7619b77e..350e78a504 100644 --- a/storage/sql/migrate.go +++ b/storage/sql/migrate.go @@ -507,4 +507,9 @@ var migrations = []migration{ `alter table auth_code add column session_id text not null default '';`, }, }, + { + stmts: []string{ + `alter table auth_session add column connector_data bytea;`, + }, + }, } diff --git a/storage/storage.go b/storage/storage.go index 84976c5df6..7d5600e7e1 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -503,6 +503,13 @@ type LogoutState struct { State string // RP's opaque state parameter ClientID string ConnectorID string + + // ConnectorState is opaque bytes returned by LogoutCallbackConnector.LogoutURL + // and handed back to HandleLogoutCallback. Used by the SAML connector to + // remember the outgoing LogoutRequest ID so it can validate InResponseTo + // against a server-side, one-shot value (defense against replay of captured + // LogoutResponses). Nil for connectors that don't need correlation state. + ConnectorState []byte } // AuthSession is one signed-in browser: a user, the connector they authenticated @@ -527,6 +534,12 @@ type AuthSession struct { UserID string ConnectorID string ClientStates map[string]*ClientAuthState // clientID -> auth state + + // ConnectorData is opaque state returned by the connector that established + // this browser session. Stateful logout connectors use it to build an + // upstream logout request for this exact session. + ConnectorData []byte + CreatedAt time.Time LastActivity time.Time