diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 150c903a3..a375361d9 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/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/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go index 9d8d88404..40ee4a4c9 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher_test.go +++ b/internal/controller/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 index 6488cca1f..beb6a037e 100644 --- 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 index c6d1d1269..7de3f8282 100644 --- 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) - } - }) - } -}