Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simplify findUserByToken in ACL, add missing testcases #2388

Merged
merged 4 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@
- `oidc.map_legacy_users` is now `false` by default
[#2350](https://github.com/juanfont/headscale/pull/2350)

## 0.24.1 (2025-01-xx)
## 0.24.2 (2025-01-30)

### Changes

- Fix issue where email and username being equal fails to match in Policy
[#2388](https://github.com/juanfont/headscale/pull/2388)

## 0.24.1 (2025-01-23)

### Changes

Expand Down
31 changes: 13 additions & 18 deletions hscontrol/policy/acls.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ func (pol *ACLPolicy) CompileSSHPolicy(
}

for _, userStr := range usersFromGroup {
user, err := findUserFromTokenOrErr(users, userStr)
user, err := findUserFromToken(users, userStr)
if err != nil {
log.Trace().Err(err).Msg("user not found")
continue
Expand All @@ -400,7 +400,7 @@ func (pol *ACLPolicy) CompileSSHPolicy(
// corresponds with the User info in the netmap.
// TODO(kradalby): This is a bit of a hack, and it should go
// away with the new policy where users can be reliably determined.
if user, err := findUserFromTokenOrErr(users, srcToken); err == nil {
if user, err := findUserFromToken(users, srcToken); err == nil {
principals = append(principals, &tailcfg.SSHPrincipal{
UserLogin: user.Username(),
})
Expand Down Expand Up @@ -1001,7 +1001,7 @@ func (pol *ACLPolicy) TagsOfNode(
}
var found bool
for _, owner := range owners {
user, err := findUserFromTokenOrErr(users, owner)
user, err := findUserFromToken(users, owner)
if err != nil {
log.Trace().Caller().Err(err).Msg("could not determine user to filter tags by")
}
Expand Down Expand Up @@ -1038,7 +1038,7 @@ func (pol *ACLPolicy) TagsOfNode(
func filterNodesByUser(nodes types.Nodes, users []types.User, userToken string) types.Nodes {
var out types.Nodes

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

func findUserFromTokenOrErr(
users []types.User,
token string,
) (types.User, error) {
// findUserFromToken finds and returns a user based on the given token, prioritizing matches by ProviderIdentifier, followed by email or name.
// If no matching user is found, it returns an error of type ErrorNoUserMatching.
// If multiple users match the token, it returns an error indicating multiple matches.
func findUserFromToken(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
// Prioritize ProviderIdentifier match and exit early
return user, nil
}
if user.Email == token {
potentialUsers = append(potentialUsers, user)
}
if user.Name == token {

if user.Email == token || user.Name == token {
potentialUsers = append(potentialUsers, user)
}
}
Expand Down
Loading
Loading