From 18b00b351883a0b119f3d28722b0dd3f5ce2c223 Mon Sep 17 00:00:00 2001 From: tylerjmchugh <163562062+tylerjmchugh@users.noreply.github.com> Date: Tue, 10 Dec 2024 10:12:50 -0500 Subject: [PATCH] Restrict setting privileges on groups (#8511) * Add new minimumProfileForPrivileges property to groups * Disable the group in privileges UI if missing required profile * Restrict setting permissions for groups with a minimumProfileForPrivileges * Documentation * Update documentation * retrigger checks * Remove special case for groups that own the record * Update help text for clarity * Update formatting --- .../java/jeeves/component/ProfileManager.java | 6 +- .../org/fao/geonet/kernel/AccessManager.java | 31 ++++++++++ .../datamanager/IMetadataOperations.java | 10 +++ .../base/BaseMetadataOperations.java | 12 ++++ .../java/jeeves/interfaces/ProfileTest.java | 53 +++++++++++----- .../creating-group.md | 31 ++++++++-- .../java/org/fao/geonet/domain/Group.java | 23 +++++++ .../java/org/fao/geonet/domain/Profile.java | 61 +++++++++++++++---- .../geonet/repository/GroupRepository.java | 8 +++ .../specification/UserGroupSpecs.java | 24 ++++++++ .../repository/GroupRepositoryTest.java | 15 +++++ .../specification/UserGroupSpecsTest.java | 20 ++++++ .../api/records/MetadataSharingApi.java | 39 +++++++++++- .../api/records/model/GroupPrivilege.java | 5 ++ .../org/fao/geonet/api/users/UsersApi.java | 4 +- .../api/users/transfer/OwnershipUtils.java | 2 +- .../components/common/share/ShareService.js | 5 +- .../common/share/partials/panel.html | 4 +- .../catalog/js/admin/UserGroupController.js | 4 ++ .../resources/catalog/locales/en-admin.json | 3 + .../templates/admin/usergroup/groups.html | 18 ++++++ 21 files changed, 337 insertions(+), 41 deletions(-) diff --git a/core/src/main/java/jeeves/component/ProfileManager.java b/core/src/main/java/jeeves/component/ProfileManager.java index 3e93896722b..25ec6e179fd 100644 --- a/core/src/main/java/jeeves/component/ProfileManager.java +++ b/core/src/main/java/jeeves/component/ProfileManager.java @@ -65,11 +65,11 @@ public class ProfileManager { */ public static Profile getLowestProfile(String[] profiles) { Profile lowestProfile = null; - int numberOfProfilesExtended = Profile.Administrator.getAll().size(); + int numberOfProfilesExtended = Profile.Administrator.getProfileAndAllChildren().size(); for (String profileName : profiles) { Profile p = Profile.valueOf(profileName); - Set currentProfileSet = p.getAll(); + Set currentProfileSet = p.getProfileAndAllChildren(); if (currentProfileSet.size() < numberOfProfilesExtended) { lowestProfile = p; numberOfProfilesExtended = currentProfileSet.size(); @@ -89,7 +89,7 @@ public static Profile getHighestProfile(Profile[] profiles) { int numberOfProfilesExtended = 0; for (Profile profile : profiles) { - Set all = profile.getAll(); + Set all = profile.getProfileAndAllChildren(); if (all.size() > numberOfProfilesExtended) { highestProfile = profile; numberOfProfilesExtended = all.size(); 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 4c8820b2488..3d9e77f356b 100644 --- a/core/src/main/java/org/fao/geonet/kernel/AccessManager.java +++ b/core/src/main/java/org/fao/geonet/kernel/AccessManager.java @@ -509,6 +509,37 @@ 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 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)) { + return false; + } + + // Grant access if the user is a global administrator + if (Profile.Administrator == us.getProfile()) { + return true; + } + + // Get the profile and all its parent profiles to consider higher-level permissions + 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())) + .and(UserGroupSpecs.hasGroupId(groupId)) + .and(UserGroupSpecs.hasProfileIn(acceptedProfiles)); + List userGroups = userGroupRepository.findAll(spec); + + return !userGroups.isEmpty(); + } + public int getPrivilegeId(final String name) { final Operation op = operationRepository.findByName(name); if (op == null) { diff --git a/core/src/main/java/org/fao/geonet/kernel/datamanager/IMetadataOperations.java b/core/src/main/java/org/fao/geonet/kernel/datamanager/IMetadataOperations.java index b2411bb2ca8..46dc7677973 100644 --- a/core/src/main/java/org/fao/geonet/kernel/datamanager/IMetadataOperations.java +++ b/core/src/main/java/org/fao/geonet/kernel/datamanager/IMetadataOperations.java @@ -24,6 +24,7 @@ package org.fao.geonet.kernel.datamanager; import java.util.Collection; +import java.util.List; import org.fao.geonet.domain.OperationAllowed; import org.fao.geonet.domain.ReservedOperation; @@ -52,6 +53,15 @@ public interface IMetadataOperations { */ void deleteMetadataOper(String metadataId, boolean skipAllReservedGroup) throws Exception; + /** + * Removes all operations stored for a metadata except for the operations of the groups in the exclude list. + * Used for preventing deletion of operations for reserved and restricted groups. + * + * @param metadataId Metadata identifier + * @param groupIdsToExclude List of group ids to exclude from deletion + */ + void deleteMetadataOper(String metadataId, List groupIdsToExclude); + /** * Adds a permission to a group. Metadata is not reindexed. */ diff --git a/core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataOperations.java b/core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataOperations.java index 47cfe3b5342..526c1d65c38 100644 --- a/core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataOperations.java +++ b/core/src/main/java/org/fao/geonet/kernel/datamanager/base/BaseMetadataOperations.java @@ -116,6 +116,18 @@ public void deleteMetadataOper(String metadataId, boolean skipAllReservedGroup) } } + /** + * Removes all operations stored for a metadata except for the operations of the groups in the exclude list. + * Used for preventing deletion of operations for reserved and restricted groups. + * + * @param metadataId Metadata identifier + * @param groupIdsToExclude List of group ids to exclude from deletion + */ + @Override + public void deleteMetadataOper(String metadataId, List groupIdsToExclude) { + opAllowedRepo.deleteAllByMetadataIdExceptGroupId(Integer.parseInt(metadataId), groupIdsToExclude); + } + /** * Adds a permission to a group. Metadata is not reindexed. */ diff --git a/core/src/test/java/jeeves/interfaces/ProfileTest.java b/core/src/test/java/jeeves/interfaces/ProfileTest.java index 0e360641f77..b52042cc58e 100644 --- a/core/src/test/java/jeeves/interfaces/ProfileTest.java +++ b/core/src/test/java/jeeves/interfaces/ProfileTest.java @@ -34,17 +34,27 @@ public class ProfileTest { + @Test + public void testGetChildren() { + assertContainsAllExactly(Administrator.getChildren(), UserAdmin, Monitor); + assertContainsOnly(Reviewer, UserAdmin.getChildren()); + assertContainsOnly(Editor, Reviewer.getChildren()); + assertContainsOnly(RegisteredUser, Editor.getChildren()); + assertContainsOnly(Guest, RegisteredUser.getChildren()); + assertEquals(0, Monitor.getChildren().size()); + assertEquals(0, Guest.getChildren().size()); + } + @Test public void testGetParents() { - assertEquals(2, Administrator.getParents().size()); - assertTrue(Administrator.getParents().contains(UserAdmin)); - assertTrue(Administrator.getParents().contains(Monitor)); - assertContainsOnly(Reviewer, UserAdmin.getParents()); - assertContainsOnly(Editor, Reviewer.getParents()); - assertContainsOnly(RegisteredUser, Editor.getParents()); - assertContainsOnly(Guest, RegisteredUser.getParents()); - assertEquals(0, Monitor.getParents().size()); - assertEquals(0, Guest.getParents().size()); + assertEquals(0, Administrator.getParents().size()); + assertContainsOnly(Administrator, UserAdmin.getParents()); + assertContainsOnly(UserAdmin, Reviewer.getParents()); + assertContainsOnly(Reviewer, Editor.getParents()); + assertContainsOnly(Editor, RegisteredUser.getParents()); + assertContainsOnly(RegisteredUser, Guest.getParents()); + assertContainsOnly(Administrator, Monitor.getParents()); + } private void assertContainsOnly(Profile profile, Set parents) { @@ -53,12 +63,25 @@ private void assertContainsOnly(Profile profile, Set parents) { } @Test - public void testGetAll() { - assertContainsAllExactly(Administrator.getAll(), Administrator, UserAdmin, Reviewer, Editor, RegisteredUser, Guest, Monitor); - assertContainsAllExactly(UserAdmin.getAll(), UserAdmin, Reviewer, Editor, RegisteredUser, Guest); - assertContainsAllExactly(Reviewer.getAll(), Reviewer, Editor, RegisteredUser, Guest); - assertContainsAllExactly(Editor.getAll(), Editor, RegisteredUser, Guest); - assertContainsAllExactly(Editor.getAll(), Editor, RegisteredUser, Guest); + public void testGetProfileAndAllChildren() { + assertContainsAllExactly(Administrator.getProfileAndAllChildren(), Administrator, UserAdmin, Reviewer, Editor, RegisteredUser, Guest, Monitor); + assertContainsAllExactly(UserAdmin.getProfileAndAllChildren(), UserAdmin, Reviewer, Editor, RegisteredUser, Guest); + assertContainsAllExactly(Reviewer.getProfileAndAllChildren(), Reviewer, Editor, RegisteredUser, Guest); + assertContainsAllExactly(Editor.getProfileAndAllChildren(), Editor, RegisteredUser, Guest); + assertContainsAllExactly(RegisteredUser.getProfileAndAllChildren(), RegisteredUser, Guest); + assertContainsAllExactly(Guest.getProfileAndAllChildren(), Guest); + assertContainsAllExactly(Monitor.getProfileAndAllChildren(), Monitor); + } + + @Test + public void testGetProfileAndAllParents() { + assertContainsAllExactly(Administrator.getProfileAndAllParents(), Administrator); + assertContainsAllExactly(UserAdmin.getProfileAndAllParents(), UserAdmin, Administrator); + assertContainsAllExactly(Reviewer.getProfileAndAllParents(), Reviewer, UserAdmin, Administrator); + assertContainsAllExactly(Editor.getProfileAndAllParents(), Editor, Reviewer, UserAdmin, Administrator); + assertContainsAllExactly(RegisteredUser.getProfileAndAllParents(), RegisteredUser, Editor, Reviewer, UserAdmin, Administrator); + assertContainsAllExactly(Guest.getProfileAndAllParents(), Guest, RegisteredUser, Editor, Reviewer, UserAdmin, Administrator); + assertContainsAllExactly(Monitor.getProfileAndAllParents(), Monitor, Administrator); } private void assertContainsAllExactly(Set all, Profile... profiles) { diff --git a/docs/manual/docs/administrator-guide/managing-users-and-groups/creating-group.md b/docs/manual/docs/administrator-guide/managing-users-and-groups/creating-group.md index 857df1f970d..650d1e9d62e 100644 --- a/docs/manual/docs/administrator-guide/managing-users-and-groups/creating-group.md +++ b/docs/manual/docs/administrator-guide/managing-users-and-groups/creating-group.md @@ -17,12 +17,33 @@ To create new groups you should be logged on with an account that has administra 4. Click *Save* -Access privileges can be set per metadata record. You can define privileges on a per Group basis. +## Access privileges -Privileges that can be set relate to visibility of the Metadata (*Publish*), data *Download*, *Interactive Map* access and display of the record in the *Featured* section of the home page. +Access privileges can be set on a per-metadata-record basis. Privileges define which actions are available to users in the group: -*Editing* defines the groups for which editors can edit the metadata record. +- **Publish**: Controls visibility of the metadata. +- **Download**: Grants access to data downloads. +- **Interactive Map**: Provides access to map tools. +- **Featured**: Displays the record in the *Featured* section on the home page. -*Notify* defines what Groups are notified when a file managed by GeoNetwork is downloaded. +Additional settings: +- **Editing**: Specifies which groups can edit the metadata record. +- **Notify**: Determines which groups are notified when a file managed by GeoNetwork is downloaded. -Below is an example of the privileges management table related to a dataset. +## Minimum user profile allowed to set privileges + +This setting allows administrators to control the minimum user profile required to assign privileges for a group. It provides enhanced control over who can manage sensitive privileges for users within the group. + +### Default setting + +By default, the **"Minimum User Profile Allowed to Set Privileges"** is set to **No Restrictions**. This means that any user with permission to manage privileges for a metadata record can assign privileges for users in this group. + +### Restricted setting + +When a specific profile is selected, only users with that profile or higher within the group can assign privileges. Users with lower profiles will have **read-only** access to privilege settings for this group. + +### Example usage + +If a group has **"Minimum User Profile Allowed to Set Privileges"** set to **Reviewer**: +- Only users with the **Reviewer** profile or higher (e.g., **Administrator**) can assign privileges for users in this group. +- Users with profiles below **Reviewer** (e.g., **Editor**) will see the group as **read-only** in the privileges interface. diff --git a/domain/src/main/java/org/fao/geonet/domain/Group.java b/domain/src/main/java/org/fao/geonet/domain/Group.java index 19fe92d6eb7..ccab64a875f 100644 --- a/domain/src/main/java/org/fao/geonet/domain/Group.java +++ b/domain/src/main/java/org/fao/geonet/domain/Group.java @@ -41,6 +41,8 @@ import javax.persistence.ElementCollection; import javax.persistence.Entity; import javax.persistence.EntityListeners; +import javax.persistence.Enumerated; +import javax.persistence.EnumType; import javax.persistence.FetchType; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; @@ -84,6 +86,7 @@ public class Group extends Localized implements Serializable { private MetadataCategory defaultCategory; private List allowedCategories; private Boolean enableAllowedCategories; + private Profile minimumProfileForPrivileges; /** * Get the id of the group. @@ -348,4 +351,24 @@ public Group setEnableAllowedCategories(Boolean enableAllowedCategories) { this.enableAllowedCategories = enableAllowedCategories; return this; } + + /** + * Get the minimum profile required to update privileges for this group. + * + * @return {@link Profile} the minimum profile required to update privileges for this group. + */ + @Enumerated(EnumType.STRING) + public Profile getMinimumProfileForPrivileges() { + return minimumProfileForPrivileges; + } + + /** + * Set the minimum profile required to update privileges for this group. + * @param minimumProfileForPrivileges the minimum {@link Profile} required to update privileges for this group. + * @return this group entity object. + */ + public Group setMinimumProfileForPrivileges(Profile minimumProfileForPrivileges) { + this.minimumProfileForPrivileges = minimumProfileForPrivileges; + return this; + } } diff --git a/domain/src/main/java/org/fao/geonet/domain/Profile.java b/domain/src/main/java/org/fao/geonet/domain/Profile.java index e3a24e71074..b6e680d0ee0 100644 --- a/domain/src/main/java/org/fao/geonet/domain/Profile.java +++ b/domain/src/main/java/org/fao/geonet/domain/Profile.java @@ -73,31 +73,70 @@ public static Profile findProfileIgnoreCase(String profileName) { return null; } - public Set getParents() { - HashSet parents = new HashSet(); + /** + * Retrieves all direct child profiles of the current profile. + * Child profiles have fewer permissions than parents. + * + * @return A set containing profiles that have this profile as a parent. + */ + public Set getChildren() { + HashSet children = new HashSet(); for (Profile profile : values()) { if (profile.parents.contains(this)) { - parents.add(profile); + children.add(profile); } } + return children; + } + + /** + * Retrieves the direct parent profiles of the current profile. + * Parent profiles have more permissions than children. + * + * @return A set of profiles that are direct parents of this profile. + */ + public Set getParents() { return parents; } - public Set getAll() { - HashSet all = new HashSet(); - all.add(this); - for (Profile parent : getParents()) { - all.addAll(parent.getAll()); + /** + * Retrieves the profile and all of its children recursively. + * The returned set will include the profile itself. + * Child profiles have fewer permissions than parents. + * + * @return A {@link Set} containing the profile and all of its children. + */ + public Set getProfileAndAllChildren() { + HashSet profiles = new HashSet(); + profiles.add(this); + for (Profile child : getChildren()) { + profiles.addAll(child.getProfileAndAllChildren()); } - return all; + return profiles; + } + + /** + * Retrieves the profile and all of its parents recursively. + * The returned set will include the profile itself. + * Parent profiles have more permissions than children. + * + * @return A {@link Set} containing the profile and all of its parents. + */ + public Set getProfileAndAllParents() { + Set profiles = new HashSet<>(); + profiles.add(this); + for (Profile parent : getParents()) { + profiles.addAll(parent.getProfileAndAllParents()); + } + return profiles; } public Element asElement() { Element elResult = new Element(PROFILES_ELEM_NAME); - for (Profile profile : getAll()) { + for (Profile profile : getProfileAndAllChildren()) { if (profile == Guest) continue; @@ -109,7 +148,7 @@ public Element asElement() { public Set getAllNames() { HashSet names = new HashSet(); - for (Profile p : getAll()) { + for (Profile p : getProfileAndAllChildren()) { names.add(p.name()); } return names; diff --git a/domain/src/main/java/org/fao/geonet/repository/GroupRepository.java b/domain/src/main/java/org/fao/geonet/repository/GroupRepository.java index 617de58a3dc..5faa3f84f12 100644 --- a/domain/src/main/java/org/fao/geonet/repository/GroupRepository.java +++ b/domain/src/main/java/org/fao/geonet/repository/GroupRepository.java @@ -52,6 +52,14 @@ public interface GroupRepository extends GeonetRepository, Group @Nullable Group findByEmail(@Nonnull String email); + /** + * Find all groups with a minimumProfileForPrivileges not equal to null. + * These groups are "restricted". + * + * @return a list of groups with a minimumProfileForPrivileges not equal to null + */ + @Nullable + List findByMinimumProfileForPrivilegesNotNull(); public @Nullable diff --git a/domain/src/main/java/org/fao/geonet/repository/specification/UserGroupSpecs.java b/domain/src/main/java/org/fao/geonet/repository/specification/UserGroupSpecs.java index 5d6c5d26568..b07e3304dd4 100644 --- a/domain/src/main/java/org/fao/geonet/repository/specification/UserGroupSpecs.java +++ b/domain/src/main/java/org/fao/geonet/repository/specification/UserGroupSpecs.java @@ -38,6 +38,7 @@ import javax.persistence.criteria.Root; import java.util.HashSet; import java.util.List; +import java.util.Set; public final class UserGroupSpecs { @@ -84,6 +85,12 @@ public Predicate toPredicate(Root root, CriteriaQuery query, Crite }; } + /** + * Specification for retrieving all the userGroups with a given profile. + * + * @param profile The {@link Profile} to filter the userGroups. + * @return the query. + */ public static Specification hasProfile(final Profile profile) { return new Specification() { @Override @@ -95,6 +102,23 @@ public Predicate toPredicate(Root root, CriteriaQuery query, Crite }; } + /** + * Specification for retrieving all the userGroups with a {@link Profile} in a given set of profiles. + * + * @param profiles The {@link Set} of {@link Profile} to filter the userGroups. + * @return the query. + */ + public static Specification hasProfileIn(final Set profiles) { + return new Specification() { + @Override + public Predicate toPredicate(Root root, CriteriaQuery query, CriteriaBuilder cb) { + Path profileIdAttributePath = root.get(UserGroup_.id).get(UserGroupId_.profile); + Predicate profileIdInPredicate = profileIdAttributePath.in(profiles); + return profileIdInPredicate; + } + }; + } + /** * Specification for retrieving all the userGroups of a given user with a given profile. * diff --git a/domain/src/test/java/org/fao/geonet/repository/GroupRepositoryTest.java b/domain/src/test/java/org/fao/geonet/repository/GroupRepositoryTest.java index a7be16cc822..f45f0b4c1bc 100644 --- a/domain/src/test/java/org/fao/geonet/repository/GroupRepositoryTest.java +++ b/domain/src/test/java/org/fao/geonet/repository/GroupRepositoryTest.java @@ -25,6 +25,7 @@ import org.fao.geonet.domain.Group; +import org.fao.geonet.domain.Profile; import org.fao.geonet.domain.ReservedGroup; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -131,6 +132,20 @@ public void testFindByEmail() throws Exception { assertNull(_repo.findByEmail("some wrong email")); } + @Test + public void testFindByMinimumProfileForPrivilegesNotNull() throws Exception { + Group savedGroup = _repo.save(newGroup().setMinimumProfileForPrivileges(Profile.Reviewer)); + Group savedGroup2 = _repo.save(newGroup()); + + _repo.flush(); + _entityManager.flush(); + _entityManager.clear(); + + List groups = _repo.findByMinimumProfileForPrivilegesNotNull(); + assertEquals(1, groups.size()); + assertSameContents(savedGroup, groups.get(0)); + } + @Test public void testFindReservedGroup() throws Exception { Group savedGroup = _repo.save(ReservedGroup.all.getGroupEntityTemplate()); diff --git a/domain/src/test/java/org/fao/geonet/repository/specification/UserGroupSpecsTest.java b/domain/src/test/java/org/fao/geonet/repository/specification/UserGroupSpecsTest.java index 3a2af23e22c..ba6e2ebcd6d 100644 --- a/domain/src/test/java/org/fao/geonet/repository/specification/UserGroupSpecsTest.java +++ b/domain/src/test/java/org/fao/geonet/repository/specification/UserGroupSpecsTest.java @@ -23,6 +23,7 @@ package org.fao.geonet.repository.specification; +import org.fao.geonet.domain.Profile; import org.fao.geonet.domain.ReservedGroup; import org.fao.geonet.domain.UserGroup; import org.fao.geonet.repository.*; @@ -34,6 +35,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import static org.fao.geonet.repository.specification.UserGroupSpecs.*; @@ -89,6 +91,24 @@ public void testHasProfile() throws Exception { } } + @Test + public void testHasProfileIn() throws Exception { + UserGroup ug1 = newUserGroup(); + ug1.setProfile(Profile.Reviewer); + _repo.save(ug1); + _repo.save(newUserGroup()); + _repo.save(newUserGroup()); + _repo.save(newUserGroup()); + + Set profiles = Set.of(Profile.Editor, Profile.Reviewer); + + List found = _repo.findAll(hasProfileIn(profiles)); + + for (UserGroup userGroup : found) { + assertTrue(profiles.contains(userGroup.getProfile())); + } + } + @Test public void testIsReservedGroup() throws Exception { UserGroup ug1 = _repo.save(newUserGroup()); 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 5967992f740..8d81e77c7f3 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 @@ -134,6 +134,9 @@ public class MetadataSharingApi implements ApplicationEventPublisherAware @Autowired IMetadataManager metadataManager; + @Autowired + IMetadataOperations metadataOperations; + @Autowired MetadataValidationRepository metadataValidationRepository; @@ -562,8 +565,24 @@ private void setOperations( // Check if the user profile can change the privileges for publication/un-publication of the reserved groups checkChangesAllowedToUserProfileForReservedGroups(context.getUserSession(), 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 + for (Group group: groupRepository.findByMinimumProfileForPrivilegesNotNull()) { + if (!canUserChangePrivilegesForGroup(context, group)) { + excludeFromDelete.add(group.getId()); + } + } + + // Exclude deleting privileges for reserved groups if the skipAllReservedGroup flag is set + if (skipAllReservedGroup) { + excludeFromDelete.add(ReservedGroup.all.getId()); + excludeFromDelete.add(ReservedGroup.intranet.getId()); + excludeFromDelete.add(ReservedGroup.guest.getId()); + } + if (sharing.isClear()) { - dataManager.deleteMetadataOper(context, String.valueOf(metadata.getId()), skipAllReservedGroup); + metadataOperations.deleteMetadataOper(String.valueOf(metadata.getId()), excludeFromDelete); } for (GroupOperations p : privileges) { @@ -742,6 +761,8 @@ public SharingResponse getRecordSharingSettings( } groupPrivilege.setUserProfile(userGroupProfile); + // Restrict changing privileges for groups with a minimum profile for setting privileges set + groupPrivilege.setRestricted(!canUserChangePrivilegesForGroup(context, g)); //--- get all operations that this group can do on given metadata Specification hasGroupIdAndMetadataId = @@ -1474,6 +1495,22 @@ private void checkUserProfileToUnpublishMetadata(UserSession userSession) { } } + /** + * Checks if the user can change the privileges for the group. + * + * @param context The {@link ServiceContext} object. + * @param group The {@link Group} to change the privileges for. + * @return True if the user can change the privileges for the group, false otherwise. + */ + private boolean canUserChangePrivilegesForGroup(final ServiceContext context, Group group) { + Profile minimumProfileForPrivileges = group.getMinimumProfileForPrivileges(); + if (minimumProfileForPrivileges == null) { + return true; + } else { + return accessManager.isProfileOrMoreOnGroup(context, minimumProfileForPrivileges, group.getId()); + } + } + /** * Returns the list of privilege changes for the reserved groups. * diff --git a/services/src/main/java/org/fao/geonet/api/records/model/GroupPrivilege.java b/services/src/main/java/org/fao/geonet/api/records/model/GroupPrivilege.java index ae5de2fa1d5..289ad3c58c6 100644 --- a/services/src/main/java/org/fao/geonet/api/records/model/GroupPrivilege.java +++ b/services/src/main/java/org/fao/geonet/api/records/model/GroupPrivilege.java @@ -33,6 +33,7 @@ public class GroupPrivilege extends GroupOperations { private List userProfiles; private boolean userGroup; private boolean reserved; + private boolean restricted; public List getUserProfiles() { return userProfiles; @@ -57,4 +58,8 @@ public boolean isReserved() { public void setReserved(boolean reserved) { this.reserved = reserved; } + + public boolean isRestricted() { return restricted; } + + public void setRestricted(boolean restricted) { this.restricted = restricted; } } diff --git a/services/src/main/java/org/fao/geonet/api/users/UsersApi.java b/services/src/main/java/org/fao/geonet/api/users/UsersApi.java index ef49de62a21..c6911f9f2bf 100644 --- a/services/src/main/java/org/fao/geonet/api/users/UsersApi.java +++ b/services/src/main/java/org/fao/geonet/api/users/UsersApi.java @@ -442,7 +442,7 @@ public ResponseEntity createUser( // TODO: CheckAccessRights - if (!myProfile.getAll().contains(profile)) { + if (!myProfile.getProfileAndAllChildren().contains(profile)) { throw new IllegalArgumentException( "Trying to set profile to " + profile + " max profile permitted is: " + myProfile); @@ -553,7 +553,7 @@ public ResponseEntity updateUser( } - if (!myProfile.getAll().contains(profile)) { + if (!myProfile.getProfileAndAllChildren().contains(profile)) { throw new IllegalArgumentException( "Trying to set profile to " + profile + " max profile permitted is: " + myProfile); diff --git a/services/src/main/java/org/fao/geonet/api/users/transfer/OwnershipUtils.java b/services/src/main/java/org/fao/geonet/api/users/transfer/OwnershipUtils.java index 70c95f1b8d2..168dc816eb9 100644 --- a/services/src/main/java/org/fao/geonet/api/users/transfer/OwnershipUtils.java +++ b/services/src/main/java/org/fao/geonet/api/users/transfer/OwnershipUtils.java @@ -86,7 +86,7 @@ public Element apply(@Nonnull User input) { Set hsMyGroups = getUserGroups(context, id); - Set profileSet = us.getProfile().getAll(); + Set profileSet = us.getProfile().getProfileAndAllChildren(); //--- now filter them diff --git a/web-ui/src/main/resources/catalog/components/common/share/ShareService.js b/web-ui/src/main/resources/catalog/components/common/share/ShareService.js index 4ecd86461ff..0b7fed29f80 100644 --- a/web-ui/src/main/resources/catalog/components/common/share/ShareService.js +++ b/web-ui/src/main/resources/catalog/components/common/share/ShareService.js @@ -245,9 +245,10 @@ // Do not submit internal groups info // If user is not allowed. var allowed = - ($.inArray(g.group, gnShareConstants.internalGroups) !== -1 && + !g.restricted && + (($.inArray(g.group, gnShareConstants.internalGroups) !== -1 && user.isReviewerOrMore()) || - $.inArray(g.group, gnShareConstants.internalGroups) === -1; + $.inArray(g.group, gnShareConstants.internalGroups) === -1); if (allowed) { ops.push({ diff --git a/web-ui/src/main/resources/catalog/components/common/share/partials/panel.html b/web-ui/src/main/resources/catalog/components/common/share/partials/panel.html index 2af0e27141f..c595acf81f0 100644 --- a/web-ui/src/main/resources/catalog/components/common/share/partials/panel.html +++ b/web-ui/src/main/resources/catalog/components/common/share/partials/panel.html @@ -116,6 +116,7 @@
whoCanAccess
data-ng-repeat="g in privileges | filter:{reserved: false} | filter:pFilterFn | orderBy:sortGroups:sorter.reverse" data-ng-show="(onlyUserGroup == true && g.userGroup == true) || onlyUserGroup == false" + data-ng-class="g.restricted ? 'warning' : ''" > {{::('group-' + g.group) | translate}} @@ -123,7 +124,7 @@
whoCanAccess
type="checkbox" name="{{g.group + '-' + key}}" data-ng-click="selectPrivilege()" - data-ng-disabled="g.privileges[key].disabled" + data-ng-disabled="g.privileges[key].disabled || g.restricted" data-ng-model="g.operations[key]" /> whoCanAccess type="checkbox" data-ng-change="checkAll(g)" data-ng-model="g.isCheckedAll" + data-ng-disabled="g.restricted" /> diff --git a/web-ui/src/main/resources/catalog/js/admin/UserGroupController.js b/web-ui/src/main/resources/catalog/js/admin/UserGroupController.js index a7417875a28..025048896f3 100644 --- a/web-ui/src/main/resources/catalog/js/admin/UserGroupController.js +++ b/web-ui/src/main/resources/catalog/js/admin/UserGroupController.js @@ -711,6 +711,10 @@ ) { $scope.groupSelected.defaultCategory = null; } + // Ensure that the minimum profile for privileges is not an empty string + if ($scope.groupSelected.minimumProfileForPrivileges == "") { + $scope.groupSelected.minimumProfileForPrivileges = null; + } $http .put( "../api/groups" + 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 80aa1114b60..8dce8d1887c 100644 --- a/web-ui/src/main/resources/catalog/locales/en-admin.json +++ b/web-ui/src/main/resources/catalog/locales/en-admin.json @@ -475,6 +475,7 @@ "newUser": "New user ", "noCommitAvailable": "No activity recorded. Enable versioning on metadata records to follow changes.", "noHarvesterHistory": "No harvester history available.", + "noRestrictions": "No restrictions", "nostatus": "No status", "notYetSupported": "Not supported yet", "numberOfSearch": "Number of searches", @@ -888,6 +889,8 @@ "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.", + "minimumProfileForPrivileges": "Minimum user profile allowed to set privileges", + "minimumProfileForPrivilegesHelp": "Specifies the lowest user profile needed within a group to assign privileges for that group on a record. Users below this profile cannot set privileges for the group, and the group will be disabled in the privilege assignment interface. Default: No Restrictions (any editor for a record can set privileges for this group).", "filterStatusByAuthor":"Status author", "filterStatusByOwner":"Status owner", "filterStatusByRecordId":"Record identifier", diff --git a/web-ui/src/main/resources/catalog/templates/admin/usergroup/groups.html b/web-ui/src/main/resources/catalog/templates/admin/usergroup/groups.html index 61741b2208d..602b99728a8 100644 --- a/web-ui/src/main/resources/catalog/templates/admin/usergroup/groups.html +++ b/web-ui/src/main/resources/catalog/templates/admin/usergroup/groups.html @@ -259,6 +259,24 @@ +
+ + +

minimumProfileForPrivilegesHelp

+