fix(auth): preserve returnUrl in local fallback login#399
Conversation
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refines the Development/Testing Auth0-placeholder fallback so /Account/Login preserves a validated returnUrl when redirecting to the local /test/login endpoint, and updates /test/login to redirect back to the same safe return path after setting the test auth cookie. It also adds AppHost and architecture-level coverage to lock in the redirect contract and improves AppHost test determinism by clearing Auth0 environment variables inside the spawned web resource.
Changes:
- Preserve (and forward)
returnUrlwhen/Account/Loginshort-circuits to/test/loginunder placeholder Auth0 config. - Teach
/test/loginto redirect to the same validated safe relative return path (instead of always/). - Add AppHost + architecture contract tests; clear Auth0 env vars in AppHost fixtures for deterministic placeholder behavior; add docs troubleshooting note.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Architecture.Tests/AuthFallbackContractTests.cs | Adds a source-order contract test ensuring placeholder short-circuit occurs before OIDC challenge. |
| tests/AppHost.Tests/Tests/Auth/LoginFallbackTests.cs | Adds runtime redirect assertions for /Account/Login and /test/login honoring returnUrl. |
| tests/AppHost.Tests/Infrastructure/ClearCommandAppFixture.cs | Clears Auth0 env vars in the spawned web resource to force placeholder behavior in Testing. |
| tests/AppHost.Tests/Infrastructure/AspireManager.cs | Clears Auth0 env vars (with logging) to keep AppHost tests deterministic even with ambient secrets. |
| src/Web/Program.cs | Preserves returnUrl through placeholder fallback and redirects /test/login back to the requested path. |
| docs/AUTH0_SETUP.md | Documents the IDX20803/test.auth0.com timeout pattern and the expected local fallback behavior. |
| .squad/agents/gimli/history.md | Records testing/coverage work and validation notes for the auth fallback coverage changes. |
| .squad/agents/gandalf/history.md | Reformats and records prior auth security findings and the placeholder-login approach. |
| app.MapGet("/Account/Login", async (HttpContext ctx, string? returnUrl) => | ||
| { | ||
| var safeReturn = !string.IsNullOrEmpty(returnUrl) | ||
| && Uri.IsWellFormedUriString(returnUrl, UriKind.Relative) | ||
| ? returnUrl | ||
| : "/"; |
| var roleValue = string.IsNullOrWhiteSpace(role) ? "user" : role; | ||
| var safeReturn = !string.IsNullOrEmpty(returnUrl) | ||
| && Uri.IsWellFormedUriString(returnUrl, UriKind.Relative) | ||
| ? returnUrl | ||
| : "/"; | ||
|
|
| var localRedirectIndex = loginHandlerSource.IndexOf( | ||
| "ctx.Response.Redirect($\"/test/login?returnUrl={Uri.EscapeDataString(safeReturn)}\")", | ||
| StringComparison.Ordinal); | ||
| var challengeIndex = loginHandlerSource.IndexOf( | ||
| "await ctx.ChallengeAsync(Auth0Constants.AuthenticationScheme, props).ConfigureAwait(false);", |
| app.MapGet("/Account/Login", async (HttpContext ctx, string? returnUrl) => | ||
| { | ||
| var safeReturn = !string.IsNullOrEmpty(returnUrl) | ||
| && Uri.IsWellFormedUriString(returnUrl, UriKind.Relative) | ||
| ? returnUrl | ||
| : "/"; |
4da387d to
351153a
Compare
Test Results Summary515 tests +1 514 ✅ +1 32s ⏱️ ±0s Results for commit 351153a. ± Comparison against base commit 3dac6f1. This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #399 +/- ##
=======================================
Coverage 85.46% 85.46%
=======================================
Files 70 70
Lines 1693 1693
Branches 207 207
=======================================
Hits 1447 1447
Misses 166 166
Partials 80 80 🚀 New features to boost your workflow:
|
Summary
returnUrlwhen placeholder Auth0 config redirects/Account/Loginto/test/loginContext
Closes #398
Working as Gandalf (Security / Auth)