Skip to content

Commit

Permalink
Merge pull request #488 from AAFC-BICoE/35738_Add_admin-based_concept…
Browse files Browse the repository at this point in the history
…_to_DinaRole

35738 add admin based concept to dina role
  • Loading branch information
brandonandre authored Feb 14, 2025
2 parents d3d5115 + cad2c62 commit 61cb47e
Show file tree
Hide file tree
Showing 12 changed files with 161 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ca.gc.aafc.dina.config;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand All @@ -22,6 +23,8 @@
@Setter
public class DevSettings {

private Set<String> adminRole;

private Map<String, Set<String>> groupRole;

public Map<String, Set<DinaRole>> getRolesPerGroup() {
Expand All @@ -38,8 +41,21 @@ public Map<String, Set<DinaRole>> getRolesPerGroup() {
.map(Optional::orElseThrow).collect(
Collectors.toSet()));
}

return groupDinaRole;
}

public Set<DinaRole> getAdminRoles() {
if (adminRole == null || adminRole.isEmpty()) {
return Set.of();
}

Set<DinaRole> adminDinaRole = new HashSet<>(adminRole.size());
for (String entry : adminRole) {
DinaRole dr = DinaRole.fromString(entry).orElseThrow();
if (dr.isAdminBased()) {
adminDinaRole.add(dr);
}
}
return adminDinaRole;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.Set;
import lombok.Getter;

import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
Expand Down Expand Up @@ -40,6 +41,10 @@ public Map<String, Set<DinaRole>> getRolesPerGroup() {
return devSettings.getRolesPerGroup();
}

public Set<DinaRole> getAdminRoles() {
return devSettings.getAdminRoles();
}

/**
* Get groups, regardless of the role within the group
* @return
Expand All @@ -64,6 +69,10 @@ public DinaAuthenticatedUser currentUser() {
authenticatedUserBuilder.rolesPerGroup(Map.of("dev-group", Set.of(DinaRole.USER)));
}

if (CollectionUtils.isNotEmpty(devSettings.getAdminRoles())) {
authenticatedUserBuilder.adminRoles(devSettings.getAdminRoles());
}

return authenticatedUserBuilder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public class DinaAuthenticatedUser {
private final String username;
private final Set<String> groups;
private final Map<String, Set<DinaRole>> rolesPerGroup;
private final Set<DinaRole> adminRoles;
private final boolean isServiceAccount;

@Builder
Expand All @@ -29,13 +30,15 @@ public DinaAuthenticatedUser(
String agentIdentifier,
String internalIdentifier,
Map<String, Set<DinaRole>> rolesPerGroup,
Set<DinaRole> adminRoles,
boolean isServiceAccount
) {
this.internalIdentifier = internalIdentifier;
this.username = username;
this.agentIdentifier = agentIdentifier;
this.rolesPerGroup = rolesPerGroup == null ? Collections.emptyMap() : rolesPerGroup;
this.groups = this.rolesPerGroup.keySet();
this.adminRoles = adminRoles == null ? Set.of() : adminRoles;
this.isServiceAccount = isServiceAccount;
}

Expand Down
48 changes: 41 additions & 7 deletions dina-base-api/src/main/java/ca/gc/aafc/dina/security/DinaRole.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package ca.gc.aafc.dina.security;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.util.Arrays;
import java.util.List;
import lombok.Getter;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
Expand All @@ -11,17 +14,18 @@

/**
* Represent user role in the context of a DINA module.
* The roles that end with _ADMIN mean that they are now restricted by group.
* The roles that end with _ADMIN mean that they are admin-based so not restricted by group.
*/
@SuppressFBWarnings(value = "MS_EXPOSE_REP")
@RequiredArgsConstructor
public enum DinaRole {

DINA_ADMIN("dina-admin", 0),
SUPER_USER("super-user", 1),
USER("user", 2),
GUEST("guest", 3),
READ_ONLY_ADMIN("read-only-admin", 4), // for service accounts like search-cli
READ_ONLY("read-only", 5);
DINA_ADMIN("dina-admin", 0, true),
SUPER_USER("super-user", 1, false),
USER("user", 2, false),
GUEST("guest", 3, false),
READ_ONLY_ADMIN("read-only-admin", 4, true), // for service accounts like search-cli
READ_ONLY("read-only", 5, false);

/**
* Read carefully since sorting is done based on priority:
Expand All @@ -31,6 +35,14 @@ public enum DinaRole {

private static final Pattern NON_ALPHA = Pattern.compile("[^A-Za-z]");

private static final List<DinaRole> ADMIN_BASED_ROLES = Arrays.stream(DinaRole.values())
.filter(DinaRole::isAdminBased)
.toList();

private static final List<DinaRole> GROUP_BASED_ROLES = Arrays.stream(DinaRole.values())
.filter(r -> !r.isAdminBased())
.toList();

/**
* Name as entered in Keycloak
*/
Expand All @@ -42,6 +54,12 @@ public enum DinaRole {
*/
private final int priority;

/**
* Is a role admin-based or not. admin-base roles are not restricted by group.
*/
@Getter
private final boolean adminBased;

/**
* Similar but more lenient than {@link #valueOf(String)}.
* String like "super-user" will match SUPER_USER.
Expand All @@ -62,6 +80,22 @@ public static Optional<DinaRole> fromString(String str) {
return Optional.empty();
}

/**
* List of roles that are group-based.
* @return
*/
public static List<DinaRole> groupBasedRoles() {
return GROUP_BASED_ROLES;
}

/**
* List of roles that are admin-based.
* @return
*/
public static List<DinaRole> adminBasedRoles() {
return ADMIN_BASED_ROLES;
}

/**
* Private function. Use {@link #COMPARATOR} or specific methods.
* @return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,16 @@ public DinaAuthenticatedUser currentUser() {
rolesPerGroup = KeycloakClaimParser.parseGroupClaims(groupClaim);
}

Set<DinaRole> adminRoles = KeycloakClaimParser
.parseAdminRoles(accessToken.getRealmAccess().getRoles());

return DinaAuthenticatedUser.builder()
.agentIdentifier(agentId)
.isServiceAccount(isServiceAccount)
.internalIdentifier(internalID)
.username(username)
.rolesPerGroup(rolesPerGroup)
.adminRoles(adminRoles)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
Expand All @@ -21,6 +22,28 @@ public final class KeycloakClaimParser {
private KeycloakClaimParser() {
}

/**
* Parses a set of roles from Keycloak.
* If the role is admin-based, it is added to adminRoles. Otherwise, the role is ignored.
* @param roles
* @return
*/
public static Set<DinaRole> parseAdminRoles(Set<String> roles) {
if (CollectionUtils.isEmpty(roles)) {
return Collections.emptySet();
}

Set<DinaRole> adminRoles = new HashSet<>();
for (String role : roles) {
DinaRole.fromString(role).ifPresent(r -> {
if (r.isAdminBased()) {
adminRoles.add(r);
}
});
}
return adminRoles;
}

/**
* Parses a list of Keycloak group claims and creates a Map of roles per group.
* Expected format of the claim: /group/subgroup where the subgroup is matching
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
public class DinaAdminCUDAuthorizationService extends PermissionAuthorizationService {

@Override
@PreAuthorize("hasDinaRole(@currentUser, 'DINA_ADMIN')")
@PreAuthorize("hasAdminRole(@currentUser, 'DINA_ADMIN')")
public void authorizeCreate(Object entity) {

}
Expand All @@ -18,13 +18,13 @@ public void authorizeRead(Object entity) {
}

@Override
@PreAuthorize("hasDinaRole(@currentUser, 'DINA_ADMIN')")
@PreAuthorize("hasAdminRole(@currentUser, 'DINA_ADMIN')")
public void authorizeUpdate(Object entity) {

}

@Override
@PreAuthorize("hasDinaRole(@currentUser, 'DINA_ADMIN')")
@PreAuthorize("hasAdminRole(@currentUser, 'DINA_ADMIN')")
public void authorizeDelete(Object entity) {

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,23 @@ public boolean hasDinaRole(DinaAuthenticatedUser user, String role) {
.anyMatch(dinaRole -> dinaRole.name().equalsIgnoreCase(role.strip()));
}

/**
* returns true if the given user has a given admin role
*
* @param user user with roles
* @param role admin role to check for
* @return - true if the given user has a given role in one of it's many groups
*/
public boolean hasAdminRole(DinaAuthenticatedUser user, String role) {
if (user == null || StringUtils.isBlank(role)) {
return false;
}

return user.getAdminRoles()
.stream()
.anyMatch(dinaRole -> dinaRole.name().equalsIgnoreCase(role.strip()));
}

/**
* Returns true if the given authenticated user is a member of the group the given target object belongs to
* and also has the given role for that group.
Expand Down Expand Up @@ -121,11 +138,12 @@ public boolean hasGroupAndRolePermissions(
/**
* Returns true if the given authenticated user is a member of the group the given target object belongs to
* and also has the given minimum role for that group.
* If the user is DINA_ADMIN this method returns true
*
* @param user user with roles
* @param minimumRole minimum role to check the user has
* @param targetDomainObject Target resource of the request
* @return true if the given user has the given minimum role for that group.
* @return true if the given user has the given minimum role for that group or is DINA_ADMIN
*/
public boolean hasMinimumGroupAndRolePermissions(
DinaAuthenticatedUser user,
Expand All @@ -136,9 +154,13 @@ public boolean hasMinimumGroupAndRolePermissions(
return false;
}

// DINA_ADMIN is always authorized
if (user.getAdminRoles().contains(DinaRole.DINA_ADMIN)) {
return true;
}

Optional<DinaRole> minimumDinaRole = DinaRole.fromString(minimumRole);
Optional<Set<DinaRole>> roles = user.getRolesForGroup(((DinaEntity) targetDomainObject).getGroup());

if (roles.isEmpty() || minimumDinaRole.isEmpty()) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

import org.junit.jupiter.api.Test;

import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class DinaRoleTest {

Expand All @@ -19,4 +24,20 @@ public void testFromString() {
assertEquals(Optional.empty(), DinaRole.fromString(null));
}

@Test
public void testAdminAndGroupBased() {

assertTrue(DinaRole.adminBasedRoles().contains(DinaRole.DINA_ADMIN));
assertFalse(DinaRole.adminBasedRoles().contains(DinaRole.SUPER_USER));

assertTrue(DinaRole.groupBasedRoles().contains(DinaRole.SUPER_USER));
assertFalse(DinaRole.groupBasedRoles().contains(DinaRole.DINA_ADMIN));

//make sure all groups are covered
List<DinaRole> allRoles = Arrays.stream(DinaRole.values()).collect(Collectors.toList());
allRoles.removeAll(DinaRole.adminBasedRoles());
allRoles.removeAll(DinaRole.groupBasedRoles());
assertEquals(0, allRoles.size());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void setUp() {
}

@Test
@WithMockKeycloakUser(groupRole = {"CNC:DINA_ADMIN"})
@WithMockKeycloakUser(adminRole = {"DINA_ADMIN"})
public void create_WhenAdmin_CreatesObject() {
ItemDto dto = ItemDto.builder().uuid(UUID.randomUUID()).group("g").build();
ItemDto result = testRepo.create(dto);
Expand All @@ -82,7 +82,7 @@ public void create_WhenNotAdmin_AccessDenied() {
}

@Test
@WithMockKeycloakUser(groupRole = {"CNC:DINA_ADMIN"})
@WithMockKeycloakUser(adminRole = {"DINA_ADMIN"})
void update_WhenAdmin_AccessAccepted() {
assertDoesNotThrow(() -> testRepo.save(ItemDto.builder().uuid(persisted.getUuid()).build()));
}
Expand All @@ -95,7 +95,7 @@ public void update_WhenNotAdmin_AccessDenied() {
}

@Test
@WithMockKeycloakUser(groupRole = {"CNC:DINA_ADMIN"})
@WithMockKeycloakUser(adminRole = {"DINA_ADMIN"})
public void delete_WhenAdmin_AccessAccepted() {
assertDoesNotThrow(() -> testRepo.delete(persisted.getUuid()));
}
Expand Down
Loading

0 comments on commit 61cb47e

Please sign in to comment.