Fix shibboleth redirect to work with all allowed origins

DSpace · Sep 17, 2020 · debf437 · debf437
1 parent e24f924
commit debf437
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 2 deletions.
diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/ShibbolethRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/ShibbolethRestController.java
@@ -8,6 +8,7 @@
 package org.dspace.app.rest;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import javax.servlet.http.HttpServletResponse;
 
@@ -62,8 +63,14 @@ public void shibboleth(HttpServletResponse response,
         // Validate that the redirectURL matches either the server or UI hostname. It *cannot* be an arbitrary URL.
         String redirectHostName = Utils.getHostName(redirectUrl);
         String serverHostName = Utils.getHostName(configurationService.getProperty("dspace.server.url"));
-        String clientHostName = Utils.getHostName(configurationService.getProperty("dspace.ui.url"));
-        if (StringUtils.equalsAnyIgnoreCase(redirectHostName, serverHostName, clientHostName)) {
+        ArrayList<String> allowedHostNames = new ArrayList<String>();
+        allowedHostNames.add(serverHostName);
+        String[] allowedUrls = configurationService.getArrayProperty("rest.cors.allowed-origins");
+        for (String url : allowedUrls) {
+            allowedHostNames.add(Utils.getHostName(url));
+        }
+
+        if (StringUtils.equalsAnyIgnoreCase(redirectHostName, allowedHostNames.toArray(new String[0]))) {
             log.debug("Shibboleth redirecting to " + redirectUrl);
             response.sendRedirect(redirectUrl);
         } else {

diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ShibbolethRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ShibbolethRestControllerIT.java
@@ -12,7 +12,10 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
+import org.dspace.services.ConfigurationService;
+import org.junit.Before;
 import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
 
 /**
  * Integration test that cover ShibbolethRestController
@@ -21,6 +24,17 @@
  */
 public class ShibbolethRestControllerIT extends AbstractControllerIntegrationTest {
 
+    @Autowired
+    ConfigurationService configurationService;
+
+
+    @Before
+    public void setup() throws Exception {
+        super.setUp();
+        configurationService.setProperty("rest.cors.allowed-origins",
+                "${dspace.ui.url}, http://anotherdspacehost:4000");
+    }
+
     @Test
     public void testRedirectToDefaultDspaceUrl() throws Exception {
         String token = getAuthToken(eperson.getEmail(), password);
@@ -32,6 +46,7 @@ public void testRedirectToDefaultDspaceUrl() throws Exception {
 
     @Test
     public void testRedirectToGivenTrustedUrl() throws Exception {
+
         String token = getAuthToken(eperson.getEmail(), password);
 
         getClient(token).perform(get("/api/authn/shibboleth")
@@ -40,6 +55,16 @@ public void testRedirectToGivenTrustedUrl() throws Exception {
                 .andExpect(redirectedUrl("http://localhost:8080/server/api/authn/status"));
     }
 
+    @Test
+    public void testRedirectToAnotherGivenTrustedUrl() throws Exception {
+        String token = getAuthToken(eperson.getEmail(), password);
+
+        getClient(token).perform(get("/api/authn/shibboleth")
+                .param("redirectUrl", "http://anotherdspacehost:4000/home"))
+                .andExpect(status().is3xxRedirection())
+                .andExpect(redirectedUrl("http://anotherdspacehost:4000/home"));
+    }
+
     @Test
     public void testRedirectToGivenUntrustedUrl() throws Exception {
         String token = getAuthToken(eperson.getEmail(), password);