diff --git a/core/src/main/java/org/fao/geonet/kernel/AccessManager.java b/core/src/main/java/org/fao/geonet/kernel/AccessManager.java index c6f2d9bc3c55..fc52accb5611 100644 --- a/core/src/main/java/org/fao/geonet/kernel/AccessManager.java +++ b/core/src/main/java/org/fao/geonet/kernel/AccessManager.java @@ -524,19 +524,18 @@ private boolean hasEditingPermissionWithProfile(final ServiceContext context, fi /** * Checks if the user has the specified profile or any profile with greater permissions within the group. * - * @param context The service context containing the user's session. + * @param userSession The user's session. * @param profile The profile to be verified. * @param groupId The ID of the group in which the user's profile is to be verified. * @return true if the user has the specified profile (or greater) within the group; false otherwise. */ - public boolean isProfileOrMoreOnGroup(final ServiceContext context, Profile profile, final int groupId) { - UserSession us = context.getUserSession(); - if (!isUserAuthenticated(us)) { + public boolean isProfileOrMoreOnGroup(final UserSession userSession, Profile profile, final int groupId) { + if (!isUserAuthenticated(userSession)) { return false; } // Grant access if the user is a global administrator - if (Profile.Administrator == us.getProfile()) { + if (Profile.Administrator == userSession.getProfile()) { return true; } @@ -544,7 +543,7 @@ public boolean isProfileOrMoreOnGroup(final ServiceContext context, Profile prof Set acceptedProfiles = profile.getProfileAndAllParents(); // Build a specification to fetch any accepted profiles for the user in the specified group - Specification spec = Specification.where(UserGroupSpecs.hasUserId(us.getUserIdAsInt())) + Specification spec = Specification.where(UserGroupSpecs.hasUserId(userSession.getUserIdAsInt())) .and(UserGroupSpecs.hasGroupId(groupId)) .and(UserGroupSpecs.hasProfileIn(acceptedProfiles)); List userGroups = userGroupRepository.findAll(spec); @@ -552,6 +551,31 @@ public boolean isProfileOrMoreOnGroup(final ServiceContext context, Profile prof return !userGroups.isEmpty(); } + /** + * Checks if the user has exactly the specified profile within the group. + * + * @param userSession The user's session. + * @param profile The exact profile to verify. + * @param groupId The ID of the group in which the user's profile is verified. + * @return true if the user has exactly the specified profile in the group; false otherwise. + */ + public boolean isProfileOnGroup(final UserSession userSession, Profile profile, final int groupId) { + if (!isUserAuthenticated(userSession)) { + return false; + } + + // Global administrators remain allowed regardless of group assignments. + if (Profile.Administrator == userSession.getProfile()) { + return true; + } + + Specification spec = Specification.where(UserGroupSpecs.hasUserId(userSession.getUserIdAsInt())) + .and(UserGroupSpecs.hasGroupId(groupId)) + .and(UserGroupSpecs.hasProfile(profile)); + + return userGroupRepository.count(spec)> 0; + } + public int getPrivilegeId(final String name) { final Operation op = operationRepository.findByName(name); if (op == null) { diff --git a/docs/manual/docs/administrator-guide/configuring-the-catalog/system-configuration.md b/docs/manual/docs/administrator-guide/configuring-the-catalog/system-configuration.md index 5cf032d6c74d..7581f8e93c08 100644 --- a/docs/manual/docs/administrator-guide/configuring-the-catalog/system-configuration.md +++ b/docs/manual/docs/administrator-guide/configuring-the-catalog/system-configuration.md @@ -340,10 +340,12 @@ Allows to configure the user profile allowed to delete published metadata. ## Metadata publication -Allows to configure the user profile allowed to publish and un-publish metadata. +Allows to configure the required user profile to publish and un-publish metadata. -- **Minimum user profile allowed to publish metadata** Minimum user profile allowed to publish metadata (`Reviewer` or `Administrator`). The default value is `Reviewer`. -- **Minimum user profile allowed to un-publish metadata** Minimum user profile allowed to un-publish metadata (`Reviewer` or `Administrator`). The default value is `Reviewer`. +The configured profile is evaluated on the metadata owner group (per-group role), not the user's global profile. The user must have exactly this profile in the record owner group. For example, with `Reviewer`, only users who are `Reviewer` in the owner group are allowed (not `UserAdmin`). Global `Administrator` is always allowed. + +- **Required profile to publish metadata** Profile required to publish metadata, evaluated in the record owner group (`Reviewer` or `Administrator`). The default value is `Reviewer`. +- **Required profile to un-publish metadata** Profile required to un-publish metadata, evaluated in the record owner group (`Reviewer` or `Administrator`). The default value is `Reviewer`. ![](img/metadata-publication.png) diff --git a/docs/manual/docs/user-guide/publishing/managing-privileges.md b/docs/manual/docs/user-guide/publishing/managing-privileges.md index 3a88d4d0e808..8b6be3096158 100644 --- a/docs/manual/docs/user-guide/publishing/managing-privileges.md +++ b/docs/manual/docs/user-guide/publishing/managing-privileges.md @@ -8,7 +8,7 @@ For example, you can specify that the metadata and related services are visible The privileges panel uses colour coding to identify different group types: - - **Blue rows** — [Reserved groups](../../administrator-guide/managing-users-and-groups/creating-group.md) (All, Intranet, Guest). Only Administrators and Reviewers can edit these privileges. + - **Blue rows** — [Reserved groups](../../administrator-guide/managing-users-and-groups/creating-group.md) (All, Intranet, Guest). Editing these privileges requires the configured publication profile on the record owner group (default: Reviewer). Administrators are always allowed. - **Yellow rows** — [Record Privilege Groups](../../administrator-guide/managing-users-and-groups/creating-group.md#2-record-privilege-group). These groups can be assigned privileges on specific records but cannot own metadata. - **No highlight** — Standard [Workspace Groups](../../administrator-guide/managing-users-and-groups/creating-group.md#1-workspace-group). @@ -82,10 +82,10 @@ A *reviewer* / *editor* can edit a metadata if: A button to access the Privileges page for a metadata record displays in the search results or when you are viewing the record for: - All Administrators -- All Reviewers that are member of one of the groups assigned to the metadata owner. +- Users who meet the configured publication profile in the metadata owner group (default: Reviewer). - The Owner of the metadata -Only Administrators and Reviewers can edit privileges for the All and Intranet groups. +Only users meeting the configured publication profile in the metadata owner group can edit privileges for reserved groups (All, Intranet, Guest). By default this is Reviewer. ## Setting Privileges on a selected set of metadata records diff --git a/services/src/main/java/org/fao/geonet/api/doiservers/DoiServersApi.java b/services/src/main/java/org/fao/geonet/api/doiservers/DoiServersApi.java index 985e42116eaa..8be60048642c 100644 --- a/services/src/main/java/org/fao/geonet/api/doiservers/DoiServersApi.java +++ b/services/src/main/java/org/fao/geonet/api/doiservers/DoiServersApi.java @@ -350,6 +350,6 @@ private boolean isMetadataOwnerOrReviewer(ServiceContext serviceContext, Integer Integer groupOwner, Profile userProfile) throws Exception { return (userProfile == Profile.Administrator) || accessManager.isOwner(serviceContext, String.valueOf(metadataId)) - || accessManager.isProfileOrMoreOnGroup(serviceContext, Profile.Reviewer, groupOwner); + || accessManager.isProfileOrMoreOnGroup(serviceContext.getUserSession(), Profile.Reviewer, groupOwner); } } diff --git a/services/src/main/java/org/fao/geonet/api/records/MetadataSharingApi.java b/services/src/main/java/org/fao/geonet/api/records/MetadataSharingApi.java index f94d2fccd489..448ef3d65106 100644 --- a/services/src/main/java/org/fao/geonet/api/records/MetadataSharingApi.java +++ b/services/src/main/java/org/fao/geonet/api/records/MetadataSharingApi.java @@ -45,6 +45,7 @@ import org.fao.geonet.api.tools.i18n.LanguageUtils; import org.fao.geonet.config.IPublicationConfig; import org.fao.geonet.config.PublicationOption; +import org.fao.geonet.constants.Geonet; import org.fao.geonet.domain.*; import org.fao.geonet.domain.utils.ObjectJSONUtils; import org.fao.geonet.events.history.RecordGroupOwnerChangeEvent; @@ -63,8 +64,8 @@ import org.fao.geonet.repository.specification.MetadataValidationSpecs; import org.fao.geonet.repository.specification.UserGroupSpecs; import org.fao.geonet.util.MetadataPublicationMailNotifier; -import org.fao.geonet.util.UserUtil; import org.fao.geonet.util.WorkflowUtil; +import org.fao.geonet.utils.Log; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationContext; @@ -254,7 +255,9 @@ public void publish( throws Exception { ServiceContext serviceContext = ApiUtils.createServiceContext(request); UserSession userSession = ApiUtils.getUserSession(request.getSession()); - checkUserProfileToPublishMetadata(userSession); + AbstractMetadata metadata = ApiUtils.getRecord(metadataUuid); + Integer groupOwner = metadata.getSourceInfo().getGroupOwner(); + checkUserProfileToPublishMetadata(groupOwner, userSession); if (StringUtils.isEmpty(publicationType)) { publicationType = DEFAULT_PUBLICATION_TYPE_NAME; @@ -264,7 +267,6 @@ public void publish( java.util.Optional publicationOption = publicationConfig.getPublicationOptionConfiguration(publicationType); if (publicationOption.isPresent()) { - AbstractMetadata metadata = ApiUtils.getRecord(metadataUuid); publicationConfig.processMetadata(serviceContext, publicationOption.get(), metadata.getId(), true); } } @@ -299,7 +301,11 @@ public void unpublish( throws Exception { ServiceContext serviceContext = ApiUtils.createServiceContext(request); UserSession userSession = ApiUtils.getUserSession(request.getSession()); - checkUserProfileToUnpublishMetadata(userSession); + + // Get the id of the group that owns the metadata record identified by the uuid + AbstractMetadata metadata = ApiUtils.getRecord(metadataUuid); + Integer groupOwner = metadata.getSourceInfo().getGroupOwner(); + checkUserProfileToUnpublishMetadata(groupOwner, userSession); if (StringUtils.isEmpty(publicationType)) { publicationType = DEFAULT_PUBLICATION_TYPE_NAME; @@ -309,7 +315,6 @@ public void unpublish( java.util.Optional publicationOption = publicationConfig.getPublicationOptionConfiguration(publicationType); if (publicationOption.isPresent()) { - AbstractMetadata metadata = ApiUtils.getRecord(metadataUuid); publicationConfig.processMetadata(serviceContext, publicationOption.get(), metadata.getId(), false); } } @@ -318,12 +323,13 @@ public void unpublish( summary = "Set record sharing", description = "Privileges are assigned by group. User needs to be able " + "to edit a record to set sharing settings. For reserved group " + - "(ie. Internet, Intranet & Guest), user MUST be reviewer of one group. " + + "(ie. Internet, Intranet & Guest), user MUST have the configured " + + "publication profile in the metadata owner group. " + "For other group, if Only set privileges to user's groups is set " + "in catalog configuration user MUST be a member of the group.
" + "Clear first allows to unset all operations first before setting the new ones." + - "Clear option does not remove reserved groups operation if user is not an " + - "administrator, a reviewer or the owner of the record.
" + + "Clear option does not remove reserved groups operation if user does not " + + "meet reserved-group publication permissions for the record.
" + "More info") @RequestMapping( value = "/{metadataUuid}/sharing", @@ -574,11 +580,11 @@ private void setOperations( SharingResponse sharingBefore = getRecordSharingSettings(metadata.getUuid(), request.getSession(), request); // Check if the user profile can change the privileges for publication/un-publication of the reserved groups - checkChangesAllowedToUserProfileForReservedGroups(context.getUserSession(), sharingBefore, privileges, !sharing.isClear()); + checkChangesAllowedToUserProfileForReservedGroups(context.getUserSession(), metadata, sharingBefore, privileges, !sharing.isClear()); List excludeFromDelete = new ArrayList<>(); - // Exclude deleting privileges for groups in which the user does not have the minimum profile for privileges + // Exclude deleting privileges for groups in which the user does not have the required profile for privileges for (Group group : groupRepository.findByMinimumProfileForPrivilegesNotNull()) { if (!canUserChangePrivilegesForGroup(context, group)) { excludeFromDelete.add(group.getId()); @@ -778,7 +784,7 @@ public SharingResponse getRecordSharingSettings( } groupPrivilege.setUserProfile(userGroupProfile); - // Restrict changing privileges for groups with a minimum profile for setting privileges set + // Restrict changing privileges for groups with a required profile for setting privileges set groupPrivilege.setRestricted(!canUserChangePrivilegesForGroup(context, g)); //--- get all operations that this group can do on given metadata @@ -1066,7 +1072,7 @@ public SharingResponse getSharingSettings( groupPrivilege.setGroup(g.getId()); groupPrivilege.setReserved(g.isReserved()); groupPrivilege.setRecordPrivilege(g.getType() == GroupType.RecordPrivilege); - // Restrict changing privileges for groups with a minimum profile for setting privileges set + // Restrict changing privileges for groups with a required profile for setting privileges set groupPrivilege.setRestricted(!canUserChangePrivilegesForGroup(context, g)); groupPrivilege.setUserGroup(userGroups.contains(g.getId())); @@ -1403,14 +1409,8 @@ private void shareMetadataWithReservedGroup(String metadataUuid, boolean publish AbstractMetadata metadata = ApiUtils.canEditRecord(metadataUuid, request); ApplicationContext appContext = ApplicationContextHolder.get(); ServiceContext context = ApiUtils.createServiceContext(request); - ResourceBundle messages = ApiUtils.getMessagesResourceBundle(request.getLocales()); Locale[] feedbackLocales = feedbackLanguages.getLocales(request.getLocale()); - if (!accessManager.hasReviewPermission(context, Integer.toString(metadata.getId()))) { - throw new Exception(String.format(messages.getString("api.metadata.share.ErrorUserNotAllowedToPublish"), - metadataUuid, messages.getString(accessManager.getReviewerRule()))); - - } List operationList = operationRepository.findAll(); Map operationMap = new HashMap<>(operationList.size()); @@ -1605,90 +1605,127 @@ private SharingParameter buildSharingForPublicationConfig(boolean publish, Strin /** * Verifies if the user profile can make the privileges changes for reserved groups. * - * @param userSession - * @param originalPrivileges - * @param newPrivileges - * @param merge + *

The authorization logic is: + *

    + *
  • Administrators may always change reserved-group privileges.
    • + *
  • For publishing: the user must have exactly the configured publication profile + * in the metadata's group owner (e.g. Reviewer means the user must be a Reviewer in that group).
    • + *
  • For unpublishing: the user must have exactly the configured unpublication profile + * in the metadata's group owner.
    • + *
+ * + * @param userSession the current user session + * @param metadata the metadata record whose privileges are being changed + * @param originalPrivileges the sharing settings before the change + * @param newPrivileges the new sharing settings to apply + * @param merge whether the new settings are merged with or replace the old ones + * @throws Exception if the per-group reviewer look-up fails */ private void checkChangesAllowedToUserProfileForReservedGroups(UserSession userSession, + AbstractMetadata metadata, SharingResponse originalPrivileges, List newPrivileges, boolean merge) { if (userSession.getProfile() == Profile.Administrator) { - return; - } - - if (userSession.getProfile() == Profile.Editor || userSession.getProfile() == Profile.UserAdmin) { - boolean hasReservedGroupPrivileges = newPrivileges.stream().anyMatch(priv -> ReservedGroup.isReserved(priv.getGroup())); - - if (hasReservedGroupPrivileges) { - throw new NotAllowedException(String.format( - "Publication/Unpublication of metadata is not allowed for %s", userSession.getProfile())); - } - - return; + return; // Administrators are always allowed to change reserved groups privileges } + // Check if there are any changes to reserved groups privileges List privilegeStatusChangesList = reservedGroupsPrivilegesStatusChanges(originalPrivileges, newPrivileges, merge); - if (!privilegeStatusChangesList.isEmpty()) { - boolean metadataWasPublishedBeforeAndNotAfter = false; - boolean metadataWasNotPublishedBeforeAndIsAfter = false; + if (privilegeStatusChangesList.isEmpty()) { + return; // No changes to reserved groups, no authorization check needed + } - for (PrivilegeStatusChange status : privilegeStatusChangesList) { - if (status.isPublishedBefore() && !status.isPublishedAfter()) { - metadataWasPublishedBeforeAndNotAfter = true; - } else if (!status.isPublishedBefore() && status.isPublishedAfter()) { - metadataWasNotPublishedBeforeAndIsAfter = true; - } - } + // Determine if publishing or unpublishing operations are being performed + boolean isPublishing = false; + boolean isUnpublishing = false; - if (metadataWasPublishedBeforeAndNotAfter) { - // Is the user profile allowed to un-publish the metadata? - checkUserProfileToUnpublishMetadata(userSession); + for (PrivilegeStatusChange status : privilegeStatusChangesList) { + if (!status.isPublishedBefore() && status.isPublishedAfter()) { + isPublishing = true; + } else if (status.isPublishedBefore() && !status.isPublishedAfter()) { + isUnpublishing = true; } + } - if (metadataWasNotPublishedBeforeAndIsAfter) { - // Is the user profile allowed to publish the metadata? - checkUserProfileToPublishMetadata(userSession); - } + Integer groupOwner = metadata.getSourceInfo().getGroupOwner(); + + // Perform authorization checks based on the operation(s) being performed + if (isPublishing) { + checkUserProfileToPublishMetadata(groupOwner, userSession); + } + + if (isUnpublishing) { + checkUserProfileToUnpublishMetadata(groupOwner, userSession); } } /** * Checks if the user profile is allowed to publish metadata. * - * @param userSession + * @param groupId the group owner of the metadata to publish + * @param userSession the user session for authorization checks */ - private void checkUserProfileToPublishMetadata(UserSession userSession) { - if (userSession.getProfile() != Profile.Administrator) { - String allowedUserProfileToPublishMetadata = - org.apache.commons.lang.StringUtils.defaultIfBlank(sm.getValue(Settings.METADATA_PUBLISH_USERPROFILE), Profile.Reviewer.toString()); - - // Is the user profile is higher than the profile allowed to import metadata? - if (!UserUtil.hasHierarchyRole(allowedUserProfileToPublishMetadata, this.roleHierarchy)) { - throw new NotAllowedException(String.format( - "Publication of metadata is not allowed. User needs to be at least %s to publish record.", allowedUserProfileToPublishMetadata)); + private void checkUserProfileToPublishMetadata(Integer groupId, UserSession userSession) { + if (userSession.getProfile() == Profile.Administrator) { + return; // Administrators are always allowed to publish metadata + } + if (groupId == null) { + throw new NotAllowedException("Publication of metadata is not allowed. Metadata without group owner cannot be published."); + } + Profile defaultProfileForPublishing = Profile.Reviewer; + String configuredProfileForPublishing = sm.getValue(Settings.METADATA_PUBLISH_USERPROFILE); + Profile requiredProfileForPublishing; + try { + requiredProfileForPublishing = Profile.valueOf(configuredProfileForPublishing); + } catch (IllegalArgumentException | NullPointerException e) { + if (e instanceof IllegalArgumentException) { + Log.error(Geonet.SETTINGS, "Invalid profile configured for publishing. Using default value: " + defaultProfileForPublishing); } + requiredProfileForPublishing = defaultProfileForPublishing; + } + + boolean canUserPublishForGroup = accessManager.isProfileOnGroup(userSession, requiredProfileForPublishing, groupId); + + if (!canUserPublishForGroup) { + throw new NotAllowedException(String.format( + "Publication of metadata is not allowed. User must have the %s profile in the record owner group.", requiredProfileForPublishing)); } } /** - * Checks if the user profile is allowed to un-publish metadata. + * Checks if the user profile is allowed to unpublish metadata. + * + * @param groupId the group owner of the metadata to unpublish + * @param userSession the user session for authorization checks * - * @param userSession */ - private void checkUserProfileToUnpublishMetadata(UserSession userSession) { - if (userSession.getProfile() != Profile.Administrator) { - String allowedUserProfileToUnpublishMetadata = - org.apache.commons.lang.StringUtils.defaultIfBlank(sm.getValue(Settings.METADATA_UNPUBLISH_USERPROFILE), Profile.Reviewer.toString()); - - // Is the user profile is higher than the profile allowed to import metadata? - if (!UserUtil.hasHierarchyRole(allowedUserProfileToUnpublishMetadata, this.roleHierarchy)) { - throw new NotAllowedException(String.format( - "Unpublication of metadata is not allowed. User needs to be at least %s to unpublish record.", allowedUserProfileToUnpublishMetadata)); + private void checkUserProfileToUnpublishMetadata(Integer groupId, UserSession userSession) { + if (userSession.getProfile() == Profile.Administrator) { + return; // Administrators are always allowed to unpublish metadata + } + if (groupId == null) { + throw new NotAllowedException("Unpublication of metadata is not allowed. Metadata without group owner cannot be unpublished."); + } + Profile defaultProfileForUnpublishing = Profile.Reviewer; + String configuredProfileForUnpublishing = sm.getValue(Settings.METADATA_UNPUBLISH_USERPROFILE); + Profile requiredProfileForUnpublishing; + try { + requiredProfileForUnpublishing = Profile.valueOf(configuredProfileForUnpublishing); + } catch (IllegalArgumentException | NullPointerException e) { + if (e instanceof IllegalArgumentException) { + Log.error(Geonet.SETTINGS, "Invalid profile configured for unpublishing. Using default value: " + defaultProfileForUnpublishing); } + requiredProfileForUnpublishing = defaultProfileForUnpublishing; + } + + boolean canUserUnpublishForGroup = accessManager.isProfileOnGroup(userSession, requiredProfileForUnpublishing, groupId); + + if (!canUserUnpublishForGroup) { + throw new NotAllowedException(String.format( + "Unpublication of metadata is not allowed. User must have the %s profile in the record owner group.", requiredProfileForUnpublishing)); } } @@ -1704,7 +1741,7 @@ private boolean canUserChangePrivilegesForGroup(final ServiceContext context, Gr if (minimumProfileForPrivileges == null) { return true; } else { - return accessManager.isProfileOrMoreOnGroup(context, minimumProfileForPrivileges, group.getId()); + return accessManager.isProfileOrMoreOnGroup(context.getUserSession(), minimumProfileForPrivileges, group.getId()); } } diff --git a/services/src/test/java/org/fao/geonet/api/records/MetadataSharingApiTest.java b/services/src/test/java/org/fao/geonet/api/records/MetadataSharingApiTest.java index 895dafe4b699..29a9dea95eb2 100644 --- a/services/src/test/java/org/fao/geonet/api/records/MetadataSharingApiTest.java +++ b/services/src/test/java/org/fao/geonet/api/records/MetadataSharingApiTest.java @@ -467,4 +467,180 @@ private void createTestData() throws Exception { metadataId = metadata.getId(); metadataUuid = metadata.getUuid(); } + + /** + * Regression test for issue documented in docs/issue-useradmin-publication.md (Symptom 1). + * + *

A user who is {@code UserAdmin} in one group AND has a + * {@code UserGroup(profile=Reviewer)} entry for the metadata's group owner MUST be + * allowed to change reserved-group (publication) privileges. + */ + @Test + public void shareMetadataForPublicationAsUserAdminWithReviewerInGroup() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build(); + + // Create a user who is UserAdmin in a dedicated group (which sets their top-level profile + // to UserAdmin) and also holds Reviewer membership in the record's group owner. + User userAdminWithReviewer = UserRepositoryTest.newUser(_inc); + userAdminWithReviewer.setUsername("useradmin_reviewer"); + userAdminWithReviewer.setProfile(Profile.UserAdmin); + _userRepo.save(userAdminWithReviewer); + grantUserAdminInNewGroup(userAdminWithReviewer, "useradmin-reviewer-admin-group"); + Group sampleGroup = _groupRepo.findById(SAMPLE_GROUP_ID).get(); + _userGroupRepo.save(new UserGroup() + .setGroup(sampleGroup) + .setProfile(Profile.Reviewer) + .setUser(userAdminWithReviewer)); + + MockHttpSession mockHttpSession = loginAs(userAdminWithReviewer); + + checkMetadataHasNoPrivileges(); + + SharingParameter privilegesRequest = createPrivilegesRequest(true); + + Gson gson = new Gson(); + String json = gson.toJson(privilegesRequest); + + mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/sharing") + .session(mockHttpSession) + .content(json) + .contentType(API_JSON_EXPECTED_ENCODING) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isNoContent()); + + List metadataOperations = + operationAllowedRepository.findAllById_MetadataId(metadataId); + boolean hasReservedGroupPrivileges = metadataOperations.stream() + .anyMatch(op -> ReservedGroup.isReserved(op.getId().getGroupId())); + assertTrue( + "UserAdmin who is also per-group Reviewer should be able to publish", + hasReservedGroupPrivileges); + } + + /** + * Back-end side of issue documented in docs/issue-useradmin-publication.md (Symptom 2). + * + *

A user who is {@code UserAdmin} in one group but has NO + * {@code UserGroup(profile=Reviewer)} entry for the metadata's group owner MUST be + * blocked from changing reserved-group (publication) privileges with 403 Forbidden. + */ + @Test + public void shareMetadataForPublicationAsUserAdminWithoutReviewerInGroup() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build(); + + // Create a user who is UserAdmin in a dedicated group (which sets their top-level profile + // to UserAdmin) but only holds Editor membership in the record's group owner. + User userAdminEditorOnly = UserRepositoryTest.newUser(_inc); + userAdminEditorOnly.setUsername("useradmin_editor"); + userAdminEditorOnly.setProfile(Profile.UserAdmin); + _userRepo.save(userAdminEditorOnly); + grantUserAdminInNewGroup(userAdminEditorOnly, "useradmin-editor-admin-group"); + Group sampleGroup = _groupRepo.findById(SAMPLE_GROUP_ID).get(); + _userGroupRepo.save(new UserGroup() + .setGroup(sampleGroup) + .setProfile(Profile.Editor) + .setUser(userAdminEditorOnly)); + + MockHttpSession mockHttpSession = loginAs(userAdminEditorOnly); + + SharingParameter privilegesRequest = createPrivilegesRequest(true); + + Gson gson = new Gson(); + String json = gson.toJson(privilegesRequest); + + mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/sharing") + .session(mockHttpSession) + .content(json) + .contentType(API_JSON_EXPECTED_ENCODING) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isForbidden()); + } + + @Test + public void publishMetadataAsUserAdminWithUserAdminInGroup() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build(); + + User userAdminInGroup = createUserAdminWithGroupProfile("useradmin_group_admin", Profile.UserAdmin); + MockHttpSession mockHttpSession = loginAs(userAdminInGroup); + + mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/publish") + .session(mockHttpSession)) + .andExpect(status().isForbidden()); + } + + @Test + public void unpublishMetadataAsUserAdminWithUserAdminInGroup() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build(); + + publishMetadata(); + + User userAdminInGroup = createUserAdminWithGroupProfile("useradmin_group_admin_unpublish", Profile.UserAdmin); + MockHttpSession mockHttpSession = loginAs(userAdminInGroup); + + mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/unpublish") + .session(mockHttpSession)) + .andExpect(status().isForbidden()); + + List metadataOperations = + operationAllowedRepository.findAllById_MetadataId(metadataId); + boolean hasReservedGroupPrivileges = metadataOperations.stream() + .anyMatch(op -> ReservedGroup.isReserved(op.getId().getGroupId())); + assertTrue("Reserved-group publication privileges should remain set when unpublish is denied", hasReservedGroupPrivileges); + } + + @Test + public void unpublishMetadataAsUserAdminWithoutRequiredGroupProfile() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build(); + + publishMetadata(); + + User userAdminEditorOnly = createUserAdminWithGroupProfile("useradmin_editor_unpublish", Profile.Editor); + MockHttpSession mockHttpSession = loginAs(userAdminEditorOnly); + + mockMvc.perform(put("/srv/api/records/" + metadataUuid + "/unpublish") + .session(mockHttpSession)) + .andExpect(status().isForbidden()); + } + + /** + * Creates a user whose top-level profile is {@code UserAdmin} because they are + * {@code UserAdmin} in a dedicated group, and who additionally holds {@code groupProfile} + * membership in the sample group (group owner of the test record). + * + *

In GeoNetwork, {@code UserAdmin} is always a per-group role – not a standalone + * "global" profile. A user's top-level {@code profile} field simply reflects the highest + * role they hold in any of their groups. By giving the user an explicit + * {@code UserGroup(profile=UserAdmin)} in a separate group we honour that constraint. + */ + private User createUserAdminWithGroupProfile(String username, Profile groupProfile) { + User user = UserRepositoryTest.newUser(_inc); + user.setUsername(username); + user.setProfile(Profile.UserAdmin); + _userRepo.save(user); + + // Give the user UserAdmin membership in a dedicated group so the top-level + // UserAdmin profile is backed by a real per-group assignment. + grantUserAdminInNewGroup(user, username + "-admin-group"); + + Group sampleGroup = _groupRepo.findById(SAMPLE_GROUP_ID).get(); + _userGroupRepo.save(new UserGroup() + .setGroup(sampleGroup) + .setProfile(groupProfile) + .setUser(user)); + + return user; + } + + /** + * Creates a new workspace group with the given name, saves it, and adds a + * {@code UserGroup(profile=UserAdmin)} entry for {@code user} in that group. + * This reflects the real-world constraint that {@code UserAdmin} is a per-group role. + */ + private void grantUserAdminInNewGroup(User user, String groupName) { + Group adminGroup = _groupRepo.save(new Group().setName(groupName)); + _userGroupRepo.save(new UserGroup() + .setGroup(adminGroup) + .setProfile(Profile.UserAdmin) + .setUser(user)); + } } diff --git a/web-ui/src/main/resources/catalog/components/toolbar/ToolbarDirective.js b/web-ui/src/main/resources/catalog/components/toolbar/ToolbarDirective.js index d628bc64428a..41c5bc1c1de9 100644 --- a/web-ui/src/main/resources/catalog/components/toolbar/ToolbarDirective.js +++ b/web-ui/src/main/resources/catalog/components/toolbar/ToolbarDirective.js @@ -281,11 +281,10 @@ scope.displayPublicationOption = function (md, user, pubOption) { return ( md && - md.canReview && md.draft != "y" && md.mdStatus != 3 && - ((md.isPublished(pubOption) && user.canUnpublishMetadata()) || - (!md.isPublished(pubOption) && user.canPublishMetadata())) + ((md.isPublished(pubOption) && user.canUnpublishMetadata(md.groupOwner)) || + (!md.isPublished(pubOption) && user.canPublishMetadata(md.groupOwner))) ); }; diff --git a/web-ui/src/main/resources/catalog/js/CatController.js b/web-ui/src/main/resources/catalog/js/CatController.js index 1a3ea08ae3e6..889cfff37e80 100644 --- a/web-ui/src/main/resources/catalog/js/CatController.js +++ b/web-ui/src/main/resources/catalog/js/CatController.js @@ -2005,23 +2005,29 @@ : ""; return angular.isFunction(this[fnName]) ? this[fnName]() : false; }, - canPublishMetadata: function () { + canPublishMetadata: function (groupId) { var profile = gnConfig["metadata.publication.profilePublishMetadata"] || "Reviewer", fnName = profile !== "" - ? "is" + profile[0].toUpperCase() + profile.substring(1) + "OrMore" + ? "is" + profile[0].toUpperCase() + profile.substring(1) + "ForGroup" : ""; - return angular.isFunction(this[fnName]) ? this[fnName]() : false; + if (groupId === undefined || groupId === null) { + return false; + } + return angular.isFunction(this[fnName]) ? this[fnName](groupId) : false; }, - canUnpublishMetadata: function () { + canUnpublishMetadata: function (groupId) { var profile = gnConfig["metadata.publication.profileUnpublishMetadata"] || "Reviewer", fnName = profile !== "" - ? "is" + profile[0].toUpperCase() + profile.substring(1) + "OrMore" + ? "is" + profile[0].toUpperCase() + profile.substring(1) + "ForGroup" : ""; - return angular.isFunction(this[fnName]) ? this[fnName]() : false; + if (groupId === undefined || groupId === null) { + return false; + } + return angular.isFunction(this[fnName]) ? this[fnName](groupId) : false; }, // The md provide the information about // if the current user can edit records or not diff --git a/web-ui/src/main/resources/catalog/locales/en-admin.json b/web-ui/src/main/resources/catalog/locales/en-admin.json index adad56b1dee8..8ef195e3340a 100644 --- a/web-ui/src/main/resources/catalog/locales/en-admin.json +++ b/web-ui/src/main/resources/catalog/locales/en-admin.json @@ -959,10 +959,10 @@ "metadata/batchediting/accesslevel": "Select the minimum user profile allowed to access batch editing", "metadata/batchediting/accesslevel-help": "Select the minimum user profile allowed to access batch editing (Editor, Reviewer or Administrator). The default value is Editor.", "metadata/publication": "Metadata publication", - "metadata/publication/profilePublishMetadata": "Minimum user profile allowed to publish metadata", - "metadata/publication/profilePublishMetadata-help": "Minimum user profile allowed to publish metadata (Reviewer or Administrator). The default value is Reviewer.", - "metadata/publication/profileUnpublishMetadata": "Minimum user profile allowed to un-publish metadata", - "metadata/publication/profileUnpublishMetadata-help": "Minimum user profile allowed to un-publish metadata (Reviewer or Administrator). The default value is Reviewer.", + "metadata/publication/profilePublishMetadata": "Required profile to publish metadata", + "metadata/publication/profilePublishMetadata-help": "Profile required to publish metadata, evaluated in the record owner group (Reviewer or Administrator). The default value is Reviewer.", + "metadata/publication/profileUnpublishMetadata": "Required profile to un-publish metadata", + "metadata/publication/profileUnpublishMetadata-help": "Profile required to un-publish metadata, evaluated in the record owner group (Reviewer or Administrator). The default value is Reviewer.", "metadata/history": "Metadata History", "metadata/history/accesslevel": "Select the minimum user profile allowed to view metadata history", "metadata/history/accesslevel-help": "Select the user profile allowed to view metadata history (Registered User, Editor or Administrator). The Registered User configuration can view the history with view permission granted to the metadata record. The Editor configuration can view the history with editing permission granted to the metadata record. The default value is Editor.",