Skip to content
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e44c91e
Build frontend redirect url in backend
TobyCyan Jun 25, 2026
22c9a27
Normalize nextUrl to relative url and add tests
TobyCyan Jun 25, 2026
d5696f7
Add tests
TobyCyan Jun 25, 2026
7813e9b
Clean up data providers
TobyCyan Jun 25, 2026
3cc736b
Merge branch 'master' into redirect-frontend-url
TobyCyan Jun 26, 2026
2430aac
Merge branch 'master' into redirect-frontend-url
TobyCyan Jun 26, 2026
f56666f
Use isEmpty to check empty url
TobyCyan Jun 26, 2026
7f63215
Update frontend port to 8080 in build dev prop
TobyCyan Jun 26, 2026
599404a
Update email test templates
TobyCyan Jun 26, 2026
fa66f37
Update email test templates
TobyCyan Jun 26, 2026
7ee639b
Merge branch 'master' into redirect-frontend-url
TobyCyan Jun 29, 2026
911a174
Pass app.url for templates
TobyCyan Jun 29, 2026
09b7e5f
Revert build-dev template changes
TobyCyan Jun 29, 2026
034a29a
Set frontend port to 8080 for e2e CI
TobyCyan Jun 29, 2026
4a184d2
Merge branch 'master' into redirect-frontend-url
TobyCyan Jul 6, 2026
3aa8c17
Consider absolute urls as unsafe for redirection
TobyCyan Jul 6, 2026
f522f71
Restore e2e and axe yml
TobyCyan Jul 6, 2026
1527dac
Merge branch 'master' into redirect-frontend-url
TobyCyan Jul 10, 2026
a405d45
Extend comment
TobyCyan Jul 10, 2026
a3b2405
Validate the redirectUrl as relative without extracting
TobyCyan Jul 23, 2026
4b28fad
Merge branch 'master' into redirect-frontend-url
TobyCyan Jul 23, 2026
b1b3267
Simplify comment
TobyCyan Jul 23, 2026
fee0787
Use default redirect url const in log out
TobyCyan Jul 23, 2026
e1c0fe5
Merge branch 'master' into redirect-frontend-url
TobyCyan Jul 24, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/axe.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ jobs:
- name: Start Docker services
run: docker compose up -d
- name: Create Config Files
run: ./gradlew createConfigs testClasses
run: ./gradlew createConfigs
- name: Set CI Frontend URL
run: sed -i 's#^app.frontend.url *=.*#app.frontend.url = http\\://localhost:8080#' src/main/resources/build-dev.properties
- name: Compile Test Classes
run: ./gradlew testClasses
- name: Install Frontend Dependencies
run: npm ci
- name: Build Frontend Bundle
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ jobs:
- name: Start Docker services
run: docker compose up -d
- name: Create Config Files
run: ./gradlew createConfigs testClasses
run: ./gradlew createConfigs
- name: Set CI Frontend URL
run: sed -i 's#^app.frontend.url *=.*#app.frontend.url = http\\://localhost:8080#' src/main/resources/build-dev.properties
- name: Compile Test Classes
run: ./gradlew testClasses
- name: Install Frontend Dependencies
run: npm ci
- name: Build Frontend Bundle
Expand Down
4 changes: 0 additions & 4 deletions src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
37 changes: 32 additions & 5 deletions src/main/java/teammates/common/util/UrlHelper.java
Comment thread
TobyCyan marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,45 @@ 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 relative URL from the given absolute URL.
*
* <p>
* If the URL is not absolute, it returns the original URL.
* If the URL doesn't have a path or is invalid, it returns the {@link #DEFAULT_REDIRECT_URL}.
*/
public static String getSafeRedirectUrl(String nextUrl) {
return isSafeRedirectUrl(nextUrl) ? nextUrl : DEFAULT_REDIRECT_URL;
public static String getRelativeUrl(String url) {
Comment thread
TobyCyan marked this conversation as resolved.
Outdated
if (StringHelper.isEmpty(url)) {
return DEFAULT_REDIRECT_URL;
}

try {
URI uri = new URI(url);
String relativeUrl = uri.getPath();
// Only preserve the query when there is a path.
return StringHelper.isEmpty(relativeUrl)
? DEFAULT_REDIRECT_URL
: relativeUrl + (uri.getQuery() != null ? "?" + uri.getQuery() : "");
} catch (URISyntaxException e) {
return DEFAULT_REDIRECT_URL;
}
}

/**
* Normalizes the given redirectUrl to a relative URL and checks if it is safe.
*
* <p>
* If the provided redirectUrl is not safe, it returns the {@link #DEFAULT_REDIRECT_URL}.
*/
public static String getSafeRelativeRedirectUrl(String redirectUrl) {
String relativeUrl = getRelativeUrl(redirectUrl);
return isSafeRedirectUrl(relativeUrl) ? relativeUrl : DEFAULT_REDIRECT_URL;
}

/**
* Checks whether the given URL is safe to use as a redirect target.
*/
public static boolean isSafeRedirectUrl(String url) {
if (url == null) {
if (StringHelper.isEmpty(url)) {
return false;
}

Expand Down
5 changes: 3 additions & 2 deletions src/main/java/teammates/ui/servlets/LoginServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ public LoginServlet() {

@Override
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
String nextUrl = UrlHelper.getSafeRedirectUrl(req.getParameter("nextUrl"));
String nextUrl = UrlHelper.getSafeRelativeRedirectUrl(req.getParameter("nextUrl"));

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;
}
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/teammates/ui/servlets/LogoutServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

import org.apache.http.HttpStatus;

import teammates.common.util.Config;
import teammates.common.util.Logger;
import teammates.common.util.UrlHelper;

/**
* Servlet that handles logout.
Expand All @@ -25,8 +25,7 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOExc
Cookie cookie = getLoginInvalidationCookie();
resp.addCookie(cookie);

String frontendUrl = UrlHelper.getSafeRedirectUrl(req.getParameter("frontendUrl"));
frontendUrl = resp.encodeRedirectURL(frontendUrl);
String frontendUrl = resp.encodeRedirectURL(Config.getFrontEndAppUrl("/").toAbsoluteString());
log.request(req, HttpStatus.SC_MOVED_TEMPORARILY, "Redirect to home page after logging out");
resp.sendRedirect(frontendUrl);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
132 changes: 105 additions & 27 deletions src/test/java/teammates/common/util/UrlHelperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +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.getRelativeUrl;
import static teammates.common.util.UrlHelper.isSafeRedirectUrl;

import java.net.URI;
Expand All @@ -19,20 +20,24 @@
*/
public class UrlHelperTest extends BaseTestCase {

@Test
public void testIsSafeRedirectUrl_relativeUrl_returnsTrue() {
String url = "/web/instructor/home";

@Test(dataProvider = "relativeUrls")
public void testIsSafeRedirectUrl_relativeUrl_returnsTrue(String url) {
assertTrue(isSafeRedirectUrl(url));
}

@Test
public void testIsSafeRedirectUrl_configuredFrontendUrl_returnsTrue() {
String url = Config.APP_FRONTEND_URL + "/web/instructor/home";

@Test(dataProvider = "configuredFrontendUrls")
public void testIsSafeRedirectUrl_configuredFrontendUrl_returnsTrue(String url) {
assertTrue(isSafeRedirectUrl(url));
}

@DataProvider
private Object[][] configuredFrontendUrls() {
return new Object[][] {
{Config.APP_FRONTEND_URL + "/web/instructor/home"},
{Config.APP_FRONTEND_URL + "/web/student/home?query=value"},
};
}

@Test(dataProvider = "externalUrls")
public void testIsSafeRedirectUrl_externalUrl_returnsFalse(String url) {
assertFalse(isSafeRedirectUrl(url));
Expand Down Expand Up @@ -61,19 +66,6 @@ public void testIsSafeRedirectUrl_protocolRelativeUrl_returnsFalse() {
assertFalse(isSafeRedirectUrl("//example.com/web/instructor/home"));
}

@Test(dataProvider = "unsupportedSchemeUrls")
public void testIsSafeRedirectUrl_unsupportedScheme_returnsFalse(String url) {
assertFalse(isSafeRedirectUrl(url));
}

@DataProvider
private Object[][] unsupportedSchemeUrls() {
return new Object[][] {
{"javascript:alert(1)"},
{"ftp://example.com/web/instructor/home"},
};
}

@Test
public void testIsSafeRedirectUrl_nullUrl_returnsFalse() {
assertFalse(isSafeRedirectUrl(null));
Expand All @@ -84,12 +76,19 @@ public void testIsSafeRedirectUrl_malformedUrl_returnsFalse(String url) {
assertFalse(isSafeRedirectUrl(url));
}

@DataProvider
private Object[][] malformedUrls() {
return new Object[][] {
{"https://[invalid"},
{"web/instructor/home"},
};
@Test(dataProvider = "invalidRedirectUrls")
public void testIsSafeRedirectUrl_invalidRedirectUrl_returnsFalse(String url) {
assertFalse(isSafeRedirectUrl(url));
}

@Test(dataProvider = "invalidUrls")
public void testIsSafeRedirectUrl_invalidUrl_returnsFalse(String url) {
assertFalse(isSafeRedirectUrl(url));
}

@Test(dataProvider = "unsupportedSchemaUrls")
public void testIsSafeRedirectUrl_unsupportedSchemaUrl_returnsFalse(String url) {
assertFalse(isSafeRedirectUrl(url));
}

@Test
Expand All @@ -107,4 +106,83 @@ public void testEncodeQueryParam_paramWithSpecialChars_returnsEncoded() {
assertEquals("with%2Fspecial%3Fchars%26", encodeQueryParam("with/special?chars&"));
}

@Test(dataProvider = "absoluteUrls")
public void testGetRelativeUrl_absoluteUrl_returnsRelative(String absoluteUrl, String expectedRelativeUrl) {
assertEquals(expectedRelativeUrl, getRelativeUrl(absoluteUrl));
}

@DataProvider
private Object[][] absoluteUrls() {
return new Object[][] {
{"https://somedomain/web/instructor/home?query=value", "/web/instructor/home?query=value"},
{"https://somedomain/web/instructor/home", "/web/instructor/home"},
};
}

@Test(dataProvider = "relativeUrls")
public void testGetRelativeUrl_relativeUrl_returnsSame(String url) {
assertEquals(url, getRelativeUrl(url));
}

@DataProvider
private Object[][] relativeUrls() {
return new Object[][] {
{"/web/instructor/home"},
{"/web/student/home?query=value"},
};
}

@Test
public void testGetRelativeUrl_emptyPath_returnsDefault() {
String emptyPathUrl = "http://somedomain";

assertEquals("/", getRelativeUrl(emptyPathUrl));
}

@Test(dataProvider = "malformedUrls")
public void testGetRelativeUrl_malformedUrl_returnsDefault(String url) {
assertEquals("/", getRelativeUrl(url));
}

@Test(dataProvider = "invalidUrls")
public void testGetRelativeUrl_invalidUrl_returnsDefault(String url) {
assertEquals("/", getRelativeUrl(url));
}

@DataProvider
private Object[][] unsupportedSchemaUrls() {
return new Object[][] {
{"ftp://example.com/resource"},
{"file:///path/to/file"},
{"mailto:example@example.com"},
{"../relative/path"},
};
}

@DataProvider
private Object[][] malformedUrls() {
return new Object[][] {
{"https://[invalid"},
{"example.com/invalid path"},
};
}

@DataProvider
private Object[][] invalidRedirectUrls() {
return new Object[][] {
{"?query=param"},
{"web/instructor"},
{"https://evil.com"},
{"//evil.com"},
};
}

@DataProvider
private Object[][] invalidUrls() {
return new Object[][] {
{""},
{null},
{"javascript:alert(1)"},
};
}
}
2 changes: 1 addition & 1 deletion src/test/java/teammates/ui/servlets/LoginServletTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading