starred/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-01-11 20:00:28 +01:00
Code Issues 98 Packages Projects Releases 10 Wiki Activity

[PR #2374] [MERGED] Rewrite authentication flow #2633

New Issue
Closed
opened 2025-12-29 03:22:07 +01:00 by adam · 0 comments
adam commented 2025-12-29 03:22:07 +01:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/juanfont/headscale/pull/2374
Author: @kradalby
Created: 1/23/2025
Status: ✅ Merged
Merged: 2/1/2025
Merged by: @kradalby

Base: main ← Head: kradalby/no-more-any-key

📝 Commits (10+)

  • bdea26f Remove AnyKey for NodeKey, missing OldNodeKey handling
  • a859f93 Rewrite and simplify registration logic
  • 2691c40 count nodes after each step in relogin tests
  • 745cd17 update changelog
  • 0607307 simplify assert
  • 76a188a auth key test for relogin with diff user
  • a39066b add TestOIDCReloginSameNode
  • d322f8d mv github integ test gen to workflow dir
  • 3783a6e update list node helper, add listuser
  • ddff6b9 fix outdated tests

📊 Changes

20 files changed (+834 additions, -982 deletions)

View changed files

📝 .github/workflows/check-tests.yaml (+1 -1)
📝 .github/workflows/gh-action-integration-generator.go (+8 -4)
📝 .github/workflows/test-integration.yaml (+3 -1)
📝 CHANGELOG.md (+12 -0)
📝 hscontrol/app.go (+9 -6)
📝 hscontrol/auth.go (+164 -652)
📝 hscontrol/db/node.go (+12 -32)
📝 hscontrol/db/node_test.go (+0 -31)
📝 hscontrol/grpcv1.go (+8 -1)
📝 hscontrol/noise.go (+48 -15)
📝 hscontrol/oidc.go (+8 -11)
📝 hscontrol/types/common.go (+14 -0)
➕ integration/auth_key_test.go (+230 -0)
📝 integration/auth_oidc_test.go (+236 -51)
📝 integration/auth_web_flow_test.go (+14 -6)
📝 integration/cli_test.go (+4 -14)
📝 integration/control.go (+2 -1)
📝 integration/general_test.go (+4 -147)
📝 integration/hsic/config.go (+1 -1)
📝 integration/hsic/hsic.go (+56 -8)

📄 Description

This PR takes a look at the authentication flow with fresh eyes, and it more or less deletes all of it and simplifies it.
I think we made a lot of "guesses" for what needs to happen in the early they so there was a lot of unnecessary conditional paths and code that was really hard to follow.

This code rearranges it so it is a relatively simple "directed graph" so it should be easier to follow.

The motivation for this was inconsistent handling and bugs causing #2327 and #1310, as well as different behaviour logging in and out with different auth methods.

Fixes #2327
Fixes #1310

Probably also:
Fixes #1522

🔄 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/2374 **Author:** [@kradalby](https://github.com/kradalby) **Created:** 1/23/2025 **Status:** ✅ Merged **Merged:** 2/1/2025 **Merged by:** [@kradalby](https://github.com/kradalby) **Base:** `main` ← **Head:** `kradalby/no-more-any-key` --- ### 📝 Commits (10+) - [`bdea26f`](https://github.com/juanfont/headscale/commit/bdea26f1419d40bada3022228cd4bd990d5bf783) Remove AnyKey for NodeKey, missing OldNodeKey handling - [`a859f93`](https://github.com/juanfont/headscale/commit/a859f93daa66f85b51e2cd976e4eb112efe112a9) Rewrite and simplify registration logic - [`2691c40`](https://github.com/juanfont/headscale/commit/2691c406305b03b32118c5008f1ea5798e72f214) count nodes after each step in relogin tests - [`745cd17`](https://github.com/juanfont/headscale/commit/745cd176730bef9f215ee2306a143db730ec4716) update changelog - [`0607307`](https://github.com/juanfont/headscale/commit/06073072147f3ce8f451d909cacab0ab9cfa1ab8) simplify assert - [`76a188a`](https://github.com/juanfont/headscale/commit/76a188ab6b6281570ceddbede9ae9f9a0cdbfcc9) auth key test for relogin with diff user - [`a39066b`](https://github.com/juanfont/headscale/commit/a39066bd374139f6a113abd1f896da6b185e751a) add TestOIDCReloginSameNode - [`d322f8d`](https://github.com/juanfont/headscale/commit/d322f8d7a85198b5ff033a40587f06d4d96de983) mv github integ test gen to workflow dir - [`3783a6e`](https://github.com/juanfont/headscale/commit/3783a6e1de3cc34e6c92c155982b782915f85d96) update list node helper, add listuser - [`ddff6b9`](https://github.com/juanfont/headscale/commit/ddff6b95bb65e4f0c03a677d106c5ab24b79bfd9) fix outdated tests ### 📊 Changes **20 files changed** (+834 additions, -982 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/check-tests.yaml` (+1 -1) 📝 `.github/workflows/gh-action-integration-generator.go` (+8 -4) 📝 `.github/workflows/test-integration.yaml` (+3 -1) 📝 `CHANGELOG.md` (+12 -0) 📝 `hscontrol/app.go` (+9 -6) 📝 `hscontrol/auth.go` (+164 -652) 📝 `hscontrol/db/node.go` (+12 -32) 📝 `hscontrol/db/node_test.go` (+0 -31) 📝 `hscontrol/grpcv1.go` (+8 -1) 📝 `hscontrol/noise.go` (+48 -15) 📝 `hscontrol/oidc.go` (+8 -11) 📝 `hscontrol/types/common.go` (+14 -0) ➕ `integration/auth_key_test.go` (+230 -0) 📝 `integration/auth_oidc_test.go` (+236 -51) 📝 `integration/auth_web_flow_test.go` (+14 -6) 📝 `integration/cli_test.go` (+4 -14) 📝 `integration/control.go` (+2 -1) 📝 `integration/general_test.go` (+4 -147) 📝 `integration/hsic/config.go` (+1 -1) 📝 `integration/hsic/hsic.go` (+56 -8) </details> ### 📄 Description This PR takes a look at the authentication flow with fresh eyes, and it more or less deletes all of it and simplifies it. I think we made a lot of "guesses" for what needs to happen in the early they so there was a lot of unnecessary conditional paths and code that was really hard to follow. This code rearranges it so it is a relatively simple "directed graph" so it should be easier to follow. The motivation for this was inconsistent handling and bugs causing #2327 and #1310, as well as different behaviour logging in and out with different auth methods. Fixes #2327 Fixes #1310 Probably also: Fixes #1522 --- <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 03:22:07 +01:00
adam closed this issue 2025-12-29 03:22:08 +01:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
CLI
DERP
DNS
Nix
OIDC
SSH
bug
database
documentation
duplicate
enhancement
faq
good first issue
grants
help wanted
might-come
needs design doc
needs investigation
no-stale-bot
out of scope
performance
policy 📝
pull-request
Mirrored from GitHub Pull Request
 question
regression
routes
stale
tags
tailscale-feature-gap
well described ❤️
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/headscale#2633
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?