Skip to content

Fix browser token auth over HTTP - #17368

Merged
Adam Ratzman (adamint) merged 6 commits into
microsoft:mainfrom
adamint:fix/issue16067-browser-token-http
May 22, 2026
Merged

Fix browser token auth over HTTP#17368
Adam Ratzman (adamint) merged 6 commits into
microsoft:mainfrom
adamint:fix/issue16067-browser-token-http

Conversation

@adamint

Copy link
Copy Markdown
Member

Description

Fixes browser-token dashboard authentication when the dashboard frontend is reachable over both HTTPS and HTTP. Browsers can handle localhost cookies differently across schemes, so a Secure HTTPS auth cookie could shadow the HTTP sign-in flow. The dashboard now uses separate auth cookie names for HTTP and HTTPS requests, and applies the same cookie manager to OpenID Connect dashboard auth as well as browser-token auth.

This revives #16818, rebased onto the latest upstream/main, and addresses James's review comments by:

  • Renaming the cookie manager to AspireDashboardCookieManager because it is used by both browser-token auth and OpenID Connect auth.
  • Making the sign-out cookie deletion assertion use Assert.Collection so the expected two deleted cookies are explicit.
  • Adding coverage that OpenID Connect auth uses scheme-specific dashboard auth cookies.

User-facing usage

Dashboard users can expose both HTTPS and HTTP frontend endpoints and still authenticate consistently:

{
  "Dashboard:Frontend:Url": "https://127.0.0.1:0;http://127.0.0.1:0",
  "Dashboard:Frontend:AuthMode": "BrowserToken"
}

HTTPS requests continue to use .Aspire.Dashboard.Auth, while HTTP requests use .Aspire.Dashboard.Auth.Http.

Security considerations

This changes dashboard authentication cookie selection. The HTTPS cookie keeps the existing secure cookie name and behavior, while HTTP requests use a separate non-secure cookie name so browser scheme-specific cookie behavior cannot cause a stale HTTPS cookie to be used for an HTTP sign-in. No new listeners, secrets, or token formats are introduced.

Fixes #16067

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

Adam Ratzman (adamint) and others added 5 commits May 21, 2026 19:10
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>
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>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented May 21, 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 -- 17368

Or

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

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

This PR fixes dashboard authentication when the frontend is reachable over both HTTPS and HTTP by separating the auth cookie names per scheme (preventing an HTTPS Secure cookie from interfering with HTTP sign-in flows). It also ensures the same scheme-aware cookie behavior is used for both BrowserToken and OpenIdConnect modes.

Changes:

  • Introduces a scheme-aware cookie manager (AspireDashboardCookieManager) so HTTP requests use a distinct cookie name from HTTPS requests.
  • Updates dashboard authentication configuration and middleware ordering (Forwarded Headers now runs before token validation so scheme normalization is applied before cookies are issued).
  • Adds integration and Playwright coverage for mixed HTTPS/HTTP flows, cookie emission, and sign-out deletion behavior.

Reviewed changes

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

Show a summary per file
File Description
tests/Aspire.Dashboard.Tests/Integration/Playwright/BrowserTokenAuthenticationTests.cs Adds a mixed HTTPS→HTTP WebKit Playwright regression test and a new server fixture to host both schemes.
tests/Aspire.Dashboard.Tests/Integration/FrontendBrowserTokenAuthTests.cs Adds HTTP/HTTPS mixed-endpoint integration tests validating cookie naming and sign-out deleting both cookies.
tests/Aspire.Dashboard.Tests/DashboardOptionsTests.cs Adds a unit test ensuring OpenIdConnect mode uses the dashboard cookie manager and produces scheme-specific cookie names.
src/Aspire.Dashboard/DashboardWebApplication.cs Adds scheme-specific HTTP cookie name constant, configures the new cookie manager for cookie auth, and moves Forwarded Headers earlier in the pipeline.
src/Aspire.Dashboard/Authentication/AspireDashboardCookieManager.cs New cookie manager implementation that selects cookie name by request scheme and deletes both names on HTTPS sign-out.

Comment on lines +33 to +43
Configuration[DashboardConfigNames.DashboardFrontendUrlName.ConfigKey] = $"https://localhost:{GetAvailablePort()};http://localhost:{GetAvailablePort()}";
Configuration[DashboardConfigNames.DashboardFrontendAuthModeName.ConfigKey] = nameof(FrontendAuthMode.BrowserToken);
Configuration[DashboardConfigNames.DashboardFrontendBrowserTokenName.ConfigKey] = "VALID_TOKEN";
}

private static int GetAvailablePort()
{
using var listener = new TcpListener(IPAddress.Loopback, 0);
listener.Start();
return ((IPEndPoint)listener.LocalEndpoint).Port;
}

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.

Addressed in 09573ba by switching the mixed HTTP/HTTPS Playwright fixture to Kestrel dynamic ports (https://localhost:0;http://localhost:0) and removing the temporary TcpListener port preallocation.

Use Kestrel dynamic port binding for the mixed HTTP/HTTPS dashboard Playwright fixture so the test does not bind and release ports before the dashboard starts.

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

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.

Clean implementation. The cookie manager logic correctly separates HTTP/HTTPS cookies, the middleware reordering (ForwardedHeaders before ValidateTokenMiddleware) is the right fix, and the test coverage is thorough — including a direct test (Get_LoginPage_ValidToken_ForwardedHttps_UsesHttpsCookie) that validates the middleware ordering change would fail if reverted.

@adamint
Adam Ratzman (adamint) enabled auto-merge (squash) May 22, 2026 03:28
@adamint
Adam Ratzman (adamint) merged commit 5930b9d into microsoft:main May 22, 2026
604 of 609 checks passed
@github-actions github-actions Bot added this to the 13.4 milestone May 22, 2026
Ankit Jain (radical) added a commit that referenced this pull request Jun 16, 2026
* Fix NewWithAgentInit test: use CLI flags for skill selection instead of positional keystrokes

Co-authored-by: radical <1472+radical@users.noreply.github.com>

* test(dashboard): bind browser-token HTTP+HTTPS fixture to 127.0.0.1

The BrowserTokenDashboardServerWithHttpAndHttpsFixture bound the dashboard
frontend to "https://localhost:0;http://localhost:0". Kestrel rejects
dynamic-port (":0") binding on the "localhost" hostname:

  System.InvalidOperationException: Dynamic port binding is not supported
  when binding to localhost. You must either bind to 127.0.0.1:0 or
  [::1]:0, or both.

So the fixture threw in InitializeAsync and
BrowserToken_QueryStringToken_HttpsThenHttp_WebKit_Success failed every
scheduled Outerloop run. Bind to 127.0.0.1:0 instead, matching the base
DashboardServerFixture. The WebKit test navigates to the resolved address
with IgnoreHTTPSErrors, so the loopback IP is fine.

Broken since the fixture was introduced in #17368.

Refs #18223

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

* test(hosting): wait for proxyless EffectivePort before asserting

ProxylessContainerCanBeReferenced and WithEndpointProxySupportDisablesProxies
([OuterloopTest], Docker-required) failed every scheduled Outerloop run on the
Hosting (8-core-ubuntu-latest) job while passing on Windows:

    Assert.IsType() Failure: Value is null
    Expected: typeof(int)
    Actual:   null
      at ...AssertAllocatedProxylessPort(Service service) line 2319
      at ...ProxylessContainerCanBeReferenced() line 1835

Both assert on the redisNoPort proxyless service's Status.EffectivePort.
PR #17924 made Aspire pre-assign the proxyless host port synchronously
(Service.Spec.Port) and deliberately excluded proxyless services from the
startup address-wait in DcpExecutor, since connection strings use the
Aspire-assigned Spec.Port immediately. DCP still echoes the bound port back
asynchronously via Status.EffectivePort. On a fast Linux agent the test reads
the service list before DCP populates EffectivePort, so the assertion sees
null; on Windows the update lands first, so it passes. The unit-test fake sets
EffectivePort = Spec.Port synchronously, hiding the race.

Fix is test-side: wait for the proxyless service to report a non-null
EffectivePort (via KubernetesHelper.GetResourceByNameAsync, the existing
watch-based waiter) before asserting, instead of reading a once-fetched
service list. No product change - the orchestrator behavior is correct.

Fixes #8773

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

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: radical <1472+radical@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 21, 2026
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

3 participants