Skip to content

Fix browser token auth over HTTP - #16818

Closed
Adam Ratzman (adamint) wants to merge 4 commits into
microsoft:mainfrom
adamint:fix/issue16067-browser-token-http
Closed

Fix browser token auth over HTTP#16818
Adam Ratzman (adamint) wants to merge 4 commits into
microsoft:mainfrom
adamint:fix/issue16067-browser-token-http

Conversation

@adamint

Copy link
Copy Markdown
Member

Description

Fixes #16067

This fixes browser-token dashboard authentication when the dashboard is available over both HTTPS and HTTP. Safari can retain HTTPS localhost cookie state that shadows a later HTTP browser-token sign-in, causing the dashboard to prompt for the token repeatedly even after a successful sign-in.

Changes:

  • Adds a browser-token cookie manager that uses a separate .Aspire.Dashboard.Auth.Http cookie name for HTTP requests while keeping .Aspire.Dashboard.Auth for HTTPS requests.
  • Adds mixed HTTPS/HTTP dashboard integration coverage for browser-token sign-in and cookie emission.

Validation:

  • dotnet test --project tests/Aspire.Dashboard.Tests/Aspire.Dashboard.Tests.csproj --no-launch-profile -- --filter-class "*.FrontendBrowserTokenAuthTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"
  • dotnet test --project tests/Aspire.Dashboard.Tests/Aspire.Dashboard.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

Use a distinct dashboard auth cookie name for browser-token auth on HTTP endpoints so stale HTTPS cookie state cannot shadow an HTTP sign-in.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16818

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16818"

Run forwarded header normalization before browser-token validation so sign-in cookie naming uses the same scheme as later authorization.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@adamint
Adam Ratzman (adamint) marked this pull request as ready for review May 6, 2026 04:44
Copilot AI review requested due to automatic review settings May 6, 2026 04:44
@adamint
Adam Ratzman (adamint) marked this pull request as draft May 6, 2026 04:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes browser-token dashboard authentication when the dashboard is reachable over both HTTPS and HTTP (notably Safari’s localhost cookie behavior) by separating cookie names per scheme and adding integration coverage for mixed-scheme scenarios.

Changes:

  • Introduces an ICookieManager implementation that uses a distinct cookie name for HTTP requests while keeping the existing cookie name for HTTPS.
  • Ensures UseForwardedHeaders() runs before browser-token validation so the normalized scheme influences cookie issuance correctly.
  • Adds integration tests covering mixed HTTPS/HTTP login flows and cookie emission semantics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/Aspire.Dashboard.Tests/Integration/FrontendBrowserTokenAuthTests.cs Adds mixed-scheme integration tests for browser-token redirects and cookie name/Secure behavior.
src/Aspire.Dashboard/DashboardWebApplication.cs Wires the new cookie manager and moves forwarded-headers middleware earlier in the pipeline.
src/Aspire.Dashboard/Authentication/BrowserTokenCookieManager.cs Adds scheme-dependent cookie naming logic to avoid HTTP/HTTPS cookie collisions.

Comment thread src/Aspire.Dashboard/Authentication/BrowserTokenCookieManager.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@adamint
Adam Ratzman (adamint) marked this pull request as ready for review May 6, 2026 05:25
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
return Task.CompletedTask;
};
options.Cookie.Name = DashboardAuthCookieName;
options.CookieManager = new BrowserTokenCookieManager(DashboardHttpAuthCookieName);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about OAuth? Should it set this? If so, rename to AspireDashboardCookieManager

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied to OpenID Connect as well in #17368: the manager is now AspireDashboardCookieManager, and both OIDC and browser-token auth use it.

Comment on lines +200 to +201
Assert.Contains(deletedCookies, c => c.StartsWith(".Aspire.Dashboard.Auth=", StringComparison.Ordinal) && c.Contains("expires=Thu, 01 Jan 1970", StringComparison.OrdinalIgnoreCase));
Assert.Contains(deletedCookies, c => c.StartsWith(".Aspire.Dashboard.Auth.Http=", StringComparison.Ordinal) && c.Contains("expires=Thu, 01 Jan 1970", StringComparison.OrdinalIgnoreCase));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert.Collection here to be explicit?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to Assert.Collection in #17368 so the two expected deleted cookies are asserted explicitly.

Comment thread src/Aspire.Dashboard/DashboardWebApplication.cs
@JamesNK

Copy link
Copy Markdown
Member

This change is too risky for 13.3. Soonest it should go in is 13.4.

@davidfowl

Copy link
Copy Markdown
Collaborator

This isn't a regression right?

@JamesNK

Copy link
Copy Markdown
Member

I don't think so. Nothing has changed here.

@adamint

Adam Ratzman (adamint) commented May 6, 2026

Copy link
Copy Markdown
Member Author

Marking as draft, since the issue was pushed back to 13.4

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Browser Token not working on dashboard over HTTP

4 participants