DaanHoogland commented on a change in pull request #3694: Ldap fixes
URL: https://github.com/apache/cloudstack/pull/3694#discussion_r360407044
##########
File path:
plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/api/command/LdapListUsersCmd.java
##########
@@ -66,34 +122,115 @@ public LdapListUsersCmd(final LdapManager ldapManager,
final QueryService queryS
_queryService = queryService;
}
+ /**
+ * (as a check for isACloudstackUser is done) only non cloudstack users
should be shown
+ * @param users a list of {@code LdapUser}s
+ * @return a (filtered?) list of user response objects
+ */
private List<LdapUserResponse> createLdapUserResponse(final List<LdapUser>
users) {
final List<LdapUserResponse> ldapResponses = new
ArrayList<LdapUserResponse>();
for (final LdapUser user : users) {
- if (getListType().equals("all") || !isACloudstackUser(user)) {
- final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
- ldapResponse.setObjectName("LdapUser");
- ldapResponses.add(ldapResponse);
- }
+ final LdapUserResponse ldapResponse =
_ldapManager.createLdapUserResponse(user);
+ ldapResponse.setObjectName("LdapUser");
+ ldapResponses.add(ldapResponse);
}
return ldapResponses;
}
+ private List<UserResponse> cloudstackUsers = null;
+
@Override
public void execute() throws ServerApiException {
- List<LdapUserResponse> ldapResponses = null;
+ cloudstackUsers = null;
+ List<LdapUserResponse> ldapResponses = new
ArrayList<LdapUserResponse>();
final ListResponse<LdapUserResponse> response = new
ListResponse<LdapUserResponse>();
try {
- final List<LdapUser> users = _ldapManager.getUsers(null);
+ final List<LdapUser> users = _ldapManager.getUsers(domainId);
ldapResponses = createLdapUserResponse(users);
+// now filter and annotate
+ ldapResponses = applyUserFilter(ldapResponses);
} catch (final NoLdapUserMatchingQueryException ex) {
- ldapResponses = new ArrayList<LdapUserResponse>();
+ // ok, we'll make do with the empty list ldapResponses = new
ArrayList<LdapUserResponse>();
} finally {
response.setResponses(ldapResponses);
response.setResponseName(getCommandName());
setResponseObject(response);
}
}
+ private List<UserResponse> getCloudstackUsers() {
+ // get a list of relevant cloudstack users, meaning
+ // if we are filtering for local domain, only get users for the
current domain
+ // if we are filtering for any domain, get recursive all users for the
root domain
+ // if we are filtering for potential imports,
+ // we are only looking for users in the linked domains/accounts,
+ // which is only relevant if we ask ldap users for this domain.
+ // So we are asking for all users in the current domain as well
+ // in case of no filter we should find all users in the current domain
for annotation.
+ if (cloudstackUsers == null) {
+ ListResponse<UserResponse> cloudstackUsersresponse;
+ switch (getUserFilter()) {
+ case ANY_DOMAIN:
+ // get the user domain so if the calling user is a root admin
....
+ cloudstackUsersresponse =
_queryService.searchForUsers(CallContext.current().getCallingAccount().getDomainId(),
true);
+ break;
+ case NO_FILTER:
+ cloudstackUsersresponse =
_queryService.searchForUsers(this.domainId,true);
+ break;
+ case POTENTIAL_IMPORT:
+ case LOCAL_DOMAIN:
+ cloudstackUsersresponse =
_queryService.searchForUsers(this.domainId,false);
+ break;
+ default:
+ throw new CloudRuntimeException("error in program login; we
are not filtering but still querying users to filter???");
+ }
+ cloudstackUsers = cloudstackUsersresponse.getResponses();
Review comment:
_queryService.searchForUsers(..) will at least return an empty list of
userresponses, never null.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
