From 348b04ecb4e1902ba311ffc45a7c61775fcc150f Mon Sep 17 00:00:00 2001 From: Michel Vocks Date: Fri, 14 Feb 2020 19:26:30 +0100 Subject: [PATCH] Fix ldap client upndomain (#8333) --- builtin/credential/ldap/backend.go | 2 +- builtin/credential/ldap/backend_test.go | 68 ++++++++++++++++++- helper/testhelpers/ldap/ldaphelper.go | 6 +- sdk/helper/ldaputil/client.go | 13 ++-- .../vault/sdk/helper/ldaputil/client.go | 13 ++-- 5 files changed, 90 insertions(+), 12 deletions(-) diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index d8a6d9d72935..5d2b1d23974c 100644 --- a/builtin/credential/ldap/backend.go +++ b/builtin/credential/ldap/backend.go @@ -127,7 +127,7 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri } } - userDN, err := ldapClient.GetUserDN(cfg.ConfigEntry, c, userBindDN) + userDN, err := ldapClient.GetUserDN(cfg.ConfigEntry, c, userBindDN, username) if err != nil { return nil, logical.ErrorResponse(err.Error()), nil, nil } diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 3119792abee8..9142d0142d56 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -8,7 +8,9 @@ import ( "testing" "time" + goldap "github.com/go-ldap/ldap/v3" "github.com/go-test/deep" + hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/testhelpers/ldap" logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical" @@ -397,7 +399,10 @@ func factory(t *testing.T) logical.Backend { defaultLeaseTTLVal := time.Hour * 24 maxLeaseTTLVal := time.Hour * 24 * 32 b, err := Factory(context.Background(), &logical.BackendConfig{ - Logger: nil, + Logger: hclog.New(&hclog.LoggerOptions{ + Name: "FactoryLogger", + Level: hclog.Debug, + }), System: &logical.StaticSystemView{ DefaultLeaseTTLVal: defaultLeaseTTLVal, MaxLeaseTTLVal: maxLeaseTTLVal, @@ -495,6 +500,66 @@ func TestBackend_basic_authbind(t *testing.T) { }) } +func TestBackend_basic_authbind_upndomain(t *testing.T) { + b := factory(t) + cleanup, cfg := ldap.PrepareTestContainer(t, "latest") + defer cleanup() + cfg.UPNDomain = "planetexpress.com" + + // Setup connection + client := &ldaputil.Client{ + Logger: hclog.New(&hclog.LoggerOptions{ + Name: "LDAPAuthTest", + Level: hclog.Debug, + }), + LDAP: ldaputil.NewLDAP(), + } + conn, err := client.DialLDAP(cfg) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if err := conn.Bind("cn=admin,cn=config", cfg.BindPassword); err != nil { + t.Fatal(err) + } + + // Add userPrincipalName attribute type + userPrincipleNameTypeReq := goldap.NewModifyRequest("cn={0}core,cn=schema,cn=config", nil) + userPrincipleNameTypeReq.Add("olcAttributetypes", []string{"( 2.25.247072656268950430024439664556757516066 NAME ( 'userPrincipalName' ) SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 EQUALITY caseIgnoreMatch SINGLE-VALUE )"}) + if err := conn.Modify(userPrincipleNameTypeReq); err != nil { + t.Fatal(err) + } + + // Add new object class + userPrincipleNameObjClassReq := goldap.NewModifyRequest("cn={0}core,cn=schema,cn=config", nil) + userPrincipleNameObjClassReq.Add("olcObjectClasses", []string{"( 1.2.840.113556.6.2.6 NAME 'PrincipalNameClass' AUXILIARY MAY ( userPrincipalName ) )"}) + if err := conn.Modify(userPrincipleNameObjClassReq); err != nil { + t.Fatal(err) + } + + // Re-authenticate with the binddn user + if err := conn.Bind(cfg.BindDN, cfg.BindPassword); err != nil { + t.Fatal(err) + } + + // Modify professor user and add userPrincipalName attribute + profDN := "cn=Hubert J. Farnsworth,ou=people,dc=planetexpress,dc=com" + modifyUserReq := goldap.NewModifyRequest(profDN, nil) + modifyUserReq.Add("objectClass", []string{"PrincipalNameClass"}) + modifyUserReq.Add("userPrincipalName", []string{"professor@planetexpress.com"}) + if err := conn.Modify(modifyUserReq); err != nil { + t.Fatal(err) + } + + logicaltest.Test(t, logicaltest.TestCase{ + CredentialBackend: b, + Steps: []logicaltest.TestStep{ + testAccStepConfigUrlWithAuthBind(t, cfg), + testAccStepLoginNoAttachedPolicies(t, "professor", "professor"), + }, + }) +} + func TestBackend_basic_discover(t *testing.T) { b := factory(t) cleanup, cfg := ldap.PrepareTestContainer(t, "latest") @@ -626,6 +691,7 @@ func testAccStepConfigUrlWithAuthBind(t *testing.T, cfg *ldaputil.ConfigEntry) l "groupattr": cfg.GroupAttr, "binddn": cfg.BindDN, "bindpass": cfg.BindPassword, + "upndomain": cfg.UPNDomain, "case_sensitive_names": true, "token_policies": "abc,xyz", "request_timeout": cfg.RequestTimeout, diff --git a/helper/testhelpers/ldap/ldaphelper.go b/helper/testhelpers/ldap/ldaphelper.go index 5571565caa1d..577b7e48fe1c 100644 --- a/helper/testhelpers/ldap/ldaphelper.go +++ b/helper/testhelpers/ldap/ldaphelper.go @@ -4,7 +4,7 @@ import ( "fmt" "testing" - "github.com/hashicorp/go-hclog" + hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/helper/testhelpers/docker" "github.com/hashicorp/vault/sdk/helper/ldaputil" "github.com/ory/dockertest" @@ -17,7 +17,9 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld } dockerOptions := &dockertest.RunOptions{ - Repository: "rroemhild/test-openldap", + // Currently set to "michelvocks" until https://github.com/rroemhild/docker-test-openldap/pull/14 + // has been merged. + Repository: "michelvocks/docker-test-openldap", Tag: version, Privileged: true, //Env: []string{"LDAP_DEBUG_LEVEL=384"}, diff --git a/sdk/helper/ldaputil/client.go b/sdk/helper/ldaputil/client.go index 34e954359b84..fe0fd670f1eb 100644 --- a/sdk/helper/ldaputil/client.go +++ b/sdk/helper/ldaputil/client.go @@ -120,6 +120,10 @@ func (c *Client) GetUserBindDN(cfg *ConfigEntry, conn Connection, username strin } filter := fmt.Sprintf("(%s=%s)", cfg.UserAttr, ldap.EscapeFilter(username)) + if cfg.UPNDomain != "" { + filter = fmt.Sprintf("(userPrincipalName=%s@%s)", EscapeLDAPValue(username), cfg.UPNDomain) + } + if c.Logger.IsDebug() { c.Logger.Debug("discovering user", "userdn", cfg.UserDN, "filter", filter) } @@ -150,11 +154,11 @@ func (c *Client) GetUserBindDN(cfg *ConfigEntry, conn Connection, username strin /* * Returns the DN of the object representing the authenticated user. */ -func (c *Client) GetUserDN(cfg *ConfigEntry, conn Connection, bindDN string) (string, error) { +func (c *Client) GetUserDN(cfg *ConfigEntry, conn Connection, bindDN, username string) (string, error) { userDN := "" if cfg.UPNDomain != "" { // Find the distinguished name for the user if userPrincipalName used for login - filter := fmt.Sprintf("(userPrincipalName=%s)", ldap.EscapeFilter(bindDN)) + filter := fmt.Sprintf("(userPrincipalName=%s@%s)", EscapeLDAPValue(username), cfg.UPNDomain) if c.Logger.IsDebug() { c.Logger.Debug("searching upn", "userdn", cfg.UserDN, "filter", filter) } @@ -167,9 +171,10 @@ func (c *Client) GetUserDN(cfg *ConfigEntry, conn Connection, bindDN string) (st if err != nil { return userDN, errwrap.Wrapf("LDAP search failed for detecting user: {{err}}", err) } - for _, e := range result.Entries { - userDN = e.DN + if len(result.Entries) != 1 { + return userDN, fmt.Errorf("LDAP search for userdn 0 or not unique") } + userDN = result.Entries[0].DN } else { userDN = bindDN } diff --git a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go index 34e954359b84..fe0fd670f1eb 100644 --- a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go +++ b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go @@ -120,6 +120,10 @@ func (c *Client) GetUserBindDN(cfg *ConfigEntry, conn Connection, username strin } filter := fmt.Sprintf("(%s=%s)", cfg.UserAttr, ldap.EscapeFilter(username)) + if cfg.UPNDomain != "" { + filter = fmt.Sprintf("(userPrincipalName=%s@%s)", EscapeLDAPValue(username), cfg.UPNDomain) + } + if c.Logger.IsDebug() { c.Logger.Debug("discovering user", "userdn", cfg.UserDN, "filter", filter) } @@ -150,11 +154,11 @@ func (c *Client) GetUserBindDN(cfg *ConfigEntry, conn Connection, username strin /* * Returns the DN of the object representing the authenticated user. */ -func (c *Client) GetUserDN(cfg *ConfigEntry, conn Connection, bindDN string) (string, error) { +func (c *Client) GetUserDN(cfg *ConfigEntry, conn Connection, bindDN, username string) (string, error) { userDN := "" if cfg.UPNDomain != "" { // Find the distinguished name for the user if userPrincipalName used for login - filter := fmt.Sprintf("(userPrincipalName=%s)", ldap.EscapeFilter(bindDN)) + filter := fmt.Sprintf("(userPrincipalName=%s@%s)", EscapeLDAPValue(username), cfg.UPNDomain) if c.Logger.IsDebug() { c.Logger.Debug("searching upn", "userdn", cfg.UserDN, "filter", filter) } @@ -167,9 +171,10 @@ func (c *Client) GetUserDN(cfg *ConfigEntry, conn Connection, bindDN string) (st if err != nil { return userDN, errwrap.Wrapf("LDAP search failed for detecting user: {{err}}", err) } - for _, e := range result.Entries { - userDN = e.DN + if len(result.Entries) != 1 { + return userDN, fmt.Errorf("LDAP search for userdn 0 or not unique") } + userDN = result.Entries[0].DN } else { userDN = bindDN }