mirror of
https://github.com/DSpace/DSpace.git
synced 2025-10-07 01:54:22 +00:00
Require POST for logout. Update CSRFTokenRepository to no longer allow server token to be read by Javascript
This commit is contained in:
Tim Donohue
@@ -85,6 +85,16 @@ public class AuthenticationRestController implements InitializingBean {
|
||||
return converter.toResource(authnRest);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check the current user's authentication status (i.e. whether they are authenticated or not)
|
||||
* <P>
|
||||
* If the user is NOT currently authenticated, a list of all currently enabled DSpace authentication endpoints
|
||||
* is returned in the WWW-Authenticate header.
|
||||
* @param request current request
|
||||
* @param response response
|
||||
* @return AuthenticationStatusResource
|
||||
* @throws SQLException
|
||||
*/
|
||||
@RequestMapping(value = "/status", method = RequestMethod.GET)
|
||||
public AuthenticationStatusResource status(HttpServletRequest request, HttpServletResponse response)
|
||||
throws SQLException {
|
||||
@@ -96,8 +106,7 @@ public class AuthenticationRestController implements InitializingBean {
|
||||
}
|
||||
|
||||
AuthenticationStatusRest authenticationStatusRest = new AuthenticationStatusRest(ePersonRest);
|
||||
// Whether authentication status is false add WWW-Authenticate so client can retrieve the available
|
||||
// authentication methods
|
||||
// When not authenticated add WWW-Authenticate so client can retrieve all available authentication methods
|
||||
if (!authenticationStatusRest.isAuthenticated()) {
|
||||
String authenticateHeaderValue = restAuthenticationService
|
||||
.getWwwAuthenticateHeaderValue(request, response);
|
||||
@@ -110,13 +119,23 @@ public class AuthenticationRestController implements InitializingBean {
|
||||
return authenticationStatusResource;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether the login has succeeded or not. The actual login is performed by one of the enabled login filters
|
||||
* (e.g. {@link org.dspace.app.rest.security.StatelessLoginFilter}).
|
||||
* See {@link org.dspace.app.rest.security.WebSecurityConfiguration} for enabled login filters.
|
||||
*
|
||||
* @param request current request
|
||||
* @param user user
|
||||
* @param password password
|
||||
* @return ResponseEntity with information about whether login was successful or failed
|
||||
*/
|
||||
@RequestMapping(value = "/login", method = {RequestMethod.POST})
|
||||
public ResponseEntity login(HttpServletRequest request, @RequestParam(name = "user", required = false) String user,
|
||||
@RequestParam(name = "password", required = false) String password) {
|
||||
//If you can get here, you should be authenticated, the actual login is handled by spring security
|
||||
//see org.dspace.app.rest.security.StatelessLoginFilter
|
||||
|
||||
//If we don't have an EPerson here, this means authentication failed and we should return an error message.
|
||||
// Build our response. This will check if we have an EPerson.
|
||||
// If not, that means the authentication failed and we should return the error message
|
||||
return getLoginResponse(request,
|
||||
"Authentication failed. The credentials you provided are not valid.");
|
||||
}
|
||||
@@ -145,18 +164,49 @@ public class AuthenticationRestController implements InitializingBean {
|
||||
return converter.toResource(authenticationTokenRest);
|
||||
}
|
||||
|
||||
/**
|
||||
* Disables GET/PUT/PATCH on the /login endpoint. You must use POST (see above method)
|
||||
* @return ResponseEntity
|
||||
*/
|
||||
@RequestMapping(value = "/login", method = { RequestMethod.GET, RequestMethod.PUT, RequestMethod.PATCH,
|
||||
RequestMethod.DELETE })
|
||||
public ResponseEntity login() {
|
||||
return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body("Only POST is allowed for login requests.");
|
||||
}
|
||||
|
||||
@RequestMapping(value = "/logout", method = {RequestMethod.GET, RequestMethod.POST})
|
||||
/**
|
||||
* Returns a successful "204 No Content" response for a logout request.
|
||||
* Actual logout is performed by our {@link org.dspace.app.rest.security.CustomLogoutHandler}
|
||||
* <P>
|
||||
* For logout we *require* POST requests. HEAD is also supported for endpoint visibility in HAL Browser, etc.
|
||||
* @return ResponseEntity (204 No Content)
|
||||
*/
|
||||
@RequestMapping(value = "/logout", method = {RequestMethod.HEAD, RequestMethod.POST})
|
||||
public ResponseEntity logout() {
|
||||
//This is handled by org.dspace.app.rest.security.CustomLogoutHandler
|
||||
return ResponseEntity.noContent().build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Disables GET/PUT/PATCH on the /logout endpoint. You must use POST (see above method)
|
||||
* @return ResponseEntity
|
||||
*/
|
||||
@RequestMapping(value = "/logout", method = { RequestMethod.GET, RequestMethod.PUT, RequestMethod.PATCH,
|
||||
RequestMethod.DELETE })
|
||||
public ResponseEntity logoutMethodNotAllowed() {
|
||||
return ResponseEntity.status(HttpStatus.METHOD_NOT_ALLOWED).body("Only POST is allowed for logout requests.");
|
||||
}
|
||||
|
||||
/**
|
||||
* Check the request to see if the login succeeded or failed.
|
||||
* If the request includes a valid EPerson, then it was successful.
|
||||
* If the request does not include a valid EPerson, then return the failedMessage.
|
||||
* <P>
|
||||
* NOTE: This method assumes that a login filter (e.g. {@link org.dspace.app.rest.security.StatelessLoginFilter})
|
||||
* has already attempted the authentication and, if successful, added EPerson data to the current request.
|
||||
* @param request current request
|
||||
* @param failedMessage message to send if no EPerson found
|
||||
* @return ResponseEntity
|
||||
*/
|
||||
protected ResponseEntity getLoginResponse(HttpServletRequest request, String failedMessage) {
|
||||
//Get the context and check if we have an authenticated eperson
|
||||
org.dspace.core.Context context = null;
|
||||
|
||||
@@ -19,7 +19,14 @@ import org.springframework.security.web.csrf.MissingCsrfTokenException;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
/**
|
||||
* This Handler customizes behavior of AccessDeniedException errors thrown by Spring Security/Boot
|
||||
* This Handler customizes behavior of AccessDeniedException errors thrown by Spring Security/Boot.
|
||||
* <P>
|
||||
* More specifically, we use this Handler to ensure exceptions related to CSRF Tokens are also sent to our
|
||||
* DSpaceApiExceptionControllerAdvice class, which manages all exceptions for the DSpace backend. Without this
|
||||
* handler, those CSRF exceptions are managed by Spring Security/Boot *before* DSpaceApiExceptionControllerAdvice
|
||||
* is triggered.
|
||||
*
|
||||
* @see DSpaceApiExceptionControllerAdvice
|
||||
*/
|
||||
@Component
|
||||
public class DSpaceAccessDeniedHandler implements AccessDeniedHandler {
|
||||
|
||||
@@ -114,8 +114,8 @@ public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {
|
||||
.logout()
|
||||
// On logout, clear the "session" salt
|
||||
.addLogoutHandler(customLogoutHandler)
|
||||
// Configure the logout entry point
|
||||
.logoutRequestMatcher(new AntPathRequestMatcher("/api/authn/logout"))
|
||||
// Configure the logout entry point & require POST
|
||||
.logoutRequestMatcher(new AntPathRequestMatcher("/api/authn/logout", HttpMethod.POST.name()))
|
||||
// When logout is successful, return OK (204) status
|
||||
.logoutSuccessHandler(new HttpStatusReturningLogoutSuccessHandler(HttpStatus.NO_CONTENT))
|
||||
// Everyone can call this endpoint
|
||||
@@ -162,8 +162,7 @@ public class WebSecurityConfiguration extends WebSecurityConfigurerAdapter {
|
||||
* @return CsrfTokenRepository as described above
|
||||
*/
|
||||
private CsrfTokenRepository getCsrfTokenRepository() {
|
||||
// NOTE: Created cookie is set to HttpOnly=false to allow Hal Browser (or other local JS clients) to access it.
|
||||
return DSpaceCsrfTokenRepository.withHttpOnlyFalse();
|
||||
return new DSpaceCsrfTokenRepository();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -83,21 +83,18 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
@Before
|
||||
public void setup() throws Exception {
|
||||
super.setUp();
|
||||
// Default all tests to Password Authentication only
|
||||
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);
|
||||
}
|
||||
|
||||
@Test
|
||||
@Ignore
|
||||
// Ignored until an endpoint is added to return all groups. Anonymous is not considered a direct group.
|
||||
public void testStatusAuthenticated() throws Exception {
|
||||
String token = getAuthToken(eperson.getEmail(), password);
|
||||
public void testStatusAuthenticatedAsAdmin() throws Exception {
|
||||
String token = getAuthToken(admin.getEmail(), password);
|
||||
|
||||
getClient(token).perform(get("/api/authn/status").param("projection", "full"))
|
||||
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$", AuthenticationStatusMatcher.matchFullEmbeds()))
|
||||
.andExpect(jsonPath("$", AuthenticationStatusMatcher.matchLinks()))
|
||||
//We expect the content type to be "application/hal+json;charset=UTF-8"
|
||||
.andExpect(content().contentType(contentType))
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(true)))
|
||||
@@ -105,23 +102,51 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
|
||||
.andExpect(jsonPath("$._links.eperson.href", startsWith(REST_SERVER_URL)))
|
||||
.andExpect(jsonPath("$._embedded.eperson",
|
||||
EPersonMatcher.matchEPersonWithGroups(eperson.getEmail(), "Anonymous")))
|
||||
;
|
||||
EPersonMatcher.matchEPersonWithGroups(admin.getEmail(), "Administrator")));
|
||||
|
||||
getClient(token).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()));
|
||||
|
||||
// Logout
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
}
|
||||
|
||||
@Test
|
||||
@Ignore
|
||||
// Ignored until an endpoint is added to return all groups. Anonymous is not considered a direct group.
|
||||
public void testStatusAuthenticatedAsNormalUser() throws Exception {
|
||||
String token = getAuthToken(eperson.getEmail(), password);
|
||||
|
||||
getClient(token).perform(get("/api/authn/status").param("projection", "full"))
|
||||
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$", AuthenticationStatusMatcher.matchFullEmbeds()))
|
||||
.andExpect(jsonPath("$", AuthenticationStatusMatcher.matchLinks()))
|
||||
.andExpect(content().contentType(contentType))
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(true)))
|
||||
.andExpect(jsonPath("$.type", is("status")))
|
||||
|
||||
.andExpect(jsonPath("$._links.eperson.href", startsWith(REST_SERVER_URL)))
|
||||
.andExpect(jsonPath("$._embedded.eperson",
|
||||
EPersonMatcher.matchEPersonWithGroups(eperson.getEmail(), "Anonymous")));
|
||||
|
||||
getClient(token).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()))
|
||||
;
|
||||
.andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()));
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testStatusNotAuthenticated() throws Exception {
|
||||
|
||||
getClient().perform(get("/api/authn/status"))
|
||||
|
||||
.andExpect(status().isOk())
|
||||
|
||||
//We expect the content type to be "application/hal+json;charset=UTF-8"
|
||||
.andExpect(content().contentType(contentType))
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(false)))
|
||||
@@ -157,7 +182,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
}
|
||||
|
||||
@@ -184,7 +209,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andExpect(jsonPath("$.authenticated", is(true)))
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
}
|
||||
|
||||
@@ -277,17 +302,28 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
|
||||
getClient(token).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(true)))
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
// Verify logout via GET does NOT work (throws a 405)
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
.andExpect(status().isMethodNotAllowed());
|
||||
|
||||
// Verify we are still logged in
|
||||
getClient(token).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(true)))
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
// Verify logout via POST works
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
|
||||
// Verify we are now logged out (authenticated=false)
|
||||
getClient(token).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(false)))
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
@@ -304,7 +340,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
|
||||
assertNotEquals(token1, token2);
|
||||
|
||||
getClient(token1).perform(get("/api/authn/logout"));
|
||||
getClient(token1).perform(post("/api/authn/logout"));
|
||||
|
||||
getClient(token1).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
@@ -365,7 +401,32 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
// Should return a 403 Forbidden, for an invalid CSRF token
|
||||
.andExpect(status().isForbidden());
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
}
|
||||
|
||||
@Test
|
||||
// This test is verifying that Spring Security's CORS settings are working as we expect
|
||||
public void testCannotReuseTokenFromUntrustedOrigin() throws Exception {
|
||||
// First, get a valid login token
|
||||
String token = getAuthToken(eperson.getEmail(), password);
|
||||
|
||||
// Verify token works
|
||||
getClient(token).perform(get("/api/authn/status"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$.okay", is(true)))
|
||||
.andExpect(jsonPath("$.authenticated", is(true)))
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
// Test token cannot be used from an *untrusted* Origin
|
||||
// (NOTE: this Origin is NOT listed in our 'rest.cors.allowed-origins' configuration)
|
||||
getClient(token).perform(get("/api/authn/status")
|
||||
.header("Origin", "https://example.org"))
|
||||
// should result in a 403 error as Spring Security returns that for untrusted origins
|
||||
.andExpect(status().isForbidden());
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
}
|
||||
|
||||
@@ -467,7 +528,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
|
||||
//Check if we are actually logged out
|
||||
@@ -530,7 +591,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testShibbolethLoginURLWithServerlURLConteiningPort() throws Exception {
|
||||
public void testShibbolethLoginURLWithServerURLContainingPort() throws Exception {
|
||||
context.turnOffAuthorisationSystem();
|
||||
//Enable Shibboleth login
|
||||
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_ONLY);
|
||||
@@ -737,7 +798,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
|
||||
//Check if we are actually logged out
|
||||
@@ -762,7 +823,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
|
||||
//Check if we are actually logged out (again)
|
||||
@@ -796,7 +857,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andExpect(jsonPath("$.type", is("status")));
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
|
||||
//Check if we are actually logged out
|
||||
@@ -847,7 +908,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
.andReturn().getResponse().getHeader(AUTHORIZATION_HEADER);
|
||||
|
||||
//Logout
|
||||
getClient(token).perform(get("/api/authn/logout"))
|
||||
getClient(token).perform(post("/api/authn/logout"))
|
||||
.andExpect(status().isNoContent());
|
||||
|
||||
//Check if we are actually logged out
|
||||
@@ -892,7 +953,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testShortLivedTokenToDowloadBitstream() throws Exception {
|
||||
public void testShortLivedTokenToDownloadBitstream() throws Exception {
|
||||
Bitstream bitstream = createPrivateBitstream();
|
||||
String shortLivedToken = getShortLivedToken(eperson);
|
||||
|
||||
@@ -902,7 +963,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testShortLivedTokenToDowloadBitstreamUnauthorized() throws Exception {
|
||||
public void testShortLivedTokenToDownloadBitstreamUnauthorized() throws Exception {
|
||||
Bitstream bitstream = createPrivateBitstream();
|
||||
|
||||
context.turnOffAuthorisationSystem();
|
||||
@@ -920,7 +981,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLoginTokenToDowloadBitstream() throws Exception {
|
||||
public void testLoginTokenToDownloadBitstream() throws Exception {
|
||||
Bitstream bitstream = createPrivateBitstream();
|
||||
|
||||
String loginToken = getAuthToken(eperson.getEmail(), password);
|
||||
@@ -930,7 +991,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testExpiredShortLivedTokenToDowloadBitstream() throws Exception {
|
||||
public void testExpiredShortLivedTokenToDownloadBitstream() throws Exception {
|
||||
Bitstream bitstream = createPrivateBitstream();
|
||||
configurationService.setProperty("jwt.shortLived.token.expiration", "1");
|
||||
String shortLivedToken = getShortLivedToken(eperson);
|
||||
|
||||
Reference in New Issue
Block a user