From 2a716ad438fb6d1db87e73f6f9b77b3e2b4eea4f Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Fri, 13 Mar 2026 15:41:28 +0300 Subject: [PATCH 1/7] feat: Add Kerberos support Signed-off-by: Ivan Zvyagintsev --- connector/ldap/kerberos.go | 332 +++++++++++++++++++++++ connector/ldap/kerberos_test.go | 452 ++++++++++++++++++++++++++++++++ connector/ldap/ldap.go | 62 ++++- connector/spnego.go | 22 ++ examples/ldap/config-ldap.yaml | 9 + go.mod | 7 + go.sum | 31 +++ server/handlers.go | 37 +++ server/handlers_test.go | 175 +++++++++++++ 9 files changed, 1126 insertions(+), 1 deletion(-) create mode 100644 connector/ldap/kerberos.go create mode 100644 connector/ldap/kerberos_test.go create mode 100644 connector/spnego.go diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go new file mode 100644 index 0000000000..7030bfb641 --- /dev/null +++ b/connector/ldap/kerberos.go @@ -0,0 +1,332 @@ +// Package ldap implements strategies for authenticating using the LDAP protocol. +// This file contains Kerberos/SPNEGO authentication support for LDAP connector. +package ldap + +import ( + "context" + "encoding/base64" + "encoding/json" + "fmt" + "log/slog" + "net/http" + "os" + "strings" + + "github.com/jcmturner/gofork/encoding/asn1" + "github.com/jcmturner/gokrb5/v8/credentials" + "github.com/jcmturner/gokrb5/v8/gssapi" + "github.com/jcmturner/gokrb5/v8/keytab" + "github.com/jcmturner/gokrb5/v8/service" + "github.com/jcmturner/gokrb5/v8/spnego" + "github.com/jcmturner/gokrb5/v8/types" + + "github.com/dexidp/dex/connector" + "github.com/go-ldap/ldap/v3" +) + +// KerberosValidator abstracts SPNEGO validation for unit-testing. +type KerberosValidator interface { + // ValidateRequest returns (principal, realm, ok, err). ok=false means header missing/invalid. + ValidateRequest(r *http.Request) (string, string, bool, error) + // Challenge writes a 401 Negotiate challenge. + Challenge(w http.ResponseWriter) + // ContinueToken tries to advance SPNEGO handshake and returns a response token to include + // in WWW-Authenticate: Negotiate . Returns (nil, false) if no continuation is needed/possible. + ContinueToken(r *http.Request) ([]byte, bool) +} + +// writeNegotiateChallenge writes a standard 401 Negotiate challenge. +func writeNegotiateChallenge(w http.ResponseWriter) { + w.Header().Set("WWW-Authenticate", "Negotiate") + w.WriteHeader(http.StatusUnauthorized) +} + +// mapPrincipal maps a Kerberos principal to LDAP username per configuration. +func mapPrincipal(principal, realm, mapping string) string { + p := principal + switch strings.ToLower(mapping) { + case "localpart", "samaccountname": + if i := strings.IndexByte(principal, '@'); i >= 0 { + p = principal[:i] + } + return strings.ToLower(p) + case "userprincipalname": + return strings.ToLower(principal) + default: + if i := strings.IndexByte(principal, '@'); i >= 0 { + p = principal[:i] + } + return strings.ToLower(p) + } +} + +// gokrb5 implementation of KerberosValidator + +// context key used by gokrb5 to store credentials in the context +var ctxCredentialsKey interface{} = "github.com/jcmturner/gokrb5/v8/ctxCredentials" + +// SPNEGO NegTokenResp (AcceptIncomplete + KRB5 mech) base64 payload used by gokrb5's HTTP server +// to prompt the client to continue the handshake. +const spnegoIncompleteKRB5B64 = "oRQwEqADCgEBoQsGCSqGSIb3EgECAg==" + +type gokrb5Validator struct { + kt *keytab.Keytab + logger *slog.Logger +} + +func newGokrb5ValidatorWithLogger(keytabPath string, logger *slog.Logger) (KerberosValidator, error) { + kt, err := keytab.Load(keytabPath) + if err != nil { + return nil, fmt.Errorf("failed to load keytab: %w", err) + } + if fi, err := os.Stat(keytabPath); err != nil || fi.IsDir() { + return nil, fmt.Errorf("invalid keytab path: %s", keytabPath) + } + if logger == nil { + logger = slog.Default() + } + return &gokrb5Validator{kt: kt, logger: logger}, nil +} + +func (v *gokrb5Validator) ValidateRequest(r *http.Request) (string, string, bool, error) { + h := r.Header.Get("Authorization") + if h == "" || !strings.HasPrefix(h, "Negotiate ") { + if v.logger != nil { + v.logger.Info("kerberos: missing or non-negotiate Authorization header", "path", r.URL.Path) + } + return "", "", false, nil + } + b64 := strings.TrimSpace(h[len("Negotiate "):]) + if b64 == "" { + if v.logger != nil { + v.logger.Info("kerberos: empty negotiate token", "path", r.URL.Path) + } + return "", "", false, nil + } + data, err := base64.StdEncoding.DecodeString(b64) + if err != nil { + if v.logger != nil { + v.logger.Info("kerberos: invalid base64 in Authorization", "err", err) + } + return "", "", false, nil + } + var tok spnego.SPNEGOToken + if err := tok.Unmarshal(data); err != nil { + // Try raw KRB5 token and wrap + var k5 spnego.KRB5Token + if k5.Unmarshal(data) != nil { + if v.logger != nil { + v.logger.Info("kerberos: failed to unmarshal SPNEGO token and not raw KRB5", "err", err) + } + return "", "", false, nil + } + tok.Init = true + tok.NegTokenInit = spnego.NegTokenInit{ + MechTypes: []asn1.ObjectIdentifier{k5.OID}, + MechTokenBytes: data, + } + } + + // Pass client address when available (improves AP-REQ validation with address-bound tickets) + var sp *spnego.SPNEGO + if ha, err := types.GetHostAddress(r.RemoteAddr); err == nil { + sp = spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) + } else { + if v.logger != nil { + v.logger.Info("kerberos: cannot parse client address", "remote", r.RemoteAddr, "err", err) + } + sp = spnego.SPNEGOService(v.kt, service.DecodePAC(false)) + } + authed, ctx, status := sp.AcceptSecContext(&tok) + if status.Code != gssapi.StatusComplete { + if v.logger != nil { + v.logger.Info("kerberos: AcceptSecContext not complete", "code", status.Code, "message", status.Message) + } + return "", "", false, nil + } + if !authed || ctx == nil { + if v.logger != nil { + v.logger.Info("kerberos: not authenticated or no context") + } + return "", "", false, nil + } + id, _ := ctx.Value(ctxCredentialsKey).(*credentials.Credentials) + if id == nil { + if v.logger != nil { + v.logger.Info("kerberos: credentials missing in context") + } + return "", "", false, fmt.Errorf("no credentials in context") + } + return id.UserName(), id.Domain(), true, nil +} + +func (v *gokrb5Validator) Challenge(w http.ResponseWriter) { writeNegotiateChallenge(w) } + +// ContinueToken attempts to continue the SPNEGO handshake and returns a response token +// (to be placed into WWW-Authenticate: Negotiate ) if available. +func (v *gokrb5Validator) ContinueToken(r *http.Request) ([]byte, bool) { + h := r.Header.Get("Authorization") + if h == "" || !strings.HasPrefix(h, "Negotiate ") { + if v.logger != nil { + v.logger.Info("kerberos: ContinueToken without negotiate header", "path", r.URL.Path) + } + return nil, false + } + b64 := strings.TrimSpace(h[len("Negotiate "):]) + data, err := base64.StdEncoding.DecodeString(b64) + if err != nil { + // Malformed header: ask client to continue with KRB5 mech + if tok, e := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64); e == nil { + if v.logger != nil { + v.logger.Info("kerberos: malformed negotiate token; sending incomplete KRB5 response") + } + return tok, true + } + return nil, false + } + var tok spnego.SPNEGOToken + if err := tok.Unmarshal(data); err != nil { + // Not a full SPNEGO token; still ask client to continue + if tokb, e := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64); e == nil { + if v.logger != nil { + v.logger.Info("kerberos: non-SPNEGO token; sending incomplete KRB5 response") + } + return tokb, true + } + // As a fallback, try wrapping as raw KRB5 + var k5 spnego.KRB5Token + if k5.Unmarshal(data) != nil { + if v.logger != nil { + v.logger.Info("kerberos: not KRB5 token; cannot continue") + } + return nil, false + } + tok.Init = true + tok.NegTokenInit = spnego.NegTokenInit{MechTypes: []asn1.ObjectIdentifier{k5.OID}, MechTokenBytes: data} + } + // Try continue with same options as in ValidateRequest + var sp *spnego.SPNEGO + if ha, err := types.GetHostAddress(r.RemoteAddr); err == nil { + sp = spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) + } else { + sp = spnego.SPNEGOService(v.kt, service.DecodePAC(false)) + } + _, ctx, status := sp.AcceptSecContext(&tok) + if status.Code != gssapi.StatusContinueNeeded || ctx == nil { + if v.logger != nil { + v.logger.Info("kerberos: no continuation required", "code", status.Code, "message", status.Message) + } + return nil, false + } + // Ask client to continue using standard NegTokenResp (KRB5, incomplete) + if tokb, e := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64); e == nil { + if v.logger != nil { + v.logger.Info("kerberos: continuation needed; sending incomplete KRB5 response") + } + return tokb, true + } + return nil, false +} + +// LDAP connector SPNEGO integration + +// krbLookupUserHook allows tests to inject a user entry without LDAP queries. +var krbLookupUserHook func(c *ldapConnector, username string) (ldap.Entry, bool, error) + +// TrySPNEGO attempts Kerberos auth and builds identity on success. +func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { + if !c.krbEnabled || c.krbValidator == nil { + return nil, false, nil + } + + principal, realm, ok, err := c.krbValidator.ValidateRequest(r) + if err != nil || !ok { + if !c.krbConf.FallbackToPassword { + // Try to get a continuation token to advance SPNEGO handshake + if tok, ok2 := c.krbValidator.ContinueToken(r); ok2 && len(tok) > 0 { + c.logger.Info("kerberos SPNEGO continuation required; sending response token") + w.Header().Set("WWW-Authenticate", "Negotiate "+base64.StdEncoding.EncodeToString(tok)) + w.WriteHeader(http.StatusUnauthorized) + return nil, true, nil + } + if err != nil { + c.logger.Info("kerberos SPNEGO validation error; sending Negotiate challenge", "err", err) + } else { + c.logger.Info("kerberos SPNEGO not completed or header missing; sending Negotiate challenge") + } + c.krbValidator.Challenge(w) + return nil, true, nil + } + c.logger.Info("kerberos SPNEGO fallback to password enabled; rendering login form") + return nil, false, nil + } + + if c.krbConf.ExpectedRealm != "" && !strings.EqualFold(c.krbConf.ExpectedRealm, realm) { + c.logger.Info("kerberos realm mismatch", "expected", c.krbConf.ExpectedRealm, "actual", realm) + if !c.krbConf.FallbackToPassword { + c.krbValidator.Challenge(w) + return nil, true, nil + } + c.logger.Info("kerberos realm mismatch but fallback enabled; rendering login form") + return nil, false, nil + } + + mapped := mapPrincipal(principal, realm, c.krbConf.UsernameFromPrincipal) + c.logger.Info("kerberos principal mapped", "principal", principal, "realm", realm, "mapped_username", mapped) + + var userEntry ldap.Entry + // Allow test hook override + if krbLookupUserHook != nil { + if v, found, herr := krbLookupUserHook(c, mapped); found { + if herr != nil { + return nil, true, herr + } + userEntry = v + } + } + + if userEntry.DN == "" { + // Reuse existing search logic via do() and userEntry + err = c.do(ctx, func(conn *ldap.Conn) error { + entry, found, err := c.userEntry(conn, mapped) + if err != nil { + return err + } + if !found { + return fmt.Errorf("user not found for principal") + } + userEntry = entry + return nil + }) + } + if err != nil { + c.logger.Error("kerberos user lookup failed", "principal", principal, "mapped", mapped, "err", err) + return nil, true, fmt.Errorf("ldap: user lookup failed for kerberos principal %q: %v", principal, err) + } + c.logger.Info("kerberos user lookup succeeded", "dn", userEntry.DN) + + ident, err := c.identityFromEntry(userEntry) + if err != nil { + c.logger.Info("failed to build identity from LDAP entry after kerberos SPNEGO", "err", err) + return nil, true, err + } + if s.Groups { + groups, err := c.groups(ctx, userEntry) + if err != nil { + c.logger.Info("failed to query groups after kerberos SPNEGO", "err", err) + return nil, true, fmt.Errorf("ldap: failed to query groups: %v", err) + } + ident.Groups = groups + } + + // No password -> no user bind; do not set ConnectorData unless OfflineAccess requested + if s.OfflineAccess { + refresh := refreshData{Username: mapped, Entry: userEntry} + if data, mErr := json.Marshal(refresh); mErr == nil { + ident.ConnectorData = data + } + } + + c.logger.Info("kerberos SPNEGO authentication succeeded", "username", ident.Username, "email", ident.Email, "groups_count", len(ident.Groups)) + return &ident, true, nil +} diff --git a/connector/ldap/kerberos_test.go b/connector/ldap/kerberos_test.go new file mode 100644 index 0000000000..3885e40bd2 --- /dev/null +++ b/connector/ldap/kerberos_test.go @@ -0,0 +1,452 @@ +package ldap + +import ( + "encoding/base64" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/dexidp/dex/connector" + ldaplib "github.com/go-ldap/ldap/v3" +) + +type mockKrbValidator struct { + principal string + realm string + ok bool + err error + challenged bool + contToken []byte + step int // -1 means disabled; 0->continue, then success +} + +func (m *mockKrbValidator) ValidateRequest(r *http.Request) (string, string, bool, error) { + if m.step >= 0 { + if m.step == 0 { + return "", "", false, nil + } + return m.principal, m.realm, true, nil + } + return m.principal, m.realm, m.ok, m.err +} + +func (m *mockKrbValidator) Challenge(w http.ResponseWriter) { + m.challenged = true + writeNegotiateChallenge(w) +} + +func (m *mockKrbValidator) ContinueToken(r *http.Request) ([]byte, bool) { + if m.step >= 0 { + if m.step == 0 && len(m.contToken) > 0 { + m.step++ + return m.contToken, true + } + return nil, false + } + if len(m.contToken) > 0 { + return m.contToken, true + } + return nil, false +} + +func TestKerberos_NoHeader_Returns401Negotiate(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} + lc.krbValidator = mv + 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 != 401 { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected Negotiate challenge") + } +} + +func TestKerberos_ExpectedRealmMismatch_401(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart", ExpectedRealm: "EXAMPLE.COM"}} + mv := &mockKrbValidator{principal: "jdoe@OTHER.COM", realm: "OTHER.COM", ok: true, err: nil, step: -1} + lc.krbValidator = mv + 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 != 401 { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected Negotiate challenge") + } +} + +func TestKerberos_FallbackToPassword_NoHeader_HandledFalse(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} + lc.krbValidator = mv + 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 not handled") + } + if ident != nil { + t.Fatalf("expected no identity") + } +} + +func TestKerberos_ContinueNeeded_SendsResponseToken(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, contToken: []byte{0x01, 0x02, 0x03}} + lc.krbValidator = mv + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate Zm9v") + 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 != 401 { + t.Fatalf("expected 401, got %d", w.Code) + } + hdr := w.Header().Get("WWW-Authenticate") + if !strings.HasPrefix(hdr, "Negotiate ") { + t.Fatalf("expected Negotiate header, got %q", hdr) + } + want := "Negotiate " + base64.StdEncoding.EncodeToString([]byte{0x01, 0x02, 0x03}) + if hdr != want { + t.Fatalf("unexpected negotiate header, got %q want %q", hdr, want) + } +} + +func TestKerberos_ContinueThenSuccess_ShortCircuitIdentity(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", contToken: []byte{0xAA, 0xBB}, step: 0} + lc.krbValidator = mv + + // First request -> 401 with continue token + r1 := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w1 := httptest.NewRecorder() + ident, handled, err := lc.TrySPNEGO(r1.Context(), connector.Scopes{}, w1, r1) + if err != nil || !bool(handled) || ident != nil || w1.Code != 401 { + t.Fatalf("unexpected first step result: ident=%v handled=%v code=%d err=%v", ident, handled, w1.Code, err) + } + hdr := w1.Header().Get("WWW-Authenticate") + if hdr == "" || !strings.HasPrefix(hdr, "Negotiate ") { + t.Fatalf("expected Negotiate header on first step, got %q", hdr) + } + + // Second request -> validator now returns ok=true (due to step increment) + krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, 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, true, nil + } + defer func() { krbLookupUserHook = nil }() + r2 := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w2 := httptest.NewRecorder() + ident2, handled2, err2 := lc.TrySPNEGO(r2.Context(), connector.Scopes{}, w2, r2) + if err2 != nil { + t.Fatalf("unexpected err: %v", err2) + } + if !bool(handled2) || ident2 == nil { + t.Fatalf("expected handled with identity on second step") + } +} + +func TestKerberos_ContinueNeeded_FallbackTrue_NotHandled(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{contToken: []byte{0x10}, step: 0} + lc.krbValidator = mv + 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 fallback=true when continue is needed") + } + if w.Code != 200 && w.Code != 0 { + t.Fatalf("expected no response written yet, got %d", w.Code) + } +} + +func TestKerberos_mapPrincipal(t *testing.T) { + cases := []struct{ in, realm, mode, want string }{ + {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "localpart", "jdoe"}, + {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "sAMAccountName", "jdoe"}, + {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "userPrincipalName", "jdoe@example.com"}, + } + for _, c := range cases { + got := mapPrincipal(c.in, c.realm, c.mode) + if got != c.want { + t.Fatalf("mapPrincipal(%q,%q,%q)=%q; want %q", c.in, c.realm, c.mode, got, c.want) + } + } +} + +func TestKerberos_UserNotFound_ReturnsError(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} + lc.krbValidator = mv + 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("expected error for user not found") + } + if !bool(handled) { + t.Fatalf("expected handled") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if !strings.Contains(err.Error(), "user lookup failed") { + t.Fatalf("expected 'user lookup failed' error, got: %v", err) + } +} + +func TestKerberos_ValidPrincipal_CompletesFlow(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} + lc.krbValidator = mv + krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, 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, true, nil + } + defer func() { krbLookupUserHook = 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 identity") + } + if ident.Username == "" || ident.Email == "" || ident.UserID == "" { + t.Fatalf("expected populated identity, got %+v", *ident) + } +} + +func TestKerberos_InvalidHeader_Returns401Negotiate(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} + lc.krbValidator = mv + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate !!!notbase64!!!") + 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 != 401 { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) + } + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected Negotiate challenge") + } +} + +func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "userPrincipalName"}} + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + mv := &mockKrbValidator{principal: "J.Doe@Example.COM", realm: "Example.COM", ok: true, err: nil, step: -1} + lc.krbValidator = mv + krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + if username != "j.doe@example.com" { + return ldaplib.Entry{}, false, nil + } + 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, true, nil + } + defer func() { krbLookupUserHook = 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 identity") + } +} + +func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} + lc.krbValidator = mv + krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, 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, true, nil + } + defer func() { krbLookupUserHook = nil }() + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + w := httptest.NewRecorder() + scopes := connector.Scopes{OfflineAccess: true} + ident, handled, err := lc.TrySPNEGO(r.Context(), 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 identity") + } + if len(ident.ConnectorData) == 0 { + t.Fatalf("expected connector data for offline access") + } +} + +func TestKerberos_FallbackTrue_InvalidHeader_NotHandled(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}} + mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} + lc.krbValidator = mv + r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) + r.Header.Set("Authorization", "Negotiate !!!") + 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 for fallback path") + } + if ident != nil { + t.Fatalf("expected no identity") + } + if w.Code != 200 && w.Code != 0 { + t.Fatalf("expected no response written yet, got %d", w.Code) + } +} + +func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "sAMAccountName"}} + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + mv := &mockKrbValidator{principal: "Admin@REALM.LOCAL", realm: "REALM.LOCAL", ok: true, err: nil, step: -1} + lc.krbValidator = mv + krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + if username != "admin" { + return ldaplib.Entry{}, false, nil + } + 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, true, nil + } + defer func() { krbLookupUserHook = 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 identity") + } +} + +func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { + lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart", ExpectedRealm: "ExAmPlE.CoM"}} + lc.Config.UserSearch.IDAttr = "uid" + lc.Config.UserSearch.EmailAttr = "mail" + lc.Config.UserSearch.NameAttr = "cn" + mv := &mockKrbValidator{principal: "user@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} + lc.krbValidator = mv + krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, 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, true, nil + } + defer func() { krbLookupUserHook = 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 identity") + } +} diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index e01e2df8c1..78e811d1bc 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,15 @@ type Config struct { } `json:"groupSearch"` } +// kerberosConfig defines optional Kerberos (SPNEGO) SSO settings for LDAP. +type kerberosConfig struct { + Enabled bool `json:"enabled"` + KeytabPath string `json:"keytabPath"` + ExpectedRealm string `json:"expectedRealm"` + UsernameFromPrincipal string `json:"usernameFromPrincipal"` + FallbackToPassword bool `json:"fallbackToPassword"` +} + func scopeString(i int) string { switch i { case ldap.ScopeBaseObject: @@ -241,6 +253,17 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro if err != nil { return nil, err } + // If Kerberos is enabled, initialize gokrb5 validator. + if lc, ok := conn.(*ldapConnector); ok && lc.krbEnabled && lc.krbValidator == nil { + v, verr := newGokrb5ValidatorWithLogger(lc.krbConf.KeytabPath, logger) + if verr != nil { + logger.Warn("failed to initialize kerberos validator; disabling kerberos", "err", verr) + lc.krbEnabled = false + } else { + lc.krbValidator = v + logger.Info("Kerberos enabled for LDAP connector", "keytab", lc.krbConf.KeytabPath, "expected_realm", lc.krbConf.ExpectedRealm) + } + } return connector.Connector(conn), nil } @@ -325,7 +348,37 @@ 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 + var krbEnabled bool + var krbConf kerberosConfig + if c.Kerberos != nil { + krbConf = *c.Kerberos + if krbConf.UsernameFromPrincipal == "" { + krbConf.UsernameFromPrincipal = "localpart" + } + if krbConf.Enabled { + if krbConf.KeytabPath == "" { + logger.Warn("kerberos enabled but keytabPath is empty; disabling kerberos") + } else { + krbEnabled = true + } + } + } + + lc := &ldapConnector{ + Config: *c, + userSearchScope: userSearchScope, + groupSearchScope: groupSearchScope, + tlsConfig: tlsConfig, + usernameAttrs: c.UserSearch.Username, + logger: logger, + } + if krbEnabled { + lc.krbEnabled = true + lc.krbConf = krbConf + } + return lc, nil } var ( @@ -344,6 +397,13 @@ type ldapConnector struct { usernameAttrs []string logger *slog.Logger + + // Kerberos/SPNEGO fields + krbEnabled bool + krbConf kerberosConfig + krbValidator KerberosValidator + // krbLookupUserHook allows tests to inject a user entry without LDAP queries. + krbLookupUserHook func(c *ldapConnector, username string) (ldap.Entry, bool, 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..1e4b358649 100644 --- a/examples/ldap/config-ldap.yaml +++ b/examples/ldap/config-ldap.yaml @@ -59,6 +59,15 @@ 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 + staticClients: - id: example-app redirectURIs: diff --git a/go.mod b/go.mod index e7150d13a6..ffcc29ee73 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/gofork v1.7.6 + 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/goidentity/v6 v6.0.1 // 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..978da119fb 100644 --- a/go.sum +++ b/go.sum @@ -129,6 +129,8 @@ 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/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= +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 +152,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 +261,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 +279,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 +321,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 +348,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 +377,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 +406,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..7a3185be7e 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -606,6 +606,43 @@ 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(r.Context(), 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(r.Context(), "SPNEGO authentication error", "err", err) + s.renderError(r, w, http.StatusUnauthorized, ErrMsgAuthenticationFailed) + return + } + if ident != nil { + redirectURL, canSkipApproval, err := s.finalizeLogin(r.Context(), *ident, authReq, conn.Connector) + if err != nil { + s.logger.ErrorContext(r.Context(), "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(r.Context(), "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 a Negotiate challenge was written; do not render form. + 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..16ec898dbf 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -1424,6 +1424,181 @@ 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 From e6d7d1c2983e1e594c56dbc284ec1e2faf27a228 Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Fri, 13 Mar 2026 16:03:06 +0300 Subject: [PATCH 2/7] fix lint issues Signed-off-by: Ivan Zvyagintsev --- connector/ldap/kerberos.go | 2 +- connector/ldap/kerberos_test.go | 3 ++- server/handlers_test.go | 10 ++++++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go index 7030bfb641..de0b1c25f8 100644 --- a/connector/ldap/kerberos.go +++ b/connector/ldap/kerberos.go @@ -12,6 +12,7 @@ import ( "os" "strings" + "github.com/go-ldap/ldap/v3" "github.com/jcmturner/gofork/encoding/asn1" "github.com/jcmturner/gokrb5/v8/credentials" "github.com/jcmturner/gokrb5/v8/gssapi" @@ -21,7 +22,6 @@ import ( "github.com/jcmturner/gokrb5/v8/types" "github.com/dexidp/dex/connector" - "github.com/go-ldap/ldap/v3" ) // KerberosValidator abstracts SPNEGO validation for unit-testing. diff --git a/connector/ldap/kerberos_test.go b/connector/ldap/kerberos_test.go index 3885e40bd2..930794d5f1 100644 --- a/connector/ldap/kerberos_test.go +++ b/connector/ldap/kerberos_test.go @@ -8,8 +8,9 @@ import ( "strings" "testing" - "github.com/dexidp/dex/connector" ldaplib "github.com/go-ldap/ldap/v3" + + "github.com/dexidp/dex/connector" ) type mockKrbValidator struct { diff --git a/server/handlers_test.go b/server/handlers_test.go index 16ec898dbf..124c635bc6 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -1499,11 +1499,14 @@ func TestHandlePasswordLogin_SPNEGOShortCircuit(t *testing.T) { // to simulate successful SPNEGO authentication on GET. type spnegoShortCircuit struct{ Identity connector.Identity } -func (s spnegoShortCircuit) Close() error { return nil } +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 @@ -1513,11 +1516,14 @@ func (s spnegoShortCircuit) TrySPNEGO(ctx context.Context, sc connector.Scopes, // 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) 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 } From fa24f20182ebf3633c4b7224f8560a98a6a86428 Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Tue, 17 Mar 2026 16:22:56 +0300 Subject: [PATCH 3/7] apply suggestions from review Signed-off-by: Ivan Zvyagintsev --- connector/ldap/kerberos.go | 173 ++++++++++++++------------------ connector/ldap/kerberos_test.go | 30 +++--- 2 files changed, 90 insertions(+), 113 deletions(-) diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go index de0b1c25f8..d0d87b3b38 100644 --- a/connector/ldap/kerberos.go +++ b/connector/ldap/kerberos.go @@ -42,28 +42,35 @@ func writeNegotiateChallenge(w http.ResponseWriter) { } // mapPrincipal maps a Kerberos principal to LDAP username per configuration. -func mapPrincipal(principal, realm, mapping string) string { - p := principal +// Supported mappings: +// - "localpart" / "samaccountname": extracts username before @ (default) +// - "userprincipalname": uses full principal as-is +func mapPrincipal(principal, mapping string) string { + // Extract localpart (before @) from principal + localpart := principal + if i := strings.IndexByte(principal, '@'); i >= 0 { + localpart = principal[:i] + } + switch strings.ToLower(mapping) { - case "localpart", "samaccountname": - if i := strings.IndexByte(principal, '@'); i >= 0 { - p = principal[:i] - } - return strings.ToLower(p) case "userprincipalname": return strings.ToLower(principal) + case "localpart", "samaccountname": + return strings.ToLower(localpart) default: - if i := strings.IndexByte(principal, '@'); i >= 0 { - p = principal[:i] - } - return strings.ToLower(p) + return strings.ToLower(localpart) } } // gokrb5 implementation of KerberosValidator -// context key used by gokrb5 to store credentials in the context -var ctxCredentialsKey interface{} = "github.com/jcmturner/gokrb5/v8/ctxCredentials" +// ctxCredentialsKeyType is the type for gokrb5 context key. +// gokrb5 uses a string constant as context key for storing credentials. +type ctxCredentialsKeyType string + +// ctxCredentialsKey is the context key used by gokrb5 to store credentials. +// This must match the exact string used in gokrb5/v8/spnego package. +const ctxCredentialsKey ctxCredentialsKeyType = "github.com/jcmturner/gokrb5/v8/ctxCredentials" // SPNEGO NegTokenResp (AcceptIncomplete + KRB5 mech) base64 payload used by gokrb5's HTTP server // to prompt the client to continue the handshake. @@ -75,50 +82,50 @@ type gokrb5Validator struct { } func newGokrb5ValidatorWithLogger(keytabPath string, logger *slog.Logger) (KerberosValidator, error) { + fi, err := os.Stat(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", keytabPath) + } kt, err := keytab.Load(keytabPath) if err != nil { return nil, fmt.Errorf("failed to load keytab: %w", err) } - if fi, err := os.Stat(keytabPath); err != nil || fi.IsDir() { - return nil, fmt.Errorf("invalid keytab path: %s", keytabPath) - } if logger == nil { logger = slog.Default() } return &gokrb5Validator{kt: kt, logger: logger}, nil } -func (v *gokrb5Validator) ValidateRequest(r *http.Request) (string, string, bool, error) { +// parseNegotiateHeader extracts and decodes the SPNEGO token from Authorization header. +// Returns (token, ok). If ok is false, the header is missing, malformed, or not Negotiate. +func (v *gokrb5Validator) parseNegotiateHeader(r *http.Request) (*spnego.SPNEGOToken, bool) { h := r.Header.Get("Authorization") if h == "" || !strings.HasPrefix(h, "Negotiate ") { - if v.logger != nil { - v.logger.Info("kerberos: missing or non-negotiate Authorization header", "path", r.URL.Path) - } - return "", "", false, nil + return nil, false } b64 := strings.TrimSpace(h[len("Negotiate "):]) if b64 == "" { - if v.logger != nil { - v.logger.Info("kerberos: empty negotiate token", "path", r.URL.Path) - } - return "", "", false, nil + return nil, false } data, err := base64.StdEncoding.DecodeString(b64) if err != nil { if v.logger != nil { v.logger.Info("kerberos: invalid base64 in Authorization", "err", err) } - return "", "", false, nil + return nil, false } var tok spnego.SPNEGOToken if err := tok.Unmarshal(data); err != nil { - // Try raw KRB5 token and wrap + // Try raw KRB5 token and wrap it as SPNEGO var k5 spnego.KRB5Token if k5.Unmarshal(data) != nil { if v.logger != nil { - v.logger.Info("kerberos: failed to unmarshal SPNEGO token and not raw KRB5", "err", err) + v.logger.Info("kerberos: failed to unmarshal SPNEGO/KRB5 token", "err", err) } - return "", "", false, nil + return nil, false } tok.Init = true tok.NegTokenInit = spnego.NegTokenInit{ @@ -126,18 +133,31 @@ func (v *gokrb5Validator) ValidateRequest(r *http.Request) (string, string, bool MechTokenBytes: data, } } + return &tok, true +} - // Pass client address when available (improves AP-REQ validation with address-bound tickets) - var sp *spnego.SPNEGO +// createSPNEGOService creates a SPNEGO service with optional client address binding. +func (v *gokrb5Validator) createSPNEGOService(r *http.Request) *spnego.SPNEGO { if ha, err := types.GetHostAddress(r.RemoteAddr); err == nil { - sp = spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) - } else { + return spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) + } + if v.logger != nil { + v.logger.Info("kerberos: cannot parse client address", "remote", r.RemoteAddr) + } + return spnego.SPNEGOService(v.kt, service.DecodePAC(false)) +} + +func (v *gokrb5Validator) ValidateRequest(r *http.Request) (string, string, bool, error) { + tok, ok := v.parseNegotiateHeader(r) + if !ok { if v.logger != nil { - v.logger.Info("kerberos: cannot parse client address", "remote", r.RemoteAddr, "err", err) + v.logger.Info("kerberos: missing or invalid Negotiate header", "path", r.URL.Path) } - sp = spnego.SPNEGOService(v.kt, service.DecodePAC(false)) + return "", "", false, nil } - authed, ctx, status := sp.AcceptSecContext(&tok) + + sp := v.createSPNEGOService(r) + authed, ctx, status := sp.AcceptSecContext(tok) if status.Code != gssapi.StatusComplete { if v.logger != nil { v.logger.Info("kerberos: AcceptSecContext not complete", "code", status.Code, "message", status.Message) @@ -165,74 +185,37 @@ func (v *gokrb5Validator) Challenge(w http.ResponseWriter) { writeNegotiateChall // ContinueToken attempts to continue the SPNEGO handshake and returns a response token // (to be placed into WWW-Authenticate: Negotiate ) if available. func (v *gokrb5Validator) ContinueToken(r *http.Request) ([]byte, bool) { - h := r.Header.Get("Authorization") - if h == "" || !strings.HasPrefix(h, "Negotiate ") { - if v.logger != nil { - v.logger.Info("kerberos: ContinueToken without negotiate header", "path", r.URL.Path) - } - return nil, false - } - b64 := strings.TrimSpace(h[len("Negotiate "):]) - data, err := base64.StdEncoding.DecodeString(b64) - if err != nil { - // Malformed header: ask client to continue with KRB5 mech - if tok, e := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64); e == nil { - if v.logger != nil { - v.logger.Info("kerberos: malformed negotiate token; sending incomplete KRB5 response") - } - return tok, true - } - return nil, false - } - var tok spnego.SPNEGOToken - if err := tok.Unmarshal(data); err != nil { - // Not a full SPNEGO token; still ask client to continue - if tokb, e := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64); e == nil { - if v.logger != nil { - v.logger.Info("kerberos: non-SPNEGO token; sending incomplete KRB5 response") - } - return tokb, true - } - // As a fallback, try wrapping as raw KRB5 - var k5 spnego.KRB5Token - if k5.Unmarshal(data) != nil { - if v.logger != nil { - v.logger.Info("kerberos: not KRB5 token; cannot continue") - } - return nil, false - } - tok.Init = true - tok.NegTokenInit = spnego.NegTokenInit{MechTypes: []asn1.ObjectIdentifier{k5.OID}, MechTokenBytes: data} - } - // Try continue with same options as in ValidateRequest - var sp *spnego.SPNEGO - if ha, err := types.GetHostAddress(r.RemoteAddr); err == nil { - sp = spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) - } else { - sp = spnego.SPNEGOService(v.kt, service.DecodePAC(false)) + tok, ok := v.parseNegotiateHeader(r) + if !ok { + // No valid token; return incomplete response to prompt client + return v.incompleteResponse() } - _, ctx, status := sp.AcceptSecContext(&tok) + + sp := v.createSPNEGOService(r) + _, ctx, status := sp.AcceptSecContext(tok) if status.Code != gssapi.StatusContinueNeeded || ctx == nil { if v.logger != nil { v.logger.Info("kerberos: no continuation required", "code", status.Code, "message", status.Message) } return nil, false } - // Ask client to continue using standard NegTokenResp (KRB5, incomplete) - if tokb, e := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64); e == nil { - if v.logger != nil { - v.logger.Info("kerberos: continuation needed; sending incomplete KRB5 response") - } - return tokb, true + return v.incompleteResponse() +} + +// incompleteResponse returns the standard NegTokenResp to prompt client continuation. +func (v *gokrb5Validator) incompleteResponse() ([]byte, bool) { + tokb, err := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64) + if err != nil { + return nil, false } - return nil, false + if v.logger != nil { + v.logger.Info("kerberos: sending incomplete KRB5 response for continuation") + } + return tokb, true } // LDAP connector SPNEGO integration -// krbLookupUserHook allows tests to inject a user entry without LDAP queries. -var krbLookupUserHook func(c *ldapConnector, username string) (ldap.Entry, bool, error) - // TrySPNEGO attempts Kerberos auth and builds identity on success. func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { if !c.krbEnabled || c.krbValidator == nil { @@ -271,13 +254,13 @@ func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w htt return nil, false, nil } - mapped := mapPrincipal(principal, realm, c.krbConf.UsernameFromPrincipal) + mapped := mapPrincipal(principal, c.krbConf.UsernameFromPrincipal) c.logger.Info("kerberos principal mapped", "principal", principal, "realm", realm, "mapped_username", mapped) var userEntry ldap.Entry // Allow test hook override - if krbLookupUserHook != nil { - if v, found, herr := krbLookupUserHook(c, mapped); found { + if c.krbLookupUserHook != nil { + if v, found, herr := c.krbLookupUserHook(c, mapped); found { if herr != nil { return nil, true, herr } diff --git a/connector/ldap/kerberos_test.go b/connector/ldap/kerberos_test.go index 930794d5f1..79358097d7 100644 --- a/connector/ldap/kerberos_test.go +++ b/connector/ldap/kerberos_test.go @@ -169,7 +169,7 @@ func TestKerberos_ContinueThenSuccess_ShortCircuitIdentity(t *testing.T) { } // Second request -> validator now returns ok=true (due to step increment) - krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ c.UserSearch.IDAttr: {"uid-jdoe"}, c.UserSearch.EmailAttr: {"jdoe@example.com"}, @@ -177,7 +177,6 @@ func TestKerberos_ContinueThenSuccess_ShortCircuitIdentity(t *testing.T) { }) return *e, true, nil } - defer func() { krbLookupUserHook = nil }() r2 := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) w2 := httptest.NewRecorder() ident2, handled2, err2 := lc.TrySPNEGO(r2.Context(), connector.Scopes{}, w2, r2) @@ -208,15 +207,15 @@ func TestKerberos_ContinueNeeded_FallbackTrue_NotHandled(t *testing.T) { } func TestKerberos_mapPrincipal(t *testing.T) { - cases := []struct{ in, realm, mode, want string }{ - {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "localpart", "jdoe"}, - {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "sAMAccountName", "jdoe"}, - {"JDoe@EXAMPLE.COM", "EXAMPLE.COM", "userPrincipalName", "jdoe@example.com"}, + cases := []struct{ in, mode, want string }{ + {"JDoe@EXAMPLE.COM", "localpart", "jdoe"}, + {"JDoe@EXAMPLE.COM", "sAMAccountName", "jdoe"}, + {"JDoe@EXAMPLE.COM", "userPrincipalName", "jdoe@example.com"}, } for _, c := range cases { - got := mapPrincipal(c.in, c.realm, c.mode) + got := mapPrincipal(c.in, c.mode) if got != c.want { - t.Fatalf("mapPrincipal(%q,%q,%q)=%q; want %q", c.in, c.realm, c.mode, got, c.want) + t.Fatalf("mapPrincipal(%q,%q)=%q; want %q", c.in, c.mode, got, c.want) } } } @@ -249,7 +248,7 @@ func TestKerberos_ValidPrincipal_CompletesFlow(t *testing.T) { lc.Config.UserSearch.NameAttr = "cn" mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} lc.krbValidator = mv - krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ c.UserSearch.IDAttr: {"uid-jdoe"}, c.UserSearch.EmailAttr: {"jdoe@example.com"}, @@ -257,7 +256,6 @@ func TestKerberos_ValidPrincipal_CompletesFlow(t *testing.T) { }) return *e, true, nil } - defer func() { krbLookupUserHook = 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) @@ -307,7 +305,7 @@ func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { lc.Config.UserSearch.NameAttr = "cn" mv := &mockKrbValidator{principal: "J.Doe@Example.COM", realm: "Example.COM", ok: true, err: nil, step: -1} lc.krbValidator = mv - krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { if username != "j.doe@example.com" { return ldaplib.Entry{}, false, nil } @@ -318,7 +316,6 @@ func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { }) return *e, true, nil } - defer func() { krbLookupUserHook = 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) @@ -340,7 +337,7 @@ func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { lc.Config.UserSearch.NameAttr = "cn" mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} lc.krbValidator = mv - krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { e := ldaplib.NewEntry("cn=jdoe,dc=example,dc=org", map[string][]string{ c.UserSearch.IDAttr: {"uid-jdoe"}, c.UserSearch.EmailAttr: {"jdoe@example.com"}, @@ -348,7 +345,6 @@ func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { }) return *e, true, nil } - defer func() { krbLookupUserHook = nil }() r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) w := httptest.NewRecorder() scopes := connector.Scopes{OfflineAccess: true} @@ -396,7 +392,7 @@ func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { lc.Config.UserSearch.NameAttr = "cn" mv := &mockKrbValidator{principal: "Admin@REALM.LOCAL", realm: "REALM.LOCAL", ok: true, err: nil, step: -1} lc.krbValidator = mv - krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { if username != "admin" { return ldaplib.Entry{}, false, nil } @@ -407,7 +403,6 @@ func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { }) return *e, true, nil } - defer func() { krbLookupUserHook = 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) @@ -429,7 +424,7 @@ func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { lc.Config.UserSearch.NameAttr = "cn" mv := &mockKrbValidator{principal: "user@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} lc.krbValidator = mv - krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { e := ldaplib.NewEntry("cn=user,dc=example,dc=com", map[string][]string{ c.UserSearch.IDAttr: {"uid-user"}, c.UserSearch.EmailAttr: {"user@example.com"}, @@ -437,7 +432,6 @@ func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { }) return *e, true, nil } - defer func() { krbLookupUserHook = 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) From 602cd5050083e596e30501bfc3fdac2082c5bf2f Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Tue, 21 Apr 2026 10:51:15 +0300 Subject: [PATCH 4/7] fix-kerberos Signed-off-by: Ivan Zvyagintsev --- connector/ldap/kerberos.go | 292 ++++++++++++++++---------------- connector/ldap/kerberos_test.go | 49 +++--- 2 files changed, 167 insertions(+), 174 deletions(-) diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go index d0d87b3b38..c0d9d3c3c0 100644 --- a/connector/ldap/kerberos.go +++ b/connector/ldap/kerberos.go @@ -24,29 +24,68 @@ import ( "github.com/dexidp/dex/connector" ) +// KerberosResult carries the outcome of a single SPNEGO validation attempt. +// +// Exactly one of the following states holds: +// - Authenticated: OK == true, Principal/Realm set. +// - Continuation needed: len(ContinueToken) > 0. +// - No/invalid header: OK == false, Err == nil, no ContinueToken. +// - Validation error: Err != nil. +type KerberosResult struct { + Principal string + Realm string + OK bool + Err error + ContinueToken []byte +} + // KerberosValidator abstracts SPNEGO validation for unit-testing. +// +// Implementations MUST call the underlying GSSAPI AcceptSecContext at most +// once per request to avoid replay detection (KRB_AP_ERR_REPEAT) on valid +// AP-REQ tokens. type KerberosValidator interface { - // ValidateRequest returns (principal, realm, ok, err). ok=false means header missing/invalid. - ValidateRequest(r *http.Request) (string, string, bool, error) - // Challenge writes a 401 Negotiate challenge. - Challenge(w http.ResponseWriter) - // ContinueToken tries to advance SPNEGO handshake and returns a response token to include - // in WWW-Authenticate: Negotiate . Returns (nil, false) if no continuation is needed/possible. - ContinueToken(r *http.Request) ([]byte, bool) + ValidateRequest(r *http.Request) KerberosResult } -// writeNegotiateChallenge writes a standard 401 Negotiate challenge. +// ctxCredentialsKey is the context key used by gokrb5 to store credentials. +// +// gokrb5/v8/spnego declares this as an untyped string const, which is passed +// into context.WithValue and therefore stored with concrete type `string`. +// To retrieve credentials the key MUST be a plain string (not a typed alias), +// otherwise context.Value returns nil because keys are compared by both type +// and value. +// +//nolint:revive,staticcheck // gokrb5 stores credentials under a plain string key; we must match it. +const ctxCredentialsKey = "github.com/jcmturner/gokrb5/v8/ctxCredentials" + +// spnegoIncompleteKRB5B64 is a pre-encoded SPNEGO NegTokenResp with +// AcceptIncomplete status and KRB5 mech, matching what gokrb5 sends when the +// handshake needs to continue. Used as the response body for the second step +// of a multi-leg SPNEGO exchange only (never as the initial challenge). +const spnegoIncompleteKRB5B64 = "oRQwEqADCgEBoQsGCSqGSIb3EgECAg==" + +// writeNegotiateChallenge writes a bare RFC 4559 Negotiate challenge. +// This is the correct response when the client has not yet sent an +// Authorization: Negotiate header. func writeNegotiateChallenge(w http.ResponseWriter) { w.Header().Set("WWW-Authenticate", "Negotiate") w.WriteHeader(http.StatusUnauthorized) } +// writeNegotiateContinuation writes a 401 with a SPNEGO continuation token. +// Used only when the client already initiated SPNEGO and gokrb5 reported +// StatusContinueNeeded. +func writeNegotiateContinuation(w http.ResponseWriter, token []byte) { + w.Header().Set("WWW-Authenticate", "Negotiate "+base64.StdEncoding.EncodeToString(token)) + w.WriteHeader(http.StatusUnauthorized) +} + // mapPrincipal maps a Kerberos principal to LDAP username per configuration. // Supported mappings: // - "localpart" / "samaccountname": extracts username before @ (default) // - "userprincipalname": uses full principal as-is func mapPrincipal(principal, mapping string) string { - // Extract localpart (before @) from principal localpart := principal if i := strings.IndexByte(principal, '@'); i >= 0 { localpart = principal[:i] @@ -62,20 +101,7 @@ func mapPrincipal(principal, mapping string) string { } } -// gokrb5 implementation of KerberosValidator - -// ctxCredentialsKeyType is the type for gokrb5 context key. -// gokrb5 uses a string constant as context key for storing credentials. -type ctxCredentialsKeyType string - -// ctxCredentialsKey is the context key used by gokrb5 to store credentials. -// This must match the exact string used in gokrb5/v8/spnego package. -const ctxCredentialsKey ctxCredentialsKeyType = "github.com/jcmturner/gokrb5/v8/ctxCredentials" - -// SPNEGO NegTokenResp (AcceptIncomplete + KRB5 mech) base64 payload used by gokrb5's HTTP server -// to prompt the client to continue the handshake. -const spnegoIncompleteKRB5B64 = "oRQwEqADCgEBoQsGCSqGSIb3EgECAg==" - +// gokrb5Validator is the default KerberosValidator backed by jcmturner/gokrb5. type gokrb5Validator struct { kt *keytab.Keytab logger *slog.Logger @@ -99,8 +125,9 @@ func newGokrb5ValidatorWithLogger(keytabPath string, logger *slog.Logger) (Kerbe return &gokrb5Validator{kt: kt, logger: logger}, nil } -// parseNegotiateHeader extracts and decodes the SPNEGO token from Authorization header. -// Returns (token, ok). If ok is false, the header is missing, malformed, or not Negotiate. +// parseNegotiateHeader extracts and decodes the SPNEGO token from the +// Authorization header. Returns (token, true) on success, (nil, false) when +// the header is absent, malformed or not a valid SPNEGO/KRB5 token. func (v *gokrb5Validator) parseNegotiateHeader(r *http.Request) (*spnego.SPNEGOToken, bool) { h := r.Header.Get("Authorization") if h == "" || !strings.HasPrefix(h, "Negotiate ") { @@ -112,19 +139,15 @@ func (v *gokrb5Validator) parseNegotiateHeader(r *http.Request) (*spnego.SPNEGOT } data, err := base64.StdEncoding.DecodeString(b64) if err != nil { - if v.logger != nil { - v.logger.Info("kerberos: invalid base64 in Authorization", "err", err) - } + v.logger.Info("kerberos: invalid base64 in Authorization", "err", err) return nil, false } var tok spnego.SPNEGOToken if err := tok.Unmarshal(data); err != nil { - // Try raw KRB5 token and wrap it as SPNEGO + // Fall back to raw KRB5 token, wrap as SPNEGO NegTokenInit. var k5 spnego.KRB5Token if k5.Unmarshal(data) != nil { - if v.logger != nil { - v.logger.Info("kerberos: failed to unmarshal SPNEGO/KRB5 token", "err", err) - } + v.logger.Info("kerberos: failed to unmarshal SPNEGO/KRB5 token", "err", err) return nil, false } tok.Init = true @@ -136,155 +159,108 @@ func (v *gokrb5Validator) parseNegotiateHeader(r *http.Request) (*spnego.SPNEGOT return &tok, true } -// createSPNEGOService creates a SPNEGO service with optional client address binding. +// createSPNEGOService creates a SPNEGO service, binding the client address +// when it can be parsed from RemoteAddr (required for address-bound tickets). func (v *gokrb5Validator) createSPNEGOService(r *http.Request) *spnego.SPNEGO { if ha, err := types.GetHostAddress(r.RemoteAddr); err == nil { return spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) } - if v.logger != nil { - v.logger.Info("kerberos: cannot parse client address", "remote", r.RemoteAddr) - } + v.logger.Info("kerberos: cannot parse client address", "remote", r.RemoteAddr) return spnego.SPNEGOService(v.kt, service.DecodePAC(false)) } -func (v *gokrb5Validator) ValidateRequest(r *http.Request) (string, string, bool, error) { +// ValidateRequest performs a single AcceptSecContext call and translates the +// GSSAPI status into a KerberosResult. It never retries the same AP-REQ. +func (v *gokrb5Validator) ValidateRequest(r *http.Request) KerberosResult { tok, ok := v.parseNegotiateHeader(r) if !ok { - if v.logger != nil { - v.logger.Info("kerberos: missing or invalid Negotiate header", "path", r.URL.Path) - } - return "", "", false, nil + v.logger.Info("kerberos: missing or invalid Negotiate header", "path", r.URL.Path) + return KerberosResult{} } sp := v.createSPNEGOService(r) authed, ctx, status := sp.AcceptSecContext(tok) - if status.Code != gssapi.StatusComplete { - if v.logger != nil { - v.logger.Info("kerberos: AcceptSecContext not complete", "code", status.Code, "message", status.Message) + + switch status.Code { + case gssapi.StatusComplete: + // Handled below. + case gssapi.StatusContinueNeeded: + v.logger.Info("kerberos: continuation needed", "message", status.Message) + tokb, err := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64) + if err != nil { + return KerberosResult{Err: fmt.Errorf("decode continuation token: %w", err)} } - return "", "", false, nil + return KerberosResult{ContinueToken: tokb} + default: + v.logger.Info("kerberos: AcceptSecContext rejected", "code", status.Code, "message", status.Message) + return KerberosResult{} } + if !authed || ctx == nil { - if v.logger != nil { - v.logger.Info("kerberos: not authenticated or no context") - } - return "", "", false, nil + v.logger.Info("kerberos: not authenticated or no context") + return KerberosResult{} } + id, _ := ctx.Value(ctxCredentialsKey).(*credentials.Credentials) if id == nil { - if v.logger != nil { - v.logger.Info("kerberos: credentials missing in context") - } - return "", "", false, fmt.Errorf("no credentials in context") + v.logger.Info("kerberos: credentials missing in context") + return KerberosResult{Err: fmt.Errorf("no credentials in context")} } - return id.UserName(), id.Domain(), true, nil -} - -func (v *gokrb5Validator) Challenge(w http.ResponseWriter) { writeNegotiateChallenge(w) } - -// ContinueToken attempts to continue the SPNEGO handshake and returns a response token -// (to be placed into WWW-Authenticate: Negotiate ) if available. -func (v *gokrb5Validator) ContinueToken(r *http.Request) ([]byte, bool) { - tok, ok := v.parseNegotiateHeader(r) - if !ok { - // No valid token; return incomplete response to prompt client - return v.incompleteResponse() - } - - sp := v.createSPNEGOService(r) - _, ctx, status := sp.AcceptSecContext(tok) - if status.Code != gssapi.StatusContinueNeeded || ctx == nil { - if v.logger != nil { - v.logger.Info("kerberos: no continuation required", "code", status.Code, "message", status.Message) - } - return nil, false - } - return v.incompleteResponse() -} - -// incompleteResponse returns the standard NegTokenResp to prompt client continuation. -func (v *gokrb5Validator) incompleteResponse() ([]byte, bool) { - tokb, err := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64) - if err != nil { - return nil, false - } - if v.logger != nil { - v.logger.Info("kerberos: sending incomplete KRB5 response for continuation") - } - return tokb, true + return KerberosResult{Principal: id.UserName(), Realm: id.Domain(), OK: true} } // LDAP connector SPNEGO integration -// TrySPNEGO attempts Kerberos auth and builds identity on success. +// TrySPNEGO attempts Kerberos authentication on the current request. Returns +// handled=true when the response has been fully written (either a challenge, +// a continuation token or a terminal error) and the caller MUST NOT render +// the password form. handled=false means the caller should continue with the +// default login flow. func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { if !c.krbEnabled || c.krbValidator == nil { return nil, false, nil } - principal, realm, ok, err := c.krbValidator.ValidateRequest(r) - if err != nil || !ok { - if !c.krbConf.FallbackToPassword { - // Try to get a continuation token to advance SPNEGO handshake - if tok, ok2 := c.krbValidator.ContinueToken(r); ok2 && len(tok) > 0 { - c.logger.Info("kerberos SPNEGO continuation required; sending response token") - w.Header().Set("WWW-Authenticate", "Negotiate "+base64.StdEncoding.EncodeToString(tok)) - w.WriteHeader(http.StatusUnauthorized) - return nil, true, nil - } - if err != nil { - c.logger.Info("kerberos SPNEGO validation error; sending Negotiate challenge", "err", err) - } else { - c.logger.Info("kerberos SPNEGO not completed or header missing; sending Negotiate challenge") - } - c.krbValidator.Challenge(w) - return nil, true, nil + res := c.krbValidator.ValidateRequest(r) + + if !res.OK { + if c.krbConf.FallbackToPassword { + c.logger.Info("kerberos SPNEGO not completed; falling back to password form") + return nil, false, nil } - c.logger.Info("kerberos SPNEGO fallback to password enabled; rendering login form") - return nil, false, nil - } - if c.krbConf.ExpectedRealm != "" && !strings.EqualFold(c.krbConf.ExpectedRealm, realm) { - c.logger.Info("kerberos realm mismatch", "expected", c.krbConf.ExpectedRealm, "actual", realm) - if !c.krbConf.FallbackToPassword { - c.krbValidator.Challenge(w) - return nil, true, nil + switch { + case len(res.ContinueToken) > 0: + c.logger.Info("kerberos SPNEGO continuation required; sending response token") + writeNegotiateContinuation(w, res.ContinueToken) + case res.Err != nil: + c.logger.Info("kerberos SPNEGO validation error; sending Negotiate challenge", "err", res.Err) + writeNegotiateChallenge(w) + default: + c.logger.Info("kerberos SPNEGO header missing or invalid; sending Negotiate challenge") + writeNegotiateChallenge(w) } - c.logger.Info("kerberos realm mismatch but fallback enabled; rendering login form") - return nil, false, nil + return nil, true, nil } - mapped := mapPrincipal(principal, c.krbConf.UsernameFromPrincipal) - c.logger.Info("kerberos principal mapped", "principal", principal, "realm", realm, "mapped_username", mapped) - - var userEntry ldap.Entry - // Allow test hook override - if c.krbLookupUserHook != nil { - if v, found, herr := c.krbLookupUserHook(c, mapped); found { - if herr != nil { - return nil, true, herr - } - userEntry = v + if c.krbConf.ExpectedRealm != "" && !strings.EqualFold(c.krbConf.ExpectedRealm, res.Realm) { + c.logger.Info("kerberos realm mismatch", "expected", c.krbConf.ExpectedRealm, "actual", res.Realm) + if c.krbConf.FallbackToPassword { + c.logger.Info("kerberos realm mismatch but fallback enabled; rendering login form") + return nil, false, nil } + writeNegotiateChallenge(w) + return nil, true, nil } - if userEntry.DN == "" { - // Reuse existing search logic via do() and userEntry - err = c.do(ctx, func(conn *ldap.Conn) error { - entry, found, err := c.userEntry(conn, mapped) - if err != nil { - return err - } - if !found { - return fmt.Errorf("user not found for principal") - } - userEntry = entry - return nil - }) - } + mapped := mapPrincipal(res.Principal, c.krbConf.UsernameFromPrincipal) + c.logger.Info("kerberos principal mapped", + "principal", res.Principal, "realm", res.Realm, "mapped_username", mapped) + + userEntry, err := c.lookupKerberosUser(ctx, mapped) if err != nil { - c.logger.Error("kerberos user lookup failed", "principal", principal, "mapped", mapped, "err", err) - return nil, true, fmt.Errorf("ldap: user lookup failed for kerberos principal %q: %v", principal, err) + c.logger.Error("kerberos user lookup failed", "principal", res.Principal, "mapped", mapped, "err", err) + return nil, true, fmt.Errorf("ldap: user lookup failed for kerberos principal %q: %v", res.Principal, err) } c.logger.Info("kerberos user lookup succeeded", "dn", userEntry.DN) @@ -302,7 +278,8 @@ func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w htt ident.Groups = groups } - // No password -> no user bind; do not set ConnectorData unless OfflineAccess requested + // No password means no user bind happened; only populate ConnectorData when + // OfflineAccess was requested so refresh can query groups later. if s.OfflineAccess { refresh := refreshData{Username: mapped, Entry: userEntry} if data, mErr := json.Marshal(refresh); mErr == nil { @@ -310,6 +287,31 @@ func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w htt } } - c.logger.Info("kerberos SPNEGO authentication succeeded", "username", ident.Username, "email", ident.Email, "groups_count", len(ident.Groups)) + c.logger.Info("kerberos SPNEGO authentication succeeded", + "username", ident.Username, "email", ident.Email, "groups_count", len(ident.Groups)) return &ident, true, nil } + +// lookupKerberosUser resolves an LDAP user entry by username. Tests may +// override the lookup via krbLookupUserHook. +func (c *ldapConnector) lookupKerberosUser(ctx context.Context, username string) (ldap.Entry, error) { + if c.krbLookupUserHook != nil { + if entry, found, err := c.krbLookupUserHook(c, username); found { + return entry, err + } + } + + 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 index 79358097d7..95b0e32b5f 100644 --- a/connector/ldap/kerberos_test.go +++ b/connector/ldap/kerberos_test.go @@ -13,43 +13,34 @@ import ( "github.com/dexidp/dex/connector" ) +// mockKrbValidator is a deterministic KerberosValidator for unit tests. +// +// Two modes: +// - step < 0: single-shot — returns principal/realm/ok/err as configured. +// - step >= 0: multi-step SPNEGO handshake — first call returns +// ContinueToken (when set) or bare challenge, subsequent calls return +// principal/realm with OK=true. type mockKrbValidator struct { - principal string - realm string - ok bool - err error - challenged bool - contToken []byte - step int // -1 means disabled; 0->continue, then success + principal string + realm string + ok bool + err error + contToken []byte + step int } -func (m *mockKrbValidator) ValidateRequest(r *http.Request) (string, string, bool, error) { +func (m *mockKrbValidator) ValidateRequest(r *http.Request) KerberosResult { if m.step >= 0 { if m.step == 0 { - return "", "", false, nil - } - return m.principal, m.realm, true, nil - } - return m.principal, m.realm, m.ok, m.err -} - -func (m *mockKrbValidator) Challenge(w http.ResponseWriter) { - m.challenged = true - writeNegotiateChallenge(w) -} - -func (m *mockKrbValidator) ContinueToken(r *http.Request) ([]byte, bool) { - if m.step >= 0 { - if m.step == 0 && len(m.contToken) > 0 { m.step++ - return m.contToken, true + if len(m.contToken) > 0 { + return KerberosResult{ContinueToken: m.contToken} + } + return KerberosResult{} } - return nil, false - } - if len(m.contToken) > 0 { - return m.contToken, true + return KerberosResult{Principal: m.principal, Realm: m.realm, OK: true} } - return nil, false + return KerberosResult{Principal: m.principal, Realm: m.realm, OK: m.ok, Err: m.err} } func TestKerberos_NoHeader_Returns401Negotiate(t *testing.T) { From ecf6d330532f745c0ceb4418f9054733c498bed7 Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Tue, 21 Apr 2026 11:37:01 +0300 Subject: [PATCH 5/7] fix-kerberos Signed-off-by: Ivan Zvyagintsev --- go.sum | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go.sum b/go.sum index 978da119fb..6da4f92dad 100644 --- a/go.sum +++ b/go.sum @@ -129,7 +129,9 @@ 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= From 62aeaceae67fb062d00bc6a029509ee4b9158fe9 Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Thu, 23 Apr 2026 15:16:11 +0300 Subject: [PATCH 6/7] refactor kerberos after review Signed-off-by: Ivan Zvyagintsev --- connector/ldap/kerberos.go | 357 +++++++++------------ connector/ldap/kerberos_test.go | 537 ++++++++++++++++++-------------- connector/ldap/ldap.go | 55 +++- go.mod | 4 +- server/handlers.go | 14 +- 5 files changed, 496 insertions(+), 471 deletions(-) diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go index c0d9d3c3c0..c81dbcf7c6 100644 --- a/connector/ldap/kerberos.go +++ b/connector/ldap/kerberos.go @@ -4,301 +4,232 @@ package ldap import ( "context" - "encoding/base64" "encoding/json" "fmt" - "log/slog" "net/http" + "net/http/httptest" "os" "strings" + "time" "github.com/go-ldap/ldap/v3" - "github.com/jcmturner/gofork/encoding/asn1" + "github.com/jcmturner/goidentity/v6" "github.com/jcmturner/gokrb5/v8/credentials" - "github.com/jcmturner/gokrb5/v8/gssapi" "github.com/jcmturner/gokrb5/v8/keytab" "github.com/jcmturner/gokrb5/v8/service" "github.com/jcmturner/gokrb5/v8/spnego" - "github.com/jcmturner/gokrb5/v8/types" "github.com/dexidp/dex/connector" ) -// KerberosResult carries the outcome of a single SPNEGO validation attempt. +// krbState holds Kerberos/SPNEGO configuration bound to a ldapConnector. // -// Exactly one of the following states holds: -// - Authenticated: OK == true, Principal/Realm set. -// - Continuation needed: len(ContinueToken) > 0. -// - No/invalid header: OK == false, Err == nil, no ContinueToken. -// - Validation error: Err != nil. -type KerberosResult struct { - Principal string - Realm string - OK bool - Err error - ContinueToken []byte +// 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 } -// KerberosValidator abstracts SPNEGO validation for unit-testing. -// -// Implementations MUST call the underlying GSSAPI AcceptSecContext at most -// once per request to avoid replay detection (KRB_AP_ERR_REPEAT) on valid -// AP-REQ tokens. -type KerberosValidator interface { - ValidateRequest(r *http.Request) KerberosResult -} - -// ctxCredentialsKey is the context key used by gokrb5 to store credentials. -// -// gokrb5/v8/spnego declares this as an untyped string const, which is passed -// into context.WithValue and therefore stored with concrete type `string`. -// To retrieve credentials the key MUST be a plain string (not a typed alias), -// otherwise context.Value returns nil because keys are compared by both type -// and value. -// -//nolint:revive,staticcheck // gokrb5 stores credentials under a plain string key; we must match it. -const ctxCredentialsKey = "github.com/jcmturner/gokrb5/v8/ctxCredentials" - -// spnegoIncompleteKRB5B64 is a pre-encoded SPNEGO NegTokenResp with -// AcceptIncomplete status and KRB5 mech, matching what gokrb5 sends when the -// handshake needs to continue. Used as the response body for the second step -// of a multi-leg SPNEGO exchange only (never as the initial challenge). -const spnegoIncompleteKRB5B64 = "oRQwEqADCgEBoQsGCSqGSIb3EgECAg==" - -// writeNegotiateChallenge writes a bare RFC 4559 Negotiate challenge. -// This is the correct response when the client has not yet sent an -// Authorization: Negotiate header. -func writeNegotiateChallenge(w http.ResponseWriter) { - w.Header().Set("WWW-Authenticate", "Negotiate") - w.WriteHeader(http.StatusUnauthorized) -} - -// writeNegotiateContinuation writes a 401 with a SPNEGO continuation token. -// Used only when the client already initiated SPNEGO and gokrb5 reported -// StatusContinueNeeded. -func writeNegotiateContinuation(w http.ResponseWriter, token []byte) { - w.Header().Set("WWW-Authenticate", "Negotiate "+base64.StdEncoding.EncodeToString(token)) - w.WriteHeader(http.StatusUnauthorized) -} - -// mapPrincipal maps a Kerberos principal to LDAP username per configuration. -// Supported mappings: -// - "localpart" / "samaccountname": extracts username before @ (default) -// - "userprincipalname": uses full principal as-is -func mapPrincipal(principal, mapping string) string { - localpart := principal - if i := strings.IndexByte(principal, '@'); i >= 0 { - localpart = principal[:i] - } - - switch strings.ToLower(mapping) { - case "userprincipalname": - return strings.ToLower(principal) - case "localpart", "samaccountname": - return strings.ToLower(localpart) - default: - return strings.ToLower(localpart) - } -} - -// gokrb5Validator is the default KerberosValidator backed by jcmturner/gokrb5. -type gokrb5Validator struct { - kt *keytab.Keytab - logger *slog.Logger -} - -func newGokrb5ValidatorWithLogger(keytabPath string, logger *slog.Logger) (KerberosValidator, error) { - fi, err := os.Stat(keytabPath) +// 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", keytabPath) + return nil, fmt.Errorf("keytab path is a directory: %s", cfg.KeytabPath) } - kt, err := keytab.Load(keytabPath) + kt, err := keytab.Load(cfg.KeytabPath) if err != nil { return nil, fmt.Errorf("failed to load keytab: %w", err) } - if logger == nil { - logger = slog.Default() - } - return &gokrb5Validator{kt: kt, logger: logger}, nil -} -// parseNegotiateHeader extracts and decodes the SPNEGO token from the -// Authorization header. Returns (token, true) on success, (nil, false) when -// the header is absent, malformed or not a valid SPNEGO/KRB5 token. -func (v *gokrb5Validator) parseNegotiateHeader(r *http.Request) (*spnego.SPNEGOToken, bool) { - h := r.Header.Get("Authorization") - if h == "" || !strings.HasPrefix(h, "Negotiate ") { - return nil, false + settings := []func(*service.Settings){service.DecodePAC(false)} + if cfg.SPN != "" { + settings = append(settings, service.SName(cfg.SPN)) } - b64 := strings.TrimSpace(h[len("Negotiate "):]) - if b64 == "" { - return nil, false + if cfg.KeytabPrincipal != "" { + settings = append(settings, service.KeytabPrincipal(cfg.KeytabPrincipal)) } - data, err := base64.StdEncoding.DecodeString(b64) - if err != nil { - v.logger.Info("kerberos: invalid base64 in Authorization", "err", err) - return nil, false + if cfg.MaxClockSkew > 0 { + settings = append(settings, service.MaxClockSkew(time.Duration(cfg.MaxClockSkew)*time.Second)) } - var tok spnego.SPNEGOToken - if err := tok.Unmarshal(data); err != nil { - // Fall back to raw KRB5 token, wrap as SPNEGO NegTokenInit. - var k5 spnego.KRB5Token - if k5.Unmarshal(data) != nil { - v.logger.Info("kerberos: failed to unmarshal SPNEGO/KRB5 token", "err", err) - return nil, false - } - tok.Init = true - tok.NegTokenInit = spnego.NegTokenInit{ - MechTypes: []asn1.ObjectIdentifier{k5.OID}, - MechTokenBytes: data, - } - } - return &tok, true -} -// createSPNEGOService creates a SPNEGO service, binding the client address -// when it can be parsed from RemoteAddr (required for address-bound tickets). -func (v *gokrb5Validator) createSPNEGOService(r *http.Request) *spnego.SPNEGO { - if ha, err := types.GetHostAddress(r.RemoteAddr); err == nil { - return spnego.SPNEGOService(v.kt, service.ClientAddress(ha), service.DecodePAC(false)) - } - v.logger.Info("kerberos: cannot parse client address", "remote", r.RemoteAddr) - return spnego.SPNEGOService(v.kt, service.DecodePAC(false)) + return &krbState{ + authenticate: func(inner http.Handler) http.Handler { + return spnego.SPNEGOKRB5Authenticate(inner, kt, settings...) + }, + }, nil } -// ValidateRequest performs a single AcceptSecContext call and translates the -// GSSAPI status into a KerberosResult. It never retries the same AP-REQ. -func (v *gokrb5Validator) ValidateRequest(r *http.Request) KerberosResult { - tok, ok := v.parseNegotiateHeader(r) - if !ok { - v.logger.Info("kerberos: missing or invalid Negotiate header", "path", r.URL.Path) - return KerberosResult{} - } - - sp := v.createSPNEGOService(r) - authed, ctx, status := sp.AcceptSecContext(tok) - - switch status.Code { - case gssapi.StatusComplete: - // Handled below. - case gssapi.StatusContinueNeeded: - v.logger.Info("kerberos: continuation needed", "message", status.Message) - tokb, err := base64.StdEncoding.DecodeString(spnegoIncompleteKRB5B64) - if err != nil { - return KerberosResult{Err: fmt.Errorf("decode continuation token: %w", err)} +// 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 KerberosResult{ContinueToken: tokb} - default: - v.logger.Info("kerberos: AcceptSecContext rejected", "code", status.Code, "message", status.Message) - return KerberosResult{} + return username + "@" + realm } + return username +} - if !authed || ctx == nil { - v.logger.Info("kerberos: not authenticated or no context") - return KerberosResult{} +// 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 and the request carries no Negotiate header: +// short-circuits to the password form without invoking the SPNEGO +// middleware. This avoids writing an unwanted 401 when a password is +// explicitly permitted. +// - Otherwise, runs the SPNEGO middleware into a response recorder. If the +// middleware does not authenticate, its buffered response is either +// forwarded to the client (FallbackToPassword=false) or discarded +// (FallbackToPassword=true, caller renders form). +// - On success, the authenticated principal is resolved in LDAP and a +// connector.Identity is returned. +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 } - id, _ := ctx.Value(ctxCredentialsKey).(*credentials.Credentials) - if id == nil { - v.logger.Info("kerberos: credentials missing in context") - return KerberosResult{Err: fmt.Errorf("no credentials in context")} + if c.krbConf.FallbackToPassword && + !strings.HasPrefix(r.Header.Get("Authorization"), "Negotiate ") { + return nil, false, nil } - return KerberosResult{Principal: id.UserName(), Realm: id.Domain(), OK: true} -} -// LDAP connector SPNEGO integration + // 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 + }) -// TrySPNEGO attempts Kerberos authentication on the current request. Returns -// handled=true when the response has been fully written (either a challenge, -// a continuation token or a terminal error) and the caller MUST NOT render -// the password form. handled=false means the caller should continue with the -// default login flow. -func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w http.ResponseWriter, r *http.Request) (*connector.Identity, connector.Handled, error) { - if !c.krbEnabled || c.krbValidator == nil { - return nil, false, nil - } + rec := httptest.NewRecorder() + c.krb.authenticate(inner).ServeHTTP(rec, r) - res := c.krbValidator.ValidateRequest(r) + if innerErr != nil { + c.logger.Error("kerberos: SPNEGO middleware completed with unusable credentials", "err", innerErr) + return nil, true, innerErr + } - if !res.OK { + if id == nil { if c.krbConf.FallbackToPassword { - c.logger.Info("kerberos SPNEGO not completed; falling back to password form") + c.logger.Info("kerberos: SPNEGO did not authenticate; falling back to password form") return nil, false, nil } - - switch { - case len(res.ContinueToken) > 0: - c.logger.Info("kerberos SPNEGO continuation required; sending response token") - writeNegotiateContinuation(w, res.ContinueToken) - case res.Err != nil: - c.logger.Info("kerberos SPNEGO validation error; sending Negotiate challenge", "err", res.Err) - writeNegotiateChallenge(w) - default: - c.logger.Info("kerberos SPNEGO header missing or invalid; sending Negotiate challenge") - writeNegotiateChallenge(w) - } + c.logger.Info("kerberos: SPNEGO did not authenticate; forwarding middleware response", "status", rec.Code) + copyRecorded(rec, w) return nil, true, nil } - if c.krbConf.ExpectedRealm != "" && !strings.EqualFold(c.krbConf.ExpectedRealm, res.Realm) { - c.logger.Info("kerberos realm mismatch", "expected", c.krbConf.ExpectedRealm, "actual", res.Realm) + 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 { - c.logger.Info("kerberos realm mismatch but fallback enabled; rendering login form") return nil, false, nil } - writeNegotiateChallenge(w) + w.Header().Set("WWW-Authenticate", "Negotiate") + w.WriteHeader(http.StatusUnauthorized) return nil, true, nil } - mapped := mapPrincipal(res.Principal, c.krbConf.UsernameFromPrincipal) - c.logger.Info("kerberos principal mapped", - "principal", res.Principal, "realm", res.Realm, "mapped_username", mapped) + 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 user lookup failed", "principal", res.Principal, "mapped", mapped, "err", err) - return nil, true, fmt.Errorf("ldap: user lookup failed for kerberos principal %q: %v", res.Principal, err) + 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 user lookup succeeded", "dn", userEntry.DN) + c.logger.Info("kerberos: LDAP user found", "dn", userEntry.DN) ident, err := c.identityFromEntry(userEntry) if err != nil { - c.logger.Info("failed to build identity from LDAP entry after kerberos SPNEGO", "err", err) + 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.Info("failed to query groups after kerberos SPNEGO", "err", err) - return nil, true, fmt.Errorf("ldap: failed to query groups: %v", err) + 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 password means no user bind happened; only populate ConnectorData when - // OfflineAccess was requested so refresh can query groups later. + // 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} - if data, mErr := json.Marshal(refresh); mErr == nil { - ident.ConnectorData = data + 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 } - c.logger.Info("kerberos SPNEGO authentication succeeded", + c.logger.Info("kerberos: SPNEGO login succeeded", "username", ident.Username, "email", ident.Email, "groups_count", len(ident.Groups)) return &ident, true, nil } -// lookupKerberosUser resolves an LDAP user entry by username. Tests may -// override the lookup via krbLookupUserHook. +// copyRecorded forwards the buffered middleware response to the client. +func copyRecorded(rec *httptest.ResponseRecorder, 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()) +} + +// 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 { - if entry, found, err := c.krbLookupUserHook(c, username); found { - return entry, err - } + return c.krbLookupUserHook(ctx, c, username) } var userEntry ldap.Entry diff --git a/connector/ldap/kerberos_test.go b/connector/ldap/kerberos_test.go index 95b0e32b5f..c602990538 100644 --- a/connector/ldap/kerberos_test.go +++ b/connector/ldap/kerberos_test.go @@ -1,7 +1,9 @@ package ldap import ( - "encoding/base64" + "context" + "errors" + "fmt" "log/slog" "net/http" "net/http/httptest" @@ -9,44 +11,63 @@ import ( "testing" ldaplib "github.com/go-ldap/ldap/v3" + "github.com/jcmturner/goidentity/v6" + "github.com/jcmturner/gokrb5/v8/credentials" "github.com/dexidp/dex/connector" ) -// mockKrbValidator is a deterministic KerberosValidator for unit tests. -// -// Two modes: -// - step < 0: single-shot — returns principal/realm/ok/err as configured. -// - step >= 0: multi-step SPNEGO handshake — first call returns -// ContinueToken (when set) or bare challenge, subsequent calls return -// principal/realm with OK=true. -type mockKrbValidator struct { - principal string - realm string - ok bool - err error - contToken []byte - step int +// 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) + }) + }, + } } -func (m *mockKrbValidator) ValidateRequest(r *http.Request) KerberosResult { - if m.step >= 0 { - if m.step == 0 { - m.step++ - if len(m.contToken) > 0 { - return KerberosResult{ContinueToken: m.contToken} - } - return KerberosResult{} - } - return KerberosResult{Principal: m.principal, Realm: m.realm, OK: true} +// 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)) + }) + }, } - return KerberosResult{Principal: m.principal, Realm: m.realm, OK: m.ok, Err: m.err} } -func TestKerberos_NoHeader_Returns401Negotiate(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} - lc.krbValidator = mv +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(), + krbEnabled: true, + 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) @@ -59,62 +80,98 @@ func TestKerberos_NoHeader_Returns401Negotiate(t *testing.T) { if ident != nil { t.Fatalf("expected no identity") } - if w.Result().StatusCode != 401 { + 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") + t.Fatalf("expected bare Negotiate challenge, got %q", hdr) } } -func TestKerberos_ExpectedRealmMismatch_401(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart", ExpectedRealm: "EXAMPLE.COM"}} - mv := &mockKrbValidator{principal: "jdoe@OTHER.COM", realm: "OTHER.COM", ok: true, err: nil, step: -1} - lc.krbValidator = mv +func TestKerberos_NoHeader_Fallback_SkipsMiddleware(t *testing.T) { + // With FallbackToPassword=true and no Authorization header, the middleware + // must not run at all (otherwise w would be tainted with a 401). + middlewareRan := false + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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) 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 bool(handled) { + t.Fatalf("expected not handled (caller should render form)") } if ident != nil { t.Fatalf("expected no identity") } - if w.Result().StatusCode != 401 { - t.Fatalf("expected 401, got %d", w.Result().StatusCode) + if middlewareRan { + t.Fatalf("SPNEGO middleware should not run when fallback=true and no Authorization header") } - if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { - t.Fatalf("expected Negotiate challenge") + if w.Code != 200 && w.Code != 0 { + t.Fatalf("expected w untouched, got status=%d", w.Code) } } -func TestKerberos_FallbackToPassword_NoHeader_HandledFalse(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} - lc.krbValidator = mv +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(), + krbEnabled: true, + 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") + 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_ContinueNeeded_SendsResponseToken(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, contToken: []byte{0x01, 0x02, 0x03}} - lc.krbValidator = mv +func TestKerberos_MiddlewareFails_NoFallback_ForwardsResponse(t *testing.T) { + // Middleware writes a reject token; no fallback: forward to client verbatim. + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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 Zm9v") + r.Header.Set("Authorization", "Negotiate deadbeef") w := httptest.NewRecorder() ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) if err != nil { @@ -126,126 +183,113 @@ func TestKerberos_ContinueNeeded_SendsResponseToken(t *testing.T) { if ident != nil { t.Fatalf("expected no identity") } - if w.Code != 401 { - t.Fatalf("expected 401, got %d", w.Code) - } - hdr := w.Header().Get("WWW-Authenticate") - if !strings.HasPrefix(hdr, "Negotiate ") { - t.Fatalf("expected Negotiate header, got %q", hdr) + if w.Code != http.StatusUnauthorized { + t.Fatalf("expected 401 forwarded, got %d", w.Code) } - want := "Negotiate " + base64.StdEncoding.EncodeToString([]byte{0x01, 0x02, 0x03}) - if hdr != want { - t.Fatalf("unexpected negotiate header, got %q want %q", hdr, want) + if hdr := w.Header().Get("WWW-Authenticate"); !strings.HasPrefix(hdr, "Negotiate ") { + t.Fatalf("expected reject token forwarded, got %q", hdr) } } -func TestKerberos_ContinueThenSuccess_ShortCircuitIdentity(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} +func TestKerberos_Success_BuildsIdentity(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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" - mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", contToken: []byte{0xAA, 0xBB}, step: 0} - lc.krbValidator = mv - - // First request -> 401 with continue token - r1 := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) - w1 := httptest.NewRecorder() - ident, handled, err := lc.TrySPNEGO(r1.Context(), connector.Scopes{}, w1, r1) - if err != nil || !bool(handled) || ident != nil || w1.Code != 401 { - t.Fatalf("unexpected first step result: ident=%v handled=%v code=%d err=%v", ident, handled, w1.Code, err) - } - hdr := w1.Header().Get("WWW-Authenticate") - if hdr == "" || !strings.HasPrefix(hdr, "Negotiate ") { - t.Fatalf("expected Negotiate header on first step, got %q", hdr) - } - - // Second request -> validator now returns ok=true (due to step increment) - lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + 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, true, nil - } - r2 := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) - w2 := httptest.NewRecorder() - ident2, handled2, err2 := lc.TrySPNEGO(r2.Context(), connector.Scopes{}, w2, r2) - if err2 != nil { - t.Fatalf("unexpected err: %v", err2) + return *e, nil } - if !bool(handled2) || ident2 == nil { - t.Fatalf("expected handled with identity on second step") - } -} - -func TestKerberos_ContinueNeeded_FallbackTrue_NotHandled(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{contToken: []byte{0x10}, step: 0} - lc.krbValidator = mv 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 fallback=true when continue is needed") + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") } - if w.Code != 200 && w.Code != 0 { - t.Fatalf("expected no response written yet, got %d", w.Code) + if ident.Username == "" || ident.Email == "" || ident.UserID == "" { + t.Fatalf("expected populated identity, got %+v", *ident) } } -func TestKerberos_mapPrincipal(t *testing.T) { - cases := []struct{ in, mode, want string }{ - {"JDoe@EXAMPLE.COM", "localpart", "jdoe"}, - {"JDoe@EXAMPLE.COM", "sAMAccountName", "jdoe"}, - {"JDoe@EXAMPLE.COM", "userPrincipalName", "jdoe@example.com"}, +// 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(), + krbEnabled: true, + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), } - for _, c := range cases { - got := mapPrincipal(c.in, c.mode) - if got != c.want { - t.Fatalf("mapPrincipal(%q,%q)=%q; want %q", c.in, c.mode, got, c.want) - } + lc.krbLookupUserHook = func(_ context.Context, _ *ldapConnector, _ string) (ldaplib.Entry, error) { + return ldaplib.Entry{}, fmt.Errorf("user not found for principal") } -} - -func TestKerberos_UserNotFound_ReturnsError(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} - lc.krbValidator = mv r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) w := httptest.NewRecorder() - ident, handled, err := lc.TrySPNEGO(r.Context(), connector.Scopes{}, w, r) + _, 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 ident != nil { - t.Fatalf("expected no identity") - } 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) + } } -func TestKerberos_ValidPrincipal_CompletesFlow(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} - lc.Config.UserSearch.IDAttr = "uid" - lc.Config.UserSearch.EmailAttr = "mail" - lc.Config.UserSearch.NameAttr = "cn" - mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} - lc.krbValidator = mv - lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, 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, true, nil +// 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(), + krbEnabled: true, + 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(), + krbEnabled: true, + 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() @@ -253,59 +297,61 @@ func TestKerberos_ValidPrincipal_CompletesFlow(t *testing.T) { if err != nil { t.Fatalf("unexpected err: %v", err) } - if !bool(handled) { - t.Fatalf("expected handled") + if !bool(handled) || ident != nil { + t.Fatalf("expected handled with no identity") } - if ident == nil { - t.Fatalf("expected identity") + if w.Result().StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401, got %d", w.Result().StatusCode) } - if ident.Username == "" || ident.Email == "" || ident.UserID == "" { - t.Fatalf("expected populated identity, got %+v", *ident) + if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { + t.Fatalf("expected bare Negotiate, got %q", hdr) } } -func TestKerberos_InvalidHeader_Returns401Negotiate(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} - lc.krbValidator = mv +func TestKerberos_ExpectedRealmMismatch_Fallback_RendersForm(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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 !!!notbase64!!!") + 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.Result().StatusCode != 401 { - t.Fatalf("expected 401, got %d", w.Result().StatusCode) - } - if hdr := w.Header().Get("WWW-Authenticate"); hdr != "Negotiate" { - t.Fatalf("expected Negotiate challenge") + if bool(handled) || ident != nil { + t.Fatalf("expected not handled, got handled=%v ident=%v", handled, ident) } } -func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "userPrincipalName"}} +func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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" - mv := &mockKrbValidator{principal: "J.Doe@Example.COM", realm: "Example.COM", ok: true, err: nil, step: -1} - lc.krbValidator = mv - lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { - if username != "j.doe@example.com" { - return ldaplib.Entry{}, false, nil - } - 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"}, + 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, true, nil + return *e, nil } r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) w := httptest.NewRecorder() @@ -313,86 +359,66 @@ func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { if err != nil { t.Fatalf("unexpected err: %v", err) } - if !bool(handled) { - t.Fatalf("expected handled") - } - if ident == nil { - t.Fatalf("expected identity") + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") } } -func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}} +func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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" - mv := &mockKrbValidator{principal: "jdoe@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} - lc.krbValidator = mv - lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { + 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, true, nil + return *e, nil } r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) w := httptest.NewRecorder() - scopes := connector.Scopes{OfflineAccess: true} - ident, handled, err := lc.TrySPNEGO(r.Context(), 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 identity") - } - if len(ident.ConnectorData) == 0 { - t.Fatalf("expected connector data for offline access") - } -} - -func TestKerberos_FallbackTrue_InvalidHeader_NotHandled(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}} - mv := &mockKrbValidator{principal: "", realm: "", ok: false, err: nil, step: -1} - lc.krbValidator = mv - r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) - r.Header.Set("Authorization", "Negotiate !!!") - 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 for fallback path") - } - if ident != nil { - t.Fatalf("expected no identity") - } - if w.Code != 200 && w.Code != 0 { - t.Fatalf("expected no response written yet, got %d", w.Code) + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") } } func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "sAMAccountName"}} + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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" - mv := &mockKrbValidator{principal: "Admin@REALM.LOCAL", realm: "REALM.LOCAL", ok: true, err: nil, step: -1} - lc.krbValidator = mv - lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, error) { - if username != "admin" { - return ldaplib.Entry{}, false, nil + 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, true, nil + return *e, nil } r := httptest.NewRequest("GET", "/auth/ldap/login?state=abc", nil) w := httptest.NewRecorder() @@ -400,39 +426,76 @@ func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { if err != nil { t.Fatalf("unexpected err: %v", err) } - if !bool(handled) { - t.Fatalf("expected handled") - } - if ident == nil { - t.Fatalf("expected identity") + if !bool(handled) || ident == nil { + t.Fatalf("expected handled with identity") } } -func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { - lc := &ldapConnector{logger: slog.Default(), krbEnabled: true, krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart", ExpectedRealm: "ExAmPlE.CoM"}} +func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { + lc := &ldapConnector{ + logger: slog.Default(), + krbEnabled: true, + 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" - mv := &mockKrbValidator{principal: "user@EXAMPLE.COM", realm: "EXAMPLE.COM", ok: true, err: nil, step: -1} - lc.krbValidator = mv - lc.krbLookupUserHook = func(c *ldapConnector, username string) (ldaplib.Entry, bool, 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"}, + 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, true, nil + 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) { - t.Fatalf("expected handled") + if bool(handled) || ident != nil { + t.Fatalf("expected not handled with no identity") } - if ident == nil { - t.Fatalf("expected 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 78e811d1bc..06f577f78e 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -192,9 +192,27 @@ type Config struct { } // 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"` @@ -253,15 +271,21 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro if err != nil { return nil, err } - // If Kerberos is enabled, initialize gokrb5 validator. - if lc, ok := conn.(*ldapConnector); ok && lc.krbEnabled && lc.krbValidator == nil { - v, verr := newGokrb5ValidatorWithLogger(lc.krbConf.KeytabPath, logger) - if verr != nil { - logger.Warn("failed to initialize kerberos validator; disabling kerberos", "err", verr) + // If Kerberos is enabled, load the keytab and bind SPNEGO middleware. + if lc, ok := conn.(*ldapConnector); ok && lc.krbEnabled && lc.krb == nil { + st, kerr := loadKerberosState(lc.krbConf) + if kerr != nil { + logger.Warn("failed to initialize kerberos; disabling kerberos", "err", kerr) lc.krbEnabled = false } else { - lc.krbValidator = v - logger.Info("Kerberos enabled for LDAP connector", "keytab", lc.krbConf.KeytabPath, "expected_realm", lc.krbConf.ExpectedRealm) + 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 @@ -384,6 +408,7 @@ func (c *Config) openConnector(logger *slog.Logger) (*ldapConnector, error) { var ( _ connector.PasswordConnector = (*ldapConnector)(nil) _ connector.RefreshConnector = (*ldapConnector)(nil) + _ connector.SPNEGOAware = (*ldapConnector)(nil) ) type ldapConnector struct { @@ -398,12 +423,16 @@ type ldapConnector struct { logger *slog.Logger - // Kerberos/SPNEGO fields - krbEnabled bool - krbConf kerberosConfig - krbValidator KerberosValidator - // krbLookupUserHook allows tests to inject a user entry without LDAP queries. - krbLookupUserHook func(c *ldapConnector, username string) (ldap.Entry, bool, error) + // Kerberos/SPNEGO support. krbEnabled gates TrySPNEGO; krb is nil until + // the keytab has been loaded successfully in Open(). + krbEnabled bool + 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/go.mod b/go.mod index ffcc29ee73..f7c2b0834d 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ 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/gofork v1.7.6 + 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 @@ -103,7 +103,7 @@ require ( 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/goidentity/v6 v6.0.1 // 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 diff --git a/server/handlers.go b/server/handlers.go index 7a3185be7e..452e5079c2 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -609,18 +609,18 @@ func (s *Server) handlePasswordLogin(w http.ResponseWriter, r *http.Request) { // 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(r.Context(), scopes, w, r); bool(handled) { + 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(r.Context(), "SPNEGO authentication error", "err", err) + 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(r.Context(), *ident, authReq, conn.Connector) + redirectURL, canSkipApproval, err := s.finalizeLogin(ctx, *ident, authReq, conn.Connector) if err != nil { - s.logger.ErrorContext(r.Context(), "failed to finalize login", "err", err) + s.logger.ErrorContext(ctx, "failed to finalize login", "err", err) s.renderError(r, w, http.StatusInternalServerError, "Login error.") return } @@ -628,7 +628,7 @@ func (s *Server) handlePasswordLogin(w http.ResponseWriter, r *http.Request) { if canSkipApproval { authReq, err = s.storage.GetAuthRequest(ctx, authReq.ID) if err != nil { - s.logger.ErrorContext(r.Context(), "failed to get finalized auth request", "err", err) + s.logger.ErrorContext(ctx, "failed to get finalized auth request", "err", err) s.renderError(r, w, http.StatusInternalServerError, "Login error.") return } @@ -639,7 +639,9 @@ func (s *Server) handlePasswordLogin(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, redirectURL, http.StatusSeeOther) return } - // handled with no identity typically means a Negotiate challenge was written; do not render form. + // 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 } } From 062c7b8702ae29ee4de08e2c4fc554b0bf8e63c8 Mon Sep 17 00:00:00 2001 From: Ivan Zvyagintsev Date: Tue, 28 Apr 2026 14:51:44 +0300 Subject: [PATCH 7/7] apply suggestions from review Signed-off-by: Ivan Zvyagintsev --- connector/ldap/kerberos.go | 200 ++++++++++++++++++++++++++++---- connector/ldap/kerberos_test.go | 195 +++++++++++++++++++++++-------- connector/ldap/ldap.go | 37 +++--- examples/ldap/config-ldap.yaml | 4 + 4 files changed, 344 insertions(+), 92 deletions(-) diff --git a/connector/ldap/kerberos.go b/connector/ldap/kerberos.go index c81dbcf7c6..1d5931d2b6 100644 --- a/connector/ldap/kerberos.go +++ b/connector/ldap/kerberos.go @@ -3,11 +3,11 @@ package ldap import ( + "bytes" "context" "encoding/json" "fmt" "net/http" - "net/http/httptest" "os" "strings" "time" @@ -22,6 +22,52 @@ import ( "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 @@ -96,24 +142,34 @@ func mapPrincipal(username, realm, mapping string) string { // Behavior: // - Kerberos disabled: returns (nil, false, nil) so the caller renders the // password form. -// - FallbackToPassword=true and the request carries no Negotiate header: -// short-circuits to the password form without invoking the SPNEGO -// middleware. This avoids writing an unwanted 401 when a password is -// explicitly permitted. -// - Otherwise, runs the SPNEGO middleware into a response recorder. If the -// middleware does not authenticate, its buffered response is either -// forwarded to the client (FallbackToPassword=false) or discarded -// (FallbackToPassword=true, caller renders 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. +// 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 } - if c.krbConf.FallbackToPassword && - !strings.HasPrefix(r.Header.Get("Authorization"), "Negotiate ") { - 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 @@ -136,7 +192,7 @@ func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w htt id = creds }) - rec := httptest.NewRecorder() + rec := newBufferedResponse() c.krb.authenticate(inner).ServeHTTP(rec, r) if innerErr != nil { @@ -145,22 +201,40 @@ func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w htt } if id == nil { - if c.krbConf.FallbackToPassword { - c.logger.Info("kerberos: SPNEGO did not authenticate; falling back to password form") + // 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 } - c.logger.Info("kerberos: SPNEGO did not authenticate; forwarding middleware response", "status", rec.Code) - copyRecorded(rec, w) + // 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 } - w.Header().Set("WWW-Authenticate", "Negotiate") - w.WriteHeader(http.StatusUnauthorized) + // 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 } @@ -208,20 +282,96 @@ func (c *ldapConnector) TrySPNEGO(ctx context.Context, s connector.Scopes, w htt 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 } -// copyRecorded forwards the buffered middleware response to the client. -func copyRecorded(rec *httptest.ResponseRecorder, w http.ResponseWriter) { - for k, vv := range rec.Header() { +// 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()) + 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 diff --git a/connector/ldap/kerberos_test.go b/connector/ldap/kerberos_test.go index c602990538..8b9df0ade8 100644 --- a/connector/ldap/kerberos_test.go +++ b/connector/ldap/kerberos_test.go @@ -63,10 +63,9 @@ func newCreds(username, realm string) *credentials.Credentials { func TestKerberos_NoHeader_NoFallback_ForwardsMiddleware401(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, - krb: fakeAuthenticate(nil), + 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() @@ -88,17 +87,19 @@ func TestKerberos_NoHeader_NoFallback_ForwardsMiddleware401(t *testing.T) { } } -func TestKerberos_NoHeader_Fallback_SkipsMiddleware(t *testing.T) { - // With FallbackToPassword=true and no Authorization header, the middleware - // must not run at all (otherwise w would be tainted with a 401). +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(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, + 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) }) }}, @@ -109,6 +110,49 @@ func TestKerberos_NoHeader_Fallback_SkipsMiddleware(t *testing.T) { 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)") } @@ -116,7 +160,7 @@ func TestKerberos_NoHeader_Fallback_SkipsMiddleware(t *testing.T) { t.Fatalf("expected no identity") } if middlewareRan { - t.Fatalf("SPNEGO middleware should not run when fallback=true and no Authorization header") + 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) @@ -128,9 +172,8 @@ func TestKerberos_MiddlewareFails_Fallback_DiscardsResponse(t *testing.T) { // 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(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: true, UsernameFromPrincipal: "localpart"}, krb: fakeAuthenticateWithResponse( http.StatusUnauthorized, map[string]string{"WWW-Authenticate": "Negotiate oQcwBaADCgEC"}, @@ -161,9 +204,8 @@ func TestKerberos_MiddlewareFails_Fallback_DiscardsResponse(t *testing.T) { func TestKerberos_MiddlewareFails_NoFallback_ForwardsResponse(t *testing.T) { // Middleware writes a reject token; no fallback: forward to client verbatim. lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, + logger: slog.Default(), + krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, krb: fakeAuthenticateWithResponse( http.StatusUnauthorized, map[string]string{"WWW-Authenticate": "Negotiate oQcwBaADCgEC"}, @@ -193,10 +235,9 @@ func TestKerberos_MiddlewareFails_NoFallback_ForwardsResponse(t *testing.T) { func TestKerberos_Success_BuildsIdentity(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, - krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + 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" @@ -224,6 +265,74 @@ func TestKerberos_Success_BuildsIdentity(t *testing.T) { 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 @@ -231,10 +340,9 @@ func TestKerberos_Success_BuildsIdentity(t *testing.T) { // is authoritative, so the test is hermetic (no network call to :636). func TestKerberos_UserNotFound_ReturnsError(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, - krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + 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") @@ -261,10 +369,9 @@ func TestKerberos_UserNotFound_ReturnsError(t *testing.T) { func TestKerberos_LookupHookError_Wrapped(t *testing.T) { sentinel := errors.New("boom: LDAP server tantrum") lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, - krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + 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 @@ -282,8 +389,7 @@ func TestKerberos_LookupHookError_Wrapped(t *testing.T) { func TestKerberos_ExpectedRealmMismatch_NoFallback_401(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, + logger: slog.Default(), krbConf: kerberosConfig{ FallbackToPassword: false, UsernameFromPrincipal: "localpart", @@ -310,8 +416,7 @@ func TestKerberos_ExpectedRealmMismatch_NoFallback_401(t *testing.T) { func TestKerberos_ExpectedRealmMismatch_Fallback_RendersForm(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, + logger: slog.Default(), krbConf: kerberosConfig{ FallbackToPassword: true, UsernameFromPrincipal: "localpart", @@ -333,8 +438,7 @@ func TestKerberos_ExpectedRealmMismatch_Fallback_RendersForm(t *testing.T) { func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, + logger: slog.Default(), krbConf: kerberosConfig{ FallbackToPassword: false, UsernameFromPrincipal: "localpart", @@ -366,10 +470,9 @@ func TestKerberos_ExpectedRealm_CaseInsensitive(t *testing.T) { func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "userPrincipalName"}, - krb: fakeAuthenticate(newCreds("J.Doe", "Example.COM")), + 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" @@ -401,10 +504,9 @@ func TestKerberos_UserPrincipalName_Mapping(t *testing.T) { func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "sAMAccountName"}, - krb: fakeAuthenticate(newCreds("Admin", "REALM.LOCAL")), + 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" @@ -433,10 +535,9 @@ func TestKerberos_sAMAccountName_EqualsLocalpart(t *testing.T) { func TestKerberos_OfflineAccess_SetsConnectorData(t *testing.T) { lc := &ldapConnector{ - logger: slog.Default(), - krbEnabled: true, - krbConf: kerberosConfig{FallbackToPassword: false, UsernameFromPrincipal: "localpart"}, - krb: fakeAuthenticate(newCreds("jdoe", "EXAMPLE.COM")), + 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" diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index 06f577f78e..27f93e3393 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -272,11 +272,12 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro return nil, err } // If Kerberos is enabled, load the keytab and bind SPNEGO middleware. - if lc, ok := conn.(*ldapConnector); ok && lc.krbEnabled && lc.krb == nil { + // 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) - lc.krbEnabled = false } else { lc.krb = st logger.Info("kerberos SPNEGO enabled for LDAP connector", @@ -373,20 +374,19 @@ func (c *Config) openConnector(logger *slog.Logger) (*ldapConnector, error) { // TODO(nabokihms): remove it after deleting deprecated groupSearch options c.GroupSearch.UserMatchers = userMatchers(c, logger) - // Normalize Kerberos defaults - var krbEnabled bool + // 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 { + if c.Kerberos != nil && c.Kerberos.Enabled { krbConf = *c.Kerberos if krbConf.UsernameFromPrincipal == "" { krbConf.UsernameFromPrincipal = "localpart" } - if krbConf.Enabled { - if krbConf.KeytabPath == "" { - logger.Warn("kerberos enabled but keytabPath is empty; disabling kerberos") - } else { - krbEnabled = true - } + if krbConf.KeytabPath == "" { + logger.Warn("kerberos enabled but keytabPath is empty; disabling kerberos") + krbConf = kerberosConfig{} } } @@ -397,10 +397,7 @@ func (c *Config) openConnector(logger *slog.Logger) (*ldapConnector, error) { tlsConfig: tlsConfig, usernameAttrs: c.UserSearch.Username, logger: logger, - } - if krbEnabled { - lc.krbEnabled = true - lc.krbConf = krbConf + krbConf: krbConf, } return lc, nil } @@ -423,11 +420,11 @@ type ldapConnector struct { logger *slog.Logger - // Kerberos/SPNEGO support. krbEnabled gates TrySPNEGO; krb is nil until - // the keytab has been loaded successfully in Open(). - krbEnabled bool - krbConf kerberosConfig - krb *krbState + // 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 diff --git a/examples/ldap/config-ldap.yaml b/examples/ldap/config-ldap.yaml index 1e4b358649..49a7e25fff 100644 --- a/examples/ldap/config-ldap.yaml +++ b/examples/ldap/config-ldap.yaml @@ -67,6 +67,10 @@ connectors: # 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