[LDAP] move attributeUnchangedSinceLogin from upstreamldap to actived…

…irectoryupstreamwatcher
vmware-tanzu · Sep 6, 2023 · cd91edf · cd91edf
1 parent 8fd55a1
commit cd91edf
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 110 deletions.
diff --git a/...ller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/...ller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go
@@ -6,6 +6,7 @@ package activedirectoryupstreamwatcher
 
 import (
 	"context"
+	"encoding/base64"
 	"fmt"
 	"regexp"
 	"strconv"
@@ -344,7 +345,7 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context,
 			"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"),
 		},
 		RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-			pwdLastSetAttribute:                 upstreamldap.AttributeUnchangedSinceLogin(pwdLastSetAttribute),
+			pwdLastSetAttribute:                 attributeUnchangedSinceLogin(pwdLastSetAttribute),
 			userAccountControlAttribute:         validUserAccountControl,
 			userAccountControlComputedAttribute: validComputedUserAccountControl,
 		},
@@ -470,3 +471,20 @@ var validComputedUserAccountControl = func(entry *ldap.Entry, _ provider.Refresh
 	}
 	return nil
 }
+
+//nolint:gochecknoglobals // this needs to be a global variable so that tests can check pointer equality
+var attributeUnchangedSinceLogin = func(attribute string) func(*ldap.Entry, provider.RefreshAttributes) error {
+	return func(entry *ldap.Entry, storedAttributes provider.RefreshAttributes) error {
+		prevAttributeValue := storedAttributes.AdditionalAttributes[attribute]
+		newValues := entry.GetRawAttributeValues(attribute)
+
+		if len(newValues) != 1 {
+			return fmt.Errorf(`expected to find 1 value for %q attribute, but found %d`, attribute, len(newValues))
+		}
+		encodedNewValue := base64.RawURLEncoding.EncodeToString(newValues[0])
+		if prevAttributeValue != encodedNewValue {
+			return fmt.Errorf(`value for attribute %q has changed since initial value at login`, attribute)
+		}
+		return nil
+	}
+}
diff --git a/...supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/...supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go
@@ -230,7 +230,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 		},
 		UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 		RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-			"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+			"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 			"userAccountControl":                 validUserAccountControl,
 			"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 		},
@@ -573,7 +573,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -643,7 +643,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -716,7 +716,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -796,7 +796,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -860,7 +860,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1011,7 +1011,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1161,7 +1161,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					}},
@@ -1233,7 +1233,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1500,7 +1500,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					UIDAttributeParsingOverrides:   map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": groupSAMAccountNameWithDomainSuffix},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1560,7 +1560,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1624,7 +1624,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1688,7 +1688,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1900,7 +1900,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -1963,7 +1963,7 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 					},
 					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
+						"pwdLastSet":                         attributeUnchangedSinceLogin("pwdLastSet"),
 						"userAccountControl":                 validUserAccountControl,
 						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
@@ -2426,3 +2426,77 @@ func TestValidComputedUserAccountControl(t *testing.T) {
 		})
 	}
 }
+
+func TestAttributeUnchangedSinceLogin(t *testing.T) {
+	initialVal := "some-attribute-value"
+	changedVal := "some-different-attribute-value"
+	attributeName := "some-attribute-name"
+	tests := []struct {
+		name       string
+		entry      *ldap.Entry
+		wantResult bool
+		wantErr    string
+	}{
+		{
+			name: "happy path where value has not changed since login",
+			entry: &ldap.Entry{
+				DN: "some-dn",
+				Attributes: []*ldap.EntryAttribute{
+					{
+						Name:       attributeName,
+						Values:     []string{initialVal},
+						ByteValues: [][]byte{[]byte(initialVal)},
+					},
+				},
+			},
+		},
+		{
+			name: "password has been reset since login",
+			entry: &ldap.Entry{
+				DN: "some-dn",
+				Attributes: []*ldap.EntryAttribute{
+					{
+						Name:       attributeName,
+						Values:     []string{changedVal},
+						ByteValues: [][]byte{[]byte(changedVal)},
+					},
+				},
+			},
+			wantErr: "value for attribute \"some-attribute-name\" has changed since initial value at login",
+		},
+		{
+			name: "no value for attribute attribute",
+			entry: &ldap.Entry{
+				DN:         "some-dn",
+				Attributes: []*ldap.EntryAttribute{},
+			},
+			wantErr: "expected to find 1 value for \"some-attribute-name\" attribute, but found 0",
+		},
+		{
+			name: "too many values for attribute",
+			entry: &ldap.Entry{
+				DN: "some-dn",
+				Attributes: []*ldap.EntryAttribute{
+					{
+						Name:       attributeName,
+						ByteValues: [][]byte{[]byte("val1"), []byte("val2")},
+					},
+				},
+			},
+			wantErr: "expected to find 1 value for \"some-attribute-name\" attribute, but found 2",
+		},
+	}
+	for _, test := range tests {
+		tt := test
+		t.Run(tt.name, func(t *testing.T) {
+			initialValRawEncoded := base64.RawURLEncoding.EncodeToString([]byte(initialVal))
+			err := attributeUnchangedSinceLogin(attributeName)(tt.entry, provider.RefreshAttributes{AdditionalAttributes: map[string]string{attributeName: initialValRawEncoded}})
+			if tt.wantErr != "" {
+				require.Error(t, err)
+				require.Equal(t, tt.wantErr, err.Error())
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go
@@ -870,20 +870,3 @@ func (p *Provider) traceRefreshFailure(t *trace.Trace, err error) {
 		trace.Field{Key: "reason", Value: err.Error()},
 	)
 }
-
-//nolint:gochecknoglobals // this needs to be a global variable so that tests can check pointer equality
-var AttributeUnchangedSinceLogin = func(attribute string) func(*ldap.Entry, provider.RefreshAttributes) error {
-	return func(entry *ldap.Entry, storedAttributes provider.RefreshAttributes) error {
-		prevAttributeValue := storedAttributes.AdditionalAttributes[attribute]
-		newValues := entry.GetRawAttributeValues(attribute)
-
-		if len(newValues) != 1 {
-			return fmt.Errorf(`expected to find 1 value for %q attribute, but found %d`, attribute, len(newValues))
-		}
-		encodedNewValue := base64.RawURLEncoding.EncodeToString(newValues[0])
-		if prevAttributeValue != encodedNewValue {
-			return fmt.Errorf(`value for attribute %q has changed since initial value at login`, attribute)
-		}
-		return nil
-	}
-}
diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go
@@ -1576,7 +1576,7 @@ func TestUpstreamRefresh(t *testing.T) {
 				GroupNameAttribute: testGroupSearchGroupNameAttribute,
 			},
 			RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
-				pwdLastSetAttribute: AttributeUnchangedSinceLogin(pwdLastSetAttribute),
+				pwdLastSetAttribute: func(*ldap.Entry, provider.RefreshAttributes) error { return nil },
 			},
 		}
 		if editFunc != nil {
@@ -2200,8 +2200,14 @@ func TestUpstreamRefresh(t *testing.T) {
 			wantErr: "found 2 values for attribute \"some-upstream-uid-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result",
 		},
 		{
-			name:           "search result has a changed pwdLastSet value",
-			providerConfig: providerConfig(nil),
+			name: "search result has a changed pwdLastSet value",
+			providerConfig: providerConfig(func(p *ProviderConfig) {
+				p.RefreshAttributeChecks = map[string]func(*ldap.Entry, provider.RefreshAttributes) error{
+					pwdLastSetAttribute: func(*ldap.Entry, provider.RefreshAttributes) error {
+						return errors.New(`value for attribute "pwdLastSet" has changed since initial value at login`)
+					},
+				}
+			}),
 			setupMocks: func(conn *mockldapconn.MockConn) {
 				conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1)
 				conn.EXPECT().Search(expectedUserSearch(nil)).Return(&ldap.SearchResult{
@@ -2588,77 +2594,3 @@ func TestRealTLSDialing(t *testing.T) {
 		})
 	}
 }
-
-func TestAttributeUnchangedSinceLogin(t *testing.T) {
-	initialVal := "some-attribute-value"
-	changedVal := "some-different-attribute-value"
-	attributeName := "some-attribute-name"
-	tests := []struct {
-		name       string
-		entry      *ldap.Entry
-		wantResult bool
-		wantErr    string
-	}{
-		{
-			name: "happy path where value has not changed since login",
-			entry: &ldap.Entry{
-				DN: "some-dn",
-				Attributes: []*ldap.EntryAttribute{
-					{
-						Name:       attributeName,
-						Values:     []string{initialVal},
-						ByteValues: [][]byte{[]byte(initialVal)},
-					},
-				},
-			},
-		},
-		{
-			name: "password has been reset since login",
-			entry: &ldap.Entry{
-				DN: "some-dn",
-				Attributes: []*ldap.EntryAttribute{
-					{
-						Name:       attributeName,
-						Values:     []string{changedVal},
-						ByteValues: [][]byte{[]byte(changedVal)},
-					},
-				},
-			},
-			wantErr: "value for attribute \"some-attribute-name\" has changed since initial value at login",
-		},
-		{
-			name: "no value for attribute attribute",
-			entry: &ldap.Entry{
-				DN:         "some-dn",
-				Attributes: []*ldap.EntryAttribute{},
-			},
-			wantErr: "expected to find 1 value for \"some-attribute-name\" attribute, but found 0",
-		},
-		{
-			name: "too many values for attribute",
-			entry: &ldap.Entry{
-				DN: "some-dn",
-				Attributes: []*ldap.EntryAttribute{
-					{
-						Name:       attributeName,
-						ByteValues: [][]byte{[]byte("val1"), []byte("val2")},
-					},
-				},
-			},
-			wantErr: "expected to find 1 value for \"some-attribute-name\" attribute, but found 2",
-		},
-	}
-	for _, test := range tests {
-		tt := test
-		t.Run(tt.name, func(t *testing.T) {
-			initialValRawEncoded := base64.RawURLEncoding.EncodeToString([]byte(initialVal))
-			err := AttributeUnchangedSinceLogin(attributeName)(tt.entry, provider.RefreshAttributes{AdditionalAttributes: map[string]string{attributeName: initialValRawEncoded}})
-			if tt.wantErr != "" {
-				require.Error(t, err)
-				require.Equal(t, tt.wantErr, err.Error())
-			} else {
-				require.NoError(t, err)
-			}
-		})
-	}
-}