Skip to content

Commit ad8dfda

Browse files
committed
VPR-151 fix(auth): handle /2 PathBase in welcome/login redirect guards
Three guards in HomeController matched root-relative literals against the un-stripped ReturnUrl, so they misbehaved only under the /2 subpath deployment (TEST/PROD), not local dev: - Login's /api guard let a "/2/api/..." ReturnUrl bypass it and be forwarded to CAS; strip the PathBase before the check. - Welcome's redirect-loop guard sent "/2/welcome" and "/2/login" to CAS instead of nulling them; strip before the check. - Authenticated Welcome (no ReturnUrl) and the CasLogin fallback redirected to "/" (the legacy ColdFusion root) instead of "~/" (the /2 app root).
1 parent fcec756 commit ad8dfda

3 files changed

Lines changed: 344 additions & 48 deletions

File tree

VueApp/src/composables/__tests__/require-login.test.ts

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,44 +10,61 @@ function withBase(base: string, run: () => void): void {
1010
}
1111
}
1212

13+
describe("buildLoginUrl endpoint selection", () => {
14+
it("defaults to the welcome splash", () => {
15+
withBase("/", () => {
16+
expect(buildLoginUrl("/Effort")).toBe("/welcome?ReturnUrl=%2FEffort")
17+
})
18+
})
19+
20+
it("goes straight to CAS when the login endpoint is requested", () => {
21+
// Explicit "Log in" buttons skip the splash so a deliberate click isn't met
22+
// with a second sign-in screen.
23+
withBase("/", () => {
24+
expect(buildLoginUrl("/Effort", "login")).toBe("/login?ReturnUrl=%2FEffort")
25+
})
26+
})
27+
})
28+
1329
describe("buildLoginUrl application base handling", () => {
1430
it("emits root-relative paths when the app is served at the domain root", () => {
1531
withBase("/", () => {
16-
expect(buildLoginUrl("/CTS/")).toBe("/login?ReturnUrl=%2FCTS%2F")
32+
expect(buildLoginUrl("/CTS/")).toBe("/welcome?ReturnUrl=%2FCTS%2F")
1733
})
1834
})
1935

2036
it("keeps the /2 base for the subpath deployment", () => {
2137
// TEST/PROD run VIPER 2 under a "/2" PathBase; dropping it escapes to the legacy site.
2238
withBase("/2/", () => {
23-
expect(buildLoginUrl("/2/CTS/")).toBe("/2/login?ReturnUrl=%2F2%2FCTS%2F")
39+
expect(buildLoginUrl("/2/CTS/")).toBe("/2/welcome?ReturnUrl=%2F2%2FCTS%2F")
40+
expect(buildLoginUrl("/2/CTS/", "login")).toBe("/2/login?ReturnUrl=%2F2%2FCTS%2F")
2441
})
2542
})
2643

2744
it("inserts the separator when the base is configured without a trailing slash", () => {
28-
// Guards the "/2login" regression: the base is normalized, then joined with "/".
45+
// Guards the "/2welcome" regression: the base is normalized, then joined with "/".
2946
withBase("/2", () => {
30-
expect(buildLoginUrl("/2/CTS/")).toBe("/2/login?ReturnUrl=%2F2%2FCTS%2F")
47+
expect(buildLoginUrl("/2/CTS/")).toBe("/2/welcome?ReturnUrl=%2F2%2FCTS%2F")
3148
})
3249
})
3350

3451
it("collapses a base with duplicate trailing slashes", () => {
3552
withBase("/2///", () => {
36-
expect(buildLoginUrl("/2/CTS/")).toBe("/2/login?ReturnUrl=%2F2%2FCTS%2F")
53+
expect(buildLoginUrl("/2/CTS/")).toBe("/2/welcome?ReturnUrl=%2F2%2FCTS%2F")
3754
})
3855
})
3956
})
4057

4158
describe("buildLoginUrl invalid return paths", () => {
4259
it("falls back to the application root instead of forwarding an off-site path", () => {
4360
withBase("/", () => {
44-
expect(buildLoginUrl("https://evil.example/x")).toBe("/login?ReturnUrl=%2F")
61+
expect(buildLoginUrl("https://evil.example/x")).toBe("/welcome?ReturnUrl=%2F")
4562
})
4663
})
4764

4865
it("falls back to the base-prefixed root in the subpath deployment", () => {
4966
withBase("/2/", () => {
50-
expect(buildLoginUrl("https://evil.example/x")).toBe("/2/login?ReturnUrl=%2F2%2F")
67+
expect(buildLoginUrl("https://evil.example/x")).toBe("/2/welcome?ReturnUrl=%2F2%2F")
5168
})
5269
})
5370
})
@@ -56,7 +73,7 @@ describe("buildLoginUrl return paths carrying a query string", () => {
5673
it("round-trips the query string (the CTS landing forwards location.search)", () => {
5774
withBase("/2/", () => {
5875
expect(buildLoginUrl("/2/CTS/?sendBackTo=/cts/epa")).toBe(
59-
"/2/login?ReturnUrl=%2F2%2FCTS%2F%3FsendBackTo%3D%2Fcts%2Fepa",
76+
"/2/welcome?ReturnUrl=%2F2%2FCTS%2F%3FsendBackTo%3D%2Fcts%2Fepa",
6077
)
6178
})
6279
})
@@ -68,14 +85,14 @@ describe("buildLoginUrl return paths carrying a query string", () => {
6885
// the app root after sign-in.
6986
withBase("/2/", () => {
7087
expect(buildLoginUrl("/2/CTS/?sendBackTo=%2Fcts%2Fepa")).toBe(
71-
"/2/login?ReturnUrl=%2F2%2FCTS%2F%3FsendBackTo%3D%252Fcts%252Fepa",
88+
"/2/welcome?ReturnUrl=%2F2%2FCTS%2F%3FsendBackTo%3D%252Fcts%252Fepa",
7289
)
7390
})
7491
})
7592

7693
it("keeps a return path whose query carries encoded dots", () => {
7794
withBase("/2/", () => {
78-
expect(buildLoginUrl("/2/CTS/?q=%2e%2e")).toBe("/2/login?ReturnUrl=%2F2%2FCTS%2F%3Fq%3D%252e%252e")
95+
expect(buildLoginUrl("/2/CTS/?q=%2e%2e")).toBe("/2/welcome?ReturnUrl=%2F2%2FCTS%2F%3Fq%3D%252e%252e")
7996
})
8097
})
8198
})

test/Controllers/HomeControllerTests.cs

Lines changed: 205 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,67 @@ private void Arrange(bool authenticated)
110110
[InlineData("/login", true)]
111111
[InlineData("/LOGIN?ReturnUrl=/x", true)]
112112
[InlineData("/welcome#frag", true)]
113+
[InlineData("/caslogin", true)] // re-entering the ticket handler without a ticket would 403
114+
[InlineData("/CasLogin/", true)]
113115
[InlineData("/RAPS/Roles", false)]
114116
[InlineData("/welcomepage", false)]
115-
public void IsWelcomeOrLoginPath_DetectsLoopTargets(string? url, bool expected)
117+
[InlineData("/caslogins", false)]
118+
public void IsAuthEntryPath_DetectsLoopTargets(string? url, bool expected)
116119
{
117-
Assert.Equal(expected, HomeController.IsWelcomeOrLoginPath(url));
120+
Assert.Equal(expected, HomeController.IsAuthEntryPath(url));
121+
}
122+
123+
// Parity with the Vue guard, which rejects "../" and any "%2e" outright. A dot-segment survives
124+
// IsLocalUrl and the root-relative /api check, then resolves somewhere else once the browser
125+
// follows it — including in the percent-encoded spellings the URL spec also resolves.
126+
[Theory]
127+
[InlineData(null, false)]
128+
[InlineData("", false)]
129+
[InlineData("/Effort/Reports", false)]
130+
[InlineData("/Effort/..api/x", false)] // ".." only counts as a whole segment
131+
[InlineData("/dots../x", false)]
132+
[InlineData("/Effort/%2ename/x", false)] // encoded dot only counts as a whole segment too
133+
[InlineData("/Effort/../api/secret", true)]
134+
[InlineData("/2/Effort/../api/secret", true)]
135+
[InlineData("/./api/secret", true)]
136+
[InlineData("/Effort/..", true)]
137+
[InlineData("/Effort/../api?tab=1", true)]
138+
[InlineData("/Effort/%2e%2e/api/secret", true)] // browsers resolve the encoded form the same way
139+
[InlineData("/Effort/%2E%2E/api/secret", true)] // and the match is ASCII case-insensitive
140+
[InlineData("/Effort/.%2e/api/secret", true)] // mixed encoding counts as ".." too
141+
[InlineData("/Effort/%2e./api/secret", true)]
142+
[InlineData("/%2e/api/secret", true)]
143+
public void ContainsDotSegment_DetectsTraversal(string? url, bool expected)
144+
{
145+
Assert.Equal(expected, HomeController.ContainsDotSegment(url));
146+
}
147+
148+
// The single ReturnUrl contract shared by /welcome, /login and /CasLogin, exercised under a
149+
// subpath deployment so the base-prefixed and "~/" spellings are covered in one place.
150+
[Theory]
151+
[InlineData("/Effort", true)]
152+
[InlineData("/2/Effort", true)]
153+
[InlineData("~/Effort", true)]
154+
[InlineData("/2/Effort/Reports?year=2026", true)]
155+
[InlineData("/welcomepage", true)] // near-match on an entry point is a normal page
156+
[InlineData(null, false)]
157+
[InlineData("", false)]
158+
[InlineData("https://evil.com/phish", false)]
159+
[InlineData("//evil.com", false)]
160+
[InlineData("/welcome", false)] // redirect loop
161+
[InlineData("/login", false)]
162+
[InlineData("/caslogin", false)] // ticketless re-entry 403s a user who just signed in
163+
[InlineData("/2/welcome", false)] // base-prefixed entry points must be caught too
164+
[InlineData("/2/CasLogin/", false)]
165+
[InlineData("~/welcome", false)] // app-relative spelling must not slip past
166+
[InlineData("/Effort/../api/secret", false)]
167+
[InlineData("/2/Effort/%2e%2e/api/secret", false)]
168+
public void IsSafeReturnUrl_EnforcesSharedContract(string? returnUrl, bool expected)
169+
{
170+
Arrange(authenticated: false);
171+
_controller.HttpContext.Request.PathBase = "/2";
172+
173+
Assert.Equal(expected, _controller.IsSafeReturnUrl(returnUrl));
118174
}
119175

120176
// Splash appears only for the front door (an empty return path or the bare site root) and for a
@@ -258,6 +314,45 @@ public void Welcome_Anonymous_SubpathDeepLink_RedirectsToCasLogin()
258314
Assert.Equal("/2/ClinicalScheduler/rotation", redirect.RouteValues?["ReturnUrl"]);
259315
}
260316

317+
// Under a subpath deployment every unsafe ReturnUrl arrives base-prefixed, so the base has to come
318+
// off before the guards run. Both classes are covered here: auth entry points that would loop (or
319+
// 403 on a ticketless re-entry), and dot-segments the browser resolves elsewhere after the CAS round
320+
// trip. In each case the splash renders with a null ReturnUrl rather than bouncing back out to CAS.
321+
[Theory]
322+
[InlineData("/2/welcome")]
323+
[InlineData("/2/login")]
324+
[InlineData("/2/Welcome/")]
325+
[InlineData("/2/caslogin")] // would re-enter the ticket handler ticketless and 403 after a good sign-in
326+
[InlineData("/Effort/../api/secret")]
327+
[InlineData("/2/Effort/../api/secret")]
328+
[InlineData("/2/Effort/%2e%2e/api/secret")] // browsers resolve the encoded spelling the same way
329+
public void Welcome_Anonymous_SubpathUnsafeReturnUrl_DropsReturnUrl(string returnUrl)
330+
{
331+
Arrange(authenticated: false);
332+
_controller.HttpContext.Request.PathBase = "/2";
333+
334+
var result = _controller.Welcome(returnUrl);
335+
336+
var view = Assert.IsType<ViewResult>(result);
337+
Assert.Equal("Welcome", view.ViewName);
338+
Assert.Null(view.ViewData["ReturnUrl"]);
339+
}
340+
341+
// Authenticated welcome with no ReturnUrl under a subpath deployment redirects to "~/" so the app
342+
// root keeps its PathBase ("/2/") instead of escaping to the domain root. Regression guard for the
343+
// bare "/" that sent logged-in users out to the legacy site.
344+
[Fact]
345+
public void Welcome_Authenticated_NoReturnUrl_RedirectsToAppRelativeRoot()
346+
{
347+
Arrange(authenticated: true);
348+
_controller.HttpContext.Request.PathBase = "/2";
349+
350+
var result = _controller.Welcome();
351+
352+
var redirect = Assert.IsType<LocalRedirectResult>(result);
353+
Assert.Equal("~/", redirect.Url);
354+
}
355+
261356
// Anonymous users still get the Welcome view, but any ReturnUrl that is non-local
262357
// (open redirect) or points back at /welcome|/login (redirect loop) is dropped.
263358
[Theory]
@@ -288,6 +383,8 @@ public void Welcome_Authenticated_RedirectsToLocalReturnUrl()
288383
Assert.Equal("/Effort/Foo", redirect.Url);
289384
}
290385

386+
// App root is "~/" (not "/") so a subpath deployment keeps its PathBase ("/2/") rather than
387+
// escaping to the domain root (the legacy site).
291388
[Theory]
292389
[InlineData("https://evil.com")]
293390
[InlineData(null)]
@@ -298,11 +395,28 @@ public void Welcome_Authenticated_RedirectsToRootWhenReturnUrlInvalidOrMissing(s
298395
var result = _controller.Welcome(returnUrl);
299396

300397
var redirect = Assert.IsType<LocalRedirectResult>(result);
301-
Assert.Equal("/", redirect.Url);
398+
Assert.Equal("~/", redirect.Url);
399+
}
400+
401+
// The /api guard matches the api segment case-insensitively (routing is case-insensitive) and
402+
// on a segment boundary, so "/api", "/api/...", "/api?..." are rejected in any casing while
403+
// non-API paths that merely start with "api" (e.g. "/apiary") pass through to CAS.
404+
[Theory]
405+
[InlineData("/api", true)]
406+
[InlineData("/api/foo", true)]
407+
[InlineData("/API/foo", true)]
408+
[InlineData("/Api?x=1", true)]
409+
[InlineData("/api#frag", true)]
410+
[InlineData("/apiary", false)]
411+
[InlineData("/", false)]
412+
public void IsApiPath_MatchesApiSegmentCaseInsensitivelyOnBoundary(string url, bool expected)
413+
{
414+
Assert.Equal(expected, HomeController.IsApiPath(url));
302415
}
303416

304417
[Theory]
305418
[InlineData("/api/secret")]
419+
[InlineData("/API/secret")] // routing is case-insensitive, so the guard must be too
306420
[InlineData("~/api/secret")] // app-relative form must not bypass the /api guard
307421
public void Login_RejectsApiReturnUrl_WithUnauthorized(string returnUrl)
308422
{
@@ -313,6 +427,35 @@ public void Login_RejectsApiReturnUrl_WithUnauthorized(string returnUrl)
313427
Assert.IsType<UnauthorizedResult>(result);
314428
}
315429

430+
// A non-API path that merely starts with "api" is not caught by the guard; it proceeds to the
431+
// normal CAS redirect.
432+
[Fact]
433+
public void Login_ForwardsNonApiPathStartingWithApiToCas()
434+
{
435+
Arrange(authenticated: false);
436+
437+
var result = _controller.Login("/apiary");
438+
439+
Assert.IsType<RedirectResult>(result);
440+
}
441+
442+
// Under a subpath deployment the /api ReturnUrl arrives base-prefixed ("/2/api/..."). The base is
443+
// stripped before the guard so it is still rejected and never forwarded to CAS. Regression guard for
444+
// the pre-strip /api check that a "/2/api/..." ReturnUrl slipped past.
445+
[Theory]
446+
[InlineData("/2/api/secret")]
447+
[InlineData("/2/API/secret")] // base-prefixed + mixed case must not bypass the guard
448+
[InlineData("~/2/api/secret")] // app-relative + base-prefixed must not bypass the guard either
449+
public void Login_RejectsSubpathApiReturnUrl_WithUnauthorized(string returnUrl)
450+
{
451+
Arrange(authenticated: false);
452+
_controller.HttpContext.Request.PathBase = "/2";
453+
454+
var result = _controller.Login(returnUrl);
455+
456+
Assert.IsType<UnauthorizedResult>(result);
457+
}
458+
316459
[Theory]
317460
[InlineData("https://evil.com/phish")]
318461
[InlineData("//evil.com")]
@@ -325,4 +468,63 @@ public void Login_DoesNotForwardNonLocalReturnUrl(string returnUrl)
325468
var redirect = Assert.IsType<RedirectResult>(result);
326469
Assert.DoesNotContain("evil.com", redirect.Url, StringComparison.OrdinalIgnoreCase);
327470
}
471+
472+
// A dot-segment ReturnUrl passes IsLocalUrl and the root-relative /api check, but the browser
473+
// resolves it after the CAS round trip: "/2/Effort/../api/secret" lands on "/2/api/secret" and
474+
// dumps the user on a JSON 401. Dropped up front instead, matching the Vue guard.
475+
// Asserting on "Effort" rather than "api/secret": the ReturnUrl is double URL-encoded into the CAS
476+
// service URL, so any assertion containing a slash would pass even with the guard removed.
477+
[Theory]
478+
[InlineData("/Effort/../api/secret")]
479+
[InlineData("/2/Effort/../api/secret")]
480+
[InlineData("~/Effort/../api/secret")]
481+
[InlineData("/2/Effort/%2e%2e/api/secret")]
482+
public void Login_DoesNotForwardDotSegmentReturnUrl(string returnUrl)
483+
{
484+
Arrange(authenticated: false);
485+
_controller.HttpContext.Request.PathBase = "/2";
486+
487+
var result = _controller.Login(returnUrl);
488+
489+
var redirect = Assert.IsType<RedirectResult>(result);
490+
Assert.DoesNotContain("Effort", redirect.Url, StringComparison.OrdinalIgnoreCase);
491+
}
492+
493+
// /login applies the same auth-entry guard as /welcome, so "/login?ReturnUrl=/welcome" cannot
494+
// bounce the user back to the splash after a successful sign-in. ("/login" and "/caslogin" are
495+
// covered by IsSafeReturnUrl above: both are substrings of the CAS service URL, so they can't be
496+
// asserted against the redirect target here.)
497+
[Theory]
498+
[InlineData("/welcome")]
499+
[InlineData("/2/welcome")]
500+
[InlineData("~/welcome")]
501+
public void Login_DoesNotForwardAuthEntryReturnUrl(string returnUrl)
502+
{
503+
Arrange(authenticated: false);
504+
_controller.HttpContext.Request.PathBase = "/2";
505+
506+
var result = _controller.Login(returnUrl);
507+
508+
var redirect = Assert.IsType<RedirectResult>(result);
509+
Assert.DoesNotContain("welcome", redirect.Url, StringComparison.OrdinalIgnoreCase);
510+
}
511+
512+
// The anonymous "/" splash must carry the same no-store headers as /welcome, which gets them from
513+
// [ResponseCache]. Both now route through the shared WelcomeSplash helper.
514+
[Fact]
515+
public void Index_And_Welcome_Anonymous_EmitMatchingNoStoreHeaders()
516+
{
517+
Arrange(authenticated: false);
518+
_controller.Index();
519+
var indexCacheControl = _controller.Response.Headers["Cache-Control"].ToString();
520+
var indexPragma = _controller.Response.Headers["Pragma"].ToString();
521+
522+
Arrange(authenticated: false);
523+
_controller.Welcome();
524+
525+
Assert.Equal("no-store,no-cache", indexCacheControl);
526+
Assert.Equal("no-cache", indexPragma);
527+
Assert.Equal(indexCacheControl, _controller.Response.Headers["Cache-Control"].ToString());
528+
Assert.Equal(indexPragma, _controller.Response.Headers["Pragma"].ToString());
529+
}
328530
}

0 commit comments

Comments
 (0)