SHIBUI-2394

test changes

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/permission/ShibUiPermissionDelegate.java

@@ -1,23 +1,29 @@
 package edu.internet2.tier.shibboleth.admin.ui.security.permission;
 
+import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor;
 import edu.internet2.tier.shibboleth.admin.ui.domain.IActivatable;
 import edu.internet2.tier.shibboleth.admin.ui.domain.IApprovable;
 import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
 import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorProjection;
 import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository;
 import edu.internet2.tier.shibboleth.admin.ui.security.model.Ownable;
+import edu.internet2.tier.shibboleth.admin.ui.security.model.User;
 import edu.internet2.tier.shibboleth.admin.ui.security.service.UserAccess;
 import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService;
 import lombok.AllArgsConstructor;
 import org.springframework.security.core.Authentication;
 
 import java.io.Serializable;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
 /**
- * The ShibUiPermissionDelegate is the default service for SHIBUI, which delegates calls (primarily) to the the userService to determine
- * whether a user has the correct abilty to act a particular way (possibly on certain objects).
+ * The ShibUiPermissionDelegate is the default service for SHIBUI, which delegates calls (primarily) to the the UserService to determine
+ * whether a user has the correct abilty to act a particular way (possibly on certain objects). Because the Authentication being
+ * supplied to this implmentation comes from the user service, we ignore it and defer to the UserService (which is ultimately using
+ * the Authentication from the security context anyway).
+ *
  */
 @AllArgsConstructor
 public class ShibUiPermissionDelegate implements IShibUiPermissionEvaluator {
@@ -26,20 +32,20 @@ public class ShibUiPermissionDelegate implements IShibUiPermissionEvaluator {
     private UserService userService;
 
     @Override
-    public Collection getPersistentEntities(Authentication authentication, ShibUiPermissibleType shibUiType, PermissionType permissionType) throws ForbiddenException {
+    public Collection getPersistentEntities(Authentication ignored, ShibUiPermissibleType shibUiType, PermissionType permissionType) throws ForbiddenException {
         switch (shibUiType) {
         case entityDescriptorProjection:
             switch (permissionType) {
             case approve:
                 return getAllEntityDescriptorProjectionsNeedingApprovalBasedOnUserAccess();
             case enable:
                 // This particular list is used for an admin function, so the user must be an ADMIN
-                if (!hasPermission(authentication, null, PermissionType.admin)) {
+                if (!hasPermission(ignored, null, PermissionType.admin)) {
                     throw new ForbiddenException();
                 }
                 return entityDescriptorRepository.getEntityDescriptorsNeedingEnabling();
             case fetch:
-                if (!hasPermission(authentication, null, PermissionType.fetch)) {
+                if (!hasPermission(ignored, null, PermissionType.fetch)) {
                     throw new ForbiddenException("User has no access rights to get a list of Metadata Sources");
                 }
                 return getAllEntityDescriptorProjectionsBasedOnUserAccess();
@@ -63,15 +69,15 @@ private List<EntityDescriptorProjection> getAllEntityDescriptorProjectionsNeedin
     }
 
     @Override
-    public boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission) {
+    public boolean hasPermission(Authentication ignored, Object targetDomainObject, Object permission) {
         switch ((PermissionType) permission) {
         case admin: // we don't care about the object - the user is an admin or not
             return userService.currentUserIsAdmin();
         case approve:
             if (userService.currentUserIsAdmin()) { return true; }
             return targetDomainObject instanceof IApprovable ? userService.getGroupsCurrentUserCanApprove().contains(((IApprovable)targetDomainObject).getIdOfOwner()) : false;
         case enable:
-            return targetDomainObject instanceof IActivatable ? userService.currentUserCanEnable((IActivatable) targetDomainObject) : false;
+            return targetDomainObject instanceof IActivatable ? currentUserCanEnable((IActivatable) targetDomainObject) : false;
         case fetch:
             return userService.currentUserIsAdmin() || userService.getCurrentUserAccess().equals(UserAccess.GROUP);
         case viewOrEdit:
@@ -84,4 +90,27 @@ public boolean hasPermission(Authentication authentication, Object targetDomainO
     public boolean hasPermission(Authentication authentication, Serializable targetId, String target, Object permission) {
         return false; // Unused and Unimplemented - we don't need for this implementation to lookup objects
     }
-}
+
+    private boolean currentUserCanEnable(IActivatable activatableObject) {
+        if (userService.currentUserIsAdmin()) { return true; }
+        switch (activatableObject.getActivatableType()) {
+        case ENTITY_DESCRIPTOR: {
+            return currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" )) && userService.getCurrentUserGroup().getOwnerId().equals(((EntityDescriptor) activatableObject).getIdOfOwner());
+        }
+        // Currently filters and providers dont have ownership, so we just look for the right role
+        case FILTER:
+        case METADATA_RESOLVER:
+            return currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" ));
+        default:
+            return false;
+        }
+    }
+
+    /**
+     * This basic logic assumes users only have a single role (despite users having a list of roles, we assume only 1 currently)
+     */
+    private boolean currentUserHasExpectedRole(List<String> acceptedRoles) {
+        User user = userService.getCurrentUser();
+        return acceptedRoles.contains(user.getRole());
+    }
+}

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java

@@ -56,37 +56,10 @@ public UserService(IGroupService groupService, OwnershipRepository ownershipRepo
         this.userRepository = userRepository;
     }
 
-    public boolean currentUserCanApprove(List<Group> approverGroups) {
-        if (currentUserIsAdmin()) {
-            return true;
-        }
-        Group currentUserGroup = getCurrentUserGroup();
-        return approverGroups.contains(currentUserGroup);
-    }
-
-    public boolean currentUserCanEnable(IActivatable activatableObject) {
-        if (currentUserIsAdmin()) { return true; }
-        switch (activatableObject.getActivatableType()) {
-        case ENTITY_DESCRIPTOR: {
-            return currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" )) && getCurrentUserGroup().getOwnerId().equals(((EntityDescriptor) activatableObject).getIdOfOwner());
-        }
-        // Currently filters and providers dont have ownership, so we just look for the right role
-        case FILTER:
-        case METADATA_RESOLVER:
-            return currentUserHasExpectedRole(Arrays.asList("ROLE_ENABLE" ));
-        default:
-            return false;
-        }
-    }
-
     /**
-     * This basic logic assumes users only have a single role (despite users having a list of roles, we assume only 1 currently)
+     * @deprecated don't call this, call the ShibUiPermissionDelegate method hasPermission(...)
      */
-    private boolean currentUserHasExpectedRole(List<String> acceptedRoles) {
-        User user = getCurrentUser();
-        return acceptedRoles.contains(user.getRole());
-    }
-
+    @Deprecated
     public boolean currentUserIsAdmin() {
         User user = getCurrentUser();
         return user != null && user.getRole().equals("ROLE_ADMIN");

backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java

@@ -85,7 +85,7 @@ public class JPAEntityDescriptorServiceImpl implements EntityDescriptorService {
     private OwnershipRepository ownershipRepository;
 
     @Autowired
-    private IShibUiPermissionEvaluator shibUiService;
+    private IShibUiPermissionEvaluator shibUiAuthorizationDelegate;
 
     @Autowired
     private UserService userService;
@@ -183,7 +183,7 @@ public EntityDescriptorRepresentation changeApproveStatusOfEntityDescriptor(Stri
         if (ed == null) {
             throw new PersistentEntityNotFound("Entity with resourceid[" + resourceId + "] was not found for approval");
         }
-        if (!shibUiService.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.approve)) {
+        if (!shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.approve)) {
             throw new ForbiddenException("You do not have the permissions necessary to approve this entity descriptor.");
         }
         if (status) { // approve
@@ -221,7 +221,7 @@ public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation e
         }
 
         EntityDescriptor ed = (EntityDescriptor) createDescriptorFromRepresentation(edRep);
-        if (ed.isServiceEnabled() && !shibUiService.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.enable)) {
+        if (ed.isServiceEnabled() && !shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.enable)) {
             throw new ForbiddenException("You do not have the permissions necessary to enable this entity descriptor.");
         }
 
@@ -231,7 +231,7 @@ public EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation e
         validateEntityIdAndACSUrls(edRep);
 
         ed.setIdOfOwner(userService.getCurrentUserGroup().getOwnerId());
-        if (shibUiService.hasPermission(userService.getCurrentUserAuthentication(), null, PermissionType.admin)) {
+        if (shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), null, PermissionType.admin)) {
             ed.setApproved(true);
         }
 
@@ -250,7 +250,7 @@ public EntityDescriptorRepresentation createNewEntityDescriptorFromXMLOrigin(Ent
         if (ed.getProtocol() == EntityDescriptorProtocol.OIDC) {
             ed.getSPSSODescriptor("").addSupportedProtocol("http://openid.net/specs/openid-connect-core-1_0.html");
         }
-        if (shibUiService.hasPermission(userService.getCurrentUserAuthentication(), null, PermissionType.admin)) {
+        if (shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), null, PermissionType.admin)) {
             ed.setApproved(true);
         }
         EntityDescriptor savedEntity = entityDescriptorRepository.save(ed);
@@ -493,15 +493,15 @@ public boolean entityExists(String entityID) {
      */
     @Override
     public List<EntityDescriptorProjection> getAllEntityDescriptorProjectionsBasedOnUserAccess() throws ForbiddenException {
-        return (List<EntityDescriptorProjection>) shibUiService.getPersistentEntities(userService.getCurrentUserAuthentication(), ShibUiPermissibleType.entityDescriptorProjection, PermissionType.fetch);
+        return (List<EntityDescriptorProjection>) shibUiAuthorizationDelegate.getPersistentEntities(userService.getCurrentUserAuthentication(), ShibUiPermissibleType.entityDescriptorProjection, PermissionType.fetch);
     }
 
     /**
      * Based on the current users group, find those entities that the user can approve that need approval
      */
     @Override
     public List<EntityDescriptorProjection> getAllEntityDescriptorProjectionsNeedingApprovalBasedOnUserAccess() throws ForbiddenException {
-        return (List<EntityDescriptorProjection>) shibUiService.getPersistentEntities(userService.getCurrentUserAuthentication(), ShibUiPermissibleType.entityDescriptorProjection, PermissionType.approve);
+        return (List<EntityDescriptorProjection>) shibUiAuthorizationDelegate.getPersistentEntities(userService.getCurrentUserAuthentication(), ShibUiPermissibleType.entityDescriptorProjection, PermissionType.approve);
     }
 
     @Override
@@ -515,7 +515,7 @@ public List<String> getAttributeReleaseListFromAttributeList(List<Attribute> att
 
     @Override
     public Iterable<EntityDescriptorProjection> getDisabledMetadataSources() throws ForbiddenException {
-        return (List<EntityDescriptorProjection>) shibUiService.getPersistentEntities(userService.getCurrentUserAuthentication(), ShibUiPermissibleType.entityDescriptorProjection, PermissionType.enable);
+        return (List<EntityDescriptorProjection>) shibUiAuthorizationDelegate.getPersistentEntities(userService.getCurrentUserAuthentication(), ShibUiPermissibleType.entityDescriptorProjection, PermissionType.enable);
     }
 
     @Override
@@ -524,7 +524,7 @@ public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throw
         if (ed == null) {
             throw new PersistentEntityNotFound(String.format("The entity descriptor with entity id [%s] was not found.", resourceId));
         }
-        if (!shibUiService.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.viewOrEdit)) {
+        if (!shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.viewOrEdit)) {
             throw new ForbiddenException();
         }
         return ed;
@@ -605,13 +605,13 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe
         if (existingEd == null) {
             throw new PersistentEntityNotFound(String.format("The entity descriptor with entity id [%s] was not found for update.", edRep.getId()));
         }
-        if (edRep.isServiceEnabled() && !shibUiService.hasPermission(userService.getCurrentUserAuthentication(), existingEd, PermissionType.enable)) {
+        if (edRep.isServiceEnabled() && !shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), existingEd, PermissionType.enable)) {
             throw new ForbiddenException("You do not have the permissions necessary to enable this service.");
         }
         if (StringUtils.isEmpty(edRep.getIdOfOwner())) {
             edRep.setIdOfOwner(StringUtils.isNotEmpty(existingEd.getIdOfOwner()) ? existingEd.getIdOfOwner() :  userService.getCurrentUserGroup().getOwnerId());
         }
-        if (!shibUiService.hasPermission(userService.getCurrentUserAuthentication(), existingEd, PermissionType.viewOrEdit)) {
+        if (!shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), existingEd, PermissionType.viewOrEdit)) {
             throw new ForbiddenException();
         }
         // Verify we're the only one attempting to update the EntityDescriptor
@@ -645,15 +645,15 @@ public EntityDescriptorRepresentation updateEntityDescriptorEnabledStatus(String
         if (ed == null) {
             throw new PersistentEntityNotFound("Entity with resourceid[" + resourceId + "] was not found for update");
         }
-        if (!shibUiService.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.enable)) {
+        if (!shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), ed, PermissionType.enable)) {
             throw new ForbiddenException("You do not have the permissions necessary to change the enable status of this entity descriptor.");
         }
         // check to see if approvals have been completed
         int approvedCount = ed.approvedCount();
         List<Approvers> approversList = groupService.find(ed.getIdOfOwner()).getApproversList();
         if (enabled == true &&
             !ed.isServiceEnabled() &&
-            !shibUiService.hasPermission(userService.getCurrentUserAuthentication(), null, PermissionType.admin) &&
+            !shibUiAuthorizationDelegate.hasPermission(userService.getCurrentUserAuthentication(), null, PermissionType.admin) &&
             approversList.size() > approvedCount) {
             throw new ForbiddenException("Approval must be completed before you can change the enable status of this entity descriptor.");
         }
@@ -691,4 +691,4 @@ private void validateEntityIdAndACSUrls(EntityDescriptorRepresentation edRep) th
             }
         }
     }
-}
+}

backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/AbstractBaseDataJpaTest.groovy

@@ -1,5 +1,7 @@
 package edu.internet2.tier.shibboleth.admin.ui
 
+import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor
+import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository
 import edu.internet2.tier.shibboleth.admin.ui.security.model.Role
 import edu.internet2.tier.shibboleth.admin.ui.security.model.User
 import edu.internet2.tier.shibboleth.admin.ui.security.model.listener.GroupUpdatedEntityListener
@@ -37,6 +39,9 @@ abstract class AbstractBaseDataJpaTest extends Specification implements ResetsDa
     @Autowired
     ApproversRepository approversRepository
 
+    @Autowired
+    EntityDescriptorRepository entityDescriptorRepository
+
     @Autowired
     EntityManager entityManager
 

backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorVersionControllerTests.groovy

@@ -43,9 +43,6 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 
 @ContextConfiguration(classes=[EDCLocalConfig])
 class EntityDescriptorVersionControllerTests extends AbstractBaseDataJpaTest {
-    @Autowired
-    EntityDescriptorRepository entityDescriptorRepository
-
     @Autowired
     private TestEntityManager testEntityManager
 
@@ -166,4 +163,4 @@ class EntityDescriptorVersionControllerTests extends AbstractBaseDataJpaTest {
             return new EnversVersionServiceSupport(entityManager)
         }
     }
-}
+}

backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/scheduled/EntityDescriptorFilesScheduledTasksTests.groovy

@@ -28,7 +28,7 @@ class EntityDescriptorFilesScheduledTasksTests extends AbstractBaseDataJpaTest {
 
     def directory
 
-    def entityDescriptorRepository = Mock(EntityDescriptorRepository)
+    def entityDescriptorRepo = Mock(EntityDescriptorRepository)
 
     def entityDescriptorFilesScheduledTasks
 
@@ -38,7 +38,7 @@ class EntityDescriptorFilesScheduledTasksTests extends AbstractBaseDataJpaTest {
         randomGenerator = new RandomGenerator()
         tempPath = tempPath + randomGenerator.randomRangeInt(10000, 20000)
         EntityDescriptorConversionUtils.setOpenSamlObjects(openSamlObjects)
-        entityDescriptorFilesScheduledTasks = new EntityDescriptorFilesScheduledTasks(tempPath, entityDescriptorRepository, openSamlObjects, new FileCheckingFileWritingService())
+        entityDescriptorFilesScheduledTasks = new EntityDescriptorFilesScheduledTasks(tempPath, entityDescriptorRepo, openSamlObjects, new FileCheckingFileWritingService())
         directory = new File(tempPath)
         directory.mkdir()
     }
@@ -74,7 +74,7 @@ class EntityDescriptorFilesScheduledTasksTests extends AbstractBaseDataJpaTest {
             }
             it
         })
-        1 * entityDescriptorRepository.findAllStreamByServiceEnabled(true) >> [entityDescriptor].stream()
+        1 * entityDescriptorRepo.findAllStreamByServiceEnabled(true) >> [entityDescriptor].stream()
 
         when:
         if (directory.exists()) {
@@ -107,7 +107,7 @@ class EntityDescriptorFilesScheduledTasksTests extends AbstractBaseDataJpaTest {
         def file = new File(directory, randomGenerator.randomId() + ".xml")
         file.text = "Delete me!"
 
-        1 * entityDescriptorRepository.findAllStreamByServiceEnabled(true) >> [entityDescriptor].stream()
+        1 * entityDescriptorRepo.findAllStreamByServiceEnabled(true) >> [entityDescriptor].stream()
 
         when:
         entityDescriptorFilesScheduledTasks.removeDanglingEntityDescriptorFiles()