DS-3819: Return HTTP redirect when login URL is supported

dspace-spring-rest/src/main/java/org/dspace/app/rest/AuthenticationRestController.java

@@ -76,11 +76,14 @@ public class AuthenticationRestController implements InitializingBean {
         Context context = ContextUtil.obtainContext(request);
         EPersonRest ePersonRest = null;
         if (context.getCurrentUser() != null) {
-            ePersonRest = ePersonConverter.fromModel(context.getCurrentUser());
+            ePersonRest = ePersonConverter.fromModelWithGroups(context, context.getCurrentUser());
         }
+
         AuthenticationStatusResource authenticationStatusResource = new AuthenticationStatusResource(
             new AuthenticationStatusRest(ePersonRest), utils);
+
         halLinkService.addLinks(authenticationStatusResource);
+
         return authenticationStatusResource;
     }
 
@@ -116,6 +119,8 @@ public class AuthenticationRestController implements InitializingBean {
 
 
         if (context == null || context.getCurrentUser() == null) {
+            // Note that the actual HTTP status in this case is set by
+            // org.dspace.app.rest.security.StatelessLoginFilter.unsuccessfulAuthentication()
             return ResponseEntity.status(HttpStatus.FORBIDDEN)
                                  .body(failedMessage);
         } else {
@@ -123,6 +128,4 @@ public class AuthenticationRestController implements InitializingBean {
             return ResponseEntity.ok().build();
         }
     }
-
-
 }

dspace-spring-rest/src/main/java/org/dspace/app/rest/converter/EPersonConverter.java

@@ -7,14 +7,17 @@
  */
 package org.dspace.app.rest.converter;
 
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.log4j.Logger;
 import org.dspace.app.rest.model.EPersonRest;
 import org.dspace.app.rest.model.GroupRest;
+import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
+import org.dspace.eperson.service.GroupService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -26,9 +29,13 @@ import org.springframework.stereotype.Component;
  */
 @Component
 public class EPersonConverter extends DSpaceObjectConverter<EPerson, org.dspace.app.rest.model.EPersonRest> {
+
     @Autowired(required = true)
     private GroupConverter epersonGroupConverter;
 
+    @Autowired(required = true)
+    private GroupService groupService;
+
     private static final Logger log = Logger.getLogger(EPersonConverter.class);
 
     @Override
@@ -40,10 +47,18 @@ public class EPersonConverter extends DSpaceObjectConverter<EPerson, org.dspace.
         eperson.setRequireCertificate(obj.getRequireCertificate());
         eperson.setSelfRegistered(obj.getSelfRegistered());
         eperson.setEmail(obj.getEmail());
+
+        return eperson;
+    }
+
+    public EPersonRest fromModelWithGroups(Context context, EPerson ePerson) throws SQLException {
+        EPersonRest eperson = fromModel(ePerson);
+
         List<GroupRest> groups = new ArrayList<GroupRest>();
-        for (Group g : obj.getGroups()) {
+        for (Group g : groupService.allMemberGroups(context, ePerson)) {
             groups.add(epersonGroupConverter.convert(g));
         }
+
         eperson.setGroups(groups);
         return eperson;
     }

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

@@ -11,6 +11,7 @@ import java.io.IOException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.dspace.authenticate.service.AuthenticationService;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
 import org.springframework.stereotype.Service;
@@ -33,4 +34,5 @@ public interface RestAuthenticationService {
 
     void invalidateAuthenticationData(HttpServletRequest request, Context context) throws Exception;
 
+    AuthenticationService getAuthenticationService();
 }

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

@@ -10,7 +10,6 @@ package org.dspace.app.rest.security;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Iterator;
-
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -19,11 +18,9 @@ import javax.servlet.http.HttpServletResponse;
 import org.apache.commons.lang3.StringUtils;
 import org.dspace.app.rest.utils.ContextUtil;
 import org.dspace.authenticate.AuthenticationMethod;
-import org.dspace.authenticate.factory.AuthenticateServiceFactory;
 import org.dspace.authenticate.service.AuthenticationService;
 import org.dspace.core.Context;
 import org.springframework.security.authentication.AuthenticationManager;
-import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;
@@ -59,25 +56,9 @@ public class StatelessLoginFilter extends AbstractAuthenticationProcessingFilter
         String user = req.getParameter("user");
         String password = req.getParameter("password");
 
-        try {
         return authenticationManager.authenticate(
                 new DSpaceAuthentication(user, password, new ArrayList<>())
         );
-        } catch (BadCredentialsException e) {
-            AuthenticationService authenticationService =
-                AuthenticateServiceFactory.getInstance().getAuthenticationService();
-            Iterator<AuthenticationMethod> authenticationMethodIterator =
-                authenticationService.authenticationMethodIterator();
-            while (authenticationMethodIterator.hasNext()) {
-                AuthenticationMethod authenticationMethod = authenticationMethodIterator.next();
-                Context context = ContextUtil.obtainContext(req);
-                String loginPageURL = authenticationMethod.loginPageURL(context, req, res);
-                if (StringUtils.isNotBlank(loginPageURL)) {
-                    res.addHeader("Location", loginPageURL);
-                }
-            }
-            throw e;
-        }
     }
 
 
@@ -90,4 +71,33 @@ public class StatelessLoginFilter extends AbstractAuthenticationProcessingFilter
         DSpaceAuthentication dSpaceAuthentication = (DSpaceAuthentication) auth;
         restAuthenticationService.addAuthenticationDataForUser(req, res, dSpaceAuthentication);
     }
+
+    @Override
+    protected void unsuccessfulAuthentication(HttpServletRequest request,
+                                              HttpServletResponse response, AuthenticationException failed)
+            throws IOException, ServletException {
+
+        AuthenticationService authenticationService = restAuthenticationService.getAuthenticationService();
+
+        Iterator<AuthenticationMethod> authenticationMethodIterator
+                = authenticationService.authenticationMethodIterator();
+        Context context = ContextUtil.obtainContext(request);
+        String redirectUrl = null;
+
+        while (authenticationMethodIterator.hasNext()) {
+            AuthenticationMethod authenticationMethod = authenticationMethodIterator.next();
+
+            String loginPageURL = authenticationMethod.loginPageURL(context, request, response);
+            if (StringUtils.isNotBlank(loginPageURL)) {
+                redirectUrl = loginPageURL;
+            }
+        }
+
+        if (redirectUrl == null) {
+            response.sendError(HttpServletResponse.SC_UNAUTHORIZED, failed.getMessage());
+        } else {
+            response.sendRedirect(redirectUrl);
+        }
+
+    }
 }

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

@@ -106,6 +106,11 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
         jwtTokenHandler.invalidateToken(token, request, context);
     }
 
+    @Override
+    public AuthenticationService getAuthenticationService() {
+        return authenticationService;
+    }
+
     private void addTokenToResponse(final HttpServletResponse response, final String token) throws IOException {
         response.setHeader(AUTHORIZATION_HEADER, String.format("%s %s", AUTHORIZATION_TYPE, token));
     }

dspace-spring-rest/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java

@@ -8,6 +8,8 @@
 package org.dspace.app.rest;
 
 import static java.lang.Thread.sleep;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.endsWith;
 import static org.hamcrest.Matchers.is;
@@ -23,12 +25,14 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 import java.util.Base64;
 
 import org.dspace.app.rest.builder.GroupBuilder;
+import org.dspace.app.rest.matcher.GroupMatcher;
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
 import org.dspace.eperson.Group;
 import org.dspace.services.ConfigurationService;
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
 
 /**
  * Integration test that covers various authentication scenarios
@@ -43,6 +47,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
 
     public static final String[] PASS_ONLY = {"org.dspace.authenticate.PasswordAuthentication"};
     public static final String[] SHIB_ONLY = {"org.dspace.authenticate.ShibAuthentication"};
+    public static final String[] SHIB_AND_IP = {"org.dspace.authenticate.IPAuthentication", "org.dspace.authenticate.ShibAuthentication"};
 
     @Before
     public void setup() throws Exception {
@@ -65,7 +70,9 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
                         .andExpect(jsonPath("$.type", is("status")))
 
                         .andExpect(jsonPath("$._links.eperson.href", startsWith(REST_SERVER_URL)))
-                        .andExpect(jsonPath("$._embedded.eperson.email", is(eperson.getEmail())));
+                        .andExpect(jsonPath("$._embedded.eperson.email", is(eperson.getEmail())))
+                        .andExpect(jsonPath("$._embedded.eperson.groups", contains(
+                                GroupMatcher.matchGroupWithName("Anonymous"))));
     }
 
     @Test
@@ -238,14 +245,27 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
     public void testReuseTokenWithDifferentIP() throws Exception {
         String token = getAuthToken(eperson.getEmail(), password);
 
+        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")));
+
         getClient(token).perform(get("/api/authn/status")
                                      .header("X-FORWARDED-FOR", "1.1.1.1"))
-
                         .andExpect(status().isOk())
                         .andExpect(jsonPath("$.okay", is(true)))
                         .andExpect(jsonPath("$.authenticated", is(false)))
                         .andExpect(jsonPath("$.type", is("status")));
 
+
+        getClient(token).perform(get("/api/authn/status")
+                                    .with(ip("1.1.1.1")))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.okay", is(true)))
+                .andExpect(jsonPath("$.authenticated", is(false)))
+                .andExpect(jsonPath("$.type", is("status")));
     }
 
     @Test
@@ -321,13 +341,12 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
     }
 
     @Test
-    public void testShibbolethLoginRequest() throws Exception {
+    public void testShibbolethLoginRequestAttribute() throws Exception {
         configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_ONLY);
 
-        getClient().perform(post("/api/authn/login").header("Referer", "http://my.uni.edu"))
+        getClient().perform(get("/api/authn/login").header("Referer", "http://my.uni.edu"))
-                .andExpect(status().isUnauthorized())
+                .andExpect(status().is3xxRedirection())
-                .andExpect(header().string("Location", "/Shibboleth.sso/Login?target=http%3A%2F%2Fmy.uni.edu"))
+                .andExpect(header().string("Location", "/Shibboleth.sso/Login?target=http%3A%2F%2Fmy.uni.edu"));
-                .andReturn().getResponse().getHeader("Location");
 
         //Simulate that a shibboleth authentication has happened
 
@@ -344,7 +363,64 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio
                 .andExpect(jsonPath("$.authenticated", is(true)))
                 .andExpect(jsonPath("$.type", is("status")))
                 .andExpect(jsonPath("$._links.eperson.href", startsWith(REST_SERVER_URL)))
-                .andExpect(jsonPath("$._embedded.eperson.email", is(eperson.getEmail())));
+                .andExpect(jsonPath("$._embedded.eperson.email", is(eperson.getEmail())))
+                .andExpect(jsonPath("$._embedded.eperson.groups", contains(
+                        GroupMatcher.matchGroupWithName("Anonymous"))));
+    }
+
+    @Test
+    public void testShibbolethLoginRequestHeaderWithIpAuthentication() throws Exception {
+        configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_AND_IP);
+        configurationService.setProperty("authentication-ip.Administrator", "123.123.123.123");
+
+
+        getClient().perform(get("/api/authn/login")
+                            .header("Referer", "http://my.uni.edu")
+                            .with(ip("123.123.123.123")))
+                .andExpect(status().is3xxRedirection())
+                .andExpect(header().string("Location", "/Shibboleth.sso/Login?target=http%3A%2F%2Fmy.uni.edu"));
+
+        //Simulate that a shibboleth authentication has happened
+        String token = getClient().perform(get("/api/authn/login")
+                    .with(ip("123.123.123.123"))
+                    .header("SHIB-MAIL", eperson.getEmail()))
+                .andExpect(status().isOk())
+                .andReturn().getResponse().getHeader(AUTHORIZATION_HEADER);
+
+        getClient(token).perform(get("/api/authn/status")
+                                    .with(ip("123.123.123.123")))
+                .andDo(MockMvcResultHandlers.print())
+                .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(true)))
+                .andExpect(jsonPath("$.type", is("status")))
+                .andExpect(jsonPath("$._links.eperson.href", startsWith(REST_SERVER_URL)))
+                .andExpect(jsonPath("$._embedded.eperson.email", is(eperson.getEmail())))
+                .andExpect(jsonPath("$._embedded.eperson.groups", containsInAnyOrder(
+                        GroupMatcher.matchGroupWithName("Administrator"), GroupMatcher.matchGroupWithName("Anonymous"))));
+
+        //Simulate that a new shibboleth authentication has happened from another IP
+        token = getClient().perform(get("/api/authn/login")
+                .with(ip("234.234.234.234"))
+                .header("SHIB-MAIL", eperson.getEmail()))
+                .andExpect(status().isOk())
+                .andReturn().getResponse().getHeader(AUTHORIZATION_HEADER);
+
+        getClient(token).perform(get("/api/authn/status")
+                .with(ip("234.234.234.234")))
+                .andDo(MockMvcResultHandlers.print())
+                .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(true)))
+                .andExpect(jsonPath("$.type", is("status")))
+                .andExpect(jsonPath("$._links.eperson.href", startsWith(REST_SERVER_URL)))
+                .andExpect(jsonPath("$._embedded.eperson.email", is(eperson.getEmail())))
+                .andExpect(jsonPath("$._embedded.eperson.groups", contains(
+                        GroupMatcher.matchGroupWithName("Anonymous"))));
     }
 
 }

dspace-spring-rest/src/test/java/org/dspace/app/rest/test/AbstractControllerIntegrationTest.java

@@ -30,6 +30,7 @@ import org.springframework.hateoas.MediaTypes;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
+import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.TestExecutionListeners;
@@ -39,6 +40,7 @@ import org.springframework.test.context.support.DirtiesContextTestExecutionListe
 import org.springframework.test.context.transaction.TransactionalTestExecutionListener;
 import org.springframework.test.context.web.WebAppConfiguration;
 import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.request.RequestPostProcessor;
 import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
 import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder;
 import org.springframework.web.context.WebApplicationContext;
@@ -119,5 +121,11 @@ public class AbstractControllerIntegrationTest extends AbstractIntegrationTestWi
         return getAuthResponse(user, password).getHeader(AUTHORIZATION_HEADER);
     }
 
+    public static RequestPostProcessor ip(final String ipAddress) {
+        return request -> {
+            request.setRemoteAddr(ipAddress);
+            return request;
+        };
+    }
 }