diff --git a/hscontrol/policy/v2/tailscale_users_compat_test.go b/hscontrol/policy/v2/tailscale_users_compat_test.go index c3a0a5202..a4154da3c 100644 --- a/hscontrol/policy/v2/tailscale_users_compat_test.go +++ b/hscontrol/policy/v2/tailscale_users_compat_test.go @@ -97,16 +97,22 @@ 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-owner-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-owner-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, // 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, @@ -136,33 +142,6 @@ 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-src/no-email-two/user-list": bareAtNoEmail, - "users-bare-at-test-src/okta-name-only/verdict": bareAtNoEmail, - "users-short-name/server/filter": shortUsername, } @@ -172,7 +151,6 @@ const ( 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" 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 diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 6ac28c700..9bbdcaf94 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -447,6 +447,9 @@ func (u *Username) resolveUser(users types.Users) (types.User, error) { // 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 { diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 7ed5c6d71..747b641cc 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -6582,3 +6582,66 @@ func TestResolveUserPrecedence(t *testing.T) { } } } + +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) + } + }) + } + } +}