Clean up nits in AD code

- Make everything private - Drop unused AuthTime field - Use %q format string instead of "%s" - Only rely on GetRawAttributeValues in AttributeUnchangedSinceLogin Signed-off-by: Monis Khan <mok@vmware.com>
2026-09-25 17:34:37 +00:00 · 2021-12-17 08:53:44 -05:00
parent a6085c9678
commit c155c6e629
6 changed files with 106 additions and 104 deletions
@@ -58,15 +58,15 @@ const (
 	defaultActiveDirectoryGroupSearchFilter = "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))"

 	sAMAccountNameAttribute = "sAMAccountName"
-	// PwdLastSetAttribute is the date and time that the password for this account was last changed.
+	// pwdLastSetAttribute is the date and time that the password for this account was last changed.
 	// https://docs.microsoft.com/en-us/windows/win32/adschema/a-pwdlastset
-	PwdLastSetAttribute = "pwdLastSet"
-	// UserAccountControlAttribute represents a bitmap of user properties.
+	pwdLastSetAttribute = "pwdLastSet"
+	// userAccountControlAttribute represents a bitmap of user properties.
 	// https://docs.microsoft.com/en-us/troubleshoot/windows-server/identity/useraccountcontrol-manipulate-account-properties
-	UserAccountControlAttribute = "userAccountControl"
-	// UserAccountControlComputedAttribute represents a bitmap of user properties.
+	userAccountControlAttribute = "userAccountControl"
+	// userAccountControlComputedAttribute represents a bitmap of user properties.
 	// https://docs.microsoft.com/en-us/windows/win32/adschema/a-msds-user-account-control-computed
-	UserAccountControlComputedAttribute = "msDS-User-Account-Control-Computed"
+	userAccountControlComputedAttribute = "msDS-User-Account-Control-Computed"
 	// 0x0002 ACCOUNTDISABLE in userAccountControl bitmap.
 	accountDisabledBitmapValue = 2
 	// 0x0010 UF_LOCKOUT in msDS-User-Account-Control-Computed bitmap.
@@ -334,17 +334,21 @@ func (c *activeDirectoryWatcherController) validateUpstream(ctx context.Context,
 			Filter:             adUpstreamImpl.Spec().GroupSearch().Filter(),
 			GroupNameAttribute: adUpstreamImpl.Spec().GroupSearch().GroupNameAttribute(),
 		},
-		Dialer:                       c.ldapDialer,
-		UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+		Dialer: c.ldapDialer,
+		UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){
+			"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID"),
+		},
 		RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
-			PwdLastSetAttribute:                 upstreamldap.AttributeUnchangedSinceLogin(PwdLastSetAttribute),
-			UserAccountControlAttribute:         ValidUserAccountControl,
-			UserAccountControlComputedAttribute: ValidComputedUserAccountControl,
+			pwdLastSetAttribute:                 upstreamldap.AttributeUnchangedSinceLogin(pwdLastSetAttribute),
+			userAccountControlAttribute:         validUserAccountControl,
+			userAccountControlComputedAttribute: validComputedUserAccountControl,
 		},
 	}

 	if spec.GroupSearch.Attributes.GroupName == "" {
-		config.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){defaultActiveDirectoryGroupNameAttributeName: GroupSAMAccountNameWithDomainSuffix}
+		config.GroupAttributeParsingOverrides = map[string]func(*ldap.Entry) (string, error){
+			defaultActiveDirectoryGroupNameAttributeName: groupSAMAccountNameWithDomainSuffix,
+		}
 	}

 	conditions := upstreamwatchers.ValidateGenericLDAP(ctx, adUpstreamImpl, c.secretInformer, c.validatedSecretVersionsCache, config)
@@ -378,7 +382,7 @@ func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, ups
 	}
 }

-func MicrosoftUUIDFromBinary(attributeName string) func(entry *ldap.Entry) (string, error) {
+func microsoftUUIDFromBinaryAttr(attributeName string) func(entry *ldap.Entry) (string, error) {
 	// validation has already been done so we can just get the attribute...
 	return func(entry *ldap.Entry) (string, error) {
 		binaryUUID := entry.GetRawAttributeValue(attributeName)
@@ -399,18 +403,18 @@ func microsoftUUIDFromBinary(binaryUUID []byte) (string, error) {
 	return uuidVal.String(), nil
 }

-func GroupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) {
+func groupSAMAccountNameWithDomainSuffix(entry *ldap.Entry) (string, error) {
 	sAMAccountNameAttributeValues := entry.GetAttributeValues(sAMAccountNameAttribute)

 	if len(sAMAccountNameAttributeValues) != 1 {
-		return "", fmt.Errorf(`found %d values for attribute "%s", but expected 1 result`,
+		return "", fmt.Errorf(`found %d values for attribute %q, but expected 1 result`,
 			len(sAMAccountNameAttributeValues), sAMAccountNameAttribute,
 		)
 	}

 	sAMAccountName := sAMAccountNameAttributeValues[0]
 	if len(sAMAccountName) == 0 {
-		return "", fmt.Errorf(`found empty value for attribute "%s", but expected value to be non-empty`,
+		return "", fmt.Errorf(`found empty value for attribute %q, but expected value to be non-empty`,
 			sAMAccountNameAttribute,
 		)
 	}
@@ -433,8 +437,8 @@ func getDomainFromDistinguishedName(distinguishedName string) (string, error) {
 	return strings.Join(domainComponents[1:], "."), nil
 }

-func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error {
-	userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlAttribute))
+func validUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error {
+	userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlAttribute))
 	if err != nil {
 		return err
 	}
@@ -446,8 +450,8 @@ func ValidUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttribut
 	return nil
 }

-func ValidComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error {
-	userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(UserAccountControlComputedAttribute))
+func validComputedUserAccountControl(entry *ldap.Entry, _ provider.StoredRefreshAttributes) error {
+	userAccountControl, err := strconv.Atoi(entry.GetAttributeValue(userAccountControlComputedAttribute))
 	if err != nil {
 		return err
 	}
@@ -220,11 +220,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 			Filter:             testGroupSearchFilter,
 			GroupNameAttribute: testGroupNameAttrName,
 		},
-		UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+		UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 		RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 			"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-			"userAccountControl":                 ValidUserAccountControl,
-			"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+			"userAccountControl":                 validUserAccountControl,
+			"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 		},
 	}

@@ -541,11 +541,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -602,11 +602,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: "sAMAccountName",
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -666,11 +666,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -730,11 +730,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -793,11 +793,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -927,11 +927,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1056,11 +1056,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					}},
 			},
 			wantResultingUpstreams: []v1alpha1.ActiveDirectoryIdentityProvider{{
@@ -1111,11 +1111,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1315,12 +1315,12 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={}))",
 						GroupNameAttribute: "sAMAccountName",
 					},
-					UIDAttributeParsingOverrides:   map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
-					GroupAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"sAMAccountName": GroupSAMAccountNameWithDomainSuffix},
+					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.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1373,11 +1373,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1434,11 +1434,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1489,11 +1489,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1690,11 +1690,11 @@ func TestActiveDirectoryUpstreamWatcherControllerSync(t *testing.T) {
 						Filter:             testGroupSearchFilter,
 						GroupNameAttribute: testGroupNameAttrName,
 					},
-					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": MicrosoftUUIDFromBinary("objectGUID")},
+					UIDAttributeParsingOverrides: map[string]func(*ldap.Entry) (string, error){"objectGUID": microsoftUUIDFromBinaryAttr("objectGUID")},
 					RefreshAttributeChecks: map[string]func(*ldap.Entry, provider.StoredRefreshAttributes) error{
 						"pwdLastSet":                         upstreamldap.AttributeUnchangedSinceLogin("pwdLastSet"),
-						"userAccountControl":                 ValidUserAccountControl,
-						"msDS-User-Account-Control-Computed": ValidComputedUserAccountControl,
+						"userAccountControl":                 validUserAccountControl,
+						"msDS-User-Account-Control-Computed": validComputedUserAccountControl,
 					},
 				},
 			},
@@ -1931,7 +1931,7 @@ func TestGroupSAMAccountNameWithDomainSuffix(t *testing.T) {
 	for _, test := range tests {
 		tt := test
 		t.Run(tt.name, func(t *testing.T) {
-			suffixedSAMAccountName, err := GroupSAMAccountNameWithDomainSuffix(tt.entry)
+			suffixedSAMAccountName, err := groupSAMAccountNameWithDomainSuffix(tt.entry)
 			if tt.wantErr != "" {
 				require.EqualError(t, err, tt.wantErr)
 			} else {
@@ -2074,7 +2074,7 @@ func TestValidUserAccountControl(t *testing.T) {
 	for _, test := range tests {
 		tt := test
 		t.Run(tt.name, func(t *testing.T) {
-			err := ValidUserAccountControl(tt.entry, provider.StoredRefreshAttributes{})
+			err := validUserAccountControl(tt.entry, provider.StoredRefreshAttributes{})

 			if tt.wantErr != "" {
 				require.Error(t, err)
@@ -2135,7 +2135,7 @@ func TestValidComputedUserAccountControl(t *testing.T) {
 	for _, test := range tests {
 		tt := test
 		t.Run(tt.name, func(t *testing.T) {
-			err := ValidComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{})
+			err := validComputedUserAccountControl(tt.entry, provider.StoredRefreshAttributes{})

 			if tt.wantErr != "" {
 				require.Error(t, err)