SHIBUI-1848

Refactoring EntityDescriptorController - moving as much logic for permission checking and error handling out of the controller to the service layer and ExceptionHandler
TIER · Jul 3, 2021 · c356817 · c356817
1 parent b592f38
commit c356817
Show file tree

Hide file tree

Showing 9 changed files with 224 additions and 128 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
@@ -3,12 +3,16 @@
 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.EntityIdExistsException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
 import edu.internet2.tier.shibboleth.admin.ui.opensaml.OpenSamlObjects;
 import edu.internet2.tier.shibboleth.admin.ui.repository.EntityDescriptorRepository;
 import edu.internet2.tier.shibboleth.admin.ui.security.model.User;
 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;
@@ -20,6 +24,7 @@
 import org.springframework.security.access.annotation.Secured;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.web.bind.annotation.DeleteMapping;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.PostMapping;
@@ -32,22 +37,24 @@
 import org.springframework.web.servlet.support.ServletUriComponentsBuilder;
 
 import javax.annotation.PostConstruct;
+
 import java.net.URI;
+import java.util.ConcurrentModificationException;
 import java.util.List;
 import java.util.stream.Collectors;
 
 @RestController
 @RequestMapping("/api")
 @Slf4j
 public class EntityDescriptorController {
-    private static URI getResourceUriFor(EntityDescriptor ed) {
+    static URI getResourceUriFor(String resourceId) {
         return ServletUriComponentsBuilder
                 .fromCurrentServletMapping().path("/api/EntityDescriptor")
-                .pathSegment(ed.getResourceId())
+                .pathSegment(resourceId)
                 .build()
                 .toUri();
     }
-
+    
     @Autowired
     private EntityDescriptorRepository entityDescriptorRepository;
 
@@ -73,58 +80,28 @@ public EntityDescriptorController(UserService userService, EntityDescriptorVersi
 
     @PostMapping("/EntityDescriptor")
     @Transactional
-    public ResponseEntity<?> create(@RequestBody EntityDescriptorRepresentation edRepresentation) {
-        final String entityId = edRepresentation.getEntityId();
-
-        ResponseEntity<?> entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck(edRepresentation.isServiceEnabled());
-        if (entityDescriptorEnablingDeniedResponse != null) {
-            return entityDescriptorEnablingDeniedResponse;
-        }
-
-        ResponseEntity<?> existingEntityDescriptorConflictResponse = existingEntityDescriptorCheck(entityId);
-        if (existingEntityDescriptorConflictResponse != null) {
-            return existingEntityDescriptorConflictResponse;
-        }
-
-        EntityDescriptor ed = (EntityDescriptor) entityDescriptorService.createDescriptorFromRepresentation(edRepresentation);
-
-        EntityDescriptor persistedEd = entityDescriptorRepository.save(ed);
-        edRepresentation.setId(persistedEd.getResourceId());
-        edRepresentation.setCreatedDate(persistedEd.getCreatedDate());
-        return ResponseEntity.created(getResourceUriFor(persistedEd)).body(entityDescriptorService.createRepresentationFromDescriptor(persistedEd));
+    public ResponseEntity<?> create(@RequestBody EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityIdExistsException {
+        EntityDescriptorRepresentation persistedEd = entityDescriptorService.createNew(edRepresentation);            
+        return ResponseEntity.created(getResourceUriFor(persistedEd.getId())).body(persistedEd);
     }
 
     @Secured("ROLE_ADMIN")
     @DeleteMapping(value = "/EntityDescriptor/{resourceId}")
     @Transactional
-    public ResponseEntity<?> deleteOne(@PathVariable String resourceId) {
-        EntityDescriptor ed = entityDescriptorRepository.findByResourceId(resourceId);
-        if (ed == null) {
-            return ResponseEntity.notFound().build();
-        } else if (ed.isServiceEnabled()) {
-            return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, "Deleting an enabled Metadata Source is not allowed. Disable the source and try again."));
-        } else {
-            entityDescriptorRepository.delete(ed);
-            return ResponseEntity.noContent().build();
-        }
-    }
-
-    private ResponseEntity<?> entityDescriptorEnablePermissionsCheck(boolean serviceEnabled) {
-        User user = userService.getCurrentUser();
-        if (user != null) {
-            if (serviceEnabled && !user.getRole().equals("ROLE_ADMIN")) {
-                return ResponseEntity.status(HttpStatus.FORBIDDEN)
-                        .body(new ErrorResponse(HttpStatus.FORBIDDEN, "You do not have the permissions necessary to enable this service."));
-            }
+    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.");
         }
-        return null;
+        entityDescriptorRepository.delete(ed);
+        return ResponseEntity.noContent().build();
     }
 
     private ResponseEntity<?> existingEntityDescriptorCheck(String entityId) {
         final EntityDescriptor ed = entityDescriptorRepository.findByEntityID(entityId);
         if (ed != null) {
             HttpHeaders headers = new HttpHeaders();
-            headers.setLocation(getResourceUriFor(ed));
+            headers.setLocation(getResourceUriFor(ed.getResourceId()));
             return ResponseEntity
                     .status(HttpStatus.CONFLICT)
                     .headers(headers)
@@ -137,12 +114,10 @@ private ResponseEntity<?> existingEntityDescriptorCheck(String entityId) {
     @GetMapping("/EntityDescriptors")
     @Transactional(readOnly = true)
     public ResponseEntity<?> getAll() {
-        User currentUser = userService.getCurrentUser();
-        if (currentUser != null) {
+        try {
             return ResponseEntity.ok(entityDescriptorService.getAllRepresentationsBasedOnUserAccess());
-        } else {
-            return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN,
-                    "You are not authorized to perform the requested operation."));
+        } catch (ForbiddenException e) {
+            return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage()));
         }
     }
 
@@ -225,6 +200,11 @@ public ResponseEntity<?> getSpecificVersion(@PathVariable String resourceId, @Pa
         return ResponseEntity.status(HttpStatus.FORBIDDEN).build();
     }
 
+    @ExceptionHandler({ ForbiddenException.class })
+    public void handleException() {
+        //
+    }
+
     private ResponseEntity<?> handleUploadingEntityDescriptorXml(byte[] rawXmlBytes, String spName) throws Exception {
         final EntityDescriptor ed = EntityDescriptor.class.cast(openSamlObjects.unmarshalFromXml(rawXmlBytes));
 
@@ -235,7 +215,7 @@ private ResponseEntity<?> handleUploadingEntityDescriptorXml(byte[] rawXmlBytes,
 
         ed.setServiceProviderName(spName);
         final EntityDescriptor persistedEd = entityDescriptorRepository.save(ed);
-        return ResponseEntity.created(getResourceUriFor(persistedEd))
+        return ResponseEntity.created(getResourceUriFor(persistedEd.getResourceId()))
                 .body(entityDescriptorService.createRepresentationFromDescriptor(persistedEd));
     }
 
@@ -246,38 +226,10 @@ public void initRestTemplate() {
 
     @PutMapping("/EntityDescriptor/{resourceId}")
     @Transactional
-    public ResponseEntity<?> update(@RequestBody EntityDescriptorRepresentation edRepresentation, @PathVariable String resourceId) {
-        User currentUser = userService.getCurrentUser();
-        if (currentUser != null) {
-            EntityDescriptor existingEd = entityDescriptorRepository.findByResourceId(resourceId);
-
-            if (existingEd == null) {
-                return ResponseEntity.notFound().build();
-            } else {
-                if (userService.isAuthorizedFor(existingEd.getCreatedBy(),
-                                existingEd.getGroup() == null ? null : existingEd.getGroup().getResourceId())) {
-                    if (!existingEd.isServiceEnabled()) {
-                        ResponseEntity<?> entityDescriptorEnablingDeniedResponse = entityDescriptorEnablePermissionsCheck(
-                                        edRepresentation.isServiceEnabled());
-                        if (entityDescriptorEnablingDeniedResponse != null) {
-                            return entityDescriptorEnablingDeniedResponse;
-                        }
-                    }
-
-                    // Verify we're the only one attempting to update the EntityDescriptor
-                    if (edRepresentation.getVersion() != existingEd.hashCode()) {
-                        return new ResponseEntity<Void>(HttpStatus.CONFLICT);
-                    }
-
-                    entityDescriptorService.updateDescriptorFromRepresentation(existingEd, edRepresentation);
-                    existingEd = entityDescriptorRepository.save(existingEd);
-
-                    return ResponseEntity.ok().body(entityDescriptorService.createRepresentationFromDescriptor(existingEd));
-                }
-            }
-        }
-        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN,
-                        "You are not authorized to perform the requested operation."));
+    public ResponseEntity<?> update(@RequestBody EntityDescriptorRepresentation edRepresentation, @PathVariable String resourceId) throws ForbiddenException, ConcurrentModificationException, EntityNotFoundException {
+        edRepresentation.setId(resourceId); // This should be the same already, but just to be safe...
+        EntityDescriptorRepresentation result = entityDescriptorService.update(edRepresentation);
+        return ResponseEntity.ok().body(result);
     }
 
     @PostMapping(value = "/EntityDescriptor", consumes = "application/xml")

diff --git a/...rnet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java b/...rnet2/tier/shibboleth/admin/ui/controller/EntityDescriptorControllerExceptionHandler.java
@@ -0,0 +1,44 @@
+package edu.internet2.tier.shibboleth.admin.ui.controller;
+
+import java.util.ConcurrentModificationException;
+
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.ControllerAdvice;
+import org.springframework.web.bind.annotation.ExceptionHandler;
+import org.springframework.web.context.request.WebRequest;
+import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
+
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
+
+@ControllerAdvice(assignableTypes = {EntityDescriptorController.class})
+public class EntityDescriptorControllerExceptionHandler extends ResponseEntityExceptionHandler {
+
+    @ExceptionHandler({ ForbiddenException.class })
+    public ResponseEntity<?> handleForbiddenAccess(ForbiddenException e, WebRequest request) {
+        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new ErrorResponse(HttpStatus.FORBIDDEN, e.getMessage()));
+    }
+
+    @ExceptionHandler({ EntityIdExistsException.class })
+    public ResponseEntity<?> handleEntityExistsException(EntityIdExistsException e, WebRequest request) {
+        HttpHeaders headers = new HttpHeaders();
+        headers.setLocation(EntityDescriptorController.getResourceUriFor(e.getMessage()));
+        return ResponseEntity.status(HttpStatus.CONFLICT).headers(headers).body(new ErrorResponse(
+                        String.valueOf(HttpStatus.CONFLICT.value()),
+                        String.format("The entity descriptor with entity id [%s] already exists.", e.getMessage())));
+
+    }
+
+    @ExceptionHandler({ EntityNotFoundException.class })
+    public ResponseEntity<?> handleEntityNotFoundException(EntityNotFoundException e, WebRequest request) {
+        return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new ErrorResponse(HttpStatus.NOT_FOUND, e.getMessage()));
+    }
+
+    @ExceptionHandler({ ConcurrentModificationException.class })
+    public ResponseEntity<?> handleConcurrentModificationException(ConcurrentModificationException e, WebRequest request) {
+        return ResponseEntity.status(HttpStatus.CONFLICT).body(new ErrorResponse(HttpStatus.CONFLICT, e.getMessage()));
+    }
+}
diff --git a/...c/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityIdExistsException.java b/...c/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityIdExistsException.java
@@ -0,0 +1,8 @@
+package edu.internet2.tier.shibboleth.admin.ui.exception;
+
+public class EntityIdExistsException extends Exception {
+    public EntityIdExistsException(String entityId) {
+        super(entityId);
+    }
+
+}
diff --git a/...c/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityNotFoundException.java b/...c/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/EntityNotFoundException.java
@@ -0,0 +1,7 @@
+package edu.internet2.tier.shibboleth.admin.ui.exception;
+
+public class EntityNotFoundException extends Exception {
+    public EntityNotFoundException(String message) {
+        super(message);
+    }
+}
diff --git a/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/ForbiddenException.java b/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/exception/ForbiddenException.java
@@ -0,0 +1,11 @@
+package edu.internet2.tier.shibboleth.admin.ui.exception;
+
+public class ForbiddenException extends Exception {
+    public ForbiddenException() {
+        super("You are not authorized to perform the requested operation.");
+    }
+
+    public ForbiddenException(String message) {
+        super(message);
+    }
+}
diff --git a/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/...nd/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java
@@ -1,5 +1,6 @@
 package edu.internet2.tier.shibboleth.admin.ui.security.service;
 
+import edu.internet2.tier.shibboleth.admin.ui.security.model.Group;
 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.repository.RoleRepository;
@@ -50,18 +51,25 @@ public UserAccess getCurrentUserAccess() {
             return UserAccess.OWNER;
         }
     }
+
+    public boolean isAuthorizedFor(String objectCreatedBy, Group objectGroup) {        
+        String groupId = objectGroup == null ? "" : objectGroup.getResourceId();      
+        return isAuthorizedFor(objectCreatedBy, groupId);
+    }
+
 
     public boolean isAuthorizedFor(String objectCreatedBy, String objectGroupResourceId) {
-        User currentUser = getCurrentUser();
         String groupId = objectGroupResourceId == null ? "" : objectGroupResourceId;
 
-        switch (getCurrentUserAccess()) {
+        switch (getCurrentUserAccess()) { // no user returns NONE
         case ADMIN:
             return true;
         case GROUP:
+            User currentUser = getCurrentUser();
             return objectCreatedBy.equals(currentUser.getUsername()) || groupId.equals(currentUser.getGroupId());
         case OWNER:
-            return objectCreatedBy.equals(currentUser.getUsername());
+            User cu = getCurrentUser();
+            return objectCreatedBy.equals(cu.getUsername());
         default:
             return false;
         }
@@ -89,4 +97,9 @@ public void updateUserRole(User user) {
             throw new RuntimeException(String.format("User with username [%s] has no role defined and therefor cannot be updated!", user.getUsername()));
         }
     }
+
+    public boolean currentUserIsAdmin() {
+        User user = getCurrentUser();
+        return user != null && user.getRole().equals("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
@@ -2,8 +2,13 @@
 
 import edu.internet2.tier.shibboleth.admin.ui.domain.Attribute;
 import edu.internet2.tier.shibboleth.admin.ui.domain.frontend.EntityDescriptorRepresentation;
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityIdExistsException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.EntityNotFoundException;
+import edu.internet2.tier.shibboleth.admin.ui.exception.ForbiddenException;
+
 import org.opensaml.saml.saml2.metadata.EntityDescriptor;
 
+import java.util.ConcurrentModificationException;
 import java.util.List;
 import java.util.Map;
 
@@ -13,7 +18,6 @@
  * @since 1.0
  */
 public interface EntityDescriptorService {
-
     /**
      * Map from front-end data representation of entity descriptor to opensaml implementation of entity descriptor model
      *
@@ -33,7 +37,7 @@ public interface EntityDescriptorService {
     /**
      * @return a list of EntityDescriptorRepresentations that a user has the rights to access
      */
-    List<EntityDescriptorRepresentation> getAllRepresentationsBasedOnUserAccess();
+    List<EntityDescriptorRepresentation> getAllRepresentationsBasedOnUserAccess() throws ForbiddenException;
 
     /**
      * Given a list of attributes, generate an AttributeReleaseList
@@ -59,4 +63,10 @@ public interface EntityDescriptorService {
      */
     void updateDescriptorFromRepresentation(final EntityDescriptor entityDescriptor, final EntityDescriptorRepresentation representation);
 
+    EntityDescriptorRepresentation createNew(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityIdExistsException;
+
+    EntityDescriptorRepresentation update(EntityDescriptorRepresentation edRepresentation) throws ForbiddenException, EntityNotFoundException, ConcurrentModificationException;
+
+    edu.internet2.tier.shibboleth.admin.ui.domain.EntityDescriptor getEntityDescriptorByResourceId(String resourceId) throws EntityNotFoundException;
+
 }