From f9fc123c08e9f144286a1af03306b290afcdd02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Prpi=C4=8D?= Date: Thu, 19 Mar 2026 11:01:28 -0400 Subject: [PATCH] Migrate LDAP integration from legacy to IPA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate from ldap.corp.redhat.com (anonymous binds, deprecated) to ipa.corp.redhat.com (authenticated binds via service account). Go client: - Replace skipTLSVerify with bindDN, bindPassword, caCertPath params - connect() loads custom CA cert (appended to system pool) and performs authenticated Bind() - Default groupBaseDN derivation updated from ou=managedGroups to cn=groups (IPA pattern) - Deduplicate group memberships Manifests: - Update ldap-config.yaml (all overlays) with IPA URL and base DNs, add LDAP_BIND_DN, remove LDAP_SKIP_TLS_VERIFY - Add ldap-credentials.yaml Secret (all overlays) for LDAP_BIND_PASSWORD - Add LDAP_CA_CERT_PATH env var and ldap-ca-cert volume mount to backend deployment (optional, for TLS verification) Co-Authored-By: Claude Opus 4.6 Signed-off-by: Martin Prpič --- components/backend/ldap/client.go | 132 ++++++++++++++---- components/backend/ldap/client_test.go | 40 ++++-- components/backend/main.go | 22 ++- .../base/core/backend-deployment.yaml | 31 +++- .../overlays/kind/kustomization.yaml | 2 + .../manifests/overlays/kind/ldap-config.yaml | 12 ++ .../overlays/kind/ldap-credentials.yaml | 9 ++ .../overlays/local-dev/kustomization.yaml | 2 + .../overlays/local-dev/ldap-config.yaml | 13 ++ .../overlays/local-dev/ldap-credentials.yaml | 9 ++ .../overlays/production/ldap-config.yaml | 9 +- .../overlays/production/ldap-credentials.yaml | 9 ++ 12 files changed, 243 insertions(+), 47 deletions(-) create mode 100644 components/manifests/overlays/kind/ldap-config.yaml create mode 100644 components/manifests/overlays/kind/ldap-credentials.yaml create mode 100644 components/manifests/overlays/local-dev/ldap-config.yaml create mode 100644 components/manifests/overlays/local-dev/ldap-credentials.yaml create mode 100644 components/manifests/overlays/production/ldap-credentials.yaml diff --git a/components/backend/ldap/client.go b/components/backend/ldap/client.go index e7a773f16..d4ce392ed 100644 --- a/components/backend/ldap/client.go +++ b/components/backend/ldap/client.go @@ -3,8 +3,10 @@ package ldap import ( "crypto/tls" + "crypto/x509" "fmt" "net" + "os" "strings" "sync" "time" @@ -52,47 +54,123 @@ type cacheEntry struct { // Client provides LDAP search functionality with in-memory caching. type Client struct { - url string - baseDN string - groupBaseDN string - skipTLSVerify bool - cache sync.Map - cacheTTL time.Duration + url string // explicit LDAP URL (fallback if srvDomain is empty) + srvDomain string // domain for DNS SRV lookup (e.g. "ipa.redhat.com") + baseDN string + groupBaseDN string + bindDN string + bindPassword string + tlsConfig *tls.Config + cache sync.Map + cacheTTL time.Duration } // NewClient creates a new LDAP client. -// baseDN is the base DN for user searches (e.g. "ou=users,dc=redhat,dc=com"). +// url is the LDAP server URL, used as a fallback when srvDomain is empty. +// srvDomain enables DNS SRV discovery (e.g. "ipa.redhat.com" queries _ldap._tcp.ipa.redhat.com). +// baseDN is the base DN for user searches (e.g. "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com"). // groupBaseDN is the base DN for group searches. If empty, it is derived from -// baseDN by replacing the first OU with "ou=managedGroups". -func NewClient(url, baseDN, groupBaseDN string, skipTLSVerify bool) *Client { +// baseDN by replacing the first component with "cn=groups". +// bindDN and bindPassword are used for authenticated binds (required for IPA). +// caCertPath is an optional path to a PEM CA certificate file to trust. +func NewClient(url, srvDomain, baseDN, groupBaseDN, bindDN, bindPassword, caCertPath string) (*Client, error) { if groupBaseDN == "" { - groupBaseDN = "ou=managedGroups,dc=redhat,dc=com" if parts := strings.SplitN(baseDN, ",", 2); len(parts) == 2 { - groupBaseDN = "ou=managedGroups," + parts[1] + groupBaseDN = "cn=groups," + parts[1] } } - return &Client{ - url: url, - baseDN: baseDN, - groupBaseDN: groupBaseDN, - skipTLSVerify: skipTLSVerify, - cacheTTL: defaultCacheTTL, + tlsConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + } + if caCertPath != "" { + rootCAs, err := x509.SystemCertPool() + if err != nil { + rootCAs = x509.NewCertPool() + } + caCert, err := os.ReadFile(caCertPath) + if err != nil { + return nil, fmt.Errorf("ldap read CA cert %s: %w", caCertPath, err) + } + if !rootCAs.AppendCertsFromPEM(caCert) { + return nil, fmt.Errorf("ldap CA cert %s: no valid PEM certificates found", caCertPath) + } + tlsConfig.RootCAs = rootCAs } + + return &Client{ + url: url, + srvDomain: srvDomain, + baseDN: baseDN, + groupBaseDN: groupBaseDN, + bindDN: bindDN, + bindPassword: bindPassword, + tlsConfig: tlsConfig, + cacheTTL: defaultCacheTTL, + }, nil } -// connect dials the LDAP server and returns a connection. +// connect dials an LDAP server (discovered via SRV or explicit URL) and performs an authenticated bind. func (c *Client) connect() (*goldap.Conn, error) { - conn, err := goldap.DialURL(c.url, goldap.DialWithTLSConfig(&tls.Config{ - MinVersion: tls.VersionTLS12, - InsecureSkipVerify: c.skipTLSVerify, //nolint:gosec // controlled by LDAP_SKIP_TLS_VERIFY env var for dev - }), goldap.DialWithDialer(&net.Dialer{Timeout: defaultConnTimeout})) + conn, err := c.dial() if err != nil { - return nil, fmt.Errorf("ldap dial %s: %w", c.url, err) + return nil, err + } + + if c.bindDN != "" { + if err := conn.Bind(c.bindDN, c.bindPassword); err != nil { + conn.Close() + return nil, fmt.Errorf("ldap bind: %w", err) + } } + return conn, nil } +// dial connects to an LDAP server. If srvDomain is configured, it discovers +// servers via DNS SRV records (_ldap._tcp.) and tries each in +// priority/weight order using LDAPS (port 636). Falls back to the explicit URL. +func (c *Client) dial() (*goldap.Conn, error) { + dialOpts := []goldap.DialOpt{ + goldap.DialWithTLSConfig(c.tlsConfig), + goldap.DialWithDialer(&net.Dialer{Timeout: defaultConnTimeout}), + } + + if c.srvDomain == "" { + conn, err := goldap.DialURL(c.url, dialOpts...) + if err != nil { + return nil, fmt.Errorf("ldap dial %s: %w", c.url, err) + } + return conn, nil + } + + _, addrs, err := net.LookupSRV("ldap", "tcp", c.srvDomain) + if err != nil || len(addrs) == 0 { + if c.url != "" { + conn, dialErr := goldap.DialURL(c.url, dialOpts...) + if dialErr != nil { + return nil, fmt.Errorf("ldap SRV lookup failed (%v) and fallback dial %s failed: %w", err, c.url, dialErr) + } + return conn, nil + } + return nil, fmt.Errorf("ldap SRV lookup _ldap._tcp.%s: %w", c.srvDomain, err) + } + + // SRV records return port 389 (plain LDAP); connect on 636 for LDAPS. + var lastErr error + for _, addr := range addrs { + host := strings.TrimSuffix(addr.Target, ".") + url := fmt.Sprintf("ldaps://%s:636", host) + conn, dialErr := goldap.DialURL(url, dialOpts...) + if dialErr != nil { + lastErr = fmt.Errorf("ldap dial %s: %w", url, dialErr) + continue + } + return conn, nil + } + return nil, fmt.Errorf("ldap all SRV targets for %s failed, last error: %w", c.srvDomain, lastErr) +} + // cacheGet returns a cached value if it exists and hasn't expired. func (c *Client) cacheGet(key string) (any, bool) { val, ok := c.cache.Load(key) @@ -133,9 +211,13 @@ func entryToUser(entry *goldap.Entry) LDAPUser { break } } + seen := make(map[string]struct{}) for _, dn := range entry.GetAttributeValues("memberOf") { if cn := extractCNFromDN(dn); cn != "" { - user.Groups = append(user.Groups, cn) + if _, dup := seen[cn]; !dup { + seen[cn] = struct{}{} + user.Groups = append(user.Groups, cn) + } } } return user @@ -188,7 +270,7 @@ func (c *Client) SearchUsers(query string) ([]LDAPUser, error) { } // SearchGroups searches for groups matching the query string. -// Searches the cn attribute with a prefix match in ou=managedGroups. +// Searches the cn attribute with a prefix match in the groups base DN. func (c *Client) SearchGroups(query string) ([]LDAPGroup, error) { query = sanitizeQuery(query) if len(query) < MinQueryLength { diff --git a/components/backend/ldap/client_test.go b/components/backend/ldap/client_test.go index 5db07d982..9b9d41a1e 100644 --- a/components/backend/ldap/client_test.go +++ b/components/backend/ldap/client_test.go @@ -268,16 +268,22 @@ func TestCacheMiss(t *testing.T) { } func TestNewClient(t *testing.T) { - client := NewClient("ldaps://ldap.example.com", "ou=users,dc=redhat,dc=com", "", false) + client, err := NewClient("ldaps://ldap.example.com", "", "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com", "", "uid=svc,cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com", "pass", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if client.url != "ldaps://ldap.example.com" { t.Errorf("expected url 'ldaps://ldap.example.com', got %q", client.url) } - if client.baseDN != "ou=users,dc=redhat,dc=com" { - t.Errorf("expected baseDN 'ou=users,dc=redhat,dc=com', got %q", client.baseDN) + if client.baseDN != "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" { + t.Errorf("expected baseDN 'cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com', got %q", client.baseDN) + } + if client.groupBaseDN != "cn=groups,cn=accounts,dc=ipa,dc=redhat,dc=com" { + t.Errorf("expected groupBaseDN 'cn=groups,cn=accounts,dc=ipa,dc=redhat,dc=com', got %q", client.groupBaseDN) } - if client.groupBaseDN != "ou=managedGroups,dc=redhat,dc=com" { - t.Errorf("expected groupBaseDN 'ou=managedGroups,dc=redhat,dc=com', got %q", client.groupBaseDN) + if client.bindDN != "uid=svc,cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" { + t.Errorf("expected bindDN 'uid=svc,cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com', got %q", client.bindDN) } if client.cacheTTL != defaultCacheTTL { t.Errorf("expected cacheTTL %v, got %v", defaultCacheTTL, client.cacheTTL) @@ -285,15 +291,21 @@ func TestNewClient(t *testing.T) { } func TestNewClientExplicitGroupBaseDN(t *testing.T) { - client := NewClient("ldaps://ldap.example.com", "ou=users,dc=example,dc=com", "ou=groups,dc=example,dc=com", false) + client, err := NewClient("ldaps://ldap.example.com", "", "cn=users,cn=accounts,dc=example,dc=com", "cn=groups,cn=accounts,dc=example,dc=com", "", "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } - if client.groupBaseDN != "ou=groups,dc=example,dc=com" { - t.Errorf("expected explicit groupBaseDN 'ou=groups,dc=example,dc=com', got %q", client.groupBaseDN) + if client.groupBaseDN != "cn=groups,cn=accounts,dc=example,dc=com" { + t.Errorf("expected explicit groupBaseDN 'cn=groups,cn=accounts,dc=example,dc=com', got %q", client.groupBaseDN) } } func TestSearchUsersShortQuery(t *testing.T) { - client := NewClient("ldaps://ldap.example.com", "ou=users,dc=redhat,dc=com", "", false) + client, err := NewClient("ldaps://ldap.example.com", "", "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com", "", "", "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } // Query too short should return nil without connecting users, err := client.SearchUsers("m") @@ -306,7 +318,10 @@ func TestSearchUsersShortQuery(t *testing.T) { } func TestSearchGroupsShortQuery(t *testing.T) { - client := NewClient("ldaps://ldap.example.com", "ou=users,dc=redhat,dc=com", "", false) + client, err := NewClient("ldaps://ldap.example.com", "", "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com", "", "", "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } groups, err := client.SearchGroups("a") if err != nil { @@ -318,7 +333,10 @@ func TestSearchGroupsShortQuery(t *testing.T) { } func TestGetUserEmptyUID(t *testing.T) { - client := NewClient("ldaps://ldap.example.com", "ou=users,dc=redhat,dc=com", "", false) + client, err := NewClient("ldaps://ldap.example.com", "", "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com", "", "", "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } user, err := client.GetUser("") if err != nil { diff --git a/components/backend/main.go b/components/backend/main.go index 6480aa308..4db25d77c 100644 --- a/components/backend/main.go +++ b/components/backend/main.go @@ -135,14 +135,24 @@ func main() { return server.Namespace } - // Initialize LDAP client (optional - requires LDAP_URL to be set) + // Initialize LDAP client (optional - requires LDAP_URL or LDAP_SRV_DOMAIN) // Access is gated by the "ldap.autocomplete.enabled" Unleash feature flag. - if ldapURL := os.Getenv("LDAP_URL"); ldapURL != "" { - ldapBaseDN := getEnvOrDefault("LDAP_BASE_DN", "ou=users,dc=redhat,dc=com") + ldapURL := os.Getenv("LDAP_URL") + ldapSRVDomain := os.Getenv("LDAP_SRV_DOMAIN") + if ldapURL != "" || ldapSRVDomain != "" { + ldapBaseDN := getEnvOrDefault("LDAP_BASE_DN", "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com") ldapGroupBaseDN := os.Getenv("LDAP_GROUP_BASE_DN") // optional, derived from LDAP_BASE_DN if empty - skipTLS := os.Getenv("LDAP_SKIP_TLS_VERIFY") == "true" - handlers.LDAPClient = ldap.NewClient(ldapURL, ldapBaseDN, ldapGroupBaseDN, skipTLS) - log.Printf("LDAP client initialized: %s (base DN: %s, group base DN: %s, skipTLSVerify: %v)", ldapURL, ldapBaseDN, ldapGroupBaseDN, skipTLS) + ldapBindDN := os.Getenv("LDAP_BIND_DN") + ldapBindPassword := os.Getenv("LDAP_BIND_PASSWORD") + ldapCACertPath := os.Getenv("LDAP_CA_CERT_PATH") + if ldapBindDN == "" || ldapBindPassword == "" { + log.Printf("LDAP disabled: missing bind credentials") + } else if ldapClient, err := ldap.NewClient(ldapURL, ldapSRVDomain, ldapBaseDN, ldapGroupBaseDN, ldapBindDN, ldapBindPassword, ldapCACertPath); err != nil { + log.Printf("LDAP disabled: %v", err) + } else { + handlers.LDAPClient = ldapClient + log.Printf("LDAP client configured (URL: %s, SRV domain: %s, base DN: %s)", ldapURL, ldapSRVDomain, ldapBaseDN) + } } // Initialize GitHub auth handlers diff --git a/components/manifests/base/core/backend-deployment.yaml b/components/manifests/base/core/backend-deployment.yaml index ea72f3927..c22482527 100644 --- a/components/manifests/base/core/backend-deployment.yaml +++ b/components/manifests/base/core/backend-deployment.yaml @@ -147,6 +147,12 @@ spec: key: GOOGLE_APPLICATION_CREDENTIALS optional: true # LDAP configuration (optional - autocomplete gated by ldap.autocomplete.enabled feature flag) + - name: LDAP_SRV_DOMAIN + valueFrom: + configMapKeyRef: + name: ldap-config + key: LDAP_SRV_DOMAIN + optional: true - name: LDAP_URL valueFrom: configMapKeyRef: @@ -165,11 +171,23 @@ spec: name: ldap-config key: LDAP_GROUP_BASE_DN optional: true - - name: LDAP_SKIP_TLS_VERIFY + - name: LDAP_BIND_DN valueFrom: configMapKeyRef: name: ldap-config - key: LDAP_SKIP_TLS_VERIFY + key: LDAP_BIND_DN + optional: true + - name: LDAP_BIND_PASSWORD + valueFrom: + secretKeyRef: + name: ldap-credentials + key: LDAP_BIND_PASSWORD + optional: true + - name: LDAP_CA_CERT_PATH + valueFrom: + configMapKeyRef: + name: ldap-config + key: LDAP_CA_CERT_PATH optional: true # Unleash feature flags (optional - all flags disabled when not set) - name: UNLEASH_URL @@ -246,6 +264,10 @@ spec: - name: agent-registry mountPath: /config/registry readOnly: true + # Red Hat Root CA cert + - name: ldap-ca-cert + mountPath: /etc/pki/custom-ca + readOnly: true volumes: - name: backend-state persistentVolumeClaim: @@ -272,6 +294,11 @@ spec: configMap: name: ambient-agent-registry optional: true # Don't fail if ConfigMap not yet created + # Red Hat Root CA cert + - name: ldap-ca-cert + configMap: + name: ldap-ca-cert + optional: true --- apiVersion: v1 diff --git a/components/manifests/overlays/kind/kustomization.yaml b/components/manifests/overlays/kind/kustomization.yaml index 2f7aa3f8f..57a075a2f 100644 --- a/components/manifests/overlays/kind/kustomization.yaml +++ b/components/manifests/overlays/kind/kustomization.yaml @@ -15,6 +15,8 @@ resources: - unleash-credentials.yaml # PostgreSQL init scripts for database creation (kind only) - postgresql-init-scripts.yaml +- ldap-config.yaml +- ldap-credentials.yaml # Patches for e2e environment patches: diff --git a/components/manifests/overlays/kind/ldap-config.yaml b/components/manifests/overlays/kind/ldap-config.yaml new file mode 100644 index 000000000..8b2ba680a --- /dev/null +++ b/components/manifests/overlays/kind/ldap-config.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: ldap-config + labels: + app: backend-api +data: + LDAP_URL: "ldaps://ipa.corp.redhat.com" + LDAP_BASE_DN: "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_GROUP_BASE_DN: "cn=groups,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_BIND_DN: "uid=ambient-code-platform,cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_CA_CERT_PATH: "/etc/pki/custom-ca/rh-it-root-ca.pem" diff --git a/components/manifests/overlays/kind/ldap-credentials.yaml b/components/manifests/overlays/kind/ldap-credentials.yaml new file mode 100644 index 000000000..7dd604b4d --- /dev/null +++ b/components/manifests/overlays/kind/ldap-credentials.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Secret +metadata: + name: ldap-credentials + labels: + app: backend-api +type: Opaque +stringData: + LDAP_BIND_PASSWORD: "REPLACE_WITH_ACTUAL_PASSWORD" diff --git a/components/manifests/overlays/local-dev/kustomization.yaml b/components/manifests/overlays/local-dev/kustomization.yaml index 05ef9ddd5..ec2984b0e 100644 --- a/components/manifests/overlays/local-dev/kustomization.yaml +++ b/components/manifests/overlays/local-dev/kustomization.yaml @@ -13,6 +13,8 @@ resources: - backend-route.yaml - frontend-route.yaml - operator-config-crc.yaml +- ldap-config.yaml +- ldap-credentials.yaml - unleash-credentials.yaml - unleash-route.yaml diff --git a/components/manifests/overlays/local-dev/ldap-config.yaml b/components/manifests/overlays/local-dev/ldap-config.yaml new file mode 100644 index 000000000..23aae125b --- /dev/null +++ b/components/manifests/overlays/local-dev/ldap-config.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: ldap-config + labels: + app: backend-api +data: + LDAP_SRV_DOMAIN: "ipa.redhat.com" + LDAP_URL: "ldaps://ipa.corp.redhat.com" + LDAP_BASE_DN: "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_GROUP_BASE_DN: "cn=groups,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_BIND_DN: "uid=ambient-code-platform,cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_CA_CERT_PATH: "/etc/pki/custom-ca/rh-it-root-ca.pem" diff --git a/components/manifests/overlays/local-dev/ldap-credentials.yaml b/components/manifests/overlays/local-dev/ldap-credentials.yaml new file mode 100644 index 000000000..7dd604b4d --- /dev/null +++ b/components/manifests/overlays/local-dev/ldap-credentials.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Secret +metadata: + name: ldap-credentials + labels: + app: backend-api +type: Opaque +stringData: + LDAP_BIND_PASSWORD: "REPLACE_WITH_ACTUAL_PASSWORD" diff --git a/components/manifests/overlays/production/ldap-config.yaml b/components/manifests/overlays/production/ldap-config.yaml index a1bec98fa..23aae125b 100644 --- a/components/manifests/overlays/production/ldap-config.yaml +++ b/components/manifests/overlays/production/ldap-config.yaml @@ -5,6 +5,9 @@ metadata: labels: app: backend-api data: - LDAP_URL: "ldaps://ldap.corp.redhat.com" - LDAP_BASE_DN: "ou=users,dc=redhat,dc=com" - LDAP_SKIP_TLS_VERIFY: "true" + LDAP_SRV_DOMAIN: "ipa.redhat.com" + LDAP_URL: "ldaps://ipa.corp.redhat.com" + LDAP_BASE_DN: "cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_GROUP_BASE_DN: "cn=groups,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_BIND_DN: "uid=ambient-code-platform,cn=users,cn=accounts,dc=ipa,dc=redhat,dc=com" + LDAP_CA_CERT_PATH: "/etc/pki/custom-ca/rh-it-root-ca.pem" diff --git a/components/manifests/overlays/production/ldap-credentials.yaml b/components/manifests/overlays/production/ldap-credentials.yaml new file mode 100644 index 000000000..7dd604b4d --- /dev/null +++ b/components/manifests/overlays/production/ldap-credentials.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Secret +metadata: + name: ldap-credentials + labels: + app: backend-api +type: Opaque +stringData: + LDAP_BIND_PASSWORD: "REPLACE_WITH_ACTUAL_PASSWORD"