[Task 70131] intermediate fixes error handling

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ErrorResponse.java

@@ -0,0 +1,39 @@
+package org.dspace.app.rest.security;
+
+public class ErrorResponse {
+
+    private int code;
+    private String message;
+
+    /**
+     * Generic getter for the code
+     * @return the code value of this ErrorResponse
+     */
+    public int getCode() {
+        return code;
+    }
+
+    /**
+     * Generic setter for the code
+     * @param code   The code to be set on this ErrorResponse
+     */
+    public void setCode(int code) {
+        this.code = code;
+    }
+
+    /**
+     * Generic getter for the message
+     * @return the message value of this ErrorResponse
+     */
+    public String getMessage() {
+        return message;
+    }
+
+    /**
+     * Generic setter for the message
+     * @param message   The message to be set on this ErrorResponse
+     */
+    public void setMessage(String message) {
+        this.message = message;
+    }
+}

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatelessAuthenticationFilter.java

@@ -16,6 +16,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.dspace.app.rest.exception.DSpaceBadRequestException;
 import org.dspace.app.rest.utils.ContextUtil;
 import org.dspace.authorize.factory.AuthorizeServiceFactory;
@@ -28,12 +29,15 @@ import org.dspace.services.RequestService;
 import org.dspace.util.UUIDUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
+import org.springframework.stereotype.Component;
+import org.springframework.web.servlet.HandlerExceptionResolver;
 
 /**
  * Custom Spring authentication filter for Stateless authentication, intercepts requests to check for valid
@@ -47,26 +51,32 @@ public class StatelessAuthenticationFilter extends BasicAuthenticationFilter {
     private static final Logger log = LoggerFactory.getLogger(StatelessAuthenticationFilter.class);
 
     private static final String ON_BEHALF_OF_REQUEST_PARAM = "X-On-Behalf-Of";
-    private static final String ADMIN = "ADMIN";
 
     private RestAuthenticationService restAuthenticationService;
 
     private EPersonRestAuthenticationProvider authenticationProvider;
 
+    private HandlerExceptionResolver handlerExceptionResolver;
+
     private RequestService requestService;
 
     private AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService();
 
     private EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
 
+
+    private boolean inErrorOnBehalfOf = false;
+
     public StatelessAuthenticationFilter(AuthenticationManager authenticationManager,
                                          RestAuthenticationService restAuthenticationService,
                                          EPersonRestAuthenticationProvider authenticationProvider,
-                                         RequestService requestService) {
+                                         RequestService requestService,
+                                         HandlerExceptionResolver handlerExceptionResolver) {
         super(authenticationManager);
         this.requestService = requestService;
         this.restAuthenticationService = restAuthenticationService;
         this.authenticationProvider = authenticationProvider;
+        this.handlerExceptionResolver = handlerExceptionResolver;
     }
 
     @Override
@@ -74,16 +84,19 @@ public class StatelessAuthenticationFilter extends BasicAuthenticationFilter {
                                     HttpServletResponse res,
                                     FilterChain chain) throws IOException, ServletException {
 
-        Authentication authentication = getAuthentication(req);
+        inErrorOnBehalfOf = false;
+        Authentication authentication = getAuthentication(req, res);
         if (authentication != null) {
             SecurityContextHolder.getContext().setAuthentication(authentication);
             restAuthenticationService.invalidateAuthenticationCookie(res);
         }
-
+        if (inErrorOnBehalfOf) {
+            return;
+        }
         chain.doFilter(req, res);
     }
 
-    private Authentication getAuthentication(HttpServletRequest request) {
+    private Authentication getAuthentication(HttpServletRequest request, HttpServletResponse res) throws IOException {
 
         if (restAuthenticationService.hasAuthenticationData(request)) {
             // parse the token.
@@ -99,7 +112,7 @@ public class StatelessAuthenticationFilter extends BasicAuthenticationFilter {
                 List<GrantedAuthority> authorities = authenticationProvider.getGrantedAuthorities(context, eperson);
                 String onBehalfOfParameterValue = request.getHeader(ON_BEHALF_OF_REQUEST_PARAM);
                 if (onBehalfOfParameterValue != null) {
-                    return getOnBehalfOfAuthentication(context, onBehalfOfParameterValue);
+                    return getOnBehalfOfAuthentication(context, onBehalfOfParameterValue, request, res);
                 }
 
                 //Return the Spring authentication object
@@ -107,22 +120,36 @@ public class StatelessAuthenticationFilter extends BasicAuthenticationFilter {
             } else {
                 return null;
             }
+        } else {
+            if (request.getHeader(ON_BEHALF_OF_REQUEST_PARAM) != null) {
+                res.setStatus(401);
+                inErrorOnBehalfOf = true;
+            }
         }
 
         return null;
     }
 
-    private Authentication getOnBehalfOfAuthentication(Context context, String onBehalfOfParameterValue) {
+    private byte[] restResponseBytes(ErrorResponse eErrorResponse) throws IOException {
+        String serialized = new ObjectMapper().writeValueAsString(eErrorResponse);
+        return serialized.getBytes();
+    }
+
+    private Authentication getOnBehalfOfAuthentication(Context context, String onBehalfOfParameterValue,
+                                                       HttpServletRequest request,
+                                                       HttpServletResponse res) throws IOException {
         UUID epersonUuid = UUIDUtils.fromString(onBehalfOfParameterValue);
         if (epersonUuid == null) {
-            throw new DSpaceBadRequestException("The parameter value of " + ON_BEHALF_OF_REQUEST_PARAM +
+            inErrorOnBehalfOf = true;
-                " was not a proper UUID");
+            res.sendError(HttpServletResponse.SC_BAD_REQUEST, "THIS IS A TEST");
         }
         try {
             EPerson onBehalfOfEPerson = ePersonService.find(context, epersonUuid);
             if (onBehalfOfEPerson == null) {
-                throw new DSpaceBadRequestException("The parameter value of " + ON_BEHALF_OF_REQUEST_PARAM +
+                res.setStatus(400);
-                                                        " was not a proper EPerson UUID");
+                inErrorOnBehalfOf = true;
+
+                return null;
             }
             if (authorizeService.isAdmin(context)) {
                 requestService.setCurrentUserId(epersonUuid);
@@ -131,8 +158,9 @@ public class StatelessAuthenticationFilter extends BasicAuthenticationFilter {
                                                 authenticationProvider.getGrantedAuthorities(context,
                                                                                              onBehalfOfEPerson));
             } else {
-                throw new AccessDeniedException("The current EPerson was not an admin and " +
+                res.setStatus(403);
-                                                    "cannot use the on behalf of functionality");
+                inErrorOnBehalfOf = true;
+                return null;
             }
 
         } catch (SQLException e) {

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityConfiguration.java

@@ -24,6 +24,7 @@ import org.springframework.security.config.http.SessionCreationPolicy;
 import org.springframework.security.web.authentication.logout.HttpStatusReturningLogoutSuccessHandler;
 import org.springframework.security.web.authentication.logout.LogoutFilter;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
+import org.springframework.web.servlet.HandlerExceptionResolver;
 
 /**
  * Spring Security configuration for DSpace Spring Rest
@@ -53,6 +54,9 @@ public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {
     @Autowired
     private CustomLogoutHandler customLogoutHandler;
 
+    @Autowired
+    private HandlerExceptionResolver handlerExceptionResolver;
+
     @Override
     public void configure(WebSecurity webSecurity) throws Exception {
         webSecurity
@@ -118,7 +122,7 @@ public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {
             // Add a custom Token based authentication filter based on the token previously given to the client
             // before each URL
             .addFilterBefore(new StatelessAuthenticationFilter(authenticationManager(), restAuthenticationService,
-                                                               ePersonRestAuthenticationProvider, requestService),
+                                                               ePersonRestAuthenticationProvider, requestService, handlerExceptionResolver),
                              StatelessLoginFilter.class);
     }
 

dspace-server-webapp/src/test/java/org/dspace/app/rest/LoginAsEPersonIT.java

@@ -11,6 +11,8 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+import java.util.UUID;
+
 import org.dspace.app.rest.matcher.EPersonMatcher;
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
 import org.junit.Test;
@@ -43,4 +45,45 @@ public class LoginAsEPersonIT extends AbstractControllerIntegrationTest {
 
 
     }
+
+    @Test
+    public void loggedInAsOtherUserNotAUuidInHeaderBadRequestRetrievalTest() throws Exception {
+
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(get("/api/authn/status")
+                                     .param("projection", "full")
+                                     .header("X-On-Behalf-Of", "not-a-uuid"))
+                        .andExpect(status().isBadRequest());
+
+
+    }
+
+    @Test
+    public void loggedInAsOtherUserWrongUuidInHeaderBadRequestRetrievalTest() throws Exception {
+
+        String token = getAuthToken(admin.getEmail(), password);
+
+        getClient(token).perform(get("/api/authn/status")
+                                     .param("projection", "full")
+                                     .header("X-On-Behalf-Of", UUID.randomUUID()))
+                        .andExpect(status().isBadRequest());
+
+
+    }
+
+    @Test
+    public void loggedInAsOtherUserNoPermissionForbiddenRetrievalTest() throws Exception {
+
+
+        String token = getAuthToken(eperson.getEmail(), password);
+
+        getClient(token).perform(get("/api/authn/status")
+                                     .param("projection", "full")
+                                     .header("X-On-Behalf-Of", eperson.getID()))
+                        .andExpect(status().isForbidden());
+
+
+    }
+
 }