Skip to content

#36936: feat(roles): add PUT /v1/roles/{roleId} for role update #37012

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
hassandotcms wants to merge 3 commits into
base: main
Choose a base branch
from
+773 −40
Open

#36936: feat(roles): add PUT /v1/roles/{roleId} for role update #37012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5465c0
#36936: feat(roles): add PUT /v1/roles/{roleId} for role update and r…
hassandotcms Aug 11, 2026
d2b55cf
Merge branch 'main' into 36936-roles-api-put-role-update
hassandotcms Aug 11, 2026
3a604e0
#36936: docs(roles): state PUT full-replace semantics and pin them wi…
hassandotcms Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 127 additions & 0 deletions dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,152 @@
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;

/**
* 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):
* <ul>
* <li>missing role or parent → {@link DoesNotExistException} (404)</li>
* <li>system or locked role → {@link DotSecurityException} (403) — same condition
* {@code RoleAPIImpl.save} enforces, surfaced cleanly</li>
* <li>reparent to self or to a descendant (cycle) → {@link BadRequestException} (400);
* net-new guard vs legacy, prevents hierarchy corruption</li>
* <li>invalid name → {@link BadRequestException} (400); duplicate key/name →
* {@link ConflictException} (409)</li>
* </ul>
*
* @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
Expand Down
165 changes: 125 additions & 40 deletions dotCMS/src/main/java/com/dotcms/rest/api/v1/system/role/RoleResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<RoleView> childrenRoles = new ArrayList<>();
final List<String> 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));
}

/**
Expand Down
Loading
Loading