From 16d69d62e1dc6952421d9019b6b3b693e08f0c9a Mon Sep 17 00:00:00 2001 From: Anton Abushkevich Date: Fri, 11 Nov 2022 15:11:02 +0300 Subject: [PATCH] Moderator cannot delete reusable despite reusable:*:delete permission #2158 (#2159) --- pom.xml | 1 + .../sensitiveinfo/AbstractAdminService.java | 15 +++++++++++++-- .../ohdsi/webapi/reusable/ReusableService.java | 2 +- .../ohdsi/webapi/service/AbstractDaoService.java | 13 +++++++++++++ src/main/resources/application.properties | 1 + 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index d65ab3f7e1..777df7b844 100644 --- a/pom.xml +++ b/pom.xml @@ -242,6 +242,7 @@ admin + Moderator txt diff --git a/src/main/java/org/ohdsi/webapi/common/sensitiveinfo/AbstractAdminService.java b/src/main/java/org/ohdsi/webapi/common/sensitiveinfo/AbstractAdminService.java index cce3eacc8b..dee4b9519a 100644 --- a/src/main/java/org/ohdsi/webapi/common/sensitiveinfo/AbstractAdminService.java +++ b/src/main/java/org/ohdsi/webapi/common/sensitiveinfo/AbstractAdminService.java @@ -18,6 +18,9 @@ public abstract class AbstractAdminService { @Value("${sensitiveinfo.admin.role}") private String adminRole; + @Value("${sensitiveinfo.moderator.role}") + private String moderatorRole; + @Value("${security.provider}") private String securityProvider; @@ -29,6 +32,14 @@ protected boolean isSecured() { } protected boolean isAdmin() { + return isInRole(this.adminRole); + } + + protected boolean isModerator() { + return isInRole(this.moderatorRole); + } + + private boolean isInRole(final String role) { if (!isSecured()) { return true; } @@ -36,10 +47,10 @@ protected boolean isAdmin() { UserEntity currentUser = permissionManager.getCurrentUser(); if (Objects.nonNull(currentUser)) { Set roles = permissionManager.getUserRoles(currentUser.getId()); - return roles.stream().anyMatch(r -> Objects.nonNull(r.getName()) && r.getName().equals(adminRole)); + return roles.stream().anyMatch(r -> Objects.nonNull(r.getName()) && r.getName().equalsIgnoreCase(role)); } } catch (Exception e) { - LOGGER.warn("Failed to check administrative rights, fallback to regular", e); + LOGGER.warn("Failed to check rights, fallback to regular", e); } return false; } diff --git a/src/main/java/org/ohdsi/webapi/reusable/ReusableService.java b/src/main/java/org/ohdsi/webapi/reusable/ReusableService.java index 69120a4b9d..b836568a48 100644 --- a/src/main/java/org/ohdsi/webapi/reusable/ReusableService.java +++ b/src/main/java/org/ohdsi/webapi/reusable/ReusableService.java @@ -133,7 +133,7 @@ public void unassignTag(Integer id, int tagId) { public void delete(Integer id) { Reusable existing = reusableRepository.findOne(id); - checkOwnerOrAdmin(existing.getCreatedBy()); + checkOwnerOrAdminOrModerator(existing.getCreatedBy()); reusableRepository.delete(id); } diff --git a/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java b/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java index 1d6c2a50aa..659b70c804 100644 --- a/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java +++ b/src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java @@ -399,6 +399,19 @@ protected void checkOwnerOrAdmin(UserEntity owner) { } } + protected void checkOwnerOrAdminOrModerator(UserEntity owner) { + if (security instanceof DisabledSecurity) { + return; + } + + UserEntity user = getCurrentUser(); + Long ownerId = Objects.nonNull(owner) ? owner.getId() : null; + + if (!(user.getId().equals(ownerId) || isAdmin() || isModerator())) { + throw new ForbiddenException(); + } + } + protected void checkOwnerOrAdminOrGranted(CommonEntity entity) { if (security instanceof DisabledSecurity) { return; diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 02791a3dcd..d6bba61bbe 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -240,6 +240,7 @@ jdbc.suppressInvalidApiException=${jdbc.suppressInvalidApiException} #Sensitive info settings sensitiveinfo.admin.role=${sensitiveinfo.admin.role} +sensitiveinfo.moderator.role=${sensitiveinfo.moderator.role} sensitiveinfo.analysis.extensions=${sensitiveinfo.analysis.extensions} analysis.result.zipVolumeSizeMb=${analysis.result.zipVolumeSizeMb}