DS-3542 Fix salt reuse + Authorization header + licenses

dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java

@@ -7,7 +7,6 @@
  */
 package org.dspace.authenticate;
 
-import javax.servlet.http.HttpServletRequest;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -15,6 +14,8 @@ import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
 
+import javax.servlet.http.HttpServletRequest;
+
 import org.dspace.authenticate.service.AuthenticationService;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.core.Context;
@@ -113,15 +114,7 @@ public class AuthenticationServiceImpl implements AuthenticationService
                     ret = AuthenticationMethod.NO_SUCH_USER;
                 }
                 if (ret == AuthenticationMethod.SUCCESS) {
-                    EPerson me = context.getCurrentUser();
+                    updateLastActiveDate(context);
-                    me.setLastActive(new Date());
-                    try {
-                        ePersonService.update(context, me);
-                    } catch (SQLException ex) {
-                        log.error("Could not update last-active stamp", ex);
-                    } catch (AuthorizeException ex) {
-                        log.error("Could not update last-active stamp", ex);
-                    }
                     return ret;
                 }
                 if (ret < bestRet) {
@@ -132,6 +125,20 @@ public class AuthenticationServiceImpl implements AuthenticationService
         return bestRet;
     }
 
+    public void updateLastActiveDate(Context context) {
+        EPerson me = context.getCurrentUser();
+        if(me != null) {
+            me.setLastActive(new Date());
+            try {
+                ePersonService.update(context, me);
+            } catch (SQLException ex) {
+                log.error("Could not update last-active stamp", ex);
+            } catch (AuthorizeException ex) {
+                log.error("Could not update last-active stamp", ex);
+            }
+        }
+    }
+
     @Override
     public boolean canSelfRegister(Context context,
                                    HttpServletRequest request,

dspace-api/src/main/java/org/dspace/authenticate/service/AuthenticationService.java

@@ -7,16 +7,17 @@
  */
 package org.dspace.authenticate.service;
 
+import java.sql.SQLException;
+import java.util.Iterator;
+import java.util.List;
+
+import javax.servlet.http.HttpServletRequest;
+
 import org.dspace.authenticate.AuthenticationMethod;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
 
-import javax.servlet.http.HttpServletRequest;
-import java.sql.SQLException;
-import java.util.Iterator;
-import java.util.List;
-
 /**
  * Access point for the stackable authentication methods.
  * <p>
@@ -158,6 +159,11 @@ public interface AuthenticationService {
                             EPerson eperson)
             throws SQLException;
 
+    /**
+     * Update the last active (login) timestamp on the current authenticated user
+     * @param context The authenticated context
+     */
+    public void updateLastActiveDate(Context context);
 
     /**
      * Get list of extra groups that user implicitly belongs to.

dspace-spring-rest/src/main/java/org/dspace/app/rest/model/hateoas/StatusResource.java

@@ -1,3 +1,10 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
 package org.dspace.app.rest.model.hateoas;
 
 import org.dspace.app.rest.model.StatusRest;

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/DSpaceAuthentication.java

@@ -7,17 +7,18 @@
  */
 package org.dspace.app.rest.security;
 
+import java.util.Collection;
+import java.util.Date;
+import java.util.List;
+
 import org.dspace.eperson.EPerson;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 
-import java.util.Collection;
-import java.util.List;
-
 public class DSpaceAuthentication implements Authentication {
 
 
-    private EPerson ePerson;
+    private Date previousLoginDate;
     private String username;
     private String password;
     private List<GrantedAuthority> authorities;
@@ -25,7 +26,7 @@ public class DSpaceAuthentication implements Authentication {
 
 
     public DSpaceAuthentication (EPerson ePerson, List<GrantedAuthority> authorities) {
-        this.ePerson = ePerson;
+        this.previousLoginDate = ePerson.getPreviousActive();
         this.username = ePerson.getEmail();
         this.authorities = authorities;
     }
@@ -68,7 +69,7 @@ public class DSpaceAuthentication implements Authentication {
         return username;
     }
 
-    public EPerson getEPerson() {
+    public Date getPreviousLoginDate() {
-        return ePerson;
+        return previousLoginDate;
     }
 }

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/EPersonClaimProvider.java

@@ -1,3 +1,10 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
 package org.dspace.app.rest.security;
 
 import com.nimbusds.jwt.JWTClaimsSet;

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java

@@ -55,9 +55,8 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
 
     @Override
     public Authentication authenticate(Authentication authentication) throws AuthenticationException {
-
         Context context = ContextUtil.obtainContext(request);
-        if(context.getCurrentUser() != null) {
+        if(context != null && context.getCurrentUser() != null) {
             return authenticateRefreshTokenRequest(context);
         } else {
             return authenticateNewLogin(authentication);
@@ -65,7 +64,8 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
     }
 
     private Authentication authenticateRefreshTokenRequest(Context context) {
-        return createAuthenticationToken(null, context);
+        authenticationService.updateLastActiveDate(context);
+        return createAuthentication(null, context);
     }
 
     private Authentication authenticateNewLogin(Authentication authentication) {
@@ -79,7 +79,7 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
 
             if (implicitStatus == AuthenticationMethod.SUCCESS) {
                 log.info(LogManager.getHeader(newContext, "login", "type=implicit"));
-                return createAuthenticationToken(password, newContext);
+                return createAuthentication(password, newContext);
             } else {
                 int authenticateResult = authenticationService.authenticate(newContext, name, password, null, request);
                 if (AuthenticationMethod.SUCCESS == authenticateResult) {
@@ -87,7 +87,7 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
                     log.info(LogManager
                             .getHeader(newContext, "login", "type=explicit"));
 
-                    return createAuthenticationToken(password, newContext);
+                    return createAuthentication(password, newContext);
                 } else {
                     log.info(LogManager.getHeader(newContext, "failed_login", "email="
                             + name + ", result="
@@ -112,8 +112,9 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider
         return null;
     }
 
-    private Authentication createAuthenticationToken(final String password, final Context context) {
+    private Authentication createAuthentication(final String password, final Context context) {
         EPerson ePerson = context.getCurrentUser();
+
         if(ePerson != null && StringUtils.isNotBlank(ePerson.getEmail())) {
             //Pass the eperson ID to the request service
             requestService.setCurrentUserId(ePerson.getID());

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/JWTClaimProvider.java

@@ -1,11 +1,19 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
 package org.dspace.app.rest.security;
 
+import java.sql.SQLException;
+
+import javax.servlet.http.HttpServletRequest;
+
 import com.nimbusds.jwt.JWTClaimsSet;
 import org.dspace.core.Context;
 
-import javax.servlet.http.HttpServletRequest;
-import java.sql.SQLException;
-
 public interface JWTClaimProvider {
 
     String getKey();

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/JWTTokenHandler.java

@@ -11,8 +11,6 @@ import java.sql.SQLException;
 import java.text.ParseException;
 import java.util.Date;
 import java.util.List;
-import java.util.UUID;
-import java.util.stream.Collectors;
 
 import javax.servlet.http.HttpServletRequest;
 
@@ -31,20 +29,19 @@ import org.dspace.authorize.AuthorizeException;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
-import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
-import org.dspace.servicemanager.config.DSpaceConfigurationService;
 import org.dspace.services.ConfigurationService;
 import org.dspace.services.factory.DSpaceServicesFactory;
 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.KeyGenerators;
 import org.springframework.security.crypto.keygen.StringKeyGenerator;
 import org.springframework.stereotype.Component;
 
 @Component
-public class JWTTokenHandler {
+public class JWTTokenHandler implements InitializingBean {
 
 
     @Autowired
@@ -65,14 +62,13 @@ public class JWTTokenHandler {
     private EPersonService ePersonService;
 
 
-    public JWTTokenHandler() {
+    @Override
+    public void afterPropertiesSet() throws Exception {
         //TODO move properties to authentication module
-        configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
         this.jwtKey = configurationService.getProperty("jwt.token.secret", "defaultjwtkeysecret");
         this.expirationTime = configurationService.getLongProperty("jwt.token.expiration", 30) * 60 * 1000;
     }
 
-
     /**
      * Retrieve EPerson from a jwt
      * @param token
@@ -110,7 +106,7 @@ public class JWTTokenHandler {
 
             return ePerson;
         } else {
-            log.warn("Someone tried to use an expired or non-valid token");
+            log.warn(getIpAddress(request) + " tried to use an expired or non-valid token");
             return null;
         }
     }
@@ -123,19 +119,14 @@ public class JWTTokenHandler {
      * Create a jwt with the EPerson details in it
      * @param context
      * @param request
-     * @param detachedEPerson
+     * @param previousLoginDate
      * @param groups
      * @return
      * @throws JOSEException
      */
-    public String createTokenForEPerson(Context context, HttpServletRequest request, EPerson detachedEPerson, List<Group> groups) throws JOSEException {
+    public String createTokenForEPerson(Context context, HttpServletRequest request, Date previousLoginDate, List<Group> groups) throws JOSEException {
-
-        //This has to be set before createNewSessionSalt, otherwise authorizeException is thrown
-        context.setCurrentUser(detachedEPerson);
-
-        EPerson ePerson = createNewSessionSalt(context, detachedEPerson);
-
 
+        EPerson ePerson = updateSessionSalt(context, previousLoginDate);
 
         JWSSigner signer = new MACSigner(buildSigningKey(request, ePerson));
         JWTClaimsSet.Builder builder = new JWTClaimsSet.Builder();
@@ -175,7 +166,7 @@ public class JWTTokenHandler {
     }
 
     private String getIpAddress(HttpServletRequest request) {
-        //TODO make using the ip address of the request optional
+        //TODO FREDERIC make using the ip address of the request optional
         String ipAddress = request.getHeader("X-FORWARDED-FOR");
         if (ipAddress == null) {
             ipAddress = request.getRemoteAddr();
@@ -183,32 +174,30 @@ public class JWTTokenHandler {
         return ipAddress;
     }
 
-    private EPerson createNewSessionSalt(Context context, EPerson detached) {
+
+    private EPerson updateSessionSalt(final Context context, final Date previousLoginDate) {
         EPerson ePerson = null;
+
         try {
-            //Reloading doesn't seem to work
+            ePerson = context.getCurrentUser();
-//            ePerson = context.reloadEntity(detached);
 
-            //This does work, even still has the previousActive
+            //If the previous login was within the configured token expiration time, we reuse the session salt.
-            ePerson = ePersonService.find(context, detached.getID());
+            //This allows a user to login on multiple devices/browsers at the same time.
-        } catch (SQLException e) {
+            if (previousLoginDate == null || (ePerson.getLastActive().getTime() - previousLoginDate.getTime() > expirationTime)) {
-            e.printStackTrace();
-        }
-        if (detached.getLastActive().getTime() - detached.getPreviousActive().getTime() > expirationTime) {
 
+                StringKeyGenerator stringKeyGenerator = KeyGenerators.string();
+                String salt = stringKeyGenerator.generateKey();
+                ePerson.setSessionSalt(salt);
 
-            StringKeyGenerator stringKeyGenerator = KeyGenerators.string();
-            String salt = stringKeyGenerator.generateKey();
-            ePerson.setSessionSalt(salt);
-
-            try {
                 ePersonService.update(context, ePerson);
-                context.commit();
-            } catch (SQLException e) {
-                e.printStackTrace();
-            } catch (AuthorizeException e) {
-                e.printStackTrace();
             }
+
+        } catch (SQLException e) {
+            //TODO FREDERIC: fail fast
+            e.printStackTrace();
+        } catch (AuthorizeException e) {
+            //TODO FREDERIC: fail fast
+            e.printStackTrace();
         }
 
         return ePerson;

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/JWTTokenRestAuthenticationServiceImpl.java

@@ -12,11 +12,11 @@ import java.sql.SQLException;
 import java.text.ParseException;
 import java.util.List;
 
-import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import com.nimbusds.jose.JOSEException;
+import org.apache.commons.lang.StringUtils;
 import org.dspace.app.rest.utils.ContextUtil;
 import org.dspace.authenticate.service.AuthenticationService;
 import org.dspace.core.Context;
@@ -29,13 +29,13 @@ import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
-import org.springframework.web.util.WebUtils;
 
 @Component
 public class JWTTokenRestAuthenticationServiceImpl implements RestAuthenticationService, InitializingBean {
 
     private static final Logger log = LoggerFactory.getLogger(RestAuthenticationService.class);
-    private static final String ACCESS_TOKEN = "access_token";
+    private static final String AUTHORIZATION_HEADER = "Authorization";
+    private static final String AUTHORIZATION_TYPE = "Bearer";
 
     @Autowired
     private JWTTokenHandler jwtTokenHandler;
@@ -55,14 +55,19 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
     }
 
     @Override
-    public void addAuthenticationDataForUser(HttpServletRequest request, HttpServletResponse response, EPerson ePerson) {
+    public void addAuthenticationDataForUser(HttpServletRequest request, HttpServletResponse response, DSpaceAuthentication authentication) {
         try {
             Context context = ContextUtil.obtainContext(request);
-//            EPerson ePerson = ePersonService.findByEmail(context, email);
+            context.setCurrentUser(ePersonService.findByEmail(context, authentication.getName()));
-            List<Group> groups = authenticationService.getSpecialGroups(context, request);
-            String token = jwtTokenHandler.createTokenForEPerson(context, request, ePerson, groups);
 
-            response.getWriter().write(wrapTokenInJsonFormat(token));
+            List<Group> groups = authenticationService.getSpecialGroups(context, request);
+
+            String token = jwtTokenHandler.createTokenForEPerson(context, request,
+                    authentication.getPreviousLoginDate(), groups);
+
+            addTokenToResponse(response, token);
+
+            context.commit();
 
         } catch (JOSEException e) {
             log.error("JOSE Exception", e);
@@ -71,7 +76,6 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
         } catch (IOException e) {
             log.error("Error writing to response", e);
         }
-
     }
 
     @Override
@@ -92,7 +96,7 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
 
     @Override
     public boolean hasAuthenticationData(HttpServletRequest request) {
-        return request.getHeader("Authorization") != null;
+        return StringUtils.isNotBlank(request.getHeader(AUTHORIZATION_HEADER));
     }
 
     @Override
@@ -101,19 +105,18 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
         jwtTokenHandler.invalidateToken(token, request, context);
     }
 
+    private void addTokenToResponse(final HttpServletResponse response, final String token) throws IOException {
+        response.setHeader(AUTHORIZATION_HEADER, String.format("%s %s", AUTHORIZATION_TYPE, token));
+    }
+
     private String getToken(HttpServletRequest request) {
-        String authHeader = request.getHeader("Authorization");
+        String authHeader = request.getHeader(AUTHORIZATION_HEADER);
-        if (authHeader != null) {
+        if (StringUtils.isNotBlank(authHeader)) {
-            String tokenValue = authHeader.replace("Bearer", "").trim();
+            String tokenValue = authHeader.replace(AUTHORIZATION_TYPE, "").trim();
             return tokenValue;
+        } else {
+            return null;
         }
-        return null;
     }
 
-    //Put the token in a json-string
-    private String wrapTokenInJsonFormat(String token) {
-        return "{ \""+ ACCESS_TOKEN +"\" : \"" + token + "\" }";
-    }
-
-
 }

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/RestAuthenticationService.java

@@ -7,20 +7,20 @@
  */
 package org.dspace.app.rest.security;
 
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Service;
 
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
 /**
  * Interface for a service that can provide authentication for the REST API
  */
 @Service
 public interface RestAuthenticationService {
 
-    void addAuthenticationDataForUser(HttpServletRequest request, HttpServletResponse response, EPerson ePerson);
+    void addAuthenticationDataForUser(HttpServletRequest request, HttpServletResponse response, DSpaceAuthentication authentication);
 
     EPerson getAuthenticatedEPerson(HttpServletRequest request, Context context);
 

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/SpecialGroupClaimProvider.java

@@ -1,3 +1,10 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
 package org.dspace.app.rest.security;
 
 import com.nimbusds.jwt.JWTClaimsSet;

dspace-spring-rest/src/main/java/org/dspace/app/rest/security/StatelessLoginFilter.java

@@ -2,24 +2,25 @@
  * The contents of this file are subject to the license and copyright
  * detailed in the LICENSE and NOTICE files at the root of the source
  * tree and available online at
- * <p>
+ *
  * http://www.dspace.org/license/
  */
 package org.dspace.app.rest.security;
 
+import java.io.IOException;
+import java.util.ArrayList;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 
-import javax.servlet.FilterChain;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
-import java.util.ArrayList;
-
 public class StatelessLoginFilter extends AbstractAuthenticationProcessingFilter {
 
     private AuthenticationManager authenticationManager;
@@ -56,6 +57,6 @@ public class StatelessLoginFilter extends AbstractAuthenticationProcessingFilter
 
         //TODO every time we log in a new token and salt is created, might need to change this
         DSpaceAuthentication dSpaceAuthentication = (DSpaceAuthentication) auth;
-        restAuthenticationService.addAuthenticationDataForUser(req, res, dSpaceAuthentication.getEPerson());
+        restAuthenticationService.addAuthenticationDataForUser(req, res, dSpaceAuthentication);
     }
 }

dspace-spring-rest/src/main/webapp/js/hal/http/client.js

@@ -1,3 +1,10 @@
+/*
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
 HAL.Http.Client = function(opts) {
     this.vent = opts.vent;
     this.defaultHeaders = { 'Accept': 'application/hal+json, application/json, */*; q=0.01' };

dspace-spring-rest/src/main/webapp/login.html

@@ -1,3 +1,12 @@
+<!--
+
+    The contents of this file are subject to the license and copyright
+    detailed in the LICENSE and NOTICE files at the root of the source
+    tree and available online at
+
+    http://www.dspace.org/license/
+
+-->
 <!DOCTYPE html>
 <html lang="en">
 <head>
@@ -47,13 +56,12 @@
                     async : false,
                     data : 'password='+$("#password").val()+'&user='+$("#username").val() ,
                     headers : {
-                        Authorization : 'Basic ' + btoa('webapp:nim-cmas'),
                         "Content-Type" : 'application/x-www-form-urlencoded',
                         "Accept" : 'application/json'
                     },
                     dataType : 'json',
-                    success : function(data) {
+                    success : function(data, textStatus, request) {
-                        document.cookie = "MyHalBrowserToken=" + data.access_token;
+                        document.cookie = "MyHalBrowserToken=" + request.getResponseHeader('Authorization');
                         window.location.href = window.location.pathname.replace("login.html", "")
                     }
                 });

dspace/config/modules/authentication.cfg

@@ -53,4 +53,4 @@ plugin.sequence.org.dspace.authenticate.AuthenticationMethod = org.dspace.authen
 # Partial key that is used to sign the authentication tokens
 jwt.token.secret = thisisatestsecretkeyforjwttokens
 # Expiration time of a token in minutes
-jwt.token.expiration = 1
+jwt.token.expiration = 30