[PR #2861] [MERGED] fix: make state cookies valid when client uses multiple login URLs #2903

Closed
opened 2025-12-29 04:19:35 +01:00 by adam · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/2861
Author: @bobelev
Created: 11/4/2025
Status: Merged
Merged: 11/10/2025
Merged by: @kradalby

Base: mainHead: bblv/state-cookies


📝 Commits (1)

  • 01ff6ca fix: make state cookies valid when client uses multiple login URLs

📊 Changes

4 files changed (+283 additions, -22 deletions)

View changed files

📝 .github/workflows/test-integration.yaml (+1 -0)
📝 hscontrol/oidc.go (+14 -3)
📝 integration/auth_oidc_test.go (+113 -0)
📝 integration/scenario.go (+155 -19)

📄 Description

On Windows, if the user clicks the Tailscale icon in the system tray,
it opens a login URL in the browser.

When the login URL is opened, state/nonce cookies are set for that particular URL.

If the user clicks the icon again, a new login URL is opened in the browser,
and new cookies are set.

If the user proceeds with auth in the first tab,
the redirect results in a "state did not match" error.

This patch ensures that each opened login URL sets an individual cookie
that remains valid on the /oidc/callback page.

TestOIDCMultipleOpenedLoginUrls illustrates and tests this behavior.

  • have read the CONTRIBUTING.md file
  • raised a GitHub issue or discussed it on the projects chat beforehand
  • added unit tests
  • added integration tests
  • updated documentation if needed
  • updated CHANGELOG.md

I've added some helpers in scenario test suite to perform requests with the same http.Client (to preserve cookies and see their values in tests).

This patch is battle-tested in my environment. Completely got rid of "state" errors for windows/mobile clients.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/juanfont/headscale/pull/2861 **Author:** [@bobelev](https://github.com/bobelev) **Created:** 11/4/2025 **Status:** ✅ Merged **Merged:** 11/10/2025 **Merged by:** [@kradalby](https://github.com/kradalby) **Base:** `main` ← **Head:** `bblv/state-cookies` --- ### 📝 Commits (1) - [`01ff6ca`](https://github.com/juanfont/headscale/commit/01ff6cae883fe8ff523cd9077f9aeb0d7ffc10d0) fix: make state cookies valid when client uses multiple login URLs ### 📊 Changes **4 files changed** (+283 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/test-integration.yaml` (+1 -0) 📝 `hscontrol/oidc.go` (+14 -3) 📝 `integration/auth_oidc_test.go` (+113 -0) 📝 `integration/scenario.go` (+155 -19) </details> ### 📄 Description On Windows, if the user clicks the Tailscale icon in the system tray, it opens a login URL in the browser. When the login URL is opened, `state/nonce` cookies are set for that particular URL. If the user clicks the icon again, a new login URL is opened in the browser, and new cookies are set. If the user proceeds with auth in the first tab, the redirect results in a "state did not match" error. This patch ensures that each opened login URL sets an individual cookie that remains valid on the `/oidc/callback` page. `TestOIDCMultipleOpenedLoginUrls` illustrates and tests this behavior. - [x] have read the [CONTRIBUTING.md](./CONTRIBUTING.md) file - [ ] raised a GitHub issue or discussed it on the projects chat beforehand - [ ] added unit tests - [x] added integration tests - [ ] updated documentation if needed - [ ] updated CHANGELOG.md I've added some helpers in scenario test suite to perform requests with the same `http.Client` (to preserve cookies and see their values in tests). This patch is battle-tested in my environment. Completely got rid of "state" errors for windows/mobile clients. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2025-12-29 04:19:35 +01:00
adam closed this issue 2025-12-29 04:19:36 +01:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#2903