fix (ldap): support only updating the bind-dn and/or bind-password (#3511)

* fix (ldap): support only updating the bind-dn and/or bind-password

* chore: changelog entry for PR #3511

CHANGELOG.md

@@ -4,6 +4,10 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
 
 ## Next
 
+### Bug Fixes
+
+* LDAP auth methods: allow bind-dn and bind-password to be updated independently. ([PR](https://github.com/hashicorp/boundary/pull/3511))
+
 ## 0.13.1 (2023/07/10)
 
 ### New and Improved

internal/auth/ldap/auth_method_test.go

@@ -127,16 +127,6 @@ func TestNewAuthMethod(t *testing.T) {
 			wantErrCode:     errors.InvalidParameter,
 			wantErrContains: `https scheme in url "https://alice.com" is not either ldap or ldaps`,
 		},
-		{
-			name:            "opt-error",
-			ctx:             testCtx,
-			scopeId:         "global",
-			urls:            TestConvertToUrls(t, "ldaps://alice.com"),
-			opts:            []Option{WithBindCredential(testCtx, "dn", "")},
-			wantErr:         true,
-			wantErrCode:     errors.InvalidParameter,
-			wantErrContains: "ldap.WithBindCredential: missing password",
-		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

internal/auth/ldap/options.go

@@ -250,10 +250,6 @@ func WithBindCredential(ctx context.Context, dn, password string) Option {
 		switch {
 		case dn == "" && password == "":
 			return errors.New(ctx, errors.InvalidParameter, op, "missing both dn and password")
-		case dn != "" && password == "":
-			return errors.New(ctx, errors.InvalidParameter, op, "missing password")
-		case dn == "" && password != "":
-			return errors.New(ctx, errors.InvalidParameter, op, "missing dn")
 		}
 		o.withBindDn = dn
 		o.withBindPassword = password

internal/auth/ldap/options_test.go

@@ -184,18 +184,18 @@ func Test_getOpts(t *testing.T) {
 		testOpts.withBindPassword = "password"
 		assert.Equal(opts, testOpts)
 	})
-	t.Run("WithBindCredential-missing-dn", func(t *testing.T) {
+	t.Run("WithBindCredential-only-password", func(t *testing.T) {
 		assert := assert.New(t)
 		opts, err := getOpts(WithBindCredential(testCtx, "", "password"))
-		require.Error(t, err)
+		require.NoError(t, err)
 		assert.Empty(opts.withBindDn)
-		assert.Empty(opts.withBindPassword)
+		assert.NotEmpty(opts.withBindPassword)
 	})
-	t.Run("WithBindCredential-missing-password", func(t *testing.T) {
+	t.Run("WithBindCredential-only-dn", func(t *testing.T) {
 		assert := assert.New(t)
 		opts, err := getOpts(WithBindCredential(testCtx, "dn", ""))
-		require.Error(t, err)
-		assert.Empty(opts.withBindDn)
+		require.NoError(t, err)
+		assert.NotEmpty(opts.withBindDn)
 		assert.Empty(opts.withBindPassword)
 	})
 	t.Run("WithBindCredential-missing-dn-and-password", func(t *testing.T) {

internal/auth/ldap/repository_auth_method_update.go

@@ -297,7 +297,7 @@ func (r *Repository) UpdateAuthMethod(ctx context.Context, am *AuthMethod, versi
 				return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op)
 			}
 			if err := bc.encrypt(ctx, dbWrapper); err != nil {
-				return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op)
+				return nil, db.NoRowsAffected, errors.Wrap(ctx, err, op, errors.WithMsg("error wrapping updated bind credential"))
 			}
 			addBindCred = bc
 		}

internal/auth/ldap/repository_auth_method_update_test.go

@@ -494,6 +494,62 @@ func TestRepository_UpdateAuthMethod(t *testing.T) {
 				return am
 			},
 		},
+		{
+			name: "update-just-binddn",
+			ctx:  testCtx,
+			repo: testRepo,
+			setup: func() *AuthMethod {
+				return TestAuthMethod(t,
+					testConn, databaseWrapper,
+					org.PublicId,
+					[]string{"ldaps://ldap1", "ldap://ldap2"},
+					WithBindCredential(testCtx, "orig-bind-dn", "orig-bind-password"),
+				)
+			},
+			updateWith: func(orig *AuthMethod) *AuthMethod {
+				am := AllocAuthMethod()
+				am.PublicId = orig.PublicId
+				am.BindDn = "bind-dn"
+				return &am
+			},
+			fieldMasks: []string{
+				BindDnField,
+			},
+			version: 1,
+			want: func(orig, updateWith *AuthMethod) *AuthMethod {
+				am := orig.clone()
+				am.BindDn = updateWith.BindDn
+				return am
+			},
+		},
+		{
+			name: "update-just-bind-password",
+			ctx:  testCtx,
+			repo: testRepo,
+			setup: func() *AuthMethod {
+				return TestAuthMethod(t,
+					testConn, databaseWrapper,
+					org.PublicId,
+					[]string{"ldaps://ldap1", "ldap://ldap2"},
+					WithBindCredential(testCtx, "orig-bind-dn", "orig-bind-password"),
+				)
+			},
+			updateWith: func(orig *AuthMethod) *AuthMethod {
+				am := AllocAuthMethod()
+				am.PublicId = orig.PublicId
+				am.BindPassword = "bind-password"
+				return &am
+			},
+			fieldMasks: []string{
+				BindPasswordField,
+			},
+			version: 1,
+			want: func(orig, updateWith *AuthMethod) *AuthMethod {
+				am := orig.clone()
+				am.BindPassword = updateWith.BindPassword
+				return am
+			},
+		},
 		{
 			name: "missing-auth-method",
 			ctx:  testCtx,

internal/daemon/controller/handlers/authmethods/authmethod_service_test.go

@@ -1232,8 +1232,7 @@ func TestCreate(t *testing.T) {
 					},
 				},
 			}},
-			err:         handlers.ApiErrorWithCode(codes.InvalidArgument),
-			errContains: "attributes.bind_password is missing required attributes.bind_dn field",
+			errContains: "missing dn",
 		},
 		{
 			name: "ldap-auth-method-missing-bind-password",
@@ -1247,8 +1246,7 @@ func TestCreate(t *testing.T) {
 					},
 				},
 			}},
-			err:         handlers.ApiErrorWithCode(codes.InvalidArgument),
-			errContains: "attributes.bind_dn is missing required attributes.bind_password field",
+			errContains: "missing password",
 		},
 		{
 			name: "ldap-auth-method-invalid-client-cert",
@@ -1352,14 +1350,20 @@ func TestCreate(t *testing.T) {
 			require.NoError(err, "Error when getting new auth_method service.")
 
 			got, gErr := s.CreateAuthMethod(requestauth.DisabledAuthTestContext(iamRepoFn, tc.req.GetItem().GetScopeId()), tc.req)
-			if tc.err != nil {
+			switch {
+			case tc.err != nil:
 				require.Error(gErr)
 				assert.True(errors.Is(gErr, tc.err), "CreateAuthMethod(%+v) got error %v, wanted %v", tc.req, gErr, tc.err)
 				if tc.errContains != "" {
 					assert.Contains(gErr.Error(), tc.errContains)
 				}
 				return
+			case tc.errContains != "":
+				require.Error(gErr)
+				assert.Contains(gErr.Error(), tc.errContains)
+				return
 			}
+
 			require.NoError(gErr)
 			if tc.res == nil {
 				require.Nil(got)

internal/daemon/controller/handlers/authmethods/ldap.go

@@ -299,12 +299,6 @@ func validateLdapAttributes(ctx context.Context, attrs *pb.LdapAuthMethodAttribu
 			badFields[certificatesField] = fmt.Sprintf("invalid %s: %s", certificatesField, err.Error())
 		}
 	}
-	if attrs.GetBindDn().GetValue() != "" && attrs.GetBindPassword().GetValue() == "" {
-		badFields[bindPasswordField] = fmt.Sprintf("%s is missing required %s field", bindDnField, bindPasswordField)
-	}
-	if attrs.GetBindPassword().GetValue() != "" && attrs.GetBindDn().GetValue() == "" {
-		badFields[bindDnField] = fmt.Sprintf("%s is missing required %s field", bindPasswordField, bindDnField)
-	}
 	if attrs.GetClientCertificate().GetValue() != "" && attrs.GetClientCertificateKey().GetValue() == "" {
 		badFields[clientCertificateKeyField] = fmt.Sprintf("%s is missing required %s field", clientCertificateField, clientCertificateKeyField)
 	}

internal/daemon/controller/handlers/authmethods/ldap_test.go

@@ -81,14 +81,17 @@ func Test_UpdateLdap(t *testing.T) {
 		},
 	}
 
-	freshAuthMethod := func(t *testing.T) (*pb.AuthMethod, func()) {
+	freshAuthMethod := func(t *testing.T, attrs *pb.AuthMethod_LdapAuthMethodsAttributes) (*pb.AuthMethod, func()) {
+		if attrs == nil {
+			attrs = defaultAttributes
+		}
 		ctx := auth.DisabledAuthTestContext(iamRepoFn, o.GetPublicId())
 		am, err := tested.CreateAuthMethod(ctx, &pbs.CreateAuthMethodRequest{Item: &pb.AuthMethod{
 			ScopeId:     o.GetPublicId(),
 			Name:        wrapperspb.String("default"),
 			Description: wrapperspb.String("default"),
 			Type:        ldap.Subtype.String(),
-			Attrs:       defaultAttributes,
+			Attrs:       attrs,
 		}})
 		require.NoError(t, err)
 
@@ -103,12 +106,13 @@ func Test_UpdateLdap(t *testing.T) {
 	_, testEncodedCert := ldap.TestGenerateCA(t, "localhost")
 
 	tests := []struct {
-		name        string
-		req         *pbs.UpdateAuthMethodRequest
-		res         *pbs.UpdateAuthMethodResponse
-		err         error
-		errContains string
-		wantErr     bool
+		name             string
+		newAttrsOverride *pb.AuthMethod_LdapAuthMethodsAttributes
+		req              *pbs.UpdateAuthMethodRequest
+		res              *pbs.UpdateAuthMethodResponse
+		err              error
+		errContains      string
+		wantErr          bool
 	}{
 		{
 			name: "update-an-existing-auth-method",
@@ -349,6 +353,128 @@ func Test_UpdateLdap(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "update-only-bind-dn",
+			newAttrsOverride: &pb.AuthMethod_LdapAuthMethodsAttributes{
+				LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+					Urls:         []string{"ldaps://ldap1"},
+					State:        "active-private",
+					BindDn:       &wrapperspb.StringValue{Value: "bind-dn"},
+					BindPassword: &wrapperspb.StringValue{Value: "bind-password"},
+				},
+			},
+			req: &pbs.UpdateAuthMethodRequest{
+				UpdateMask: &field_mask.FieldMask{
+					Paths: []string{"attributes.bind_dn"},
+				},
+				Item: &pb.AuthMethod{
+					Attrs: &pb.AuthMethod_LdapAuthMethodsAttributes{
+						LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+							BindDn: &wrapperspb.StringValue{Value: "updated"},
+						},
+					},
+				},
+			},
+			res: &pbs.UpdateAuthMethodResponse{
+				Item: &pb.AuthMethod{
+					ScopeId:     o.GetPublicId(),
+					Version:     2,
+					Name:        &wrapperspb.StringValue{Value: "default"},
+					Description: &wrapperspb.StringValue{Value: "default"},
+					Type:        ldap.Subtype.String(),
+					Attrs: &pb.AuthMethod_LdapAuthMethodsAttributes{
+						LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+							Urls:   []string{"ldaps://ldap1"},
+							State:  "active-private",
+							BindDn: &wrapperspb.StringValue{Value: "updated"},
+							// note: BindPassword is never returned (an HMAC'd
+							// value is returned in a separate attribute)
+						},
+					},
+					Scope:                       defaultScopeInfo,
+					AuthorizedActions:           ldapAuthorizedActions,
+					AuthorizedCollectionActions: authorizedCollectionActions,
+				},
+			},
+		},
+		{
+			name: "err-update-only-bind-dn-with-no-orig-bind-password",
+			req: &pbs.UpdateAuthMethodRequest{
+				UpdateMask: &field_mask.FieldMask{
+					Paths: []string{"attributes.bind_dn"},
+				},
+				Item: &pb.AuthMethod{
+					Attrs: &pb.AuthMethod_LdapAuthMethodsAttributes{
+						LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+							BindDn: &wrapperspb.StringValue{Value: "updated"},
+						},
+					},
+				},
+			},
+			err:         handlers.ApiErrorWithCode(codes.InvalidArgument),
+			errContains: "missing password",
+		},
+		{
+			name: "update-only-bind-password",
+			newAttrsOverride: &pb.AuthMethod_LdapAuthMethodsAttributes{
+				LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+					Urls:         []string{"ldaps://ldap1"},
+					State:        "active-private",
+					BindDn:       &wrapperspb.StringValue{Value: "bind-dn"},
+					BindPassword: &wrapperspb.StringValue{Value: "bind-password"},
+				},
+			},
+			req: &pbs.UpdateAuthMethodRequest{
+				UpdateMask: &field_mask.FieldMask{
+					Paths: []string{"attributes.bind_password"},
+				},
+				Item: &pb.AuthMethod{
+					Attrs: &pb.AuthMethod_LdapAuthMethodsAttributes{
+						LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+							BindPassword: &wrapperspb.StringValue{Value: "updated"},
+						},
+					},
+				},
+			},
+			res: &pbs.UpdateAuthMethodResponse{
+				Item: &pb.AuthMethod{
+					ScopeId:     o.GetPublicId(),
+					Version:     2,
+					Name:        &wrapperspb.StringValue{Value: "default"},
+					Description: &wrapperspb.StringValue{Value: "default"},
+					Type:        ldap.Subtype.String(),
+					Attrs: &pb.AuthMethod_LdapAuthMethodsAttributes{
+						LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+							Urls:   []string{"ldaps://ldap1"},
+							State:  "active-private",
+							BindDn: &wrapperspb.StringValue{Value: "bind-dn"},
+							// note: BindPassword is never returned (an HMAC'd
+							// value is returned in a separate attribute)
+						},
+					},
+					Scope:                       defaultScopeInfo,
+					AuthorizedActions:           ldapAuthorizedActions,
+					AuthorizedCollectionActions: authorizedCollectionActions,
+				},
+			},
+		},
+		{
+			name: "err-update-only-bind-password-with-no-orig-bind-dn",
+			req: &pbs.UpdateAuthMethodRequest{
+				UpdateMask: &field_mask.FieldMask{
+					Paths: []string{"attributes.bind_password"},
+				},
+				Item: &pb.AuthMethod{
+					Attrs: &pb.AuthMethod_LdapAuthMethodsAttributes{
+						LdapAuthMethodsAttributes: &pb.LdapAuthMethodAttributes{
+							BindPassword: &wrapperspb.StringValue{Value: "updated"},
+						},
+					},
+				},
+			},
+			err:         handlers.ApiErrorWithCode(codes.InvalidArgument),
+			errContains: "missing dn",
+		},
 		{
 			name: "update-a-non-existent-auth-method",
 			req: &pbs.UpdateAuthMethodRequest{
@@ -689,7 +815,7 @@ func Test_UpdateLdap(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			assert, require := assert.New(t), require.New(t)
-			am, cleanup := freshAuthMethod(t)
+			am, cleanup := freshAuthMethod(t, tc.newAttrsOverride)
 			defer cleanup()
 
 			tc.req.Item.Version = am.GetVersion()