state: allow untagging nodes via reauth with empty RequestTags

When a node re-authenticates via OIDC/web auth with empty RequestTags (from `tailscale up --advertise-tags= --force-reauth`), remove all tags and return ownership to the authenticating user. This allows nodes to transition from any tagged state (including nodes originally registered with a tagged pre-auth key) back to user-owned. Fixes #2979
2026-01-18 15:06:56 +01:00 · 2026-01-14 12:22:02 +00:00
parent a6696582a4
commit 0451dd4718
3 changed files with 302 additions and 4 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,7 +12,8 @@ tags rather than users, making them suitable for servers and infrastructure. App
 ownership. See the [Tailscale tags documentation](https://tailscale.com/kb/1068/tags) for details on how tags work.

 User-owned nodes can now request tags during registration using `--advertise-tags`. Tags are validated against the `tagOwners` policy
-and applied at registration time. Tags can be managed via the CLI or API after registration.
+and applied at registration time. Tags can be managed via the CLI or API after registration. Tagged nodes can return to user-owned
+by re-authenticating with `tailscale up --advertise-tags= --force-reauth`.

 ### Smarter map updates

@@ -41,7 +42,7 @@ sequentially through each stable release, selecting the latest patch version ava
 - **API**: The Node message in the gRPC/REST API has been simplified - the `ForcedTags`, `InvalidTags`, and `ValidTags` fields have been removed and replaced with a single `Tags` field that contains the node's applied tags [#2993](https://github.com/juanfont/headscale/pull/2993)
  - API clients should use the `Tags` field instead of `ValidTags`
  - The `headscale nodes list` CLI command now always shows a Tags column and the `--tags` flag has been removed
- **Tags**: The gRPC `SetTags` endpoint now allows converting user-owned nodes to tagged nodes by setting tags. Once a node is tagged, it cannot be converted back to a user-owned node. [#2885](https://github.com/juanfont/headscale/pull/2885)
+- **Tags**: The gRPC `SetTags` endpoint now allows converting user-owned nodes to tagged nodes by setting tags. [#2885](https://github.com/juanfont/headscale/pull/2885)
 - **Tags**: Tags are now resolved from the node's stored Tags field only [#2931](https://github.com/juanfont/headscale/pull/2931)
  - `--advertise-tags` is processed during registration, not on every policy evaluation
  - PreAuthKey tagged devices ignore `--advertise-tags` from clients
--- a/hscontrol/auth_test.go
+++ b/hscontrol/auth_test.go
@@ -3541,3 +3541,187 @@ func TestWebAuthRejectsUnauthorizedRequestTags(t *testing.T) {
 	_, found := app.state.GetNodeByNodeKey(nodeKey.Public())
 	require.False(t, found, "Node should not be created when tags are unauthorized")
 }
+
+// TestWebAuthReauthWithEmptyTagsRemovesAllTags tests that when an existing tagged node
+// reauths with empty RequestTags, all tags are removed and ownership returns to user.
+// This is the fix for issue #2979.
+func TestWebAuthReauthWithEmptyTagsRemovesAllTags(t *testing.T) {
+	t.Parallel()
+
+	app := createTestApp(t)
+
+	// Create a user
+	user := app.state.CreateUserForTest("reauth-untag-user")
+
+	// Update policy manager to recognize the new user
+	// This is necessary because CreateUserForTest doesn't update the policy manager
+	err := app.state.UpdatePolicyManagerUsersForTest()
+	require.NoError(t, err, "Failed to update policy manager users")
+
+	// Set up policy that allows the user to own these tags
+	policy := `{
+		"tagOwners": {
+			"tag:valid-owned": ["reauth-untag-user@"],
+			"tag:second": ["reauth-untag-user@"]
+		},
+		"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}]
+	}`
+	_, err = app.state.SetPolicy([]byte(policy))
+	require.NoError(t, err, "Failed to set policy")
+
+	machineKey := key.NewMachine()
+	nodeKey1 := key.NewNode()
+
+	// Step 1: Initial registration with tags
+	registrationID1 := types.MustRegistrationID()
+	regEntry1 := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(),
+		NodeKey:    nodeKey1.Public(),
+		Hostname:   "reauth-untag-node",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "reauth-untag-node",
+			RequestTags: []string{"tag:valid-owned", "tag:second"},
+		},
+	})
+	app.state.SetRegistrationCacheEntry(registrationID1, regEntry1)
+
+	// Complete initial registration with tags
+	node, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID1,
+		types.UserID(user.ID),
+		nil,
+		"webauth",
+	)
+	require.NoError(t, err, "Initial registration should succeed")
+	require.True(t, node.IsTagged(), "Node should be tagged after initial registration")
+	require.ElementsMatch(t, []string{"tag:valid-owned", "tag:second"}, node.Tags().AsSlice())
+	t.Logf("Initial registration complete - Node ID: %d, Tags: %v, IsTagged: %t",
+		node.ID().Uint64(), node.Tags().AsSlice(), node.IsTagged())
+
+	// Step 2: Reauth with EMPTY tags to untag
+	nodeKey2 := key.NewNode() // New node key for reauth
+	registrationID2 := types.MustRegistrationID()
+	regEntry2 := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(), // Same machine key
+		NodeKey:    nodeKey2.Public(),   // Different node key (rotation)
+		Hostname:   "reauth-untag-node",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "reauth-untag-node",
+			RequestTags: []string{}, // EMPTY - should untag
+		},
+	})
+	app.state.SetRegistrationCacheEntry(registrationID2, regEntry2)
+
+	// Complete reauth with empty tags
+	nodeAfterReauth, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID2,
+		types.UserID(user.ID),
+		nil,
+		"webauth",
+	)
+	require.NoError(t, err, "Reauth should succeed")
+
+	// Verify tags were removed
+	require.False(t, nodeAfterReauth.IsTagged(), "Node should NOT be tagged after reauth with empty tags")
+	require.Empty(t, nodeAfterReauth.Tags().AsSlice(), "Node should have no tags")
+
+	// Verify ownership returned to user
+	require.True(t, nodeAfterReauth.UserID().Valid(), "Node should have a user ID")
+	require.Equal(t, user.ID, nodeAfterReauth.UserID().Get(), "Node should be owned by the user again")
+
+	// Verify it's the same node (not a new one)
+	require.Equal(t, node.ID(), nodeAfterReauth.ID(), "Should be the same node after reauth")
+
+	t.Logf("Reauth complete - Node ID: %d, Tags: %v, IsTagged: %t, UserID: %d",
+		nodeAfterReauth.ID().Uint64(), nodeAfterReauth.Tags().AsSlice(),
+		nodeAfterReauth.IsTagged(), nodeAfterReauth.UserID().Get())
+}
+
+// TestAuthKeyTaggedToUserOwnedViaReauth tests that a node originally registered
+// with a tagged pre-auth key can transition to user-owned by re-authenticating
+// via web auth with empty RequestTags. This ensures authkey-tagged nodes are
+// not permanently locked to being tagged.
+func TestAuthKeyTaggedToUserOwnedViaReauth(t *testing.T) {
+	t.Parallel()
+
+	app := createTestApp(t)
+
+	// Create a user
+	user := app.state.CreateUserForTest("authkey-to-user")
+
+	// Create a tagged pre-auth key
+	authKeyTags := []string{"tag:server", "tag:prod"}
+	pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, authKeyTags)
+	require.NoError(t, err, "Failed to create tagged pre-auth key")
+
+	machineKey := key.NewMachine()
+	nodeKey1 := key.NewNode()
+
+	// Step 1: Initial registration with tagged pre-auth key
+	regReq := tailcfg.RegisterRequest{
+		Auth: &tailcfg.RegisterResponseAuth{
+			AuthKey: pak.Key,
+		},
+		NodeKey: nodeKey1.Public(),
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname: "authkey-tagged-node",
+		},
+		Expiry: time.Now().Add(24 * time.Hour),
+	}
+
+	resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err, "Initial registration should succeed")
+	require.True(t, resp.MachineAuthorized, "Node should be authorized")
+
+	// Verify initial state: node is tagged via authkey
+	node, found := app.state.GetNodeByNodeKey(nodeKey1.Public())
+	require.True(t, found, "Node should be found")
+	require.True(t, node.IsTagged(), "Node should be tagged after authkey registration")
+	require.ElementsMatch(t, authKeyTags, node.Tags().AsSlice(), "Node should have authkey tags")
+	require.NotNil(t, node.AuthKey(), "Node should have AuthKey reference")
+	require.Positive(t, node.AuthKey().Tags().Len(), "AuthKey should have tags")
+
+	t.Logf("Initial registration complete - Node ID: %d, Tags: %v, IsTagged: %t, AuthKey.Tags.Len: %d",
+		node.ID().Uint64(), node.Tags().AsSlice(), node.IsTagged(), node.AuthKey().Tags().Len())
+
+	// Step 2: Reauth via web auth with EMPTY tags to transition to user-owned
+	nodeKey2 := key.NewNode() // New node key for reauth
+	registrationID := types.MustRegistrationID()
+	regEntry := types.NewRegisterNode(types.Node{
+		MachineKey: machineKey.Public(), // Same machine key
+		NodeKey:    nodeKey2.Public(),   // Different node key (rotation)
+		Hostname:   "authkey-tagged-node",
+		Hostinfo: &tailcfg.Hostinfo{
+			Hostname:    "authkey-tagged-node",
+			RequestTags: []string{}, // EMPTY - should untag
+		},
+	})
+	app.state.SetRegistrationCacheEntry(registrationID, regEntry)
+
+	// Complete reauth with empty tags
+	nodeAfterReauth, _, err := app.state.HandleNodeFromAuthPath(
+		registrationID,
+		types.UserID(user.ID),
+		nil,
+		"webauth",
+	)
+	require.NoError(t, err, "Reauth should succeed")
+
+	// Verify tags were removed (authkey-tagged → user-owned transition)
+	require.False(t, nodeAfterReauth.IsTagged(), "Node should NOT be tagged after reauth with empty tags")
+	require.Empty(t, nodeAfterReauth.Tags().AsSlice(), "Node should have no tags")
+
+	// Verify ownership returned to user
+	require.True(t, nodeAfterReauth.UserID().Valid(), "Node should have a user ID")
+	require.Equal(t, user.ID, nodeAfterReauth.UserID().Get(), "Node should be owned by the user")
+
+	// Verify it's the same node (not a new one)
+	require.Equal(t, node.ID(), nodeAfterReauth.ID(), "Should be the same node after reauth")
+
+	// AuthKey reference should still exist (for audit purposes)
+	require.NotNil(t, nodeAfterReauth.AuthKey(), "AuthKey reference should be preserved")
+
+	t.Logf("Reauth complete - Node ID: %d, Tags: %v, IsTagged: %t, UserID: %d",
+		nodeAfterReauth.ID().Uint64(), nodeAfterReauth.Tags().AsSlice(),
+		nodeAfterReauth.IsTagged(), nodeAfterReauth.UserID().Get())
+}
--- a/hscontrol/state/state.go
+++ b/hscontrol/state/state.go
@@ -1246,6 +1246,93 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 	return s.nodeStore.PutNode(*savedNode), nil
 }

+// processReauthTags handles tag changes during node re-authentication.
+// It processes RequestTags from the client and updates node tags accordingly.
+// Returns rejected tags (if any) for post-validation error handling.
+func (s *State) processReauthTags(
+	node *types.Node,
+	requestTags []string,
+	user *types.User,
+	oldTags []string,
+) []string {
+	wasAuthKeyTagged := node.AuthKey != nil && node.AuthKey.IsTagged()
+
+	logEvent := log.Debug().
+		Uint64("node.id", uint64(node.ID)).
+		Str("node.name", node.Hostname).
+		Strs("request.tags", requestTags).
+		Strs("current.tags", node.Tags).
+		Bool("is.tagged", node.IsTagged()).
+		Bool("was.authkey.tagged", wasAuthKeyTagged)
+	logEvent.Msg("Processing RequestTags during reauth")
+
+	// Empty RequestTags means untag node (transition to user-owned)
+	if len(requestTags) == 0 {
+		if node.IsTagged() {
+			log.Info().
+				Uint64("node.id", uint64(node.ID)).
+				Str("node.name", node.Hostname).
+				Strs("removed.tags", node.Tags).
+				Str("user.name", user.Name).
+				Bool("was.authkey.tagged", wasAuthKeyTagged).
+				Msg("Reauth: removing all tags, returning node ownership to user")
+
+			node.Tags = []string{}
+			node.UserID = &user.ID
+		}
+
+		return nil
+	}
+
+	// Non-empty RequestTags: validate and apply
+	var approvedTags, rejectedTags []string
+
+	for _, tag := range requestTags {
+		if s.polMan.NodeCanHaveTag(node.View(), tag) {
+			approvedTags = append(approvedTags, tag)
+		} else {
+			rejectedTags = append(rejectedTags, tag)
+		}
+	}
+
+	if len(rejectedTags) > 0 {
+		log.Warn().
+			Uint64("node.id", uint64(node.ID)).
+			Str("node.name", node.Hostname).
+			Strs("rejected.tags", rejectedTags).
+			Msg("Reauth: requested tags are not permitted")
+
+		return rejectedTags
+	}
+
+	if len(approvedTags) > 0 {
+		slices.Sort(approvedTags)
+		approvedTags = slices.Compact(approvedTags)
+
+		wasTagged := node.IsTagged()
+		node.Tags = approvedTags
+
+		// Note: UserID is preserved as "created by" tracking, consistent with SetNodeTags
+		if !wasTagged {
+			log.Info().
+				Uint64("node.id", uint64(node.ID)).
+				Str("node.name", node.Hostname).
+				Strs("new.tags", approvedTags).
+				Str("old.user", user.Name).
+				Msg("Reauth: applying tags, transferring node to tagged-devices")
+		} else {
+			log.Info().
+				Uint64("node.id", uint64(node.ID)).
+				Str("node.name", node.Hostname).
+				Strs("old.tags", oldTags).
+				Strs("new.tags", approvedTags).
+				Msg("Reauth: updating tags on already-tagged node")
+		}
+	}
+
+	return nil
+}
+
 // HandleNodeFromAuthPath handles node registration through authentication flow (like OIDC).
 func (s *State) HandleNodeFromAuthPath(
 	registrationID types.RegistrationID,
@@ -1291,14 +1378,26 @@ func (s *State) HandleNodeFromAuthPath(

 	// If this node exists for this user, update the node in place.
 	if existsSameUser && existingNodeSameUser.Valid() {
-		log.Debug().
+		log.Info().
 			Caller().
 			Str("registration_id", registrationID.String()).
 			Str("user.name", user.Name).
 			Str("registrationMethod", registrationMethod).
 			Str("node.name", existingNodeSameUser.Hostname()).
 			Uint64("node.id", existingNodeSameUser.ID().Uint64()).
-			Msg("Updating existing node registration")
+			Interface("hostinfo", regEntry.Node.Hostinfo).
+			Msg("Updating existing node registration via reauth")
+
+		// Process RequestTags during reauth (#2979)
+		// Due to json:",omitempty", we treat empty/nil as "clear tags"
+		var requestTags []string
+		if regEntry.Node.Hostinfo != nil {
+			requestTags = regEntry.Node.Hostinfo.RequestTags
+		}
+
+		oldTags := existingNodeSameUser.Tags().AsSlice()
+
+		var rejectedTags []string

 		// Update existing node - NodeStore first, then database
 		updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
@@ -1327,12 +1426,18 @@ func (s *State) HandleNodeFromAuthPath(
 					node.Expiry = regEntry.Node.Expiry
 				}
 			}
+
+			rejectedTags = s.processReauthTags(node, requestTags, user, oldTags)
 		})

 		if !ok {
 			return types.NodeView{}, change.Change{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, existingNodeSameUser.ID())
 		}

+		if len(rejectedTags) > 0 {
+			return types.NodeView{}, change.Change{}, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, rejectedTags)
+		}
+
 		_, err = hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
 			// Use Updates() to preserve fields not modified by UpdateNode.
 			err := tx.Updates(updatedNodeView.AsStruct()).Error
@@ -1686,6 +1791,14 @@ func (s *State) updatePolicyManagerUsers() (change.Change, error) {
 	return change.Change{}, nil
 }

+// UpdatePolicyManagerUsersForTest updates the policy manager's user cache.
+// This is exposed for testing purposes to sync the policy manager after
+// creating test users via CreateUserForTest().
+func (s *State) UpdatePolicyManagerUsersForTest() error {
+	_, err := s.updatePolicyManagerUsers()
+	return err
+}
+
 // updatePolicyManagerNodes updates the policy manager with current nodes.
 // Returns true if the policy changed and notifications should be sent.
 // TODO(kradalby): This is a temporary stepping stone, ultimately we should