Skip to content

Commit

Permalink
Merge pull request #2765 from tdonohue/authentication_test_cleanup
Browse files Browse the repository at this point in the history 
Add JavaDocs to JWTTokenHolder, cleanup and enhance related Tests
  • Loading branch information
@tdonohue
tdonohue committed May 21, 2020
2 parents 9dd0e69 + ed3c2e3 commit 1376563
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 40 deletions.
100 changes: 85 additions & 15 deletions dspace-server-webapp/src/main/java/org/dspace/app/rest/security/jwt/JWTTokenHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@
import org.springframework.stereotype.Component;

/**
* Class responsible for creating and parsing JWTs, supports both JWS and JWE
* Class responsible for creating and parsing JSON Web Tokens (JWTs), supports both JWS and JWE
* https://jwt.io/
*
* @author Frederic Van Reet (frederic dot vanreet at atmire dot com)
* @author Tom Desair (tom dot desair at atmire dot com)
Expand Down Expand Up @@ -95,12 +96,12 @@ public void afterPropertiesSet() throws Exception {
}

/**
* Retrieve EPerson from a jwt
* Retrieve EPerson from a JSON Web Token (JWT)
*
* @param token
* @param request
* @param context
* @return
* @param token token as a string
* @param request current request
* @param context current Context
* @return DSpace EPerson object parsed from the token
* @throws JOSEException
* @throws ParseException
* @throws SQLException
Expand All @@ -110,13 +111,14 @@ public EPerson parseEPersonFromToken(String token, HttpServletRequest request, C
if (StringUtils.isBlank(token)) {
return null;
}

// parse/decrypt the token
SignedJWT signedJWT = getSignedJWT(token);

// get the claims set from the parsed token
JWTClaimsSet jwtClaimsSet = signedJWT.getJWTClaimsSet();

// retrieve the EPerson from the claims set
EPerson ePerson = getEPerson(context, jwtClaimsSet);

// As long as the JWT is valid, parse all claims and return the EPerson
if (isValidToken(request, signedJWT, jwtClaimsSet, ePerson)) {

log.debug("Received valid token for username: " + ePerson.getEmail());
Expand All @@ -133,22 +135,25 @@ public EPerson parseEPersonFromToken(String token, HttpServletRequest request, C
}

/**
* Create a jwt with the EPerson details in it
* Create a JWT with the EPerson details in it
*
* @param context
* @param request
* @param previousLoginDate
* @param groups
* @return
* @param context current Context
* @param request current Request
* @param previousLoginDate date of last login (before this one)
* @param groups List of user Groups
* @return string version of signed JWT
* @throws JOSEException
*/
public String createTokenForEPerson(Context context, HttpServletRequest request, Date previousLoginDate,
List<Group> groups) throws JOSEException, SQLException {

// Update the saved session salt for the currently logged in user, returning the user object
EPerson ePerson = updateSessionSalt(context, previousLoginDate);

// Create a claims set based on currently logged in user
JWTClaimsSet claimsSet = buildJwtClaimsSet(context, request);

// Create a signed JWT from those two things
SignedJWT signedJWT = createSignedJWT(request, ePerson, claimsSet);

String token;
Expand All @@ -161,6 +166,13 @@ public String createTokenForEPerson(Context context, HttpServletRequest request,
return token;
}

/**
* Invalidate the current Java Web Token (JWT) in the current request
* @param token current token
* @param request current request
* @param context current Context
* @throws Exception
*/
public void invalidateToken(String token, HttpServletRequest request, Context context) throws Exception {
if (StringUtils.isNotBlank(token)) {

Expand Down Expand Up @@ -197,6 +209,17 @@ private JWEObject encryptJWT(SignedJWT signedJWT) throws JOSEException {
return jweObject;
}

/**
* Determine if current JWT is valid for the given EPerson object.
* To be valid, current JWT *must* have been signed by the EPerson and not be expired.
* If EPerson is null or does not have a known active session, false is returned immediately.
* @param request current request
* @param signedJWT current signed JWT
* @param jwtClaimsSet claims set of current JWT
* @param ePerson EPerson parsed from current signed JWT
* @return true if valid, false otherwise
* @throws JOSEException
*/
private boolean isValidToken(HttpServletRequest request, SignedJWT signedJWT, JWTClaimsSet jwtClaimsSet,
EPerson ePerson) throws JOSEException {
if (ePerson == null || StringUtils.isBlank(ePerson.getSessionSalt())) {
Expand All @@ -213,6 +236,15 @@ private boolean isValidToken(HttpServletRequest request, SignedJWT signedJWT, JW
}
}

/**
* Return the signed JWT.
* If JWT encryption is enabled, decrypt the token and return.
* Otherwise, parse the string into a signed JWT
* @param token string token
* @return parsed (possibly decrypted) SignedJWT
* @throws ParseException
* @throws JOSEException
*/
private SignedJWT getSignedJWT(String token) throws ParseException, JOSEException {
SignedJWT signedJWT;

Expand All @@ -227,10 +259,26 @@ private SignedJWT getSignedJWT(String token) throws ParseException, JOSEExceptio
return signedJWT;
}

/**
* Based on the given JWT claims set (which should include an EPerson ID), locate the
* corresponding EPerson in the current Context
* @param context current context
* @param jwtClaimsSet JWT claims set
* @return EPerson object (or null, if not found)
* @throws SQLException
*/
private EPerson getEPerson(Context context, JWTClaimsSet jwtClaimsSet) throws SQLException {
return ePersonClaimProvider.getEPerson(context, jwtClaimsSet);
}

/**
* Create a signed JWT from the given EPerson and claims set.
* @param request current request
* @param ePerson EPerson to create signed JWT for
* @param claimsSet claims set of JWT
* @return signed JWT
* @throws JOSEException
*/
private SignedJWT createSignedJWT(HttpServletRequest request, EPerson ePerson, JWTClaimsSet claimsSet)
throws JOSEException {
SignedJWT signedJWT = new SignedJWT(
Expand All @@ -241,6 +289,13 @@ private SignedJWT createSignedJWT(HttpServletRequest request, EPerson ePerson, J
return signedJWT;
}

/**
* Create a new JWT claims set based on the current Context (and currently logged in user).
* Set its expiration time based on the configured expiration period.
* @param context current Context
* @param request current Request
* @return new JWTClaimsSet
*/
private JWTClaimsSet buildJwtClaimsSet(Context context, HttpServletRequest request) {
JWTClaimsSet.Builder builder = new JWTClaimsSet.Builder();

Expand Down Expand Up @@ -283,6 +338,16 @@ private String getIpAddress(HttpServletRequest request) {
return clientInfoService.getClientIp(request);
}


/**
* Update session salt information for the currently logged in user.
* The session salt is a random key that is saved to EPerson object (and database table) and used to validate
* a JWT on later requests.
* @param context current DSpace Context
* @param previousLoginDate date of last login (prior to this one)
* @return EPerson object of current user, with an updated session salt
* @throws SQLException
*/
private EPerson updateSessionSalt(final Context context, final Date previousLoginDate) throws SQLException {
EPerson ePerson;

Expand All @@ -306,6 +371,11 @@ private EPerson updateSessionSalt(final Context context, final Date previousLogi
return ePerson;
}

/**
* Retrieve the given secret key from configuration. If not specified, generate a random 32 byte key
* @param property configuration property to check for
* @return configuration value or random 32 byte key
*/
private String getSecret(String property) {
String secret = configurationService.getProperty(property);

Expand Down
46 changes: 21 additions & 25 deletions dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,9 @@ public void testStatusNotAuthenticated() throws Exception {

@Test
public void testStatusAuthenticatedWithCookie() throws Exception {
context.turnOffAuthorisationSystem();
//Enable Shibboleth login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_ONLY);

context.restoreAuthSystemState();

//Simulate that a shibboleth authentication has happened
String token = getClient().perform(post("/api/authn/login")
.requestAttr("SHIB-MAIL", eperson.getEmail())
Expand Down Expand Up @@ -603,12 +600,9 @@ public void testShibbolethLoginRequestHeaderWithIpAuthentication() throws Except

@Test
public void testShibbolethAndPasswordAuthentication() throws Exception {
context.turnOffAuthorisationSystem();
//Enable Shibboleth and password login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_AND_PASS);

context.restoreAuthSystemState();

//Check if WWW-Authenticate header contains shibboleth and password
getClient().perform(get("/api/authn/status").header("Referer", "http://my.uni.edu"))
.andExpect(status().isOk())
Expand Down Expand Up @@ -658,16 +652,20 @@ public void testShibbolethAndPasswordAuthentication() throws Exception {
getClient(token).perform(get("/api/authn/logout"))
.andExpect(status().isNoContent());

//Check if we are actually logged out (again)
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")));

}

@Test
public void testOnlyPasswordAuthenticationWorks() throws Exception {
context.turnOffAuthorisationSystem();
//Enable only password login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);

context.restoreAuthSystemState();

//Check if WWW-Authenticate header contains only
getClient().perform(get("/api/authn/status").header("Referer", "http://my.uni.edu"))
.andExpect(status().isOk())
Expand All @@ -687,16 +685,20 @@ public void testOnlyPasswordAuthenticationWorks() throws Exception {
//Logout
getClient(token).perform(get("/api/authn/logout"))
.andExpect(status().isNoContent());

//Check if we are actually logged out
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")));
}

@Test
public void testShibbolethAuthenticationDoesNotWorkWithPassOnly() throws Exception {
context.turnOffAuthorisationSystem();
//Enable only password login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", PASS_ONLY);

context.restoreAuthSystemState();

//Check if WWW-Authenticate header contains only password
getClient().perform(get("/api/authn/status").header("Referer", "http://my.uni.edu"))
.andExpect(status().isOk())
Expand All @@ -713,12 +715,9 @@ public void testShibbolethAuthenticationDoesNotWorkWithPassOnly() throws Excepti

@Test
public void testOnlyShibbolethAuthenticationWorks() throws Exception {
context.turnOffAuthorisationSystem();
//Enable only Shibboleth login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_ONLY);

context.restoreAuthSystemState();

//Check if WWW-Authenticate header contains only shibboleth
getClient().perform(get("/api/authn/status").header("Referer", "http://my.uni.edu"))
.andExpect(status().isOk())
Expand All @@ -738,23 +737,20 @@ public void testOnlyShibbolethAuthenticationWorks() throws Exception {
//Logout
getClient(token).perform(get("/api/authn/logout"))
.andExpect(status().isNoContent());

//Check if we are actually logged out
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")));
}

@Test
public void testPasswordAuthenticationDoesNotWorkWithShibOnly() throws Exception {
context.turnOffAuthorisationSystem();
//Enable only Shibboleth login
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_ONLY);

//Create a reviewers group
Group reviewersGroup = GroupBuilder.createGroup(context)
.withName("Reviewers")
.build();

//Faculty members are assigned to the Reviewers group
configurationService.setProperty("authentication-shibboleth.role.faculty", "Reviewers");
context.restoreAuthSystemState();

getClient().perform(post("/api/authn/login")
.param("user", eperson.getEmail())
.param("password", password))
Expand Down
13 changes: 13 additions & 0 deletions dspace-server-webapp/src/test/java/org/dspace/app/rest/security/jwt/JWTTokenHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,17 @@ public void testTokenTampering() throws Exception {
assertEquals(null, parsed);
}

@Test
public void testInvalidatedToken() throws Exception {
Date previous = new Date(System.currentTimeMillis() - 10000000000L);
// create a new token
String token = jwtTokenHandler
.createTokenForEPerson(context, new MockHttpServletRequest(), previous, new ArrayList<>());
// immediately invalidate it
jwtTokenHandler.invalidateToken(token, new MockHttpServletRequest(), context);
// Check if it is still valid by trying to parse the EPerson from it (should return null)
EPerson parsed = jwtTokenHandler.parseEPersonFromToken(token, httpServletRequest, context);
assertEquals(null, parsed);
}

}

0 comments on commit 1376563

Please sign in to comment.