diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleHelper.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleHelper.java index 8d0ff266c301..bf37100a1736 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleHelper.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleHelper.java @@ -34,6 +34,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -496,4 +497,61 @@ public List deleteRoleLayouts(final Role role, final Set layoutI return layoutsDeleted; } + + /** + * Builds the {@link RoleView}s for the given roles, resolving the {@code userCount} of every + * view (parents and, when requested, their hydrated children) with a single aggregated query + * instead of one query per role. + * + * @param roles the roles to build views for, order is preserved + * @param loadChildrenRoles when true, each role's direct children are hydrated as child views + * @param roleAPI the {@link RoleAPI} used to load children and resolve counts + * @return the views in the same order as the given roles + * @throws DotDataException if loading a child role or the count query fails + */ + public List toRoleViews(final List roles, final boolean loadChildrenRoles, + final RoleAPI roleAPI) throws DotDataException { + + final List allRoleIds = new ArrayList<>(); + final Map> childrenByParentId = new LinkedHashMap<>(); + + for (final Role role : roles) { + + allRoleIds.add(role.getId()); + if (loadChildrenRoles && null != role.getRoleChildren()) { + + final List children = new ArrayList<>(); + for (final String childRoleId : role.getRoleChildren()) { + + final Role child = roleAPI.loadRoleById(childRoleId); + if (null == child || !UtilMethods.isSet(child.getId())) { + + Logger.warn(this, "Child role: " + childRoleId + " of role: " + + role.getId() + " does not resolve, skipping it"); + continue; + } + children.add(child); + allRoleIds.add(childRoleId); + } + childrenByParentId.put(role.getId(), children); + } + } + + final Map userCounts = roleAPI.countUsersByRoleIds(allRoleIds); + + final List views = new ArrayList<>(); + for (final Role role : roles) { + + final List childViews = new ArrayList<>(); + for (final Role child : childrenByParentId.getOrDefault(role.getId(), List.of())) { + + childViews.add(new RoleView(child, new ArrayList<>(), + userCounts.getOrDefault(child.getId(), 0))); + } + views.add(new RoleView(role, childViews, + userCounts.getOrDefault(role.getId(), 0))); + } + + return views; + } } diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleResource.java index b47194aec4f3..7c379e32cb37 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleResource.java @@ -2,12 +2,18 @@ import com.google.common.annotations.VisibleForTesting; import com.dotcms.rest.InitDataObject; +import com.dotcms.rest.ResponseEntityPaginatedDataView; import com.dotcms.rest.ResponseEntityView; import com.dotcms.rest.WebResource; +import com.dotcms.rest.annotation.NoCache; import com.dotcms.rest.annotation.SwaggerCompliant; import com.dotcms.rest.exception.BadRequestException; import com.dotcms.rest.exception.ForbiddenException; import com.dotcms.rest.exception.mapper.ExceptionMapperUtil; +import com.dotcms.util.PaginationUtil; +import com.dotcms.util.PaginationUtilParams; +import com.dotcms.util.pagination.OrderDirection; +import com.dotcms.util.pagination.UserPaginator; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.ApiProvider; import com.dotmarketing.business.DotStateException; @@ -29,6 +35,7 @@ import com.dotmarketing.util.Logger; import com.dotmarketing.util.PortletID; import com.dotmarketing.util.SecurityLogger; +import com.dotmarketing.common.util.SQLUtil; import com.dotmarketing.util.StringUtils; import com.dotmarketing.util.UtilMethods; import com.liferay.portal.PortalException; @@ -46,6 +53,7 @@ import io.swagger.v3.oas.annotations.tags.Tag; import io.vavr.control.Try; import org.apache.commons.beanutils.BeanUtils; +import org.glassfish.jersey.server.JSONP; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -97,6 +105,7 @@ public class RoleResource implements Serializable { private final RoleAPI roleAPI; private final RoleHelper roleHelper = new RoleHelper(); private final UserAPI userAPI = APILocator.getUserAPI(); + private final PaginationUtil userPaginationUtil = new PaginationUtil(new UserPaginator()); /** * Default class constructor. @@ -412,15 +421,9 @@ public ResponseEntityRoleDetailView updateRole( final Role updatedRole = this.roleHelper.updateRole(roleId, roleForm, user); - // same response shape as GET /v1/roles/{roleid} - final List childrenRoles = new ArrayList<>(); - final List roleChildrenIdList = null != updatedRole.getRoleChildren() - ? updatedRole.getRoleChildren() : new ArrayList<>(); - for (final String childRoleId : roleChildrenIdList) { - childrenRoles.add(new RoleView(this.roleAPI.loadRoleById(childRoleId), new ArrayList<>())); - } - - return new ResponseEntityRoleDetailView(new RoleView(updatedRole, childrenRoles)); + // same response shape as GET /v1/roles/{roleid}, counts included + return new ResponseEntityRoleDetailView( + this.roleHelper.toRoleViews(List.of(updatedRole), true, this.roleAPI).get(0)); } /** @@ -811,6 +814,110 @@ private final List filterRoleList(final String roleNameToFilter, final Lis roleList; } + /** + * Returns the paginated list of users directly granted the given role, using the standard + * user serialization (email address included). Grants inherited through the role hierarchy + * are not part of the response: clients that need the effective member list walk the + * ancestor chain through the {@code parent} attribute of {@link RoleView} and call this + * endpoint per role. + * + * @param request {@link HttpServletRequest} + * @param response {@link HttpServletResponse} + * @param roleId id of the role to list users for + * @param filter optional search matching user id, first name, last name, email or full name + * @param page page number, 1-based + * @param perPage page size + * @param orderBy column to sort by + * @param direction sorting direction, ASC or DESC + * @return the paginated user list + * @throws DotDataException if loading the role fails + */ + @Operation( + operationId = "loadUsersByRoleId", + summary = "Get the users directly granted a role", + description = "Returns the paginated list of users directly granted the given role, " + + "using the standard user serialization (email address included). Grants " + + "inherited through the role hierarchy are not included." + ) + @ApiResponses(value = { + @ApiResponse(responseCode = "200", + description = "Users retrieved successfully", + content = @Content(mediaType = "application/json", + schema = @Schema(implementation = ResponseEntityPaginatedDataView.class))), + @ApiResponse(responseCode = "400", + description = "Bad request - invalid pagination or sorting parameters", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "401", + description = "Unauthorized - authentication required", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "403", + description = "Forbidden - roles portlet access required", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "404", + description = "Role not found", + content = @Content(mediaType = "application/json")) + }) + @GET + @Path("/{roleid}/users") + @JSONP + @NoCache + @Produces(MediaType.APPLICATION_JSON) + public ResponseEntityPaginatedDataView loadUsersByRoleId( + @Parameter(hidden = true) @Context final HttpServletRequest request, + @Parameter(hidden = true) @Context final HttpServletResponse response, + @Parameter(description = "Id of the role to list users for", required = true) + @PathParam("roleid") final String roleId, + @Parameter(description = "Filter matching user id, first name, last name, email or full name") + @QueryParam("filter") final String filter, + @Parameter(description = "Page number for pagination") + @DefaultValue("1") @QueryParam(PaginationUtil.PAGE) final int page, + @Parameter(description = "Number of items per page") + @DefaultValue("40") @QueryParam(PaginationUtil.PER_PAGE) final int perPage, + @Parameter(description = "Column name for sorting results") + @QueryParam(PaginationUtil.ORDER_BY) final String orderBy, + @Parameter(description = "Sorting direction: ASC or DESC") + @DefaultValue("ASC") @QueryParam(PaginationUtil.DIRECTION) final String direction) + throws DotDataException { + + final InitDataObject initData = new WebResource.InitBuilder(this.webResource) + .requiredBackendUser(true).requiredFrontendUser(false) + .requiredPortlet("roles") + .requestAndResponse(request, response) + .rejectWhenNoUser(true).init(); + + Logger.debug(this, () -> "Loading the users directly granted the role: " + roleId); + + final Role role = this.roleAPI.loadRoleById(roleId); + if (null == role || !UtilMethods.isSet(role.getId())) { + + throw new DoesNotExistException("The role: " + roleId + " does not exist"); + } + + final OrderDirection orderDirection = OrderDirection.valueOf(direction); + + // UserPaginator reads ordering from FilteringParams keys, not from the + // PaginationUtil orderBy/direction arguments, so pass them explicitly (the + // same wiring /v1/users/filter uses). The direction value is enum-gated and + // mapped to the SQLUtil constants FilteringParams expects (leading space). + final Map extraParams = new HashMap<>( + Map.of(UserPaginator.ROLES_PARAM, List.of(role), + UserAPI.FilteringParams.ORDER_DIRECTION_PARAM, + OrderDirection.DESC == orderDirection ? SQLUtil._DESC : SQLUtil._ASC)); + if (UtilMethods.isSet(orderBy)) { + extraParams.put(UserAPI.FilteringParams.ORDER_BY_PARAM, orderBy); + } + + final PaginationUtilParams, List>> params = + new PaginationUtilParams.Builder, List>>() + .withRequest(request).withResponse(response) + .withUser(initData.getUser()).withFilter(filter) + .withPage(page).withPerPage(perPage) + .withOrderBy(orderBy).withDirection(orderDirection) + .withExtraParams(extraParams).build(); + + return this.userPaginationUtil.getPageView(params); + } + /** * Load role based on the role id. @@ -867,15 +974,9 @@ public Response loadRoleByRoleId(@Context final HttpServletRequest request, throw new DoesNotExistException("The role: " + roleId + " does not exists"); } - final List childrenRoles = new ArrayList<>(); - if(loadChildrenRoles){ - final List roleChildrenIdList = null!=role.getRoleChildren() ? role.getRoleChildren() : new ArrayList<>(); - for(final String childRoleId : roleChildrenIdList){ - childrenRoles.add(new RoleView(this.roleAPI.loadRoleById(childRoleId),new ArrayList<>())); - } - } - - return Response.ok(new ResponseEntityRoleDetailView(new RoleView(role,childrenRoles))).build(); + return Response.ok(new ResponseEntityRoleDetailView( + this.roleHelper.toRoleViews(List.of(role), loadChildrenRoles, this.roleAPI) + .get(0))).build(); } @@ -917,26 +1018,10 @@ public Response loadRootRoles(@Context final HttpServletRequest request, .requiredFrontendUser(false).requestAndResponse(request, response) .rejectWhenNoUser(true).init(); - final List rootRolesView = new ArrayList<>(); final List rootRoles = this.roleAPI.findRootRoles(); - if(loadChildrenRoles){ - for(final Role role : rootRoles) { - final List childrenRoles = new ArrayList<>(); - final List roleChildrenIdList = - null != role.getRoleChildren() ? role.getRoleChildren() : new ArrayList<>(); - for (final String childRoleId : roleChildrenIdList) { - childrenRoles.add(new RoleView(this.roleAPI.loadRoleById(childRoleId), - new ArrayList<>())); - } - rootRolesView.add(new RoleView(role,childrenRoles)); - } - } else { - rootRoles.stream() - .forEach(role -> rootRolesView.add(new RoleView(role, new ArrayList<>()))); - } - - return Response.ok(new ResponseEntityRoleViewListView(rootRolesView)).build(); + return Response.ok(new ResponseEntityRoleViewListView( + this.roleHelper.toRoleViews(rootRoles, loadChildrenRoles, this.roleAPI))).build(); } /** @@ -1140,13 +1225,10 @@ public ResponseEntityRoleViewListView loadUserRoles(@Context final HttpServletRe throw new com.dotmarketing.business.NoSuchUserException("No user found with id: " + userIdOrEmail); } - final List userRolesView = new ArrayList<>(); final List userRoles = this.roleAPI.loadRolesForUser(userRecover.getUserId()); - userRoles.stream() - .forEach(role -> userRolesView.add(new RoleView(role, new ArrayList<>()))); - - return new ResponseEntityRoleViewListView(userRolesView); + return new ResponseEntityRoleViewListView( + this.roleHelper.toRoleViews(userRoles, false, this.roleAPI)); } final String forbiddenMessage = "The User: " + modUser.getUserId() + " does not have permissions to retrieve users roles"; diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleView.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleView.java index 7e908b3c773f..a4aed507cc24 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleView.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleView.java @@ -1,6 +1,7 @@ package com.dotcms.rest.api.v1.system.role; import com.dotmarketing.business.Role; +import io.swagger.v3.oas.annotations.media.Schema; import java.util.List; /** @@ -24,8 +25,10 @@ public class RoleView { private final boolean locked; private final boolean system; private final List roleChildren; + private final int childCount; + private final int userCount; - public RoleView(final Role role, final List roleChildren){ + public RoleView(final Role role, final List roleChildren, final int userCount){ this.id = role.getId(); this.name = role.getName(); this.description = role.getDescription(); @@ -39,6 +42,8 @@ public RoleView(final Role role, final List roleChildren){ this.locked = role.isLocked(); this.system = role.isSystem(); this.roleChildren = roleChildren; + this.childCount = null != role.getRoleChildren() ? role.getRoleChildren().size() : 0; + this.userCount = userCount; } @@ -93,4 +98,18 @@ public boolean isSystem() { public List getRoleChildren() { return roleChildren; } + + @Schema(description = "Number of direct child roles, independent of children hydration", + example = "3", requiredMode = Schema.RequiredMode.REQUIRED, minimum = "0") + public int getChildCount() { + return childCount; + } + + @Schema(description = "Number of users directly granted this role, matching the totals of " + + "the role users listing: inherited grants and hidden users (system, anonymous, " + + "default, flagged for deletion) are not included", + example = "12", requiredMode = Schema.RequiredMode.REQUIRED, minimum = "0") + public int getUserCount() { + return userCount; + } } diff --git a/dotCMS/src/main/java/com/dotmarketing/business/RoleAPI.java b/dotCMS/src/main/java/com/dotmarketing/business/RoleAPI.java index 88564bc54262..870de072a29d 100644 --- a/dotCMS/src/main/java/com/dotmarketing/business/RoleAPI.java +++ b/dotCMS/src/main/java/com/dotmarketing/business/RoleAPI.java @@ -4,7 +4,9 @@ import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.exception.DotSecurityException; import com.liferay.portal.model.User; +import java.util.Collection; import java.util.List; +import java.util.Map; public interface RoleAPI { @@ -323,9 +325,22 @@ public interface RoleAPI { * @throws DotDataException */ List findUserIdsForRole(Role role) throws DotDataException; - + /** - * + * Counts the users directly granted each of the given roles, resolved with one aggregated + * query per call (chunked for large inputs). Grants inherited through the role hierarchy + * are not included, and neither are users hidden from the user listing (the system and + * anonymous users, the default user, and users flagged for deletion), so the counts always + * match the totals returned by the role users endpoint. + * + * @param roleIds the role ids to count direct user grants for + * @return a map of role id to direct-user count; ids with no countable grants are absent + * @throws DotDataException if the count query fails + */ + Map countUsersByRoleIds(Collection roleIds) throws DotDataException; + + /** + * * @param FQN * @return * @throws DotDataException diff --git a/dotCMS/src/main/java/com/dotmarketing/business/RoleAPIImpl.java b/dotCMS/src/main/java/com/dotmarketing/business/RoleAPIImpl.java index 8074eeaae409..6952393242f4 100644 --- a/dotCMS/src/main/java/com/dotmarketing/business/RoleAPIImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/business/RoleAPIImpl.java @@ -20,7 +20,9 @@ import com.liferay.util.SystemProperties; import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; @@ -388,6 +390,12 @@ public List findUserIdsForRole(final Role role) throws DotDataException return roleFactory.findUserIdsForRole(role); } + @CloseDBIfOpened + @Override + public Map countUsersByRoleIds(final Collection roleIds) throws DotDataException { + return roleFactory.countUsersByRoleIds(roleIds); + } + @Override public List findUsersForRole(final Role role) throws DotDataException, NoSuchUserException, DotSecurityException { List uids = findUserIdsForRole(role); diff --git a/dotCMS/src/main/java/com/dotmarketing/business/RoleFactory.java b/dotCMS/src/main/java/com/dotmarketing/business/RoleFactory.java index f953586f7488..49ce9b7499e1 100644 --- a/dotCMS/src/main/java/com/dotmarketing/business/RoleFactory.java +++ b/dotCMS/src/main/java/com/dotmarketing/business/RoleFactory.java @@ -3,7 +3,9 @@ */ package com.dotmarketing.business; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import com.dotmarketing.exception.DotDataException; @@ -70,6 +72,8 @@ public abstract class RoleFactory { protected abstract List findUserIdsForRole(Role role,boolean includeInherited) throws DotDataException; protected abstract List findUserIdsForRole(Role role) throws DotDataException; + + protected abstract Map countUsersByRoleIds(Collection roleIds) throws DotDataException; protected abstract List loadLayoutIdsForRole(Role role) throws DotDataException; diff --git a/dotCMS/src/main/java/com/dotmarketing/business/RoleFactoryImpl.java b/dotCMS/src/main/java/com/dotmarketing/business/RoleFactoryImpl.java index 753368524810..6f892d717727 100644 --- a/dotCMS/src/main/java/com/dotmarketing/business/RoleFactoryImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/business/RoleFactoryImpl.java @@ -16,6 +16,8 @@ import java.lang.reflect.InvocationTargetException; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -23,6 +25,7 @@ import java.util.Optional; import java.util.Queue; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.beanutils.BeanUtils; /** @@ -572,6 +575,46 @@ private List getUserIdsForRoleIds ( StringBuffer inRolesIdsQuery ) throw return result; } + @Override + protected Map countUsersByRoleIds(final Collection roleIds) throws DotDataException { + + final Map counts = new HashMap<>(); + if (roleIds == null || roleIds.isEmpty()) { + return counts; + } + + // Chunked like findUserIdsForRole above, to keep IN lists bounded + final List ids = new ArrayList<>(roleIds); + final int chunkSize = 100; + for (int from = 0; from < ids.size(); from += chunkSize) { + + final List chunk = ids.subList(from, Math.min(from + chunkSize, ids.size())); + final String placeholders = chunk.stream().map(id -> "?") + .collect(Collectors.joining(",")); + + final DotConnect dc = new DotConnect(); + // Mirrors the visibility rules of UserFactoryImpl.getUsersByName (system, anonymous, + // default and delete-in-progress users excluded) so the count always matches the + // totals returned by the users listing + dc.setSQL("select ur.role_id, count(distinct ur.user_id) as user_count" + + " from users_cms_roles ur join user_ u on u.userid = ur.user_id" + + " where ur.role_id in (" + placeholders + ")" + + " and u.userid <> 'system' and u.userid <> 'anonymous'" + + " and u.companyid <> ? and u.delete_in_progress = " + + DbConnectionFactory.getDBFalse() + + " group by ur.role_id"); + chunk.forEach(dc::addParam); + dc.addParam(User.DEFAULT); + + for (final Map row : dc.loadObjectResults()) { + counts.put(row.get("role_id").toString(), + Integer.valueOf(row.get("user_count").toString())); + } + } + + return counts; + } + @Override protected List findUserIdsForRole(Role role) throws DotDataException { HibernateUtil hu = new HibernateUtil(Role.class); diff --git a/dotCMS/src/main/java/com/dotmarketing/business/UserFactoryImpl.java b/dotCMS/src/main/java/com/dotmarketing/business/UserFactoryImpl.java index 285558b7f1d2..bb5839982acc 100644 --- a/dotCMS/src/main/java/com/dotmarketing/business/UserFactoryImpl.java +++ b/dotCMS/src/main/java/com/dotmarketing/business/UserFactoryImpl.java @@ -229,8 +229,9 @@ private static void addUserFilterParams(final DotConnect dotConnect, final Strin /** * Appends the parameterized {@code EXISTS} sub-query restricting results to users that hold - * any of the specified Roles. Callers must bind one parameter per Role at the same SQL - * position via {@link #addRoleFilterParams(DotConnect, List)}. + * any of the specified Roles, matched by role id so roles without a roleKey are supported. + * Callers must bind one parameter per Role at the same SQL position via + * {@link #addRoleFilterParams(DotConnect, List)}. */ private static void appendRoleFilter(final StringBuilder sql, final List roles) { if (!UtilMethods.isSet(roles)) { @@ -238,20 +239,46 @@ private static void appendRoleFilter(final StringBuilder sql, final List r } final String placeholders = roles.stream().map(role -> "?") .collect(Collectors.joining(StringPool.COMMA)); - sql.append(" and exists ( select ur.user_id from users_cms_roles ur join cms_role r on ur.role_id = r.id where r.role_key in (") + sql.append(" and exists ( select ur.user_id from users_cms_roles ur where ur.role_id in (") .append(placeholders) .append(") and ur.user_id = user_.userId )"); } /** - * Binds the Role keys expected by the placeholders appended via - * {@link #appendRoleFilter(StringBuilder, List)}. + * Binds the Role ids expected by the placeholders appended via + * {@link #appendRoleFilter(StringBuilder, List)}, resolving each id defensively through + * {@link #resolveRoleId(Role)}. */ private static void addRoleFilterParams(final DotConnect dotConnect, final List roles) { if (!UtilMethods.isSet(roles)) { return; } - roles.forEach(role -> dotConnect.addParam(role.getRoleKey())); + roles.forEach(role -> dotConnect.addParam(resolveRoleId(role))); + } + + /** + * Returns the role's id, falling back to a lookup by roleKey for Role instances that were + * not loaded through the API and carry only a key. A role that does not resolve returns + * null, which matches no users (the same behavior the previous role_key binding had for + * unresolvable roles). A lookup infrastructure failure propagates instead of masquerading + * as an empty result. + */ + private static String resolveRoleId(final Role role) { + if (UtilMethods.isSet(role.getId())) { + return role.getId(); + } + if (UtilMethods.isSet(role.getRoleKey())) { + try { + final Role loaded = APILocator.getRoleAPI().loadRoleByKey(role.getRoleKey()); + if (null != loaded && UtilMethods.isSet(loaded.getId())) { + return loaded.getId(); + } + } catch (final DotDataException e) { + throw new DotRuntimeException( + "Unable to resolve role id for roleKey: " + role.getRoleKey(), e); + } + } + return null; } @Override diff --git a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml index 0f409e597c54..72bda77f8a84 100644 --- a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml +++ b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml @@ -15537,6 +15537,75 @@ paths: summary: Remove users from a role tags: - Roles + get: + description: "Returns the paginated list of users directly granted the given\ + \ role, using the standard user serialization (email address included). Grants\ + \ inherited through the role hierarchy are not included." + operationId: loadUsersByRoleId + parameters: + - description: Id of the role to list users for + in: path + name: roleid + required: true + schema: + type: string + - description: "Filter matching user id, first name, last name, email or full\ + \ name" + in: query + name: filter + schema: + type: string + - description: Page number for pagination + in: query + name: page + schema: + type: integer + format: int32 + default: 1 + - description: Number of items per page + in: query + name: per_page + schema: + type: integer + format: int32 + default: 40 + - description: Column name for sorting results + in: query + name: orderby + schema: + type: string + - description: "Sorting direction: ASC or DESC" + in: query + name: direction + schema: + type: string + default: ASC + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/ResponseEntityPaginatedDataView" + description: Users retrieved successfully + "400": + content: + application/json: {} + description: Bad request - invalid pagination or sorting parameters + "401": + content: + application/json: {} + description: Unauthorized - authentication required + "403": + content: + application/json: {} + description: Forbidden - roles portlet access required + "404": + content: + application/json: {} + description: Role not found + summary: Get the users directly granted a role + tags: + - Roles /v1/roles/{roleid}/users/{userId}: post: description: "Grants the role to the user as a DIRECT membership. The operation\ @@ -36534,6 +36603,12 @@ components: RoleView: type: object properties: + childCount: + type: integer + format: int32 + description: "Number of direct child roles, independent of children hydration" + example: 3 + minimum: 0 dbfqn: type: string description: @@ -36562,6 +36637,17 @@ components: type: string system: type: boolean + userCount: + type: integer + format: int32 + description: "Number of users directly granted this role, matching the totals\ + \ of the role users listing: inherited grants and hidden users (system,\ + \ anonymous, default, flagged for deletion) are not included" + example: 12 + minimum: 0 + required: + - childCount + - userCount RowField: type: object allOf: diff --git a/dotcms-integration/src/test/java/com/dotcms/MainSuite2b.java b/dotcms-integration/src/test/java/com/dotcms/MainSuite2b.java index d864203e1850..b17ca5e09497 100644 --- a/dotcms-integration/src/test/java/com/dotcms/MainSuite2b.java +++ b/dotcms-integration/src/test/java/com/dotcms/MainSuite2b.java @@ -102,6 +102,8 @@ import com.dotcms.rest.api.v1.announcements.RemoteAnnouncementsLoaderIntegrationTest; import com.dotcms.rest.api.v1.apps.SiteViewPaginatorIntegrationTest; import com.dotcms.rest.api.v1.apps.view.AppsInterpolationTest; +import com.dotcms.rest.api.v1.system.role.RoleResourceCountsIntegrationTest; +import com.dotcms.rest.api.v1.system.role.RoleResourceUsersIntegrationTest; import com.dotcms.rest.api.v1.asset.AssetPathResolverImplIntegrationTest; import com.dotcms.rest.api.v1.asset.WebAssetHelperIntegrationTest; import com.dotcms.rest.api.v1.authentication.ResetPasswordTokenUtilTest; @@ -567,6 +569,8 @@ CustomAttributeFactoryTest.class, PermissionResourceIntegrationTest.class, FileAssetBaseTypeToContentTypeStrategyImplTest.class, + RoleResourceCountsIntegrationTest.class, + RoleResourceUsersIntegrationTest.class, }) public class MainSuite2b { diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceCountsIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceCountsIntegrationTest.java new file mode 100644 index 000000000000..b40600d36c53 --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceCountsIntegrationTest.java @@ -0,0 +1,304 @@ +package com.dotcms.rest.api.v1.system.role; + +import com.dotcms.datagen.RoleDataGen; +import com.dotcms.datagen.UserDataGen; +import com.dotcms.mock.request.MockAttributeRequest; +import com.dotcms.mock.request.MockHeaderRequest; +import com.dotcms.mock.request.MockHttpRequestIntegrationTest; +import com.dotcms.mock.request.MockSessionRequest; +import com.dotcms.mock.response.MockHttpResponse; +import com.dotcms.rest.ResponseEntityPaginatedDataView; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.business.RoleAPI; +import com.dotmarketing.common.db.DotConnect; +import com.dotmarketing.db.DbConnectionFactory; +import com.liferay.portal.model.User; +import org.junit.BeforeClass; +import org.junit.Test; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.core.Response; +import java.util.Base64; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +/** + * Integration tests for the {@code childCount} / {@code userCount} fields on {@link RoleView} + * and the backing {@link RoleAPI#countUsersByRoleIds(java.util.Collection)} aggregate. + * See https://github.com/dotCMS/core/issues/37071 + * + * @author hassandotcms + */ +public class RoleResourceCountsIntegrationTest { + + static HttpServletResponse response; + static RoleResource resource; + static RoleAPI roleAPI; + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + resource = new RoleResource(); + roleAPI = APILocator.getRoleAPI(); + response = new MockHttpResponse(); + } + + private static HttpServletRequest mockAdminRequest() { + final MockHeaderRequest request = new MockHeaderRequest( + new MockSessionRequest( + new MockAttributeRequest( + new MockHttpRequestIntegrationTest("localhost", "/").request()) + .request()) + .request()); + request.setHeader("Authorization", + "Basic " + Base64.getEncoder().encodeToString("admin@dotcms.com:admin".getBytes())); + return request; + } + + @SuppressWarnings("unchecked") + private static RoleView loadRoleView(final String roleId, final boolean loadChildren) + throws Exception { + final Response restResponse = resource.loadRoleByRoleId( + mockAdminRequest(), response, roleId, loadChildren); + assertEquals(Response.Status.OK.getStatusCode(), restResponse.getStatus()); + final ResponseEntityRoleDetailView view = + (ResponseEntityRoleDetailView) restResponse.getEntity(); + return view.getEntity(); + } + + /** + * Given a role with N direct children, when it is loaded through the endpoint, + * then childCount equals N regardless of the loadChildrenRoles flag. + */ + @Test + public void childCount_matchesDirectChildren() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role childA = new RoleDataGen().parent(parent.getId()).nextPersisted(); + final Role childB = new RoleDataGen().parent(parent.getId()).nextPersisted(); + final Role childC = new RoleDataGen().parent(parent.getId()).nextPersisted(); + + final RoleView withChildren = loadRoleView(parent.getId(), true); + assertEquals(3, withChildren.getChildCount()); + assertEquals(3, withChildren.getRoleChildren().size()); + + final RoleView withoutChildren = loadRoleView(parent.getId(), false); + assertEquals("childCount must not depend on children hydration", + 3, withoutChildren.getChildCount()); + assertTrue(withoutChildren.getRoleChildren().isEmpty()); + + assertEquals("a leaf role must report zero children", + 0, loadRoleView(childA.getId(), true).getChildCount()); + assertNotNull(childB); + assertNotNull(childC); + } + + /** + * Given a role with M directly granted users, userCount equals M. + */ + @Test + public void userCount_matchesDirectGrants() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final User userA = new UserDataGen().roles(role).nextPersisted(); + final User userB = new UserDataGen().roles(role).nextPersisted(); + + assertNotNull(userA); + assertNotNull(userB); + assertEquals(2, loadRoleView(role.getId(), false).getUserCount()); + } + + /** + * Grants are direct-only in both directions: a grant on the parent does not count + * for the child, and a grant on the child does not count for the parent. + */ + @Test + public void userCount_excludesInheritedGrants() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(parent.getId()).nextPersisted(); + final User parentUser = new UserDataGen().roles(parent).nextPersisted(); + assertNotNull(parentUser); + + assertEquals(1, loadRoleView(parent.getId(), false).getUserCount()); + assertEquals("a grant on the parent must not count for the child", + 0, loadRoleView(child.getId(), false).getUserCount()); + + final User childUser = new UserDataGen().roles(child).nextPersisted(); + assertNotNull(childUser); + assertEquals("a grant on the child must not count for the parent", + 1, loadRoleView(parent.getId(), false).getUserCount()); + assertEquals(1, loadRoleView(child.getId(), false).getUserCount()); + } + + /** + * A role with no children and no grants reports 0 for both fields. + */ + @Test + public void counts_zeroForEmptyRole() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final RoleView view = loadRoleView(role.getId(), true); + assertEquals(0, view.getChildCount()); + assertEquals(0, view.getUserCount()); + } + + /** + * When children are hydrated, each child view carries its own counts too. + */ + @Test + public void childViews_carryTheirOwnCounts() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(parent.getId()).nextPersisted(); + final Role grandChild = new RoleDataGen().parent(child.getId()).nextPersisted(); + final User childUser = new UserDataGen().roles(child).nextPersisted(); + assertNotNull(grandChild); + assertNotNull(childUser); + + final RoleView parentView = loadRoleView(parent.getId(), true); + assertEquals(1, parentView.getRoleChildren().size()); + final RoleView childView = parentView.getRoleChildren().get(0); + assertEquals(child.getId(), childView.getId()); + assertEquals(1, childView.getChildCount()); + assertEquals(1, childView.getUserCount()); + } + + /** + * The root-roles listing carries both fields on every view. + */ + @Test + public void loadRootRoles_carriesCounts() throws Exception { + final Role rootRole = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(rootRole.getId()).nextPersisted(); + final User granted = new UserDataGen().roles(rootRole).nextPersisted(); + assertNotNull(child); + assertNotNull(granted); + + final Response restResponse = resource.loadRootRoles(mockAdminRequest(), response, false); + assertEquals(Response.Status.OK.getStatusCode(), restResponse.getStatus()); + final ResponseEntityRoleViewListView view = + (ResponseEntityRoleViewListView) restResponse.getEntity(); + final RoleView rootView = view.getEntity().stream() + .filter(roleView -> rootRole.getId().equals(roleView.getId())) + .findFirst().orElse(null); + assertNotNull("the created root role must be in the listing", rootView); + assertEquals(1, rootView.getChildCount()); + assertEquals(1, rootView.getUserCount()); + } + + /** + * The roles-of-a-user listing carries both fields; the user's own user-role + * reports exactly one direct grant (the creation-time self-grant). + */ + @Test + public void loadUserRoles_carriesCounts() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(role.getId()).nextPersisted(); + final User user = new UserDataGen().roles(role).nextPersisted(); + final Role userRole = roleAPI.getUserRole(user); + assertNotNull(child); + + final ResponseEntityRoleViewListView view = resource.loadUserRoles( + mockAdminRequest(), response, user.getUserId()); + final List roleViews = view.getEntity(); + assertFalse(roleViews.isEmpty()); + + final RoleView grantedView = roleViews.stream() + .filter(roleView -> role.getId().equals(roleView.getId())) + .findFirst().orElse(null); + assertNotNull("the granted role must be listed", grantedView); + assertEquals(1, grantedView.getUserCount()); + assertEquals("roles from loadRolesForUser must carry a hydrated childCount", + 1, grantedView.getChildCount()); + + final RoleView userRoleView = roleViews.stream() + .filter(roleView -> userRole.getId().equals(roleView.getId())) + .findFirst().orElse(null); + assertNotNull("the user's own user-role must be listed", userRoleView); + assertEquals("a user-role holds exactly its self-grant", 1, userRoleView.getUserCount()); + } + + /** + * userCount matches the users the companion GET /{roleid}/users endpoint actually + * returns: hidden users (the system user, users flagged delete_in_progress) are + * excluded from the count exactly like they are excluded from the listing, so the + * tree badge always equals the Users tab total. + */ + @Test + public void userCount_matchesVisibleUsersOnly() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final User visible = new UserDataGen().roles(role).nextPersisted(); + final User deleting = new UserDataGen().roles(role).nextPersisted(); + roleAPI.addRoleToUser(role, APILocator.systemUser()); + new DotConnect().setSQL("update user_ set delete_in_progress = " + + DbConnectionFactory.getDBTrue() + " where userid = ?") + .addParam(deleting.getUserId()).loadResult(); + + assertEquals("hidden users must not be counted", + 1, loadRoleView(role.getId(), false).getUserCount()); + + final ResponseEntityPaginatedDataView usersView = resource.loadUsersByRoleId( + mockAdminRequest(), new MockHttpResponse(), role.getId(), + null, 1, 40, null, "ASC"); + assertEquals("the badge must equal the users listing total", + (long) loadRoleView(role.getId(), false).getUserCount(), + usersView.getPagination().getTotalEntries()); + assertNotNull(visible); + } + + /** + * Security: hostile SQL payloads passed as role ids are inert. The IN-list is built + * from constant "?" placeholders and every id is bound as a PreparedStatement + * parameter, so metacharacters, stacked statements, tautologies and UNIONs must be + * treated as data: the query returns nothing for them, throws nothing, and the + * tables remain intact for subsequent legitimate queries. + * Covers the Semgrep CUSTOM_INJECTION-2 findings on countUsersByRoleIds. + */ + @Test + public void countUsersByRoleIds_hostileIdsAreInert() throws Exception { + final Role legit = new RoleDataGen().nextPersisted(); + new UserDataGen().roles(legit).nextPersisted(); + + final Map counts = roleAPI.countUsersByRoleIds(List.of( + "'; drop table users_cms_roles; --", + "x' OR '1'='1", + "?) union select userid, 1 from user_ --", + "1; update user_ set delete_in_progress = true; --", + legit.getId())); + + assertEquals("hostile ids must match nothing, legit id must still count", + 1, counts.size()); + assertEquals(Integer.valueOf(1), counts.get(legit.getId())); + + final Map after = roleAPI.countUsersByRoleIds(List.of(legit.getId())); + assertEquals("tables must be intact after the hostile call", + Integer.valueOf(1), after.get(legit.getId())); + } + + /** + * The aggregate itself: one call resolves counts for many roles; ids without + * grants are absent from the map; empty input returns an empty map. + */ + @Test + public void countUsersByRoleIds_batchesCorrectly() throws Exception { + final Role grantedTwice = new RoleDataGen().nextPersisted(); + final Role grantedOnce = new RoleDataGen().nextPersisted(); + final Role neverGranted = new RoleDataGen().nextPersisted(); + new UserDataGen().roles(grantedTwice).nextPersisted(); + new UserDataGen().roles(grantedTwice).nextPersisted(); + new UserDataGen().roles(grantedOnce).nextPersisted(); + + final Map counts = roleAPI.countUsersByRoleIds( + List.of(grantedTwice.getId(), grantedOnce.getId(), neverGranted.getId())); + assertEquals(Integer.valueOf(2), counts.get(grantedTwice.getId())); + assertEquals(Integer.valueOf(1), counts.get(grantedOnce.getId())); + assertFalse("ids with no grants must be absent", + counts.containsKey(neverGranted.getId())); + + assertTrue(roleAPI.countUsersByRoleIds(List.of()).isEmpty()); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceUsersIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceUsersIntegrationTest.java new file mode 100644 index 000000000000..b446e155e16b --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceUsersIntegrationTest.java @@ -0,0 +1,230 @@ +package com.dotcms.rest.api.v1.system.role; + +import com.dotcms.datagen.RoleDataGen; +import com.dotcms.datagen.UserDataGen; +import com.dotcms.mock.request.MockAttributeRequest; +import com.dotcms.mock.request.MockHeaderRequest; +import com.dotcms.mock.request.MockHttpRequestIntegrationTest; +import com.dotcms.mock.request.MockSessionRequest; +import com.dotcms.mock.response.MockHttpResponse; +import com.dotcms.rest.ResponseEntityPaginatedDataView; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.business.RoleAPI; +import com.dotmarketing.exception.DoesNotExistException; +import com.dotmarketing.util.UUIDGenerator; +import com.liferay.portal.model.User; +import org.junit.BeforeClass; +import org.junit.Test; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.Base64; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Integration tests for {@code GET /v1/roles/{roleId}/users}: the paginated list of users + * directly granted a role. See https://github.com/dotCMS/core/issues/37070 + * + * @author hassandotcms + */ +public class RoleResourceUsersIntegrationTest { + + static HttpServletResponse response; + static RoleResource resource; + static RoleAPI roleAPI; + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + resource = new RoleResource(); + roleAPI = APILocator.getRoleAPI(); + response = new MockHttpResponse(); + } + + private static MockSessionRequest baseRequest() { + return new MockSessionRequest( + new MockAttributeRequest( + new MockHttpRequestIntegrationTest("localhost", "/").request()) + .request()); + } + + private static HttpServletRequest mockAdminRequest() { + final MockHeaderRequest request = new MockHeaderRequest(baseRequest().request()); + request.setHeader("Authorization", + "Basic " + Base64.getEncoder().encodeToString("admin@dotcms.com:admin".getBytes())); + return request; + } + + private static ResponseEntityPaginatedDataView loadUsers(final String roleId, + final String filter, final int page, final int perPage) throws Exception { + return resource.loadUsersByRoleId(mockAdminRequest(), new MockHttpResponse(), + roleId, filter, page, perPage, null, "ASC"); + } + + @SuppressWarnings("unchecked") + private static List> users(final ResponseEntityPaginatedDataView view) { + return (List>) view.getEntity(); + } + + private static List userIds(final ResponseEntityPaginatedDataView view) { + return users(view).stream().map(map -> map.get("userId").toString()) + .collect(Collectors.toList()); + } + + /** + * Directly granted users come back with the standard user detail fields, email included. + */ + @Test + public void directGrants_returnUserDetailFields() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final User userA = new UserDataGen().roles(role).nextPersisted(); + final User userB = new UserDataGen().roles(role).nextPersisted(); + + final ResponseEntityPaginatedDataView view = loadUsers(role.getId(), null, 1, 40); + assertEquals(2, view.getPagination().getTotalEntries()); + assertTrue(userIds(view).containsAll(List.of(userA.getUserId(), userB.getUserId()))); + for (final Map userMap : users(view)) { + assertTrue("emailAddress must be present", + userMap.get("emailAddress").toString().contains("@")); + assertFalse(userMap.get("firstName").toString().isEmpty()); + assertFalse(userMap.get("lastName").toString().isEmpty()); + } + } + + /** + * The headline regression: a role WITHOUT a roleKey returns its granted users. + */ + @Test + public void keylessRole_returnsUsers() throws Exception { + final Role keylessRole = new RoleDataGen().key(null).nextPersisted(); + final User granted = new UserDataGen().roles(keylessRole).nextPersisted(); + + final ResponseEntityPaginatedDataView view = loadUsers(keylessRole.getId(), null, 1, 40); + assertEquals(1, view.getPagination().getTotalEntries()); + assertTrue(userIds(view).contains(granted.getUserId())); + } + + /** + * Inheritance is a client concern: users granted only on an ancestor are not returned + * for the descendant role. + */ + @Test + public void ancestorGrants_notIncluded() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(parent.getId()).nextPersisted(); + new UserDataGen().roles(parent).nextPersisted(); + + final ResponseEntityPaginatedDataView view = loadUsers(child.getId(), null, 1, 40); + assertEquals(0, view.getPagination().getTotalEntries()); + assertTrue(users(view).isEmpty()); + } + + /** + * A user-role is served uniformly: it returns the user themself via the creation-time + * self-grant row. + */ + @Test + public void userRole_returnsSelfGrant() throws Exception { + final User user = new UserDataGen().nextPersisted(); + final Role userRole = roleAPI.getUserRole(user); + + final ResponseEntityPaginatedDataView view = loadUsers(userRole.getId(), null, 1, 40); + assertEquals(1, view.getPagination().getTotalEntries()); + assertEquals(user.getUserId(), userIds(view).get(0)); + } + + /** + * The filter matches on name and on email address. + */ + @Test + public void filter_matchesNameAndEmail() throws Exception { + final String unique = "roleusers" + System.currentTimeMillis(); + final Role role = new RoleDataGen().nextPersisted(); + final User byName = new UserDataGen().firstName(unique + "Alpha").roles(role).nextPersisted(); + final User byEmail = new UserDataGen() + .emailAddress(unique + "beta@filter.test").roles(role).nextPersisted(); + + final List nameMatches = userIds(loadUsers(role.getId(), unique + "Alpha", 1, 40)); + assertEquals(List.of(byName.getUserId()), nameMatches); + + final List emailMatches = userIds(loadUsers(role.getId(), unique + "beta", 1, 40)); + assertEquals(List.of(byEmail.getUserId()), emailMatches); + } + + /** + * The direction parameter orders the listing. Without an orderBy the listing sorts by + * full name, so DESC must return the exact reverse of ASC. + */ + @Test + public void direction_ordersTheListing() throws Exception { + final String unique = "roleorder" + System.currentTimeMillis(); + final Role role = new RoleDataGen().nextPersisted(); + final User first = new UserDataGen().firstName("Aaa" + unique).roles(role).nextPersisted(); + final User last = new UserDataGen().firstName("Zzz" + unique).roles(role).nextPersisted(); + + final List ascending = userIds(resource.loadUsersByRoleId(mockAdminRequest(), + new MockHttpResponse(), role.getId(), unique, 1, 40, null, "ASC")); + assertEquals(List.of(first.getUserId(), last.getUserId()), ascending); + + final List descending = userIds(resource.loadUsersByRoleId(mockAdminRequest(), + new MockHttpResponse(), role.getId(), unique, 1, 40, null, "DESC")); + assertEquals("DESC must reverse the listing", + List.of(last.getUserId(), first.getUserId()), descending); + } + + /** + * Pagination boundaries: pages split the result set, the total stays constant, and a + * page past the end is empty. + */ + @Test + public void pagination_boundaries() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + new UserDataGen().roles(role).nextPersisted(); + new UserDataGen().roles(role).nextPersisted(); + new UserDataGen().roles(role).nextPersisted(); + + final ResponseEntityPaginatedDataView firstPage = loadUsers(role.getId(), null, 1, 2); + assertEquals(2, users(firstPage).size()); + assertEquals(3, firstPage.getPagination().getTotalEntries()); + + final ResponseEntityPaginatedDataView secondPage = loadUsers(role.getId(), null, 2, 2); + assertEquals(1, users(secondPage).size()); + assertEquals(3, secondPage.getPagination().getTotalEntries()); + + final ResponseEntityPaginatedDataView pastEnd = loadUsers(role.getId(), null, 5, 2); + assertTrue(users(pastEnd).isEmpty()); + } + + /** + * A missing role resolves to 404 through the DoesNotExistException mapper. + */ + @Test(expected = DoesNotExistException.class) + public void missingRole_throwsDoesNotExist() throws Exception { + loadUsers(UUIDGenerator.generateUuid(), null, 1, 40); + } + + /** + * A backend user without access to the roles portlet is rejected. + */ + @Test(expected = com.dotcms.rest.exception.SecurityException.class) + public void backendUserWithoutRolesPortlet_isRejected() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final User limited = new UserDataGen() + .roles(roleAPI.loadBackEndUserRole()).nextPersisted(); + + final MockSessionRequest request = baseRequest(); + request.getSession().setAttribute( + com.liferay.portal.util.WebKeys.USER_ID, limited.getUserId()); + + resource.loadUsersByRoleId(request.request(), new MockHttpResponse(), + role.getId(), null, 1, 40, null, "ASC"); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/user/UserResourceIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/user/UserResourceIntegrationTest.java index 10b0d4467db6..de86db162ddb 100644 --- a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/user/UserResourceIntegrationTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/user/UserResourceIntegrationTest.java @@ -1,8 +1,11 @@ package com.dotcms.rest.api.v1.user; +import com.dotcms.datagen.RoleDataGen; import com.dotcms.datagen.SiteDataGen; import com.dotcms.datagen.TestUserUtils; +import com.dotcms.datagen.UserDataGen; import com.dotcms.rest.ErrorResponseHelper; +import com.dotcms.rest.ResponseEntityView; import com.dotcms.rest.WebResource; import com.dotcms.rest.api.DotRestInstanceProvider; import com.dotcms.util.PaginationUtil; @@ -18,6 +21,7 @@ import com.dotmarketing.beans.Permission; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.PermissionAPI; +import com.dotmarketing.business.Role; import com.liferay.portal.model.User; import com.liferay.portal.util.WebKeys; import javax.servlet.http.HttpServletRequest; @@ -25,6 +29,9 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import java.util.Base64; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import static org.junit.Assert.*; import org.junit.BeforeClass; @@ -111,4 +118,54 @@ public void test_logoutAs_success() throws Exception { assertNull(request.getSession().getAttribute(WebKeys.USER)); assertNull(request.getSession().getAttribute(WebKeys.PRINCIPAL_USER_ID)); } + + @SuppressWarnings("unchecked") + private List filterUserIdsByRoleKeys(final String filter, final List roleKeys) { + final Response resourceResponse = resource.filter(mockRequest(), response, filter, 0, 40, + null, "ASC", false, false, null, 0, roleKeys); + assertEquals(Status.OK.getStatusCode(), resourceResponse.getStatus()); + final List> userMaps = (List>) + ((ResponseEntityView) resourceResponse.getEntity()).getEntity(); + return userMaps.stream().map(map -> map.get("userId").toString()) + .collect(Collectors.toList()); + } + + /** + * Method to test: {@link UserResource#filter} + * Given Scenario: A role with a roleKey has one user directly granted to it; another user + * with the same name prefix is not granted. The endpoint is called with that roleKey. + * ExpectedResult: Only the granted user is returned. + */ + @Test + public void test_filter_byRoleKey_returnsOnlyGrantedUsers() throws Exception { + final String unique = "rkFilter" + System.currentTimeMillis(); + final Role role = new RoleDataGen().key(unique + "Key").nextPersisted(); + final User granted = new UserDataGen().firstName(unique).roles(role).nextPersisted(); + final User notGranted = new UserDataGen().firstName(unique).nextPersisted(); + + final List userIds = filterUserIdsByRoleKeys(unique, List.of(role.getRoleKey())); + assertTrue("granted user must be returned", userIds.contains(granted.getUserId())); + assertFalse("user without the role must not be returned", + userIds.contains(notGranted.getUserId())); + } + + /** + * Method to test: {@link UserResource#filter} + * Given Scenario: Two roles with roleKeys hold one granted user each, sharing a name prefix. + * The endpoint is called with both roleKeys. + * ExpectedResult: Users holding any of the roles are returned. + */ + @Test + public void test_filter_byMultipleRoleKeys_returnsUnion() throws Exception { + final String unique = "rkUnion" + System.currentTimeMillis(); + final Role roleA = new RoleDataGen().key(unique + "A").nextPersisted(); + final Role roleB = new RoleDataGen().key(unique + "B").nextPersisted(); + final User userA = new UserDataGen().firstName(unique).roles(roleA).nextPersisted(); + final User userB = new UserDataGen().firstName(unique).roles(roleB).nextPersisted(); + + final List userIds = filterUserIdsByRoleKeys(unique, + List.of(roleA.getRoleKey(), roleB.getRoleKey())); + assertTrue(userIds.contains(userA.getUserId())); + assertTrue(userIds.contains(userB.getUserId())); + } } diff --git a/dotcms-integration/src/test/java/com/dotmarketing/business/UserAPITest.java b/dotcms-integration/src/test/java/com/dotmarketing/business/UserAPITest.java index 6c082f4b6c25..5894e2645a42 100644 --- a/dotcms-integration/src/test/java/com/dotmarketing/business/UserAPITest.java +++ b/dotcms-integration/src/test/java/com/dotmarketing/business/UserAPITest.java @@ -2,6 +2,7 @@ import com.dotcms.IntegrationTestBase; import com.dotcms.LicenseTestUtil; +import com.dotcms.datagen.RoleDataGen; import com.dotcms.datagen.TestUserUtils; import com.dotcms.datagen.UserDataGen; import com.dotcms.notifications.bean.Notification; @@ -1606,4 +1607,59 @@ public void testGetUsersByNameFilteredByRole() throws DotDataException { UserDataGen.remove(plainUser); } } + + /** + * Method to test: {@link UserAPI#getUsersByName(String, List, int, int)} and + * {@link UserAPI#getCountUsersByName(String, List)} + * Given Scenario: A role WITHOUT a roleKey has one user directly granted to it, and another + * user with the same name prefix is not granted. The search combines the shared name filter + * with that keyless role. + * ExpectedResult: The granted user is returned and counted. The role filter must work for + * roles that only carry an id; this powers GET /v1/roles/{roleId}/users for keyless roles. + */ + @Test + public void testGetUsersByNameFilteredByKeylessRole() throws DotDataException { + final String unique = String.valueOf(System.currentTimeMillis()); + final Role keylessRole = new RoleDataGen().key(null).nextPersisted(); + final User granted = new UserDataGen().firstName("keylessFilter" + unique + "granted") + .roles(keylessRole).nextPersisted(); + final User plainUser = new UserDataGen().firstName("keylessFilter" + unique + "plain").nextPersisted(); + try { + final List matches = userAPI.getUsersByName("keylessFilter" + unique, + List.of(keylessRole), 0, 40); + assertEquals(1, matches.size()); + assertEquals(granted.getUserId(), matches.get(0).getUserId()); + assertEquals(1, userAPI.getCountUsersByName("keylessFilter" + unique, List.of(keylessRole))); + } finally { + UserDataGen.remove(granted); + UserDataGen.remove(plainUser); + RoleDataGen.remove(keylessRole); + } + } + + /** + * Method to test: {@link UserAPI#getUsersByName(String, List, int, int)} + * Given Scenario: The roles filter receives a hand-built Role object carrying only a + * roleKey and no id, as an external UserAPI caller might construct it. + * ExpectedResult: The role is resolved through its key and the granted user is returned. + */ + @Test + public void testGetUsersByNameFilteredByKeyOnlyRoleObject() throws DotDataException { + final String unique = String.valueOf(System.currentTimeMillis()); + final Role persisted = new RoleDataGen().key("keyOnlyFilter" + unique).nextPersisted(); + final User granted = new UserDataGen().firstName("keyOnlyFilter" + unique) + .roles(persisted).nextPersisted(); + try { + final Role keyOnly = new Role(); + keyOnly.setRoleKey(persisted.getRoleKey()); + + final List matches = userAPI.getUsersByName("keyOnlyFilter" + unique, + List.of(keyOnly), 0, 40); + assertEquals(1, matches.size()); + assertEquals(granted.getUserId(), matches.get(0).getUserId()); + } finally { + UserDataGen.remove(granted); + RoleDataGen.remove(persisted); + } + } } \ No newline at end of file