Skip to content

Commit

Permalink
Consider multiple users with the same GUID as a hard error
Browse files Browse the repository at this point in the history
This shouldn't be possible in practice, so it almost certainly
indicates either a configuration error, or something wrong on the
AD side of things. Either way we will refuse to process any user
that trips this logic, and complain about it quite loudly.
  • Loading branch information
nflynt committed Aug 9, 2023
1 parent 0cebb89 commit 3963205
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions pkg/agent/clean/active_directory.go
Expand Up @@ -79,6 +79,17 @@ func (e LdapErrorNotFound) Error() string {
return "ldap query returned no results"
}

// LdapFoundDuplicateGuid indicates either a configuration error or
// a corruption on the Active Directory side. In theory it should never
// be possible when talking to a real Active Directory server, but just
// in case we detect and handle it anyway.
type LdapFoundDuplicateGuid struct{}

// Error provides a string representation of an LdapErrorNotFound
func (e LdapFoundDuplicateGuid) Error() string {
return "ldap query returned multiple users for the same GUID"
}

type LdapConnectionPermanentlyFailed struct{}

// Error provides a string representation of an LdapConnectionPermanentlyFailed
Expand Down Expand Up @@ -219,7 +230,7 @@ func findDistinguishedName(guid string, lConn *ldapv3.Conn, adConfig *v3.ActiveD
if len(result.Entries) < 1 {
return "", LdapErrorNotFound{}
} else if len(result.Entries) > 1 {
return "", LdapErrorNotFound{}
return "", LdapFoundDuplicateGuid{}
}

entry := result.Entries[0]
Expand All @@ -233,7 +244,7 @@ func findDistinguishedNameWithRetries(guid string, lConn *ldapv3.Conn, adConfig

for retry := 0; retry < maxRetries; retry++ {
distinguishedName, err := findDistinguishedName(guid, lConn, adConfig)
if err == nil || errors.Is(err, LdapErrorNotFound{}) {
if err == nil || errors.Is(err, LdapErrorNotFound{}) || errors.Is(err, LdapFoundDuplicateGuid{}) {
return distinguishedName, err
}
logrus.Warnf("[%v] LDAP connection failed: '%v', retrying in %v...", migrateAdUserOperation, err, retryDelay.Seconds())
Expand Down Expand Up @@ -505,6 +516,9 @@ func identifyMigrationWorkUnits(users *v3.UserList, lConn *ldapv3.Conn, adConfig
logrus.Warnf("[%v] LDAP connection has permanently failed! will continue to migrate previously identified users", identifyAdUserOperation)
skippedUsers = append(skippedUsers, skippedUserWorkUnit{guid: guid, originalUser: user.DeepCopy()})
ldapPermanentlyFailed = true
} else if errors.Is(err, LdapFoundDuplicateGuid{}) {
logrus.Errorf("[%v] LDAP returned multiple users with GUID '%v'. this should not be possible, and may indicate a configuration error! this user will be skipped", identifyAdUserOperation, guid)
skippedUsers = append(skippedUsers, skippedUserWorkUnit{guid: guid, originalUser: user.DeepCopy()})
} else if errors.Is(err, LdapErrorNotFound{}) {
logrus.Debugf("[%v] user %v is GUID-based (%v) and the Active Directory server doesn't know about it. marking it as missing", identifyAdUserOperation, user.Name, guid)
knownGUIDMissingUnits[guid] = len(missingUsers)
Expand Down

0 comments on commit 3963205

Please sign in to comment.