fix: stricter check in checkCommonEntityOwnership for teamProject own…

…ership check

src/main/java/org/ohdsi/webapi/security/PermissionController.java

@@ -6,6 +6,8 @@
 import org.ohdsi.webapi.service.UserService;
 import org.ohdsi.webapi.shiro.Entities.PermissionEntity;
 import org.ohdsi.webapi.shiro.Entities.RoleEntity;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.ohdsi.webapi.shiro.PermissionManager;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.stereotype.Controller;
@@ -39,6 +41,7 @@
 @Path(value = "/permission")
 @Transactional
 public class PermissionController {
+    private final Logger logger = LoggerFactory.getLogger(PermissionController.class);
 
     private final PermissionService permissionService;
     private final PermissionManager permissionManager;
@@ -163,11 +166,13 @@ public void grantEntityPermissionsForRole(
 
         // check if user, or his "teamProject" own the entity being given access:
         permissionService.checkCommonEntityOwnership(entityType, entityId);
-        // furthermore, check if the entity is being shared with "public" role (roleId==1). If yes, then
+        // furthermore, check if the entity is being shared to the "public" role (roleId==1). If yes, then
         // check if user has the necessary global/public sharing permission ("artifact:global:share:put") to do so:
         if (roleId == RoleEntity.PUBLIC_ROLE_ID) {
             Subject subject = SecurityUtils.getSubject();
             if (!subject.isPermitted(new WildcardPermission("artifact:global:share:put"))) {
+                logger.error("Permission denied: user {} has no permission for sharing entities globally (making them visible to all with a 'public' role)",
+                    this.permissionManager.getSubjectName());
                 throw new UnauthorizedException();
             }
         }

src/main/java/org/ohdsi/webapi/security/PermissionService.java

@@ -125,10 +125,13 @@ public void checkCommonEntityOwnership(EntityType entityType, Integer entityId)
         CommonEntity entity = (CommonEntity) entityRepository.getOne((Serializable) conversionService.convert(entityId, idClazz));
 
         if (this.authorizationMode.equals("teamproject")) {
-            // in teamproject mode, it is sufficient if the user has write permission to this entity,
+            // in teamproject mode, it is sufficient if the team has ALL write permission to this entity,
             // as entity ownership and maintenance is a shared responsibility within a team:
-            boolean hasAccess = hasWriteAccess(entity);
-            if (!hasAccess) {
+            RoleEntity teamProjectRole = this.permissionManager.getCurrentTeamProjectRoleForCurrentUser();
+            boolean teamHasWriteAccess = roleHasAccess(teamProjectRole, entity, AccessType.WRITE);
+            if (!teamHasWriteAccess) {
+                logger.error("Permission denied: teamProject {} does not have write access to the entity {}",
+                    teamProjectRole.getName(), entity.getId());
                 throw new UnauthorizedException();
             }
         } else {
@@ -217,6 +220,21 @@ public boolean hasAccess(CommonEntity entity, AccessType accessType) {
         }
         return hasAccess;
     }
+
+    public boolean roleHasAccess(RoleEntity role, CommonEntity entity, AccessType accessType) {
+        boolean hasAccess = false;
+        if (securityEnabled) {
+            try {
+                EntityType entityType = entityPermissionSchemaResolver.getEntityType(entity.getClass());
+                List<Permission> permsToCheck = getEntityPermissions(entityType, entity.getId(), accessType);
+                hasAccess = permsToCheck.stream().allMatch(p -> role.isPermitted(p));
+            } catch (Exception e) {
+                logger.error("Error getting and verifying role's permissions", e);
+                throw new RuntimeException(e);
+            }
+        }
+        return hasAccess;
+    }
 
     public boolean hasWriteAccess(CommonEntity entity) {
       return hasAccess(entity, AccessType.WRITE);

src/main/java/org/ohdsi/webapi/shiro/Entities/RoleEntity.java

@@ -11,6 +11,8 @@
 import javax.persistence.Id;
 import javax.persistence.OneToMany;
 import javax.persistence.Table;
+import org.apache.shiro.authz.permission.WildcardPermission;
+import org.apache.shiro.authz.Permission;
 import org.hibernate.annotations.GenericGenerator;
 import org.hibernate.annotations.Parameter;
 
@@ -90,4 +92,15 @@ public Boolean isSystemRole() {
   public void setSystemRole(Boolean system) {
     systemRole = system;
   }
+
+  public boolean isPermitted(Permission p) {
+    for (RolePermissionEntity rolePermissionEntity : this.getRolePermissions()) {
+        PermissionEntity permissionEntity = rolePermissionEntity.getPermission();
+        WildcardPermission rolePermission = new WildcardPermission(permissionEntity.getValue());
+        if (rolePermission.implies(p)) {
+            return true;
+        }
+    }
+    return false;
+  }
 }