[issue-2953] Fixed access to resources for non-admin Tag Managers (having a 'tag:management' permission)

Author: oleg-odysseus

src/main/java/org/ohdsi/webapi/cohortcharacterization/CcServiceImpl.java

@@ -322,15 +322,13 @@ public void onFeAnalysisChanged(FeAnalysisChangedEvent event) {
     @Transactional
     public void assignTag(Long id, int tagId) {
         CohortCharacterizationEntity entity = findById(id);
-        checkOwnerOrAdminOrGranted(entity);
         assignTag(entity, tagId);
     }
 
     @Override
     @Transactional
     public void unassignTag(Long id, int tagId) {
         CohortCharacterizationEntity entity = findById(id);
-        checkOwnerOrAdminOrGranted(entity);
         unassignTag(entity, tagId);
     }
 

src/main/java/org/ohdsi/webapi/pathway/PathwayServiceImpl.java

@@ -534,19 +534,17 @@ public String findDesignByGenerationId(@PathwayAnalysisGenerationId final Long i
 		return entity.getDesign();
 	}
 
-	@Override
-	public void assignTag(Integer id, int tagId) {
-		PathwayAnalysisEntity entity = getById(id);
-		checkOwnerOrAdminOrGranted(entity);
-		assignTag(entity, tagId);
-	}
-
-	@Override
-	public void unassignTag(Integer id, int tagId) {
-		PathwayAnalysisEntity entity = getById(id);
-		checkOwnerOrAdminOrGranted(entity);
-		unassignTag(entity, tagId);
-	}
+    @Override
+    public void assignTag(Integer id, int tagId) {
+        PathwayAnalysisEntity entity = getById(id);
+        assignTag(entity, tagId);
+    }
+
+    @Override
+    public void unassignTag(Integer id, int tagId) {
+        PathwayAnalysisEntity entity = getById(id);
+        unassignTag(entity, tagId);
+    }
 
 	@Override
 	public List<VersionDTO> getVersions(long id) {
@@ -657,7 +655,7 @@ private PathwayAnalysisResult queryGenerationResults(Source source, Long generat
 						new String[]{GENERATION_ID},
 						new Object[]{generationId}
 		);
-		Map<Integer, Map<String, Integer>> pathwayResults = 
+		Map<Integer, Map<String, Integer>> pathwayResults =
 						getSourceJdbcTemplate(source).query(pathwayResultsPsr.getSql(), pathwayResultsPsr.getOrderedParams(), pathwayExtractor);
 
 		cohortStats.stream().forEach((cp) -> {

src/main/java/org/ohdsi/webapi/service/AbstractDaoService.java

@@ -371,6 +371,7 @@ protected PermissionService getPermissionService() {
   }
 
   protected void assignTag(CommonEntityExt<?> entity, int tagId) {
+    checkOwnerOrAdminOrGrantedOrTagManager(entity);
     if (Objects.nonNull(entity)) {
       Tag tag = tagService.getById(tagId);
       if (Objects.nonNull(tag)) {
@@ -397,6 +398,7 @@ protected void assignTag(CommonEntityExt<?> entity, int tagId) {
   }
 
   protected void unassignTag(CommonEntityExt<?> entity, int tagId) {
+    checkOwnerOrAdminOrGrantedOrTagManager(entity);
     if (Objects.nonNull(entity)) {
       Tag tag = tagService.getById(tagId);
       if (Objects.nonNull(tag)) {
@@ -459,6 +461,19 @@ protected void checkOwnerOrAdminOrGranted(CommonEntity<?> entity) {
     }
   }
 
+  protected void checkOwnerOrAdminOrGrantedOrTagManager(CommonEntity<?> entity) {
+    if (security instanceof DisabledSecurity) {
+      return;
+    }
+
+    UserEntity user = getCurrentUser();
+    Long ownerId = Objects.nonNull(entity.getCreatedBy()) ? entity.getCreatedBy().getId() : null;
+
+    if (!(user.getId().equals(ownerId) || isAdmin() || permissionService.hasWriteAccess(entity) || TagSecurityUtils.canManageTags())) {
+      throw new ForbiddenException();
+    }
+  }
+
   protected <T extends CommonEntityDTO> List<T> listByTags(List<? extends CommonEntityExt<? extends Number>> entities,
                                                            List<String> names,
                                                            Class<T> clazz) {

src/main/java/org/ohdsi/webapi/service/CohortDefinitionService.java

@@ -1265,7 +1265,6 @@ private Response printFrindly(String markdown, String format) {
 	@Transactional
 	public void assignTag(@PathParam("id") final Integer id, final int tagId) {
 		CohortDefinition entity = cohortDefinitionRepository.findOne(id);
-		checkOwnerOrAdminOrGranted(entity);
 		assignTag(entity, tagId);
 	}
 
@@ -1283,7 +1282,6 @@ public void assignTag(@PathParam("id") final Integer id, final int tagId) {
 	@Transactional
 	public void unassignTag(@PathParam("id") final Integer id, @PathParam("tagId") final int tagId) {
 		CohortDefinition entity = cohortDefinitionRepository.findOne(id);
-		checkOwnerOrAdminOrGranted(entity);
 		unassignTag(entity, tagId);
 	}
 

src/main/java/org/ohdsi/webapi/service/ConceptSetService.java

@@ -674,7 +674,6 @@ public void deleteConceptSet(@PathParam("id") final int id) {
 		@CacheEvict(cacheNames = CachingSetup.CONCEPT_SET_LIST_CACHE, allEntries = true)
 		public void assignTag(@PathParam("id") final Integer id, final int tagId) {
         ConceptSet entity = getConceptSetRepository().findById(id);
-        checkOwnerOrAdminOrGranted(entity);
         assignTag(entity, tagId);
     }
 
@@ -692,7 +691,6 @@ public void assignTag(@PathParam("id") final Integer id, final int tagId) {
     @Transactional
     public void unassignTag(@PathParam("id") final Integer id, @PathParam("tagId") final int tagId) {
         ConceptSet entity = getConceptSetRepository().findById(id);
-        checkOwnerOrAdminOrGranted(entity);
         unassignTag(entity, tagId);
     }
 

src/main/java/org/ohdsi/webapi/service/IRAnalysisService.java

@@ -806,15 +806,13 @@ public void deleteInfo(final int id, final String sourceKey) {
   @Transactional
   public void assignTag(final Integer id, final int tagId) {
     IncidenceRateAnalysis entity = irAnalysisRepository.findOne(id);
-    checkOwnerOrAdminOrGranted(entity);
     assignTag(entity, tagId);
   }
 
   @Override
   @Transactional
   public void unassignTag(final Integer id, final int tagId) {
     IncidenceRateAnalysis entity = irAnalysisRepository.findOne(id);
-    checkOwnerOrAdminOrGranted(entity);
     unassignTag(entity, tagId);
   }
 

src/main/java/org/ohdsi/webapi/tag/TagGroupService.java

@@ -75,6 +75,7 @@ private <T extends Number> void assignGroup(HasTags<T> service, List<T> assetIds
                 service.assignTag(id, tagId);
             } catch (final ForbiddenException e) {
                 log.warn("Tag {} cannot be assigned to entity {} in service {} - forbidden", tagId, id, service.getClass().getName());
+                throw e;
             }
         });
     }

src/main/java/org/ohdsi/webapi/tag/TagSecurityUtils.java

@@ -74,4 +74,8 @@ public static String getAssetName(final CommonEntityExt<?> entity) {
         }
         return null;
     }
+
+    public static boolean canManageTags() {
+        return SecurityUtils.getSubject().isPermitted("tag:management");
+    }
 }

src/main/java/org/ohdsi/webapi/tag/TagService.java

@@ -65,7 +65,7 @@ public Tag create(Tag tag) {
                 .filter(Tag::isAllowCustom)
                 .count() == groups.size();
 
-        if (this.getPermissionService().isSecurityEnabled() && !SecurityUtils.getSubject().isPermitted("tag:management") && !allowCustom) {
+        if (this.getPermissionService().isSecurityEnabled() && !TagSecurityUtils.canManageTags() && !allowCustom) {
             throw new IllegalArgumentException("Tag can be added only to groups that allows to do it");
         }