Skip to content

Commit

Permalink
[Backport 4.2.x] Restrict setting privileges on groups (#8511)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
tylerjmchugh authored and ianwallen committed Dec 10, 2024
1 parent 89adb8c commit 158373c
Show file tree
Hide file tree
Showing 21 changed files with 339 additions and 41 deletions.
6 changes: 3 additions & 3 deletions core/src/main/java/jeeves/component/ProfileManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
31 changes: 31 additions & 0 deletions core/src/main/java/org/fao/geonet/kernel/AccessManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
53 changes: 38 additions & 15 deletions core/src/test/java/jeeves/interfaces/ProfileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
23 changes: 23 additions & 0 deletions domain/src/main/java/org/fao/geonet/domain/Group.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}
61 changes: 50 additions & 11 deletions domain/src/main/java/org/fao/geonet/domain/Profile.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 158373c

Please sign in to comment.