SHIBUI-1848

Continued refactoring of EntityDescriptorController to move auth check responsibilities to the service layer.
TIER · Jul 3, 2021 · b7aa821 · b7aa821
1 parent f338237
commit b7aa821
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 30 deletions.
diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java b/...in/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntityDescriptorController.java
@@ -12,7 +12,6 @@
 import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService;
 import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorService;
 import edu.internet2.tier.shibboleth.admin.ui.service.EntityDescriptorVersionService;
-import edu.internet2.tier.shibboleth.admin.ui.service.EntityService;
 import lombok.extern.slf4j.Slf4j;
 
 import org.opensaml.core.xml.io.MarshallingException;
@@ -89,11 +88,7 @@ public ResponseEntity<?> create(@RequestBody EntityDescriptorRepresentation edRe
     @DeleteMapping(value = "/EntityDescriptor/{resourceId}")
     @Transactional
     public ResponseEntity<?> deleteOne(@PathVariable String resourceId) throws ForbiddenException, EntityNotFoundException {
-        EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId);
-        if (ed.isServiceEnabled()) {
-            throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again.");
-        }
-        entityDescriptorRepository.delete(ed);
+        entityDescriptorService.delete(resourceId);
         return ResponseEntity.noContent().build();
     }
 
@@ -113,29 +108,16 @@ private ResponseEntity<?> existingEntityDescriptorCheck(String entityId) {
 
     @GetMapping("/EntityDescriptors")
     @Transactional(readOnly = true)
-    public ResponseEntity<?> getAll() {
-        try {
-            return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess());
-        } catch (ForbiddenException e) {
-            return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage()));
-        }
+    public ResponseEntity<?> getAll() throws ForbiddenException {
+        return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess());
     }
 
     @GetMapping("/EntityDescriptor/{resourceId}/Versions")
     @Transactional(readOnly = true)
-    public ResponseEntity<?> getAllVersions(@PathVariable String resourceId) {
-        EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId);
-        if (ed == null) {
-            return ResponseEntity.notFound().build();
-        }
-        List<Version> versions = versionService.findVersionsForEntityDescriptor(resourceId);
-        if (versions.isEmpty()) {
-            return ResponseEntity.notFound().build();
-        }
-        if(userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup() == null ? null : ed.getGroup().getResourceId())) {
-            return ResponseEntity.ok(versions);
-        }
-        return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
+    public ResponseEntity<?> getAllVersions(@PathVariable String resourceId) throws EntityNotFoundException, ForbiddenException {
+        // this verifies that both the ED exists and the user has proper access, so needs to remain
+        EntityDescriptor ed = entityDescriptorService.getEntityDescriptorByResourceId(resourceId);
+        return ResponseEntity.ok(versionService.findVersionsForEntityDescriptor(ed.getResourceId()));
     }
 
     @Secured("ROLE_ADMIN")

diff --git a/...src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java b/...src/main/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorService.java
@@ -67,6 +67,8 @@ public interface EntityDescriptorService {
 
     EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityNotFoundException, ConcurrentModificationException;
 
-    edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException;
+    edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException;
+
+    void delete(String resourceId) throws ForbiddenException, EntityNotFoundException;
 
 }
diff --git a/...n/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java b/...n/java/edu/internet2/tier/shibboleth/admin/ui/service/EntityDescriptorVersionService.java
@@ -3,6 +3,7 @@
 import edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor;
 import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation;
 import edu.internet2.tier.shibboleth.admin.ui.domain.versioning.Version;
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
 
 import java.util.List;
 
@@ -11,7 +12,7 @@
  */
 public interface EntityDescriptorVersionService {
 
-    List<Version> findVersionsForEntityDescriptor(String resourceId);
+    List<Version> findVersionsForEntityDescriptor(String resourceId) throws EntityNotFoundException;
 
     EntityDescriptorRepresentation findSpecificVersionOfEntityDescriptor(String resourceId, String versionId);
 }
diff --git a/.../edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java b/.../edu/internet2/tier/shibboleth/admin/ui/service/EnversEntityDescriptorVersionService.java
@@ -4,6 +4,7 @@
 import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation;
 import edu.internet2.tier.shibboleth.admin.ui.domain.versioning.Version;
 import edu.internet2.tier.shibboleth.admin.ui.envers.EnversVersionServiceSupport;
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
 
 import java.util.List;
 
@@ -22,8 +23,12 @@ public EnversEntityDescriptorVersionService(EnversVersionServiceSupport enversVe
     }
 
     @Override
-    public List<Version> findVersionsForEntityDescriptor(String resourceId) {
-        return enversVersionServiceSupport.findVersionsForPersistentEntity(resourceId, EntityDescriptor.class);
+    public List<Version> findVersionsForEntityDescriptor(String resourceId) throws EntityNotFoundException {
+        List<Version> results = enversVersionServiceSupport.findVersionsForPersistentEntity(resourceId, EntityDescriptor.class); 
+        if (results.isEmpty()) {
+            throw new EntityNotFoundException(String.format("No versions found for entity descriptor with resource id [%s].", resourceId));
+        }
+        return results;
     }
 
     @Override

diff --git a/...n/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java b/...n/java/edu/internet2/tier/shibboleth/admin/ui/service/JPAEntityDescriptorServiceImpl.java
@@ -779,11 +779,24 @@ public EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRe
     }
 
     @Override
-    public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException {
+    public EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException, ForbiddenException {
         EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId);
         if (ed == null) {
             throw new EntityNotFoundException(String.format("The entity descriptor with entity id [%s] was not found.", resourceId));
         }
+        if (!userService.isAuthorizedFor(ed.getCreatedBy(), ed.getGroup())) {
+            throw new ForbiddenException("You are not authorized to perform the requested operation.");
+        }     
         return ed;
     }
+
+    @Override
+    public void delete(String resourceId) throws ForbiddenException, EntityNotFoundException {
+        EntityDescriptor ed = getEntityDescriptorByResourceId(resourceId);
+        if (ed.isServiceEnabled()) {
+            throw new ForbiddenException("Deleting an enabled Metadata Source is not allowed. Disable the source and try again.");
+        }
+        entityDescriptorRepository.delete(ed);
+
+    }
 }