71343: Authorization for Downloads of restricted Bitstreams #2

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

@@ -42,7 +42,6 @@ import org.dspace.service.ClientInfoService;
 import org.dspace.services.ConfigurationService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.crypto.keygen.BytesKeyGenerator;
 import org.springframework.security.crypto.keygen.KeyGenerators;
@@ -55,7 +54,7 @@ import org.springframework.security.crypto.keygen.KeyGenerators;
  * @author Frederic Van Reet (frederic dot vanreet at atmire dot com)
  * @author Tom Desair (tom dot desair at atmire dot com)
  */
-public abstract class JWTTokenHandler implements InitializingBean {
+public abstract class JWTTokenHandler {
 
     private static final int MAX_CLOCK_SKEW_SECONDS = 60;
     private static final Logger log = LoggerFactory.getLogger(JWTTokenHandler.class);
@@ -75,30 +74,8 @@ public abstract class JWTTokenHandler implements InitializingBean {
     @Autowired
     private ClientInfoService clientInfoService;
 
-    private String jwtKey;
+    private String generatedJwtKey;
-    private long expirationTime;
+    private String generatedEncryptionKey;
-    private boolean includeIP;
-    private boolean encryptionEnabled;
-    private boolean compressionEnabled;
-    private byte[] encryptionKey;
-
-
-    @Override
-    public void afterPropertiesSet() throws Exception {
-        this.jwtKey =
-            getSecret(getTokenSecretConfigurationKey());
-        this.encryptionKey =
-            getSecret(getEncryptionSecretConfigurationKey()).getBytes();
-
-        this.expirationTime =
-            configurationService.getLongProperty(getTokenExpirationConfigurationKey(), 30);
-        this.includeIP =
-            configurationService.getBooleanProperty(getTokenIncludeIPConfigurationKey(), true);
-        this.encryptionEnabled =
-            configurationService.getBooleanProperty(getEncryptionEnabledConfigurationKey(), false);
-        this.compressionEnabled =
-            configurationService.getBooleanProperty(getCompressionEnabledConfigurationKey(), false);
-    }
 
     /**
      * Get the configuration property key for the token secret.
@@ -225,17 +202,54 @@ public abstract class JWTTokenHandler implements InitializingBean {
         }
     }
 
-    public long getExpirationPeriod() {
+    /**
-        return expirationTime;
+     * Retrieve the token secret key from configuration. If not specified, generate and cache a random 32 byte key
+     * @return configuration value or random 32 byte key
+     */
+    public String getJwtKey() {
+        String secret = configurationService.getProperty(getTokenSecretConfigurationKey());
+
+        if (StringUtils.isBlank(secret)) {
+            if (StringUtils.isBlank(generatedJwtKey)) {
+                generatedJwtKey = generateRandomKey();
+            }
+            secret = generatedJwtKey;
+        }
+
+        return secret;
     }
 
+    public boolean getIncludeIP() {
+        return configurationService.getBooleanProperty(getTokenIncludeIPConfigurationKey(), true);
+    }
+
+    public long getExpirationPeriod() {
+        return configurationService.getLongProperty(getTokenExpirationConfigurationKey(), 30);
+    }
 
     public boolean isEncryptionEnabled() {
-        return encryptionEnabled;
+        return configurationService.getBooleanProperty(getEncryptionEnabledConfigurationKey(), false);
     }
 
+    public boolean getCompressionEnabled() {
+        return configurationService.getBooleanProperty(getCompressionEnabledConfigurationKey(), false);
+    }
+
+    /**
+     * Retrieve the encryption secret key from configuration. If not specified, generate and cache a random 32 byte key
+     * @return configuration value or random 32 byte key
+     */
     public byte[] getEncryptionKey() {
-        return encryptionKey;
+        String secretString = configurationService.getProperty(getEncryptionSecretConfigurationKey());
+
+        if (StringUtils.isBlank(secretString)) {
+            if (StringUtils.isBlank(generatedEncryptionKey)) {
+                generatedEncryptionKey = generateRandomKey();
+            }
+            secretString = generatedEncryptionKey;
+        }
+
+        return secretString.getBytes();
     }
 
     private JWEObject encryptJWT(SignedJWT signedJWT) throws JOSEException {
@@ -261,7 +275,7 @@ public abstract class JWTTokenHandler implements InitializingBean {
      * @return true if valid, false otherwise
      * @throws JOSEException
      */
-    private boolean isValidToken(HttpServletRequest request, SignedJWT signedJWT, JWTClaimsSet jwtClaimsSet,
+    protected boolean isValidToken(HttpServletRequest request, SignedJWT signedJWT, JWTClaimsSet jwtClaimsSet,
                                  EPerson ePerson) throws JOSEException {
         if (ePerson == null || StringUtils.isBlank(ePerson.getSessionSalt())) {
             return false;
@@ -351,7 +365,7 @@ public abstract class JWTTokenHandler implements InitializingBean {
 
     //This method makes compression configurable
     private JWEHeader.Builder compression(JWEHeader.Builder builder) {
-        if (compressionEnabled) {
+        if (getCompressionEnabled()) {
             return builder.compressionAlgorithm(CompressionAlgorithm.DEF);
         }
         return builder;
@@ -367,12 +381,12 @@ public abstract class JWTTokenHandler implements InitializingBean {
      * @param ePerson
      * @return
      */
-    private String buildSigningKey(HttpServletRequest request, EPerson ePerson) {
+    protected String buildSigningKey(HttpServletRequest request, EPerson ePerson) {
         String ipAddress = "";
-        if (includeIP) {
+        if (getIncludeIP()) {
             ipAddress = getIpAddress(request);
         }
-        return jwtKey + ePerson.getSessionSalt() + ipAddress;
+        return getJwtKey() + ePerson.getSessionSalt() + ipAddress;
     }
 
     private String getIpAddress(HttpServletRequest request) {
@@ -399,7 +413,7 @@ public abstract class JWTTokenHandler implements InitializingBean {
             //This allows a user to login on multiple devices/browsers at the same time.
             if (StringUtils.isBlank(ePerson.getSessionSalt())
                 || previousLoginDate == null
-                || (ePerson.getLastActive().getTime() - previousLoginDate.getTime() > expirationTime)) {
+                || (ePerson.getLastActive().getTime() - previousLoginDate.getTime() > getExpirationPeriod())) {
 
                 ePerson.setSessionSalt(generateRandomKey());
                 ePersonService.update(context, ePerson);
@@ -412,21 +426,6 @@ public abstract class JWTTokenHandler implements InitializingBean {
         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);
-
-        if (StringUtils.isBlank(secret)) {
-            secret = generateRandomKey();
-        }
-
-        return secret;
-    }
-
     /**
      * Generate a random 32 bytes key
      */

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

@@ -47,6 +47,7 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
     private static final String AUTHORIZATION_COOKIE = "Authorization-cookie";
     private static final String AUTHORIZATION_HEADER = "Authorization";
     private static final String AUTHORIZATION_TYPE = "Bearer";
+    private static final String AUTHORIZATION_TOKEN_PARAMETER = "token";
 
     @Autowired
     private SessionJWTTokenHandler sessionJWTTokenHandler;
@@ -112,9 +113,15 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
 
     @Override
     public EPerson getAuthenticatedEPerson(HttpServletRequest request, Context context) {
-        String token = getToken(request);
         try {
-            EPerson ePerson = sessionJWTTokenHandler.parseEPersonFromToken(token, request, context);
+            String token = getSessionToken(request);
+            EPerson ePerson = null;
+            if (token == null) {
+                token = getShortLivedToken(request);
+                ePerson = shortLivedJWTTokenHandler.parseEPersonFromToken(token, request, context);
+            } else {
+                ePerson = sessionJWTTokenHandler.parseEPersonFromToken(token, request, context);
+            }
             return ePerson;
         } catch (JOSEException e) {
             log.error("Jose error", e);
@@ -129,13 +136,14 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
     @Override
     public boolean hasAuthenticationData(HttpServletRequest request) {
         return StringUtils.isNotBlank(request.getHeader(AUTHORIZATION_HEADER))
-                || StringUtils.isNotBlank(getAuthorizationCookie(request));
+                || StringUtils.isNotBlank(getAuthorizationCookie(request))
+                || StringUtils.isNotBlank(request.getParameter(AUTHORIZATION_TOKEN_PARAMETER));
     }
 
     @Override
     public void invalidateAuthenticationData(HttpServletRequest request, HttpServletResponse response,
                                              Context context) throws Exception {
-        String token = getToken(request);
+        String token = getSessionToken(request);
         invalidateAuthenticationCookie(response);
         sessionJWTTokenHandler.invalidateToken(token, request, context);
     }
@@ -192,7 +200,7 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
         response.setHeader(AUTHORIZATION_HEADER, String.format("%s %s", AUTHORIZATION_TYPE, token));
     }
 
-    private String getToken(HttpServletRequest request) {
+    private String getSessionToken(HttpServletRequest request) {
         String tokenValue = null;
         String authHeader = request.getHeader(AUTHORIZATION_HEADER);
         String authCookie = getAuthorizationCookie(request);
@@ -205,6 +213,15 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
         return tokenValue;
     }
 
+    private String getShortLivedToken(HttpServletRequest request) {
+        String tokenValue = null;
+        if (StringUtils.isNotBlank(request.getParameter(AUTHORIZATION_TOKEN_PARAMETER))) {
+            tokenValue = request.getParameter(AUTHORIZATION_TOKEN_PARAMETER);
+        }
+
+        return tokenValue;
+    }
+
     private String getAuthorizationCookie(HttpServletRequest request) {
         String authCookie = "";
         Cookie[] cookies = request.getCookies();

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

@@ -7,6 +7,17 @@
  */
 package org.dspace.app.rest.security.jwt;
 
+import java.util.Date;
+import javax.servlet.http.HttpServletRequest;
+
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.MACVerifier;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import com.nimbusds.jwt.util.DateUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Component;
 
 /**
@@ -15,6 +26,35 @@ import org.springframework.stereotype.Component;
  */
 @Component
 public class ShortLivedJWTTokenHandler extends JWTTokenHandler {
+
+    /**
+     * 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
+     */
+    @Override
+    protected boolean isValidToken(HttpServletRequest request, SignedJWT signedJWT, JWTClaimsSet jwtClaimsSet,
+                                 EPerson ePerson) throws JOSEException {
+        if (ePerson == null || StringUtils.isBlank(ePerson.getSessionSalt())) {
+            return false;
+        } else {
+            JWSVerifier verifier = new MACVerifier(buildSigningKey(request, ePerson));
+
+            //If token is valid and not expired return eperson in token
+            Date expirationTime = jwtClaimsSet.getExpirationTime();
+            return signedJWT.verify(verifier)
+                && expirationTime != null
+                //Ensure expiration timestamp is after the current time, with a minute of acceptable clock skew.
+                && DateUtils.isAfter(expirationTime, new Date(), 0);
+        }
+    }
+
     @Override
     protected String getTokenSecretConfigurationKey() {
         return "jwt.shortLived.token.secret";

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

@@ -21,14 +21,29 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
+import java.io.InputStream;
 import java.util.Base64;
+import java.util.Map;
 import javax.servlet.http.Cookie;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.codec.CharEncoding;
+import org.apache.commons.io.IOUtils;
+import org.dspace.app.rest.builder.BitstreamBuilder;
+import org.dspace.app.rest.builder.BundleBuilder;
+import org.dspace.app.rest.builder.CollectionBuilder;
+import org.dspace.app.rest.builder.CommunityBuilder;
 import org.dspace.app.rest.builder.GroupBuilder;
+import org.dspace.app.rest.builder.ItemBuilder;
 import org.dspace.app.rest.matcher.AuthenticationStatusMatcher;
 import org.dspace.app.rest.matcher.EPersonMatcher;
 import org.dspace.app.rest.matcher.HalMatcher;
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
+import org.dspace.content.Bitstream;
+import org.dspace.content.Bundle;
+import org.dspace.content.Collection;
+import org.dspace.content.Item;
+import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 import org.dspace.services.ConfigurationService;
 import org.hamcrest.Matchers;
@@ -36,6 +51,7 @@ import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.web.servlet.MvcResult;
 import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
 
 /**
@@ -774,4 +790,90 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
         getClient().perform(post("/api/authn/shortlivedtokens"))
             .andExpect(status().isUnauthorized());
     }
+
+    @Test
+    public void testShortLivedTokenToDowloadBitstream() throws Exception {
+        Bitstream bitstream = createPrivateBitstream();
+        String shortLivedToken = getShortLivedToken(eperson);
+
+        getClient().perform(get("/api/core/bitstreams/" + bitstream.getID() + "/content?token=" + shortLivedToken))
+            .andExpect(status().isOk());
+    }
+
+    @Test
+    public void testSessionTokenToDowloadBitstream() throws Exception {
+        Bitstream bitstream = createPrivateBitstream();
+
+        String sessionToken = getAuthToken(eperson.getEmail(), password);
+        getClient().perform(get("/api/core/bitstreams/" + bitstream.getID() + "/content?token=" + sessionToken))
+            .andExpect(status().isForbidden());
+    }
+
+    @Test
+    public void testExpiredShortLivedTokenToDowloadBitstream() throws Exception {
+        Bitstream bitstream = createPrivateBitstream();
+        configurationService.setProperty("jwt.shortLived.token.expiration", "1");
+        String shortLivedToken = getShortLivedToken(eperson);
+        Thread.sleep(1);
+        getClient().perform(get("/api/core/bitstreams/" + bitstream.getID() + "/content?token=" + shortLivedToken))
+            .andExpect(status().isForbidden());
+    }
+
+    private String getShortLivedToken(EPerson ePerson) throws Exception {
+        ObjectMapper mapper = new ObjectMapper();
+
+        String token = getAuthToken(eperson.getEmail(), password);
+        MvcResult mvcResult = getClient(token).perform(post("/api/authn/shortlivedtokens"))
+            .andReturn();
+
+        String content = mvcResult.getResponse().getContentAsString();
+        Map<String,Object> map = mapper.readValue(content, Map.class);
+        return String.valueOf(map.get("token"));
+    }
+
+    private Bitstream createPrivateBitstream() throws Exception {
+        context.turnOffAuthorisationSystem();
+
+        //** GIVEN **
+        //1. A community-collection structure with one parent community with sub-community and one collection.
+        parentCommunity = CommunityBuilder.createCommunity(context)
+            .withName("Parent Community")
+            .build();
+        Collection col1 = CollectionBuilder.createCollection(context, parentCommunity).withName("Collection 1").build();
+
+        //2. One public items that is readable by Anonymous
+        Item publicItem1 = ItemBuilder.createItem(context, col1)
+            .withTitle("Test")
+            .withIssueDate("2010-10-17")
+            .withAuthor("Smith, Donald")
+            .withSubject("ExtraEntry")
+            .build();
+
+        Bundle bundle1 = BundleBuilder.createBundle(context, publicItem1)
+            .withName("TEST BUNDLE")
+            .build();
+
+        //2. An item restricted to a specific internal group
+        Group staffGroup = GroupBuilder.createGroup(context)
+            .withName("Staff")
+            .addMember(eperson)
+            .build();
+
+        String bitstreamContent = "ThisIsSomeDummyText";
+        Bitstream bitstream = null;
+        try (InputStream is = IOUtils.toInputStream(bitstreamContent, CharEncoding.UTF_8)) {
+            bitstream = BitstreamBuilder.
+                createBitstream(context, bundle1, is)
+                .withName("Bitstream")
+                .withDescription("description")
+                .withMimeType("text/plain")
+                .withReaderGroup(staffGroup)
+                .build();
+        }
+
+        context.restoreAuthSystemState();
+
+        return bitstream;
+    }
 }
+

dspace-server-webapp/src/test/java/org/dspace/app/rest/security/jwt/JWTTokenHandlerTest.java

@@ -24,6 +24,7 @@ import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.service.EPersonService;
 import org.dspace.service.ClientInfoService;
+import org.dspace.services.ConfigurationService;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -48,6 +49,9 @@ public class JWTTokenHandlerTest {
     @Spy
     SessionJWTTokenHandler sessionJWTTokenHandler;
 
+    @Mock
+    private ConfigurationService configurationService;
+
     @Mock
     private Context context;
 
@@ -99,7 +103,7 @@ public class JWTTokenHandlerTest {
         when(sessionJWTTokenHandler.isEncryptionEnabled()).thenReturn(true);
         Date previous = new Date(System.currentTimeMillis() - 10000000000L);
         StringKeyGenerator keyGenerator = KeyGenerators.string();
-        when(sessionJWTTokenHandler.getEncryptionKey()).thenReturn(keyGenerator.generateKey().getBytes());
+        when(configurationService.getProperty("jwt.session.encryption.secret")).thenReturn(keyGenerator.generateKey());
         String token = sessionJWTTokenHandler
             .createTokenForEPerson(context, new MockHttpServletRequest(), previous, new ArrayList<>());
         SignedJWT signedJWT = SignedJWT.parse(token);
@@ -108,7 +112,7 @@ public class JWTTokenHandlerTest {
     //temporary set a negative expiration time so the token is invalid immediately
     @Test
     public void testExpiredToken() throws Exception {
-        when(sessionJWTTokenHandler.getExpirationPeriod()).thenReturn(-99999999L);
+        when(configurationService.getLongProperty("jwt.session.token.expiration", 30)).thenReturn(-99999999L);
         when(ePersonClaimProvider.getEPerson(any(Context.class), any(JWTClaimsSet.class))).thenReturn(ePerson);
         Date previous = new Date(new Date().getTime() - 10000000000L);
         String token = sessionJWTTokenHandler