mirror of
https://github.com/juanfont/headscale.git
synced 2026-10-04 22:03:39 +09:00
cli: distinguish no match from ambiguous match when resolving users
resolveSingleUser reported every non-single result as "multiple users match query", including zero matches. An explicit --identifier 0 was sent to the API, which treats id=0 as no filter, so it listed every user and failed as ambiguous. Return a not-found error for zero matches, list the matching users when several match, and reject a non-positive identifier before calling the API.
This commit is contained in:
committed by
Kristoffer Dalby
parent
e4b4b89448
commit
3746ad20db
@@ -53,6 +53,7 @@ tags; any other tag is rejected, for new and re-registering nodes alike. See
|
|||||||
|
|
||||||
- Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324)
|
- Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324)
|
||||||
- Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341)
|
- Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341)
|
||||||
|
- Fix `headscale users destroy`/`rename` reporting "multiple users match query" when no user matches; an ambiguous match now lists the matching users [#3476](https://github.com/juanfont/headscale/pull/3476)
|
||||||
- Headscale now requires Go 1.27 to build
|
- Headscale now requires Go 1.27 to build
|
||||||
|
|
||||||
## 0.29.4 (2026-09-23)
|
## 0.29.4 (2026-09-23)
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
|
||||||
clientv1 "github.com/juanfont/headscale/gen/client/v1"
|
clientv1 "github.com/juanfont/headscale/gen/client/v1"
|
||||||
"github.com/juanfont/headscale/hscontrol/util"
|
"github.com/juanfont/headscale/hscontrol/util"
|
||||||
@@ -19,6 +20,8 @@ import (
|
|||||||
var (
|
var (
|
||||||
errFlagRequired = errors.New("--name or --identifier flag is required")
|
errFlagRequired = errors.New("--name or --identifier flag is required")
|
||||||
errMultipleUsersMatch = errors.New("multiple users match query, specify an ID")
|
errMultipleUsersMatch = errors.New("multiple users match query, specify an ID")
|
||||||
|
errUserNotFound = errors.New("no user matches query")
|
||||||
|
errInvalidIdentifier = errors.New("--identifier must be a positive user ID")
|
||||||
)
|
)
|
||||||
|
|
||||||
func usernameAndIDFlag(cmd *cobra.Command) {
|
func usernameAndIDFlag(cmd *cobra.Command) {
|
||||||
@@ -31,6 +34,13 @@ func usernameAndIDFromFlag(cmd *cobra.Command) (uint64, string, error) {
|
|||||||
username, _ := cmd.Flags().GetString("name")
|
username, _ := cmd.Flags().GetString("name")
|
||||||
|
|
||||||
identifier, _ := cmd.Flags().GetInt64("identifier")
|
identifier, _ := cmd.Flags().GetInt64("identifier")
|
||||||
|
|
||||||
|
// An explicit zero or negative identifier never matches a user; the
|
||||||
|
// API treats id=0 as "no filter", which would list every user.
|
||||||
|
if cmd.Flags().Changed("identifier") && identifier <= 0 {
|
||||||
|
return 0, "", errInvalidIdentifier
|
||||||
|
}
|
||||||
|
|
||||||
if username == "" && identifier < 0 {
|
if username == "" && identifier < 0 {
|
||||||
return 0, "", errFlagRequired
|
return 0, "", errFlagRequired
|
||||||
}
|
}
|
||||||
@@ -74,11 +84,29 @@ func resolveSingleUser(
|
|||||||
}
|
}
|
||||||
|
|
||||||
users := resp.JSON200.Users
|
users := resp.JSON200.Users
|
||||||
if len(users) != 1 {
|
|
||||||
return "", nil, errMultipleUsersMatch
|
switch len(users) {
|
||||||
|
case 0:
|
||||||
|
return "", nil, errUserNotFound
|
||||||
|
case 1:
|
||||||
|
return users[0].Id, &users[0], nil
|
||||||
|
default:
|
||||||
|
return "", nil, fmt.Errorf("%w: %s", errMultipleUsersMatch, describeUsers(users))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return users[0].Id, &users[0], nil
|
// describeUsers renders the users that matched an ambiguous query so the
|
||||||
|
// operator can pick one by ID.
|
||||||
|
func describeUsers(users []clientv1.User) string {
|
||||||
|
parts := make([]string, len(users))
|
||||||
|
for i, user := range users {
|
||||||
|
parts[i] = fmt.Sprintf(
|
||||||
|
"id=%s name=%s email=%s provider=%s",
|
||||||
|
user.Id, user.Name, user.Email, user.Provider,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return strings.Join(parts, "; ")
|
||||||
}
|
}
|
||||||
|
|
||||||
func init() {
|
func init() {
|
||||||
@@ -162,7 +190,7 @@ var destroyUserCmd = &cobra.Command{
|
|||||||
}
|
}
|
||||||
|
|
||||||
if !confirmAction(cmd, fmt.Sprintf(
|
if !confirmAction(cmd, fmt.Sprintf(
|
||||||
"Do you want to remove the user %q (%s) and any associated preauthkeys?",
|
"Do you want to remove the user %q (%s) and its pre-auth keys? Nodes owned by the user must be deleted or moved first.",
|
||||||
user.Name, user.Id,
|
user.Name, user.Id,
|
||||||
)) {
|
)) {
|
||||||
return printOutput(cmd, map[string]string{colResult: "User not destroyed"}, "User not destroyed")
|
return printOutput(cmd, map[string]string{colResult: "User not destroyed"}, "User not destroyed")
|
||||||
|
|||||||
@@ -3,8 +3,10 @@ package cli
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"errors"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
clientv1 "github.com/juanfont/headscale/gen/client/v1"
|
clientv1 "github.com/juanfont/headscale/gen/client/v1"
|
||||||
@@ -81,6 +83,9 @@ func TestResolveSingleUser(t *testing.T) {
|
|||||||
flagName string
|
flagName string
|
||||||
wantId string
|
wantId string
|
||||||
wantErr bool
|
wantErr bool
|
||||||
|
wantErrIs error
|
||||||
|
// wantErrHas are substrings the error message must contain.
|
||||||
|
wantErrHas []string
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
// Regression: renaming by name used to return the raw flag
|
// Regression: renaming by name used to return the raw flag
|
||||||
@@ -97,17 +102,40 @@ func TestResolveSingleUser(t *testing.T) {
|
|||||||
wantId: "9",
|
wantId: "9",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "no match is an error",
|
// Regression: zero matches used to be reported as
|
||||||
|
// "multiple users match query".
|
||||||
|
name: "no match is a not-found error",
|
||||||
users: []clientv1.User{lukas},
|
users: []clientv1.User{lukas},
|
||||||
flagName: "nobody@example.com",
|
flagName: "nobody@example.com",
|
||||||
wantErr: true,
|
wantErr: true,
|
||||||
|
wantErrIs: errUserNotFound,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
// OIDC users can share a name, see issue #3429.
|
// OIDC users can share a name, see issue #3429.
|
||||||
name: "multiple matches are an error",
|
name: "multiple matches are an ambiguity error listing the matches",
|
||||||
users: []clientv1.User{hannes, hannesDup},
|
users: []clientv1.User{hannes, hannesDup},
|
||||||
flagName: "hannes@rueger.events",
|
flagName: "hannes@rueger.events",
|
||||||
wantErr: true,
|
wantErr: true,
|
||||||
|
wantErrIs: errMultipleUsersMatch,
|
||||||
|
wantErrHas: []string{
|
||||||
|
"id=9 name=hannes@rueger.events email=hannes@rueger.events",
|
||||||
|
"id=10 name=hannes@rueger.events email=other@example.com",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// Regression: --identifier 0 was sent to the API as "no
|
||||||
|
// filter", listing every user and failing as ambiguous.
|
||||||
|
name: "identifier zero is rejected before calling the API",
|
||||||
|
users: []clientv1.User{lukas, hannes},
|
||||||
|
identifier: "0",
|
||||||
|
wantErr: true,
|
||||||
|
wantErrIs: errInvalidIdentifier,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "no flags is a usage error",
|
||||||
|
users: []clientv1.User{lukas},
|
||||||
|
wantErr: true,
|
||||||
|
wantErrIs: errFlagRequired,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -129,6 +157,16 @@ func TestResolveSingleUser(t *testing.T) {
|
|||||||
t.Fatalf("resolveSingleUser() error = nil, want error")
|
t.Fatalf("resolveSingleUser() error = nil, want error")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if tt.wantErrIs != nil && !errors.Is(err, tt.wantErrIs) {
|
||||||
|
t.Fatalf("resolveSingleUser() error = %v, want %v", err, tt.wantErrIs)
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, want := range tt.wantErrHas {
|
||||||
|
if !strings.Contains(err.Error(), want) {
|
||||||
|
t.Errorf("resolveSingleUser() error = %q, want it to contain %q", err, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user