From 2ac4d3fbe56a8c06e4e7431fd4cf635ee7577718 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 18 Dec 2024 11:05:36 +0100 Subject: [PATCH] fix issue where tags were only assigned to email, not username Fixes #2300 Fixes #2307 Signed-off-by: Kristoffer Dalby --- hscontrol/policy/acls.go | 72 ++++++++++++++++++++++++----------- hscontrol/policy/acls_test.go | 27 ++++++------- hscontrol/policy/pm.go | 2 +- 3 files changed, 62 insertions(+), 39 deletions(-) diff --git a/hscontrol/policy/acls.go b/hscontrol/policy/acls.go index 5848ec33..88461d27 100644 --- a/hscontrol/policy/acls.go +++ b/hscontrol/policy/acls.go @@ -934,6 +934,7 @@ func isAutoGroup(str string) bool { // Invalid tags are tags added by a user on a node, and that user doesn't have authority to add this tag. // Valid tags are tags added by a user that is allowed in the ACL policy to add this tag. func (pol *ACLPolicy) TagsOfNode( + users []types.User, node *types.Node, ) ([]string, []string) { var validTags []string @@ -956,7 +957,12 @@ func (pol *ACLPolicy) TagsOfNode( } var found bool for _, owner := range owners { - if node.User.Username() == owner { + user, err := findUserFromTokenOrErr(users, owner) + if err != nil { + log.Trace().Caller().Err(err).Msg("could not determine user to filter tags by") + } + + if node.User.ID == user.ID { found = true } } @@ -988,30 +994,12 @@ func (pol *ACLPolicy) TagsOfNode( func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string) types.Nodes { var out types.Nodes - var potentialUsers []types.User - for _, user := range users { - if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == userToken { - // If a user is matching with a known unique field, - // disgard all other users and only keep the current - // user. - potentialUsers = []types.User{user} - - break - } - if user.Email == userToken { - potentialUsers = append(potentialUsers, user) - } - if user.Name == userToken { - potentialUsers = append(potentialUsers, user) - } + user, err := findUserFromTokenOrErr(users, userToken) + if err != nil { + log.Trace().Caller().Err(err).Msg("could not determine user to filter nodes by") + return out } - if len(potentialUsers) != 1 { - return nil - } - - user := potentialUsers[0] - for _, node := range nodes { if node.User.ID == user.ID { out = append(out, node) @@ -1021,6 +1009,44 @@ func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string) return out } +var ( + ErrorNoUserMatching = errors.New("no user matching") + ErrorMultipleUserMatching = errors.New("multiple users matching") +) + +func findUserFromTokenOrErr( + users []types.User, + token string, +) (types.User, error) { + var potentialUsers []types.User + for _, user := range users { + if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == token { + // If a user is matching with a known unique field, + // disgard all other users and only keep the current + // user. + potentialUsers = []types.User{user} + + break + } + if user.Email == token { + potentialUsers = append(potentialUsers, user) + } + if user.Name == token { + potentialUsers = append(potentialUsers, user) + } + } + + if len(potentialUsers) == 0 { + return types.User{}, fmt.Errorf("user with token %q not found: %w", token, ErrorNoUserMatching) + } + + if len(potentialUsers) > 1 { + return types.User{}, fmt.Errorf("multiple users with token %q found: %w", token, ErrorNoUserMatching) + } + + return potentialUsers[0], nil +} + // FilterNodesByACL returns the list of peers authorized to be accessed from a given node. func FilterNodesByACL( node *types.Node, diff --git a/hscontrol/policy/acls_test.go b/hscontrol/policy/acls_test.go index b00cec12..cc7a9c71 100644 --- a/hscontrol/policy/acls_test.go +++ b/hscontrol/policy/acls_test.go @@ -2735,6 +2735,12 @@ func TestReduceFilterRules(t *testing.T) { } func Test_getTags(t *testing.T) { + users := []types.User{ + { + Model: gorm.Model{ID: 1}, + Name: "joe", + }, + } type args struct { aclPolicy *ACLPolicy node *types.Node @@ -2754,9 +2760,7 @@ func Test_getTags(t *testing.T) { }, }, node: &types.Node{ - User: types.User{ - Name: "joe", - }, + User: users[0], Hostinfo: &tailcfg.Hostinfo{ RequestTags: []string{"tag:valid"}, }, @@ -2774,9 +2778,7 @@ func Test_getTags(t *testing.T) { }, }, node: &types.Node{ - User: types.User{ - Name: "joe", - }, + User: users[0], Hostinfo: &tailcfg.Hostinfo{ RequestTags: []string{"tag:valid", "tag:invalid"}, }, @@ -2794,9 +2796,7 @@ func Test_getTags(t *testing.T) { }, }, node: &types.Node{ - User: types.User{ - Name: "joe", - }, + User: users[0], Hostinfo: &tailcfg.Hostinfo{ RequestTags: []string{ "tag:invalid", @@ -2818,9 +2818,7 @@ func Test_getTags(t *testing.T) { }, }, node: &types.Node{ - User: types.User{ - Name: "joe", - }, + User: users[0], Hostinfo: &tailcfg.Hostinfo{ RequestTags: []string{"tag:invalid", "very-invalid"}, }, @@ -2834,9 +2832,7 @@ func Test_getTags(t *testing.T) { args: args{ aclPolicy: &ACLPolicy{}, node: &types.Node{ - User: types.User{ - Name: "joe", - }, + User: users[0], Hostinfo: &tailcfg.Hostinfo{ RequestTags: []string{"tag:invalid", "very-invalid"}, }, @@ -2849,6 +2845,7 @@ func Test_getTags(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { gotValid, gotInvalid := test.args.aclPolicy.TagsOfNode( + users, test.args.node, ) for _, valid := range gotValid { diff --git a/hscontrol/policy/pm.go b/hscontrol/policy/pm.go index 7b92d771..4e10003e 100644 --- a/hscontrol/policy/pm.go +++ b/hscontrol/policy/pm.go @@ -162,7 +162,7 @@ func (pm *PolicyManagerV1) Tags(node *types.Node) []string { return nil } - tags, invalid := pm.pol.TagsOfNode(node) + tags, invalid := pm.pol.TagsOfNode(pm.users, node) log.Debug().Strs("authorised_tags", tags).Strs("unauthorised_tags", invalid).Uint64("node.id", node.ID.Uint64()).Msg("tags provided by policy") return tags }