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.

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

TASK-5980 - REST Admin - Endpoint GET /admin/users/search only allowed for opencga user, not for organization admin/owner #2438

Merged
merged 5 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2024-04-10 OpenCB
* Copyright 2015-2024-04-29 OpenCB
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -85,7 +85,7 @@ public abstract class OpenCgaCompleter implements Completer {
.map(Candidate::new)
.collect(toList());

private List<Candidate> usersList = asList( "anonymous","create","login","password","info","configs","configs-update","filters","password-reset","update")
private List<Candidate> usersList = asList( "anonymous","create","login","password","search","info","configs","configs-update","filters","password-reset","update")
.stream()
.map(Candidate::new)
.collect(toList());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2015-2024-04-10 OpenCB
* Copyright 2015-2024-04-29 OpenCB
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -229,6 +229,7 @@ public OpencgaCliOptionsParser() {
usersSubCommands.addCommand("create", usersCommandOptions.createCommandOptions);
usersSubCommands.addCommand("login", usersCommandOptions.loginCommandOptions);
usersSubCommands.addCommand("password", usersCommandOptions.passwordCommandOptions);
usersSubCommands.addCommand("search", usersCommandOptions.searchCommandOptions);
usersSubCommands.addCommand("info", usersCommandOptions.infoCommandOptions);
usersSubCommands.addCommand("configs", usersCommandOptions.configsCommandOptions);
usersSubCommands.addCommand("configs-update", usersCommandOptions.updateConfigsCommandOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ private RestResponse<Sample> searchUsers() throws Exception {
queryParams.putIfNotNull("count", commandOptions.count);
queryParams.putIfNotEmpty("organization", commandOptions.organization);
queryParams.putIfNotEmpty("user", commandOptions.user);
queryParams.putIfNotEmpty("account", commandOptions.account);
queryParams.putIfNotEmpty("authenticationId", commandOptions.authenticationId);

return openCGAClient.getAdminClient().searchUsers(queryParams);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.opencb.opencga.core.models.organizations.OrganizationConfiguration;
import org.opencb.opencga.core.models.organizations.OrganizationCreateParams;
import org.opencb.opencga.core.models.organizations.OrganizationUpdateParams;
import org.opencb.opencga.core.models.organizations.TokenConfiguration;
import org.opencb.opencga.core.response.QueryType;
import org.opencb.opencga.core.response.RestResponse;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ public void execute() throws Exception {
case "password":
queryResponse = password();
break;
case "search":
queryResponse = search();
break;
case "info":
queryResponse = info();
break;
Expand Down Expand Up @@ -188,6 +191,24 @@ private RestResponse<User> password() throws Exception {
return openCGAClient.getUserClient().password(passwordChangeParams);
}

private RestResponse<User> search() throws Exception {
logger.debug("Executing search in Users command line");

UsersCommandOptions.SearchCommandOptions commandOptions = usersCommandOptions.searchCommandOptions;

ObjectMap queryParams = new ObjectMap();
queryParams.putIfNotEmpty("include", commandOptions.include);
queryParams.putIfNotEmpty("exclude", commandOptions.exclude);
queryParams.putIfNotNull("limit", commandOptions.limit);
queryParams.putIfNotNull("skip", commandOptions.skip);
queryParams.putIfNotNull("count", commandOptions.count);
queryParams.putIfNotEmpty("organization", commandOptions.organization);
queryParams.putIfNotEmpty("id", commandOptions.id);
queryParams.putIfNotEmpty("authenticationId", commandOptions.authenticationId);

return openCGAClient.getUserClient().search(queryParams);
}

private RestResponse<User> info() throws Exception {
logger.debug("Executing info in Users command line");

Expand Down Expand Up @@ -284,7 +305,6 @@ private RestResponse<User> update() throws Exception {
ObjectMap beanParams = new ObjectMap();
putNestedIfNotEmpty(beanParams, "name",commandOptions.name, true);
putNestedIfNotEmpty(beanParams, "email",commandOptions.email, true);
putNestedIfNotEmpty(beanParams, "organization",commandOptions.organization, true);
putNestedIfNotNull(beanParams, "attributes",commandOptions.attributes, true);

userUpdateParams = JacksonUtils.getDefaultObjectMapper().copy()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,6 @@ public class SearchUsersCommandOptions {
@Parameter(names = {"--user", "-u"}, description = "User ID", required = false, arity = 1)
public String user;

@Parameter(names = {"--account"}, description = "Account type [GUEST, FULL, ADMINISTRATOR]", required = false, arity = 1)
public String account;

@Parameter(names = {"--authentication-id"}, description = "Authentication origin ID", required = false, arity = 1)
public String authenticationId;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class UsersCommandOptions extends CustomUsersCommandOptions {
public CreateCommandOptions createCommandOptions;
public LoginCommandOptions loginCommandOptions;
public PasswordCommandOptions passwordCommandOptions;
public SearchCommandOptions searchCommandOptions;
public InfoCommandOptions infoCommandOptions;
public ConfigsCommandOptions configsCommandOptions;
public UpdateConfigsCommandOptions updateConfigsCommandOptions;
Expand All @@ -52,6 +53,7 @@ public UsersCommandOptions(CommonCommandOptions commonCommandOptions, JCommander
this.createCommandOptions = new CreateCommandOptions();
this.loginCommandOptions = new LoginCommandOptions();
this.passwordCommandOptions = new PasswordCommandOptions();
this.searchCommandOptions = new SearchCommandOptions();
this.infoCommandOptions = new InfoCommandOptions();
this.configsCommandOptions = new ConfigsCommandOptions();
this.updateConfigsCommandOptions = new UpdateConfigsCommandOptions();
Expand Down Expand Up @@ -136,6 +138,38 @@ public class PasswordCommandOptions {

}

@Parameters(commandNames = {"search"}, commandDescription ="User search method")
public class SearchCommandOptions {

@ParametersDelegate
public CommonCommandOptions commonOptions = commonCommandOptions;

@Parameter(names = {"--include", "-I"}, description = "Fields included in the response, whole JSON path must be provided", required = false, arity = 1)
public String include;

@Parameter(names = {"--exclude", "-E"}, description = "Fields excluded in the response, whole JSON path must be provided", required = false, arity = 1)
public String exclude;

@Parameter(names = {"--limit"}, description = "Number of results to be returned", required = false, arity = 1)
public Integer limit;

@Parameter(names = {"--skip"}, description = "Number of results to skip", required = false, arity = 1)
public Integer skip;

@Parameter(names = {"--count"}, description = "Get the total number of results matching the query. Deactivated by default.", required = false, help = true, arity = 0)
public boolean count = false;

@Parameter(names = {"--organization"}, description = "Organization id", required = false, arity = 1)
public String organization;

@Parameter(names = {"--id"}, description = "Comma separated list user IDs up to a maximum of 100. Also admits basic regular expressions using the operator '~', i.e. '~{perl-regex}' e.g. '~value' for case sensitive, '~/value/i' for case insensitive search.", required = false, arity = 1)
public String id;

@Parameter(names = {"--authentication-id"}, description = "Authentication origin ID", required = false, arity = 1)
public String authenticationId;

}

@Parameters(commandNames = {"info"}, commandDescription ="Return the user information including its projects and studies")
public class InfoCommandOptions {

Expand Down Expand Up @@ -251,9 +285,6 @@ public class UpdateCommandOptions {
@Parameter(names = {"--email"}, description = "The body web service email parameter", required = false, arity = 1)
public String email;

@Parameter(names = {"--organization"}, description = "The body web service organization parameter", required = false, arity = 1)
public String organization;

@DynamicParameter(names = {"--attributes"}, description = "The body web service attributes parameter. Use: --attributes key=value", required = false)
public java.util.Map<java.lang.String,java.lang.Object> attributes = new HashMap<>(); //Dynamic parameters must be initialized;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ public class AdminManager extends AbstractManager {

public OpenCGAResult<User> userSearch(String organizationId, Query query, QueryOptions options, String token)
throws CatalogException {
query = ParamUtils.defaultObject(query, Query::new);
options = ParamUtils.defaultObject(options, QueryOptions::new);

ObjectMap auditParams = new ObjectMap()
.append("organizationId", organizationId)
.append("query", query)
Expand All @@ -58,30 +55,23 @@ public OpenCGAResult<User> userSearch(String organizationId, Query query, QueryO
JwtPayload jwtPayload = catalogManager.getUserManager().validateToken(token);
try {
authorizationManager.checkIsOpencgaAdministrator(jwtPayload);

// Fix query object
if (query.containsKey(ParamConstants.USER)) {
query.put(UserDBAdaptor.QueryParams.ID.key(), query.get(ParamConstants.USER));
query.remove(ParamConstants.USER);
}
if (query.containsKey(ParamConstants.USER_AUTHENTICATION_ORIGIN)) {
query.put(UserDBAdaptor.QueryParams.ACCOUNT_AUTHENTICATION_ID.key(), query.get(ParamConstants.USER_AUTHENTICATION_ORIGIN));
query.remove(ParamConstants.USER_AUTHENTICATION_ORIGIN);
}
if (query.containsKey(ParamConstants.USER_CREATION_DATE)) {
query.put(UserDBAdaptor.QueryParams.ACCOUNT_CREATION_DATE.key(), query.get(ParamConstants.USER_CREATION_DATE));
query.remove(ParamConstants.USER_CREATION_DATE);
}

OpenCGAResult<User> userDataResult = getUserDBAdaptor(organizationId).get(query, options);
auditManager.auditSearch(organizationId, jwtPayload.getUserId(organizationId), Enums.Resource.USER, "", "", auditParams,
new AuditRecord.Status(AuditRecord.Status.Result.SUCCESS));
return userDataResult;
} catch (CatalogException e) {
auditManager.auditSearch(organizationId, jwtPayload.getUserId(organizationId), Enums.Resource.USER, "", "", auditParams,
new AuditRecord.Status(AuditRecord.Status.Result.ERROR, e.getError()));
throw e;
}

// Fix query object
if (query.containsKey(ParamConstants.USER)) {
query.put(UserDBAdaptor.QueryParams.ID.key(), query.get(ParamConstants.USER));
query.remove(ParamConstants.USER);
}
if (query.containsKey(ParamConstants.USER_CREATION_DATE)) {
query.put(UserDBAdaptor.QueryParams.ACCOUNT_CREATION_DATE.key(), query.get(ParamConstants.USER_CREATION_DATE));
query.remove(ParamConstants.USER_CREATION_DATE);
}

return catalogManager.getUserManager().search(organizationId, query, options, token);
}

public OpenCGAResult<Group> updateGroups(String organizationId, String userId, List<String> studyIds, List<String> groupIds,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,47 @@ public OpenCGAResult<User> create(String id, String name, String email, String p
return create(user, password, token);
}

/**
* Search users from Organization. Token must belong to at least an Organization administrator.
*
* @param organizationId Organization id.
* @param query Query object.
* @param options QueryOptions object.
* @param token JWT token.
* @return OpenCGAResult with the list of users.
* @throws CatalogException if the token does not belong to an Organization administrator or there are any parameters wrong.
*/
public OpenCGAResult<User> search(@Nullable String organizationId, Query query, QueryOptions options, String token)
throws CatalogException {
JwtPayload tokenPayload = catalogManager.getUserManager().validateToken(token);
ObjectMap auditParams = new ObjectMap()
.append("organizationId", organizationId)
.append("query", query)
.append("options", options)
.append("token", token);

options = ParamUtils.defaultObject(options, QueryOptions::new);
String myOrganizationId = StringUtils.isNotEmpty(organizationId) ? organizationId : tokenPayload.getOrganization();
try {
authorizationManager.checkIsAtLeastOrganizationOwnerOrAdmin(myOrganizationId, tokenPayload.getUserId(myOrganizationId));

// Fix query params
if (query.containsKey(ParamConstants.USER_AUTHENTICATION_ORIGIN)) {
query.put(UserDBAdaptor.QueryParams.ACCOUNT_AUTHENTICATION_ID.key(), query.get(ParamConstants.USER_AUTHENTICATION_ORIGIN));
query.remove(ParamConstants.USER_AUTHENTICATION_ORIGIN);
}

OpenCGAResult<User> result = getUserDBAdaptor(myOrganizationId).get(query, options);
auditManager.auditSearch(myOrganizationId, tokenPayload.getUserId(myOrganizationId), Enums.Resource.USER, "", "", auditParams,
new AuditRecord.Status(AuditRecord.Status.Result.SUCCESS));
return result;
} catch (Exception e) {
auditManager.auditSearch(myOrganizationId, tokenPayload.getUserId(myOrganizationId), Enums.Resource.USER, "", "", auditParams,
new AuditRecord.Status(AuditRecord.Status.Result.ERROR, new Error(0, "User search", e.getMessage())));
throw e;
}
}

public JwtPayload validateToken(String token) throws CatalogException {
JwtPayload jwtPayload = new JwtPayload(token);
ParamUtils.checkParameter(jwtPayload.getUserId(), "jwt user");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,35 @@ public void testAdminUserExists() throws Exception {
assertEquals(ParamConstants.ADMIN_ORGANIZATION, payload.getOrganization());
}

@Test
public void searchUsersTest() throws CatalogException {
OpenCGAResult<User> search = catalogManager.getUserManager().search(organizationId, new Query(), QueryOptions.empty(), opencgaToken);
assertEquals(8, search.getNumResults());
for (User user : search.getResults()) {
if (noAccessUserId1.equals(user.getId())) {
assertEquals(0, user.getProjects().size());
} else if (user.getId().startsWith("normalUser")) {
assertEquals(1, user.getProjects().size());
} else {
assertEquals(2, user.getProjects().size());
}
}

search = catalogManager.getUserManager().search(null, new Query(), QueryOptions.empty(), ownerToken);
assertEquals(8, search.getNumResults());

search = catalogManager.getUserManager().search(null, new Query(), QueryOptions.empty(), orgAdminToken2);
assertEquals(8, search.getNumResults());

search = catalogManager.getUserManager().search(null, new Query(), QueryOptions.empty(), orgAdminToken1);
assertEquals(8, search.getNumResults());

assertThrows(CatalogAuthorizationException.class, () -> catalogManager.getUserManager().search(null, new Query(),
QueryOptions.empty(), studyAdminToken1));
assertThrows(CatalogAuthorizationException.class, () -> catalogManager.getUserManager().search(null, new Query(),
QueryOptions.empty(), normalToken1));
}

@Test
public void testGetToken() throws Exception {
String token = catalogManager.getUserManager().loginAsAdmin(TestParamConstants.ADMIN_PASSWORD).getToken();
Expand Down
5 changes: 2 additions & 3 deletions opencga-client/src/main/R/R/Admin-methods.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# WARNING: AUTOGENERATED CODE
#
# This code was generated by a tool.
# Autogenerated on: 2024-04-10
# Autogenerated on: 2024-04-29
#
# Manual changes to this file may cause unexpected behavior in your application.
# Manual changes to this file will be overwritten if the code is regenerated.
Expand All @@ -26,7 +26,7 @@
#' | createUsers | /{apiVersion}/admin/users/create | body[*] |
#' | importUsers | /{apiVersion}/admin/users/import | organization, body[*] |
#' | permissionsUsers | /{apiVersion}/admin/users/permissions | study, entryIds, permissions, category |
#' | searchUsers | /{apiVersion}/admin/users/search | include, exclude, limit, skip, count, organization, user, account, authenticationId |
#' | searchUsers | /{apiVersion}/admin/users/search | include, exclude, limit, skip, count, organization, user, authenticationId |
#' | syncUsers | /{apiVersion}/admin/users/sync | organization, body[*] |
#' | usersUpdateGroups | /{apiVersion}/admin/users/{user}/groups/update | organization, user[*], action, body[*] |
#'
Expand Down Expand Up @@ -97,7 +97,6 @@ setMethod("adminClient", "OpencgaR", function(OpencgaR, user, endpointName, para
#' @param count Get the total number of results matching the query. Deactivated by default.
#' @param organization Organization id.
#' @param user User ID.
#' @param account Account type [GUEST, FULL, ADMINISTRATOR].
#' @param authenticationId Authentication origin ID.
searchUsers=fetchOpenCGA(object=OpencgaR, category="admin", categoryId=NULL, subcategory="users",
subcategoryId=NULL, action="search", params=params, httpMethod="GET", as.queryParam=NULL, ...),
Expand Down
2 changes: 1 addition & 1 deletion opencga-client/src/main/R/R/Alignment-methods.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# WARNING: AUTOGENERATED CODE
#
# This code was generated by a tool.
# Autogenerated on: 2024-04-10
# Autogenerated on: 2024-04-29
#
# Manual changes to this file may cause unexpected behavior in your application.
# Manual changes to this file will be overwritten if the code is regenerated.
Expand Down
Loading
Loading