diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go new file mode 100644 index 0000000000..1d5931d2b6 --- /dev/null +++ b/connector/ldap/kerberos.go @@ -0,0 +1,398 @@ +// Package ldap implements strategies for authenticating using the LDAP protocol. +// This file contains Kerberos/SPNEGO authentication support for LDAP connector. +package ldap + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "os" + "strings" + "time" + + "github.com/go-ldap/ldap/v3" + "github.com/jcmturner/goidentity/v6" + "github.com/jcmturner/gokrb5/v8/credentials" + "github.com/jcmturner/gokrb5/v8/keytab" + "github.com/jcmturner/gokrb5/v8/service" + "github.com/jcmturner/gokrb5/v8/spnego" + + "github.com/dexidp/dex/connector" +) + +// bufferedResponse is a minimal http.ResponseWriter that buffers status, +// headers, and body in memory so the SPNEGO middleware's response can be +// inspected and conditionally forwarded. spnego.SPNEGOKRB5Authenticate is a +// "terminal" middleware (it commits failure responses directly to the +// ResponseWriter), but Dex needs to layer policy on top — FallbackToPassword +// may want to discard a 401 so the password form can render, and +// ExpectedRealm checks happen after a successful auth. Buffering decouples +// the middleware's output from the eventual client response. +type bufferedResponse struct { + header http.Header + body bytes.Buffer + code int + wroteHeader bool +} + +func newBufferedResponse() *bufferedResponse { + return &bufferedResponse{ + header: make(http.Header), + code: http.StatusOK, + } +} + +func (r *bufferedResponse) Header() http.Header { + return r.header +} + +// WriteHeader mirrors net/http: the first call wins, subsequent calls are +// silently ignored (stdlib emits a "superfluous WriteHeader" warning and +// keeps the original status; we just drop the override). +func (r *bufferedResponse) WriteHeader(code int) { + if r.wroteHeader { + return + } + r.code = code + r.wroteHeader = true +} + +// Write mirrors net/http: a Write before any WriteHeader implicitly commits +// status 200, locking the status against later overrides. +func (r *bufferedResponse) Write(b []byte) (int, error) { + if !r.wroteHeader { + r.WriteHeader(http.StatusOK) + } + return r.body.Write(b) +} + +// krbState holds Kerberos/SPNEGO configuration bound to a ldapConnector. +// +// The authenticate field is the SPNEGO HTTP middleware factory: it wraps an +// inner http.Handler so that inner runs only after successful SPNEGO auth, +// with an authenticated *credentials.Credentials attached to the request +// context under goidentity.CTXKey. In production it delegates to +// spnego.SPNEGOKRB5Authenticate. Unit tests replace it with a stub that +// injects a fake credential without any real Kerberos exchange. +type krbState struct { + authenticate func(inner http.Handler) http.Handler +} + +// loadKerberosState validates the keytab and returns a krbState wired to +// spnego.SPNEGOKRB5Authenticate with the requested service settings. +func loadKerberosState(cfg kerberosConfig) (*krbState, error) { + fi, err := os.Stat(cfg.KeytabPath) + if err != nil { + return nil, fmt.Errorf("keytab file not found: %w", err) + } + if fi.IsDir() { + return nil, fmt.Errorf("keytab path is a directory: %s", cfg.KeytabPath) + } + kt, err := keytab.Load(cfg.KeytabPath) + if err != nil { + return nil, fmt.Errorf("failed to load keytab: %w", err) + } + + settings := []func(*service.Settings){service.DecodePAC(false)} + if cfg.SPN != "" { + settings = append(settings, service.SName(cfg.SPN)) + } + if cfg.KeytabPrincipal != "" { + settings = append(settings, service.KeytabPrincipal(cfg.KeytabPrincipal)) + } + if cfg.MaxClockSkew > 0 { + settings = append(settings, service.MaxClockSkew(time.Duration(cfg.MaxClockSkew)*time.Second)) + } + + return &krbState{ + authenticate: func(inner http.Handler) http.Handler { + return spnego.SPNEGOKRB5Authenticate(inner, kt, settings...) + }, + }, nil +} + +// mapPrincipal builds the LDAP-side username from the Kerberos credentials +// according to the configured mapping. Inputs come from gokrb5: +// username is credentials.UserName() (bare, no realm); realm is Domain(). +// +// - "userprincipalname": "username@realm". +// - "localpart"/"samaccountname" (default): bare username. +// +// Case of the output is preserved; callers rely on the LDAP server's +// attribute matching rules (typically case-insensitive) for comparisons. +func mapPrincipal(username, realm, mapping string) string { + // Defensive: if username somehow carries an '@', trim to before it; we + // always derive the realm from the separate Domain() field. + if i := strings.IndexByte(username, '@'); i >= 0 { + username = username[:i] + } + if strings.EqualFold(mapping, "userprincipalname") { + if realm == "" { + return username + } + return username + "@" + realm + } + return username +} + +// TrySPNEGO attempts SPNEGO authentication against the LDAP connector's keytab. +// +// Behavior: +// - Kerberos disabled: returns (nil, false, nil) so the caller renders the +// password form. +// - FallbackToPassword=true with no Authorization header: a cookie probe +// gives the browser exactly one round to negotiate. The first such +// request sets a short-lived "tried" cookie and forwards the middleware's +// 401 Negotiate challenge so a Kerberos-aware client can respond. If the +// follow-up request still has no Authorization header (cookie present), +// we treat the client as unable to SPNEGO and render the password form. +// - FallbackToPassword=true with an Authorization header that the +// middleware rejects: render the password form (the client tried and +// failed; do not loop 401s). +// - FallbackToPassword=false: forward the middleware's response verbatim. +// - On success, the authenticated principal is resolved in LDAP and a +// connector.Identity is returned. Any prior probe cookie is cleared. +func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { + if c.krb == nil { + return nil, false, nil + } + + hasNegotiate := strings.HasPrefix(r.Header.Get("Authorization"), "Negotiate ") + + // Cookie probe: see the package-level doc on spnegoProbeCookieName. Only + // applies when fallback is enabled and the client has not (yet) sent a + // SPNEGO token; "Authorization present" paths bypass the probe entirely. + if c.krbConf.FallbackToPassword && !hasNegotiate { + if hasSPNEGOProbeCookie(r) { + c.logger.Info("kerberos: SPNEGO probe cookie present and no Negotiate header; falling back to password form") + return nil, false, nil + } + setSPNEGOProbeCookie(w, r) + } + + // Run the SPNEGO middleware with a capturing recorder so we can decide + // whether to forward its response or fall back to the password form. + var ( + id *credentials.Credentials + innerErr error + ) + inner := http.HandlerFunc(func(_ http.ResponseWriter, rr *http.Request) { + ident := goidentity.FromHTTPRequestContext(rr) + if ident == nil { + innerErr = fmt.Errorf("kerberos: no identity in request context after SPNEGO") + return + } + creds, ok := ident.(*credentials.Credentials) + if !ok { + innerErr = fmt.Errorf("kerberos: unexpected identity type %T", ident) + return + } + id = creds + }) + + rec := newBufferedResponse() + c.krb.authenticate(inner).ServeHTTP(rec, r) + + if innerErr != nil { + c.logger.Error("kerberos: SPNEGO middleware completed with unusable credentials", "err", innerErr) + return nil, true, innerErr + } + + if id == nil { + // Client offered a token and the middleware rejected it: under + // fallback semantics this is "tried and failed" — render the form + // rather than looping 401s. + if c.krbConf.FallbackToPassword && hasNegotiate { + c.logger.Info("kerberos: SPNEGO rejected client token; falling back to password form") + return nil, false, nil + } + // Otherwise (fallback=false, OR fallback=true probe round): forward + // the middleware-authored response (challenge token, error payload, + // etc.) verbatim — the protocol decided to reject and we preserve + // its wire details. + c.logger.Info("kerberos: SPNEGO did not authenticate; forwarding middleware response", "status", rec.code) + copyBuffered(rec, w) + return nil, true, nil + } + + if c.krbConf.ExpectedRealm != "" && !strings.EqualFold(c.krbConf.ExpectedRealm, id.Domain()) { + c.logger.Info("kerberos: realm mismatch", "expected", c.krbConf.ExpectedRealm, "actual", id.Domain()) + if c.krbConf.FallbackToPassword { + // Intentionally do NOT clear the probe cookie here: the + // client's TGT is for a realm we will never accept. Clearing + // would re-arm a probe round on the next no-Authorization GET, + // re-issue 401 Negotiate, the browser would resend the same + // wrong-realm token, and we'd land back here — an infinite + // flap. Keeping the cookie pins the client to the password + // form until the cookie's Max-Age elapses. + return nil, false, nil + } + // SPNEGO authenticated successfully, but our ExpectedRealm policy + // rejects it. The middleware's buffered response is irrelevant here + // (it represents the post-success path through the inner handler); + // emit our own bare Negotiate challenge so the client knows to + // retry with a different realm. + writeBareNegotiateChallenge(w) + return nil, true, nil + } + + mapped := mapPrincipal(id.UserName(), id.Domain(), c.krbConf.UsernameFromPrincipal) + c.logger.Info("kerberos: principal authenticated", + "principal", id.UserName(), + "realm", id.Domain(), + "auth_time", id.AuthTime(), + "mapped_username", mapped, + ) + + userEntry, err := c.lookupKerberosUser(ctx, mapped) + if err != nil { + c.logger.Error("kerberos: LDAP user lookup failed", + "principal", id.UserName(), "mapped", mapped, "err", err) + return nil, true, fmt.Errorf("ldap: user lookup failed for kerberos principal %q: %w", id.UserName(), err) + } + c.logger.Info("kerberos: LDAP user found", "dn", userEntry.DN) + + ident, err := c.identityFromEntry(userEntry) + if err != nil { + c.logger.Error("kerberos: failed to build identity from LDAP entry", "err", err) + return nil, true, err + } + if s.Groups { + groups, err := c.groups(ctx, userEntry) + if err != nil { + c.logger.Error("kerberos: failed to query groups", "err", err) + return nil, true, fmt.Errorf("ldap: failed to query groups: %w", err) + } + ident.Groups = groups + } + + // No user-bind has happened; only materialize ConnectorData when refresh + // needs the LDAP entry later (OfflineAccess). Mirror (*ldapConnector).Login: + // a marshal failure here would silently break a subsequent Refresh call + // (Unmarshal on nil), so fail the login instead of letting that happen. + if s.OfflineAccess { + refresh := refreshData{Username: mapped, Entry: userEntry} + data, mErr := json.Marshal(refresh) + if mErr != nil { + c.logger.Error("kerberos: failed to marshal refresh data", "err", mErr) + return nil, true, fmt.Errorf("ldap: marshal refresh data: %w", mErr) + } + ident.ConnectorData = data + } + + // If the client carried a stale probe cookie from a prior fallback + // round, clear it so a future logout/re-login starts negotiation fresh. + if hasSPNEGOProbeCookie(r) { + clearSPNEGOProbeCookie(w, r) + } + + c.logger.Info("kerberos: SPNEGO login succeeded", + "username", ident.Username, "email", ident.Email, "groups_count", len(ident.Groups)) + return &ident, true, nil +} + +// copyBuffered forwards a buffered handler response to the real client. +// Used when we want the middleware-authored bytes to reach the user agent +// unchanged (e.g. SPNEGO continuation/reject tokens). +func copyBuffered(rec *bufferedResponse, w http.ResponseWriter) { + for k, vv := range rec.header { + for _, v := range vv { + w.Header().Add(k, v) + } + } + w.WriteHeader(rec.code) + _, _ = w.Write(rec.body.Bytes()) +} + +// writeBareNegotiateChallenge emits an unsolicited "WWW-Authenticate: +// Negotiate" 401 directly to the client. This is reserved for cases where +// Dex itself rejects an otherwise-successful SPNEGO exchange (currently +// only ExpectedRealm mismatch); in those cases the middleware's buffered +// output does not represent the answer we want to send. +func writeBareNegotiateChallenge(w http.ResponseWriter) { + w.Header().Set("WWW-Authenticate", "Negotiate") + w.WriteHeader(http.StatusUnauthorized) +} + +// spnegoProbeCookieName names the short-lived cookie that records "we +// already issued a 401 Negotiate challenge to this client". It implements +// the fallback-to-password semantics: the very first GET without an +// Authorization header gets a real Negotiate challenge so a Kerberos-aware +// browser can SSO; if the client comes back without a token we treat that +// as "client cannot/will not negotiate" and render the password form +// instead of looping 401s. The cookie is bounded by a small Max-Age so a +// later visit gets a fresh chance to negotiate. +const ( + spnegoProbeCookieName = "dex_spnego_tried" + spnegoProbeMaxAge = 60 // seconds; long enough for one challenge round-trip +) + +func hasSPNEGOProbeCookie(r *http.Request) bool { + _, err := r.Cookie(spnegoProbeCookieName) + return err == nil +} + +// newSPNEGOProbeCookie returns a probe-cookie carrier with the attributes +// shared between "set" and "clear" forms (Path, HttpOnly, Secure, SameSite), +// so the two callers cannot accidentally drift apart. +func newSPNEGOProbeCookie(r *http.Request, value string, maxAge int) *http.Cookie { + return &http.Cookie{ + Name: spnegoProbeCookieName, + Value: value, + // Scope to "/" so the cookie reaches every Dex auth endpoint + // regardless of issuer prefix; the cookie carries no secret and + // expires within spnegoProbeMaxAge seconds, so the broad path is + // not a privacy or security boundary concern. + Path: "/", + MaxAge: maxAge, + HttpOnly: true, + Secure: isSecureRequest(r), + SameSite: http.SameSiteLaxMode, + } +} + +func setSPNEGOProbeCookie(w http.ResponseWriter, r *http.Request) { + http.SetCookie(w, newSPNEGOProbeCookie(r, "1", spnegoProbeMaxAge)) +} + +func clearSPNEGOProbeCookie(w http.ResponseWriter, r *http.Request) { + http.SetCookie(w, newSPNEGOProbeCookie(r, "", -1)) +} + +// isSecureRequest reports whether the original client connection is HTTPS. +// It honors X-Forwarded-Proto so the cookie's Secure attribute is correct +// when Dex sits behind a TLS-terminating proxy. The header is treated as +// advisory: misbehaving/spoofed values can only flip Secure to true on a +// plain-HTTP setup (which makes browsers refuse to echo the cookie back — +// a graceful no-op for the probe), never to false on a real HTTPS link. +func isSecureRequest(r *http.Request) bool { + if r.TLS != nil { + return true + } + return strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") +} + +// lookupKerberosUser resolves an LDAP user entry by username. When +// krbLookupUserHook is non-nil it is used exclusively and the real LDAP +// search is skipped entirely — this keeps unit tests hermetic. +func (c *ldapConnector) lookupKerberosUser(ctx context.Context, username string) (ldap.Entry, error) { + if c.krbLookupUserHook != nil { + return c.krbLookupUserHook(ctx, c, username) + } + + var userEntry ldap.Entry + err := c.do(ctx, func(conn *ldap.Conn) error { + entry, found, err := c.userEntry(conn, username) + if err != nil { + return err + } + if !found { + return fmt.Errorf("user not found for principal") + } + userEntry = entry + return nil + }) + return userEntry, err +} diff --git a/connector/ldap/kerberos_test.go b/connector/ldap/kerberos_test.go new file mode 100644 index 0000000000..8b9df0ade8 --- /dev/null +++ b/connector/ldap/kerberos_test.go @@ -0,0 +1,602 @@ +package ldap + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + ldaplib "github.com/go-ldap/ldap/v3" + "github.com/jcmturner/goidentity/v6" + "github.com/jcmturner/gokrb5/v8/credentials" + + "github.com/dexidp/dex/connector" +) + +// fakeAuthenticate returns a krbState whose authenticate middleware immediately +// invokes inner with the given credentials attached to the request context +// under goidentity.CTXKey — mimicking a successful SPNEGO handshake. +func fakeAuthenticate(creds *credentials.Credentials) *krbState { + return &krbState{ + authenticate: func(inner http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if creds == nil { + // Emulate a failed SPNEGO exchange: middleware writes 401 + // without invoking the inner handler. + w.Header().Set("WWW-Authenticate", "Negotiate") + http.Error(w, "auth failed\n", http.StatusUnauthorized) + return + } + rr := r.WithContext(context.WithValue(r.Context(), goidentity.CTXKey, goidentity.Identity(creds))) + inner.ServeHTTP(w, rr) + }) + }, + } +} + +// fakeAuthenticateWithResponse returns a krbState whose middleware writes an +// arbitrary response and does not invoke inner. Useful for testing the +// forward-vs-discard branch on failed SPNEGO. +func fakeAuthenticateWithResponse(status int, headers map[string]string, body string) *krbState { + return &krbState{ + authenticate: func(_ http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + for k, v := range headers { + w.Header().Set(k, v) + } + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + }) + }, + } +} + +func newCreds(username, realm string) *credentials.Credentials { + c := credentials.New(username, realm) + c.SetAuthenticated(true) + return c +} + +func TestKerberos_NoHeader_NoFallback_ForwardsMiddleware401(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(nil), + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) { + t.Fatalf("expected handled") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if w.Result().StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected bare Negotiate challenge, got %q", hdr) + } +} + +func TestKerberos_NoHeader_Fallback_NoCookie_ChallengesAndSetsCookie(t *testing.T) { + // First contact under fallback: no Authorization header, no probe cookie. + // We expect the middleware to run, its 401 Negotiate to be forwarded to + // the client, and a probe cookie to be set so the *next* round (still + // without an Authorization header) can short-circuit to the form. + middlewareRan := false + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, + krb: &krbState{authenticate: func(_ http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + middlewareRan = true + w.Header().Set("WWW-Authenticate", "Negotiate") + w.WriteHeader(http.StatusUnauthorized) + }) + }}, + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) { + t.Fatalf("expected handled (probe round forwards 401)") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if !middlewareRan { + t.Fatalf("SPNEGO middleware should run on the probe round") + } + if w.Result().StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected Negotiate challenge, got %q", hdr) + } + if !setCookieHasName(w, spnegoProbeCookieName) { + t.Fatalf("expected probe cookie %q to be set, got %q", spnegoProbeCookieName, w.Header().Values("Set-Cookie")) + } +} + +func TestKerberos_NoHeader_Fallback_WithCookie_RendersForm(t *testing.T) { + // Follow-up round: probe cookie is already in the request and the client + // still hasn't sent a Negotiate token. Treat that as "client cannot/will + // not SPNEGO" and short-circuit to the password form without running the + // middleware (otherwise w would get tainted with another 401). + middlewareRan := false + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, + krb: &krbState{authenticate: func(_ http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + middlewareRan = true + w.WriteHeader(http.StatusUnauthorized) + }) + }}, + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.AddCookie(&http.Cookie{Name: spnegoProbeCookieName, Value: "1"}) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if bool(handled) { + t.Fatalf("expected not handled (caller should render form)") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if middlewareRan { + t.Fatalf("SPNEGO middleware should not run on probe-follow-up round") + } + if w.Code != 200 && w.Code != 0 { + t.Fatalf("expected w untouched, got status=%d", w.Code) + } +} + +func TestKerberos_MiddlewareFails_Fallback_DiscardsResponse(t *testing.T) { + // Authorization header is present (so we enter middleware), middleware + // rejects the ticket, fallback is on: TrySPNEGO must return (nil, false, nil) + // and must not forward the middleware's 401 to the real ResponseWriter. + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticateWithResponse( + http.StatusUnauthorized, + map[string]string{"WWW-Authenticate": "Negotiate oQcwBaADCgEC"}, + "auth failed\n", + ), + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate deadbeef") + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if bool(handled) { + t.Fatalf("expected not handled (caller should render form)") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if w.Code != 200 && w.Code != 0 { + t.Fatalf("expected w untouched, got status=%d", w.Code) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "" { + t.Fatalf("expected no WWW-Authenticate on fallback, got %q", hdr) + } +} + +func TestKerberos_MiddlewareFails_NoFallback_ForwardsResponse(t *testing.T) { + // Middleware writes a reject token; no fallback: forward to client verbatim. + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticateWithResponse( + http.StatusUnauthorized, + map[string]string{"WWW-Authenticate": "Negotiate oQcwBaADCgEC"}, + "auth failed\n", + ), + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate deadbeef") + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) { + t.Fatalf("expected handled") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 forwarded, got %d", w.Code) + } + if hdr := w.Header().Get("WWW-Authenticate"); !strings.HasPrefix(hdr, "Negotiate ") { + t.Fatalf("expected reject token forwarded, got %q", hdr) + } +} + +func TestKerberos_Success_BuildsIdentity(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + } + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + lc.krbLookupUserHook = func(_ context.Context, c *ldapConnector, username string) (ldaplib.Entry, error) { + if username != "jdoe" { + t.Fatalf("expected username=jdoe, got %q", username) + } + e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ + c.UserSearch.IDAttr: {"uid-jdoe"}, + c.UserSearch.EmailAttr: {"jdoe@example.com"}, + c.UserSearch.NameAttr: {"John Doe"}, + }) + return *e, nil + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") + } + if ident.Username == "" || ident.Email == "" || ident.UserID == "" { + t.Fatalf("expected populated identity, got %+v", *ident) + } + // No probe cookie was on the request, so the success path must not emit + // a clearing Set-Cookie either (avoids needless header noise). + if setCookieHasName(w, spnegoProbeCookieName) { + t.Fatalf("did not expect Set-Cookie %q on success without prior probe cookie, got %q", + spnegoProbeCookieName, w.Header().Values("Set-Cookie")) + } +} + +// TestKerberos_Success_ClearsProbeCookie verifies that a successful SPNEGO +// login on a request that *did* carry a probe cookie clears it, so a future +// no-Authorization GET starts a fresh negotiation round instead of going +// straight to the password form. +func TestKerberos_Success_ClearsProbeCookie(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + } + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + lc.krbLookupUserHook = func(_ context.Context, c *ldapConnector, _ string) (ldaplib.Entry, error) { + e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ + c.UserSearch.IDAttr: {"uid-jdoe"}, + c.UserSearch.EmailAttr: {"jdoe@example.com"}, + c.UserSearch.NameAttr: {"John Doe"}, + }) + return *e, nil + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate deadbeef") + r.AddCookie(&http.Cookie{Name: spnegoProbeCookieName, Value: "1"}) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") + } + if !setCookieClearsName(w, spnegoProbeCookieName) { + t.Fatalf("expected probe cookie %q to be cleared, got %q", + spnegoProbeCookieName, w.Header().Values("Set-Cookie")) + } +} + +// setCookieHasName reports whether any Set-Cookie response header sets a +// cookie with the given name and a non-empty value (i.e. an actual set, not +// a clear). Order-tolerant; useful for assertions that don't care about +// exact attributes. +func setCookieHasName(w *httptest.ResponseRecorder, name string) bool { + for _, sc := range w.Result().Cookies() { + if sc.Name == name && sc.Value != "" && sc.MaxAge >= 0 { + return true + } + } + return false +} + +// setCookieClearsName reports whether any Set-Cookie response header clears +// the cookie with the given name (Max-Age<=0 or empty value). +func setCookieClearsName(w *httptest.ResponseRecorder, name string) bool { + for _, sc := range w.Result().Cookies() { + if sc.Name == name && (sc.MaxAge < 0 || sc.Value == "") { + return true + } + } + return false +} + +// TestKerberos_UserNotFound_ReturnsError pins the behavior when the LDAP +// directory has no entry for an authenticated Kerberos principal. The hook +// is authoritative, so the test is hermetic (no network call to :636). +func TestKerberos_UserNotFound_ReturnsError(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + } + lc.krbLookupUserHook = func(_ context.Context, _ *ldapConnector, _ string) (ldaplib.Entry, error) { + return ldaplib.Entry{}, fmt.Errorf("user not found for principal") + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + _, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err == nil { + t.Fatalf("expected error for user not found") + } + if !bool(handled) { + t.Fatalf("expected handled") + } + if !strings.Contains(err.Error(), "user lookup failed") { + t.Fatalf("expected 'user lookup failed' error, got: %v", err) + } + if !strings.Contains(err.Error(), "user not found for principal") { + t.Fatalf("expected wrapped hook error, got: %v", err) + } +} + +// TestKerberos_LookupHookError_Wrapped verifies hook errors propagate through +// TrySPNEGO with %w wrapping so callers can errors.Is against the original. +func TestKerberos_LookupHookError_Wrapped(t *testing.T) { + sentinel := errors.New("boom: LDAP server tantrum") + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + } + lc.krbLookupUserHook = func(_ context.Context, _ *ldapConnector, _ string) (ldaplib.Entry, error) { + return ldaplib.Entry{}, sentinel + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + _, _, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err == nil { + t.Fatalf("expected error") + } + if !errors.Is(err, sentinel) { + t.Fatalf("expected errors.Is(err, sentinel), got: %v", err) + } +} + +func TestKerberos_ExpectedRealmMismatch_NoFallback_401(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{ + FallbackToPassword: false, + UsernameFromPrincipal: "localpart", + ExpectedRealm: "EXAMPLE.COM", + }, + krb: fakeAuthenticate(newCreds("jdoe", "OTHER.COM")), + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident != nil { + t.Fatalf("expected handled with no identity") + } + if w.Result().StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected bare Negotiate, got %q", hdr) + } +} + +func TestKerberos_ExpectedRealmMismatch_Fallback_RendersForm(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{ + FallbackToPassword: true, + UsernameFromPrincipal: "localpart", + ExpectedRealm: "EXAMPLE.COM", + }, + krb: fakeAuthenticate(newCreds("jdoe", "OTHER.COM")), + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate deadbeef") + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if bool(handled) || ident != nil { + t.Fatalf("expected not handled, got handled=%v ident=%v", handled, ident) + } +} + +func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{ + FallbackToPassword: false, + UsernameFromPrincipal: "localpart", + ExpectedRealm: "ExAmPlE.CoM", + }, + krb: fakeAuthenticate(newCreds("user", "EXAMPLE.COM")), + } + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + lc.krbLookupUserHook = func(_ context.Context, c *ldapConnector, _ string) (ldaplib.Entry, error) { + e := ldaplib.NewEntry("cn=user,dc=example,dc=com", map[string][]string{ + c.UserSearch.IDAttr: {"uid-user"}, + c.UserSearch.EmailAttr: {"user@example.com"}, + c.UserSearch.NameAttr: {"User"}, + }) + return *e, nil + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") + } +} + +func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "userPrincipalName"}, + krb: fakeAuthenticate(newCreds("J.Doe", "Example.COM")), + } + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + lc.krbLookupUserHook = func(_ context.Context, c *ldapConnector, username string) (ldaplib.Entry, error) { + // userPrincipalName mapping reconstructs "username@realm" from the + // gokrb5 credentials; LDAP server handles case according to the + // attribute's matching rule. + if username != "J.Doe@Example.COM" { + t.Fatalf("expected reconstructed UPN, got %q", username) + } + e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ + c.UserSearch.IDAttr: {"uid-jdoe"}, + c.UserSearch.EmailAttr: {"jdoe@example.com"}, + c.UserSearch.NameAttr: {"John Doe"}, + }) + return *e, nil + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") + } +} + +func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "sAMAccountName"}, + krb: fakeAuthenticate(newCreds("Admin", "REALM.LOCAL")), + } + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + lc.krbLookupUserHook = func(_ context.Context, c *ldapConnector, username string) (ldaplib.Entry, error) { + if username != "Admin" { + t.Fatalf("expected localpart-derived username Admin, got %q", username) + } + e := ldaplib.NewEntry("cn=admin,dc=local", map[string][]string{ + c.UserSearch.IDAttr: {"uid-admin"}, + c.UserSearch.EmailAttr: {"admin@local"}, + c.UserSearch.NameAttr: {"Admin"}, + }) + return *e, nil + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") + } +} + +func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + } + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + lc.krbLookupUserHook = func(_ context.Context, c *ldapConnector, _ string) (ldaplib.Entry, error) { + e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ + c.UserSearch.IDAttr: {"uid-jdoe"}, + c.UserSearch.EmailAttr: {"jdoe@example.com"}, + c.UserSearch.NameAttr: {"John Doe"}, + }) + return *e, nil + } + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{OfflineAccess: true}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") + } + if len(ident.ConnectorData) == 0 { + t.Fatalf("expected connector data for offline access") + } +} + +func TestKerberos_Disabled_PassThrough(t *testing.T) { + // krb==nil: TrySPNEGO must leave w untouched and return (nil, false, nil). + lc := &ldapConnector{logger: slog.Default()} + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if bool(handled) || ident != nil { + t.Fatalf("expected not handled with no identity") + } +} + +func TestKerberos_mapPrincipal(t *testing.T) { + cases := []struct { + username, realm, mode, want string + }{ + {"JDoe", "EXAMPLE.COM", "localpart", "JDoe"}, + {"JDoe", "EXAMPLE.COM", "sAMAccountName", "JDoe"}, + {"JDoe", "EXAMPLE.COM", "userPrincipalName", "JDoe@EXAMPLE.COM"}, + {"JDoe", "EXAMPLE.COM", "", "JDoe"}, + // Defensive cases: username accidentally contains '@'. + {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "localpart", "JDoe"}, + {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "userPrincipalName", "JDoe@EXAMPLE.COM"}, + // Empty realm for userPrincipalName degrades to bare username. + {"jdoe", "", "userPrincipalName", "jdoe"}, + {"", "EXAMPLE.COM", "localpart", ""}, + } + for _, c := range cases { + got := mapPrincipal(c.username, c.realm, c.mode) + if got != c.want { + t.Fatalf("mapPrincipal(%q,%q,%q)=%q; want %q", c.username, c.realm, c.mode, got, c.want) + } + } +} diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index e01e2df8c1..27f93e3393 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -125,6 +125,9 @@ type Config struct { // "Username". UsernamePrompt string `json:"usernamePrompt"` + // Optional Kerberos (SPNEGO) SSO configuration. + Kerberos *kerberosConfig `json:"kerberos"` + // User entry search configuration. UserSearch struct { // BaseDN to start the search from. For example "cn=users,dc=example,dc=com" @@ -188,6 +191,33 @@ type Config struct { } `json:"groupSearch"` } +// kerberosConfig defines optional Kerberos (SPNEGO) SSO settings for LDAP. +// +// Required: +// - Enabled, KeytabPath. +// +// Optional tuning: +// - SPN: overrides the service principal name extracted from the keytab +// (useful when Dex sits behind a reverse proxy that rewrites Host). +// - KeytabPrincipal: selects a specific principal out of a multi-entry +// keytab (e.g. "HTTP/dex.example.com"). +// - MaxClockSkew: seconds of acceptable clock skew between the KDC, the +// client and Dex; defaults to gokrb5's 300s when unset. +// - ExpectedRealm: rejects tickets from realms other than this one. +// - UsernameFromPrincipal: "localpart" (default) or "userPrincipalName". +// - FallbackToPassword: render the password form when SPNEGO is absent or +// fails, instead of returning 401. +type kerberosConfig struct { + Enabled bool `json:"enabled"` + KeytabPath string `json:"keytabPath"` + SPN string `json:"spn"` + KeytabPrincipal string `json:"keytabPrincipal"` + MaxClockSkew int `json:"maxClockSkew"` + ExpectedRealm string `json:"expectedRealm"` + UsernameFromPrincipal string `json:"usernameFromPrincipal"` + FallbackToPassword bool `json:"fallbackToPassword"` +} + func scopeString(i int) string { switch i { case ldap.ScopeBaseObject: @@ -241,6 +271,24 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro if err != nil { return nil, err } + // If Kerberos is enabled, load the keytab and bind SPNEGO middleware. + // The presence of a non-nil krb on ldapConnector is the single source of + // truth for "SPNEGO is live"; TrySPNEGO short-circuits when it's nil. + if lc, ok := conn.(*ldapConnector); ok && lc.krbConf.Enabled && lc.krb == nil { + st, kerr := loadKerberosState(lc.krbConf) + if kerr != nil { + logger.Warn("failed to initialize kerberos; disabling kerberos", "err", kerr) + } else { + lc.krb = st + logger.Info("kerberos SPNEGO enabled for LDAP connector", + "keytab", lc.krbConf.KeytabPath, + "spn", lc.krbConf.SPN, + "keytab_principal", lc.krbConf.KeytabPrincipal, + "expected_realm", lc.krbConf.ExpectedRealm, + "fallback_to_password", lc.krbConf.FallbackToPassword, + ) + } + } return connector.Connector(conn), nil } @@ -325,12 +373,39 @@ func (c *Config) openConnector(logger *slog.Logger) (*ldapConnector, error) { // TODO(nabokihms): remove it after deleting deprecated groupSearch options c.GroupSearch.UserMatchers = userMatchers(c, logger) - return &ldapConnector{*c, userSearchScope, groupSearchScope, tlsConfig, c.UserSearch.Username, logger}, nil + + // Normalize Kerberos defaults. We persist krbConf only when it is + // usable (Enabled + KeytabPath set); otherwise it stays at the zero + // value and Open() will skip loadKerberosState — which keeps krb nil + // and effectively disables SPNEGO without any extra flag. + var krbConf kerberosConfig + if c.Kerberos != nil && c.Kerberos.Enabled { + krbConf = *c.Kerberos + if krbConf.UsernameFromPrincipal == "" { + krbConf.UsernameFromPrincipal = "localpart" + } + if krbConf.KeytabPath == "" { + logger.Warn("kerberos enabled but keytabPath is empty; disabling kerberos") + krbConf = kerberosConfig{} + } + } + + lc := &ldapConnector{ + Config: *c, + userSearchScope: userSearchScope, + groupSearchScope: groupSearchScope, + tlsConfig: tlsConfig, + usernameAttrs: c.UserSearch.Username, + logger: logger, + krbConf: krbConf, + } + return lc, nil } var ( _ connector.PasswordConnector = (*ldapConnector)(nil) _ connector.RefreshConnector = (*ldapConnector)(nil) + _ connector.SPNEGOAware = (*ldapConnector)(nil) ) type ldapConnector struct { @@ -344,6 +419,17 @@ type ldapConnector struct { usernameAttrs []string logger *slog.Logger + + // Kerberos/SPNEGO support. krb is nil until the keytab has been loaded + // successfully in Open(); TrySPNEGO uses (krb == nil) as the single + // "is SPNEGO available" check, so no separate enable flag is needed. + krbConf kerberosConfig + krb *krbState + // krbLookupUserHook allows tests to replace the LDAP user lookup entirely. + // When set it is authoritative: lookupKerberosUser does not fall through + // to a real LDAP search. Tests signal "user not found" by returning an + // error, not by returning a zero entry with a nil error. + krbLookupUserHook func(ctx context.Context, c *ldapConnector, username string) (ldap.Entry, error) } // do initializes a connection to the LDAP directory and passes it to the diff --git a/connector/spnego.go b/connector/spnego.go new file mode 100644 index 0000000000..ab89e84069 --- /dev/null +++ b/connector/spnego.go @@ -0,0 +1,22 @@ +package connector + +import ( + "context" + "net/http" +) + +// Handled indicates whether the SPNEGO-aware connector handled the request. +type Handled bool + +// SPNEGOAware is an optional extension for connectors that can authenticate +// users via Kerberos SPNEGO on the initial GET to the password login endpoint. +// +// If handled is true and ident is non-nil, the caller should complete the +// OAuth flow as with a successful password login. If handled is true and +// ident is nil, the implementation has already written an appropriate +// response (e.g., 401 with WWW-Authenticate: Negotiate) and the caller should +// return without rendering the password form. If handled is false, proceed +// with the legacy password form flow. +type SPNEGOAware interface { + TrySPNEGO(ctx context.Context, s Scopes, w http.ResponseWriter, r *http.Request) (*Identity, Handled, error) +} diff --git a/examples/ldap/config-ldap.yaml b/examples/ldap/config-ldap.yaml index 05d1661826..49a7e25fff 100644 --- a/examples/ldap/config-ldap.yaml +++ b/examples/ldap/config-ldap.yaml @@ -59,6 +59,19 @@ connectors: # The group name should be the "cn" value. nameAttr: cn + # Optional Kerberos (SPNEGO) SSO. When enabled, Dex will challenge with + # WWW-Authenticate: Negotiate on GET and skip the password form on success. + #kerberos: + # enabled: true + # keytabPath: /etc/dex/krb5.keytab + # expectedRealm: EXAMPLE.COM + # usernameFromPrincipal: sAMAccountName # or userPrincipalName or localpart + # fallbackToPassword: false + # # Optional gokrb5 service settings — leave empty to use library defaults. + # spn: HTTP/dex.example.com # service principal name expected in tickets + # keytabPrincipal: HTTP/dex.example.com@EXAMPLE.COM # explicit principal to load from the keytab + # maxClockSkew: 300 # tolerated clock skew, seconds (default 300) + staticClients: - id: example-app redirectURIs: diff --git a/go.mod b/go.mod index e7150d13a6..f7c2b0834d 100644 --- a/go.mod +++ b/go.mod @@ -22,6 +22,8 @@ require ( github.com/gorilla/handlers v1.5.2 github.com/gorilla/mux v1.8.1 github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 + github.com/jcmturner/goidentity/v6 v6.0.1 + github.com/jcmturner/gokrb5/v8 v8.4.4 github.com/kylelemons/godebug v1.1.0 github.com/lib/pq v1.12.3 github.com/mattermost/xml-roundtrip-validator v0.1.0 @@ -94,10 +96,15 @@ require ( github.com/hashicorp/go-secure-stdlib/parseutil v0.2.0 // indirect github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect github.com/hashicorp/go-sockaddr v1.0.7 // indirect + github.com/hashicorp/go-uuid v1.0.3 // indirect github.com/hashicorp/hcl v1.0.1-vault-7 // indirect github.com/hashicorp/hcl/v2 v2.18.1 // indirect github.com/huandu/xstrings v1.5.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/jcmturner/aescts/v2 v2.0.0 // indirect + github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect + github.com/jcmturner/gofork v1.7.6 // indirect + github.com/jcmturner/rpc/v2 v2.0.3 // indirect github.com/jonboulle/clockwork v0.5.0 // indirect github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-isatty v0.0.20 // indirect diff --git a/go.sum b/go.sum index 4fc0627904..6da4f92dad 100644 --- a/go.sum +++ b/go.sum @@ -129,6 +129,10 @@ github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyE github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= +github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyCS8BvQ= +github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= +github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7FsgI= +github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 h1:5ZPtiqj0JL5oKWmcsq4VMaAW5ukBEgSGXEN89zeH1Jo= @@ -150,6 +154,7 @@ github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 h1:kes8mmyCpxJsI7FTwtzRqEy9 github.com/hashicorp/go-secure-stdlib/strutil v0.1.2/go.mod h1:Gou2R9+il93BqX25LAKCLuM+y9U2T4hlwvT1yprcna4= github.com/hashicorp/go-sockaddr v1.0.7 h1:G+pTkSO01HpR5qCxg7lxfsFEZaG+C0VssTy/9dbT+Fw= github.com/hashicorp/go-sockaddr v1.0.7/go.mod h1:FZQbEYa1pxkQ7WLpyXJ6cbjpT8q0YgQaK/JakXqGyWw= +github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I= @@ -258,10 +263,16 @@ github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/tinylib/msgp v1.6.3 h1:bCSxiTz386UTgyT1i0MSCvdbWjVW+8sG3PjkGsZQt4s= @@ -270,6 +281,7 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= github.com/zclconf/go-cty v1.14.4 h1:uXXczd9QDGsgu0i/QFR/hzI5NYCHLf6NQw/atrbnhq8= github.com/zclconf/go-cty v1.14.4/go.mod h1:VvMs5i0vgZdhYawQNq5kePSpLAoz8u1xvZgrPIxfnZE= github.com/zclconf/go-cty-yaml v1.1.0 h1:nP+jp0qPHv2IhUVqmQSzjvqAWcObN0KBkUl2rWBdig0= @@ -311,18 +323,26 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= golang.org/x/exp v0.0.0-20240823005443-9b4947da3948 h1:kx6Ds3MlpiUHKj7syVnbp57++8WpuKPcR5yjLBjvLEA= golang.org/x/exp v0.0.0-20240823005443-9b4947da3948/go.mod h1:akd2r19cwCdwSwWeIdzYQGa/EZZyqcOdwWiwj5L5eKQ= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= +golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= +golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= @@ -330,16 +350,27 @@ golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= @@ -348,6 +379,7 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= golang.org/x/tools/go/expect v0.1.0-deprecated h1:jY2C5HGYR5lqex3gEniOQL0r7Dq5+VGVgY1nudX5lXY= @@ -376,6 +408,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/server/handlers.go b/server/handlers.go index d2412c9825..452e5079c2 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -606,6 +606,45 @@ func (s *Server) handlePasswordLogin(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: + // Before rendering the password form, allow connectors that support SPNEGO to try Kerberos auth. + if sp, ok := pwConn.(connector.SPNEGOAware); ok { + scopes := parseScopes(authReq.Scopes) + if ident, handled, err := sp.TrySPNEGO(ctx, scopes, w, r); bool(handled) { + if err != nil { + // SPNEGO handled the request but reported an error (e.g., LDAP lookup failed + // after successful Kerberos auth). Log error details, show generic message to user. + s.logger.ErrorContext(ctx, "SPNEGO authentication error", "err", err) + s.renderError(r, w, http.StatusUnauthorized, ErrMsgAuthenticationFailed) + return + } + if ident != nil { + redirectURL, canSkipApproval, err := s.finalizeLogin(ctx, *ident, authReq, conn.Connector) + if err != nil { + s.logger.ErrorContext(ctx, "failed to finalize login", "err", err) + s.renderError(r, w, http.StatusInternalServerError, "Login error.") + return + } + + if canSkipApproval { + authReq, err = s.storage.GetAuthRequest(ctx, authReq.ID) + if err != nil { + s.logger.ErrorContext(ctx, "failed to get finalized auth request", "err", err) + s.renderError(r, w, http.StatusInternalServerError, "Login error.") + return + } + s.sendCodeResponse(w, r, authReq) + return + } + + http.Redirect(w, r, redirectURL, http.StatusSeeOther) + return + } + // handled with no identity typically means the SPNEGO middleware + // wrote its own 401 (bare challenge, continuation, or reject); do + // not render the password form on top of it. + return + } + } if err := s.templates.password(r, w, r.URL.String(), "", usernamePrompt(pwConn), false, backLink, rememberMe); err != nil { s.logger.ErrorContext(r.Context(), "server template error", "err", err) } diff --git a/server/handlers_test.go b/server/handlers_test.go index d21244125c..124c635bc6 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -1424,6 +1424,187 @@ func TestHandleConnectorCallbackWithSkipApproval(t *testing.T) { } } +// SPNEGO integration test (server layer): on GET login, if connector implements SPNEGOAware +// and returns an identity, server should finalize login and redirect without rendering form. +func TestHandlePasswordLogin_SPNEGOShortCircuit(t *testing.T) { + ctx := t.Context() + connID := "mockPassword" + authReqID := "spnego" + expiry := time.Now().Add(100 * time.Second) + resTypes := []string{responseTypeCode} + + httpServer, s := newTestServer(t, func(c *Config) { + c.SkipApprovalScreen = true + c.Now = time.Now + }) + defer httpServer.Close() + + // Create password connector which we will wrap with a SPNEGO-aware adapter in storage config + sc := storage.Connector{ + ID: connID, + Type: "mockPassword", + Name: "MockPassword", + ResourceVersion: "1", + Config: []byte("{\"username\": \"foo\", \"password\": \"password\"}"), + } + require.NoError(t, s.storage.CreateConnector(ctx, sc)) + _, err := s.OpenConnector(sc) + require.NoError(t, err) + + // Prepare auth request + require.NoError(t, s.storage.CreateAuthRequest(ctx, storage.AuthRequest{ + ID: authReqID, + ClientID: "client_1", + ConnectorID: connID, + RedirectURI: "cb", + Expiry: expiry, + ResponseTypes: resTypes, + Scopes: []string{"openid"}, + })) + + // Replace the server connector with a SPNEGO-aware fake that short-circuits + s.mu.Lock() + orig := s.connectors[connID] + s.connectors[connID] = Connector{ + ResourceVersion: orig.ResourceVersion, + Connector: spnegoShortCircuit{Identity: connector.Identity{ + UserID: "user-id", + Username: "user", + Email: "user@example.com", + EmailVerified: true, + }}, + } + s.mu.Unlock() + + // Need a client for finalizeLogin to succeed + require.NoError(t, s.storage.CreateClient(ctx, storage.Client{ + ID: "client_1", + Secret: "secret", + RedirectURIs: []string{"http://127.0.0.1/callback"}, + Name: "test", + })) + + // GET login should short-circuit and redirect to /approval or code response + rr := httptest.NewRecorder() + path := fmt.Sprintf("/auth/%s/login?state=%s&back=", connID, authReqID) + s.handlePasswordLogin(rr, httptest.NewRequest("GET", path, nil)) + + // In SkipApproval mode server may directly send code response (200) or 303 redirect + if rr.Code != http.StatusSeeOther && rr.Code != http.StatusOK { + t.Fatalf("expected 200 or 303, got %d", rr.Code) + } +} + +// spnegoShortCircuit implements connector.PasswordConnector and connector.SPNEGOAware +// to simulate successful SPNEGO authentication on GET. +type spnegoShortCircuit struct{ Identity connector.Identity } + +func (s spnegoShortCircuit) Close() error { return nil } + +func (s spnegoShortCircuit) Prompt() string { return "" } + +func (s spnegoShortCircuit) Login(ctx context.Context, sc connector.Scopes, u, p string) (connector.Identity, bool, error) { + return connector.Identity{}, false, nil +} + +func (s spnegoShortCircuit) TrySPNEGO(ctx context.Context, sc connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { + id := s.Identity + return &id, true, nil +} + +// spnegoError implements connector.PasswordConnector and connector.SPNEGOAware +// to simulate SPNEGO authentication that fails with an error (e.g., LDAP lookup failed). +type spnegoError struct{ Err error } + +func (s spnegoError) Close() error { return nil } + +func (s spnegoError) Prompt() string { return "" } + +func (s spnegoError) Login(ctx context.Context, sc connector.Scopes, u, p string) (connector.Identity, bool, error) { + return connector.Identity{}, false, nil +} + +func (s spnegoError) TrySPNEGO(ctx context.Context, sc connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { + return nil, true, s.Err +} + +// TestHandlePasswordLogin_SPNEGOError verifies that when SPNEGO returns an error +// (e.g., Kerberos auth succeeded but LDAP lookup failed), the server renders +// an error page instead of showing an empty 401 or falling back to password form. +func TestHandlePasswordLogin_SPNEGOError(t *testing.T) { + ctx := t.Context() + connID := "mockPassword" + authReqID := "spnego-err" + expiry := time.Now().Add(100 * time.Second) + resTypes := []string{responseTypeCode} + + httpServer, s := newTestServer(t, func(c *Config) { + c.SkipApprovalScreen = true + c.Now = time.Now + }) + defer httpServer.Close() + + // Create password connector + sc := storage.Connector{ + ID: connID, + Type: "mockPassword", + Name: "MockPassword", + ResourceVersion: "1", + Config: []byte("{\"username\": \"foo\", \"password\": \"password\"}"), + } + require.NoError(t, s.storage.CreateConnector(ctx, sc)) + _, err := s.OpenConnector(sc) + require.NoError(t, err) + + // Prepare auth request + require.NoError(t, s.storage.CreateAuthRequest(ctx, storage.AuthRequest{ + ID: authReqID, + ClientID: "client_1", + ConnectorID: connID, + RedirectURI: "cb", + Expiry: expiry, + ResponseTypes: resTypes, + Scopes: []string{"openid"}, + })) + + // Replace the server connector with a SPNEGO-aware fake that returns an error + s.mu.Lock() + orig := s.connectors[connID] + s.connectors[connID] = Connector{ + ResourceVersion: orig.ResourceVersion, + Connector: spnegoError{Err: errors.New("ldap: user lookup failed for kerberos principal")}, + } + s.mu.Unlock() + + // Need a client for the flow + require.NoError(t, s.storage.CreateClient(ctx, storage.Client{ + ID: "client_1", + Secret: "secret", + RedirectURIs: []string{"http://127.0.0.1/callback"}, + Name: "test", + })) + + // GET login should return 401 with error message rendered + rr := httptest.NewRecorder() + path := fmt.Sprintf("/auth/%s/login?state=%s&back=", connID, authReqID) + s.handlePasswordLogin(rr, httptest.NewRequest("GET", path, nil)) + + // Should return 401 (unauthorized) with an error page, not 200 (form) or empty response + if rr.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 Unauthorized, got %d", rr.Code) + } + + // The response body should contain safe generic error message (not internal details) + body := rr.Body.String() + if !strings.Contains(body, "Authentication failed") { + t.Fatalf("expected error page with 'Authentication failed', got: %s", body) + } + // Should NOT contain internal error details (per 008-hide-internal-500-error-details.patch) + if strings.Contains(body, "ldap: user lookup failed") { + t.Fatalf("error page should not contain internal error details") + } +} + func TestHandleTokenExchange(t *testing.T) { tests := []struct { name string