[Backport 4.2.x] Restrict setting privileges on groups (#8511) (#8553)

* 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 Co-authored-by: tylerjmchugh <[email protected]>
geonetwork · Dec 10, 2024 · d3067ba · d3067ba
1 parent b243377
commit d3067ba
Show file tree

Hide file tree

Showing 21 changed files with 339 additions and 41 deletions.
diff --git 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<Profile> currentProfileSet = p.getAll();
+            Set<Profile> 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<Profile> all = profile.getAll();
+            Set<Profile> 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
@@ -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<Profile> acceptedProfiles = profile.getProfileAndAllParents();
+
+        // Build a specification to fetch any accepted profiles for the user in the specified group
+        Specification<UserGroup> spec = Specification.where(UserGroupSpecs.hasUserId(us.getUserIdAsInt()))
+            .and(UserGroupSpecs.hasGroupId(groupId))
+            .and(UserGroupSpecs.hasProfileIn(acceptedProfiles));
+        List<UserGroup> 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
@@ -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<Integer> 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
@@ -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<Integer> 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
@@ -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<Profile> parents) {
@@ -53,12 +63,25 @@ private void assertContainsOnly(Profile profile, Set<Profile> 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<Profile> 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
@@ -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
@@ -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<MetadataCategory> 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
@@ -73,31 +73,70 @@ public static Profile findProfileIgnoreCase(String profileName) {
         return null;
     }
 
-    public Set<Profile> getParents() {
-        HashSet<Profile> parents = new HashSet<Profile>();
+    /**
+     * 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<Profile> getChildren() {
+        HashSet<Profile> children = new HashSet<Profile>();
         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<Profile> getParents() {
         return parents;
     }
 
-    public Set<Profile> getAll() {
-        HashSet<Profile> all = new HashSet<Profile>();
-        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<Profile>} containing the profile and all of its children.
+     */
+    public Set<Profile> getProfileAndAllChildren() {
+        HashSet<Profile> profiles = new HashSet<Profile>();
+        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<Profile>} containing the profile and all of its parents.
+     */
+    public Set<Profile> getProfileAndAllParents() {
+        Set<Profile> 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<String> getAllNames() {
         HashSet<String> names = new HashSet<String>();
-        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
@@ -52,6 +52,14 @@ public interface GroupRepository extends GeonetRepository<Group, Integer>, 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<Group> findByMinimumProfileForPrivilegesNotNull();
 
     public
     @Nullable