Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ldap/roles): fix the issue of multiple entries in Ldap for single user #1173

Merged
merged 3 commits into from
Jul 31, 2024
Merged

fix(ldap/roles): fix the issue of multiple entries in Ldap for single user #1173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c367c5
test(ldap/roles): strengthen assertions in existing tests
kirangodishala Jul 30, 2024
6e71120
test(ldap/roles): add test to demonstrate the bug when multiple DNs e…
kirangodishala Jul 30, 2024
7ac5388
fix(ldap/roles): fix the issue of multiple entries in Ldap for single…
kirangodishala Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 67 additions & 40 deletions fiat-ldap/src/main/java/com/netflix/spinnaker/fiat/roles/ldap/LdapUserRolesProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,50 +71,69 @@ public void setConfigProps(LdapConfig.ConfigProps configProps) {
@Override
protected List<Role> loadRolesForUser(ExternalUser user) {
String userId = user.getId();

Set<String> userDns = new HashSet<>();
String[] params;
Set<String> userRolesPerDn;
Set<String> userRoles = new HashSet<>();
log.debug("loadRoles for user " + userId);
if (StringUtils.isEmpty(configProps.getGroupSearchBase())) {
return new ArrayList<>();
}

String fullUserDn = getUserFullDn(userId);
// handle multiple DNs in Ldap for single user
if (!isSingleDnToUserId(userId)) {
Map<String, String> userToDnMap = getUserDNs(List.of(userId));
userDns = new HashSet<>(userToDnMap.keySet());
log.debug("userDns : {} for the user : {}", userDns, userId);
} else {
String fullUserDn = getUserFullDn(userId);
if (fullUserDn != null) {
userDns = Set.of(fullUserDn);
}
}

if (fullUserDn == null) {
if (userDns.isEmpty()) {
// Likely a service account
log.debug("fullUserDn is null for {}", userId);
log.debug("userDn is empty for {}", userId);
return new ArrayList<>();
}

String[] params = new String[] {fullUserDn, userId};
for (String userDn : userDns) {
params = new String[] {userDn, userId};

if (log.isDebugEnabled()) {
log.debug(
new StringBuilder("Searching for groups using ")
.append("\ngroupSearchBase: ")
.append(configProps.getGroupSearchBase())
.append("\ngroupSearchFilter: ")
.append(configProps.getGroupSearchFilter())
.append("\nparams: ")
.append(StringUtils.join(params, " :: "))
.append("\ngroupRoleAttributes: ")
.append(configProps.getGroupRoleAttributes())
.toString());
}

if (log.isDebugEnabled()) {
log.debug(
new StringBuilder("Searching for groups using ")
.append("\ngroupSearchBase: ")
.append(configProps.getGroupSearchBase())
.append("\ngroupSearchFilter: ")
.append(configProps.getGroupSearchFilter())
.append("\nparams: ")
.append(StringUtils.join(params, " :: "))
.append("\ngroupRoleAttributes: ")
.append(configProps.getGroupRoleAttributes())
.toString());
// Copied from org.springframework.security.ldap.userdetails.DefaultLdapAuthoritiesPopulator.
userRolesPerDn =
ldapTemplate.searchForSingleAttributeValues(
configProps.getGroupSearchBase(),
configProps.getGroupSearchFilter(),
params,
configProps.getGroupRoleAttributes());
userRoles.addAll(userRolesPerDn);
}

// Copied from org.springframework.security.ldap.userdetails.DefaultLdapAuthoritiesPopulator.
Set<String> userRoles =
ldapTemplate.searchForSingleAttributeValues(
configProps.getGroupSearchBase(),
configProps.getGroupSearchFilter(),
params,
configProps.getGroupRoleAttributes());

log.debug("Got roles for user " + userId + ": " + userRoles);
return userRoles.stream()
.map(role -> new Role(role).setSource(Role.Source.LDAP))
.collect(Collectors.toList());
}

private boolean isSingleDnToUserId(String userId) {
return getPartialUserDn(userId) != null;
}

/** Mapper for mapping user ids to their groups */
class UserGroupMapper implements AttributesMapper<List<Pair<String, Role>>> {

Expand Down Expand Up @@ -248,8 +267,27 @@ String getUserFullDn(String userId) {
DistinguishedName root = new DistinguishedName(rootDn);
log.debug("Root DN: " + root.toString());

String[] formatArgs = new String[] {LdapEncoder.nameEncode(userId)};
String partialUserDn = getPartialUserDn(userId);
if (partialUserDn == null) {
return null;
}

DistinguishedName user = new DistinguishedName(partialUserDn);
log.debug("User portion: " + user.toString());

try {
Name fullUser = root.addAll(user);
log.debug("Full user DN: " + fullUser.toString());
return fullUser.toString();
} catch (InvalidNameException ine) {
log.error("Could not assemble full userDn", ine);
}
return null;
}

@VisibleForTesting
String getPartialUserDn(String userId) {
String[] formatArgs = new String[] {LdapEncoder.nameEncode(userId)};
String partialUserDn;
if (!StringUtils.isEmpty(configProps.getUserSearchFilter())) {
try {
Expand All @@ -258,24 +296,13 @@ String getUserFullDn(String userId) {
configProps.getUserSearchBase(), configProps.getUserSearchFilter(), formatArgs);
partialUserDn = res.getDn().toString();
} catch (IncorrectResultSizeDataAccessException e) {
log.error("Unable to find a single user entry for {}", userId, e);
log.warn("Unable to find a single user entry for {}", userId, e);
return null;
}
} else {
partialUserDn = configProps.getUserDnPattern().format(formatArgs);
}

DistinguishedName user = new DistinguishedName(partialUserDn);
log.debug("User portion: " + user.toString());

try {
Name fullUser = root.addAll(user);
log.debug("Full user DN: " + fullUser.toString());
return fullUser.toString();
} catch (InvalidNameException ine) {
log.error("Could not assemble full userDn", ine);
}
return null;
return partialUserDn;
}

/**
Expand Down
Loading