[#14346] Redirect with back-end configured frontendUrl

src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java

@@ -180,10 +180,6 @@ protected <T extends AppPage> T loginAdminToPage(AppUrl url, Class<T> typeOfPage
      */
     protected void logout() {
         AppUrl url = createBackendUrl(Const.WebPageURIs.LOGOUT);
-        if (!TestProperties.TEAMMATES_FRONTEND_URL.equals(TestProperties.TEAMMATES_BACKEND_URL)) {
-            url = url.withParam("frontendUrl", TestProperties.TEAMMATES_FRONTEND_URL);
-        }
-
         browser.goToUrl(TestProperties.TEAMMATES_FRONTEND_URL);
         browser.waitForPageReadyState();
         browser.removeSessionStorageItem(MASQUERADE_ACCOUNT_ID_STORAGE_KEY);

src/main/java/teammates/common/util/UrlHelper.java

@@ -12,9 +12,6 @@ public final class UrlHelper {
 
     public static final String DEFAULT_REDIRECT_URL = "/";
 
-    private static final int HTTPS_PORT = 443;
-    private static final int HTTP_PORT = 80;
-
     private UrlHelper() {
         // utility class
     }
@@ -27,70 +24,37 @@ public static String encodeQueryParam(String param) {
     }
 
     /**
-     * Returns a safe redirect URL.
-     * If the provided nextUrl is not safe, it returns the {@link #DEFAULT_REDIRECT_URL}.
+     * Returns a safe relative redirect URL.
+     *
+     * <p>
+     * If the provided redirectUrl is not safe, or isn't relative, it returns the {@link #DEFAULT_REDIRECT_URL}.
      */
-    public static String getSafeRedirectUrl(String nextUrl) {
-        return isSafeRedirectUrl(nextUrl) ? nextUrl : DEFAULT_REDIRECT_URL;
+    public static String getSafeRelativeRedirectUrl(String redirectUrl) {
+        return isSafeRelativeRedirectUrl(redirectUrl) ? redirectUrl : DEFAULT_REDIRECT_URL;
     }
 
     /**
-     * Checks whether the given URL is safe to use as a redirect target.
+     * Checks whether the given relative URL is safe to use as a redirect target.
+     *
+     * <p>
+     * Rejects absolute URLs.
      */
-    public static boolean isSafeRedirectUrl(String url) {
-        if (url == null) {
+    public static boolean isSafeRelativeRedirectUrl(String url) {
+        if (StringHelper.isEmpty(url)) {
             return false;
         }
 
         try {
             URI uri = new URI(url);
             if (uri.isAbsolute()) {
-                return isSafeAbsoluteRedirectUrl(uri);
+                return false;
             }
 
-            return isSafeRelativeRedirectUrl(url);
+            return url.startsWith("/")
+                    && !url.startsWith("//");
         } catch (URISyntaxException e) {
             return false;
         }
     }
 
-    private static boolean isSafeRelativeRedirectUrl(String url) {
-        return url.startsWith("/")
-                && !url.startsWith("//");
-    }
-
-    private static boolean isSafeAbsoluteRedirectUrl(URI uri) throws URISyntaxException {
-        if (!isHttp(uri) && !isHttps(uri)) {
-            return false;
-        }
-
-        URI frontendUri = new URI(Config.APP_FRONTEND_URL);
-        return isSameOrigin(uri, frontendUri);
-    }
-
-    private static boolean isSameOrigin(URI firstUri, URI secondUri) {
-        boolean isSameScheme = firstUri.getScheme().equalsIgnoreCase(secondUri.getScheme());
-        boolean isSameHost = firstUri.getHost() != null
-                && firstUri.getHost().equalsIgnoreCase(secondUri.getHost());
-        boolean isSamePort = getPort(firstUri) == getPort(secondUri);
-
-        return isSameScheme && isSameHost && isSamePort;
-    }
-
-    private static int getPort(URI uri) {
-        if (uri.getPort() != -1) {
-            return uri.getPort();
-        }
-
-        return isHttps(uri) ? HTTPS_PORT : HTTP_PORT;
-    }
-
-    private static boolean isHttp(URI uri) {
-        return "http".equalsIgnoreCase(uri.getScheme());
-    }
-
-    private static boolean isHttps(URI uri) {
-        return "https".equalsIgnoreCase(uri.getScheme());
-    }
-
 }

src/main/java/teammates/ui/servlets/LoginServlet.java

@@ -37,11 +37,12 @@ public LoginServlet() {
 
     @Override
     public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        String nextUrl = UrlHelper.getSafeRedirectUrl(req.getParameter(Const.ParamsNames.NEXT_URL));
+        String nextUrl = UrlHelper.getSafeRelativeRedirectUrl(req.getParameter(Const.ParamsNames.NEXT_URL));
 
         if (!isLoginNeeded(req)) {
             log.request(req, HttpStatus.SC_MOVED_TEMPORARILY, "Redirect to next URL");
-            String redirectUrl = resp.encodeRedirectURL(nextUrl);
+            String redirectUrl = Config.getFrontEndAppUrl(nextUrl).toAbsoluteString();
+            redirectUrl = resp.encodeRedirectURL(redirectUrl);
             resp.sendRedirect(redirectUrl);
             return;
         }

src/main/java/teammates/ui/servlets/LogoutServlet.java

@@ -8,6 +8,7 @@
 
 import org.apache.http.HttpStatus;
 
+import teammates.common.util.Config;
 import teammates.common.util.Logger;
 import teammates.common.util.UrlHelper;
 
@@ -25,7 +26,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc
         Cookie cookie = getLoginInvalidationCookie();
         resp.addCookie(cookie);
 
-        String frontendUrl = UrlHelper.getSafeRedirectUrl(req.getParameter("frontendUrl"));
+        String frontendUrl = Config.getFrontEndAppUrl(UrlHelper.DEFAULT_REDIRECT_URL).toAbsoluteString();
         frontendUrl = resp.encodeRedirectURL(frontendUrl);
         log.request(req, HttpStatus.SC_MOVED_TEMPORARILY, "Redirect to home page after logging out");
         resp.sendRedirect(frontendUrl);

src/main/java/teammates/ui/servlets/OAuth2CallbackServlet.java

@@ -71,8 +71,9 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO
             return;
         }
 
-        String nextUrl = UrlHelper.getSafeRedirectUrl(state.nextUrl());
-        String redirectUrl = resp.encodeRedirectURL(nextUrl);
+        String nextUrl = UrlHelper.getSafeRelativeRedirectUrl(state.nextUrl());
+        String redirectUrl = Config.getFrontEndAppUrl(nextUrl).toAbsoluteString();
+        redirectUrl = resp.encodeRedirectURL(redirectUrl);
         log.info("Going to redirect to: " + redirectUrl);
 
         log.request(req, HttpStatus.SC_MOVED_TEMPORARILY, "Login successful");

src/main/resources/userEmailTemplate-feedbackSessionPublishedPreview.html

@@ -10,7 +10,7 @@
   The results for the TEAMMATES session ${feedbackSessionName} in course [${courseId}] ${courseName} are now
   available for viewing.
   If a student did not receive the unique access link via email, and cannot find it in the spam box either, go to
-  <a href=${sessionsRecoveryLink}>this link</a> to recover the access link.
+  <a href="${sessionsRecoveryLink}">this link</a> to recover the access link.
   <hr>
   The email below has been sent to student recipients of course: [${courseId}] ${courseName}.
   <br><br>

src/test/java/teammates/common/util/UrlHelperTest.java

@@ -4,10 +4,7 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static teammates.common.util.UrlHelper.encodeQueryParam;
-import static teammates.common.util.UrlHelper.isSafeRedirectUrl;
-
-import java.net.URI;
-import java.net.URISyntaxException;
+import static teammates.common.util.UrlHelper.isSafeRelativeRedirectUrl;
 
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -19,76 +16,101 @@
  */
 public class UrlHelperTest extends BaseTestCase {
 
-    @Test
-    public void testIsSafeRedirectUrl_relativeUrl_returnsTrue() {
-        String url = "/web/instructor/home";
+    @Test(dataProvider = "relativeUrls")
+    public void testIsSafeRelativeRedirectUrl_relativeUrl_returnsTrue(String url) {
+        assertTrue(isSafeRelativeRedirectUrl(url));
+    }
 
-        assertTrue(isSafeRedirectUrl(url));
+    @DataProvider
+    private Object[][] relativeUrls() {
+        return new Object[][] {
+                {"/web/instructor/home"},
+                {"/web/student/home?query=value"},
+        };
     }
 
-    @Test
-    public void testIsSafeRedirectUrl_configuredFrontendUrl_returnsTrue() {
-        String url = Config.APP_FRONTEND_URL + "/web/instructor/home";
+    @Test(dataProvider = "absoluteUrls")
+    public void testIsSafeRelativeRedirectUrl_absoluteUrl_returnsFalse(String url) {
+        assertFalse(isSafeRelativeRedirectUrl(url));
+    }
 
-        assertTrue(isSafeRedirectUrl(url));
+    @DataProvider
+    private Object[][] absoluteUrls() {
+        return new Object[][] {
+                {"https://example.com/web/instructor/home"},
+                {"http://example.com/web/student/home"},
+        };
     }
 
     @Test(dataProvider = "externalUrls")
-    public void testIsSafeRedirectUrl_externalUrl_returnsFalse(String url) {
-        assertFalse(isSafeRedirectUrl(url));
+    public void testIsSafeRelativeRedirectUrl_externalUrl_returnsFalse(String url) {
+        assertFalse(isSafeRelativeRedirectUrl(url));
     }
 
     @DataProvider
     private Object[][] externalUrls() {
         return new Object[][] {
                 {"https://example.com/web/instructor/home"},
                 {"https://evil.example.com"},
+                {"evil.com/web/instructor/home"},
+                {"//evil.com/web/instructor/home"},
         };
     }
 
-    @Test
-    public void testIsSafeRedirectUrl_differentPort_returnsFalse() throws URISyntaxException {
-        URI frontendUri = new URI(Config.APP_FRONTEND_URL);
-        int differentPort = frontendUri.getPort() == 8080 ? 8081 : 8080;
-        String url = String.format("%s://%s:%d/web/instructor/home",
-                frontendUri.getScheme(), frontendUri.getHost(), differentPort);
-
-        assertFalse(isSafeRedirectUrl(url));
+    @Test(dataProvider = "malformedUrls")
+    public void testIsSafeRelativeRedirectUrl_malformedUrl_returnsFalse(String url) {
+        assertFalse(isSafeRelativeRedirectUrl(url));
     }
 
-    @Test
-    public void testIsSafeRedirectUrl_protocolRelativeUrl_returnsFalse() {
-        assertFalse(isSafeRedirectUrl("//example.com/web/instructor/home"));
+    @DataProvider
+    private Object[][] malformedUrls() {
+        return new Object[][] {
+                {"https://[invalid"},
+                {"example.com/invalid path"},
+        };
     }
 
-    @Test(dataProvider = "unsupportedSchemeUrls")
-    public void testIsSafeRedirectUrl_unsupportedScheme_returnsFalse(String url) {
-        assertFalse(isSafeRedirectUrl(url));
+    @Test(dataProvider = "invalidRedirectUrls")
+    public void testIsSafeRelativeRedirectUrl_invalidRedirectUrl_returnsFalse(String url) {
+        assertFalse(isSafeRelativeRedirectUrl(url));
     }
 
     @DataProvider
-    private Object[][] unsupportedSchemeUrls() {
+    private Object[][] invalidRedirectUrls() {
         return new Object[][] {
-                {"javascript:alert(1)"},
-                {"ftp://example.com/web/instructor/home"},
+                {"?query=param"},
+                {"web/instructor"},
+                {"https://evil.com"},
+                {"//evil.com"},
         };
     }
 
-    @Test
-    public void testIsSafeRedirectUrl_nullUrl_returnsFalse() {
-        assertFalse(isSafeRedirectUrl(null));
+    @Test(dataProvider = "invalidUrls")
+    public void testIsSafeRelativeRedirectUrl_invalidUrl_returnsFalse(String url) {
+        assertFalse(isSafeRelativeRedirectUrl(url));
     }
 
-    @Test(dataProvider = "malformedUrls")
-    public void testIsSafeRedirectUrl_malformedUrl_returnsFalse(String url) {
-        assertFalse(isSafeRedirectUrl(url));
+    @DataProvider
+    private Object[][] invalidUrls() {
+        return new Object[][] {
+                {""},
+                {null},
+                {"javascript:alert(1)"},
+        };
+    }
+
+    @Test(dataProvider = "unsupportedSchemaUrls")
+    public void testIsSafeRelativeRedirectUrl_unsupportedSchemaUrl_returnsFalse(String url) {
+        assertFalse(isSafeRelativeRedirectUrl(url));
     }
 
     @DataProvider
-    private Object[][] malformedUrls() {
+    private Object[][] unsupportedSchemaUrls() {
         return new Object[][] {
-                {"https://[invalid"},
-                {"web/instructor/home"},
+                {"ftp://example.com/resource"},
+                {"file:///path/to/file"},
+                {"mailto:example@example.com"},
+                {"../relative/path"},
         };
     }
 

src/test/java/teammates/ui/servlets/LoginServletTest.java

@@ -99,7 +99,7 @@ public void doGet_validCookieForExistingAccount_redirectsToNextUrl() throws Exce
             servlet.doGet(req, resp);
         }
 
-        assertEquals("/web/instructor/home", resp.getRedirectUrl());
+        assertEquals(Config.APP_FRONTEND_URL + "/web/instructor/home", resp.getRedirectUrl());
     }
 
     @Test

src/test/java/teammates/ui/servlets/OAuth2CallbackServletTest.java

@@ -193,7 +193,7 @@ public void doGet_callbackHandlerReturnsAuthResult_redirectsToNextUrl() throws E
             servlet.doGet(req, resp);
         }
 
-        assertEquals("/web/instructor/home", resp.getRedirectUrl());
+        assertEquals(Config.APP_FRONTEND_URL + "/web/instructor/home", resp.getRedirectUrl());
     }
 
     private static MockedStatic<Config> mockSupportedLoginMethod(LoginMethod loginMethod, boolean isSupported) {

src/test/resources/emails/sessionPublishedEmailCopyToInstructor.html

@@ -10,7 +10,7 @@
   The results for the TEAMMATES session First feedback session in course [idOfTypicalCourse1] Typical Course 1 with 2 Evals are now
   available for viewing.
   If a student did not receive the unique access link via email, and cannot find it in the spam box either, go to
-  <a href=http://localhost:4200/web/front/help/session-links-recovery>this link</a> to recover the access link.
+  <a href="${app.url}/web/front/help/session-links-recovery">this link</a> to recover the access link.
   <hr>
   The email below has been sent to student recipients of course: [idOfTypicalCourse1] Typical Course 1 with 2 Evals.
   <br><br>

src/test/resources/emails/sessionPublishedEmailForStudent.html

@@ -30,7 +30,7 @@
   <ul>
     <li>
       <strong>To view the responses for this session, please go to this Web address: </strong>
-      <a href="http://localhost:4200/web/sessions/${fsid}/result?key=${key.enc}">http://localhost:4200/web/sessions/${fsid}/result?key=${key.enc}</a>
+      <a href="${app.url}/web/sessions/${fsid}/result?key=${key.enc}">${app.url}/web/sessions/${fsid}/result?key=${key.enc}</a>
     </li>
     <li>The above link is unique to you. Please do not share it with others.</li>
   </ul>