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 c9971b65b5ef..9608386c6378 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 @@ -4,16 +4,32 @@ import com.dotcms.api.system.event.SystemEventType; import com.dotcms.api.system.event.SystemEventsAPI; import com.dotcms.business.WrapInTransaction; +import com.dotcms.rest.exception.BadRequestException; +import com.dotcms.rest.exception.ConflictException; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.DuplicateRoleException; +import com.dotmarketing.business.DuplicateRoleKeyException; import com.dotmarketing.business.Layout; import com.dotmarketing.business.LayoutAPI; import com.dotmarketing.business.Role; import com.dotmarketing.business.RoleAPI; +import com.dotmarketing.exception.DoesNotExistException; import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.exception.RoleNameException; +import com.dotmarketing.util.ActivityLogger; +import com.dotmarketing.util.AdminLogger; +import com.dotmarketing.util.DateUtil; import com.dotmarketing.util.UtilMethods; +import com.google.common.annotations.VisibleForTesting; +import com.liferay.portal.model.User; +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -21,8 +37,119 @@ * Helper to encapsulate Roles logic * @author jsanca */ +@ApplicationScoped public class RoleHelper { + private final RoleAPI roleAPI; + + public RoleHelper() { + this(APILocator.getRoleAPI()); + } + + @Inject + @VisibleForTesting + public RoleHelper(final RoleAPI roleAPI) { + this.roleAPI = roleAPI; + } + + /** + * Updates an existing role — name, key, description, can-grant flags, and parent + * (reparent). Mirrors the legacy DWR {@code RoleAjax#updateRole} behavior: a null + * {@code parentRoleId} turns the role into a root role (parent == own id). + * + * Guards (see #36936): + * + * + * @param roleId id of the role to update + * @param roleForm new field values (same shape POST /v1/roles consumes) + * @param modUser authenticated user performing the change (audit logging) + * @return the updated {@link Role} + */ + @WrapInTransaction + public Role updateRole(final String roleId, final RoleForm roleForm, final User modUser) + throws DotDataException, DotSecurityException { + + final Role role = this.roleAPI.loadRoleById(roleId); + if (null == role || !UtilMethods.isSet(role.getId())) { + throw new DoesNotExistException("Role not found: " + roleId); + } + + if (role.isSystem() || role.isLocked()) { + throw new DotSecurityException( + String.format("Role '%s' (%s) is a system or locked role and cannot be updated", + role.getName(), role.getId())); + } + + role.setName(roleForm.getRoleName()); + role.setRoleKey(roleForm.getRoleKey()); + role.setEditUsers(roleForm.isCanEditUsers()); + role.setEditPermissions(roleForm.isCanEditPermissions()); + role.setEditLayouts(roleForm.isCanEditLayouts()); + role.setDescription(roleForm.getDescription()); + + final String parentRoleId = roleForm.getParentRoleId(); + if (Objects.nonNull(parentRoleId)) { + + if (parentRoleId.equals(roleId)) { + throw new BadRequestException("A role cannot be its own parent: " + roleId); + } + + final Role parentRole = this.roleAPI.loadRoleById(parentRoleId); + if (null == parentRole || !UtilMethods.isSet(parentRole.getId())) { + throw new DoesNotExistException("Parent role not found: " + parentRoleId); + } + + // findRoleHierarchy walks getParent() up to the root, so it returns the proposed + // parent and all its ancestors — if the edited role is among them, the reparent + // would create a cycle + for (final Role ancestor : this.roleAPI.findRoleHierarchy(parentRole)) { + if (roleId.equals(ancestor.getId())) { + throw new BadRequestException(String.format( + "Cannot move role '%s' under '%s': the target parent is one of its descendants", + roleId, parentRoleId)); + } + } + + role.setParent(parentRole.getId()); + } else { + role.setParent(role.getId()); + } + + final String date = DateUtil.getCurrentDate(); + ActivityLogger.logInfo(getClass(), "Modifying Role", + "Date: " + date + "; User:" + modUser.getUserId() + "; RoleID: " + role.getId()); + AdminLogger.log(getClass(), "Modifying Role", + "Date: " + date + "; User:" + modUser.getUserId() + "; RoleID: " + role.getId()); + + final Role updatedRole; + try { + updatedRole = this.roleAPI.save(role); + } catch (final DuplicateRoleKeyException e) { + throw new ConflictException( + "A role with key '" + roleForm.getRoleKey() + "' already exists", e); + } catch (final DuplicateRoleException e) { + throw new ConflictException( + "A role named '" + roleForm.getRoleName() + "' already exists under the same parent", e); + } catch (final RoleNameException e) { + throw new BadRequestException("Role name is not valid: " + roleForm.getRoleName()); + } + + ActivityLogger.logInfo(getClass(), "Role Modified", + "Date: " + date + "; User:" + modUser.getUserId() + "; RoleID: " + role.getId()); + AdminLogger.log(getClass(), "Role Modified", + "Date: " + date + "; User:" + modUser.getUserId() + "; RoleID: " + role.getId()); + + return updatedRole; + } + /** * Saves only the existing layouts on layoutIds, any issue previous added not in the list will be removed * @param role 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 e7ec030fa6c1..a15fa6dccd4c 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 @@ -53,6 +53,7 @@ import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; import javax.ws.rs.POST; +import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; @@ -281,60 +282,144 @@ public Response addNewRole( ) final RoleForm roleForm) throws DotDataException, DotSecurityException { - final InitDataObject initDataObject = new WebResource.InitBuilder(this.webResource) - .requiredFrontendUser(false).rejectWhenNoUser(true) - .requiredBackendUser(true).requiredPortlet("roles") - .requestAndResponse(request, response).init(); + final User user = this.initRequireRolesPortletAndCmsAdmin(request, response); - if (this.roleAPI.doesUserHaveRole(initDataObject.getUser(), this.roleAPI.loadCMSAdminRole())) { + Role role = new Role(); + role.setName(roleForm.getRoleName()); + role.setRoleKey(roleForm.getRoleKey()); + role.setEditUsers(roleForm.isCanEditUsers()); + role.setEditPermissions(roleForm.isCanEditPermissions()); + role.setEditLayouts(roleForm.isCanEditLayouts()); + role.setDescription(roleForm.getDescription()); - final User user = initDataObject.getUser(); - Role role = new Role(); - role.setName(roleForm.getRoleName()); - role.setRoleKey(roleForm.getRoleKey()); - role.setEditUsers(roleForm.isCanEditUsers()); - role.setEditPermissions(roleForm.isCanEditPermissions()); - role.setEditLayouts(roleForm.isCanEditLayouts()); - role.setDescription(roleForm.getDescription()); + if(Objects.nonNull(roleForm.getParentRoleId())) { - if(Objects.nonNull(roleForm.getParentRoleId())) { + final Role parentRole = roleAPI.loadRoleById(roleForm.getParentRoleId()); + role.setParent(parentRole.getId()); + } - final Role parentRole = roleAPI.loadRoleById(roleForm.getParentRoleId()); - role.setParent(parentRole.getId()); - } + final String date = DateUtil.getCurrentDate(); - final String date = DateUtil.getCurrentDate(); + ActivityLogger.logInfo(getClass(), "Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); + AdminLogger.log(getClass(), "Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); - ActivityLogger.logInfo(getClass(), "Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); - AdminLogger.log(getClass(), "Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); + try { + + role = roleAPI.save(role); + } catch(RoleNameException e) { - try { + ActivityLogger.logInfo(getClass(), "Error Adding Role. Invalid Name", "Date: " + date + "; "+ "User:" + user.getUserId()); + AdminLogger.log(getClass(), "Error Adding Role. Invalid Name", "Date: " + date + "; "+ "User:" + user.getUserId()); + throw new DotDataException( + Try.of(()->LanguageUtil.get(user,"Role-Save-Name-Failed")).getOrElse("Role Name not valid"), + "Role-Save-Name-Failed", e); - role = roleAPI.save(role); - } catch(RoleNameException e) { + } catch(DotDataException | DotStateException e) { + ActivityLogger.logInfo(getClass(), "Error Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); + AdminLogger.log(getClass(), "Error Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); + throw e; + } - ActivityLogger.logInfo(getClass(), "Error Adding Role. Invalid Name", "Date: " + date + "; "+ "User:" + user.getUserId()); - AdminLogger.log(getClass(), "Error Adding Role. Invalid Name", "Date: " + date + "; "+ "User:" + user.getUserId()); - throw new DotDataException( - Try.of(()->LanguageUtil.get(initDataObject.getUser(),"Role-Save-Name-Failed")).getOrElse("Role Name not valid"), - "Role-Save-Name-Failed", e); + ActivityLogger.logInfo(getClass(), "Role Created", "Date: " + date + "; "+ "User:" + user.getUserId() + "; RoleID: " + role.getId() ); + AdminLogger.log(getClass(), "Role Created", "Date: " + date + "; "+ "User:" + user.getUserId() + "; RoleID: " + role.getId() ); - } catch(DotDataException | DotStateException e) { - ActivityLogger.logInfo(getClass(), "Error Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); - AdminLogger.log(getClass(), "Error Adding Role", "Date: " + date + "; "+ "User:" + user.getUserId()); - throw e; - } + return Response.ok(new RoleResponseEntityView(role.toMap())).build(); + } + + /** + * Shared authorization gate for the role-mutation endpoints (#36936–#36939): requires an + * authenticated backend user with access to the Roles portlet AND the CMS Administrator + * role. Rejections are security-logged. + * + * @return the authenticated, authorized user + */ + private User initRequireRolesPortletAndCmsAdmin(final HttpServletRequest request, + final HttpServletResponse response) throws DotDataException, DotSecurityException { + + final InitDataObject initDataObject = new WebResource.InitBuilder(this.webResource) + .requiredFrontendUser(false).rejectWhenNoUser(true) + .requiredBackendUser(true).requiredPortlet("roles") + .requestAndResponse(request, response).init(); - ActivityLogger.logInfo(getClass(), "Role Created", "Date: " + date + "; "+ "User:" + user.getUserId() + "; RoleID: " + role.getId() ); - AdminLogger.log(getClass(), "Role Created", "Date: " + date + "; "+ "User:" + user.getUserId() + "; RoleID: " + role.getId() ); + final User user = initDataObject.getUser(); + if (!this.roleAPI.doesUserHaveRole(user, this.roleAPI.loadCMSAdminRole())) { - return Response.ok(new RoleResponseEntityView(role.toMap())).build(); + SecurityLogger.logInfo(this.getClass(), "unauthorized attempt to modify roles by user " + + user.getUserId() + " from " + request.getRemoteHost()); + throw new DotSecurityException("User: '" + user.getUserId() + "' not authorized"); } - final String remoteIp = request.getRemoteHost(); - SecurityLogger.logInfo(UserAjax.class, "unauthorized attempt to call create a role by user "+ - initDataObject.getUser().getUserId() + " from " + remoteIp); - throw new DotSecurityException("User: '" + initDataObject.getUser().getUserId() + "' not authorized"); + return user; + } + + /** + * Updates an existing role — name, key, description, can-grant flags and parent + * (reparent). A null {@code parentRoleId} turns the role into a root role, mirroring the + * legacy DWR {@code RoleAjax#updateRole} behavior. System and locked roles are rejected. + * The caller must be a backend user with access to the Roles portlet and the CMS + * Administrator role. + */ + @Operation( + operationId = "updateRole", + summary = "Update a role", + description = "Updates an existing role's name, key, description, can-grant flags and parent. " + + "PUT is a full replace: every field of the role is overwritten from the request body, so " + + "clients must send the complete role representation — omitted fields are reset (booleans " + + "default to false, omitted roleKey/description are cleared, omitted parentRoleId reparents " + + "to root). A null parentRoleId turns the role into a root role. Reparenting under the role's " + + "own descendant is rejected. System and locked roles cannot be updated. Note: the role is " + + "updated in place — grants and permissions attached to the role are preserved." + ) + @ApiResponses(value = { + @ApiResponse(responseCode = "200", + description = "Role updated successfully", + content = @Content(mediaType = "application/json", + schema = @Schema(implementation = ResponseEntityRoleDetailView.class))), + @ApiResponse(responseCode = "400", + description = "Bad request - invalid role name, or the reparent would create a hierarchy cycle", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "401", + description = "Unauthorized - authentication required", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "403", + description = "Forbidden - admin permissions required, or the role is a system or locked role", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "404", + description = "Role or parent role not found", + content = @Content(mediaType = "application/json")), + @ApiResponse(responseCode = "409", + description = "Conflict - duplicate role key, or duplicate role name under the same parent", + content = @Content(mediaType = "application/json")) + }) + @PUT + @Path("/{roleId}") + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + public ResponseEntityRoleDetailView updateRole( + final @Context HttpServletRequest request, + final @Context HttpServletResponse response, + @Parameter(description = "Id of the role to update", required = true) + final @PathParam("roleId") String roleId, + @io.swagger.v3.oas.annotations.parameters.RequestBody( + description = "Role information — same shape as POST /v1/roles", + required = true, + content = @Content(schema = @Schema(implementation = RoleForm.class)) + ) + final RoleForm roleForm) throws DotDataException, DotSecurityException { + + final User user = this.initRequireRolesPortletAndCmsAdmin(request, response); + + 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)); } /** diff --git a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml index 0dde0846bdfb..8a3ac2c454e1 100644 --- a/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml +++ b/dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml @@ -15126,6 +15126,64 @@ paths: summary: Load user roles tags: - Roles + /v1/roles/{roleId}: + put: + description: "Updates an existing role's name, key, description, can-grant flags\ + \ and parent. PUT is a full replace: every field of the role is overwritten\ + \ from the request body, so clients must send the complete role representation\ + \ — omitted fields are reset (booleans default to false, omitted roleKey/description\ + \ are cleared, omitted parentRoleId reparents to root). A null parentRoleId\ + \ turns the role into a root role. Reparenting under the role's own descendant\ + \ is rejected. System and locked roles cannot be updated. Note: the role is\ + \ updated in place — grants and permissions attached to the role are preserved." + operationId: updateRole + parameters: + - description: Id of the role to update + in: path + name: roleId + required: true + schema: + type: string + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/RoleForm" + description: Role information — same shape as POST /v1/roles + required: true + responses: + "200": + content: + application/json: + schema: + $ref: "#/components/schemas/ResponseEntityRoleDetailView" + description: Role updated successfully + "400": + content: + application/json: {} + description: "Bad request - invalid role name, or the reparent would create\ + \ a hierarchy cycle" + "401": + content: + application/json: {} + description: Unauthorized - authentication required + "403": + content: + application/json: {} + description: "Forbidden - admin permissions required, or the role is a system\ + \ or locked role" + "404": + content: + application/json: {} + description: Role or parent role not found + "409": + content: + application/json: {} + description: "Conflict - duplicate role key, or duplicate role name under\ + \ the same parent" + summary: Update a role + tags: + - Roles /v1/roles/{roleId}/layouts: get: description: Returns a collection of layouts associated to a role @@ -35424,6 +35482,10 @@ components: type: string parent: type: string + roleChildren: + type: array + items: + $ref: "#/components/schemas/RoleView" roleKey: type: string system: diff --git a/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java b/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java index a09f59ad9a90..4ea17f1b6134 100644 --- a/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java +++ b/dotcms-integration/src/test/java/com/dotcms/MainSuite3a.java @@ -102,6 +102,7 @@ ContentToStringUtilTest.class, CacheResourceIntegrationTest.class, InodeExistenceCheckIntegrationTest.class, + com.dotcms.rest.api.v1.system.role.RoleResourceIntegrationTest.class, }) public class MainSuite3a { diff --git a/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceIntegrationTest.java b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceIntegrationTest.java new file mode 100644 index 000000000000..57da1f5767b3 --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rest/api/v1/system/role/RoleResourceIntegrationTest.java @@ -0,0 +1,458 @@ +package com.dotcms.rest.api.v1.system.role; + +import com.dotmarketing.exception.DoesNotExistException; +import com.dotcms.datagen.RoleDataGen; +import com.dotcms.datagen.SiteDataGen; +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.exception.BadRequestException; +import com.dotcms.rest.exception.ConflictException; +import com.dotcms.util.IntegrationTestInitService; +import com.dotmarketing.beans.Host; +import com.dotmarketing.business.APILocator; +import com.dotmarketing.business.Role; +import com.dotmarketing.business.RoleAPI; +import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.util.UtilMethods; +import com.liferay.portal.ejb.UserTestUtil; +import com.liferay.portal.model.User; +import com.liferay.util.Base64; +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.Map; +import java.util.UUID; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Integration tests for the {@link RoleResource} role-mutation endpoints introduced for the + * Angular Roles & Tools portlet migration (epic #36909). + * + * Covered here: + * - PUT /api/v1/roles/{roleId} (update role + reparent) — issue #36936 + * + * These tests invoke the resource directly with mock authenticated requests, following the + * pattern established by {@code PermissionResourceIntegrationTest}. + * + * @author hassandotcms + */ +public class RoleResourceIntegrationTest { + + private static RoleResource resource; + private static RoleAPI roleAPI; + private static Host testHost; + private static User limitedUser; + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + + resource = new RoleResource(); + roleAPI = APILocator.getRoleAPI(); + testHost = new SiteDataGen().nextPersisted(); + + // Backend user WITHOUT roles-portlet access and WITHOUT the CMS admin role, + // for authorization tests + limitedUser = UserTestUtil.getUser("limiteduser", false, true); + final Role backendRole = roleAPI.loadBackEndUserRole(); + if (!roleAPI.doesUserHaveRole(limitedUser, backendRole)) { + roleAPI.addRoleToUser(backendRole, limitedUser); + } + } + + // ==================== Helpers ==================== + + private static HttpServletRequest adminRequest() { + final MockHeaderRequest request = new MockHeaderRequest( + new MockSessionRequest( + new MockAttributeRequest( + new MockHttpRequestIntegrationTest(testHost.getHostname(), "/").request()) + .request()) + .request()); + + request.setHeader("Authorization", + "Basic " + new String(Base64.encode("admin@dotcms.com:admin".getBytes()))); + return request; + } + + private static HttpServletRequest requestFor(final User user) { + final MockHeaderRequest request = new MockHeaderRequest( + new MockSessionRequest( + new MockAttributeRequest( + new MockHttpRequestIntegrationTest(testHost.getHostname(), "/").request()) + .request()) + .request()); + + request.getSession().setAttribute(com.liferay.portal.util.WebKeys.USER_ID, user.getUserId()); + request.getSession().setAttribute(com.liferay.portal.util.WebKeys.USER, user); + return request; + } + + private static RoleForm.Builder formFrom(final Role role) { + return new RoleForm.Builder() + .roleName(role.getName()) + .roleKey(role.getRoleKey()) + .description(role.getDescription()) + .canEditUsers(role.isEditUsers()) + .canEditPermissions(role.isEditPermissions()) + .canEditLayouts(role.isEditLayouts()); + } + + private static String uniq() { + return Long.toString(System.nanoTime()); + } + + // ==================== PUT /v1/roles/{roleId} — #36936 ==================== + + /** + * Method to test: {@link RoleResource#updateRole(HttpServletRequest, HttpServletResponse, String, RoleForm)} + * Given Scenario: An admin updates every editable field of an existing role. + * Expected Result: 200; the response carries the updated role map and the changes are persisted. + */ + @Test + public void testUpdateRole_success_updatesAllFields() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + + final String newName = "updated-name-" + uniq(); + final String newKey = "updated-key-" + uniq(); + final String newDescription = "updated description"; + + final RoleForm form = new RoleForm.Builder() + .roleName(newName) + .roleKey(newKey) + .description(newDescription) + .canEditUsers(false) + .canEditPermissions(false) + .canEditLayouts(false) + .build(); + + final ResponseEntityRoleDetailView view = resource.updateRole( + adminRequest(), new MockHttpResponse().response(), role.getId(), form); + + final RoleView entity = view.getEntity(); + assertNotNull(entity); + assertEquals(newName, entity.getName()); + assertEquals(newKey, entity.getRoleKey()); + assertFalse(entity.isEditUsers()); + + final Role reloaded = roleAPI.loadRoleById(role.getId()); + assertEquals(newName, reloaded.getName()); + assertEquals(newKey, reloaded.getRoleKey()); + assertEquals(newDescription, reloaded.getDescription()); + assertFalse(reloaded.isEditUsers()); + assertFalse(reloaded.isEditPermissions()); + assertFalse(reloaded.isEditLayouts()); + } + + /** + * Given Scenario: A child role is reparented under a different role. + * Expected Result: 200; the persisted role's parent is the new parent's id. + */ + @Test + public void testUpdateRole_reparent_toAnotherRole() throws Exception { + final Role oldParent = new RoleDataGen().nextPersisted(); + final Role newParent = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(oldParent.getId()).nextPersisted(); + + final RoleForm form = formFrom(child).parentRoleId(newParent.getId()).build(); + + final ResponseEntityRoleDetailView view = resource.updateRole( + adminRequest(), new MockHttpResponse().response(), child.getId(), form); + + assertEquals(newParent.getId(), view.getEntity().getParent()); + assertEquals(newParent.getId(), roleAPI.loadRoleById(child.getId()).getParent()); + } + + /** + * Given Scenario: A child role is updated with a null parentRoleId. + * Expected Result: 200; the role becomes a root role (parent == own id), matching legacy + * DWR behavior (RoleAjax#updateRole). + */ + @Test + public void testUpdateRole_reparent_toRoot_whenParentNull() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(parent.getId()).nextPersisted(); + + final RoleForm form = formFrom(child).parentRoleId(null).build(); + + final ResponseEntityRoleDetailView view = resource.updateRole( + adminRequest(), new MockHttpResponse().response(), child.getId(), form); + + assertEquals(child.getId(), view.getEntity().getParent()); + assertEquals(child.getId(), roleAPI.loadRoleById(child.getId()).getParent()); + } + + /** + * Given Scenario: A parent role is reparented under its own descendant (cycle). + * Expected Result: 400 BadRequestException and the hierarchy is unchanged. This guard is + * net-new vs legacy (the Dojo tree simply never offered the bad drop target). + */ + @Test + public void testUpdateRole_reparent_cycle_badRequest() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role child = new RoleDataGen().parent(parent.getId()).nextPersisted(); + final String originalParentOfParent = parent.getParent(); + + final RoleForm form = formFrom(parent).parentRoleId(child.getId()).build(); + + try { + resource.updateRole(adminRequest(), new MockHttpResponse().response(), parent.getId(), form); + fail("Should have thrown BadRequestException for a hierarchy cycle"); + } catch (final BadRequestException e) { + // expected + } + + assertEquals(originalParentOfParent, roleAPI.loadRoleById(parent.getId()).getParent()); + assertEquals(parent.getId(), roleAPI.loadRoleById(child.getId()).getParent()); + } + + /** + * Given Scenario: A role is reparented under itself. + * Expected Result: 400 BadRequestException. + */ + @Test(expected = BadRequestException.class) + public void testUpdateRole_reparent_toSelf_badRequest() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + + final RoleForm form = formFrom(role).parentRoleId(role.getId()).build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), role.getId(), form); + } + + /** + * Given Scenario: An admin attempts to update a system role. + * Expected Result: DotSecurityException (403). Legacy RoleAPIImpl.save blocks locked/system + * roles; the endpoint surfaces it as a clean 403 instead of a 500. + */ + @Test(expected = DotSecurityException.class) + public void testUpdateRole_systemRole_forbidden() throws Exception { + // a user's individual role is flagged system=true on creation (RoleFactoryImpl#addUserRole) + final Role systemRole = roleAPI.getUserRole(limitedUser); + assertTrue(systemRole.isSystem()); + + final RoleForm form = formFrom(systemRole).roleName("renamed-" + uniq()).build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), systemRole.getId(), form); + } + + /** + * Given Scenario: An admin attempts to update a locked role. + * Expected Result: DotSecurityException (403). + */ + @Test(expected = DotSecurityException.class) + public void testUpdateRole_lockedRole_forbidden() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + roleAPI.lock(role); + + try { + final RoleForm form = formFrom(role).roleName("renamed-" + uniq()).build(); + resource.updateRole(adminRequest(), new MockHttpResponse().response(), role.getId(), form); + } finally { + roleAPI.unLock(role); + } + } + + /** + * Given Scenario: A role is updated to use another role's roleKey. + * Expected Result: 409 ConflictException (DuplicateRoleKeyException from RoleAPIImpl.save). + */ + @Test(expected = ConflictException.class) + public void testUpdateRole_duplicateKey_conflict() throws Exception { + final Role roleA = new RoleDataGen().key("key-a-" + uniq()).nextPersisted(); + final Role roleB = new RoleDataGen().key("key-b-" + uniq()).nextPersisted(); + + final RoleForm form = formFrom(roleB).roleKey(roleA.getRoleKey()).build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), roleB.getId(), form); + } + + /** + * Given Scenario: Two sibling roles under the same parent; one is renamed to the other's name. + * Expected Result: 409 ConflictException (DuplicateRoleException from RoleAPIImpl.save). + */ + @Test(expected = ConflictException.class) + public void testUpdateRole_duplicateNameUnderSameParent_conflict() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role roleA = new RoleDataGen().parent(parent.getId()).nextPersisted(); + final Role roleB = new RoleDataGen().parent(parent.getId()).nextPersisted(); + + final RoleForm form = formFrom(roleB) + .roleName(roleA.getName()) + .parentRoleId(parent.getId()) + .build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), roleB.getId(), form); + } + + /** + * Given Scenario: A role is renamed to an invalid name (over the 100-char limit enforced by + * RoleAPIImpl.save's RoleNameException). + * Expected Result: 400 BadRequestException — a validation error, not a conflict. + */ + @Test(expected = BadRequestException.class) + public void testUpdateRole_invalidName_badRequest() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + + final StringBuilder longName = new StringBuilder(); + for (int i = 0; i <= 100; i++) { + longName.append('a'); + } + final RoleForm form = formFrom(role).roleName(longName.toString()).build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), role.getId(), form); + } + + /** + * Given Scenario: The roleId path parameter does not match any role. + * Expected Result: 404 DoesNotExistException. + */ + @Test(expected = DoesNotExistException.class) + public void testUpdateRole_missingRole_notFound() throws Exception { + final RoleForm form = new RoleForm.Builder().roleName("whatever-" + uniq()).build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), + UUID.randomUUID().toString(), form); + } + + /** + * Given Scenario: The parentRoleId in the form does not match any role. + * Expected Result: 404 DoesNotExistException; the role is not modified. + */ + @Test(expected = DoesNotExistException.class) + public void testUpdateRole_missingParent_notFound() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + + final RoleForm form = formFrom(role).parentRoleId(UUID.randomUUID().toString()).build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), role.getId(), form); + } + + /** + * Given Scenario: A backend user without the roles portlet and without the CMS admin role + * calls the endpoint. + * Expected Result: rejected with a security exception (403); the role is not modified. + */ + @Test + public void testUpdateRole_nonAdmin_forbidden() throws Exception { + final Role role = new RoleDataGen().nextPersisted(); + final String originalName = role.getName(); + + final RoleForm form = formFrom(role).roleName("hacked-" + uniq()).build(); + + try { + resource.updateRole(requestFor(limitedUser), + new MockHttpResponse().response(), role.getId(), form); + fail("Should have thrown a security exception"); + } catch (final DotSecurityException | com.dotcms.rest.exception.SecurityException e) { + // expected: the InitBuilder portlet gate throws the REST SecurityException (→ 403), + // the CMS-admin check throws DotSecurityException (→ 403) + } + + assertEquals(originalName, roleAPI.loadRoleById(role.getId()).getName()); + } + + /** + * Given Scenario: A backend user WITH access to the roles portlet but WITHOUT the CMS admin + * role calls the endpoint. This exercises the CMS-admin gate specifically, as opposed to the + * portlet gate covered by {@link #testUpdateRole_nonAdmin_forbidden()}. + * Expected Result: rejected with a security exception (403); the role is not modified. + */ + @Test + public void testUpdateRole_rolesPortletUserWithoutAdmin_forbidden() throws Exception { + final com.dotmarketing.business.Layout rolesLayout = + new com.dotcms.datagen.LayoutDataGen().portletIds("roles").nextPersisted(); + final Role portletRole = new RoleDataGen().layout(rolesLayout).nextPersisted(); + final User portletUser = UserTestUtil.getUser("rolesportletuser" + uniq(), false, true); + roleAPI.addRoleToUser(roleAPI.loadBackEndUserRole(), portletUser); + roleAPI.addRoleToUser(portletRole, portletUser); + + final Role role = new RoleDataGen().nextPersisted(); + final String originalName = role.getName(); + final RoleForm form = formFrom(role).roleName("hacked-" + uniq()).build(); + + try { + resource.updateRole(requestFor(portletUser), + new MockHttpResponse().response(), role.getId(), form); + fail("Should have thrown a security exception for a non-admin caller"); + } catch (final DotSecurityException | com.dotcms.rest.exception.SecurityException e) { + // expected: the CMS-admin check + } + + assertEquals(originalName, roleAPI.loadRoleById(role.getId()).getName()); + } + + /** + * Given Scenario: PUT is a full replace — a minimal form (only the required roleName) is + * sent for a role that has key, description, can-grant flags, and a parent. + * Expected Result: every omitted field is overwritten: flags reset to false, roleKey and + * description become null, and the role is reparented to root. This pins the documented + * full-replace contract (clients must send the complete role representation) so any future + * drift to merge/PATCH semantics is a deliberate, test-breaking change. + */ + @Test + public void testUpdateRole_fullReplace_omittedFieldsAreReset() throws Exception { + final Role parent = new RoleDataGen().nextPersisted(); + final Role role = new RoleDataGen() + .parent(parent.getId()) + .key("full-replace-key-" + uniq()) + .description("full replace description") + .editUsers(true) + .editPermissions(true) + .editLayouts(true) + .nextPersisted(); + + final RoleForm minimalForm = new RoleForm.Builder() + .roleName(role.getName()) + .build(); + + resource.updateRole(adminRequest(), new MockHttpResponse().response(), role.getId(), minimalForm); + + final Role reloaded = roleAPI.loadRoleById(role.getId()); + assertFalse(reloaded.isEditUsers()); + assertFalse(reloaded.isEditPermissions()); + assertFalse(reloaded.isEditLayouts()); + // the persistence layer normalizes omitted (null) values to empty strings + assertFalse(UtilMethods.isSet(reloaded.getRoleKey())); + assertFalse(UtilMethods.isSet(reloaded.getDescription())); + assertEquals(role.getId(), reloaded.getParent()); + } + + /** + * Given Scenario: Regression guard — PR #36936 extracts the shared auth gate out of + * {@link RoleResource#addNewRole}. Creating a role through POST must keep working unchanged. + * Expected Result: 200; the role is persisted with the submitted fields. + */ + @Test + public void testAddNewRole_regression_createStillWorks() throws Exception { + final String name = "created-role-" + uniq(); + + final RoleForm form = new RoleForm.Builder() + .roleName(name) + .roleKey("created-key-" + uniq()) + .description("created by regression test") + .canEditUsers(true) + .canEditPermissions(true) + .canEditLayouts(true) + .build(); + + final Response restResponse = resource.addNewRole( + adminRequest(), new MockHttpResponse().response(), form); + + assertEquals(200, restResponse.getStatus()); + final Map entity = + ((RoleResponseEntityView) restResponse.getEntity()).getEntity(); + assertNotNull(entity.get("id")); + assertEquals(name, roleAPI.loadRoleById((String) entity.get("id")).getName()); + } +}