Skip to content

Commit 5cff88f

Browse files
kradalbysocoldkiller
authored andcommitted
simplify findUserByToken in ACL, add missing testcases (juanfont#2388)
* update users doc on unique constraints Signed-off-by: Kristoffer Dalby <[email protected]> * simplify finduser func Signed-off-by: Kristoffer Dalby <[email protected]> * add initial tests for findUserFromToken Signed-off-by: Kristoffer Dalby <[email protected]> * add changelog Signed-off-by: Kristoffer Dalby <[email protected]> --------- Signed-off-by: Kristoffer Dalby <[email protected]>
1 parent 9947f49 commit 5cff88f

File tree

4 files changed

+340
-23
lines changed

4 files changed

+340
-23
lines changed

CHANGELOG.md

+8-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,14 @@
77
- `oidc.map_legacy_users` is now `false` by default
88
[#2350](https://github.com/juanfont/headscale/pull/2350)
99

10-
## 0.24.1 (2025-01-xx)
10+
## 0.24.2 (2025-01-30)
11+
12+
### Changes
13+
14+
- Fix issue where email and username being equal fails to match in Policy
15+
[#2388](https://github.com/juanfont/headscale/pull/2388)
16+
17+
## 0.24.1 (2025-01-23)
1118

1219
### Changes
1320

hscontrol/policy/acls.go

+13-18
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ func (pol *ACLPolicy) CompileSSHPolicy(
381381
}
382382

383383
for _, userStr := range usersFromGroup {
384-
user, err := findUserFromTokenOrErr(users, userStr)
384+
user, err := findUserFromToken(users, userStr)
385385
if err != nil {
386386
log.Trace().Err(err).Msg("user not found")
387387
continue
@@ -400,7 +400,7 @@ func (pol *ACLPolicy) CompileSSHPolicy(
400400
// corresponds with the User info in the netmap.
401401
// TODO(kradalby): This is a bit of a hack, and it should go
402402
// away with the new policy where users can be reliably determined.
403-
if user, err := findUserFromTokenOrErr(users, srcToken); err == nil {
403+
if user, err := findUserFromToken(users, srcToken); err == nil {
404404
principals = append(principals, &tailcfg.SSHPrincipal{
405405
UserLogin: user.Username(),
406406
})
@@ -1001,7 +1001,7 @@ func (pol *ACLPolicy) TagsOfNode(
10011001
}
10021002
var found bool
10031003
for _, owner := range owners {
1004-
user, err := findUserFromTokenOrErr(users, owner)
1004+
user, err := findUserFromToken(users, owner)
10051005
if err != nil {
10061006
log.Trace().Caller().Err(err).Msg("could not determine user to filter tags by")
10071007
}
@@ -1038,7 +1038,7 @@ func (pol *ACLPolicy) TagsOfNode(
10381038
func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string) types.Nodes {
10391039
var out types.Nodes
10401040

1041-
user, err := findUserFromTokenOrErr(users, userToken)
1041+
user, err := findUserFromToken(users, userToken)
10421042
if err != nil {
10431043
log.Trace().Caller().Err(err).Msg("could not determine user to filter nodes by")
10441044
return out
@@ -1058,24 +1058,19 @@ var (
10581058
ErrorMultipleUserMatching = errors.New("multiple users matching")
10591059
)
10601060

1061-
func findUserFromTokenOrErr(
1062-
users []types.User,
1063-
token string,
1064-
) (types.User, error) {
1061+
// findUserFromToken finds and returns a user based on the given token, prioritizing matches by ProviderIdentifier, followed by email or name.
1062+
// If no matching user is found, it returns an error of type ErrorNoUserMatching.
1063+
// If multiple users match the token, it returns an error indicating multiple matches.
1064+
func findUserFromToken(users []types.User, token string) (types.User, error) {
10651065
var potentialUsers []types.User
1066+
10661067
for _, user := range users {
10671068
if user.ProviderIdentifier.Valid && user.ProviderIdentifier.String == token {
1068-
// If a user is matching with a known unique field,
1069-
// disgard all other users and only keep the current
1070-
// user.
1071-
potentialUsers = []types.User{user}
1072-
1073-
break
1069+
// Prioritize ProviderIdentifier match and exit early
1070+
return user, nil
10741071
}
1075-
if user.Email == token {
1076-
potentialUsers = append(potentialUsers, user)
1077-
}
1078-
if user.Name == token {
1072+
1073+
if user.Email == token || user.Name == token {
10791074
potentialUsers = append(potentialUsers, user)
10801075
}
10811076
}

0 commit comments

Comments
 (0)