diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ErrorResponse.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ErrorResponse.java new file mode 100644 index 0000000000..960f37470b --- /dev/null +++ b/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; + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatelessAuthenticationFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatelessAuthenticationFilter.java index 19886c498c..e28c2c64b2 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatelessAuthenticationFilter.java +++ b/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 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 + - " was not a proper UUID"); + inErrorOnBehalfOf = true; + 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 + - " was not a proper EPerson UUID"); + res.setStatus(400); + 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 " + - "cannot use the on behalf of functionality"); + res.setStatus(403); + inErrorOnBehalfOf = true; + return null; } } catch (SQLException e) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityConfiguration.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityConfiguration.java index 32c0cdda00..4daa83c53f 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityConfiguration.java +++ b/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); } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/LoginAsEPersonIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/LoginAsEPersonIT.java index 7ceb48a963..bd55da318e 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/LoginAsEPersonIT.java +++ b/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()); + + + } + }