policy/v2: resolve user references consistently across updates

Prefer email owners over usernames and validate grants and nodeAttrs on
write. Reject ambiguous writes; log and match nobody on reload. Bare @
matches nobody.

docs/ref/oidc.md

@@ -185,6 +185,11 @@ You may refer to users in the Headscale policy via:
 - Provider identifier (this value is currently only available from the [API](api.md), database or directly from your
   identity provider)
 
+References match a provider identifier first, then a stored email address, then a username. A token shaped like an
+email address also matches a username with that value when no user has that email. If one user has the email and
+another has the username, the email owner wins. Multiple matching emails, or multiple matching usernames when no email
+matches, remain ambiguous.
+
 !!! note "A user identifier in the policy must contain a single `@`"
 
     The Headscale policy requires a single `@` to reference a user. If the username or provider identifier doesn't
@@ -200,6 +205,8 @@ You may refer to users in the Headscale policy via:
     consequences for Headscale where a policy might no longer work or a user might obtain more access by hijacking an
     existing username or email address.
 
+    Use provider identifiers in policies when your identity provider lets users change their own usernames.
+
 !!! tip "Howto use the provider identifier in the policy"
 
     The provider identifier uniquely identifies an OIDC user and a well-behaving identity provider guarantees that this

hscontrol/policy/v2/compiled_test.go

@@ -320,14 +320,15 @@ func TestCompileOtherDestsMatchesReference(t *testing.T) {
 					hp("undefined"),
 				}
 			case "partial-group":
+				// An ambiguous member matches no one; the others stay.
 				users = append(users, types.User{ID: 100, Name: users[0].Name})
 				ips, err := grant.Destinations[0].Resolve(pol, users, nodes.ViewSlice())
-				require.Error(t, err)
+				require.NoError(t, err)
 				require.NotNil(t, ips)
 				require.False(
 					t,
 					ips.Empty(),
-					"exercise a partial result alongside the error",
+					"exercise a partial result",
 				)
 			}
 

hscontrol/policy/v2/policy.go

@@ -135,8 +135,7 @@ type filterAndPolicy struct {
 	Policy *Policy
 }
 
-// validateUserReferences surfaces ambiguous user@ tokens at policy load so
-// duplicate DB rows fail loudly instead of silently dropping rules.
+// validateUserReferences reports ambiguous user@ tokens so writes can refuse them.
 // Missing-user tokens stay tolerant. Empty users → no-op for
 // syntax-only checks.
 func validateUserReferences(pol *Policy, users types.Users) error {
@@ -207,6 +206,22 @@ func validateUserReferences(pol *Policy, users types.Users) error {
 		}
 	}
 
+	for _, grant := range pol.Grants {
+		for _, src := range grant.Sources {
+			checkAlias(src)
+		}
+
+		for _, dst := range grant.Destinations {
+			checkAlias(dst)
+		}
+	}
+
+	for _, attr := range pol.NodeAttrs {
+		for _, target := range attr.Targets {
+			checkAlias(target)
+		}
+	}
+
 	for _, ssh := range pol.SSHs {
 		for _, src := range ssh.Sources {
 			checkAlias(src)
@@ -231,7 +246,7 @@ func NewPolicyManager(b []byte, users []types.User, nodes views.Slice[types.Node
 
 	err = validateUserReferences(policy, users)
 	if err != nil {
-		return nil, fmt.Errorf("validating policy user references: %w", err)
+		log.Error().Err(err).Msg("ambiguous policy users at boot")
 	}
 
 	pm := PolicyManager{
@@ -1106,6 +1121,11 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, bool, error) {
 		return false, false, err
 	}
 
+	err = validateUserReferences(pm.pol, users)
+	if err != nil {
+		log.Error().Err(err).Msg("ambiguous policy users after updating users")
+	}
+
 	// SSH rules and per-node filters embed user identity outside the filter
 	// hash, so a user change needs a client refresh even when it did not move.
 	if hadPerNodeFilter || pm.needsPerNodeFilter ||

hscontrol/policy/v2/policy_test.go

@@ -96,44 +96,6 @@ func TestPolicyManager(t *testing.T) {
 	}
 }
 
-// TestSetPolicyRejectedKeepsLiveFilter pins that a policy SetPolicy rejects
-// does not take effect. A nodeAttrs target naming an ambiguous user passes
-// validation but fails to compile after the filter is already compiled; keeping that
-// filter would run the rejected policy while the stored one is unchanged.
-func TestSetPolicyRejectedKeepsLiveFilter(t *testing.T) {
-	// IDs are assigned after construction so the test also builds where
-	// types.User embeds gorm.Model.
-	users := types.Users{{Name: "user1"}, {Name: "user2"}, {Name: "dup"}, {Name: "dup"}}
-	users[0].ID, users[1].ID, users[2].ID, users[3].ID = 1, 2, 3, 4
-	nodes := types.Nodes{
-		node("n1", "100.64.0.1", "fd7a:115c:a1e0::1", users[0]),
-		node("n2", "100.64.0.2", "fd7a:115c:a1e0::2", users[1]),
-	}
-	nodes[0].ID, nodes[1].ID = 1, 2
-
-	pm, err := NewPolicyManager([]byte(`{
-		"acls": [{"action": "accept", "src": ["user1@"], "dst": ["user1@:*"]}]
-	}`), users, nodes.ViewSlice())
-	require.NoError(t, err)
-
-	before, _ := pm.Filter()
-	beforeRules, err := pm.FilterForNode(nodes[1].View())
-	require.NoError(t, err)
-
-	_, err = pm.SetPolicy([]byte(`{
-		"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}],
-		"nodeAttrs": [{"target": ["dup@"], "attr": ["randomize-client-port"]}]
-	}`))
-	require.Error(t, err)
-
-	after, _ := pm.Filter()
-	require.Equal(t, before, after, "a rejected policy must not replace the live filter")
-
-	afterRules, err := pm.FilterForNode(nodes[1].View())
-	require.NoError(t, err)
-	require.Equal(t, beforeRules, afterRules)
-}
-
 func TestInvalidateAutogroupSelfCache(t *testing.T) {
 	users := types.Users{
 		{ID: 1, Name: "user1", Email: "user1@headscale.net"},
@@ -2900,14 +2862,16 @@ func TestNewPolicyManager_DuplicateUsername(t *testing.T) {
   ]
 }`)
 
-	_, err := NewPolicyManager(polB, users, types.Nodes{}.ViewSlice())
-	require.Error(t, err, "NewPolicyManager must reject policy with ambiguous username")
+	pm, err := NewPolicyManager(polB, users, types.Nodes{}.ViewSlice())
+	require.NoError(t, err, "ambiguous usernames must not block loading a stored policy")
+
+	_, err = pm.SetPolicy(polB)
 	require.ErrorIs(t, err, ErrMultipleUsersFound)
 	require.Contains(t, err.Error(), "yala@",
 		"error must name the offending token")
 }
 
-// Missing-user tokens stay tolerant per #2863; only multi-match blocks load.
+// Missing-user tokens stay tolerant per #2863.
 func TestNewPolicyManager_UnknownUsernameTolerant(t *testing.T) {
 	users := types.Users{
 		{ID: 1, Name: "alice"},
@@ -3211,6 +3175,8 @@ func TestValidateUserReferences_AllSites(t *testing.T) {
 		{ID: 2, Name: "dup"},
 		{ID: 3, Name: "dup"},
 	}
+	nodes := types.Nodes{node("alice", "100.64.0.1", "fd7a:115c:a1e0::1", users[0])}
+	nodes[0].ID = 1
 
 	tests := []struct {
 		name string
@@ -3254,6 +3220,18 @@ func TestValidateUserReferences_AllSites(t *testing.T) {
 			name: "acls.dst",
 			pol: `{
   "acls": [{"action":"accept","src":["alice@"],"dst":["dup@:*"]}]
+}`,
+		},
+		{
+			name: "grants.src",
+			pol: `{
+  "grants": [{"src":["dup@"],"dst":["*"],"ip":["*"]}]
+}`,
+		},
+		{
+			name: "grants.dst",
+			pol: `{
+  "grants": [{"src":["alice@"],"dst":["dup@"],"ip":["*"]}]
 }`,
 		},
 		{
@@ -3283,9 +3261,25 @@ func TestValidateUserReferences_AllSites(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := NewPolicyManager([]byte(tt.pol), users, types.Nodes{}.ViewSlice())
+			pol, err := unmarshalPolicy([]byte(tt.pol))
+			require.NoError(t, err)
+
+			err = validateUserReferences(pol, users)
 			require.Error(t, err, "site %q must surface duplicate-user errors", tt.name)
 			require.ErrorIs(t, err, ErrMultipleUsersFound)
+
+			pm, err := NewPolicyManager([]byte(`{
+  "acls": [{"action":"accept","src":["alice@"],"dst":["alice@:*"]}]
+}`), users, nodes.ViewSlice())
+			require.NoError(t, err)
+
+			before, _ := pm.Filter()
+
+			_, err = pm.SetPolicy([]byte(tt.pol))
+			require.ErrorIs(t, err, ErrMultipleUsersFound)
+
+			after, _ := pm.Filter()
+			require.Equal(t, before, after, "a rejected policy must not replace the live filter")
 		})
 	}
 }
@@ -4021,9 +4015,9 @@ func TestSetNodesCachedResultsMatchFresh(t *testing.T) {
 	}
 }
 
-// TestFailedSetUsersKeepsCompiledPolicy renames alice to charlie and adds
-// a second bob, so the recompile resolves charlie's grant and then fails on
-// the ambiguous nodeAttrs target. A failed Set* must leave every compiled result as it was:
+// TestFailedSetUsersKeepsCompiledPolicy renames alice to charlie while
+// an unresolvable tag owner prevents the recompile from succeeding.
+// A failed Set* must leave every compiled result as it was:
 // a partial compile would hand alice's nodes charlie's port 22, and the
 // reverse rename, which SetUsers sees as no change, would not clear it.
 func TestFailedSetUsersKeepsCompiledPolicy(t *testing.T) {
@@ -4071,9 +4065,13 @@ func TestFailedSetUsersKeepsCompiledPolicy(t *testing.T) {
 
 	renamed := slices.Clone(users)
 	renamed[0].Name = "charlie"
-	renamed = append(renamed, types.User{ID: 3, Name: "bob"})
+	good := pm.pol.TagOwners
+	missing := Tag("tag:missing")
+	pm.pol.TagOwners = TagOwners{"tag:srv": Owners{&missing}}
 
 	_, _, err = pm.SetUsers(renamed)
+	pm.pol.TagOwners = good
+
 	require.Error(t, err)
 	require.Equal(t, before, read(pm, nodes[0]), "a failed SetUsers must not change compiled results")
 
@@ -4642,8 +4640,8 @@ func TestWithResolvedGroups(t *testing.T) {
 		}
 	}
 
-	require.ErrorIs(t,
-		resolved.resolvedGroups["group:ambiguous"].err, ErrMultipleUsersFound)
+	// An ambiguous member matches no one, as outside a policy write.
+	require.NoError(t, resolved.resolvedGroups["group:ambiguous"].err)
 
 	unknown := Group("group:unknown")
 	got, err := unknown.resolve(resolved, users, nodes)
@@ -4811,3 +4809,52 @@ func TestSetNodesUnapprovedRoutesKeepGeneration(t *testing.T) {
 		})
 	}
 }
+
+func TestAmbiguousUserKeepsOtherGroupMembers(t *testing.T) {
+	users := types.Users{
+		{ID: 1, Name: "alice"},
+		{ID: 2, Name: "dup"},
+		{ID: 3, Name: "dup"},
+	}
+
+	nodes := types.Nodes{
+		node("alice", "100.64.0.1", "fd7a:115c:a1e0::1", users[0]),
+		node("first-dup", "100.64.0.2", "fd7a:115c:a1e0::2", users[1]),
+		node("second-dup", "100.64.0.3", "fd7a:115c:a1e0::3", users[2]),
+	}
+	for i, n := range nodes {
+		n.ID = types.NodeID(i + 1) //nolint:gosec
+	}
+
+	pol := []byte(`{
+		"groups": {"group:members": ["alice@", "dup@"]},
+		"acls": [{"action":"accept", "src":["group:members"], "dst":["*:*"]}],
+		"nodeAttrs": [{"target":["group:members"], "attr":["randomize-client-port"]}]
+	}`)
+	pm, err := NewPolicyManager(pol, users[:2], nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotEmpty(t, pm.NodeCapMap(nodes[0].ID))
+	require.NotEmpty(t, pm.NodeCapMap(nodes[1].ID))
+	require.Empty(t, pm.NodeCapMap(nodes[2].ID))
+
+	_, usersChanged, err := pm.SetUsers(users)
+	require.NoError(t, err)
+	require.True(t, usersChanged)
+
+	filter, _ := pm.Filter()
+	require.Len(t, filter, 1)
+	require.ElementsMatch(t, []string{"100.64.0.1", "fd7a:115c:a1e0::1"}, filter[0].SrcIPs)
+	require.Equal(t, tailcfg.NodeCapMap{"randomize-client-port": nil}, pm.NodeCapMap(nodes[0].ID))
+	require.Empty(t, pm.NodeCapMap(nodes[1].ID))
+	require.Empty(t, pm.NodeCapMap(nodes[2].ID))
+
+	fresh, err := NewPolicyManager(pol, users, nodes.ViewSlice())
+	require.NoError(t, err)
+
+	freshFilter, _ := fresh.Filter()
+	require.Equal(t, filter, freshFilter)
+	require.Equal(t, pm.NodeCapMaps(), fresh.NodeCapMaps())
+
+	_, err = pm.SetPolicy(pol)
+	require.ErrorIs(t, err, ErrMultipleUsersFound)
+}

hscontrol/policy/v2/tailscale_users_compat_test.go

@@ -97,35 +97,25 @@ var userTokenOverlays = []userTokenOverlay{
 var userTokenSkipReasons = map[string]string{
 	"users-short-name/okta-name-only/server/filter": shortUsernameRow,
 
-	"users-bare-at-test-dst/breakdown":                         testUnknownAlias,
-	"users-bare-at-test-dst/duplicate-unknown-email/breakdown": testUnknownAlias,
-	"users-bare-at-test-dst/name-is-unknown-email/breakdown":   testUnknownAlias,
-	"users-bare-at-test-dst/no-email-two/breakdown":            testUnknownAlias,
-	"users-bare-at-test-src/breakdown":                         testUnknownAlias,
-	"users-bare-at-test-src/duplicate-unknown-email/breakdown": testUnknownAlias,
-	"users-bare-at-test-src/name-is-unknown-email/breakdown":   testUnknownAlias,
-	"users-bare-at-test-src/no-email-two/breakdown":            testUnknownAlias,
+	"users-bare-at-test-dst/breakdown":                                  testUnknownAlias,
+	"users-bare-at-test-dst/duplicate-unknown-email/breakdown":          testUnknownAlias,
+	"users-bare-at-test-dst/name-is-owner-email/breakdown":              testUnknownAlias,
+	"users-bare-at-test-dst/name-is-unknown-email-unverified/breakdown": testUnknownAlias,
+	"users-bare-at-test-dst/name-is-unknown-email/breakdown":            testUnknownAlias,
+	"users-bare-at-test-dst/no-email-one/breakdown":                     testUnknownAlias,
+	"users-bare-at-test-dst/no-email-two/breakdown":                     testUnknownAlias,
+	"users-bare-at-test-dst/okta-name-only/breakdown":                   testUnknownAlias,
+	"users-bare-at-test-src/breakdown":                                  testUnknownAlias,
+	"users-bare-at-test-src/duplicate-unknown-email/breakdown":          testUnknownAlias,
+	"users-bare-at-test-src/name-is-owner-email/breakdown":              testUnknownAlias,
+	"users-bare-at-test-src/name-is-unknown-email-unverified/breakdown": testUnknownAlias,
+	"users-bare-at-test-src/name-is-unknown-email/breakdown":            testUnknownAlias,
+	"users-bare-at-test-src/no-email-one/breakdown":                     testUnknownAlias,
+	"users-bare-at-test-src/no-email-two/breakdown":                     testUnknownAlias,
+	"users-bare-at-test-src/okta-name-only/breakdown":                   testUnknownAlias,
 
-	"users-bare-at-dst/name-is-owner-email/boot":           ambiguousLoad,
-	"users-bare-at-dst/no-email-two/boot":                  ambiguousLoad,
-	"users-bare-at-group/name-is-owner-email/boot":         ambiguousLoad,
-	"users-bare-at-group/no-email-two/boot":                ambiguousLoad,
-	"users-bare-at-nodeattrs/name-is-owner-email/boot":     ambiguousLoad,
-	"users-bare-at-nodeattrs/no-email-two/boot":            ambiguousLoad,
-	"users-bare-at-nodeattrs/no-email-two/runtime":         ambiguousLoad,
-	"users-bare-at-src/name-is-owner-email/boot":           ambiguousLoad,
-	"users-bare-at-src/no-email-two/boot":                  ambiguousLoad,
-	"users-bare-at-tagowner/name-is-owner-email/boot":      ambiguousLoad,
-	"users-bare-at-tagowner/no-email-two/boot":             ambiguousLoad,
-	"users-bare-at-test-dst/name-is-owner-email/boot":      ambiguousLoad,
-	"users-bare-at-test-src/name-is-owner-email/boot":      ambiguousLoad,
-	"users-short-name/name-is-owner-email/boot":            ambiguousLoad,
-	"users-unknown-direct/duplicate-unknown-email/boot":    ambiguousLoad,
-	"users-unknown-direct/name-is-owner-email/boot":        ambiguousLoad,
-	"users-unknown-member/duplicate-unknown-email/boot":    ambiguousLoad,
-	"users-unknown-member/duplicate-unknown-email/runtime": ambiguousLoad,
-	"users-unknown-member/name-is-owner-email/boot":        ambiguousLoad,
-	"users-unknown-member/name-is-owner-email/runtime":     ambiguousLoad,
+	// The evaluation error repeats the test's source unquoted, here a user's name.
+	"users-bare-at-test-dst/name-is-owner-email/user-list": testUnknownAlias,
 
 	"users-unknown-direct/duplicate-unknown-email/verdict": ambiguousWrite,
 	"users-unknown-member/duplicate-unknown-email/verdict": ambiguousWrite,
@@ -133,32 +123,6 @@ var userTokenSkipReasons = map[string]string{
 	"users-unknown-direct/duplicate-unknown-email/user-list": ambiguityListsUsers,
 	"users-unknown-member/duplicate-unknown-email/user-list": ambiguityListsUsers,
 
-	"users-bare-at-dst/name-is-owner-email/verdict":             emailOwnerAmbiguous,
-	"users-bare-at-group/name-is-owner-email/verdict":           emailOwnerAmbiguous,
-	"users-bare-at-nodeattrs/name-is-owner-email/server/filter": emailOwnerAmbiguous,
-	"users-bare-at-nodeattrs/name-is-owner-email/verdict":       emailOwnerAmbiguous,
-	"users-bare-at-src/name-is-owner-email/verdict":             emailOwnerAmbiguous,
-	"users-bare-at-tagowner/name-is-owner-email/server/filter":  emailOwnerAmbiguous,
-	"users-bare-at-tagowner/name-is-owner-email/verdict":        emailOwnerAmbiguous,
-	"users-bare-at-test-dst/name-is-owner-email/breakdown":      emailOwnerAmbiguous,
-	"users-bare-at-test-dst/name-is-owner-email/message":        emailOwnerAmbiguous,
-	"users-bare-at-test-src/name-is-owner-email/breakdown":      emailOwnerAmbiguous,
-	"users-bare-at-test-src/name-is-owner-email/message":        emailOwnerAmbiguous,
-	"users-short-name/name-is-owner-email/verdict":              emailOwnerAmbiguous,
-	"users-unknown-direct/name-is-owner-email/verdict":          emailOwnerAmbiguous,
-	"users-unknown-member/name-is-owner-email/verdict":          emailOwnerAmbiguous,
-
-	"users-bare-at-dst/name-is-owner-email/user-list":       emailOwnerAmbiguous,
-	"users-bare-at-group/name-is-owner-email/user-list":     emailOwnerAmbiguous,
-	"users-bare-at-nodeattrs/name-is-owner-email/user-list": emailOwnerAmbiguous,
-	"users-bare-at-src/name-is-owner-email/user-list":       emailOwnerAmbiguous,
-	"users-bare-at-tagowner/name-is-owner-email/user-list":  emailOwnerAmbiguous,
-	"users-bare-at-test-dst/name-is-owner-email/user-list":  emailOwnerAmbiguous,
-	"users-bare-at-test-src/name-is-owner-email/user-list":  emailOwnerAmbiguous,
-	"users-short-name/name-is-owner-email/user-list":        emailOwnerAmbiguous,
-	"users-unknown-direct/name-is-owner-email/user-list":    emailOwnerAmbiguous,
-	"users-unknown-member/name-is-owner-email/user-list":    emailOwnerAmbiguous,
-
 	"users-unknown-direct/name-is-unknown-email-unverified/overlay-1/filter":         usernameAnswersEmail,
 	"users-unknown-direct/name-is-unknown-email-unverified/overlay-1/route-approver": usernameAnswersEmail,
 	"users-unknown-direct/name-is-unknown-email-unverified/overlay-1/tag-owner":      usernameAnswersEmail,
@@ -178,46 +142,15 @@ var userTokenSkipReasons = map[string]string{
 	"users-unknown-member/name-is-unknown-email/server/filter":                       usernameAnswersEmail,
 	"users-unknown-member/name-is-unknown-email/server/ssh":                          usernameAnswersEmail,
 
-	"users-bare-at-dst/name-is-unknown-email-unverified/overlay-1/filter":         bareAtNoEmail,
-	"users-bare-at-dst/no-email-one/overlay-1/filter":                             bareAtNoEmail,
-	"users-bare-at-dst/no-email-two/user-list":                                    bareAtNoEmail,
-	"users-bare-at-dst/no-email-two/verdict":                                      bareAtNoEmail,
-	"users-bare-at-dst/okta-name-only/user-tid/filter":                            bareAtNoEmail,
-	"users-bare-at-group/name-is-unknown-email-unverified/server/filter":          bareAtNoEmail,
-	"users-bare-at-group/no-email-one/server/filter":                              bareAtNoEmail,
-	"users-bare-at-group/no-email-two/user-list":                                  bareAtNoEmail,
-	"users-bare-at-group/no-email-two/verdict":                                    bareAtNoEmail,
-	"users-bare-at-group/okta-name-only/server/filter":                            bareAtNoEmail,
-	"users-bare-at-nodeattrs/name-is-unknown-email-unverified/overlay-1/capmap":   bareAtNoEmail,
-	"users-bare-at-nodeattrs/no-email-one/overlay-1/capmap":                       bareAtNoEmail,
-	"users-bare-at-nodeattrs/no-email-two/user-list":                              bareAtNoEmail,
-	"users-bare-at-nodeattrs/no-email-two/verdict":                                bareAtNoEmail,
-	"users-bare-at-nodeattrs/okta-name-only/user-tid/capmap":                      bareAtNoEmail,
-	"users-bare-at-src/name-is-unknown-email-unverified/server/filter":            bareAtNoEmail,
-	"users-bare-at-src/no-email-one/server/filter":                                bareAtNoEmail,
-	"users-bare-at-src/no-email-two/user-list":                                    bareAtNoEmail,
-	"users-bare-at-src/no-email-two/verdict":                                      bareAtNoEmail,
-	"users-bare-at-src/okta-name-only/server/filter":                              bareAtNoEmail,
-	"users-bare-at-tagowner/name-is-unknown-email-unverified/overlay-1/tag-owner": bareAtNoEmail,
-	"users-bare-at-tagowner/no-email-one/overlay-1/tag-owner":                     bareAtNoEmail,
-	"users-bare-at-tagowner/no-email-two/user-list":                               bareAtNoEmail,
-	"users-bare-at-tagowner/no-email-two/verdict":                                 bareAtNoEmail,
-	"users-bare-at-test-dst/no-email-two/user-list":                               bareAtNoEmail,
-	"users-bare-at-test-src/no-email-two/user-list":                               bareAtNoEmail,
-	"users-bare-at-test-src/okta-name-only/verdict":                               bareAtNoEmail,
-
 	"users-short-name/server/filter": shortUsername,
 }
 
 const (
 	shortUsername        = "HEADSCALE_SHORT_USERNAME: headscale lets odin@ name the user called odin; SaaS matches it to no one"
 	shortUsernameRow     = "HEADSCALE_SHORT_USERNAME: a row storing the address as its name leaves odin@ nothing to match"
-	ambiguousLoad        = "AMBIGUOUS_USER_LOAD: a user token matching two users stops the policy loading; SaaS keeps serving it"
 	ambiguousWrite       = "AMBIGUOUS_USER_WRITE: headscale refuses to store a policy whose user token matches two users (#3160)"
 	ambiguityListsUsers  = "AMBIGUITY_LISTS_USERS: headscale's refusal of a token matching two users lists both users by ID and name"
-	emailOwnerAmbiguous  = "EMAIL_OWNER_FIRST: a username equal to another user's e-mail makes that e-mail match two users"
 	usernameAnswersEmail = "USERNAME_ANSWERS_EMAIL_TOKEN: a username shaped like an unclaimed address takes that address's rights"
-	bareAtNoEmail        = `BARE_AT_NO_EMAIL: "@" matches the users without an e-mail; SaaS matches it to no one`
 )
 
 // TestUserTokenCompat replays every users capture as captured and under

hscontrol/policy/v2/types.go

@@ -434,19 +434,22 @@ func (u *Username) CanBeAutoApprover() bool {
 }
 
 // resolveUser attempts to find a user in the provided [types.Users] slice that matches the [Username].
-// It prioritizes matching the [types.User.ProviderIdentifier], and if not found, it falls back to matching
-// the [types.User.Email] or [types.User.Name].
+// It prioritizes matching the [types.User.ProviderIdentifier], then the [types.User.Email],
+// and falls back to the [types.User.Name] only when no email matches.
 // If no matching user is found, it returns an error indicating no user matching.
 // If multiple matching users are found, it returns an error indicating multiple users matching.
 // It returns the matched [types.User] and a nil error if exactly one match is found.
 func (u *Username) resolveUser(users types.Users) (types.User, error) {
-	var potentialUsers types.Users
+	var emailMatches, nameMatches types.Users
 
 	// At parsetime, we require all usernames to contain an "@" character, if the
 	// username token does not naturally do so (like email), the user have to
 	// add it to the end of the username. We strip it here as we do not expect the
 	// usernames to be stored with the "@".
 	uTrimmed := strings.TrimSuffix(u.String(), "@")
+	if uTrimmed == "" {
+		return types.User{}, fmt.Errorf("%w: token %q", ErrUserNotFound, u.String())
+	}
 
 	for _, user := range users {
 		if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == uTrimmed {
@@ -454,9 +457,18 @@ func (u *Username) resolveUser(users types.Users) (types.User, error) {
 			return user, nil
 		}
 
-		if user.Email == uTrimmed || user.Name == uTrimmed {
-			potentialUsers = append(potentialUsers, user)
+		if user.Email == uTrimmed {
+			emailMatches = append(emailMatches, user)
 		}
+
+		if user.Name == uTrimmed {
+			nameMatches = append(nameMatches, user)
+		}
+	}
+
+	potentialUsers := emailMatches
+	if len(potentialUsers) == 0 {
+		potentialUsers = nameMatches
 	}
 
 	if len(potentialUsers) == 0 {
@@ -475,21 +487,14 @@ func (u *Username) Resolve(_ *Policy, users types.Users, nodes views.Slice[types
 }
 
 func (u *Username) resolve(_ *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) {
-	var (
-		ips  netipx.IPSetBuilder
-		errs []error
-	)
+	var ips netipx.IPSetBuilder
 
 	user, err := u.resolveUser(users)
-	if errors.Is(err, ErrUserNotFound) {
-		// A policy may name users before they register (#2863). Report it
-		// as an empty set, not an error: callers that bail on error would
-		// otherwise drop a whole group for one missing member (#3513).
-		return &netipx.IPSet{}, nil
-	}
-
 	if err != nil {
-		errs = append(errs, err)
+		// Users may be missing before registration (#2863), or become
+		// ambiguous after a login. An empty set preserves other group
+		// members (#3513); the policy write boundary rejects ambiguity.
+		return &netipx.IPSet{}, nil //nolint:nilerr // Unresolved users intentionally match no nodes.
 	}
 
 	for _, node := range nodes.All() {
@@ -508,7 +513,7 @@ func (u *Username) resolve(_ *Policy, users types.Users, nodes views.Slice[types
 		}
 	}
 
-	return buildIPSetMultiErr(&ips, errs)
+	return ips.IPSet()
 }
 
 // Group is a special string which is always prefixed with `group:`.

hscontrol/policy/v2/types_test.go

@@ -2,9 +2,11 @@ package v2
 
 import (
 	"bytes"
+	"database/sql"
 	"encoding/json"
 	"fmt"
 	"net/netip"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -6485,3 +6487,161 @@ func TestUnmarshalPolicyMaxDstPorts(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveUserPrecedence(t *testing.T) {
+	const (
+		address    = "alice@example.com"
+		providerID = "https://idp.example/alice"
+	)
+
+	owner := types.User{ID: 1, Name: "alice", Email: address}
+	oidcName := types.User{ID: 2, Name: address, Provider: "oidc"}
+	localName := types.User{ID: 3, Name: address}
+	both := types.User{ID: 4, Name: address, Email: address}
+	provider := types.User{
+		ID:                 5,
+		Name:               "provider-user",
+		ProviderIdentifier: sql.NullString{String: providerID, Valid: true},
+	}
+
+	tests := []struct {
+		name    string
+		token   Username
+		users   types.Users
+		want    types.User
+		wantErr error
+	}{
+		{
+			name:  "email owner beats OIDC username",
+			token: Username(address),
+			users: types.Users{owner, oidcName},
+			want:  owner,
+		},
+		{
+			name:  "email owner beats local username",
+			token: Username(address),
+			users: types.Users{owner, localName},
+			want:  owner,
+		},
+		{
+			name:  "address-shaped username without email owner",
+			token: Username(address),
+			users: types.Users{oidcName, {ID: 6, Name: "other", Email: "other@example.com"}},
+			want:  oidcName,
+		},
+		{
+			name:  "email and username on the same user count once",
+			token: Username(address),
+			users: types.Users{both, oidcName},
+			want:  both,
+		},
+		{
+			name:    "duplicate usernames remain ambiguous",
+			token:   Username(address),
+			users:   types.Users{oidcName, localName},
+			wantErr: ErrMultipleUsersFound,
+		},
+		{
+			name:    "duplicate emails remain ambiguous despite a unique username",
+			token:   Username(address),
+			users:   types.Users{owner, {ID: 7, Name: "second-owner", Email: address}, oidcName},
+			wantErr: ErrMultipleUsersFound,
+		},
+		{
+			name:  "provider identifier wins",
+			token: Username(providerID + "@"),
+			users: types.Users{provider, {ID: 8, Name: providerID}},
+			want:  provider,
+		},
+		{
+			name:    "unknown user",
+			token:   "missing@",
+			users:   types.Users{owner, oidcName, localName},
+			wantErr: ErrUserNotFound,
+		},
+	}
+
+	for _, tt := range tests {
+		for _, reverse := range []bool{false, true} {
+			t.Run(fmt.Sprintf("%s/reverse=%t", tt.name, reverse), func(t *testing.T) {
+				users := slices.Clone(tt.users)
+				if reverse {
+					slices.Reverse(users)
+				}
+
+				got, err := tt.token.resolveUser(users)
+				if tt.wantErr != nil {
+					require.ErrorIs(t, err, tt.wantErr)
+
+					return
+				}
+
+				require.NoError(t, err)
+				require.Equal(t, tt.want, got)
+			})
+		}
+	}
+}
+
+func TestBareAtTokenMatchesNobody(t *testing.T) {
+	users := types.Users{
+		{ID: 1, Name: "first-private-user"},
+		{ID: 2, Name: "second-private-user"},
+	}
+
+	for count := range 3 {
+		t.Run(fmt.Sprintf("users=%d", count), func(t *testing.T) {
+			token := Username("@")
+			_, err := token.resolveUser(users[:count])
+			require.ErrorIs(t, err, ErrUserNotFound)
+
+			for _, user := range users {
+				require.NotContains(t, err.Error(), user.Name)
+			}
+
+			pm, err := NewPolicyManager([]byte(`{
+				"grants": [{"src":["@"], "dst":["*"], "ip":["*"]}]
+			}`), users[:count], types.Nodes{}.ViewSlice())
+			require.NoError(t, err, "stored policies with a bare @ must still load")
+
+			filter, _ := pm.Filter()
+			require.Empty(t, filter)
+		})
+	}
+}
+
+func TestSetPolicyBareAtToken(t *testing.T) {
+	users := types.Users{
+		{ID: 1, Name: "first-private-user"},
+		{ID: 2, Name: "second-private-user"},
+	}
+
+	nodes := types.Nodes{node("first-node", "100.64.0.1", "fd7a:115c:a1e0::1", users[0])}
+	nodes[0].ID = 1
+
+	const allowAll = `"acls": [{"action":"accept", "src":["*"], "dst":["*:*"]}]`
+
+	for _, count := range []int{1, 2} {
+		for _, tt := range []struct {
+			name string
+			src  string
+			dst  string
+		}{
+			{name: "source", src: "@", dst: "100.64.0.1:22"},
+			{name: "destination", src: "100.64.0.1", dst: "@:22"},
+		} {
+			t.Run(fmt.Sprintf("%s/users=%d", tt.name, count), func(t *testing.T) {
+				pm, err := NewPolicyManager([]byte(`{`+allowAll+`}`), users[:count], nodes.ViewSlice())
+				require.NoError(t, err)
+
+				candidate := fmt.Sprintf(`{%s, "tests":[{"src":%q, "accept":[%q]}]}`, allowAll, tt.src, tt.dst)
+				_, err = pm.SetPolicy([]byte(candidate))
+				require.ErrorIs(t, err, errPolicyTestsFailed)
+
+				for _, user := range users {
+					require.NotContains(t, err.Error(), user.Name)
+				}
+			})
+		}
+	}
+}

hscontrol/state/persist_test.go

@@ -1,6 +1,7 @@
 package state
 
 import (
+	"database/sql"
 	"encoding/json"
 	"errors"
 	"maps"
@@ -2775,3 +2776,48 @@ func TestUpdateUserRefreshesNodeCopiesWhenPolicyUpdateFails(t *testing.T) {
 	assert.Equal(t, "renamed-user", updated.User().Name(),
 		"the committed row and its cached copy must agree")
 }
+
+func TestUserNameCollisionDoesNotBlockCreationOrRestart(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+	database, err := db.NewHeadscaleDatabase(cfg)
+	require.NoError(t, err)
+
+	user := database.CreateUserForTest("dup")
+	node := database.CreateRegisteredNodeForTest(user, "duplicate-name-node")
+	_, err = database.SetPolicy(`{
+		"acls": [],
+		"nodeAttrs": [{"target":["dup@"], "attr":["randomize-client-port"]}]
+	}`)
+	require.NoError(t, err)
+	require.NoError(t, database.Close())
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		if s != nil {
+			_ = s.Close()
+		}
+	})
+	require.NotEmpty(t, s.polMan.NodeCapMap(node.ID))
+
+	created, _, err := s.CreateUser(types.User{
+		Name:               "dup",
+		Provider:           "oidc",
+		ProviderIdentifier: sql.NullString{String: "https://idp.example/second", Valid: true},
+	})
+	require.NoError(t, err, "a colliding user must not prevent user creation")
+	require.NotNil(t, created)
+
+	_, _, err = s.CreateUser(types.User{Name: "later"})
+	require.NoError(t, err, "a previous collision must not prevent later user creation")
+	assert.Empty(t, s.polMan.NodeCapMap(node.ID))
+	require.NoError(t, s.Close())
+	s = nil
+
+	reopened := persistTestReopen(t, dbPath)
+	users, err := reopened.ListAllUsers()
+	require.NoError(t, err)
+	require.Len(t, users, 3)
+	require.Empty(t, reopened.polMan.NodeCapMap(node.ID))
+}