db: reject unknown OAuth client scopes at creation (#3424)

Unknown scopes grant nothing, so clients were silently under-privileged.
Report every invalid scope at once.

Fixes #3406

cmd/headscale/cli/oauth_client.go

@@ -23,7 +23,7 @@ func init() {
 	oauthClientsCmd.AddCommand(listOAuthClientsCmd)
 
 	createOAuthClientCmd.Flags().
-		StringArrayP("scope", "s", nil, "Scope the client's tokens are granted (repeatable): auth_keys, oauth_keys, devices:core, devices:routes, policy_file, feature_settings (each with a :read variant), or all/all:read")
+		StringArrayP("scope", "s", nil, "Scope the client's tokens are granted (repeatable): auth_keys, oauth_keys, devices:core, devices:routes, policy_file, feature_settings, users (each with a :read variant), or all/all:read")
 	createOAuthClientCmd.Flags().
 		StringArrayP("tag", "t", nil, "Tag the client's tokens may assign to devices (repeatable), e.g. tag:k8s-operator")
 	createOAuthClientCmd.Flags().StringP("description", "d", "", "Human-readable description")

hscontrol/api/v2/errors.go

@@ -72,6 +72,7 @@ func mapError(msg string, err error) error {
 
 	case errors.Is(err, db.ErrPreAuthKeyNotTaggedOrOwned),
 		errors.Is(err, db.ErrPreAuthKeyACLTagInvalid),
+		errors.Is(err, db.ErrOAuthClientScopeInvalid),
 		errors.Is(err, state.ErrGivenNameInvalid),
 		errors.Is(err, state.ErrGivenNameTaken),
 		errors.Is(err, state.ErrNodeNameNotUnique),

hscontrol/db/oauth.go

@@ -11,9 +11,11 @@ import (
 	"strings"
 	"time"
 
+	"github.com/juanfont/headscale/hscontrol/scope"
 	"github.com/juanfont/headscale/hscontrol/types"
 	"golang.org/x/crypto/argon2"
 	"gorm.io/gorm"
+	"tailscale.com/util/multierr"
 	"tailscale.com/util/rands"
 	"tailscale.com/util/set"
 )
@@ -37,6 +39,7 @@ var (
 	ErrOAuthClientNotFound      = fmt.Errorf("oauth client not found: %w", gorm.ErrRecordNotFound)
 	ErrOAuthClientFailedToParse = errors.New("failed to parse oauth client secret")
 	ErrOAuthClientRevoked       = errors.New("oauth client revoked")
+	ErrOAuthClientScopeInvalid  = errors.New("oauth client scope invalid")
 
 	ErrAccessTokenNotFound      = fmt.Errorf("oauth access token not found: %w", gorm.ErrRecordNotFound)
 	ErrAccessTokenFailedToParse = errors.New("failed to parse oauth access token")
@@ -132,6 +135,22 @@ func verifySecret(encoded []byte, secret string) error {
 	return nil
 }
 
+// validateScopes rejects any value outside the vocabulary in [scope.Known]. An
+// unknown scope satisfies no requirement, so storing one would silently mint a
+// client with fewer permissions than asked for. Every offending scope is
+// reported so the caller can correct them all in one pass.
+func validateScopes(scopes []string) error {
+	var errs []error
+
+	for _, s := range scopes {
+		if !scope.Scope(s).Valid() {
+			errs = append(errs, fmt.Errorf("%w: %q", ErrOAuthClientScopeInvalid, s))
+		}
+	}
+
+	return multierr.New(errs...)
+}
+
 // CreateOAuthClient creates a new [types.OAuthClient] and returns the plaintext
 // secret (shown ONCE) alongside the stored client. creatorUserID is the user who
 // created it (informational), or nil.
@@ -145,6 +164,11 @@ func (hsdb *HSDatabase) CreateOAuthClient(
 		return "", nil, err
 	}
 
+	err = validateScopes(scopes)
+	if err != nil {
+		return "", nil, err
+	}
+
 	scopes = set.SetOf(scopes).Slice()
 	slices.Sort(scopes)
 

hscontrol/db/oauth_test.go

@@ -250,3 +250,61 @@ func TestAccessTokenRejectedWhenClientGone(t *testing.T) {
 	_, err = db.AuthenticateAccessToken(tokenStr2)
 	require.ErrorIs(t, err, ErrAccessTokenClientRevoked)
 }
+
+// TestOAuthClientCreateRejectsUnknownScopes asserts scopes are validated against
+// the known vocabulary the same way tags are, instead of being stored verbatim.
+func TestOAuthClientCreateRejectsUnknownScopes(t *testing.T) {
+	tests := []struct {
+		name   string
+		scopes []string
+		valid  bool
+	}{
+		{name: "known write and read scopes", scopes: []string{"auth_keys", "devices:core:read"}, valid: true},
+		{name: "super scopes", scopes: []string{"all", "all:read"}, valid: true},
+		{name: "arbitrary scope", scopes: []string{"EVIL:superuser"}},
+		{name: "path traversal", scopes: []string{"../../etc/passwd"}},
+		{name: "wildcard", scopes: []string{"*::*"}},
+		{name: "empty scope", scopes: []string{""}},
+		{name: "wrong case", scopes: []string{"AUTH_KEYS"}},
+		{name: "one unknown among known", scopes: []string{"auth_keys", "devices:everything"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			db, err := newSQLiteTestDB()
+			require.NoError(t, err)
+
+			_, client, err := db.CreateOAuthClient(tt.scopes, []string{"tag:ci"}, "", nil)
+			if tt.valid {
+				require.NoError(t, err)
+				assert.ElementsMatch(t, tt.scopes, client.Scopes)
+
+				return
+			}
+
+			require.ErrorIs(t, err, ErrOAuthClientScopeInvalid)
+			assert.Nil(t, client)
+
+			clients, err := db.ListOAuthClients()
+			require.NoError(t, err)
+			assert.Empty(t, clients, "rejected client must not be stored")
+		})
+	}
+}
+
+// TestOAuthClientCreateReportsEveryUnknownScope asserts validation does not stop
+// at the first bad scope, so a caller can fix them all in one pass.
+func TestOAuthClientCreateReportsEveryUnknownScope(t *testing.T) {
+	db, err := newSQLiteTestDB()
+	require.NoError(t, err)
+
+	_, _, err = db.CreateOAuthClient(
+		[]string{"auth_keys", "devices:everything", "EVIL:superuser"},
+		[]string{"tag:ci"},
+		"",
+		nil,
+	)
+	require.ErrorIs(t, err, ErrOAuthClientScopeInvalid)
+	assert.Contains(t, err.Error(), "devices:everything")
+	assert.Contains(t, err.Error(), "EVIL:superuser")
+}

hscontrol/scope/scope.go

@@ -8,7 +8,10 @@
 // hscontrol/api/v2, so it can be tested exhaustively on its own.
 package scope
 
-import "strings"
+import (
+	"slices"
+	"strings"
+)
 
 // Scope is an OAuth capability an operation requires and a token grants. The names
 // mirror Tailscale's API scopes; a "...:read" scope is the read-only subset of its
@@ -62,6 +65,13 @@ func Known() []Scope {
 	}
 }
 
+// Valid reports whether s is part of the known scope vocabulary. Scopes outside
+// it never satisfy any requirement, so they are rejected at creation time
+// rather than stored as permanently inert grants.
+func (s Scope) Valid() bool {
+	return slices.Contains(Known(), s)
+}
+
 // IsRead reports whether s is a read-only scope (its name ends with ":read").
 func (s Scope) IsRead() bool {
 	return strings.HasSuffix(string(s), readSuffix)

hscontrol/scope/scope_test.go

@@ -232,3 +232,17 @@ func TestKnownIsComplete(t *testing.T) {
 		t.Errorf("Known() has %d scopes, want 16", len(known))
 	}
 }
+
+func TestValid(t *testing.T) {
+	for _, s := range Known() {
+		if !s.Valid() {
+			t.Errorf("Known scope %q reported invalid", s)
+		}
+	}
+
+	for _, s := range []Scope{"", "EVIL:superuser", "*::*", "../../etc/passwd", "AUTH_KEYS", "devices", "all:write"} {
+		if s.Valid() {
+			t.Errorf("unknown scope %q reported valid", s)
+		}
+	}
+}