fix(auth): honor ephemeral registration for interactive auth#3078
fix(auth): honor ephemeral registration for interactive auth#3078LouisLau-art wants to merge 1 commit into
Conversation
| // Only SQLite will be migrated for consistency. | ||
| { | ||
| ID: "202507021200", | ||
| Migrate: func(tx *gorm.DB) error { |
There was a problem hiding this comment.
This changes a previous migration, which we cannot do, we can only add migrations.
kradalby
left a comment
kradalby
left a comment
There was a problem hiding this comment.
Looks pretty good, some comments and things needed.
If you are assisted by an AI, remember to declare the agent and model.
| Rollback: func(db *gorm.DB) error { return nil }, | ||
| }, | ||
| { | ||
| ID: "202602111000-node-ephemeral-column", |
There was a problem hiding this comment.
I think this is the right approach, the ephemeral field should be on the node and not the auth key. This is essentially the same change as we did for tags in the previous release.
At authentication, the ephemeral field should be set on the node, and from there on, the pre auth key should always be ignored.
| err := rx. | ||
| Joins("LEFT JOIN pre_auth_keys pak ON pak.id = nodes.auth_key_id"). | ||
| Where("nodes.ephemeral = ? OR pak.ephemeral = ?", true, true). | ||
| Find(&nodes).Error |
There was a problem hiding this comment.
This change should be reverted and the authkey part should be removed. We should only be looking at the ephemeral field on the node, so no join should be necessary.
|
|
||
| nodeToRegister.AuthKey = params.PreAuthKey | ||
| nodeToRegister.AuthKeyID = ¶ms.PreAuthKey.ID | ||
| nodeToRegister.Ephemeral = params.PreAuthKey.Ephemeral |
There was a problem hiding this comment.
This should be the last place PreAuthKey.Ephemeral is used, and after that, the Node field is the "authority".
| // https://tailscale.com/kb/1111/ephemeral-nodes/ | ||
| func (node *Node) IsEphemeral() bool { | ||
| return node.AuthKey != nil && node.AuthKey.Ephemeral | ||
| return node.Ephemeral || (node.AuthKey != nil && node.AuthKey.Ephemeral) |
There was a problem hiding this comment.
This can drop || (node.AuthKey != nil && node.AuthKey.Ephemeral) as it is only used at auth time.
| expectedAuthURLPattern: "/register/", | ||
| }, | ||
| { | ||
| name: "full_interactive_workflow_ephemeral_node", |
There was a problem hiding this comment.
We need an integration test for the webauth and oidc auth to use --state=mem: to validate the behaviour.
2 participants
Summary
Fixes
tailscaled --state=mem:registrations that go through interactive auth (OIDC/web), so nodes are persisted as ephemeral and handled consistently.Closes #2719.
What changed
nodes.ephemeralas a node-level persisted flag.RegisterRequest.Ephemeralinto interactive registration cache entries.HandleNodeFromAuthPath) into created/updated nodes.PreAuthKey.Ephemeralintonodes.ephemeral.IsEphemeral()checks.nodes.ephemeralfrom existing ephemeral pre-auth keys.Ephemeral: truenow asserts node is ephemeral;nodes.ephemeral = trueand no pre-auth key.Verification
go test ./hscontrol -run TestAuthenticationFlows -count=1go test ./hscontrol/db -run TestListEphemeralNodes -count=1go test ./hscontrol/state -run TestEphemeralNodeLogoutRaceCondition -count=1go test ./hscontrol/db -run 'TestSQLiteMigrationAndDataValidation|TestSQLiteAllTestdataMigrations' -count=1go test ./hscontrol/types -run TestNodeRegisterMethodToV1Enum -count=1