From 1f4842644330c9aeedf426883257929027b28ec7 Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Thu, 4 Feb 2021 23:53:48 -0500 Subject: [PATCH 01/44] docs for disable/delete/anon, need to move to user admin section --- doc/sphinx-guides/source/api/native-api.rst | 67 ++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 44d099a1685..36b3193cdb4 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2961,6 +2961,8 @@ Example: ``curl -H "X-Dataverse-key: $API_TOKEN" -X POST http://demo.dataverse.o This action moves account data from jsmith2 into the account jsmith and deletes the account of jsmith2. +Note: User accounts can only be merged if they are either both enabled or both disabled. See :ref:`disable a user`. + .. _change-identifier-label: Change User Identifier @@ -2992,9 +2994,60 @@ Deletes an ``AuthenticatedUser`` whose ``id`` is passed. :: DELETE http://$SERVER/api/admin/authenticatedUsers/id/$id -Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. +Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. If a user cannot be deleted due to performing certain actions, you can choose to :ref:`disable a user`. +.. _disable-a-user: + +Disable a User +~~~~~~~~~~~~~~ + +Disables an ``AuthenticatedUser`` whose ``identifier`` (without the ``@`` sign) is passed. :: + + PUT http://$SERVER/api/admin/authenticatedUsers/disable/$identifier + +Disables an ``AuthenticatedUser`` whose ``id`` is passed. :: + + PUT http://$SERVER/api/admin/authenticatedUsers/disable/id/$id +Note: Since the primary purpose of Dataverse is to serve as an archive and to track data accesses and the modifications of data and metadata, a simple mechanism to delete users that have performed certain actions in the system is not provided by design. + +Disabling a user with this endpoint will: + +- Disable the user's ability to log in to the Dataverse installation +- Remove the user's access from all datasets and files +- Cancel any pending file access requests generated by the user +- Remove the user from all groups +- No longer have notifications generated or sent by the Dataverse installation + +Disabling a user with this endpoint will keep: + +- The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. +- Any Dataverse Collections where the user is the sole administrator +- The user's access to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. +- The user's account information (specifically name, email, affiliation, and position) + +If it is determined that disabling a user is not sufficient, it is possible anonymize a user through use of the :ref:`merge-accounts-label` feature. See :ref:`Anonymize a User ` for more information about this process. + +.. _anonymize-a-user: + +Anonymize a User +~~~~~~~~~~~~~~~~ + +In some cases, due to privacy laws or other organizational policies, it may be required to anonymize a user. This process should only be used in cases where a Dataverse installation has the need to completely remove the traces of a user from the Dataverse installation. This process is only provided because it is not feasible for the Dataverse Software to provide deletion and deactivation options that will satisfy all use cases and all privacy laws, so the most complete deletion option is provided here. + +Note: Before going through these steps, please note that you will be permanently losing: + +- The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. This will all be marked as "Anonymous User" in your Dataverse installation, and it will not be possible to track who made changes as part of the archival record. +- Records of the user's access to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. This could potentially introduce issues where it is no longer possible for data curators and owners to follow up with those who have accessed data on the application, which may be a legal requirement or data sharing agreement. It will be up to the Dataverse installation administrators to determine which datasets the user has accessed to determine if anonymization is feasible in these situations. +- The user's account information (specifically name, email, affiliation, and position). This will make it impossible to follow up with the user being deleted. You may find that you need to contact a user about a security breach or something else in the future, and you will not able to do so. + +To anonymize a user in your Dataverse installation: + +- Create a user called "Anonymous User" +- Use the :ref:`merge-accounts-label` functionality to merge the user you wish to anonymize INTO the Anonymous User +- Remove All Roles from the Anonymous User, either through the Dashboard or the :ref:`Remove Role Assignments ` API. + +Once you've created the Anonymous User once, you will not need to create it again. List Role Assignments of a Role Assignee ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -3005,6 +3058,18 @@ List all role assignments of a role assignee (i.e. a user or a group):: Note that ``identifier`` can contain slashes (e.g. ``&ip/localhost-users``). +.. _remove-role-assignments: + +Remove Role Assignments of Role Assignee +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Delete all role assignments of a role assignee (i.e. a user or a group):: + + DELETE http://$SERVER/api/admin/assignments/assignees/$identifier + +Note that ``identifier`` can contain slashes (e.g. ``&ip/-users``). + + List Permissions a User Has on a Dataverse Collection or Dataset ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 2af33fd9662328d86518c6f8c985c187398d1a0b Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Fri, 5 Feb 2021 15:04:20 -0500 Subject: [PATCH 02/44] draft code review feedback --- doc/sphinx-guides/source/api/native-api.rst | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 36b3193cdb4..1d00ea80206 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2994,7 +2994,7 @@ Deletes an ``AuthenticatedUser`` whose ``id`` is passed. :: DELETE http://$SERVER/api/admin/authenticatedUsers/id/$id -Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. If a user cannot be deleted due to performing certain actions, you can choose to :ref:`disable a user`. +Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. If a user cannot be deleted for this reason, you can choose to :ref:`disable a user`. .. _disable-a-user: @@ -3022,7 +3022,6 @@ Disabling a user with this endpoint will: Disabling a user with this endpoint will keep: - The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. -- Any Dataverse Collections where the user is the sole administrator - The user's access to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. - The user's account information (specifically name, email, affiliation, and position) @@ -3037,15 +3036,18 @@ In some cases, due to privacy laws or other organizational policies, it may be r Note: Before going through these steps, please note that you will be permanently losing: -- The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. This will all be marked as "Anonymous User" in your Dataverse installation, and it will not be possible to track who made changes as part of the archival record. -- Records of the user's access to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. This could potentially introduce issues where it is no longer possible for data curators and owners to follow up with those who have accessed data on the application, which may be a legal requirement or data sharing agreement. It will be up to the Dataverse installation administrators to determine which datasets the user has accessed to determine if anonymization is feasible in these situations. +- The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. This will all be marked as "Anonymous User" in your Dataverse installation, and it will not be possible to track who made changes as part of the archival record. - The user's account information (specifically name, email, affiliation, and position). This will make it impossible to follow up with the user being deleted. You may find that you need to contact a user about a security breach or something else in the future, and you will not able to do so. +You will not lose: + +- Email, First Name, and Last Name in Guestbook records. + To anonymize a user in your Dataverse installation: -- Create a user called "Anonymous User" +- Create a user called "Anonymous User". This is a one-time step. - Use the :ref:`merge-accounts-label` functionality to merge the user you wish to anonymize INTO the Anonymous User -- Remove All Roles from the Anonymous User, either through the Dashboard or the :ref:`Remove Role Assignments ` API. +- Disable the user using the :ref:`disable a user` API. Once you've created the Anonymous User once, you will not need to create it again. From ecaaa9bda382951eb6e4a62a81d0aac575fc6a8c Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Fri, 5 Feb 2021 16:06:15 -0500 Subject: [PATCH 03/44] removing anon and documentation for remove roles API --- doc/sphinx-guides/source/api/native-api.rst | 42 ++------------------- 1 file changed, 3 insertions(+), 39 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 1d00ea80206..43b9e668cb6 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3014,43 +3014,19 @@ Note: Since the primary purpose of Dataverse is to serve as an archive and to tr Disabling a user with this endpoint will: - Disable the user's ability to log in to the Dataverse installation -- Remove the user's access from all datasets and files +- Remove the user's access from all Dataverse collections, datasets and files - Cancel any pending file access requests generated by the user - Remove the user from all groups - No longer have notifications generated or sent by the Dataverse installation Disabling a user with this endpoint will keep: -- The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. -- The user's access to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. +- The user's contributions to datasets, including dataset creation, file uploads, and publishing. +- The user's access history to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. - The user's account information (specifically name, email, affiliation, and position) If it is determined that disabling a user is not sufficient, it is possible anonymize a user through use of the :ref:`merge-accounts-label` feature. See :ref:`Anonymize a User ` for more information about this process. -.. _anonymize-a-user: - -Anonymize a User -~~~~~~~~~~~~~~~~ - -In some cases, due to privacy laws or other organizational policies, it may be required to anonymize a user. This process should only be used in cases where a Dataverse installation has the need to completely remove the traces of a user from the Dataverse installation. This process is only provided because it is not feasible for the Dataverse Software to provide deletion and deactivation options that will satisfy all use cases and all privacy laws, so the most complete deletion option is provided here. - -Note: Before going through these steps, please note that you will be permanently losing: - -- The user's contributions to datasets, including dataset creation, file uploads, publishing, submitting for review, and returning to author. This will all be marked as "Anonymous User" in your Dataverse installation, and it will not be possible to track who made changes as part of the archival record. -- The user's account information (specifically name, email, affiliation, and position). This will make it impossible to follow up with the user being deleted. You may find that you need to contact a user about a security breach or something else in the future, and you will not able to do so. - -You will not lose: - -- Email, First Name, and Last Name in Guestbook records. - -To anonymize a user in your Dataverse installation: - -- Create a user called "Anonymous User". This is a one-time step. -- Use the :ref:`merge-accounts-label` functionality to merge the user you wish to anonymize INTO the Anonymous User -- Disable the user using the :ref:`disable a user` API. - -Once you've created the Anonymous User once, you will not need to create it again. - List Role Assignments of a Role Assignee ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -3060,18 +3036,6 @@ List all role assignments of a role assignee (i.e. a user or a group):: Note that ``identifier`` can contain slashes (e.g. ``&ip/localhost-users``). -.. _remove-role-assignments: - -Remove Role Assignments of Role Assignee -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Delete all role assignments of a role assignee (i.e. a user or a group):: - - DELETE http://$SERVER/api/admin/assignments/assignees/$identifier - -Note that ``identifier`` can contain slashes (e.g. ``&ip/-users``). - - List Permissions a User Has on a Dataverse Collection or Dataset ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 1c5d63277cf58d2c6d012b2bc57881da44889e4a Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 5 Feb 2021 16:57:06 -0500 Subject: [PATCH 04/44] user merge and delete cleanup, get traces #4475 #7575 Primarily this has been an investigation into existing code for deleting and merging users. Additionally, a "get user traces" command was added to get a sense of why a user can't be deleted or what would be merged. The top of DeleteUsersIT has a lengthy comment about which database tables are in play and a variety of scenarios involving users to be deleted, merged or (in the future) disabled (#2419). The following bugs where fixed: - Unable to merge (or delete) a user when they have initiated a password reset. #7575 - Delete OAuth2 tokens on delete. --- .../edu/harvard/iq/dataverse/DataFile.java | 4 + .../iq/dataverse/DataFileServiceBean.java | 10 +- .../edu/harvard/iq/dataverse/Dataset.java | 4 + .../iq/dataverse/DatasetServiceBean.java | 8 + .../edu/harvard/iq/dataverse/Dataverse.java | 2 + .../iq/dataverse/DataverseServiceBean.java | 8 + .../edu/harvard/iq/dataverse/api/Users.java | 30 + .../AuthenticationServiceBean.java | 15 +- .../builtin/BuiltinUserServiceBean.java | 2 + .../command/impl/GetUserTracesCommand.java | 228 ++++++ .../command/impl/MergeInAccountCommand.java | 5 +- .../passwordreset/PasswordResetData.java | 4 +- .../PasswordResetServiceBean.java | 6 + src/main/java/propertyFiles/Bundle.properties | 3 +- .../iq/dataverse/api/DeleteUsersIT.java | 693 ++++++++++++++++++ .../edu/harvard/iq/dataverse/api/UtilIT.java | 14 + 16 files changed, 1027 insertions(+), 9 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetUserTracesCommand.java create mode 100644 src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java diff --git a/src/main/java/edu/harvard/iq/dataverse/DataFile.java b/src/main/java/edu/harvard/iq/dataverse/DataFile.java index cb2b6e221e8..707cbc9daf0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataFile.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataFile.java @@ -57,6 +57,10 @@ @NamedQueries({ @NamedQuery( name="DataFile.removeFromDatasetVersion", query="DELETE FROM FileMetadata f WHERE f.datasetVersion.id=:versionId and f.dataFile.id=:fileId"), + @NamedQuery(name = "DataFile.findByCreatorId", + query = "SELECT o FROM DataFile o WHERE o.creator.id=:creatorId"), + @NamedQuery(name = "DataFile.findByReleaseUserId", + query = "SELECT o FROM DataFile o WHERE o.releaseUser.id=:releaseUserId"), @NamedQuery(name="DataFile.findDataFileByIdProtocolAuth", query="SELECT s FROM DataFile s WHERE s.identifier=:identifier AND s.protocol=:protocol AND s.authority=:authority"), @NamedQuery(name="DataFile.findDataFileThatReplacedId", diff --git a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java index 094e20edf28..bc1dab51d2e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataFileServiceBean.java @@ -155,7 +155,15 @@ public DataFile find(Object pk) { public DataFile findByGlobalId(String globalId) { return (DataFile) dvObjectService.findByGlobalId(globalId, DataFile.DATAFILE_DTYPE_STRING); } - + + public List findByCreatorId(Long creatorId) { + return em.createNamedQuery("DataFile.findByCreatorId").setParameter("creatorId", creatorId).getResultList(); + } + + public List findByReleaseUserId(Long releaseUserId) { + return em.createNamedQuery("DataFile.findByReleaseUserId").setParameter("releaseUserId", releaseUserId).getResultList(); + } + public DataFile findReplacementFile(Long previousFileId){ Query query = em.createQuery("select object(o) from DataFile as o where o.previousDataFileId = :previousFileId"); query.setParameter("previousFileId", previousFileId); diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataset.java b/src/main/java/edu/harvard/iq/dataverse/Dataset.java index 4cf95dda250..cd40e76a304 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataset.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataset.java @@ -53,6 +53,10 @@ query = "SELECT o.id FROM Dataset o WHERE o.owner.id=:ownerId"), @NamedQuery(name = "Dataset.findByOwnerId", query = "SELECT o FROM Dataset o WHERE o.owner.id=:ownerId"), + @NamedQuery(name = "Dataset.findByCreatorId", + query = "SELECT o FROM Dataset o WHERE o.creator.id=:creatorId"), + @NamedQuery(name = "Dataset.findByReleaseUserId", + query = "SELECT o FROM Dataset o WHERE o.releaseUser.id=:releaseUserId"), }) /* diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java index c1efe119fd2..4b36f466c61 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java @@ -150,6 +150,14 @@ private List findIdsByOwnerId(Long ownerId, boolean onlyPublished) { } } + public List findByCreatorId(Long creatorId) { + return em.createNamedQuery("Dataset.findByCreatorId").setParameter("creatorId", creatorId).getResultList(); + } + + public List findByReleaseUserId(Long releaseUserId) { + return em.createNamedQuery("Dataset.findByReleaseUserId").setParameter("releaseUserId", releaseUserId).getResultList(); + } + public List filterByPidQuery(String filterQuery) { // finds only exact matches Dataset ds = findByGlobalId(filterQuery); diff --git a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java index 5aab9ef9a9e..b46333a4287 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Dataverse.java +++ b/src/main/java/edu/harvard/iq/dataverse/Dataverse.java @@ -50,6 +50,8 @@ @NamedQuery(name = "Dataverse.findRoot", query = "SELECT d FROM Dataverse d where d.owner.id=null"), @NamedQuery(name = "Dataverse.findByAlias", query="SELECT dv FROM Dataverse dv WHERE LOWER(dv.alias)=:alias"), @NamedQuery(name = "Dataverse.findByOwnerId", query="select object(o) from Dataverse as o where o.owner.id =:ownerId order by o.name"), + @NamedQuery(name = "Dataverse.findByCreatorId", query="select object(o) from Dataverse as o where o.creator.id =:creatorId order by o.name"), + @NamedQuery(name = "Dataverse.findByReleaseUserId", query="select object(o) from Dataverse as o where o.releaseUser.id =:releaseUserId order by o.name"), @NamedQuery(name = "Dataverse.filterByAlias", query="SELECT dv FROM Dataverse dv WHERE LOWER(dv.alias) LIKE :alias order by dv.alias"), @NamedQuery(name = "Dataverse.filterByAliasNameAffiliation", query="SELECT dv FROM Dataverse dv WHERE (LOWER(dv.alias) LIKE :alias) OR (LOWER(dv.name) LIKE :name) OR (LOWER(dv.affiliation) LIKE :affiliation) order by dv.alias"), @NamedQuery(name = "Dataverse.filterByName", query="SELECT dv FROM Dataverse dv WHERE LOWER(dv.name) LIKE :name order by dv.alias") diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java index 96963bb9cb4..9df4ca52938 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseServiceBean.java @@ -172,6 +172,14 @@ public List findDataverseIdsForIndexing(boolean skipIndexed) { } + public List findByCreatorId(Long creatorId) { + return em.createNamedQuery("Dataverse.findByCreatorId").setParameter("creatorId", creatorId).getResultList(); + } + + public List findByReleaseUserId(Long releaseUserId) { + return em.createNamedQuery("Dataverse.findByReleaseUserId").setParameter("releaseUserId", releaseUserId).getResultList(); + } + public List findByOwnerId(Long ownerId) { return em.createNamedQuery("Dataverse.findByOwnerId").setParameter("ownerId", ownerId).getResultList(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Users.java b/src/main/java/edu/harvard/iq/dataverse/api/Users.java index 37eedbe7714..e8558f88d6e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Users.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Users.java @@ -10,10 +10,13 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.User; import edu.harvard.iq.dataverse.engine.command.impl.ChangeUserIdentifierCommand; +import edu.harvard.iq.dataverse.engine.command.impl.GetUserTracesCommand; import edu.harvard.iq.dataverse.engine.command.impl.MergeInAccountCommand; +import edu.harvard.iq.dataverse.engine.command.impl.RevokeAllRolesCommand; import static edu.harvard.iq.dataverse.util.json.JsonPrinter.json; import java.util.logging.Logger; import javax.ejb.Stateless; +import javax.json.JsonObjectBuilder; import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.POST; @@ -191,4 +194,31 @@ public Response getAuthenticatedUserByToken() { } + @POST + @Path("{identifier}/removeRoles") + public Response removeUserRoles(@PathParam("identifier") String identifier) { + try { + AuthenticatedUser userToModify = authSvc.getAuthenticatedUser(identifier); + if (userToModify == null) { + return error(Response.Status.BAD_REQUEST, "Cannot find user based on " + identifier + "."); + } + execCommand(new RevokeAllRolesCommand(userToModify, createDataverseRequest(findUserOrDie()))); + return ok("Roles removed for user " + identifier + "."); + } catch (Exception ex) { + return error(Response.Status.BAD_REQUEST, "Unable to revoke all roles: " + ex.getLocalizedMessage()); + } + } + + @GET + @Path("{identifier}/traces") + public Response getTraces(@PathParam("identifier") String identifier) { + try { + AuthenticatedUser userToQuery = authSvc.getAuthenticatedUser(identifier); // TODO: do lookup here? What if null? + JsonObjectBuilder jsonObj = execCommand(new GetUserTracesCommand(createDataverseRequest(findUserOrDie()), userToQuery)); + return ok(jsonObj); + } catch (WrappedResponse ex) { + return ex.getResponse(); + } + } + } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java index af644f8a4de..c9a54dba078 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java @@ -30,8 +30,10 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.confirmemail.ConfirmEmailData; import edu.harvard.iq.dataverse.confirmemail.ConfirmEmailServiceBean; +import edu.harvard.iq.dataverse.engine.command.impl.RevokeAllRolesCommand; import edu.harvard.iq.dataverse.passwordreset.PasswordResetData; import edu.harvard.iq.dataverse.passwordreset.PasswordResetServiceBean; +import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.validation.PasswordValidatorServiceBean; import edu.harvard.iq.dataverse.workflows.WorkflowComment; @@ -120,7 +122,10 @@ public class AuthenticationServiceBean { @EJB ExplicitGroupServiceBean explicitGroupService; - + + @EJB + SavedSearchServiceBean savedSearchService; + @PersistenceContext(unitName = "VDCNet-ejbPU") private EntityManager em; @@ -193,7 +198,7 @@ public boolean isOrcidEnabled() { * * Before calling this method, make sure you've deleted all the stuff tied * to the user, including stuff they've created, role assignments, group - * assignments, etc. + * assignments, etc. See the "removeAuthentictedUserItems" (sic) method. * * Longer term, the intention is to have a "disableAuthenticatedUser" * method/command. See https://github.com/IQSS/dataverse/issues/2419 @@ -215,6 +220,7 @@ public void deleteAuthenticatedUser(Object pk) { em.remove(confirmEmailData); } userNotificationService.findByUser(user.getId()).forEach(userNotificationService::delete); + em.createNativeQuery("delete from OAuth2TokenData where user_id =" + user.getId()).executeUpdate(); AuthenticationProvider prv = lookupProvider(user); if ( prv != null && prv.isUserDeletionAllowed() ) { @@ -478,6 +484,10 @@ public String getDeleteUserErrorMessages(AuthenticatedUser au) { if (!datasetVersionService.getDatasetVersionUsersByAuthenticatedUser(au).isEmpty()) { reasons.add(BundleUtil.getStringFromBundle("admin.api.deleteUser.failure.versionUser")); } + + if (!savedSearchService.findByAuthenticatedUser(au).isEmpty()) { + reasons.add(BundleUtil.getStringFromBundle("admin.api.deleteUser.failure.savedSearches")); + } if (!reasons.isEmpty()) { retVal = BundleUtil.getStringFromBundle("admin.api.deleteUser.failure.prefix", Arrays.asList(au.getIdentifier())); @@ -518,7 +528,6 @@ private void deletePendingAccessRequests(AuthenticatedUser au){ } - public AuthenticatedUser save( AuthenticatedUser user ) { em.persist(user); em.flush(); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java index c39c7cb2985..7804716561b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java @@ -79,6 +79,8 @@ public BuiltinUser find(Long pk) { public void removeUser( String userName ) { final BuiltinUser user = findByUserName(userName); if ( user != null ) { + // TODO: Consider adding a cascade delete instead. + passwordResetService.deleteResetDataByDataverseUser(user); em.remove(user); } } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetUserTracesCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetUserTracesCommand.java new file mode 100644 index 00000000000..41a1708e4c5 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GetUserTracesCommand.java @@ -0,0 +1,228 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.DataFile; +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersionUser; +import edu.harvard.iq.dataverse.Dataverse; +import edu.harvard.iq.dataverse.DvObject; +import edu.harvard.iq.dataverse.GuestbookResponse; +import edu.harvard.iq.dataverse.RoleAssignment; +import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroup; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.engine.command.AbstractCommand; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.PermissionException; +import edu.harvard.iq.dataverse.search.savedsearch.SavedSearch; +import edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder; +import java.math.BigDecimal; +import java.util.List; +import java.util.Set; +import javax.json.Json; +import javax.json.JsonArrayBuilder; +import javax.json.JsonObjectBuilder; + +// Superuser-only enforced below. +@RequiredPermissions({}) +public class GetUserTracesCommand extends AbstractCommand { + + private DataverseRequest request; + private AuthenticatedUser user; + + public GetUserTracesCommand(DataverseRequest request, AuthenticatedUser user) { + super(request, (DvObject) null); + this.request = request; + this.user = user; + } + + @Override + public JsonObjectBuilder execute(CommandContext ctxt) throws CommandException { + if (!(getUser() instanceof AuthenticatedUser) || !getUser().isSuperuser()) { + throw new PermissionException("Get user traces command can only be called by superusers.", this, null, null); + } + if (user == null) { + throw new CommandException("Cannot get traces. User not found.", this); + } + Long userId = user.getId(); + JsonObjectBuilder traces = Json.createObjectBuilder(); +// List roleAssignments = ctxt.permissions().getDvObjectsUserHasRoleOn(user); + List roleAssignments = ctxt.roleAssignees().getAssignmentsFor(user.getIdentifier()); + if (roleAssignments != null && !roleAssignments.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (RoleAssignment roleAssignment : roleAssignments) { + jab.add(NullSafeJsonBuilder.jsonObjectBuilder() + .add("id", roleAssignment.getId()) + .add("definitionPointName", roleAssignment.getDefinitionPoint().getCurrentName()) + .add("definitionPointIdentifier", roleAssignment.getDefinitionPoint().getIdentifier()) + .add("definitionPointId", roleAssignment.getDefinitionPoint().getId()) + .add("roleAlias", roleAssignment.getRole().getAlias()) + .add("roleName", roleAssignment.getRole().getName()) + ); + } + job.add("count", roleAssignments.size()); + job.add("items", jab); + traces.add("roleAssignments", job); + } + List dataversesCreated = ctxt.dataverses().findByCreatorId(userId); + if (dataversesCreated != null && !dataversesCreated.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (Dataverse dataverse : dataversesCreated) { + jab.add(Json.createObjectBuilder() + .add("id", dataverse.getId()) + .add("alias", dataverse.getAlias()) + ); + } + job.add("count", dataversesCreated.size()); + job.add("items", jab); + traces.add("dataverseCreator", job); + } + List dataversesPublished = ctxt.dataverses().findByReleaseUserId(userId); + if (dataversesPublished != null && !dataversesPublished.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (Dataverse dataverse : dataversesPublished) { + jab.add(Json.createObjectBuilder() + .add("id", dataverse.getId()) + .add("alias", dataverse.getAlias()) + ); + } + job.add("count", dataversesPublished.size()); + job.add("items", jab); + traces.add("dataversePublisher", job); + } + List datasetsCreated = ctxt.datasets().findByCreatorId(userId); + if (datasetsCreated != null && !datasetsCreated.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (Dataset dataset : datasetsCreated) { + jab.add(Json.createObjectBuilder() + .add("id", dataset.getId()) + .add("pid", dataset.getGlobalId().asString()) + ); + } + job.add("count", datasetsCreated.size()); + job.add("items", jab); + traces.add("datasetCreator", job); + } + List datasetsPublished = ctxt.datasets().findByReleaseUserId(userId); + if (datasetsPublished != null && !datasetsPublished.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (Dataset dataset : datasetsPublished) { + jab.add(Json.createObjectBuilder() + .add("id", dataset.getId()) + .add("pid", dataset.getGlobalId().asString()) + ); + } + job.add("count", datasetsPublished.size()); + job.add("items", jab); + traces.add("datasetPublisher", job); + } + List dataFilesCreated = ctxt.files().findByCreatorId(userId); + if (dataFilesCreated != null && !dataFilesCreated.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (DataFile dataFile : dataFilesCreated) { + jab.add(Json.createObjectBuilder() + .add("id", dataFile.getId()) + .add("filename", dataFile.getCurrentName()) + .add("datasetPid", dataFile.getOwner().getGlobalId().asString()) + ); + } + job.add("count", dataFilesCreated.size()); + job.add("items", jab); + traces.add("dataFileCreator", job); + } + // TODO: Consider removing this because we don't seem to populate releaseuser_id for files. + List dataFilesPublished = ctxt.files().findByReleaseUserId(userId); + if (dataFilesPublished != null && !dataFilesPublished.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (DataFile dataFile : dataFilesPublished) { + jab.add(Json.createObjectBuilder() + .add("id", dataFile.getId()) + .add("filename", dataFile.getCurrentName()) + .add("datasetPid", dataFile.getOwner().getGlobalId().asString()) + ); + } + job.add("count", dataFilesPublished.size()); + job.add("items", jab); + traces.add("dataFileCreator", job); + } + // These are the users who have published a version (or created a draft). + List datasetVersionUsers = ctxt.datasetVersion().getDatasetVersionUsersByAuthenticatedUser(user); + if (datasetVersionUsers != null && !datasetVersionUsers.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (DatasetVersionUser datasetVersionUser : datasetVersionUsers) { + jab.add(Json.createObjectBuilder() + .add("id", datasetVersionUser.getId()) + .add("dataset", datasetVersionUser.getDatasetVersion().getDataset().getGlobalId().asString()) + .add("version", datasetVersionUser.getDatasetVersion().getSemanticVersion()) + ); + } + job.add("count", datasetVersionUsers.size()); + job.add("items", jab); + traces.add("datasetVersionUsers", job); + } + Set explicitGroups = ctxt.explicitGroups().findDirectlyContainingGroups(user); + if (explicitGroups != null && !explicitGroups.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (ExplicitGroup explicitGroup : explicitGroups) { + jab.add(Json.createObjectBuilder() + .add("id", explicitGroup.getId()) + .add("name", explicitGroup.getDisplayName()) + ); + } + job.add("count", explicitGroups.size()); + job.add("items", jab); + traces.add("explicitGroups", job); + } + List guestbookResponses = ctxt.responses().findByAuthenticatedUserId(user); + if (guestbookResponses != null && !guestbookResponses.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + // The feeling is that this is too much detail for now so we only show a count. +// JsonArrayBuilder jab = Json.createArrayBuilder(); +// for (GuestbookResponse guestbookResponse : guestbookResponses) { +// jab.add(Json.createObjectBuilder() +// .add("id", guestbookResponse.getId()) +// .add("downloadType", guestbookResponse.getDownloadtype()) +// .add("filename", guestbookResponse.getDataFile().getCurrentName()) +// .add("date", guestbookResponse.getResponseDate()) +// .add("guestbookName", guestbookResponse.getGuestbook().getName()) +// .add("dataset", guestbookResponse.getDatasetVersion().getDataset().getGlobalId().asString()) +// .add("version", guestbookResponse.getDatasetVersion().getSemanticVersion()) +// ); +// } + job.add("count", guestbookResponses.size()); +// job.add("items", jab); + traces.add("guestbookEntries", job); + } + List savedSearchs = ctxt.savedSearches().findByAuthenticatedUser(user); + if (savedSearchs != null && !savedSearchs.isEmpty()) { + JsonObjectBuilder job = Json.createObjectBuilder(); + JsonArrayBuilder jab = Json.createArrayBuilder(); + for (SavedSearch savedSearch : savedSearchs) { + jab.add(Json.createObjectBuilder() + .add("id", savedSearch.getId()) + ); + } + job.add("count", savedSearchs.size()); + job.add("items", jab); + traces.add("savedSearches", job); + } + JsonObjectBuilder result = Json.createObjectBuilder(); + result.add("user", Json.createObjectBuilder() + .add("identifier", user.getIdentifier()) + .add("name", user.getName()) + ); + result.add("traces", traces); + return result; + } + +} diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java index 28db9b890e9..a13cf27fcd6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java @@ -25,6 +25,7 @@ import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; +import edu.harvard.iq.dataverse.passwordreset.PasswordResetData; import edu.harvard.iq.dataverse.search.IndexResponse; import edu.harvard.iq.dataverse.search.savedsearch.SavedSearch; import edu.harvard.iq.dataverse.workflows.WorkflowComment; @@ -185,8 +186,8 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { ctxt.em().remove(consumedAUL); ctxt.em().remove(consumedAU); BuiltinUser consumedBuiltinUser = ctxt.builtinUsers().findByUserName(consumedAU.getUserIdentifier()); - if (consumedBuiltinUser != null){ - ctxt.em().remove(consumedBuiltinUser); + if (consumedBuiltinUser != null) { + ctxt.builtinUsers().removeUser(consumedBuiltinUser.getUserName()); } diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetData.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetData.java index 78af2a31dc2..a3150161c52 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetData.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetData.java @@ -28,7 +28,9 @@ @NamedQuery(name="PasswordResetData.findByUser", query="SELECT prd FROM PasswordResetData prd WHERE prd.builtinUser = :user"), @NamedQuery(name="PasswordResetData.findByToken", - query="SELECT prd FROM PasswordResetData prd WHERE prd.token = :token") + query="SELECT prd FROM PasswordResetData prd WHERE prd.token = :token"), + @NamedQuery(name="PasswordResetData.deleteByUser", + query="DELETE FROM PasswordResetData prd WHERE prd.builtinUser = :user"), }) @Entity public class PasswordResetData implements Serializable { diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java index 507c31f5595..c3f0f7f2abc 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java @@ -186,6 +186,12 @@ private long deleteAllExpiredTokens() { return numDeleted; } + public void deleteResetDataByDataverseUser(BuiltinUser user) { + TypedQuery typedQuery = em.createNamedQuery("PasswordResetData.deleteByUser", PasswordResetData.class); + typedQuery.setParameter("user", user); + int numRowsAffected = typedQuery.executeUpdate(); + } + public PasswordChangeAttemptResponse attemptPasswordReset(BuiltinUser user, String newPassword, String token) { final String messageSummarySuccess = "Password Reset Successfully"; diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 1fd7c300ab4..6cdf8b93550 100755 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -2280,8 +2280,7 @@ admin.api.deleteUser.failure.dvobjects= the user has created Dataverse object(s) admin.api.deleteUser.failure.gbResps= the user is associated with file download (Guestbook Response) record(s) admin.api.deleteUser.failure.roleAssignments=the user is associated with role assignment record(s) admin.api.deleteUser.failure.versionUser=the user has contributed to dataset version(s) -admin.api.deleteUser.failure.groupMember=the user is a member of Explicit Group(s) -admin.api.deleteUser.failure.pendingRequests=the user has pending File Access Request(s) +admin.api.deleteUser.failure.savedSearches=the user has created saved searches admin.api.deleteUser.success=Authenticated User {0} deleted. #Files.java diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java new file mode 100644 index 00000000000..cbfe761234d --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java @@ -0,0 +1,693 @@ +package edu.harvard.iq.dataverse.api; + +import com.jayway.restassured.RestAssured; +import com.jayway.restassured.path.json.JsonPath; +import com.jayway.restassured.response.Response; +import edu.harvard.iq.dataverse.authorization.DataverseRole; +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import javax.json.Json; +import javax.json.JsonObjectBuilder; +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.CREATED; +import static javax.ws.rs.core.Response.Status.OK; +import static junit.framework.Assert.assertEquals; +import static org.hamcrest.CoreMatchers.equalTo; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * The following query has been helpful in discovering places where user ids + * appear throughout the database. Here's a summary of how user deletion affects + * these tables. + * + * - apitoken: Not a concern. Tokens are deleted. + * + * - authenticateduserlookup: Not a concern. Rows are deleted. + * + * - confirmemaildata: Not a concern. Rows are deleted. + * + * - datasetlock: Not a concern, locks are deleted. + * + * - datasetversionuser: Definitely a concern. This table is what feeds the + * "Contributors" list under the "Version" tab on the dataset page. You can't + * delete the user. You can merge the user but the name under "Contributors" + * will change to the user you merged into. There is talk of implementing the + * concept of disabling users to handle this. + * + * - dvobject (creator_id): Definitely a concern. You can't delete a user. You + * have to merge instead. + * + * - dvobject (releaseuser_id): Definitely a concern. You can't delete a user. + * You have to merge instead. It seems that for files, releaseuser_id is not + * populated. + * + * - explicitgroup: Not a concern. Group membership is deleted. + * + * - fileaccessrequests: Not a concern. File requests are deleted. + * + * - guestbookresponse: Definitely a concern but it's possible to null out the + * user id. You can't delete a user but you can merge instead. There is talk of + * disable which would probably null out the id. In all cases the name and email + * address in the rows are left alone. + * + * - oauth2tokendata: Not a concern. Rows are deleted. + * + * - savedsearch: Definitely a concern. You can't delete a user. You have to + * merge. + * + * - userbannermessage: Not a concern. Rows are deleted. + * + * - usernotification (user_id): Not a concern. Deleted by a cascade. + * + * - usernotification (requestor_id): Not a big concern because of other + * constraints. This is only populated by "submit for review" (so that the + * curator has the name and email address of the author). All these + * notifications would be deleted by a cascade but deleting the user itself is + * prevented because the user recorded in the datasetversionuser table. (Both + * "submit for review" and "return to author" add you to that table.) So the + * bottom line is that the user can't be deleted. It has to be merged. + * + * - workflowcomment: Not a big concern because of other constraints. A workflow + * comment is optionally added as part of "return to author" but this also + * creates a row in the datasetversionuser table which means the user can't be + * deleted. It has to be merged instead. + * + * + * The tables that aren't captured above are actionlogrecord and roleassignment + * because the relationship is to the identifier (username) rather than the id. + * So we'll list them separately: + * + * - actionlogrecord: Not a concern. Delete can go through. On merge, they are + * changed from one user identifier to another. + * + * - roleassignment: Not a concern. Delete can go through. On merge, they are + * changed from one user identifier to another. + */ +/* + table_name | constraint_name +---------------------------------+---------------------------------------------------------------- + apitoken | fk_apitoken_authenticateduser_id + authenticateduserlookup | fk_authenticateduserlookup_authenticateduser_id + confirmemaildata | fk_confirmemaildata_authenticateduser_id + datasetlock | fk_datasetlock_user_id + datasetversionuser | fk_datasetversionuser_authenticateduser_id + dvobject | fk_dvobject_creator_id + dvobject | fk_dvobject_releaseuser_id + explicitgroup_authenticateduser | explicitgroup_authenticateduser_containedauthenticatedusers_id + fileaccessrequests | fk_fileaccessrequests_authenticated_user_id + guestbookresponse | fk_guestbookresponse_authenticateduser_id + oauth2tokendata | fk_oauth2tokendata_user_id + savedsearch | fk_savedsearch_creator_id + userbannermessage | fk_userbannermessage_user_id + usernotification | fk_usernotification_user_id + usernotification | fk_usernotification_requestor_id + workflowcomment | fk_workflowcomment_authenticateduser_id +(16 rows) + +-- https://stackoverflow.com/questions/5347050/postgresql-sql-script-to-get-a-list-of-all-tables-that-has-a-particular-column +select R.TABLE_NAME, R.CONSTRAINT_NAME +from INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE u +inner join INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS FK + on U.CONSTRAINT_CATALOG = FK.UNIQUE_CONSTRAINT_CATALOG + and U.CONSTRAINT_SCHEMA = FK.UNIQUE_CONSTRAINT_SCHEMA + and U.CONSTRAINT_NAME = FK.UNIQUE_CONSTRAINT_NAME +inner join INFORMATION_SCHEMA.KEY_COLUMN_USAGE R + ON R.CONSTRAINT_CATALOG = FK.CONSTRAINT_CATALOG + AND R.CONSTRAINT_SCHEMA = FK.CONSTRAINT_SCHEMA + AND R.CONSTRAINT_NAME = FK.CONSTRAINT_NAME +WHERE U.COLUMN_NAME = 'id' +-- AND U.TABLE_CATALOG = 'b' +-- AND U.TABLE_SCHEMA = 'c' + AND U.TABLE_NAME = 'authenticateduser' +ORDER BY R.TABLE_NAME; + */ +public class DeleteUsersIT { + + @BeforeClass + public static void setUp() { + RestAssured.baseURI = UtilIT.getRestAssuredBaseUri(); + } + + @Test + public void testDeleteRolesAndUnpublishedDataverse() { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String usernameForCreateDV = UtilIT.getUsernameFromResponse(createUser); + String normalApiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response getTraces1 = UtilIT.getUserTraces(usernameForCreateDV, superuserApiToken); + getTraces1.prettyPrint(); + getTraces1.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.user.identifier", equalTo("@" + usernameForCreateDV)) + // traces is {} when user hasn't left a trace + .body("data.traces", equalTo(Collections.emptyMap())); + + Response createDataverse = UtilIT.createRandomDataverse(normalApiToken); + createDataverse.prettyPrint(); + createDataverse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + + Response getTraces2 = UtilIT.getUserTraces(usernameForCreateDV, superuserApiToken); + getTraces2.prettyPrint(); + getTraces2.then().assertThat().statusCode(OK.getStatusCode()); + + if (true) { + return; + } + + createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String usernameForAssignedRole = UtilIT.getUsernameFromResponse(createUser); + String roleApiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response assignRole = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.EDITOR.toString(), + "@" + usernameForAssignedRole, superuserApiToken); + + // Shouldn't be able to delete user with a role + Response deleteUserRole = UtilIT.deleteUser(usernameForAssignedRole); + + deleteUserRole.prettyPrint(); + deleteUserRole.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo("Could not delete Authenticated User @" + usernameForAssignedRole + " because the user is associated with role assignment record(s).")); + + // Now remove that role + Response removeRoles1 = UtilIT.deleteUserRoles(usernameForAssignedRole, superuserApiToken); + removeRoles1.prettyPrint(); + removeRoles1.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.message", equalTo("Roles removed for user " + usernameForAssignedRole + ".")); + + // Now the delete should work + Response deleteUserRole2 = UtilIT.deleteUser(usernameForAssignedRole); + deleteUserRole2.prettyPrint(); + deleteUserRole2.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.message", equalTo("AuthenticatedUser @" + usernameForAssignedRole + " deleted. ")); + + // The owner of the dataverse that was just created is dataverseAdmin because it created the parent dataverse (root). + Response getTraces3 = UtilIT.getUserTraces(usernameForCreateDV, superuserApiToken); + getTraces3.prettyPrint(); + getTraces3.then().assertThat().statusCode(OK.getStatusCode()); + + // Removing roles here but could equally just delete the dataverse. + Response removeRoles2 = UtilIT.deleteUserRoles(usernameForCreateDV, superuserApiToken); + removeRoles2.prettyPrint(); + removeRoles2.then().assertThat() + .statusCode(OK.getStatusCode()); + + // Shouldn't be able to delete a user who has created a DV + Response deleteUserCreateDV = UtilIT.deleteUser(usernameForCreateDV); + deleteUserCreateDV.prettyPrint(); + deleteUserCreateDV.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo("Could not delete Authenticated User @" + usernameForCreateDV + " because the user has created Dataverse object(s).")); + + Response deleteDataverse = UtilIT.deleteDataverse(dataverseAlias, superuserApiToken); + deleteDataverse.prettyPrint(); + deleteDataverse.then().assertThat() + .statusCode(OK.getStatusCode()); + + // Should be able to delete user after dv is deleted + Response deleteUserAfterDeleteDV = UtilIT.deleteUser(usernameForCreateDV); + deleteUserAfterDeleteDV.prettyPrint(); + deleteUserAfterDeleteDV.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response deleteSuperuser = UtilIT.deleteUser(superuserUsername); + deleteSuperuser.prettyPrint(); + assertEquals(200, deleteSuperuser.getStatusCode()); + + } + + @Test + public void testDeleteUserWithUnPublishedDataverse() { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String username = UtilIT.getUsernameFromResponse(createUser); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response createDataverse = UtilIT.createRandomDataverse(apiToken); + createDataverse.prettyPrint(); + createDataverse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + + Response removeRoles1 = UtilIT.deleteUserRoles(username, superuserApiToken); + removeRoles1.prettyPrint(); + removeRoles1.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.message", equalTo("Roles removed for user " + username + ".")); + + Response deleteUser1 = UtilIT.deleteUser(username); + deleteUser1.prettyPrint(); + deleteUser1.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo("Could not delete Authenticated User @" + username + " because the user has created Dataverse object(s).")); + + Response traces = UtilIT.getUserTraces(username, superuserApiToken); + traces.prettyPrint(); + traces.then().assertThat().statusCode(OK.getStatusCode()); + + // You can't delete. You have to merge. + Response mergeAccounts = UtilIT.mergeAccounts(superuserUsername, username, superuserApiToken); + mergeAccounts.prettyPrint(); + mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); + } + + /** + * You can't delete an account with guestbook entries so you have to merge + * it instead. + */ + @Test + public void testDeleteUserWithGuestbookEntries() throws IOException { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String authorUsername = UtilIT.getUsernameFromResponse(createUser); + String authorApiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response downloader = UtilIT.createRandomUser(); + downloader.prettyPrint(); + String downloaderUsername = UtilIT.getUsernameFromResponse(downloader); + String downloaderApiToken = UtilIT.getApiTokenFromResponse(downloader); + + Response createDataverse = UtilIT.createRandomDataverse(authorApiToken); + createDataverse.prettyPrint(); + createDataverse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, authorApiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); + + Path pathtoReadme = Paths.get(java.nio.file.Files.createTempDirectory(null) + File.separator + "README.md"); + java.nio.file.Files.write(pathtoReadme, "In the beginning...".getBytes()); + + Response uploadReadme = UtilIT.uploadFileViaNative(datasetId.toString(), pathtoReadme.toString(), authorApiToken); + uploadReadme.prettyPrint(); + uploadReadme.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.files[0].label", equalTo("README.md")); + + int fileId = JsonPath.from(uploadReadme.body().asString()).getInt("data.files[0].dataFile.id"); + + Response publishDataverse = UtilIT.publishDataverseViaNativeApi(dataverseAlias, authorApiToken); + publishDataverse.then().assertThat().statusCode(OK.getStatusCode()); + Response publishDataset = UtilIT.publishDatasetViaNativeApi(datasetPid, "major", authorApiToken); + publishDataset.then().assertThat().statusCode(OK.getStatusCode()); + // This download creates a guestbook entry. + Response downloadFile = UtilIT.downloadFile(fileId, downloaderApiToken); + downloadFile.then().assertThat().statusCode(OK.getStatusCode()); + + // We can't delete the downloader because a guestbook record (a download) has been created. + Response deleteDownloaderFail = UtilIT.deleteUser(downloaderUsername); + deleteDownloaderFail.prettyPrint(); + deleteDownloaderFail.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()); + + // Let's see why we can't download. + Response getTraces = UtilIT.getUserTraces(downloaderUsername, superuserApiToken); + getTraces.prettyPrint(); + getTraces.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.traces.guestbookEntries.count", equalTo(1)); + + // We can't delete so we do a merge instead. + Response mergeAccounts = UtilIT.mergeAccounts(superuserUsername, downloaderUsername, superuserApiToken); + mergeAccounts.prettyPrint(); + mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); + + } + + @Test + public void testDatasetLocks() throws IOException { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String authorUsername = UtilIT.getUsernameFromResponse(createUser); + String authorApiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response downloader = UtilIT.createRandomUser(); + downloader.prettyPrint(); + String downloaderUsername = UtilIT.getUsernameFromResponse(downloader); + String downloaderApiToken = UtilIT.getApiTokenFromResponse(downloader); + + Response createDataverse = UtilIT.createRandomDataverse(authorApiToken); + createDataverse.prettyPrint(); + createDataverse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, authorApiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); + + Response lockDatasetResponse = UtilIT.lockDataset(datasetId.longValue(), "Ingest", superuserApiToken); + lockDatasetResponse.prettyPrint(); + lockDatasetResponse.then().assertThat() + .body("data.message", equalTo("dataset locked with lock type Ingest")) + .statusCode(200); + + Response checkDatasetLocks = UtilIT.checkDatasetLocks(datasetId.longValue(), "Ingest", superuserApiToken); + checkDatasetLocks.prettyPrint(); + checkDatasetLocks.then().assertThat() + .body("data[0].lockType", equalTo("Ingest")) + .statusCode(200); + Response deleteUserWhoCreatedLock = UtilIT.deleteUser(superuserUsername); + deleteUserWhoCreatedLock.prettyPrint(); + deleteUserWhoCreatedLock.then().assertThat() + .statusCode(OK.getStatusCode()); + } + + @Test + public void testDeleteUserWhoIsMemberOfGroup() throws IOException { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String authorUsername = UtilIT.getUsernameFromResponse(createUser); + String authorApiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response downloader = UtilIT.createRandomUser(); + downloader.prettyPrint(); + String downloaderUsername = UtilIT.getUsernameFromResponse(downloader); + String downloaderApiToken = UtilIT.getApiTokenFromResponse(downloader); + + Response createDataverse = UtilIT.createRandomDataverse(authorApiToken); + createDataverse.prettyPrint(); + createDataverse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + + Response createGroupMember = UtilIT.createRandomUser(); + createGroupMember.prettyPrint(); + String groupMemberUsername = UtilIT.getUsernameFromResponse(createGroupMember); + String groupMemberApiToken = UtilIT.getApiTokenFromResponse(createGroupMember); + + String aliasInOwner = "groupFor" + dataverseAlias; + String displayName = "Group for " + dataverseAlias; + String user2identifier = "@" + groupMemberUsername; + Response createGroup = UtilIT.createGroup(dataverseAlias, aliasInOwner, displayName, superuserApiToken); + createGroup.prettyPrint(); + createGroup.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String groupIdentifier = JsonPath.from(createGroup.asString()).getString("data.identifier"); + + List roleAssigneesToAdd = new ArrayList<>(); + roleAssigneesToAdd.add(user2identifier); + Response addToGroup = UtilIT.addToGroup(dataverseAlias, aliasInOwner, roleAssigneesToAdd, superuserApiToken); + addToGroup.prettyPrint(); + addToGroup.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response getTraces = UtilIT.getUserTraces(groupMemberUsername, superuserApiToken); + getTraces.prettyPrint(); + getTraces.then().assertThat().statusCode(OK.getStatusCode()); + + Response deleteUserInGroup = UtilIT.deleteUser(groupMemberUsername); + deleteUserInGroup.prettyPrint(); + deleteUserInGroup.then().assertThat() + .statusCode(OK.getStatusCode()); + + } + + @Test + public void testDeleteUserWithFileAccessRequests() throws IOException { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String authorUsername = UtilIT.getUsernameFromResponse(createUser); + String authorApiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response fileRequester = UtilIT.createRandomUser(); + fileRequester.prettyPrint(); + String fileRequesterUsername = UtilIT.getUsernameFromResponse(fileRequester); + String fileRequesterApiToken = UtilIT.getApiTokenFromResponse(fileRequester); + + Response createDataverse = UtilIT.createRandomDataverse(authorApiToken); + createDataverse.prettyPrint(); + createDataverse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, authorApiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + String datasetPid = JsonPath.from(createDataset.asString()).getString("data.persistentId"); + + Path pathtoReadme = Paths.get(java.nio.file.Files.createTempDirectory(null) + File.separator + "README.md"); + java.nio.file.Files.write(pathtoReadme, "In the beginning...".getBytes()); + + Response uploadReadme = UtilIT.uploadFileViaNative(datasetId.toString(), pathtoReadme.toString(), authorApiToken); + uploadReadme.prettyPrint(); + uploadReadme.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.files[0].label", equalTo("README.md")); + + Integer fileId = JsonPath.from(uploadReadme.body().asString()).getInt("data.files[0].dataFile.id"); + + Response restrictResponse = UtilIT.restrictFile(fileId.toString(), true, authorApiToken); + restrictResponse.prettyPrint(); + restrictResponse.then().assertThat().statusCode(OK.getStatusCode()); + + //Update Dataset to allow requests + Response allowAccessRequestsResponse = UtilIT.allowAccessRequests(datasetPid, true, authorApiToken); + allowAccessRequestsResponse.prettyPrint(); + allowAccessRequestsResponse.then().assertThat().statusCode(OK.getStatusCode()); + + Response publishDataverse = UtilIT.publishDataverseViaNativeApi(dataverseAlias, authorApiToken); + publishDataverse.then().assertThat().statusCode(OK.getStatusCode()); + Response publishDataset = UtilIT.publishDatasetViaNativeApi(datasetPid, "major", authorApiToken); + publishDataset.then().assertThat().statusCode(OK.getStatusCode()); + + Response requestFileAccessResponse = UtilIT.requestFileAccess(fileId.toString(), fileRequesterApiToken); + requestFileAccessResponse.prettyPrint(); + requestFileAccessResponse.then().assertThat().statusCode(OK.getStatusCode()); + + // Let's see why we can't download. + Response getTraces = UtilIT.getUserTraces(fileRequesterUsername, superuserApiToken); + getTraces.prettyPrint(); + getTraces.then().assertThat() + .statusCode(OK.getStatusCode()); + + // Even if users have outstanding file requests, they can be deleted. + Response deleteDownloaderSuccess = UtilIT.deleteUser(fileRequesterUsername); + deleteDownloaderSuccess.prettyPrint(); + deleteDownloaderSuccess.then().assertThat() + .statusCode(OK.getStatusCode()); + } + + @Test + public void testCuratorSendsCommentsToAuthor() throws InterruptedException { + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createCurator1 = UtilIT.createRandomUser(); + createCurator1.prettyPrint(); + createCurator1.then().assertThat() + .statusCode(OK.getStatusCode()); + String curator1Username = UtilIT.getUsernameFromResponse(createCurator1); + String curator1ApiToken = UtilIT.getApiTokenFromResponse(createCurator1); + + Response createCurator2 = UtilIT.createRandomUser(); + createCurator2.prettyPrint(); + createCurator2.then().assertThat() + .statusCode(OK.getStatusCode()); + String curator2Username = UtilIT.getUsernameFromResponse(createCurator2); + String curator2ApiToken = UtilIT.getApiTokenFromResponse(createCurator2); + + Response createDataverseResponse = UtilIT.createRandomDataverse(curator1ApiToken); + createDataverseResponse.prettyPrint(); + createDataverseResponse.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse); + + Response makeCurator2Admin = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + curator2Username, curator1ApiToken); + makeCurator2Admin.prettyPrint(); + makeCurator2Admin.then().assertThat() + .body("data.assignee", equalTo("@" + curator2Username)) + .body("data._roleAlias", equalTo("admin")) + .statusCode(OK.getStatusCode()); + + Response createAuthor1 = UtilIT.createRandomUser(); + createAuthor1.prettyPrint(); + createAuthor1.then().assertThat() + .statusCode(OK.getStatusCode()); + String author1Username = UtilIT.getUsernameFromResponse(createAuthor1); + String author1ApiToken = UtilIT.getApiTokenFromResponse(createAuthor1); + + Response createAuthor2 = UtilIT.createRandomUser(); + createAuthor2.prettyPrint(); + createAuthor2.then().assertThat() + .statusCode(OK.getStatusCode()); + String author2Username = UtilIT.getUsernameFromResponse(createAuthor2); + String author2ApiToken = UtilIT.getApiTokenFromResponse(createAuthor2); + + Response grantAuthor1AddDataset = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.DS_CONTRIBUTOR.toString(), "@" + author1Username, curator1ApiToken); + grantAuthor1AddDataset.prettyPrint(); + grantAuthor1AddDataset.then().assertThat() + .body("data.assignee", equalTo("@" + author1Username)) + .body("data._roleAlias", equalTo("dsContributor")) + .statusCode(OK.getStatusCode()); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, author1ApiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset); + + // FIXME: have the initial create return the DOI or Handle to obviate the need for this call. + Response getDatasetJsonBeforePublishing = UtilIT.nativeGet(datasetId, author1ApiToken); + getDatasetJsonBeforePublishing.prettyPrint(); + String protocol = JsonPath.from(getDatasetJsonBeforePublishing.getBody().asString()).getString("data.protocol"); + String authority = JsonPath.from(getDatasetJsonBeforePublishing.getBody().asString()).getString("data.authority"); + String identifier = JsonPath.from(getDatasetJsonBeforePublishing.getBody().asString()).getString("data.identifier"); + + String datasetPersistentId = protocol + ":" + authority + "/" + identifier; + System.out.println("datasetPersistentId: " + datasetPersistentId); + +// Response grantAuthor2ContributorOnDataset = UtilIT.grantRoleOnDataset(datasetPersistentId, DataverseRole.DS_CONTRIBUTOR.toString(), "@" + author2Username, curatorApiToken); + // TODO: Tighten this down to something more realistic than ADMIN. + Response grantAuthor2ContributorOnDataset = UtilIT.grantRoleOnDataset(datasetPersistentId, DataverseRole.ADMIN.toString(), "@" + author2Username, curator1ApiToken); + grantAuthor2ContributorOnDataset.prettyPrint(); + grantAuthor2ContributorOnDataset.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.assignee", equalTo("@" + author2Username)) + .body("data._roleAlias", equalTo("admin")); + +// // Whoops, the author tries to publish but isn't allowed. The curator will take a look. +// Response noPermToPublish = UtilIT.publishDatasetViaNativeApi(datasetPersistentId, "major", author1ApiToken); +// noPermToPublish.prettyPrint(); +// noPermToPublish.then().assertThat() +// .body("message", equalTo("User @" + author1Username + " is not permitted to perform requested action.")) +// .statusCode(UNAUTHORIZED.getStatusCode()); + Response submitForReview = UtilIT.submitDatasetForReview(datasetPersistentId, author2ApiToken); + submitForReview.prettyPrint(); + submitForReview.then().assertThat() + .statusCode(OK.getStatusCode()); + + // curator2 returns dataset to author. This makes curator2 a contributor. + String comments = "You forgot to upload any files."; + JsonObjectBuilder jsonObjectBuilder = Json.createObjectBuilder(); + jsonObjectBuilder.add("reasonForReturn", comments); + Response returnToAuthor = UtilIT.returnDatasetToAuthor(datasetPersistentId, jsonObjectBuilder.build(), curator2ApiToken); + returnToAuthor.prettyPrint(); + returnToAuthor.then().assertThat() + .body("data.inReview", equalTo(false)) + .statusCode(OK.getStatusCode()); + + Response getTracesForCurator2 = UtilIT.getUserTraces(curator2Username, superuserApiToken); + getTracesForCurator2.prettyPrint(); + getTracesForCurator2.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response removeRolesFromCurator2 = UtilIT.deleteUserRoles(curator2Username, superuserApiToken); + removeRolesFromCurator2.prettyPrint(); + removeRolesFromCurator2.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.message", equalTo("Roles removed for user " + curator2Username + ".")); + + // Because curator2 returned the dataset to the authors, curator2 is now a contributor + // and cannot be deleted. + Response deleteCurator2Fail = UtilIT.deleteUser(curator2Username); + deleteCurator2Fail.prettyPrint(); + deleteCurator2Fail.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo("Could not delete Authenticated User @" + curator2Username + + " because the user has contributed to dataset version(s).")); + + // What should we do with curator2 instead of deleting? The only option is to merge + // curator2 into some other account. Once implemented, we'll disable curator2's account + // so that curator2 continues to be displayed as a contributor. + // + // TODO: disable curator2 here + // + Response removeRolesFromAuthor2 = UtilIT.deleteUserRoles(author2Username, superuserApiToken); + removeRolesFromAuthor2.prettyPrint(); + removeRolesFromAuthor2.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.message", equalTo("Roles removed for user " + author2Username + ".")); + + // Similarly, we can't delete author2 because author2 submitted + // the dataset for review, which makes one a contributor. + Response deleteAuthor2Fail = UtilIT.deleteUser(author2Username); + deleteAuthor2Fail.prettyPrint(); + deleteAuthor2Fail.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo("Could not delete Authenticated User @" + author2Username + + " because the user has contributed to dataset version(s).")); + + } + +} diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index 27b34aba5e1..dee35556729 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -956,6 +956,20 @@ public static Response deleteUser(String username) { return deleteUserResponse; } + public static Response deleteUserRoles(String username, String apiToken) { + Response deleteUserResponse = given() + .header(API_TOKEN_HTTP_HEADER, apiToken) + .post("/api/users/" + username + "/removeRoles"); + return deleteUserResponse; + } + + public static Response getUserTraces(String username, String apiToken) { + Response response = given() + .header(API_TOKEN_HTTP_HEADER, apiToken) + .get("/api/users/" + username + "/traces"); + return response; + } + public static Response reingestFile(Long fileId, String apiToken) { Response response = given() .header(API_TOKEN_HTTP_HEADER, apiToken) From eafdc73ecde663d04f6790b19bb5be57b8883932 Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Tue, 9 Feb 2021 22:16:44 -0500 Subject: [PATCH 05/44] updates from draft code review --- doc/sphinx-guides/source/api/native-api.rst | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 43b9e668cb6..ee606ea94bd 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3009,7 +3009,7 @@ Disables an ``AuthenticatedUser`` whose ``id`` is passed. :: PUT http://$SERVER/api/admin/authenticatedUsers/disable/id/$id -Note: Since the primary purpose of Dataverse is to serve as an archive and to track data accesses and the modifications of data and metadata, a simple mechanism to delete users that have performed certain actions in the system is not provided by design. +Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Disable User endpoint instead of a Delete User endpoint is by design. Disabling a user with this endpoint will: @@ -3022,11 +3022,9 @@ Disabling a user with this endpoint will: Disabling a user with this endpoint will keep: - The user's contributions to datasets, including dataset creation, file uploads, and publishing. -- The user's access history to datafiles in the Dataverse installation, including guestbook records and acceptance of terms of use. +- The user's access history to datafiles in the Dataverse installation, including guestbook records. - The user's account information (specifically name, email, affiliation, and position) -If it is determined that disabling a user is not sufficient, it is possible anonymize a user through use of the :ref:`merge-accounts-label` feature. See :ref:`Anonymize a User ` for more information about this process. - List Role Assignments of a Role Assignee ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From a3b51d729f12e111b20a0a6928204a6e85ccaab1 Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Tue, 9 Feb 2021 22:30:56 -0500 Subject: [PATCH 06/44] adding links from user administration --- doc/sphinx-guides/source/admin/user-administration.rst | 10 ++++++++++ doc/sphinx-guides/source/api/native-api.rst | 4 +++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/admin/user-administration.rst b/doc/sphinx-guides/source/admin/user-administration.rst index bc9be64775f..2999c6e0856 100644 --- a/doc/sphinx-guides/source/admin/user-administration.rst +++ b/doc/sphinx-guides/source/admin/user-administration.rst @@ -44,6 +44,16 @@ Change User Identifier See :ref:`change-identifier-label` +Delete a User +------------- + +See :ref:`delete-a-user` + +Disable a User +-------------- + +See :ref:`disable-a-user` + Confirm Email ------------- diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index ee606ea94bd..312175d4c74 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -2982,7 +2982,9 @@ Make User a SuperUser Toggles superuser mode on the ``AuthenticatedUser`` whose ``identifier`` (without the ``@`` sign) is passed. :: POST http://$SERVER/api/admin/superuser/$identifier - + +.. _delete-a-user: + Delete a User ~~~~~~~~~~~~~ From c85691bc19fc88b5f1df2d46b63d121bfa1dc16e Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Thu, 11 Feb 2021 09:59:20 -0500 Subject: [PATCH 07/44] updates from more review --- doc/sphinx-guides/source/api/native-api.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 312175d4c74..a05bc8d81c1 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3011,12 +3011,13 @@ Disables an ``AuthenticatedUser`` whose ``id`` is passed. :: PUT http://$SERVER/api/admin/authenticatedUsers/disable/id/$id -Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Disable User endpoint instead of a Delete User endpoint is by design. +Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Disable User endpoint for users who have taken certain actions in the system alongside a Delete User endpoint to remove users that haven't taken certain actions in the system is by design. Disabling a user with this endpoint will: - Disable the user's ability to log in to the Dataverse installation - Remove the user's access from all Dataverse collections, datasets and files +- Prevent a user from being assigned any roles - Cancel any pending file access requests generated by the user - Remove the user from all groups - No longer have notifications generated or sent by the Dataverse installation From bb03310dd388beebbc762f1f780fac3c50cdb935 Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Wed, 17 Feb 2021 12:40:52 -0500 Subject: [PATCH 08/44] Update native-api.rst --- doc/sphinx-guides/source/api/native-api.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index a05bc8d81c1..2146060ef37 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3013,9 +3013,11 @@ Disables an ``AuthenticatedUser`` whose ``id`` is passed. :: Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Disable User endpoint for users who have taken certain actions in the system alongside a Delete User endpoint to remove users that haven't taken certain actions in the system is by design. +This is an irreversible action. There is no option to undisable a user. + Disabling a user with this endpoint will: -- Disable the user's ability to log in to the Dataverse installation +- Disable the user's ability to log in to the Dataverse installation. A message will be shown, stating that the account has been disabled. - Remove the user's access from all Dataverse collections, datasets and files - Prevent a user from being assigned any roles - Cancel any pending file access requests generated by the user From e9845c077ef847e7bb57feb95c67ef7859519389 Mon Sep 17 00:00:00 2001 From: Danny Brooke Date: Thu, 18 Feb 2021 12:31:41 -0500 Subject: [PATCH 09/44] Update native-api.rst --- doc/sphinx-guides/source/api/native-api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 2146060ef37..6d7b200f11a 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3017,7 +3017,7 @@ This is an irreversible action. There is no option to undisable a user. Disabling a user with this endpoint will: -- Disable the user's ability to log in to the Dataverse installation. A message will be shown, stating that the account has been disabled. +- Disable the user's ability to log in to the Dataverse installation. A message will be shown, stating that the account has been disabled. The user will not able to create a new account with the same email address, ORCID, Shibboleth, or other login type. - Remove the user's access from all Dataverse collections, datasets and files - Prevent a user from being assigned any roles - Cancel any pending file access requests generated by the user From 51022aa8135c7acd9284bb0ecf5f04cbd9c9f415 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 18 Feb 2021 14:17:32 -0500 Subject: [PATCH 10/44] disable users #2419 #4475 This commit adds an API endpoint for disabling users. Once users are disabled: - They lose all their roles. - They lose pending file access requests. - They lose group memberships. - They lose banner messages. - They lose all notifications. - They get an error on login. (Builtin/Shib/OAuth tested.) - They get an error if they try to use the API. - They cannot be assigned roles. - They cannot be added to groups. - They cannot initiate password reset. - They cannot confirm their email (from a previous link). --- .../source/developers/remote-users.rst | 2 +- .../iq/dataverse/DataverseSession.java | 16 +- .../java/edu/harvard/iq/dataverse/Shib.java | 1 + .../UserNotificationServiceBean.java | 4 + .../iq/dataverse/api/AbstractApiBean.java | 1 + .../edu/harvard/iq/dataverse/api/Admin.java | 34 +++- .../dataverse/api/datadeposit/SwordAuth.java | 1 + .../AuthenticationServiceBean.java | 10 +- .../oauth2/OAuth2LoginBackingBean.java | 1 + .../users/AuthenticatedUser.java | 26 ++- .../authorization/users/GuestUser.java | 7 +- .../authorization/users/PrivateUrlUser.java | 5 + .../dataverse/authorization/users/User.java | 2 + .../confirmemail/ConfirmEmailServiceBean.java | 4 + ...ddRoleAssigneesToExplicitGroupCommand.java | 7 + .../command/impl/AssignRoleCommand.java | 8 + .../command/impl/DisableUserCommand.java | 44 +++++ .../passwordreset/PasswordResetPage.java | 14 +- .../PasswordResetServiceBean.java | 12 +- .../iq/dataverse/util/json/JsonPrinter.java | 2 + src/main/java/propertyFiles/Bundle.properties | 7 +- .../V5.3.0.3__4475-disable-users.sql | 4 + .../iq/dataverse/api/DisableUsersIT.java | 160 ++++++++++++++++++ .../edu/harvard/iq/dataverse/api/UtilIT.java | 17 ++ 24 files changed, 371 insertions(+), 18 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java create mode 100644 src/main/resources/db/migration/V5.3.0.3__4475-disable-users.sql create mode 100644 src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java diff --git a/doc/sphinx-guides/source/developers/remote-users.rst b/doc/sphinx-guides/source/developers/remote-users.rst index c85571a55c0..3f8dd836661 100755 --- a/doc/sphinx-guides/source/developers/remote-users.rst +++ b/doc/sphinx-guides/source/developers/remote-users.rst @@ -10,7 +10,7 @@ Shibboleth and OAuth If you are working on anything related to users, please keep in mind that your changes will likely affect Shibboleth and OAuth users. For some background on user accounts in the Dataverse Software, see :ref:`auth-modes` section of Configuration in the Installation Guide. -Rather than setting up Shibboleth on your laptop, developers are advised to simply add a value to their database to enable Shibboleth "dev mode" like this: +Rather than setting up Shibboleth on your laptop, developers are advised to add the Shibboleth auth provider (see "Add the Shibboleth Authentication Provider to Your Dataverse Installation" at :doc:`/installation/shibboleth`) and add a value to their database to enable Shibboleth "dev mode" like this: ``curl http://localhost:8080/api/admin/settings/:DebugShibAccountType -X PUT -d RANDOM`` diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java index 2a2b02c5b18..fcf8da22f7e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java @@ -7,6 +7,8 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import edu.harvard.iq.dataverse.authorization.users.User; +import edu.harvard.iq.dataverse.util.BundleUtil; +import edu.harvard.iq.dataverse.util.JsfHelper; import edu.harvard.iq.dataverse.util.SessionUtil; import edu.harvard.iq.dataverse.util.SystemConfig; import java.io.IOException; @@ -92,7 +94,16 @@ public User getUser() { } public void setUser(User aUser) { - + // We check for disabled status here in "setUser" to ensure a common user + // experience across Builtin, Shib, OAuth, and OIDC users. + // If we want a different user experience for Builtin users, we can + // modify getUpdateAuthenticatedUser in AuthenticationServiceBean + // (and probably other places). + if (aUser instanceof AuthenticatedUser && aUser.isDisabled()) { + logger.info("Login attempt by disabled user " + aUser.getIdentifier() + "."); + JsfHelper.addErrorMessage(BundleUtil.getStringFromBundle("disabled.error")); + return; + } FacesContext context = FacesContext.getCurrentInstance(); // Log the login/logout and Change the session id if we're using the UI and have // a session, versus an API call with no session - (i.e. /admin/submitToArchive() @@ -210,6 +221,9 @@ public void dismissMessage(BannerMessage message){ } public void configureSessionTimeout() { + if (user instanceof GuestUser) { + return; + } HttpSession httpSession = (HttpSession) FacesContext.getCurrentInstance().getExternalContext().getSession(false); if (httpSession != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/Shib.java b/src/main/java/edu/harvard/iq/dataverse/Shib.java index 889bdaff03a..4bd38d2377d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Shib.java +++ b/src/main/java/edu/harvard/iq/dataverse/Shib.java @@ -358,6 +358,7 @@ public String confirmAndConvertAccount() { private void logInUserAndSetShibAttributes(AuthenticatedUser au) { au.setShibIdentityProvider(shibIdp); + // setUser checks for disabled users. session.setUser(au); session.configureSessionTimeout(); logger.fine("Groups for user " + au.getId() + " (" + au.getIdentifier() + "): " + getGroups(au)); diff --git a/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java index 071805d3d26..f02962498c8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java @@ -103,6 +103,10 @@ public void sendNotification(AuthenticatedUser dataverseUser, Timestamp sendDate } public void sendNotification(AuthenticatedUser dataverseUser, Timestamp sendDate, Type type, Long objectId, String comment, AuthenticatedUser requestor, boolean isHtmlContent) { + if (dataverseUser.isDisabled()) { + logger.info("An attempt was made to send a " + type + " notification to a disabled user: " + dataverseUser); + return; + } UserNotification userNotification = new UserNotification(); userNotification.setUser(dataverseUser); userNotification.setSendDate(sendDate); diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 551c0a797fc..fdc0b97aa86 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -372,6 +372,7 @@ protected AuthenticatedUser findAuthenticatedUserOrDie() throws WrappedResponse private AuthenticatedUser findAuthenticatedUserOrDie( String key ) throws WrappedResponse { + // No check for disabled user because it's done in authSvc.lookupUser. AuthenticatedUser authUser = authSvc.lookupUser(key); if ( authUser != null ) { authUser = userSvc.updateLastApiUseTime(authUser); 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 b52665a7747..4c39103ce3e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -87,6 +87,7 @@ import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.impl.MergeInAccountCommand; import edu.harvard.iq.dataverse.engine.command.impl.ChangeUserIdentifierCommand; +import edu.harvard.iq.dataverse.engine.command.impl.DisableUserCommand; import edu.harvard.iq.dataverse.engine.command.impl.RegisterDvObjectCommand; import edu.harvard.iq.dataverse.ingest.IngestServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; @@ -381,9 +382,36 @@ private Response deleteAuthenticatedUser(AuthenticatedUser au) { return ok("AuthenticatedUser " + au.getIdentifier() + " deleted. "); } - - + @POST + @Path("authenticatedUsers/{identifier}/disable") + public Response disableAuthenticatedUser(@PathParam("identifier") String identifier) { + AuthenticatedUser user = authSvc.getAuthenticatedUser(identifier); + if (user != null) { + return disableAuthenticatedUser(user); + } + return error(Response.Status.BAD_REQUEST, "User " + identifier + " not found."); + } + + @POST + @Path("authenticatedUsers/id/{id}/disable") + public Response disableAuthenticatedUserById(@PathParam("id") Long id) { + AuthenticatedUser user = authSvc.findByID(id); + if (user != null) { + return disableAuthenticatedUser(user); + } + return error(Response.Status.BAD_REQUEST, "User " + id + " not found."); + } + + private Response disableAuthenticatedUser(AuthenticatedUser userToDisable) { + try { + execCommand(new DisableUserCommand(createDataverseRequest(findUserOrDie()), userToDisable)); + return ok("User " + userToDisable.getIdentifier() + " disabled."); + } catch (WrappedResponse ex) { + return ex.getResponse(); + } + } + @POST @Path("publishDataverseAsCreator/{id}") public Response publishDataverseAsCreator(@PathParam("id") long id) { @@ -1686,7 +1714,7 @@ public Response submitDatasetVersionToArchive(@PathParam("id") String dsid, @Pat // DataverseRequest and is sent to the back-end command where it is used to get // the API Token which is then used to retrieve files (e.g. via S3 direct // downloads) to create the Bag - session.setUser(au); + session.setUser(au); // TODO: Stop using session. Use createDataverseRequest instead. Dataset ds = findDatasetOrDie(dsid); DatasetVersion dv = datasetversionService.findByFriendlyVersionNumber(ds.getId(), versionNumber); diff --git a/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java b/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java index 4a474f62894..339832989d6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java @@ -36,6 +36,7 @@ public AuthenticatedUser auth(AuthCredentials authCredentials) throws SwordAuthE throw new SwordAuthException(msg); } + // Checking if the user is disabled is done inside findUserByApiToken. AuthenticatedUser authenticatedUserFromToken = findUserByApiToken(username); if (authenticatedUserFromToken == null) { String msg = "User not found based on API token."; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java index c9a54dba078..c34015a5dda 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java @@ -309,7 +309,7 @@ public AuthenticatedUser getUpdateAuthenticatedUser( String authenticationProvid // yay! see if we already have this user. AuthenticatedUser user = lookupUser(authenticationProviderId, resp.getUserId()); - if (user != null){ + if (user != null && !user.isDisabled()) { user = userService.updateLastLogin(user); } @@ -453,7 +453,13 @@ public AuthenticatedUser lookupUser( String apiToken ) { } } - return tkn.getAuthenticatedUser(); + AuthenticatedUser user = tkn.getAuthenticatedUser(); + if (!user.isDisabled()) { + return user; + } else { + logger.info("attempted access with token from disabled user: " + apiToken); + return null; + } } /* diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java index e42f82d48d8..c034f7ebc8b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java @@ -106,6 +106,7 @@ public void exchangeCodeForToken() throws IOException { } else { // login the user and redirect to HOME of intended page (if any). + // setUser checks for disabled users. session.setUser(dvUser); session.configureSessionTimeout(); final OAuth2TokenData tokenData = oauthUser.getTokenData(); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index 12161eb1a59..ae485103be6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -55,7 +55,8 @@ @NamedQuery( name="AuthenticatedUser.filter", query="select au from AuthenticatedUser au WHERE (" + "LOWER(au.userIdentifier) like LOWER(:query) OR " - + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query))"), + + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query)) " + + "AND au.disabled != true"), @NamedQuery( name="AuthenticatedUser.findAdminUser", query="select au from AuthenticatedUser au WHERE " + "au.superuser = true " @@ -114,6 +115,12 @@ public class AuthenticatedUser implements User, Serializable { private boolean superuser; + @Column(nullable=true) + private boolean disabled; + + @Column(nullable=true) + private Timestamp disabledTime; + /** * @todo Consider storing a hash of *all* potentially interesting Shibboleth * attribute key/value pairs, not just the Identity Provider (IdP). @@ -303,6 +310,23 @@ public void setSuperuser(boolean superuser) { this.superuser = superuser; } + @Override + public boolean isDisabled() { + return disabled; + } + + public void setDisabled(boolean disabled) { + this.disabled = disabled; + } + + public Timestamp getDisabledTime() { + return disabledTime; + } + + public void setDisabledTime(Timestamp disabledTime) { + this.disabledTime = disabledTime; + } + @OneToOne(mappedBy = "authenticatedUser") private AuthenticatedUserLookup authenticatedUserLookup; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java index f16fa5afe36..6c4d45d6674 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java @@ -32,7 +32,12 @@ public RoleAssigneeDisplayInfo getDisplayInfo() { public boolean isSuperuser() { return false; } - + + @Override + public boolean isDisabled() { + return false; + } + @Override public boolean equals( Object o ) { return (o instanceof GuestUser); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java index 59c3240fdfa..e318cf5ce8c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java @@ -47,6 +47,11 @@ public boolean isSuperuser() { return false; } + @Override + public boolean isDisabled() { + return false; + } + @Override public String getIdentifier() { return PREFIX + datasetId; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java index ea35f87d178..f3291a2a11f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java @@ -14,4 +14,6 @@ public interface User extends RoleAssignee, Serializable { public boolean isSuperuser(); + public boolean isDisabled(); + } diff --git a/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java index bc3b70326f1..574cfa11cde 100644 --- a/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java @@ -169,6 +169,10 @@ public ConfirmEmailExecResponse processToken(String tokenQueried) { long nowInMilliseconds = new Date().getTime(); Timestamp emailConfirmed = new Timestamp(nowInMilliseconds); AuthenticatedUser authenticatedUser = confirmEmailData.getAuthenticatedUser(); + if (authenticatedUser.isDisabled()) { + logger.fine("User is disabled."); + return null; + } authenticatedUser.setEmailConfirmed(emailConfirmed); em.remove(confirmEmailData); return goodTokenCanProceed; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java index f6bd1316e44..d4e47a5d449 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java @@ -4,6 +4,7 @@ import edu.harvard.iq.dataverse.authorization.RoleAssignee; import edu.harvard.iq.dataverse.authorization.groups.GroupException; import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroup; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.engine.command.AbstractCommand; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; @@ -49,6 +50,12 @@ public ExplicitGroup execute(CommandContext ctxt) throws CommandException { if ( ra == null ) { nonexistentRAs.add( rai ); } else { + if (ra instanceof AuthenticatedUser) { + AuthenticatedUser user = (AuthenticatedUser) ra; + if (user.isDisabled()) { + throw new IllegalCommandException("User " + user.getUserIdentifier() + " is disabled and cannot be added to a group.", this); + } + } try { explicitGroup.add(ra); } catch (GroupException ex) { diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java index 34263599ff0..274d485c5d2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java @@ -10,10 +10,12 @@ import edu.harvard.iq.dataverse.RoleAssignment; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.RoleAssignee; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.engine.command.AbstractCommand; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; import java.util.Collections; import java.util.Map; import java.util.Set; @@ -49,6 +51,12 @@ public AssignRoleCommand(RoleAssignee anAssignee, DataverseRole aRole, DvObject @Override public RoleAssignment execute(CommandContext ctxt) throws CommandException { + if (grantee instanceof AuthenticatedUser) { + AuthenticatedUser user = (AuthenticatedUser) grantee; + if (user.isDisabled()) { + throw new IllegalCommandException("User " + user.getUserIdentifier() + " is disabled and cannot be given a role.", this); + } + } // TODO make sure the role is defined on the dataverse. RoleAssignment roleAssignment = new RoleAssignment(role, grantee, defPoint, privateUrlToken); return ctxt.roles().save(roleAssignment); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java new file mode 100644 index 00000000000..58ee7295e22 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java @@ -0,0 +1,44 @@ +package edu.harvard.iq.dataverse.engine.command.impl; + +import edu.harvard.iq.dataverse.DvObject; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.engine.command.AbstractCommand; +import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.engine.command.DataverseRequest; +import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; +import edu.harvard.iq.dataverse.engine.command.exception.CommandException; +import edu.harvard.iq.dataverse.engine.command.exception.PermissionException; +import java.sql.Timestamp; +import java.util.Date; + +// Superuser-only enforced below. +@RequiredPermissions({}) +public class DisableUserCommand extends AbstractCommand { + + private DataverseRequest request; + private AuthenticatedUser userToDisable; + + public DisableUserCommand(DataverseRequest request, AuthenticatedUser userToDisable) { + super(request, (DvObject) null); + this.request = request; + this.userToDisable = userToDisable; + } + + @Override + public AuthenticatedUser execute(CommandContext ctxt) throws CommandException { + if (!(getUser() instanceof AuthenticatedUser) || !getUser().isSuperuser()) { + throw new PermissionException("Disable user command can only be called by superusers.", this, null, null); + } + if (userToDisable == null) { + throw new CommandException("Cannot disable user. User not found.", this); + } + ctxt.engine().submit(new RevokeAllRolesCommand(userToDisable, request)); + ctxt.authentication().removeAuthentictedUserItems(userToDisable); + ctxt.notifications().findByUser(userToDisable.getId()).forEach(ctxt.notifications()::delete); + userToDisable.setDisabled(true); + userToDisable.setDisabledTime(new Timestamp(new Date().getTime())); + AuthenticatedUser disabledUser = ctxt.authentication().save(userToDisable); + return disabledUser; + } + +} diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java index 532c0216038..b5cef3731f5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java @@ -121,13 +121,15 @@ public String sendPasswordResetLink() { actionLogSvc.log( new ActionLogRecord(ActionLogRecord.ActionType.BuiltinUser, "passwordResetSent") .setInfo("Email Address: " + emailAddress) ); } else { - /** - * @todo remove "single" when it's no longer necessary. See - * https://github.com/IQSS/dataverse/issues/844 and - * https://github.com/IQSS/dataverse/issues/1141 - */ - logger.log(Level.INFO, "Couldn''t find single account using {0}", emailAddress); + logger.log(Level.INFO, "Cannot find account (or it's disabled) given {0}", emailAddress); } + /** + * We show this "an email will be sent" message no matter what (if + * the account can be found or not, if the account has been disabled + * or not) to prevent hackers from figuring out if you have an + * account based on your email address. Yes, this is a white lie + * sometimes, in the name of security. + */ FacesContext.getCurrentInstance().addMessage(null, new FacesMessage(FacesMessage.SEVERITY_INFO, BundleUtil.getStringFromBundle("passwdVal.passwdReset.resetInitiated"), BundleUtil.getStringFromBundle("passwdReset.successSubmit.tip", Arrays.asList(emailAddress)))); } catch (PasswordResetException ex) { diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java index c3f0f7f2abc..9b28be6527c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java @@ -52,13 +52,23 @@ public class PasswordResetServiceBean { * Initiate the password reset process. * * @param emailAddress - * @return {@link PasswordResetInitResponse} + * @return {@link PasswordResetInitResponse} with empty PasswordResetData if + * the reset won't continue (no user, disabled user). * @throws edu.harvard.iq.dataverse.passwordreset.PasswordResetException */ // inspired by Troy Hunt: Everything you ever wanted to know about building a secure password reset feature - http://www.troyhunt.com/2012/05/everything-you-ever-wanted-to-know.html public PasswordResetInitResponse requestReset(String emailAddress) throws PasswordResetException { deleteAllExpiredTokens(); AuthenticatedUser authUser = authService.getAuthenticatedUserByEmail(emailAddress); + // This null check is for the NPE reported in https://github.com/IQSS/dataverse/issues/5462 + if (authUser == null) { + logger.info("Cannot find a user based on " + emailAddress + ". Cannot reset password."); + return new PasswordResetInitResponse(false); + } + if (authUser.isDisabled()) { + logger.info("Cannot reset password for " + emailAddress + " because account is disabled."); + return new PasswordResetInitResponse(false); + } BuiltinUser user = dataverseUserService.findByUserName(authUser.getUserIdentifier()); if (user != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index c37efc3178f..668f3cbf771 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -107,6 +107,8 @@ public static JsonObjectBuilder json(AuthenticatedUser authenticatedUser) { .add("lastName", authenticatedUser.getLastName()) .add("email", authenticatedUser.getEmail()) .add("superuser", authenticatedUser.isSuperuser()) + .add("disabled", authenticatedUser.isDisabled()) + .add("disabledTime", authenticatedUser.getDisabledTime()) .add("affiliation", authenticatedUser.getAffiliation()) .add("position", authenticatedUser.getPosition()) .add("persistentUserId", authenticatedUser.getAuthenticatedUserLookup().getPersistentUserId()) diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 6cdf8b93550..227234da279 100755 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -361,7 +361,7 @@ shib.dataverseUsername=Dataverse Username shib.currentDataversePassword=Current Dataverse Password shib.accountInformation=Account Information shib.offerToCreateNewAccount=This information is provided by your institution and will be used to create your Dataverse account. -shib.passwordRejected=Validation Error - Your account can only be converted if you provide the correct password for your existing account. +shib.passwordRejected=Validation Error - Your account can only be converted if you provide the correct password for your existing account. If your existing account has been disabled by an administrator, you cannot convert your account. # oauth2/firstLogin.xhtml oauth2.btn.convertAccount=Convert Existing Account @@ -396,7 +396,7 @@ oauth2.newAccount.emailInvalid=Invalid email address. oauth2.convertAccount.explanation=Please enter your {0} account username or email and password to convert your account to the {1} log in option. Learn more about converting your account. oauth2.convertAccount.username=Existing username oauth2.convertAccount.password=Password -oauth2.convertAccount.authenticationFailed=Authentication failed - bad username or password. +oauth2.convertAccount.authenticationFailed=Your account can only be converted if you provide the correct username and password for your existing account. If your existing account has been disabled by an administrator, you cannot convert your account. oauth2.convertAccount.buttonTitle=Convert Account oauth2.convertAccount.success=Your Dataverse account is now associated with your {0} account. @@ -404,6 +404,9 @@ oauth2.convertAccount.success=Your Dataverse account is now associated with your oauth2.callback.page.title=OAuth Callback oauth2.callback.message=Authentication Error - Dataverse could not authenticate your login with the provider that you selected. Please make sure you authorize your account to connect with Dataverse. For more details about the information being requested, see the User Guide. +# disabled user accounts +disabled.error=Sorry, your account has been disabled. + # tab on dataverseuser.xhtml apitoken.title=API Token apitoken.message=Your API Token is valid for a year. Check out our {0}API Guide{1} for more information on using your API Token with the Dataverse APIs. diff --git a/src/main/resources/db/migration/V5.3.0.3__4475-disable-users.sql b/src/main/resources/db/migration/V5.3.0.3__4475-disable-users.sql new file mode 100644 index 00000000000..48823a72247 --- /dev/null +++ b/src/main/resources/db/migration/V5.3.0.3__4475-disable-users.sql @@ -0,0 +1,4 @@ +-- Users can be disabled. +ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS disabled BOOLEAN; +-- A timestamp of when the user was disabled. +ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS disabledtime timestamp without time zone; diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java new file mode 100644 index 00000000000..fdec2480db6 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java @@ -0,0 +1,160 @@ +package edu.harvard.iq.dataverse.api; + +import com.jayway.restassured.RestAssured; +import com.jayway.restassured.path.json.JsonPath; +import com.jayway.restassured.response.Response; +import edu.harvard.iq.dataverse.authorization.DataverseRole; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.CREATED; +import static javax.ws.rs.core.Response.Status.FORBIDDEN; +import static javax.ws.rs.core.Response.Status.OK; +import static javax.ws.rs.core.Response.Status.UNAUTHORIZED; +import static org.hamcrest.CoreMatchers.equalTo; +import org.junit.BeforeClass; +import org.junit.Test; + +public class DisableUsersIT { + + @BeforeClass + public static void setUp() { + RestAssured.baseURI = UtilIT.getRestAssuredBaseUri(); + } + + @Test + public void testDisableUser() { + + Response createSuperuser = UtilIT.createRandomUser(); + createSuperuser.then().assertThat().statusCode(OK.getStatusCode()); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createDataverse = UtilIT.createRandomDataverse(superuserApiToken); + createDataverse.then().assertThat().statusCode(CREATED.getStatusCode()); + String dataverseAlias = UtilIT.getAliasFromResponse(createDataverse); + Integer dataverseId = UtilIT.getDataverseIdFromResponse(createDataverse); + + Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, superuserApiToken); + createDataset.prettyPrint(); + createDataset.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String datasetPersistentId = UtilIT.getDatasetPersistentIdFromResponse(createDataset); + + Response createUser = UtilIT.createRandomUser(); + createUser.then().assertThat().statusCode(OK.getStatusCode()); + String username = UtilIT.getUsernameFromResponse(createUser); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response grantRoleBeforeDisable = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); + grantRoleBeforeDisable.prettyPrint(); + grantRoleBeforeDisable.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.assignee", equalTo("@" + username)) + .body("data._roleAlias", equalTo("admin")); + + String aliasInOwner = "groupFor" + dataverseAlias; + String displayName = "Group for " + dataverseAlias; + String user2identifier = "@" + username; + Response createGroup = UtilIT.createGroup(dataverseAlias, aliasInOwner, displayName, superuserApiToken); + createGroup.prettyPrint(); + createGroup.then().assertThat() + .statusCode(CREATED.getStatusCode()); + + String groupIdentifier = JsonPath.from(createGroup.asString()).getString("data.identifier"); + + List roleAssigneesToAdd = new ArrayList<>(); + roleAssigneesToAdd.add(user2identifier); + Response addToGroup = UtilIT.addToGroup(dataverseAlias, aliasInOwner, roleAssigneesToAdd, superuserApiToken); + addToGroup.prettyPrint(); + addToGroup.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response userTracesBeforeDisable = UtilIT.getUserTraces(username, superuserApiToken); + userTracesBeforeDisable.prettyPrint(); + userTracesBeforeDisable.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.traces.roleAssignments.items[0].definitionPointName", equalTo(dataverseAlias)) + .body("data.traces.roleAssignments.items[0].definitionPointId", equalTo(dataverseId)) + .body("data.traces.explicitGroups.items[0].name", equalTo("Group for " + dataverseAlias)); + + Response failToDisableUserWithoutSuperuserApiToken = UtilIT.disableUser(username, apiToken); + failToDisableUserWithoutSuperuserApiToken.prettyPrint(); + failToDisableUserWithoutSuperuserApiToken.then().assertThat().statusCode(UNAUTHORIZED.getStatusCode()); + + Response disableUser = UtilIT.disableUser(username, superuserApiToken); + disableUser.prettyPrint(); + disableUser.then().assertThat().statusCode(OK.getStatusCode()); + + Response getUser = UtilIT.getAuthenticatedUser(username, superuserApiToken); + getUser.prettyPrint(); + getUser.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.disabled", equalTo(true)); + + Response getUserDisabled = UtilIT.getAuthenticatedUserByToken(apiToken); + getUserDisabled.prettyPrint(); + getUserDisabled.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + + Response userTracesAfterDisable = UtilIT.getUserTraces(username, superuserApiToken); + userTracesAfterDisable.prettyPrint(); + userTracesAfterDisable.then().assertThat() + .statusCode(OK.getStatusCode()) + /** + * Here we are showing the the following were deleted: + * + * - role assignments + * + * - membership in explict groups. + */ + .body("data.traces", equalTo(Collections.EMPTY_MAP)); + + Response grantRoleAfterDisable = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); + grantRoleAfterDisable.prettyPrint(); + grantRoleAfterDisable.then().assertThat() + .statusCode(FORBIDDEN.getStatusCode()) + .body("message", equalTo("User " + username + " is disabled and cannot be given a role.")); + + Response addToGroupAfter = UtilIT.addToGroup(dataverseAlias, aliasInOwner, roleAssigneesToAdd, superuserApiToken); + addToGroupAfter.prettyPrint(); + addToGroupAfter.then().assertThat() + .statusCode(FORBIDDEN.getStatusCode()) + .body("message", equalTo("User " + username + " is disabled and cannot be added to a group.")); + + Response grantRoleOnDataset = UtilIT.grantRoleOnDataset(datasetPersistentId, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); + grantRoleOnDataset.prettyPrint(); + grantRoleOnDataset.then().assertThat() + .statusCode(FORBIDDEN.getStatusCode()) + .body("message", equalTo("User " + username + " is disabled and cannot be given a role.")); + + } + + @Test + public void testDisableUserById() { + + Response createSuperuser = UtilIT.createRandomUser(); + createSuperuser.then().assertThat().statusCode(OK.getStatusCode()); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + createUser.then().assertThat().statusCode(OK.getStatusCode()); + String username = UtilIT.getUsernameFromResponse(createUser); + Long userId = JsonPath.from(createUser.body().asString()).getLong("data.user.id"); + String apiToken = UtilIT.getApiTokenFromResponse(createUser); + + Response disableUser = UtilIT.disableUser(userId, superuserApiToken); + disableUser.prettyPrint(); + disableUser.then().assertThat().statusCode(OK.getStatusCode()); + } + +} diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index dee35556729..4bf55054ba6 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -950,6 +950,22 @@ static public Response grantRoleOnDataverse(String definitionPoint, String role, .post("api/dataverses/" + definitionPoint + "/assignments?key=" + apiToken); } + public static Response disableUser(String username, String apiToken) { +// public static Response disableUser(String username, String apiToken) { + Response disableUserResponse = given() + .header(API_TOKEN_HTTP_HEADER, apiToken) +// .post("/api/users/" + username + "/disable"); + .post("/api/admin/authenticatedUsers/" + username + "/disable"); + return disableUserResponse; + } + + public static Response disableUser(Long userId, String apiToken) { + Response disableUserResponse = given() + .header(API_TOKEN_HTTP_HEADER, apiToken) + .post("/api/admin/authenticatedUsers/id/" + userId + "/disable"); + return disableUserResponse; + } + public static Response deleteUser(String username) { Response deleteUserResponse = given() .delete("/api/admin/authenticatedUsers/" + username + "/"); @@ -1188,6 +1204,7 @@ static Response listAuthenticatedUsers(String apiToken) { return response; } + // TODO: Consider removing apiToken since it isn't used by the API itself. static Response getAuthenticatedUser(String userIdentifier, String apiToken) { Response response = given() .header(API_TOKEN_HTTP_HEADER, apiToken) From 0f2e74bcfa0c6e000070d14e4944232e42d4431b Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 18 Feb 2021 16:48:55 -0500 Subject: [PATCH 11/44] tweaks to API guide, docs for traces, removeRoles #4475 --- doc/sphinx-guides/source/api/native-api.rst | 82 ++++++++++++++++++--- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 646961e1526..fbf9ed266fc 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3079,21 +3079,41 @@ Deletes an ``AuthenticatedUser`` whose ``id`` is passed. :: DELETE http://$SERVER/api/admin/authenticatedUsers/id/$id -Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. If a user cannot be deleted for this reason, you can choose to :ref:`disable a user`. - +Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. To see where in the database these actions are stored you can use the :ref:`show-user-traces-api` API. If a user cannot be deleted for this reason, you can choose to :ref:`disable a user`. + .. _disable-a-user: Disable a User ~~~~~~~~~~~~~~ -Disables an ``AuthenticatedUser`` whose ``identifier`` (without the ``@`` sign) is passed. :: +Disables a user. - PUT http://$SERVER/api/admin/authenticatedUsers/disable/$identifier - -Disables an ``AuthenticatedUser`` whose ``id`` is passed. :: +.. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of export below. + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=http://localhost:8080 + export USERNAME=jdoe + + curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/$USERNAME/disable + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash + + curl -H X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -X POST http://localhost:8080/api/admin/authenticatedUsers/jdoe/disable + +The database ID of the user can be passed instead of the username. + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=http://localhost:8080 + export USERID=42 + + curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/id/$USERID/disable - PUT http://$SERVER/api/admin/authenticatedUsers/disable/id/$id - Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Disable User endpoint for users who have taken certain actions in the system alongside a Delete User endpoint to remove users that haven't taken certain actions in the system is by design. This is an irreversible action. There is no option to undisable a user. @@ -3109,10 +3129,54 @@ Disabling a user with this endpoint will: Disabling a user with this endpoint will keep: -- The user's contributions to datasets, including dataset creation, file uploads, and publishing. +- The user's contributions to datasets, including dataset creation, file uploads, and publishing. - The user's access history to datafiles in the Dataverse installation, including guestbook records. - The user's account information (specifically name, email, affiliation, and position) +.. _show-user-traces-api: + +Show User Traces +~~~~~~~~~~~~~~~~ + +Show the traces that the user has left in the system, such as datasets created, guestbooks filled out, etc. This can be useful for understanding why a user cannot be deleted. A superuser API token is required. + +.. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of export below. + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=https://demo.dataverse.org + export USERNAME=jdoe + + curl -H "X-Dataverse-key:$API_TOKEN" -X GET $SERVER_URL/api/users/$USERNAME/traces + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash + + curl -H X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -X GET https://demo.dataverse.org/api/users/jdoe/traces + +Remove All Roles from a User +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Removes all roles from the user. This is equivalent of clicking the "Remove All Roles" button in the superuser dashboard. Note that you can preview the roles that will be removed with the :ref:`show-user-traces-api` API. A superuser API token is required. + +.. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of export below. + +.. code-block:: bash + + export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + export SERVER_URL=https://demo.dataverse.org + export USERNAME=jdoe + + curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/users/$USERNAME/removeRoles + +The fully expanded example above (without environment variables) looks like this: + +.. code-block:: bash + + curl -H X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -X POST http://localhost:8080/api/users/jdoe/removeRoles + List Role Assignments of a Role Assignee ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 96fc087486c9a883cb69987dfad90b5150faa053 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 19 Feb 2021 11:01:38 -0500 Subject: [PATCH 12/44] add release note #2419 #4475 --- doc/release-notes/2419-4475-7575-disable-users.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 doc/release-notes/2419-4475-7575-disable-users.md diff --git a/doc/release-notes/2419-4475-7575-disable-users.md b/doc/release-notes/2419-4475-7575-disable-users.md new file mode 100644 index 00000000000..612200905dc --- /dev/null +++ b/doc/release-notes/2419-4475-7575-disable-users.md @@ -0,0 +1,5 @@ +## Release Highlights + +### Disable Users API, Get User Traces API, Revoke Roles API + +A new API has been added to disable users to prevent them from logging in or otherwise being active in the system. Disabling a user is an alternative to deleting a user, especially when the latter is not possible due to the amount of interaction the user has had with the system. In order to learn more about a user before deleting, disabling, or merging, a new "get user traces" API is available that will show objects created, roles, group memberships, and more. Finally, the "remove all roles" button available in the superuser dashboard is now also available via API. From a9a368b9f1981c5c67b3dbf1d4c1fa0d5cbbe24d Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 19 Feb 2021 12:13:29 -0500 Subject: [PATCH 13/44] implement rule on merging disabled accounts #2419 #4475 "User accounts can only be merged if they are either both enabled or both disabled." --- .../command/impl/MergeInAccountCommand.java | 6 +- .../iq/dataverse/api/DisableUsersIT.java | 88 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java index a13cf27fcd6..c0e3b3ea89a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java @@ -58,7 +58,11 @@ public MergeInAccountCommand(DataverseRequest createDataverseRequest, Authentica @Override protected void executeImpl(CommandContext ctxt) throws CommandException { - + + if (consumedAU.isDisabled() && !ongoingAU.isDisabled() || !consumedAU.isDisabled() && ongoingAU.isDisabled()) { + throw new IllegalCommandException("User accounts can only be merged if they are either both enabled or both disabled.", this); + } + List baseRAList = ctxt.roleAssignees().getAssignmentsFor(ongoingAU.getIdentifier()); List consumedRAList = ctxt.roleAssignees().getAssignmentsFor(consumedAU.getIdentifier()); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java index fdec2480db6..bfba9388b93 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java @@ -157,4 +157,92 @@ public void testDisableUserById() { disableUser.then().assertThat().statusCode(OK.getStatusCode()); } + @Test + public void testMergeDisabledIntoEnabledUser() { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUserMergeTarget = UtilIT.createRandomUser(); + createUserMergeTarget.prettyPrint(); + String usernameMergeTarget = UtilIT.getUsernameFromResponse(createUserMergeTarget); + + Response createUserToMerge = UtilIT.createRandomUser(); + createUserToMerge.prettyPrint(); + String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); + + Response disableUser = UtilIT.disableUser(usernameToMerge, superuserApiToken); + disableUser.prettyPrint(); + disableUser.then().assertThat().statusCode(OK.getStatusCode()); + + // User accounts can only be merged if they are either both enabled or both disabled. + Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); + mergeAccounts.prettyPrint(); + mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + } + + @Test + public void testMergeEnabledIntoDisabledUser() { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUserMergeTarget = UtilIT.createRandomUser(); + createUserMergeTarget.prettyPrint(); + String usernameMergeTarget = UtilIT.getUsernameFromResponse(createUserMergeTarget); + + Response createUserToMerge = UtilIT.createRandomUser(); + createUserToMerge.prettyPrint(); + String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); + + Response disableUser = UtilIT.disableUser(usernameMergeTarget, superuserApiToken); + disableUser.prettyPrint(); + disableUser.then().assertThat().statusCode(OK.getStatusCode()); + + // User accounts can only be merged if they are either both enabled or both disabled. + Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); + mergeAccounts.prettyPrint(); + mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + } + + @Test + public void testMergeDisabledIntoDisabledUser() { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUserMergeTarget = UtilIT.createRandomUser(); + createUserMergeTarget.prettyPrint(); + String usernameMergeTarget = UtilIT.getUsernameFromResponse(createUserMergeTarget); + + Response createUserToMerge = UtilIT.createRandomUser(); + createUserToMerge.prettyPrint(); + String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); + + Response disableUserMergeTarget = UtilIT.disableUser(usernameMergeTarget, superuserApiToken); + disableUserMergeTarget.prettyPrint(); + disableUserMergeTarget.then().assertThat().statusCode(OK.getStatusCode()); + + Response disableUserToMerge = UtilIT.disableUser(usernameToMerge, superuserApiToken); + disableUserToMerge.prettyPrint(); + disableUserToMerge.then().assertThat().statusCode(OK.getStatusCode()); + + // User accounts can only be merged if they are either both enabled or both disabled. + Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); + mergeAccounts.prettyPrint(); + mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); + } + } From 8a7ed7b393263afc7f8c7a65d7bacd866b034d4b Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 19 Feb 2021 12:16:03 -0500 Subject: [PATCH 14/44] add new test classes: DeleteUsersIT and DisableUsersIT #2419 #4475 --- conf/docker-aio/run-test-suite.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf/docker-aio/run-test-suite.sh b/conf/docker-aio/run-test-suite.sh index 2b24f6c90b2..da9e56e0176 100755 --- a/conf/docker-aio/run-test-suite.sh +++ b/conf/docker-aio/run-test-suite.sh @@ -8,4 +8,4 @@ fi # Please note the "dataverse.test.baseurl" is set to run for "all-in-one" Docker environment. # TODO: Rather than hard-coding the list of "IT" classes here, add a profile to pom.xml. -source maven/maven.sh && mvn test -Dtest=DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT -Ddataverse.test.baseurl=$dvurl +source maven/maven.sh && mvn test -Dtest=DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DisableUsersIT -Ddataverse.test.baseurl=$dvurl From ef1061e7fe05be56317deb8d3dacbfc6fe35ff86 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 19 Feb 2021 15:09:35 -0500 Subject: [PATCH 15/44] disabled users cannot use the API #2419 #4475 --- doc/sphinx-guides/source/api/native-api.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index fbf9ed266fc..9c4f5ffb70d 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3121,6 +3121,7 @@ This is an irreversible action. There is no option to undisable a user. Disabling a user with this endpoint will: - Disable the user's ability to log in to the Dataverse installation. A message will be shown, stating that the account has been disabled. The user will not able to create a new account with the same email address, ORCID, Shibboleth, or other login type. +- Disable the user's ability to use the API - Remove the user's access from all Dataverse collections, datasets and files - Prevent a user from being assigned any roles - Cancel any pending file access requests generated by the user From fa33eee2237f135204cb078742d540145fb463c8 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 19 Feb 2021 15:24:09 -0500 Subject: [PATCH 16/44] cleanup --- src/main/java/edu/harvard/iq/dataverse/api/Users.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Users.java b/src/main/java/edu/harvard/iq/dataverse/api/Users.java index e8558f88d6e..6e0476711ec 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Users.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Users.java @@ -213,7 +213,7 @@ public Response removeUserRoles(@PathParam("identifier") String identifier) { @Path("{identifier}/traces") public Response getTraces(@PathParam("identifier") String identifier) { try { - AuthenticatedUser userToQuery = authSvc.getAuthenticatedUser(identifier); // TODO: do lookup here? What if null? + AuthenticatedUser userToQuery = authSvc.getAuthenticatedUser(identifier); JsonObjectBuilder jsonObj = execCommand(new GetUserTracesCommand(createDataverseRequest(findUserOrDie()), userToQuery)); return ok(jsonObj); } catch (WrappedResponse ex) { From e73c43b701956b6be71cfa85d2d82c829624d847 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 23 Feb 2021 12:16:48 -0500 Subject: [PATCH 17/44] rename sql update script #2419 #4475 --- ...3__4475-disable-users.sql => V5.3.0.4__4475-disable-users.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/{V5.3.0.3__4475-disable-users.sql => V5.3.0.4__4475-disable-users.sql} (100%) diff --git a/src/main/resources/db/migration/V5.3.0.3__4475-disable-users.sql b/src/main/resources/db/migration/V5.3.0.4__4475-disable-users.sql similarity index 100% rename from src/main/resources/db/migration/V5.3.0.3__4475-disable-users.sql rename to src/main/resources/db/migration/V5.3.0.4__4475-disable-users.sql From 8f08f7b31f268f9a649db93383824c7aab629e61 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 26 Feb 2021 16:02:13 -0500 Subject: [PATCH 18/44] disabled users should never get notifications #2419 #4475 That means it's safe not to have this check. --- .../edu/harvard/iq/dataverse/UserNotificationServiceBean.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java index f02962498c8..071805d3d26 100644 --- a/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/UserNotificationServiceBean.java @@ -103,10 +103,6 @@ public void sendNotification(AuthenticatedUser dataverseUser, Timestamp sendDate } public void sendNotification(AuthenticatedUser dataverseUser, Timestamp sendDate, Type type, Long objectId, String comment, AuthenticatedUser requestor, boolean isHtmlContent) { - if (dataverseUser.isDisabled()) { - logger.info("An attempt was made to send a " + type + " notification to a disabled user: " + dataverseUser); - return; - } UserNotification userNotification = new UserNotification(); userNotification.setUser(dataverseUser); userNotification.setSendDate(sendDate); From 23b8bf6384eb50c9596f04a8caf4f633b4937596 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 2 Mar 2021 14:38:17 -0500 Subject: [PATCH 19/44] use default methods for isDisabled on User #2419 #4475 --- .../harvard/iq/dataverse/authorization/users/GuestUser.java | 5 ----- .../iq/dataverse/authorization/users/PrivateUrlUser.java | 5 ----- .../edu/harvard/iq/dataverse/authorization/users/User.java | 4 +++- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java index 6c4d45d6674..16de1b2eaff 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/GuestUser.java @@ -33,11 +33,6 @@ public boolean isSuperuser() { return false; } - @Override - public boolean isDisabled() { - return false; - } - @Override public boolean equals( Object o ) { return (o instanceof GuestUser); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java index e318cf5ce8c..59c3240fdfa 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java @@ -47,11 +47,6 @@ public boolean isSuperuser() { return false; } - @Override - public boolean isDisabled() { - return false; - } - @Override public String getIdentifier() { return PREFIX + datasetId; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java index f3291a2a11f..baaf0e5f627 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java @@ -14,6 +14,8 @@ public interface User extends RoleAssignee, Serializable { public boolean isSuperuser(); - public boolean isDisabled(); + default boolean isDisabled() { + return false; + } } From d6e11732cb7cacd67ccd77e6f6ffb38eb0de8515 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 2 Mar 2021 17:05:33 -0500 Subject: [PATCH 20/44] run disable user command without superuser API token #2419 #4475 --- doc/sphinx-guides/source/api/native-api.rst | 2 +- .../java/edu/harvard/iq/dataverse/api/Admin.java | 6 +++++- .../harvard/iq/dataverse/api/DisableUsersIT.java | 16 ++++++---------- .../edu/harvard/iq/dataverse/api/UtilIT.java | 8 ++------ 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 9c4f5ffb70d..c99b903ff38 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3086,7 +3086,7 @@ Note: If the user has performed certain actions such as creating or contributing Disable a User ~~~~~~~~~~~~~~ -Disables a user. +Disables a user. A superuser API token is not required but the command will operate using the first superuser it finds. .. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of export below. 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 4c39103ce3e..6e8ed301a68 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -404,8 +404,12 @@ public Response disableAuthenticatedUserById(@PathParam("id") Long id) { } private Response disableAuthenticatedUser(AuthenticatedUser userToDisable) { + AuthenticatedUser superuser = authSvc.getAdminUser(); + if (superuser == null) { + return error(Response.Status.INTERNAL_SERVER_ERROR, "Cannot find superuser to execute DisableUserCommand."); + } try { - execCommand(new DisableUserCommand(createDataverseRequest(findUserOrDie()), userToDisable)); + execCommand(new DisableUserCommand(createDataverseRequest(superuser), userToDisable)); return ok("User " + userToDisable.getIdentifier() + " disabled."); } catch (WrappedResponse ex) { return ex.getResponse(); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java index bfba9388b93..d9c23b100e0 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java @@ -83,11 +83,7 @@ public void testDisableUser() { .body("data.traces.roleAssignments.items[0].definitionPointId", equalTo(dataverseId)) .body("data.traces.explicitGroups.items[0].name", equalTo("Group for " + dataverseAlias)); - Response failToDisableUserWithoutSuperuserApiToken = UtilIT.disableUser(username, apiToken); - failToDisableUserWithoutSuperuserApiToken.prettyPrint(); - failToDisableUserWithoutSuperuserApiToken.then().assertThat().statusCode(UNAUTHORIZED.getStatusCode()); - - Response disableUser = UtilIT.disableUser(username, superuserApiToken); + Response disableUser = UtilIT.disableUser(username); disableUser.prettyPrint(); disableUser.then().assertThat().statusCode(OK.getStatusCode()); @@ -152,7 +148,7 @@ public void testDisableUserById() { Long userId = JsonPath.from(createUser.body().asString()).getLong("data.user.id"); String apiToken = UtilIT.getApiTokenFromResponse(createUser); - Response disableUser = UtilIT.disableUser(userId, superuserApiToken); + Response disableUser = UtilIT.disableUser(userId); disableUser.prettyPrint(); disableUser.then().assertThat().statusCode(OK.getStatusCode()); } @@ -175,7 +171,7 @@ public void testMergeDisabledIntoEnabledUser() { createUserToMerge.prettyPrint(); String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); - Response disableUser = UtilIT.disableUser(usernameToMerge, superuserApiToken); + Response disableUser = UtilIT.disableUser(usernameToMerge); disableUser.prettyPrint(); disableUser.then().assertThat().statusCode(OK.getStatusCode()); @@ -203,7 +199,7 @@ public void testMergeEnabledIntoDisabledUser() { createUserToMerge.prettyPrint(); String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); - Response disableUser = UtilIT.disableUser(usernameMergeTarget, superuserApiToken); + Response disableUser = UtilIT.disableUser(usernameMergeTarget); disableUser.prettyPrint(); disableUser.then().assertThat().statusCode(OK.getStatusCode()); @@ -231,11 +227,11 @@ public void testMergeDisabledIntoDisabledUser() { createUserToMerge.prettyPrint(); String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); - Response disableUserMergeTarget = UtilIT.disableUser(usernameMergeTarget, superuserApiToken); + Response disableUserMergeTarget = UtilIT.disableUser(usernameMergeTarget); disableUserMergeTarget.prettyPrint(); disableUserMergeTarget.then().assertThat().statusCode(OK.getStatusCode()); - Response disableUserToMerge = UtilIT.disableUser(usernameToMerge, superuserApiToken); + Response disableUserToMerge = UtilIT.disableUser(usernameToMerge); disableUserToMerge.prettyPrint(); disableUserToMerge.then().assertThat().statusCode(OK.getStatusCode()); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index 54ce10640dc..db9f94bf28d 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -961,18 +961,14 @@ static public Response grantRoleOnDataverse(String definitionPoint, String role, .post("api/dataverses/" + definitionPoint + "/assignments?key=" + apiToken); } - public static Response disableUser(String username, String apiToken) { -// public static Response disableUser(String username, String apiToken) { + public static Response disableUser(String username) { Response disableUserResponse = given() - .header(API_TOKEN_HTTP_HEADER, apiToken) -// .post("/api/users/" + username + "/disable"); .post("/api/admin/authenticatedUsers/" + username + "/disable"); return disableUserResponse; } - public static Response disableUser(Long userId, String apiToken) { + public static Response disableUser(Long userId) { Response disableUserResponse = given() - .header(API_TOKEN_HTTP_HEADER, apiToken) .post("/api/admin/authenticatedUsers/id/" + userId + "/disable"); return disableUserResponse; } From 6e61841cb6a5d870cdd6c16ae718c1fdf1485f3c Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 3 Mar 2021 15:45:24 -0500 Subject: [PATCH 21/44] remove password reset data with cascade #7575 --- .../authorization/providers/builtin/BuiltinUser.java | 7 +++++++ .../providers/builtin/BuiltinUserServiceBean.java | 2 -- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java index fd7231e827c..9ae4e4b0e87 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java @@ -3,8 +3,10 @@ import edu.harvard.iq.dataverse.ValidateEmail; import edu.harvard.iq.dataverse.ValidateUserName; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; +import edu.harvard.iq.dataverse.passwordreset.PasswordResetData; import static edu.harvard.iq.dataverse.util.StringUtil.nonEmpty; import java.io.Serializable; +import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.GeneratedValue; @@ -13,6 +15,7 @@ import javax.persistence.Index; import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; +import javax.persistence.OneToOne; import javax.persistence.Table; import javax.persistence.Transient; import javax.validation.constraints.Size; @@ -47,6 +50,10 @@ public class BuiltinUser implements Serializable { private String userName; private int passwordEncryptionVersion; + + @OneToOne(mappedBy = "builtinUser", cascade = {CascadeType.REMOVE, CascadeType.MERGE, CascadeType.PERSIST}) + private PasswordResetData passwordResetData; + private String encryptedPassword; /** diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java index 7804716561b..c39c7cb2985 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUserServiceBean.java @@ -79,8 +79,6 @@ public BuiltinUser find(Long pk) { public void removeUser( String userName ) { final BuiltinUser user = findByUserName(userName); if ( user != null ) { - // TODO: Consider adding a cascade delete instead. - passwordResetService.deleteResetDataByDataverseUser(user); em.remove(user); } } From 8afbf5e929de840f3e1683ee07176e18541c54d8 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 3 Mar 2021 16:32:23 -0500 Subject: [PATCH 22/44] remove oauth2tokendata with a cascade #2419 #4475 --- .../dataverse/authorization/AuthenticationServiceBean.java | 1 - .../iq/dataverse/authorization/users/AuthenticatedUser.java | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java index c34015a5dda..5b11dd1b1cb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java @@ -220,7 +220,6 @@ public void deleteAuthenticatedUser(Object pk) { em.remove(confirmEmailData); } userNotificationService.findByUser(user.getId()).forEach(userNotificationService::delete); - em.createNativeQuery("delete from OAuth2TokenData where user_id =" + user.getId()).executeUpdate(); AuthenticationProvider prv = lookupProvider(user); if ( prv != null && prv.isUserDeletionAllowed() ) { diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index ae485103be6..03069536287 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -7,6 +7,7 @@ import edu.harvard.iq.dataverse.authorization.AccessRequest; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserDisplayInfo; import edu.harvard.iq.dataverse.authorization.AuthenticatedUserLookup; +import edu.harvard.iq.dataverse.authorization.providers.oauth2.OAuth2TokenData; import edu.harvard.iq.dataverse.userdata.UserUtil; import edu.harvard.iq.dataverse.authorization.providers.oauth2.impl.OrcidOAuth2AP; import edu.harvard.iq.dataverse.util.BundleUtil; @@ -166,7 +167,10 @@ public List getDatasetLocks() { public void setDatasetLocks(List datasetLocks) { this.datasetLocks = datasetLocks; } - + + @OneToMany(mappedBy = "user", cascade={CascadeType.REMOVE, CascadeType.MERGE, CascadeType.PERSIST}) + private List oAuth2TokenDatas; + @Override public AuthenticatedUserDisplayInfo getDisplayInfo() { return new AuthenticatedUserDisplayInfo(firstName, lastName, email, affiliation, position); From 610e9ca32e4c0d883f3d160625bfac40d7ab34ef Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 4 Mar 2021 11:11:51 -0500 Subject: [PATCH 23/44] merge configureSessionTimeout into setUser #2419 #4475 --- .../iq/dataverse/DataverseSession.java | 23 ++++++++----------- .../edu/harvard/iq/dataverse/LoginPage.java | 1 - .../java/edu/harvard/iq/dataverse/Shib.java | 1 - .../providers/builtin/DataverseUserPage.java | 1 - .../oauth2/OAuth2FirstLoginPage.java | 2 -- .../oauth2/OAuth2LoginBackingBean.java | 1 - .../confirmemail/ConfirmEmailPage.java | 1 - .../passwordreset/PasswordResetPage.java | 1 - .../dataverse/privateurl/PrivateUrlPage.java | 1 - 9 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java index fcf8da22f7e..5093cd99b3f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java @@ -93,6 +93,9 @@ public User getUser() { return user; } + /** + * Sets the user and configures the session timeout. + */ public void setUser(User aUser) { // We check for disabled status here in "setUser" to ensure a common user // experience across Builtin, Shib, OAuth, and OIDC users. @@ -108,6 +111,7 @@ public void setUser(User aUser) { // Log the login/logout and Change the session id if we're using the UI and have // a session, versus an API call with no session - (i.e. /admin/submitToArchive() // which sets the user in the session to pass it through to the underlying command) + // TODO: reformat to remove tabs etc. if(context != null) { logSvc.log( new ActionLogRecord(ActionLogRecord.ActionType.SessionManagement,(aUser==null) ? "logout" : "login") @@ -115,6 +119,12 @@ public void setUser(User aUser) { //#3254 - change session id when user changes SessionUtil.changeSessionId((HttpServletRequest) context.getExternalContext().getRequest()); + HttpSession httpSession = (HttpSession) context.getExternalContext().getSession(false); + if (httpSession != null) { + // Configure session timeout. + logger.fine("jsession: " + httpSession.getId() + " setting the lifespan of the session to " + systemConfig.getLoginSessionTimeout() + " minutes"); + httpSession.setMaxInactiveInterval(systemConfig.getLoginSessionTimeout() * 60); // session timeout, in seconds + } } this.user = aUser; } @@ -219,18 +229,5 @@ public void dismissMessage(BannerMessage message){ } } - - public void configureSessionTimeout() { - if (user instanceof GuestUser) { - return; - } - HttpSession httpSession = (HttpSession) FacesContext.getCurrentInstance().getExternalContext().getSession(false); - - if (httpSession != null) { - logger.fine("jsession: "+httpSession.getId()+" setting the lifespan of the session to " + systemConfig.getLoginSessionTimeout() + " minutes"); - httpSession.setMaxInactiveInterval(systemConfig.getLoginSessionTimeout() * 60); // session timeout, in seconds - } - - } } diff --git a/src/main/java/edu/harvard/iq/dataverse/LoginPage.java b/src/main/java/edu/harvard/iq/dataverse/LoginPage.java index 38376fa84c0..166c0c081d0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/LoginPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/LoginPage.java @@ -171,7 +171,6 @@ public String login() { AuthenticatedUser r = authSvc.getUpdateAuthenticatedUser(credentialsAuthProviderId, authReq); logger.log(Level.FINE, "User authenticated: {0}", r.getEmail()); session.setUser(r); - session.configureSessionTimeout(); if ("dataverse.xhtml".equals(redirectPage)) { redirectPage = redirectToRoot(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/Shib.java b/src/main/java/edu/harvard/iq/dataverse/Shib.java index 4bd38d2377d..cd3b4a30f52 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Shib.java +++ b/src/main/java/edu/harvard/iq/dataverse/Shib.java @@ -360,7 +360,6 @@ private void logInUserAndSetShibAttributes(AuthenticatedUser au) { au.setShibIdentityProvider(shibIdp); // setUser checks for disabled users. session.setUser(au); - session.configureSessionTimeout(); logger.fine("Groups for user " + au.getId() + " (" + au.getIdentifier() + "): " + getGroups(au)); } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java index 204d93b5b8f..663cdf4b4b2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java @@ -327,7 +327,6 @@ public String save() { // Authenticated user registered. Save the new bulitin, and log in. builtinUserService.save(builtinUser); session.setUser(au); - session.configureSessionTimeout(); /** * @todo Move this to * AuthenticationServiceBean.createAuthenticatedUser diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java index a3ce3c5bdf7..bd65e296d52 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java @@ -183,7 +183,6 @@ public String createNewAccount() { newUser.getDisplayInfo().getPosition()); final AuthenticatedUser user = authenticationSvc.createAuthenticatedUser(newUser.getUserRecordIdentifier(), getUsername(), newAud, true); session.setUser(user); - session.configureSessionTimeout(); /** * @todo Move this to AuthenticationServiceBean.createAuthenticatedUser */ @@ -213,7 +212,6 @@ public String convertExistingAccount() { builtinUserSvc.removeUser(existingUser.getUserIdentifier()); session.setUser(existingUser); - session.configureSessionTimeout(); AuthenticationProvider newUserAuthProvider = authenticationSvc.getAuthenticationProvider(newUser.getServiceId()); JsfHelper.addSuccessMessage(BundleUtil.getStringFromBundle("oauth2.convertAccount.success", Arrays.asList(newUserAuthProvider.getInfo().getTitle()))); diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java index c034f7ebc8b..8daa6b2c17c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java @@ -108,7 +108,6 @@ public void exchangeCodeForToken() throws IOException { // login the user and redirect to HOME of intended page (if any). // setUser checks for disabled users. session.setUser(dvUser); - session.configureSessionTimeout(); final OAuth2TokenData tokenData = oauthUser.getTokenData(); if (tokenData != null) { tokenData.setUser(dvUser); diff --git a/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailPage.java b/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailPage.java index 45a04ba4185..823d2c111f2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailPage.java @@ -55,7 +55,6 @@ public String init() { if (confirmEmailData != null) { user = confirmEmailData.getAuthenticatedUser(); session.setUser(user); - session.configureSessionTimeout(); // TODO: is this needed here? (it can't hurt, but still) JsfHelper.addSuccessMessage(BundleUtil.getStringFromBundle("confirmEmail.details.success")); return "/dataverse.xhtml?faces-redirect=true"; } diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java index b5cef3731f5..f27621cef74 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java @@ -148,7 +148,6 @@ public String resetPassword() { String builtinAuthProviderId = BuiltinAuthenticationProvider.PROVIDER_ID; AuthenticatedUser au = authSvc.lookupUser(builtinAuthProviderId, user.getUserName()); session.setUser(au); - session.configureSessionTimeout(); return "/dataverse.xhtml?alias=" + dataverseService.findRootDataverse().getAlias() + "faces-redirect=true"; } else { FacesContext.getCurrentInstance().addMessage(null, new FacesMessage(FacesMessage.SEVERITY_ERROR, response.getMessageSummary(), response.getMessageDetail())); diff --git a/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlPage.java b/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlPage.java index e8bc9fc3da7..b0658f10b34 100644 --- a/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlPage.java @@ -35,7 +35,6 @@ public String init() { String draftDatasetPageToBeRedirectedTo = privateUrlRedirectData.getDraftDatasetPageToBeRedirectedTo() + "&faces-redirect=true"; PrivateUrlUser privateUrlUser = privateUrlRedirectData.getPrivateUrlUser(); session.setUser(privateUrlUser); - session.configureSessionTimeout(); logger.info("Redirecting PrivateUrlUser '" + privateUrlUser.getIdentifier() + "' to " + draftDatasetPageToBeRedirectedTo); return draftDatasetPageToBeRedirectedTo; } catch (Exception ex) { From 28579e2699338ec9e37b3a75356024966a4c2739 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 4 Mar 2021 11:25:08 -0500 Subject: [PATCH 24/44] rename SQL script #2419 #4475 --- ...4__4475-disable-users.sql => V5.3.0.5__4475-disable-users.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/{V5.3.0.4__4475-disable-users.sql => V5.3.0.5__4475-disable-users.sql} (100%) diff --git a/src/main/resources/db/migration/V5.3.0.4__4475-disable-users.sql b/src/main/resources/db/migration/V5.3.0.5__4475-disable-users.sql similarity index 100% rename from src/main/resources/db/migration/V5.3.0.4__4475-disable-users.sql rename to src/main/resources/db/migration/V5.3.0.5__4475-disable-users.sql From ddc4f3acc7302804bad2cec526cc433fb6c2c41c Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 11 Mar 2021 16:28:40 -0500 Subject: [PATCH 25/44] rebrand "disable users" as "deactivate users" #2419 #4475 --- conf/docker-aio/run-test-suite.sh | 2 +- .../2419-4475-7575-disable-users.md | 4 +- .../source/admin/user-administration.rst | 6 +- doc/sphinx-guides/source/api/native-api.rst | 30 +++--- .../iq/dataverse/DataverseSession.java | 8 +- .../java/edu/harvard/iq/dataverse/Shib.java | 2 +- .../iq/dataverse/api/AbstractApiBean.java | 2 +- .../edu/harvard/iq/dataverse/api/Admin.java | 22 ++--- .../dataverse/api/datadeposit/SwordAuth.java | 2 +- .../AuthenticationServiceBean.java | 6 +- .../oauth2/OAuth2LoginBackingBean.java | 2 +- .../users/AuthenticatedUser.java | 22 ++--- .../dataverse/authorization/users/User.java | 2 +- .../confirmemail/ConfirmEmailServiceBean.java | 4 +- ...ddRoleAssigneesToExplicitGroupCommand.java | 4 +- .../command/impl/AssignRoleCommand.java | 4 +- ...ommand.java => DeactivateUserCommand.java} | 28 +++--- .../command/impl/MergeInAccountCommand.java | 4 +- .../PasswordResetServiceBean.java | 4 +- .../iq/dataverse/util/json/JsonPrinter.java | 4 +- src/main/java/propertyFiles/Bundle.properties | 8 +- .../V5.3.0.5__2419-deactivate-users.sql | 4 + .../V5.3.0.5__4475-disable-users.sql | 4 - ...bleUsersIT.java => DeactivateUsersIT.java} | 92 +++++++++---------- .../iq/dataverse/api/DeleteUsersIT.java | 8 +- .../edu/harvard/iq/dataverse/api/UtilIT.java | 16 ++-- 26 files changed, 147 insertions(+), 147 deletions(-) rename src/main/java/edu/harvard/iq/dataverse/engine/command/impl/{DisableUserCommand.java => DeactivateUserCommand.java} (52%) create mode 100644 src/main/resources/db/migration/V5.3.0.5__2419-deactivate-users.sql delete mode 100644 src/main/resources/db/migration/V5.3.0.5__4475-disable-users.sql rename src/test/java/edu/harvard/iq/dataverse/api/{DisableUsersIT.java => DeactivateUsersIT.java} (76%) diff --git a/conf/docker-aio/run-test-suite.sh b/conf/docker-aio/run-test-suite.sh index da9e56e0176..47a4c3b9576 100755 --- a/conf/docker-aio/run-test-suite.sh +++ b/conf/docker-aio/run-test-suite.sh @@ -8,4 +8,4 @@ fi # Please note the "dataverse.test.baseurl" is set to run for "all-in-one" Docker environment. # TODO: Rather than hard-coding the list of "IT" classes here, add a profile to pom.xml. -source maven/maven.sh && mvn test -Dtest=DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DisableUsersIT -Ddataverse.test.baseurl=$dvurl +source maven/maven.sh && mvn test -Dtest=DataversesIT,DatasetsIT,SwordIT,AdminIT,BuiltinUsersIT,UsersIT,UtilIT,ConfirmEmailIT,FileMetadataIT,FilesIT,SearchIT,InReviewWorkflowIT,HarvestingServerIT,MoveIT,MakeDataCountApiIT,FileTypeDetectionIT,EditDDIIT,ExternalToolsIT,AccessIT,DuplicateFilesIT,DownloadFilesIT,LinkIT,DeleteUsersIT,DeactivateUsersIT -Ddataverse.test.baseurl=$dvurl diff --git a/doc/release-notes/2419-4475-7575-disable-users.md b/doc/release-notes/2419-4475-7575-disable-users.md index 612200905dc..2678ebc6ec9 100644 --- a/doc/release-notes/2419-4475-7575-disable-users.md +++ b/doc/release-notes/2419-4475-7575-disable-users.md @@ -1,5 +1,5 @@ ## Release Highlights -### Disable Users API, Get User Traces API, Revoke Roles API +### Deactivate Users API, Get User Traces API, Revoke Roles API -A new API has been added to disable users to prevent them from logging in or otherwise being active in the system. Disabling a user is an alternative to deleting a user, especially when the latter is not possible due to the amount of interaction the user has had with the system. In order to learn more about a user before deleting, disabling, or merging, a new "get user traces" API is available that will show objects created, roles, group memberships, and more. Finally, the "remove all roles" button available in the superuser dashboard is now also available via API. +A new API has been added to deactivate users to prevent them from logging in or otherwise being active in the system. Deactivating a user is an alternative to deleting a user, especially when the latter is not possible due to the amount of interaction the user has had with the system. In order to learn more about a user before deleting, deactivating, or merging, a new "get user traces" API is available that will show objects created, roles, group memberships, and more. Finally, the "remove all roles" button available in the superuser dashboard is now also available via API. diff --git a/doc/sphinx-guides/source/admin/user-administration.rst b/doc/sphinx-guides/source/admin/user-administration.rst index 2999c6e0856..867f06bde8e 100644 --- a/doc/sphinx-guides/source/admin/user-administration.rst +++ b/doc/sphinx-guides/source/admin/user-administration.rst @@ -49,10 +49,10 @@ Delete a User See :ref:`delete-a-user` -Disable a User --------------- +Deactivate a User +----------------- -See :ref:`disable-a-user` +See :ref:`deactivate-a-user` Confirm Email ------------- diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index c99b903ff38..ca11e77a78a 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3044,7 +3044,7 @@ Example: ``curl -H "X-Dataverse-key: $API_TOKEN" -X POST http://demo.dataverse.o This action moves account data from jsmith2 into the account jsmith and deletes the account of jsmith2. -Note: User accounts can only be merged if they are either both enabled or both disabled. See :ref:`disable a user`. +Note: User accounts can only be merged if they are either both non-deactivated or both deactivated. See :ref:`deactivate a user`. .. _change-identifier-label: @@ -3079,14 +3079,14 @@ Deletes an ``AuthenticatedUser`` whose ``id`` is passed. :: DELETE http://$SERVER/api/admin/authenticatedUsers/id/$id -Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. To see where in the database these actions are stored you can use the :ref:`show-user-traces-api` API. If a user cannot be deleted for this reason, you can choose to :ref:`disable a user`. +Note: If the user has performed certain actions such as creating or contributing to a Dataset or downloading a file they cannot be deleted. To see where in the database these actions are stored you can use the :ref:`show-user-traces-api` API. If a user cannot be deleted for this reason, you can choose to :ref:`deactivate a user`. -.. _disable-a-user: +.. _deactivate-a-user: -Disable a User -~~~~~~~~~~~~~~ +Deactivate a User +~~~~~~~~~~~~~~~~~ -Disables a user. A superuser API token is not required but the command will operate using the first superuser it finds. +Deactivates a user. A superuser API token is not required but the command will operate using the first superuser it finds. .. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of export below. @@ -3096,13 +3096,13 @@ Disables a user. A superuser API token is not required but the command will oper export SERVER_URL=http://localhost:8080 export USERNAME=jdoe - curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/$USERNAME/disable + curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/$USERNAME/deactivate The fully expanded example above (without environment variables) looks like this: .. code-block:: bash - curl -H X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -X POST http://localhost:8080/api/admin/authenticatedUsers/jdoe/disable + curl -H X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -X POST http://localhost:8080/api/admin/authenticatedUsers/jdoe/deactivate The database ID of the user can be passed instead of the username. @@ -3112,23 +3112,23 @@ The database ID of the user can be passed instead of the username. export SERVER_URL=http://localhost:8080 export USERID=42 - curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/id/$USERID/disable + curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/id/$USERID/deactivate -Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Disable User endpoint for users who have taken certain actions in the system alongside a Delete User endpoint to remove users that haven't taken certain actions in the system is by design. +Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Deactivate User endpoint for users who have taken certain actions in the system alongside a Delete User endpoint to remove users that haven't taken certain actions in the system is by design. -This is an irreversible action. There is no option to undisable a user. +This is an irreversible action. There is no option to undeactivate a user. -Disabling a user with this endpoint will: +Deactivating a user with this endpoint will: -- Disable the user's ability to log in to the Dataverse installation. A message will be shown, stating that the account has been disabled. The user will not able to create a new account with the same email address, ORCID, Shibboleth, or other login type. -- Disable the user's ability to use the API +- Deactivate the user's ability to log in to the Dataverse installation. A message will be shown, stating that the account has been deactivated. The user will not able to create a new account with the same email address, ORCID, Shibboleth, or other login type. +- Deactivate the user's ability to use the API - Remove the user's access from all Dataverse collections, datasets and files - Prevent a user from being assigned any roles - Cancel any pending file access requests generated by the user - Remove the user from all groups - No longer have notifications generated or sent by the Dataverse installation -Disabling a user with this endpoint will keep: +Deactivating a user with this endpoint will keep: - The user's contributions to datasets, including dataset creation, file uploads, and publishing. - The user's access history to datafiles in the Dataverse installation, including guestbook records. diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java index 5093cd99b3f..315b7c89da7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java @@ -97,14 +97,14 @@ public User getUser() { * Sets the user and configures the session timeout. */ public void setUser(User aUser) { - // We check for disabled status here in "setUser" to ensure a common user + // We check for deactivated status here in "setUser" to ensure a common user // experience across Builtin, Shib, OAuth, and OIDC users. // If we want a different user experience for Builtin users, we can // modify getUpdateAuthenticatedUser in AuthenticationServiceBean // (and probably other places). - if (aUser instanceof AuthenticatedUser && aUser.isDisabled()) { - logger.info("Login attempt by disabled user " + aUser.getIdentifier() + "."); - JsfHelper.addErrorMessage(BundleUtil.getStringFromBundle("disabled.error")); + if (aUser instanceof AuthenticatedUser && aUser.isDeactivated()) { + logger.info("Login attempt by deactivated user " + aUser.getIdentifier() + "."); + JsfHelper.addErrorMessage(BundleUtil.getStringFromBundle("deactivated.error")); return; } FacesContext context = FacesContext.getCurrentInstance(); diff --git a/src/main/java/edu/harvard/iq/dataverse/Shib.java b/src/main/java/edu/harvard/iq/dataverse/Shib.java index cd3b4a30f52..6dd48cc782a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Shib.java +++ b/src/main/java/edu/harvard/iq/dataverse/Shib.java @@ -358,7 +358,7 @@ public String confirmAndConvertAccount() { private void logInUserAndSetShibAttributes(AuthenticatedUser au) { au.setShibIdentityProvider(shibIdp); - // setUser checks for disabled users. + // setUser checks for deactivated users. session.setUser(au); logger.fine("Groups for user " + au.getId() + " (" + au.getIdentifier() + "): " + getGroups(au)); } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index e3e450a8ade..d79879be9d9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -382,7 +382,7 @@ protected AuthenticatedUser findAuthenticatedUserOrDie() throws WrappedResponse private AuthenticatedUser findAuthenticatedUserOrDie( String key, String wfid ) throws WrappedResponse { if (key != null) { - // No check for disabled user because it's done in authSvc.lookupUser. + // No check for deactivated user because it's done in authSvc.lookupUser. AuthenticatedUser authUser = authSvc.lookupUser(key); if (authUser != null) { 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 6e8ed301a68..96add147743 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -87,7 +87,7 @@ import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.impl.MergeInAccountCommand; import edu.harvard.iq.dataverse.engine.command.impl.ChangeUserIdentifierCommand; -import edu.harvard.iq.dataverse.engine.command.impl.DisableUserCommand; +import edu.harvard.iq.dataverse.engine.command.impl.DeactivateUserCommand; import edu.harvard.iq.dataverse.engine.command.impl.RegisterDvObjectCommand; import edu.harvard.iq.dataverse.ingest.IngestServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; @@ -384,33 +384,33 @@ private Response deleteAuthenticatedUser(AuthenticatedUser au) { } @POST - @Path("authenticatedUsers/{identifier}/disable") - public Response disableAuthenticatedUser(@PathParam("identifier") String identifier) { + @Path("authenticatedUsers/{identifier}/deactivate") + public Response deactivateAuthenticatedUser(@PathParam("identifier") String identifier) { AuthenticatedUser user = authSvc.getAuthenticatedUser(identifier); if (user != null) { - return disableAuthenticatedUser(user); + return deactivateAuthenticatedUser(user); } return error(Response.Status.BAD_REQUEST, "User " + identifier + " not found."); } @POST - @Path("authenticatedUsers/id/{id}/disable") - public Response disableAuthenticatedUserById(@PathParam("id") Long id) { + @Path("authenticatedUsers/id/{id}/deactivate") + public Response deactivateAuthenticatedUserById(@PathParam("id") Long id) { AuthenticatedUser user = authSvc.findByID(id); if (user != null) { - return disableAuthenticatedUser(user); + return deactivateAuthenticatedUser(user); } return error(Response.Status.BAD_REQUEST, "User " + id + " not found."); } - private Response disableAuthenticatedUser(AuthenticatedUser userToDisable) { + private Response deactivateAuthenticatedUser(AuthenticatedUser userToDisable) { AuthenticatedUser superuser = authSvc.getAdminUser(); if (superuser == null) { - return error(Response.Status.INTERNAL_SERVER_ERROR, "Cannot find superuser to execute DisableUserCommand."); + return error(Response.Status.INTERNAL_SERVER_ERROR, "Cannot find superuser to execute DeactivateUserCommand."); } try { - execCommand(new DisableUserCommand(createDataverseRequest(superuser), userToDisable)); - return ok("User " + userToDisable.getIdentifier() + " disabled."); + execCommand(new DeactivateUserCommand(createDataverseRequest(superuser), userToDisable)); + return ok("User " + userToDisable.getIdentifier() + " deactivated."); } catch (WrappedResponse ex) { return ex.getResponse(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java b/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java index 339832989d6..13fc37bdc40 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/datadeposit/SwordAuth.java @@ -36,7 +36,7 @@ public AuthenticatedUser auth(AuthCredentials authCredentials) throws SwordAuthE throw new SwordAuthException(msg); } - // Checking if the user is disabled is done inside findUserByApiToken. + // Checking if the user is deactivated is done inside findUserByApiToken. AuthenticatedUser authenticatedUserFromToken = findUserByApiToken(username); if (authenticatedUserFromToken == null) { String msg = "User not found based on API token."; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java index f3613e499f7..8a3d51bf1a7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java @@ -309,7 +309,7 @@ public AuthenticatedUser getUpdateAuthenticatedUser( String authenticationProvid // yay! see if we already have this user. AuthenticatedUser user = lookupUser(authenticationProviderId, resp.getUserId()); - if (user != null && !user.isDisabled()) { + if (user != null && !user.isDeactivated()) { user = userService.updateLastLogin(user); } @@ -454,10 +454,10 @@ public AuthenticatedUser lookupUser( String apiToken ) { } AuthenticatedUser user = tkn.getAuthenticatedUser(); - if (!user.isDisabled()) { + if (!user.isDeactivated()) { return user; } else { - logger.info("attempted access with token from disabled user: " + apiToken); + logger.info("attempted access with token from deactivated user: " + apiToken); return null; } } diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java index 8daa6b2c17c..225352dec43 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2LoginBackingBean.java @@ -106,7 +106,7 @@ public void exchangeCodeForToken() throws IOException { } else { // login the user and redirect to HOME of intended page (if any). - // setUser checks for disabled users. + // setUser checks for deactivated users. session.setUser(dvUser); final OAuth2TokenData tokenData = oauthUser.getTokenData(); if (tokenData != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index 03069536287..abe0fef597f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -57,7 +57,7 @@ query="select au from AuthenticatedUser au WHERE (" + "LOWER(au.userIdentifier) like LOWER(:query) OR " + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query)) " - + "AND au.disabled != true"), + + "AND au.deactivated != true"), @NamedQuery( name="AuthenticatedUser.findAdminUser", query="select au from AuthenticatedUser au WHERE " + "au.superuser = true " @@ -117,10 +117,10 @@ public class AuthenticatedUser implements User, Serializable { private boolean superuser; @Column(nullable=true) - private boolean disabled; + private boolean deactivated; @Column(nullable=true) - private Timestamp disabledTime; + private Timestamp deactivatedTime; /** * @todo Consider storing a hash of *all* potentially interesting Shibboleth @@ -315,20 +315,20 @@ public void setSuperuser(boolean superuser) { } @Override - public boolean isDisabled() { - return disabled; + public boolean isDeactivated() { + return deactivated; } - public void setDisabled(boolean disabled) { - this.disabled = disabled; + public void setDeactivated(boolean deactivated) { + this.deactivated = deactivated; } - public Timestamp getDisabledTime() { - return disabledTime; + public Timestamp getDeactivatedTime() { + return deactivatedTime; } - public void setDisabledTime(Timestamp disabledTime) { - this.disabledTime = disabledTime; + public void setDeactivatedTime(Timestamp deactivatedTime) { + this.deactivatedTime = deactivatedTime; } @OneToOne(mappedBy = "authenticatedUser") diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java index baaf0e5f627..4655c9c9f0d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/User.java @@ -14,7 +14,7 @@ public interface User extends RoleAssignee, Serializable { public boolean isSuperuser(); - default boolean isDisabled() { + default boolean isDeactivated() { return false; } diff --git a/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java index 574cfa11cde..244dd476909 100644 --- a/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/confirmemail/ConfirmEmailServiceBean.java @@ -169,8 +169,8 @@ public ConfirmEmailExecResponse processToken(String tokenQueried) { long nowInMilliseconds = new Date().getTime(); Timestamp emailConfirmed = new Timestamp(nowInMilliseconds); AuthenticatedUser authenticatedUser = confirmEmailData.getAuthenticatedUser(); - if (authenticatedUser.isDisabled()) { - logger.fine("User is disabled."); + if (authenticatedUser.isDeactivated()) { + logger.fine("User is deactivated."); return null; } authenticatedUser.setEmailConfirmed(emailConfirmed); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java index d4e47a5d449..8ba1d181609 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AddRoleAssigneesToExplicitGroupCommand.java @@ -52,8 +52,8 @@ public ExplicitGroup execute(CommandContext ctxt) throws CommandException { } else { if (ra instanceof AuthenticatedUser) { AuthenticatedUser user = (AuthenticatedUser) ra; - if (user.isDisabled()) { - throw new IllegalCommandException("User " + user.getUserIdentifier() + " is disabled and cannot be added to a group.", this); + if (user.isDeactivated()) { + throw new IllegalCommandException("User " + user.getUserIdentifier() + " is deactivated and cannot be added to a group.", this); } } try { diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java index 274d485c5d2..276f52a5802 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/AssignRoleCommand.java @@ -53,8 +53,8 @@ public AssignRoleCommand(RoleAssignee anAssignee, DataverseRole aRole, DvObject public RoleAssignment execute(CommandContext ctxt) throws CommandException { if (grantee instanceof AuthenticatedUser) { AuthenticatedUser user = (AuthenticatedUser) grantee; - if (user.isDisabled()) { - throw new IllegalCommandException("User " + user.getUserIdentifier() + " is disabled and cannot be given a role.", this); + if (user.isDeactivated()) { + throw new IllegalCommandException("User " + user.getUserIdentifier() + " is deactivated and cannot be given a role.", this); } } // TODO make sure the role is defined on the dataverse. diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeactivateUserCommand.java similarity index 52% rename from src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java rename to src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeactivateUserCommand.java index 58ee7295e22..1dab8120767 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DisableUserCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeactivateUserCommand.java @@ -13,32 +13,32 @@ // Superuser-only enforced below. @RequiredPermissions({}) -public class DisableUserCommand extends AbstractCommand { +public class DeactivateUserCommand extends AbstractCommand { private DataverseRequest request; - private AuthenticatedUser userToDisable; + private AuthenticatedUser userToDeactivate; - public DisableUserCommand(DataverseRequest request, AuthenticatedUser userToDisable) { + public DeactivateUserCommand(DataverseRequest request, AuthenticatedUser userToDeactivate) { super(request, (DvObject) null); this.request = request; - this.userToDisable = userToDisable; + this.userToDeactivate = userToDeactivate; } @Override public AuthenticatedUser execute(CommandContext ctxt) throws CommandException { if (!(getUser() instanceof AuthenticatedUser) || !getUser().isSuperuser()) { - throw new PermissionException("Disable user command can only be called by superusers.", this, null, null); + throw new PermissionException("Deactivate user command can only be called by superusers.", this, null, null); } - if (userToDisable == null) { - throw new CommandException("Cannot disable user. User not found.", this); + if (userToDeactivate == null) { + throw new CommandException("Cannot deactivate user. User not found.", this); } - ctxt.engine().submit(new RevokeAllRolesCommand(userToDisable, request)); - ctxt.authentication().removeAuthentictedUserItems(userToDisable); - ctxt.notifications().findByUser(userToDisable.getId()).forEach(ctxt.notifications()::delete); - userToDisable.setDisabled(true); - userToDisable.setDisabledTime(new Timestamp(new Date().getTime())); - AuthenticatedUser disabledUser = ctxt.authentication().save(userToDisable); - return disabledUser; + ctxt.engine().submit(new RevokeAllRolesCommand(userToDeactivate, request)); + ctxt.authentication().removeAuthentictedUserItems(userToDeactivate); + ctxt.notifications().findByUser(userToDeactivate.getId()).forEach(ctxt.notifications()::delete); + userToDeactivate.setDeactivated(true); + userToDeactivate.setDeactivatedTime(new Timestamp(new Date().getTime())); + AuthenticatedUser deactivatedUser = ctxt.authentication().save(userToDeactivate); + return deactivatedUser; } } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java index c0e3b3ea89a..c0dce9799e1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java @@ -59,8 +59,8 @@ public MergeInAccountCommand(DataverseRequest createDataverseRequest, Authentica @Override protected void executeImpl(CommandContext ctxt) throws CommandException { - if (consumedAU.isDisabled() && !ongoingAU.isDisabled() || !consumedAU.isDisabled() && ongoingAU.isDisabled()) { - throw new IllegalCommandException("User accounts can only be merged if they are either both enabled or both disabled.", this); + if (consumedAU.isDeactivated() && !ongoingAU.isDeactivated() || !consumedAU.isDeactivated() && ongoingAU.isDeactivated()) { + throw new IllegalCommandException("User accounts can only be merged if they are either both non-deactivated or both deactivated.", this); } List baseRAList = ctxt.roleAssignees().getAssignmentsFor(ongoingAU.getIdentifier()); diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java index 9b28be6527c..4e92d861a2f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java @@ -65,8 +65,8 @@ public PasswordResetInitResponse requestReset(String emailAddress) throws Passwo logger.info("Cannot find a user based on " + emailAddress + ". Cannot reset password."); return new PasswordResetInitResponse(false); } - if (authUser.isDisabled()) { - logger.info("Cannot reset password for " + emailAddress + " because account is disabled."); + if (authUser.isDeactivated()) { + logger.info("Cannot reset password for " + emailAddress + " because account is deactivated."); return new PasswordResetInitResponse(false); } BuiltinUser user = dataverseUserService.findByUserName(authUser.getUserIdentifier()); diff --git a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java index 668f3cbf771..172a63db983 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/json/JsonPrinter.java @@ -107,8 +107,8 @@ public static JsonObjectBuilder json(AuthenticatedUser authenticatedUser) { .add("lastName", authenticatedUser.getLastName()) .add("email", authenticatedUser.getEmail()) .add("superuser", authenticatedUser.isSuperuser()) - .add("disabled", authenticatedUser.isDisabled()) - .add("disabledTime", authenticatedUser.getDisabledTime()) + .add("deactivated", authenticatedUser.isDeactivated()) + .add("deactivatedTime", authenticatedUser.getDeactivatedTime()) .add("affiliation", authenticatedUser.getAffiliation()) .add("position", authenticatedUser.getPosition()) .add("persistentUserId", authenticatedUser.getAuthenticatedUserLookup().getPersistentUserId()) diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index a76a7111479..8b5e2c0b49e 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -364,7 +364,7 @@ shib.dataverseUsername=Dataverse Username shib.currentDataversePassword=Current Dataverse Password shib.accountInformation=Account Information shib.offerToCreateNewAccount=This information is provided by your institution and will be used to create your Dataverse account. -shib.passwordRejected=Validation Error - Your account can only be converted if you provide the correct password for your existing account. If your existing account has been disabled by an administrator, you cannot convert your account. +shib.passwordRejected=Validation Error - Your account can only be converted if you provide the correct password for your existing account. If your existing account has been deactivated by an administrator, you cannot convert your account. # oauth2/firstLogin.xhtml oauth2.btn.convertAccount=Convert Existing Account @@ -399,7 +399,7 @@ oauth2.newAccount.emailInvalid=Invalid email address. oauth2.convertAccount.explanation=Please enter your {0} account username or email and password to convert your account to the {1} log in option. Learn more about converting your account. oauth2.convertAccount.username=Existing username oauth2.convertAccount.password=Password -oauth2.convertAccount.authenticationFailed=Your account can only be converted if you provide the correct username and password for your existing account. If your existing account has been disabled by an administrator, you cannot convert your account. +oauth2.convertAccount.authenticationFailed=Your account can only be converted if you provide the correct username and password for your existing account. If your existing account has been deactivated by an administrator, you cannot convert your account. oauth2.convertAccount.buttonTitle=Convert Account oauth2.convertAccount.success=Your Dataverse account is now associated with your {0} account. @@ -407,8 +407,8 @@ oauth2.convertAccount.success=Your Dataverse account is now associated with your oauth2.callback.page.title=OAuth Callback oauth2.callback.message=Authentication Error - Dataverse could not authenticate your login with the provider that you selected. Please make sure you authorize your account to connect with Dataverse. For more details about the information being requested, see the User Guide. -# disabled user accounts -disabled.error=Sorry, your account has been disabled. +# deactivated user accounts +deactivated.error=Sorry, your account has been deactivated. # tab on dataverseuser.xhtml apitoken.title=API Token diff --git a/src/main/resources/db/migration/V5.3.0.5__2419-deactivate-users.sql b/src/main/resources/db/migration/V5.3.0.5__2419-deactivate-users.sql new file mode 100644 index 00000000000..c87fe019a52 --- /dev/null +++ b/src/main/resources/db/migration/V5.3.0.5__2419-deactivate-users.sql @@ -0,0 +1,4 @@ +-- Users can be deactivated. +ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS deactivated BOOLEAN; +-- A timestamp of when the user was deactivated. +ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS deactivatedtime timestamp without time zone; diff --git a/src/main/resources/db/migration/V5.3.0.5__4475-disable-users.sql b/src/main/resources/db/migration/V5.3.0.5__4475-disable-users.sql deleted file mode 100644 index 48823a72247..00000000000 --- a/src/main/resources/db/migration/V5.3.0.5__4475-disable-users.sql +++ /dev/null @@ -1,4 +0,0 @@ --- Users can be disabled. -ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS disabled BOOLEAN; --- A timestamp of when the user was disabled. -ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS disabledtime timestamp without time zone; diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java similarity index 76% rename from src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java rename to src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java index d9c23b100e0..ed979ff785c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DisableUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java @@ -16,7 +16,7 @@ import org.junit.BeforeClass; import org.junit.Test; -public class DisableUsersIT { +public class DeactivateUsersIT { @BeforeClass public static void setUp() { @@ -24,7 +24,7 @@ public static void setUp() { } @Test - public void testDisableUser() { + public void testDeactivateUser() { Response createSuperuser = UtilIT.createRandomUser(); createSuperuser.then().assertThat().statusCode(OK.getStatusCode()); @@ -51,9 +51,9 @@ public void testDisableUser() { String username = UtilIT.getUsernameFromResponse(createUser); String apiToken = UtilIT.getApiTokenFromResponse(createUser); - Response grantRoleBeforeDisable = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); - grantRoleBeforeDisable.prettyPrint(); - grantRoleBeforeDisable.then().assertThat() + Response grantRoleBeforeDeactivate = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); + grantRoleBeforeDeactivate.prettyPrint(); + grantRoleBeforeDeactivate.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.assignee", equalTo("@" + username)) .body("data._roleAlias", equalTo("admin")); @@ -75,31 +75,31 @@ public void testDisableUser() { addToGroup.then().assertThat() .statusCode(OK.getStatusCode()); - Response userTracesBeforeDisable = UtilIT.getUserTraces(username, superuserApiToken); - userTracesBeforeDisable.prettyPrint(); - userTracesBeforeDisable.then().assertThat() + Response userTracesBeforeDeactivate = UtilIT.getUserTraces(username, superuserApiToken); + userTracesBeforeDeactivate.prettyPrint(); + userTracesBeforeDeactivate.then().assertThat() .statusCode(OK.getStatusCode()) .body("data.traces.roleAssignments.items[0].definitionPointName", equalTo(dataverseAlias)) .body("data.traces.roleAssignments.items[0].definitionPointId", equalTo(dataverseId)) .body("data.traces.explicitGroups.items[0].name", equalTo("Group for " + dataverseAlias)); - Response disableUser = UtilIT.disableUser(username); - disableUser.prettyPrint(); - disableUser.then().assertThat().statusCode(OK.getStatusCode()); + Response deactivateUser = UtilIT.deactivateUser(username); + deactivateUser.prettyPrint(); + deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); Response getUser = UtilIT.getAuthenticatedUser(username, superuserApiToken); getUser.prettyPrint(); getUser.then().assertThat() .statusCode(OK.getStatusCode()) - .body("data.disabled", equalTo(true)); + .body("data.deactivated", equalTo(true)); - Response getUserDisabled = UtilIT.getAuthenticatedUserByToken(apiToken); - getUserDisabled.prettyPrint(); - getUserDisabled.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); + Response getUserDeactivated = UtilIT.getAuthenticatedUserByToken(apiToken); + getUserDeactivated.prettyPrint(); + getUserDeactivated.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); - Response userTracesAfterDisable = UtilIT.getUserTraces(username, superuserApiToken); - userTracesAfterDisable.prettyPrint(); - userTracesAfterDisable.then().assertThat() + Response userTracesAfterDeactivate = UtilIT.getUserTraces(username, superuserApiToken); + userTracesAfterDeactivate.prettyPrint(); + userTracesAfterDeactivate.then().assertThat() .statusCode(OK.getStatusCode()) /** * Here we are showing the the following were deleted: @@ -110,28 +110,28 @@ public void testDisableUser() { */ .body("data.traces", equalTo(Collections.EMPTY_MAP)); - Response grantRoleAfterDisable = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); - grantRoleAfterDisable.prettyPrint(); - grantRoleAfterDisable.then().assertThat() + Response grantRoleAfterDeactivate = UtilIT.grantRoleOnDataverse(dataverseAlias, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); + grantRoleAfterDeactivate.prettyPrint(); + grantRoleAfterDeactivate.then().assertThat() .statusCode(FORBIDDEN.getStatusCode()) - .body("message", equalTo("User " + username + " is disabled and cannot be given a role.")); + .body("message", equalTo("User " + username + " is deactivated and cannot be given a role.")); Response addToGroupAfter = UtilIT.addToGroup(dataverseAlias, aliasInOwner, roleAssigneesToAdd, superuserApiToken); addToGroupAfter.prettyPrint(); addToGroupAfter.then().assertThat() .statusCode(FORBIDDEN.getStatusCode()) - .body("message", equalTo("User " + username + " is disabled and cannot be added to a group.")); + .body("message", equalTo("User " + username + " is deactivated and cannot be added to a group.")); Response grantRoleOnDataset = UtilIT.grantRoleOnDataset(datasetPersistentId, DataverseRole.ADMIN.toString(), "@" + username, superuserApiToken); grantRoleOnDataset.prettyPrint(); grantRoleOnDataset.then().assertThat() .statusCode(FORBIDDEN.getStatusCode()) - .body("message", equalTo("User " + username + " is disabled and cannot be given a role.")); + .body("message", equalTo("User " + username + " is deactivated and cannot be given a role.")); } @Test - public void testDisableUserById() { + public void testDeactivateUserById() { Response createSuperuser = UtilIT.createRandomUser(); createSuperuser.then().assertThat().statusCode(OK.getStatusCode()); @@ -148,13 +148,13 @@ public void testDisableUserById() { Long userId = JsonPath.from(createUser.body().asString()).getLong("data.user.id"); String apiToken = UtilIT.getApiTokenFromResponse(createUser); - Response disableUser = UtilIT.disableUser(userId); - disableUser.prettyPrint(); - disableUser.then().assertThat().statusCode(OK.getStatusCode()); + Response deactivateUser = UtilIT.deactivateUser(userId); + deactivateUser.prettyPrint(); + deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); } @Test - public void testMergeDisabledIntoEnabledUser() { + public void testMergeDeactivatedIntoNonDeactivatedUser() { Response createSuperuser = UtilIT.createRandomUser(); String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); @@ -171,18 +171,18 @@ public void testMergeDisabledIntoEnabledUser() { createUserToMerge.prettyPrint(); String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); - Response disableUser = UtilIT.disableUser(usernameToMerge); - disableUser.prettyPrint(); - disableUser.then().assertThat().statusCode(OK.getStatusCode()); + Response deactivateUser = UtilIT.deactivateUser(usernameToMerge); + deactivateUser.prettyPrint(); + deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); - // User accounts can only be merged if they are either both enabled or both disabled. + // User accounts can only be merged if they are either both non-deactivated or both deactivated. Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); } @Test - public void testMergeEnabledIntoDisabledUser() { + public void testMergeNonDeactivatedIntoDeactivatedUser() { Response createSuperuser = UtilIT.createRandomUser(); String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); @@ -199,18 +199,18 @@ public void testMergeEnabledIntoDisabledUser() { createUserToMerge.prettyPrint(); String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); - Response disableUser = UtilIT.disableUser(usernameMergeTarget); - disableUser.prettyPrint(); - disableUser.then().assertThat().statusCode(OK.getStatusCode()); + Response deactivateUser = UtilIT.deactivateUser(usernameMergeTarget); + deactivateUser.prettyPrint(); + deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); - // User accounts can only be merged if they are either both enabled or both disabled. + // User accounts can only be merged if they are either both non-deactivated or both deactivated. Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); } @Test - public void testMergeDisabledIntoDisabledUser() { + public void testMergeDeactivatedIntoDeactivatedUser() { Response createSuperuser = UtilIT.createRandomUser(); String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); @@ -227,15 +227,15 @@ public void testMergeDisabledIntoDisabledUser() { createUserToMerge.prettyPrint(); String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); - Response disableUserMergeTarget = UtilIT.disableUser(usernameMergeTarget); - disableUserMergeTarget.prettyPrint(); - disableUserMergeTarget.then().assertThat().statusCode(OK.getStatusCode()); + Response deactivatedUserMergeTarget = UtilIT.deactivateUser(usernameMergeTarget); + deactivatedUserMergeTarget.prettyPrint(); + deactivatedUserMergeTarget.then().assertThat().statusCode(OK.getStatusCode()); - Response disableUserToMerge = UtilIT.disableUser(usernameToMerge); - disableUserToMerge.prettyPrint(); - disableUserToMerge.then().assertThat().statusCode(OK.getStatusCode()); + Response deactivatedUserToMerge = UtilIT.deactivateUser(usernameToMerge); + deactivatedUserToMerge.prettyPrint(); + deactivatedUserToMerge.then().assertThat().statusCode(OK.getStatusCode()); - // User accounts can only be merged if they are either both enabled or both disabled. + // User accounts can only be merged if they are either both non-deactivated or both deactivated. Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java index cbfe761234d..a7f5153d69f 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java @@ -53,8 +53,8 @@ * * - guestbookresponse: Definitely a concern but it's possible to null out the * user id. You can't delete a user but you can merge instead. There is talk of - * disable which would probably null out the id. In all cases the name and email - * address in the rows are left alone. + * deactivate which would probably null out the id. In all cases the name and + * email address in the rows are left alone. * * - oauth2tokendata: Not a concern. Rows are deleted. * @@ -668,10 +668,10 @@ public void testCuratorSendsCommentsToAuthor() throws InterruptedException { + " because the user has contributed to dataset version(s).")); // What should we do with curator2 instead of deleting? The only option is to merge - // curator2 into some other account. Once implemented, we'll disable curator2's account + // curator2 into some other account. Once implemented, we'll deactivate curator2's account // so that curator2 continues to be displayed as a contributor. // - // TODO: disable curator2 here + // TODO: deactivate curator2 here // Response removeRolesFromAuthor2 = UtilIT.deleteUserRoles(author2Username, superuserApiToken); removeRolesFromAuthor2.prettyPrint(); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index cc4b3e2029e..63911d09e80 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -977,16 +977,16 @@ static public Response grantRoleOnDataverse(String definitionPoint, String role, .post("api/dataverses/" + definitionPoint + "/assignments?key=" + apiToken); } - public static Response disableUser(String username) { - Response disableUserResponse = given() - .post("/api/admin/authenticatedUsers/" + username + "/disable"); - return disableUserResponse; + public static Response deactivateUser(String username) { + Response deactivateUserResponse = given() + .post("/api/admin/authenticatedUsers/" + username + "/deactivate"); + return deactivateUserResponse; } - public static Response disableUser(Long userId) { - Response disableUserResponse = given() - .post("/api/admin/authenticatedUsers/id/" + userId + "/disable"); - return disableUserResponse; + public static Response deactivateUser(Long userId) { + Response deactivateUserResponse = given() + .post("/api/admin/authenticatedUsers/id/" + userId + "/deactivate"); + return deactivateUserResponse; } public static Response deleteUser(String username) { From abb8be01e7a92ef4bef66780e4ae677cd67e49f9 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Mar 2021 10:53:35 -0500 Subject: [PATCH 26/44] remove API token from deactivate user examples (not needed) #2419 #4475 --- doc/sphinx-guides/source/api/native-api.rst | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index ca11e77a78a..b59f67b32b7 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3092,27 +3092,25 @@ Deactivates a user. A superuser API token is not required but the command will o .. code-block:: bash - export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx export SERVER_URL=http://localhost:8080 export USERNAME=jdoe - curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/$USERNAME/deactivate + curl -X POST $SERVER_URL/api/admin/authenticatedUsers/$USERNAME/deactivate The fully expanded example above (without environment variables) looks like this: .. code-block:: bash - curl -H X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -X POST http://localhost:8080/api/admin/authenticatedUsers/jdoe/deactivate + curl -X POST http://localhost:8080/api/admin/authenticatedUsers/jdoe/deactivate The database ID of the user can be passed instead of the username. .. code-block:: bash - export API_TOKEN=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx export SERVER_URL=http://localhost:8080 export USERID=42 - curl -H "X-Dataverse-key:$API_TOKEN" -X POST $SERVER_URL/api/admin/authenticatedUsers/id/$USERID/deactivate + curl -X POST $SERVER_URL/api/admin/authenticatedUsers/id/$USERID/deactivate Note: A primary purpose of most Dataverse installations is to serve an archive. In the archival space, there are best practices around the tracking of data access and the tracking of modifications to data and metadata. In support of these key workflows, a simple mechanism to delete users that have performed edit or access actions in the system is not provided. Providing a Deactivate User endpoint for users who have taken certain actions in the system alongside a Delete User endpoint to remove users that haven't taken certain actions in the system is by design. From 3713104c35e0b526bba374795c6ba9da3dd5242e Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Mar 2021 12:05:19 -0500 Subject: [PATCH 27/44] fix error handing for revoke all roles (bubble up msg) #2419 #4475 --- src/main/java/edu/harvard/iq/dataverse/api/Users.java | 4 ++-- .../java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Users.java b/src/main/java/edu/harvard/iq/dataverse/api/Users.java index 6e0476711ec..ce226ea14b8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Users.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Users.java @@ -204,8 +204,8 @@ public Response removeUserRoles(@PathParam("identifier") String identifier) { } execCommand(new RevokeAllRolesCommand(userToModify, createDataverseRequest(findUserOrDie()))); return ok("Roles removed for user " + identifier + "."); - } catch (Exception ex) { - return error(Response.Status.BAD_REQUEST, "Unable to revoke all roles: " + ex.getLocalizedMessage()); + } catch (WrappedResponse wr) { + return wr.getResponse(); } } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java index a7f5153d69f..cae1d0e210a 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeleteUsersIT.java @@ -16,6 +16,7 @@ import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.OK; +import static javax.ws.rs.core.Response.Status.UNAUTHORIZED; import static junit.framework.Assert.assertEquals; import static org.hamcrest.CoreMatchers.equalTo; import org.junit.BeforeClass; @@ -673,6 +674,13 @@ public void testCuratorSendsCommentsToAuthor() throws InterruptedException { // // TODO: deactivate curator2 here // + // Show the error if you don't have permission. + Response failToRemoveRole = UtilIT.deleteUserRoles(author2Username, curator2ApiToken); + failToRemoveRole.prettyPrint(); + failToRemoveRole.then().assertThat() + .statusCode(UNAUTHORIZED.getStatusCode()) + .body("message", equalTo("User @" + curator2Username + " is not permitted to perform requested action.")); + Response removeRolesFromAuthor2 = UtilIT.deleteUserRoles(author2Username, superuserApiToken); removeRolesFromAuthor2.prettyPrint(); removeRolesFromAuthor2.then().assertThat() From c2b7b0aaf38c332f21da2adcae0b1233122afbfe Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Mar 2021 17:10:42 -0500 Subject: [PATCH 28/44] add "deactivated" to user dashboard, list-users API #2419 #4475 --- .../java/edu/harvard/iq/dataverse/UserServiceBean.java | 8 ++++++-- .../dataverse/authorization/users/AuthenticatedUser.java | 3 +++ src/main/java/propertyFiles/Bundle.properties | 1 + src/main/webapp/dashboard-users.xhtml | 1 + .../edu/harvard/iq/dataverse/api/DeactivateUsersIT.java | 9 +++++++++ 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java index e395e9a90ec..5707f477a87 100644 --- a/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/UserServiceBean.java @@ -140,7 +140,10 @@ private AuthenticatedUser createAuthenticatedUserForView (Object[] dbRowValues, user.setAuthProviderId(UserUtil.getStringOrNull(dbRowValues[11])); user.setAuthProviderFactoryAlias(UserUtil.getStringOrNull(dbRowValues[12])); - + + user.setDeactivated((Boolean)(dbRowValues[13])); + user.setDeactivatedTime(UserUtil.getTimestampOrNull(dbRowValues[14])); + user.setRoles(roles); return user; } @@ -417,7 +420,8 @@ private List getUserListCore(String searchTerm, qstr += " u.affiliation, u.superuser,"; qstr += " u.position,"; qstr += " u.createdtime, u.lastlogintime, u.lastapiusetime, "; - qstr += " prov.id, prov.factoryalias"; + qstr += " prov.id, prov.factoryalias, "; + qstr += " u.deactivated, u.deactivatedtime "; qstr += " FROM authenticateduser u,"; qstr += " authenticateduserlookup prov_lookup,"; qstr += " authenticationproviderrow prov"; diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index abe0fef597f..0f4bf8b5878 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -388,6 +388,9 @@ public JsonObjectBuilder toJson() { authenicatedUserJson.add("lastLoginTime", UserUtil.getTimestampStringOrNull(this.lastLoginTime)); authenicatedUserJson.add("lastApiUseTime", UserUtil.getTimestampStringOrNull(this.lastApiUseTime)); + authenicatedUserJson.add("deactivated", this.deactivated); + authenicatedUserJson.add("deactivatedTime", UserUtil.getTimestampStringOrNull(this.deactivatedTime)); + return authenicatedUserJson; } diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 8b5e2c0b49e..030d80e4d5c 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -607,6 +607,7 @@ dashboard.list_users.tbl_header.authProviderFactoryAlias=Authentication dashboard.list_users.tbl_header.createdTime=Created Time dashboard.list_users.tbl_header.lastLoginTime=Last Login Time dashboard.list_users.tbl_header.lastApiUseTime=Last API Use Time +dashboard.list_users.tbl_header.deactivated=deactivated dashboard.list_users.tbl_header.roles.removeAll=Remove All dashboard.list_users.tbl_header.roles.removeAll.header=Remove All Roles dashboard.list_users.tbl_header.roles.removeAll.confirmationText=Are you sure you want to remove all roles for user {0}? diff --git a/src/main/webapp/dashboard-users.xhtml b/src/main/webapp/dashboard-users.xhtml index a9e7461f1fb..ebb447dfe30 100644 --- a/src/main/webapp/dashboard-users.xhtml +++ b/src/main/webapp/dashboard-users.xhtml @@ -65,6 +65,7 @@ + diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java index ed979ff785c..f7f5979fcbd 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java @@ -13,6 +13,7 @@ import static javax.ws.rs.core.Response.Status.OK; import static javax.ws.rs.core.Response.Status.UNAUTHORIZED; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.startsWith; import org.junit.BeforeClass; import org.junit.Test; @@ -93,6 +94,14 @@ public void testDeactivateUser() { .statusCode(OK.getStatusCode()) .body("data.deactivated", equalTo(true)); + Response findUser = UtilIT.filterAuthenticatedUsers(superuserApiToken, username, null, 100, null); + findUser.prettyPrint(); + findUser.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.users[0].userIdentifier", equalTo(username)) + .body("data.users[0].deactivated", equalTo(true)) + .body("data.users[0].deactivatedTime", startsWith("2")); + Response getUserDeactivated = UtilIT.getAuthenticatedUserByToken(apiToken); getUserDeactivated.prettyPrint(); getUserDeactivated.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); From 3e59e70b64194a66debbf72738fc43e1ef007c33 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Mon, 15 Mar 2021 15:39:28 -0400 Subject: [PATCH 29/44] in session, ensure user hasn't been deleted or deactivated #2419 #4475 --- .../iq/dataverse/DataverseSession.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java index 315b7c89da7..95c0e124faf 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java @@ -4,6 +4,7 @@ import edu.harvard.iq.dataverse.PermissionServiceBean.StaticPermissionQuery; import edu.harvard.iq.dataverse.actionlogging.ActionLogRecord; import edu.harvard.iq.dataverse.actionlogging.ActionLogServiceBean; +import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; import edu.harvard.iq.dataverse.authorization.users.User; @@ -56,7 +57,10 @@ public class DataverseSession implements Serializable{ @EJB BannerMessageServiceBean bannerMessageService; - + + @EJB + AuthenticationServiceBean authenticationService; + private static final Logger logger = Logger.getLogger(DataverseSession.class.getCanonicalName()); private boolean statusDismissed = false; @@ -89,7 +93,19 @@ public User getUser() { if ( user == null ) { user = GuestUser.get(); } - + if (user instanceof AuthenticatedUser) { + AuthenticatedUser auFromSession = (AuthenticatedUser) user; + AuthenticatedUser auFreshLookup = authenticationService.findByID(auFromSession.getId()); + if (auFreshLookup == null) { + logger.fine("getUser found user no longer exists (was deleted). Returning GuestUser."); + user = GuestUser.get(); + } else { + if (auFreshLookup.isDeactivated()) { + logger.fine("getUser found user is deactivated. Returning GuestUser."); + user = GuestUser.get(); + } + } + } return user; } From 4f81fbde7f9ff4a6a0752619fff2c033c7265354 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 16 Mar 2021 11:37:06 -0400 Subject: [PATCH 30/44] prevent accounts from being merged into themselves #2419 #4475 Otherwise, the account is deleted! --- .../command/impl/MergeInAccountCommand.java | 4 ++++ .../iq/dataverse/api/DeactivateUsersIT.java | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java index c0dce9799e1..950357a1ad2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java @@ -59,6 +59,10 @@ public MergeInAccountCommand(DataverseRequest createDataverseRequest, Authentica @Override protected void executeImpl(CommandContext ctxt) throws CommandException { + if (consumedAU.getId() == ongoingAU.getId()) { + throw new IllegalCommandException("You cannot merge an account into itself.", this); + } + if (consumedAU.isDeactivated() && !ongoingAU.isDeactivated() || !consumedAU.isDeactivated() && ongoingAU.isDeactivated()) { throw new IllegalCommandException("User accounts can only be merged if they are either both non-deactivated or both deactivated.", this); } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java index f7f5979fcbd..cc79f39345a 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java @@ -250,4 +250,25 @@ public void testMergeDeactivatedIntoDeactivatedUser() { mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); } + @Test + public void testMergeUserIntoSelf() { + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response createUserToMerge = UtilIT.createRandomUser(); + createUserToMerge.prettyPrint(); + String usernameToMerge = UtilIT.getUsernameFromResponse(createUserToMerge); + + String usernameMergeTarget = usernameToMerge; + + Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); + mergeAccounts.prettyPrint(); + mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); + } + } From 9fdaf4183769921110c5ac76cc6d1df77b9f40b9 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 16 Mar 2021 14:06:19 -0400 Subject: [PATCH 31/44] fix "merge into self" test #2419 #4475 This should have been included in 4f81fbde7 --- .../java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java index cc79f39345a..a34b3076445 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java @@ -268,7 +268,7 @@ public void testMergeUserIntoSelf() { Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); - mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); + mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); } } From fbded77e18e7fc38399586e748a63a30651a531e Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 16 Mar 2021 14:49:25 -0400 Subject: [PATCH 32/44] prevent deactivated accounts from being converted to OAuth #2419 #4475 --- doc/sphinx-guides/source/user/account.rst | 2 ++ .../authorization/providers/oauth2/OAuth2FirstLoginPage.java | 4 ++++ src/main/java/propertyFiles/Bundle.properties | 1 + 3 files changed, 7 insertions(+) diff --git a/doc/sphinx-guides/source/user/account.rst b/doc/sphinx-guides/source/user/account.rst index 18a44bcb85d..e4fa58d3274 100755 --- a/doc/sphinx-guides/source/user/account.rst +++ b/doc/sphinx-guides/source/user/account.rst @@ -132,6 +132,8 @@ If you already have a Dataverse installation account associated with the Usernam #. Enter your username and password for your Dataverse installation account and click "Convert Account". #. Now you have finished converting your Dataverse installation account to use ORCID for log in. +Note that you cannot go through this conversion process if your Dataverse installation account associated with the Username/Email log in option has been deactivated. + Convert your Dataverse installation account away from ORCID for log in ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java index bd65e296d52..44f00f797a0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/oauth2/OAuth2FirstLoginPage.java @@ -208,6 +208,10 @@ public String convertExistingAccount() { auReq.putCredential(creds.get(1).getKey(), getPassword()); try { AuthenticatedUser existingUser = authenticationSvc.getUpdateAuthenticatedUser(BuiltinAuthenticationProvider.PROVIDER_ID, auReq); + if (existingUser.isDeactivated()) { + JsfHelper.addErrorMessage(BundleUtil.getStringFromBundle("oauth2.convertAccount.failedDeactivated")); + return null; + } authenticationSvc.updateProvider(existingUser, newUser.getServiceId(), newUser.getIdInService()); builtinUserSvc.removeUser(existingUser.getUserIdentifier()); diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 030d80e4d5c..5c23e3dd34e 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -402,6 +402,7 @@ oauth2.convertAccount.password=Password oauth2.convertAccount.authenticationFailed=Your account can only be converted if you provide the correct username and password for your existing account. If your existing account has been deactivated by an administrator, you cannot convert your account. oauth2.convertAccount.buttonTitle=Convert Account oauth2.convertAccount.success=Your Dataverse account is now associated with your {0} account. +oauth2.convertAccount.failedDeactivated=Your existing account cannot be converted because it has been deactivated. # oauth2/callback.xhtml oauth2.callback.page.title=OAuth Callback From fdc9cbe448d5c10b39a550be25adaa864aeab273 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 16 Mar 2021 16:56:18 -0400 Subject: [PATCH 33/44] prevent deactivated accounts from being converted to Shib #2419 #4475 --- doc/sphinx-guides/source/user/account.rst | 2 + .../java/edu/harvard/iq/dataverse/Shib.java | 4 ++ .../edu/harvard/iq/dataverse/api/Admin.java | 4 ++ src/main/java/propertyFiles/Bundle.properties | 1 + .../edu/harvard/iq/dataverse/api/AdminIT.java | 57 +++++++++++++++++++ 5 files changed, 68 insertions(+) diff --git a/doc/sphinx-guides/source/user/account.rst b/doc/sphinx-guides/source/user/account.rst index e4fa58d3274..4c343ff85d4 100755 --- a/doc/sphinx-guides/source/user/account.rst +++ b/doc/sphinx-guides/source/user/account.rst @@ -99,6 +99,8 @@ If you already have a Dataverse installation account associated with the Usernam #. Enter your current password for your Dataverse installation account and click "Convert Account". #. Now you have finished converting your Dataverse installation account to use your institutional log in. +Note that you cannot go through this conversion process if your Dataverse installation account associated with the Username/Email log in option has been deactivated. + Convert your Dataverse installation account away from your Institutional Log In ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/main/java/edu/harvard/iq/dataverse/Shib.java b/src/main/java/edu/harvard/iq/dataverse/Shib.java index 6dd48cc782a..4ad50320f23 100644 --- a/src/main/java/edu/harvard/iq/dataverse/Shib.java +++ b/src/main/java/edu/harvard/iq/dataverse/Shib.java @@ -338,6 +338,10 @@ public String confirmAndConvertAccount() { logger.fine("builtin username: " + builtinUsername); AuthenticatedUser builtInUserToConvert = authSvc.canLogInAsBuiltinUser(builtinUsername, builtinPassword); if (builtInUserToConvert != null) { + if (builtInUserToConvert.isDeactivated()) { + JsfHelper.addErrorMessage(BundleUtil.getStringFromBundle("shib.convert.fail.deactivated")); + return null; + } // TODO: Switch from authSvc.convertBuiltInToShib to authSvc.convertBuiltInUserToRemoteUser AuthenticatedUser au = authSvc.convertBuiltInToShib(builtInUserToConvert, shibAuthProvider.getId(), userIdentifier); if (au != null) { 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 96add147743..92866dfc15a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -695,6 +695,10 @@ public Response builtin2shib(String content) { password); if (authenticatedUser != null) { knowsExistingPassword = true; + if (builtInUserToConvert.isDeactivated()) { + problems.add("builtin account has been deactivated"); + return error(Status.BAD_REQUEST, problems.build().toString()); + } AuthenticatedUser convertedUser = authSvc.convertBuiltInToShib(builtInUserToConvert, shibProviderId, newUserIdentifierInLookupTable); if (convertedUser != null) { diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 5c23e3dd34e..ae7064762eb 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -2217,6 +2217,7 @@ shib.invalidEmailAddress=The SAML assertion contained an invalid email address: shib.emailAddress.error=A single valid address could not be found. shib.nullerror=The SAML assertion for "{0}" was null. Please contact support. dataverse.shib.success=Your Dataverse account is now associated with your institutional account. +shib.convert.fail.deactivated=Your existing account cannot be converted because it has been deactivated. shib.createUser.fail=Couldn't create user. shib.duplicate.email.error=Cannot login, because the e-mail address associated with it has changed since previous login and is already in use by another account. diff --git a/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java b/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java index 84da33cd3ee..e7c42132b0d 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java @@ -358,6 +358,63 @@ public void testConvertShibUserToBuiltin() throws Exception { } + /** + * Here we are asserting that deactivated users cannot be converted into + * shib users. + */ + @Test + public void testConvertDeactivateUserToShib() { + + Response createUserToConvert = UtilIT.createRandomUser(); + createUserToConvert.then().assertThat().statusCode(OK.getStatusCode()); + createUserToConvert.prettyPrint(); + + long idOfUserToConvert = createUserToConvert.body().jsonPath().getLong("data.authenticatedUser.id"); + String emailOfUserToConvert = createUserToConvert.body().jsonPath().getString("data.authenticatedUser.email"); + String usernameOfUserToConvert = UtilIT.getUsernameFromResponse(createUserToConvert); + + Response deactivateUser = UtilIT.deactivateUser(usernameOfUserToConvert); + deactivateUser.prettyPrint(); + deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); + + String password = usernameOfUserToConvert; + String newEmailAddressToUse = "builtin2shib." + UUID.randomUUID().toString().substring(0, 8) + "@mailinator.com"; + String data = emailOfUserToConvert + ":" + password + ":" + newEmailAddressToUse; + + Response builtinToShibAnon = UtilIT.migrateBuiltinToShib(data, ""); + builtinToShibAnon.prettyPrint(); + builtinToShibAnon.then().assertThat().statusCode(FORBIDDEN.getStatusCode()); + + Response createSuperuser = UtilIT.createRandomUser(); + String superuserUsername = UtilIT.getUsernameFromResponse(createSuperuser); + String superuserApiToken = UtilIT.getApiTokenFromResponse(createSuperuser); + Response toggleSuperuser = UtilIT.makeSuperUser(superuserUsername); + toggleSuperuser.then().assertThat() + .statusCode(OK.getStatusCode()); + + Response getAuthProviders = UtilIT.getAuthProviders(superuserApiToken); + getAuthProviders.prettyPrint(); + if (!getAuthProviders.body().asString().contains(BuiltinAuthenticationProvider.PROVIDER_ID)) { + System.out.println("Can't proceed with test without builtin provider."); + return; + } + + Response makeShibUser = UtilIT.migrateBuiltinToShib(data, superuserApiToken); + makeShibUser.prettyPrint(); + makeShibUser.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo("[\"builtin account has been deactivated\"]")); + + Response userIsStillBuiltin = UtilIT.getAuthenticatedUser(usernameOfUserToConvert, superuserApiToken); + userIsStillBuiltin.prettyPrint(); + userIsStillBuiltin.then().assertThat() + .statusCode(OK.getStatusCode()) + .body("data.id", equalTo(Long.valueOf(idOfUserToConvert).intValue())) + .body("data.identifier", equalTo("@" + usernameOfUserToConvert)) + .body("data.authenticationProviderId", equalTo("builtin")); + + } + @Test public void testConvertOAuthUserToBuiltin() throws Exception { From 0efee2810d555c9e241f8e2fa7728389d5483aba Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 17 Mar 2021 11:45:14 -0400 Subject: [PATCH 34/44] exclude deactivated users from role assignment autocomplete #2419 #4475 Restore the query to what it was in previous releases. Put the logic in the method. --- .../edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java | 4 +++- .../iq/dataverse/authorization/users/AuthenticatedUser.java | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java index b31b55b2e4f..6b207ed0e75 100644 --- a/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java @@ -376,7 +376,9 @@ public List filterRoleAssignees(String query, DvObject dvObject, L .getResultList().stream() .filter(ra -> roleAssignSelectedRoleAssignees == null || !roleAssignSelectedRoleAssignees.contains(ra)) .forEach((ra) -> { - roleAssigneeList.add(ra); + if (!ra.isDeactivated()) { + roleAssigneeList.add(ra); + } }); // now we add groups to the list, both global and explicit diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java index 0f4bf8b5878..db6164e0ac7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/users/AuthenticatedUser.java @@ -56,8 +56,7 @@ @NamedQuery( name="AuthenticatedUser.filter", query="select au from AuthenticatedUser au WHERE (" + "LOWER(au.userIdentifier) like LOWER(:query) OR " - + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query)) " - + "AND au.deactivated != true"), + + "lower(concat(au.firstName,' ',au.lastName)) like lower(:query))"), @NamedQuery( name="AuthenticatedUser.findAdminUser", query="select au from AuthenticatedUser au WHERE " + "au.superuser = true " From 4b2ac9644977db21cf20d5bf2a93321e6a7c3f42 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 17 Mar 2021 11:59:09 -0400 Subject: [PATCH 35/44] cleanup, add shib/oauth convert to list for admins #2419 #4475 --- doc/sphinx-guides/source/api/native-api.rst | 1 + .../authorization/AuthenticationServiceBean.java | 3 --- .../iq/dataverse/passwordreset/PasswordResetPage.java | 10 +++++----- .../passwordreset/PasswordResetServiceBean.java | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index ff496df4f20..9f3f89406e5 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3136,6 +3136,7 @@ Deactivating a user with this endpoint will: - Cancel any pending file access requests generated by the user - Remove the user from all groups - No longer have notifications generated or sent by the Dataverse installation +- Prevent the account from being converted into an OAuth or Shibboleth account. Deactivating a user with this endpoint will keep: diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java index 8a3d51bf1a7..349a86301a6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/AuthenticationServiceBean.java @@ -200,9 +200,6 @@ public boolean isOrcidEnabled() { * Before calling this method, make sure you've deleted all the stuff tied * to the user, including stuff they've created, role assignments, group * assignments, etc. See the "removeAuthentictedUserItems" (sic) method. - * - * Longer term, the intention is to have a "disableAuthenticatedUser" - * method/command. See https://github.com/IQSS/dataverse/issues/2419 */ public void deleteAuthenticatedUser(Object pk) { AuthenticatedUser user = em.find(AuthenticatedUser.class, pk); diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java index f27621cef74..aea910c496e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetPage.java @@ -121,14 +121,14 @@ public String sendPasswordResetLink() { actionLogSvc.log( new ActionLogRecord(ActionLogRecord.ActionType.BuiltinUser, "passwordResetSent") .setInfo("Email Address: " + emailAddress) ); } else { - logger.log(Level.INFO, "Cannot find account (or it's disabled) given {0}", emailAddress); + logger.log(Level.INFO, "Cannot find account (or it's deactivated) given {0}", emailAddress); } /** * We show this "an email will be sent" message no matter what (if - * the account can be found or not, if the account has been disabled - * or not) to prevent hackers from figuring out if you have an - * account based on your email address. Yes, this is a white lie - * sometimes, in the name of security. + * the account can be found or not, if the account has been + * deactivated or not) to prevent hackers from figuring out if you + * have an account based on your email address. Yes, this is a white + * lie sometimes, in the name of security. */ FacesContext.getCurrentInstance().addMessage(null, new FacesMessage(FacesMessage.SEVERITY_INFO, BundleUtil.getStringFromBundle("passwdVal.passwdReset.resetInitiated"), BundleUtil.getStringFromBundle("passwdReset.successSubmit.tip", Arrays.asList(emailAddress)))); diff --git a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java index 4e92d861a2f..c8db23985d8 100644 --- a/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/passwordreset/PasswordResetServiceBean.java @@ -53,7 +53,7 @@ public class PasswordResetServiceBean { * * @param emailAddress * @return {@link PasswordResetInitResponse} with empty PasswordResetData if - * the reset won't continue (no user, disabled user). + * the reset won't continue (no user, deactivated user). * @throws edu.harvard.iq.dataverse.passwordreset.PasswordResetException */ // inspired by Troy Hunt: Everything you ever wanted to know about building a secure password reset feature - http://www.troyhunt.com/2012/05/everything-you-ever-wanted-to-know.html From d0191c394e345f02cfb99b71a48845b0fb1b1632 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 18 Mar 2021 13:56:36 -0400 Subject: [PATCH 36/44] more efficient check for disabled or deleted users #2419 #4475 Previously, in 3e59e70 we added logic to check if the user in the session has been deleted or deactivated. The problem is that this code is called often so now we only do this check if the user tries to go to the account page or tries to execute a command. --- .../harvard/iq/dataverse/DataversePage.java | 2 +- .../iq/dataverse/DataverseSession.java | 15 ++++++++++++++- .../iq/dataverse/EjbDataverseEngine.java | 19 ++++++++++++++++++- .../providers/builtin/DataverseUserPage.java | 2 +- src/main/java/propertyFiles/Bundle.properties | 2 ++ 5 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataversePage.java b/src/main/java/edu/harvard/iq/dataverse/DataversePage.java index c7f816ce219..2e5ec9840ed 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataversePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataversePage.java @@ -695,7 +695,7 @@ public String save() { } catch (CommandException ex) { logger.log(Level.SEVERE, "Unexpected Exception calling dataverse command", ex); - String errMsg = create ? BundleUtil.getStringFromBundle("dataverse.create.failure") : BundleUtil.getStringFromBundle("dataverse.update.failure"); + String errMsg = create ? ex.getLocalizedMessage() : BundleUtil.getStringFromBundle("dataverse.update.failure"); JH.addMessage(FacesMessage.SEVERITY_FATAL, errMsg); return null; } catch (Exception e) { diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java index 95c0e124faf..c6016939c08 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataverseSession.java @@ -90,10 +90,23 @@ public void setDismissedMessages(List dismissedMessages) { private Boolean debug; public User getUser() { + return getUser(false); + } + + /** + * For performance reasons, we only lookup the authenticated user again (to + * check if it has been deleted or deactivated, for example) when we have + * to. + * + * @param lookupAuthenticatedUserAgain A boolean to indicate if we should go + * to the database again to lookup the user to get the latest values that + * may have been updated outside the session. + */ + public User getUser(boolean lookupAuthenticatedUserAgain) { if ( user == null ) { user = GuestUser.get(); } - if (user instanceof AuthenticatedUser) { + if (lookupAuthenticatedUserAgain && user instanceof AuthenticatedUser) { AuthenticatedUser auFromSession = (AuthenticatedUser) user; AuthenticatedUser auFreshLookup = authenticationService.findByID(auFromSession.getId()); if (auFreshLookup == null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java b/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java index da9aba43498..0937f6f6cf7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java +++ b/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java @@ -8,6 +8,7 @@ import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.groups.GroupServiceBean; import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroupServiceBean; +import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.confirmemail.ConfirmEmailServiceBean; import edu.harvard.iq.dataverse.datacapturemodule.DataCaptureModuleServiceBean; import edu.harvard.iq.dataverse.engine.command.Command; @@ -30,8 +31,10 @@ import edu.harvard.iq.dataverse.search.SolrIndexServiceBean; import edu.harvard.iq.dataverse.search.savedsearch.SavedSearchServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; +import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.SystemConfig; import edu.harvard.iq.dataverse.workflow.WorkflowServiceBean; +import java.util.Arrays; import java.util.EnumSet; import java.util.Stack; import java.util.logging.Level; @@ -215,7 +218,21 @@ public R submit(Command aCommand) throws CommandException { } DataverseRequest dvReq = aCommand.getRequest(); - + + AuthenticatedUser authenticatedUser = dvReq.getAuthenticatedUser(); + if (authenticatedUser != null) { + AuthenticatedUser auFreshLookup = authentication.findByID(authenticatedUser.getId()); + if (auFreshLookup == null) { + logger.fine("submit method found user no longer exists (was deleted)."); + throw new CommandException(BundleUtil.getStringFromBundle("command.exception.user.deleted", Arrays.asList(aCommand.getClass().getSimpleName())), aCommand); + } else { + if (auFreshLookup.isDeactivated()) { + logger.fine("submit method found user is deactivated."); + throw new CommandException(BundleUtil.getStringFromBundle("command.exception.user.deactivated", Arrays.asList(aCommand.getClass().getSimpleName())), aCommand); + } + } + } + Map affectedDvObjects = aCommand.getAffectedDvObjects(); logRec.setInfo(aCommand.describe()); for (Map.Entry> pair : requiredMap.entrySet()) { diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java index 663cdf4b4b2..65cb11b5351 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java @@ -162,7 +162,7 @@ public String init() { } } - if ( session.getUser().isAuthenticated() ) { + if (session.getUser(true).isAuthenticated()) { setCurrentUser((AuthenticatedUser) session.getUser()); userAuthProvider = authenticationService.lookupProvider(currentUser); notificationsList = userNotificationService.findByUser(currentUser.getId()); diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index ff9522b6bf4..e78c5433bd9 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -2283,6 +2283,8 @@ pid.allowedCharacters=^[A-Za-z0-9._/:\\-]* #General Command Exception command.exception.only.superusers={1} can only be called by superusers. +command.exception.user.deactivated={0} failed: User account has been deactivated. +command.exception.user.deleted={0} failed: User account has been deleted. #Admin-API admin.api.auth.mustBeSuperUser=Forbidden. You must be a superuser. From 530dab34dbbbe0f3a07bdc12d35b5cae330470ec Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Mon, 22 Mar 2021 14:37:19 -0400 Subject: [PATCH 37/44] move logic up (earlier) #2419 #4475 --- src/main/java/edu/harvard/iq/dataverse/api/Admin.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 92866dfc15a..c86dcd8118f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -689,16 +689,16 @@ public Response builtin2shib(String content) { boolean knowsExistingPassword = false; BuiltinUser oldBuiltInUser = builtinUserService.findByUserName(builtInUserToConvert.getUserIdentifier()); if (oldBuiltInUser != null) { + if (builtInUserToConvert.isDeactivated()) { + problems.add("builtin account has been deactivated"); + return error(Status.BAD_REQUEST, problems.build().toString()); + } String usernameOfBuiltinAccountToConvert = oldBuiltInUser.getUserName(); response.add("old username", usernameOfBuiltinAccountToConvert); AuthenticatedUser authenticatedUser = authSvc.canLogInAsBuiltinUser(usernameOfBuiltinAccountToConvert, password); if (authenticatedUser != null) { knowsExistingPassword = true; - if (builtInUserToConvert.isDeactivated()) { - problems.add("builtin account has been deactivated"); - return error(Status.BAD_REQUEST, problems.build().toString()); - } AuthenticatedUser convertedUser = authSvc.convertBuiltInToShib(builtInUserToConvert, shibProviderId, newUserIdentifierInLookupTable); if (convertedUser != null) { From df32ec0128ea2a054ae4fb1bb01871960a9d01df Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Mon, 22 Mar 2021 14:55:19 -0400 Subject: [PATCH 38/44] change "non-deactivated" to "active" #2419 #4475 --- doc/sphinx-guides/source/api/native-api.rst | 2 +- .../engine/command/impl/MergeInAccountCommand.java | 2 +- .../edu/harvard/iq/dataverse/api/DeactivateUsersIT.java | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 9f3f89406e5..d047952a834 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3055,7 +3055,7 @@ Example: ``curl -H "X-Dataverse-key: $API_TOKEN" -X POST http://demo.dataverse.o This action moves account data from jsmith2 into the account jsmith and deletes the account of jsmith2. -Note: User accounts can only be merged if they are either both non-deactivated or both deactivated. See :ref:`deactivate a user`. +Note: User accounts can only be merged if they are either both active or both deactivated. See :ref:`deactivate a user`. .. _change-identifier-label: diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java index 950357a1ad2..1ec51764d73 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/MergeInAccountCommand.java @@ -64,7 +64,7 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { } if (consumedAU.isDeactivated() && !ongoingAU.isDeactivated() || !consumedAU.isDeactivated() && ongoingAU.isDeactivated()) { - throw new IllegalCommandException("User accounts can only be merged if they are either both non-deactivated or both deactivated.", this); + throw new IllegalCommandException("User accounts can only be merged if they are either both active or both deactivated.", this); } List baseRAList = ctxt.roleAssignees().getAssignmentsFor(ongoingAU.getIdentifier()); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java index a34b3076445..25fa03dca79 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java @@ -184,7 +184,7 @@ public void testMergeDeactivatedIntoNonDeactivatedUser() { deactivateUser.prettyPrint(); deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); - // User accounts can only be merged if they are either both non-deactivated or both deactivated. + // User accounts can only be merged if they are either both active or both deactivated. Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); @@ -212,7 +212,7 @@ public void testMergeNonDeactivatedIntoDeactivatedUser() { deactivateUser.prettyPrint(); deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); - // User accounts can only be merged if they are either both non-deactivated or both deactivated. + // User accounts can only be merged if they are either both active or both deactivated. Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); @@ -244,7 +244,7 @@ public void testMergeDeactivatedIntoDeactivatedUser() { deactivatedUserToMerge.prettyPrint(); deactivatedUserToMerge.then().assertThat().statusCode(OK.getStatusCode()); - // User accounts can only be merged if they are either both non-deactivated or both deactivated. + // User accounts can only be merged if they are either both active or both deactivated. Response mergeAccounts = UtilIT.mergeAccounts(usernameMergeTarget, usernameToMerge, superuserApiToken); mergeAccounts.prettyPrint(); mergeAccounts.then().assertThat().statusCode(OK.getStatusCode()); From d00d0ec9493c6d820be29ea38f1429975cde0a62 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Tue, 23 Mar 2021 16:51:30 -0400 Subject: [PATCH 39/44] rename SQL script #2419 #4475 --- ...9-deactivate-users.sql => V5.3.0.6__2419-deactivate-users.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/{V5.3.0.5__2419-deactivate-users.sql => V5.3.0.6__2419-deactivate-users.sql} (100%) diff --git a/src/main/resources/db/migration/V5.3.0.5__2419-deactivate-users.sql b/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql similarity index 100% rename from src/main/resources/db/migration/V5.3.0.5__2419-deactivate-users.sql rename to src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql From 81274c9f65ac839df101e850d50d9fd153052648 Mon Sep 17 00:00:00 2001 From: Gustavo Durand Date: Wed, 24 Mar 2021 12:07:12 -0400 Subject: [PATCH 40/44] Update DataversePage.java --- src/main/java/edu/harvard/iq/dataverse/DataversePage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DataversePage.java b/src/main/java/edu/harvard/iq/dataverse/DataversePage.java index 2e5ec9840ed..c7f816ce219 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DataversePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DataversePage.java @@ -695,7 +695,7 @@ public String save() { } catch (CommandException ex) { logger.log(Level.SEVERE, "Unexpected Exception calling dataverse command", ex); - String errMsg = create ? ex.getLocalizedMessage() : BundleUtil.getStringFromBundle("dataverse.update.failure"); + String errMsg = create ? BundleUtil.getStringFromBundle("dataverse.create.failure") : BundleUtil.getStringFromBundle("dataverse.update.failure"); JH.addMessage(FacesMessage.SEVERITY_FATAL, errMsg); return null; } catch (Exception e) { From 569136f5ae4967c442b527aa0cb0066961c7abba Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 25 Mar 2021 14:55:47 -0400 Subject: [PATCH 41/44] prevent user table, user-list API from blowing up #2419 #4475 The createAuthenticatedUserForView method can't handle a null for the deactivated boolean so we'll set the boolean to false for existing users. For new users, this is set to false already. --- .../resources/db/migration/V5.3.0.6__2419-deactivate-users.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql b/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql index c87fe019a52..81f01107be3 100644 --- a/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql +++ b/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql @@ -1,4 +1,6 @@ -- Users can be deactivated. ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS deactivated BOOLEAN; +-- Prevent old users from having null for deactivated. +UPDATE authenticateduser SET deactivated = false; -- A timestamp of when the user was deactivated. ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS deactivatedtime timestamp without time zone; From c326af4c5b9d2c8f421cf9eb0bb5e5267303c481 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 25 Mar 2021 16:54:37 -0400 Subject: [PATCH 42/44] deactivated users cannot become superusers #2419 #4475 --- doc/sphinx-guides/source/api/native-api.rst | 1 + .../edu/harvard/iq/dataverse/api/Admin.java | 3 +++ .../impl/GrantSuperuserStatusCommand.java | 4 ++++ src/main/webapp/dashboard-users.xhtml | 3 ++- .../iq/dataverse/api/DeactivateUsersIT.java | 18 ++++++++++++++++++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 5f76bb4ba0b..85e65d26694 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3229,6 +3229,7 @@ Deactivating a user with this endpoint will: - Remove the user from all groups - No longer have notifications generated or sent by the Dataverse installation - Prevent the account from being converted into an OAuth or Shibboleth account. +- Prevent the user from becoming a superuser. Deactivating a user with this endpoint will keep: 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 7343b11ed3f..f0a9f8cf780 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -945,6 +945,9 @@ public Response toggleSuperuser(@PathParam("identifier") String identifier) { .setInfo(identifier); try { AuthenticatedUser user = authSvc.getAuthenticatedUser(identifier); + if (user.isDeactivated()) { + return error(Status.BAD_REQUEST, "You cannot make a deactivated user a superuser."); + } user.setSuperuser(!user.isSuperuser()); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GrantSuperuserStatusCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GrantSuperuserStatusCommand.java index 29f1b891c91..42af43b7247 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GrantSuperuserStatusCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GrantSuperuserStatusCommand.java @@ -40,6 +40,10 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { this, null, null); } + if (targetUser.isDeactivated()) { + throw new CommandException("User " + targetUser.getIdentifier() + " has been deactivated and cannot become a superuser.", this); + } + try { targetUser.setSuperuser(true); ctxt.em().merge(targetUser); diff --git a/src/main/webapp/dashboard-users.xhtml b/src/main/webapp/dashboard-users.xhtml index ebb447dfe30..3f6087cf01c 100644 --- a/src/main/webapp/dashboard-users.xhtml +++ b/src/main/webapp/dashboard-users.xhtml @@ -85,7 +85,8 @@ - + + diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java index 25fa03dca79..374a19f453d 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/DeactivateUsersIT.java @@ -271,4 +271,22 @@ public void testMergeUserIntoSelf() { mergeAccounts.then().assertThat().statusCode(BAD_REQUEST.getStatusCode()); } + @Test + public void testTurnDeactivatedUserIntoSuperuser() { + + Response createUser = UtilIT.createRandomUser(); + createUser.prettyPrint(); + String username = UtilIT.getUsernameFromResponse(createUser); + + Response deactivateUser = UtilIT.deactivateUser(username); + deactivateUser.prettyPrint(); + deactivateUser.then().assertThat().statusCode(OK.getStatusCode()); + + Response toggleSuperuser = UtilIT.makeSuperUser(username); + toggleSuperuser.prettyPrint(); + toggleSuperuser.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()); + + } + } From 36f25bb179d4527aeb08d5b90554f330c8771318 Mon Sep 17 00:00:00 2001 From: Stephen Kraffmiller Date: Fri, 26 Mar 2021 15:20:37 -0400 Subject: [PATCH 43/44] #2419 retest session user on save --- .../authorization/providers/builtin/DataverseUserPage.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java index 59843495b94..673839450d6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/DataverseUserPage.java @@ -284,6 +284,12 @@ public void validateNewPassword(FacesContext context, UIComponent toValidate, Ob public String save() { boolean passwordChanged = false; + + //First reget user to make sure they weren't deactivated or deleted + if (session.getUser().isAuthenticated() && !session.getUser(true).isAuthenticated()) { + return "dataverse.xhtml?alias=" + dataverseService.findRootDataverse().getAlias() + "&faces-redirect=true"; + } + if (editMode == EditMode.CHANGE_PASSWORD) { final AuthenticationProvider prv = getUserAuthProvider(); if (prv.isPasswordUpdateAllowed()) { From 274f150d7cd1382245856fdbdc9b64f5b9412e85 Mon Sep 17 00:00:00 2001 From: Stephen Kraffmiller Date: Mon, 29 Mar 2021 09:34:53 -0400 Subject: [PATCH 44/44] #2419 update update query --- .../resources/db/migration/V5.3.0.6__2419-deactivate-users.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql b/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql index 81f01107be3..a5e4b69e00b 100644 --- a/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql +++ b/src/main/resources/db/migration/V5.3.0.6__2419-deactivate-users.sql @@ -1,6 +1,6 @@ -- Users can be deactivated. ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS deactivated BOOLEAN; -- Prevent old users from having null for deactivated. -UPDATE authenticateduser SET deactivated = false; +UPDATE authenticateduser SET deactivated = false WHERE deactivated IS NULL; -- A timestamp of when the user was deactivated. ALTER TABLE authenticateduser ADD COLUMN IF NOT EXISTS deactivatedtime timestamp without time zone;