diff --git a/doc/release-notes/8808-10575-update-global-role.md b/doc/release-notes/8808-10575-update-global-role.md new file mode 100644 index 00000000000..38142f972e8 --- /dev/null +++ b/doc/release-notes/8808-10575-update-global-role.md @@ -0,0 +1,11 @@ +## Release Highlights + +### Update a Global Role + +A new API endpoint has been added that allows a global role to be updated. See [Native API Guide > Update Global Role](https://guides.dataverse.org/en/6.3/api/native-api.html#update-global-role) (#10612) + +## Bug fixes + +### Edition of custom role fixed + +It is now possible to edit a custom role with the same alias (reported in #8808) \ No newline at end of file diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index f8b8620f121..7d6327a222a 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -4461,12 +4461,12 @@ The JSON representation of a role (``roles.json``) looks like this:: { "alias": "sys1", - "name": “Restricted System Role”, - "description": “A person who may only add datasets.”, + "name": "Restricted System Role", + "description": "A person who may only add datasets.", "permissions": [ "AddDataset" ] - } + } .. note:: alias is constrained to a length of 16 characters @@ -5593,22 +5593,43 @@ Creates a global role in the Dataverse installation. The data POSTed are assumed .. code-block:: bash export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx - export SERVER_URL=https://demo.dataverse.org - export ID=root + export SERVER_URL=http://localhost:8080 + + curl -H "Content-Type: application/json" -H "X-Dataverse-key:$API_TOKEN" -X POST "$SERVER_URL/api/admin/roles" --upload-file roles.json + +``roles.json`` see :ref:`json-representation-of-a-role` + +Update Global Role +~~~~~~~~~~~~~~~~~~ + +Update a global role in the Dataverse installation. The PUTed data is assumed to be a complete JSON role as it will overwrite the existing role. :: + + PUT http://$SERVER/api/admin/roles/$ID - curl -H "X-Dataverse-key:$API_TOKEN" -X POST "$SERVER_URL/api/admin/roles" --upload-file roles.json +A curl example using an ``ID`` + +.. code-block:: bash + + export SERVER_URL=http://localhost:8080 + export ID=24 + + curl -H "Content-Type: application/json" -X PUT "$SERVER_URL/api/admin/roles/$ID" --upload-file roles.json ``roles.json`` see :ref:`json-representation-of-a-role` Delete Global Role ~~~~~~~~~~~~~~~~~~ +Deletes an ``DataverseRole`` whose ``id`` is passed. :: + + DELETE http://$SERVER/api/admin/roles/$ID + A curl example using an ``ID`` .. code-block:: bash export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx - export SERVER_URL=https://demo.dataverse.org + export SERVER_URL=http://localhost:8080 export ID=24 curl -H "X-Dataverse-key:$API_TOKEN" -X DELETE "$SERVER_URL/api/admin/roles/$ID" diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseRoleServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataverseRoleServiceBean.java index 78d5eaf3414..b751841da74 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseRoleServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseRoleServiceBean.java @@ -23,7 +23,6 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.PersistenceContext; import jakarta.persistence.TypedQuery; -//import jakarta.validation.constraints.NotNull; /** * @@ -40,6 +39,9 @@ public class DataverseRoleServiceBean implements java.io.Serializable { @EJB RoleAssigneeServiceBean roleAssigneeService; + + @EJB + DataverseServiceBean dataverseService; @EJB IndexServiceBean indexService; @EJB @@ -48,22 +50,23 @@ public class DataverseRoleServiceBean implements java.io.Serializable { IndexAsync indexAsync; public DataverseRole save(DataverseRole aRole) { - if (aRole.getId() == null) { + if (aRole.getId() == null) { // persist a new Role em.persist(aRole); - /** - * @todo Why would getId be null? Should we call - * indexDefinitionPoint here too? A: it's null for new roles. - */ - return aRole; - } else { - DataverseRole merged = em.merge(aRole); - /** - * @todo update permissionModificationTime here. - */ - IndexResponse indexDefinitionPountResult = indexDefinitionPoint(merged.getOwner()); - logger.info("aRole getId was not null. Indexing result: " + indexDefinitionPountResult); - return merged; + } else { // update an existing Role + aRole = em.merge(aRole); + } + + DvObject owner = aRole.getOwner(); + if(owner == null) { // Builtin Role + owner = dataverseService.findByAlias("root"); + } + + if(owner != null) { // owner may be null if a role is created before the root collection as in setup-all.sh + IndexResponse indexDefinitionPointResult = indexDefinitionPoint(owner); + logger.info("Indexing result: " + indexDefinitionPointResult); } + + return aRole; } public RoleAssignment save(RoleAssignment assignment) { diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java index 54e5eaf7b84..7b7e8c855c5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -1010,6 +1010,22 @@ public Response createNewBuiltinRole(RoleDTO roleDto) { actionLogSvc.log(alr); } } + @Path("roles/{id}") + @PUT + public Response updateBuiltinRole(RoleDTO roleDto, @PathParam("id") long roleId) { + ActionLogRecord alr = new ActionLogRecord(ActionLogRecord.ActionType.Admin, "updateBuiltInRole") + .setInfo(roleDto.getAlias() + ":" + roleDto.getDescription()); + try { + DataverseRole role = roleDto.updateRoleFromDTO(rolesSvc.find(roleId)); + return ok(json(rolesSvc.save(role))); + } catch (Exception e) { + alr.setActionResult(ActionLogRecord.Result.InternalError); + alr.setInfo(alr.getInfo() + "// " + e.getMessage()); + return error(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage()); + } finally { + actionLogSvc.log(alr); + } + } @Path("roles") @GET diff --git a/src/main/java/edu/harvard/iq/dataverse/api/dto/RoleDTO.java b/src/main/java/edu/harvard/iq/dataverse/api/dto/RoleDTO.java index 58e30ade584..5769ab430ad 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/dto/RoleDTO.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/dto/RoleDTO.java @@ -47,11 +47,11 @@ public void setPermissions(String[] permissions) { this.permissions = permissions; } - public DataverseRole asRole() { - DataverseRole r = new DataverseRole(); + public DataverseRole updateRoleFromDTO(DataverseRole r) { r.setAlias(alias); r.setDescription(description); r.setName(name); + r.clearPermissions(); if (permissions != null) { if (permissions.length > 0) { if (permissions[0].trim().toLowerCase().equals("all")) { @@ -65,5 +65,9 @@ public DataverseRole asRole() { } return r; } + + public DataverseRole asRole() { + return updateRoleFromDTO(new DataverseRole()); + } } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateRoleCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateRoleCommand.java index 8cffcd3d821..4a897adefa2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateRoleCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CreateRoleCommand.java @@ -22,12 +22,12 @@ @RequiredPermissions(Permission.ManageDataversePermissions) public class CreateRoleCommand extends AbstractCommand { - private final DataverseRole created; + private final DataverseRole role; private final Dataverse dv; public CreateRoleCommand(DataverseRole aRole, DataverseRequest aRequest, Dataverse anAffectedDataverse) { super(aRequest, anAffectedDataverse); - created = aRole; + role = aRole; dv = anAffectedDataverse; } @@ -41,16 +41,16 @@ public DataverseRole execute(CommandContext ctxt) throws CommandException { //Test to see if the role already exists in DB try { DataverseRole testRole = ctxt.em().createNamedQuery("DataverseRole.findDataverseRoleByAlias", DataverseRole.class) - .setParameter("alias", created.getAlias()) + .setParameter("alias", role.getAlias()) .getSingleResult(); - if (!(testRole == null)) { + if (testRole != null && !testRole.getId().equals(role.getId())) { throw new IllegalCommandException(BundleUtil.getStringFromBundle("permission.role.not.created.alias.already.exists"), this); } } catch (NoResultException nre) { - // we want no results because that meand we can create a role + // we want no results because that meant we can create a role } - dv.addRole(created); - return ctxt.roles().save(created); + dv.addRole(role); + return ctxt.roles().save(role); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java index e4d885276d0..ef6ca4a57cb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/search/SolrIndexServiceBean.java @@ -375,6 +375,12 @@ public IndexResponse indexPermissionsOnSelfAndChildren(long definitionPointId) { * inheritance */ public IndexResponse indexPermissionsOnSelfAndChildren(DvObject definitionPoint) { + + if (definitionPoint == null) { + logger.log(Level.WARNING, "Cannot perform indexPermissionsOnSelfAndChildren with a definitionPoint null"); + return null; + } + List filesToReindexAsBatch = new ArrayList<>(); /** * @todo Re-indexing the definition point itself seems to be necessary