Fix ldap client upndomain (#8333)

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
 	}

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,

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"},

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
 	}