api/v2: keep a tagged all-scope client's tokens inside its tags

Only an untagged all-scope client may mint tokens for any tag, so a
tagged token may not create one.

hscontrol/api/v2/keys.go

@@ -3,6 +3,7 @@ package apiv2
 import (
 	"context"
 	"net/http"
+	"slices"
 	"strconv"
 	"time"
 
@@ -430,6 +431,15 @@ func createOAuthClient(ctx context.Context, b Backend, body CreateKeyRequest) (*
 
 		tokenTags, _ := principalTags(ctx)
 
+		// An untagged "all" client may mint tokens for any tag (narrowTags), so a
+		// tag-limited token would escape its tags one client away.
+		if len(tokenTags) > 0 && len(body.Tags) == 0 &&
+			slices.Contains(body.Scopes, string(scope.All)) {
+			return nil, huma.Error403Forbidden(
+				"client may not be granted scope all without tags by a tagged token",
+			)
+		}
+
 		for _, tag := range body.Tags {
 			if !b.State.TagExists(tag) {
 				return nil, huma.Error400BadRequest("tag " + tag + " is not defined in policy")

hscontrol/api/v2/oauth.go

@@ -90,7 +90,7 @@ func oauthTokenHandler(b Backend) http.HandlerFunc {
 		tags, badTag, ok := narrowTags(client, strings.Fields(r.PostForm.Get("tags")))
 		if !ok {
 			writeOAuthError(w, http.StatusBadRequest, "invalid_target",
-				"tag "+badTag+" is not granted to this client")
+				"requested tags ["+badTag+"] are invalid or not permitted")
 
 			return
 		}
@@ -133,24 +133,25 @@ func narrowScopes(granted, requested []string) ([]string, string, bool) {
 
 // narrowTags returns the requested tags if each is within the client's grant (an
 // empty request means "the client's full tag set"), otherwise the offending tag
-// and false. A client with the "all" scope may assign any tag, matching Tailscale.
+// and false. Only an untagged client with the "all" scope may assign any tag;
+// a tagged one stays inside its tags, matching Tailscale.
 func narrowTags(client *types.OAuthClient, requested []string) ([]string, string, bool) {
 	if len(requested) == 0 {
 		return client.Tags, "", true
 	}
 
-	allScope := slices.Contains(client.Scopes, string(scope.All))
+	anyTag := len(client.Tags) == 0 && slices.Contains(client.Scopes, string(scope.All))
 
 	for _, req := range requested {
 		// Reject malformed tags at the trust boundary. A client's own tags are
-		// validated at creation, so this only matters for an "all"-scope client,
+		// validated at creation, so this only matters for an any-tag client,
 		// whose tags would otherwise skip the membership check below and flow
 		// unvalidated into auth-key creation.
 		if !strings.HasPrefix(req, "tag:") {
 			return nil, req, false
 		}
 
-		if !allScope && !slices.Contains(client.Tags, req) {
+		if !anyTag && !slices.Contains(client.Tags, req) {
 			return nil, req, false
 		}
 	}

hscontrol/apiv2_compat_test.go

@@ -83,11 +83,6 @@ var apiv2CompatSkipReasons = map[string]string{
 	"oauth/J3/1/detail": refusalDetail,
 	"oauth/J4/0/detail": refusalDetail,
 
-	"oauth/E2/0/status":   taggedAllClientAnyTag,
-	"oauth/E2/0/outcome":  taggedAllClientAnyTag,
-	"oauth/E9a/1/status":  taggedAllClientAnyTag,
-	"oauth/E9a/1/outcome": taggedAllClientAnyTag,
-
 	"oauth/E4/0/status": tagRefusalStatus,
 	"oauth/E5/0/status": tagRefusalStatus,
 	"oauth/T2/1/status": tagRefusalStatus,
@@ -213,7 +208,6 @@ func notCompared(prefixes []string, key string) bool {
 const (
 	scopeEchoAsSent        = "SCOPE_ECHO_AS_SENT: SaaS echoes a token's scopes sorted and without repeats; headscale echoes the request as sent"
 	bodyBeforeScope        = "BODY_BEFORE_SCOPE: headscale validates a createKey body before the token's scope and answers 422; SaaS answers 404 for the missing scope"
-	taggedAllClientAnyTag  = "TAGGED_ALL_CLIENT_ANY_TAG: SaaS lets a tagged all-scope client's token ask only for the client's tags; headscale lets it ask for any tag"
 	retagRemovesUnownedTag = "RETAG_REMOVES_UNOWNED_TAG: SaaS refuses a token's retag of a device holding a tag outside the token's grant with 400 cannot remove current tags; headscale replaces the tags"
 )
 

hscontrol/apiv2_oauth_matrix_test.go

@@ -183,7 +183,7 @@ func TestAPIv2OAuthMatrix_ScopeNarrowing(t *testing.T) {
 
 // TestAPIv2OAuthMatrix_TagNarrowing proves P1 for tags at mint time: a client may
 // only mint a token for tags within its grant (closing the /oauth/token tags-param
-// path). A client with the "all" scope may request any tag.
+// path). Only an untagged client with the "all" scope may request any tag.
 func TestAPIv2OAuthMatrix_TagNarrowing(t *testing.T) {
 	_, baseURL, admin := newOAuthTestServer(t)
 
@@ -202,10 +202,15 @@ func TestAPIv2OAuthMatrix_TagNarrowing(t *testing.T) {
 	assert.Equal(t, http.StatusBadRequest, st, mm)
 	assert.Equal(t, "invalid_target", mm["error"])
 
-	// An all-scope client may request any tag.
+	// A tagged all-scope client stays inside its tags; an untagged one may
+	// request any tag.
 	_, allSecret := createClient(t, baseURL, admin, []string{"all"}, []string{"tag:a"})
 	stAll, mAll := mintTokenWithTags(t, baseURL, allSecret, "tag:anything")
-	assert.Equalf(t, http.StatusOK, stAll, "all-scope client may assign any tag: %v", mAll)
+	assert.Equalf(t, http.StatusBadRequest, stAll, "tagged all-scope client: %v", mAll)
+
+	_, anySecret := createClient(t, baseURL, admin, []string{"all"}, nil)
+	stAny, mAny := mintTokenWithTags(t, baseURL, anySecret, "tag:anything")
+	assert.Equalf(t, http.StatusOK, stAny, "untagged all-scope client may assign any tag: %v", mAny)
 }
 
 // TestAPIv2OAuthMatrix_Lifecycle proves expired and revoked credentials are denied

hscontrol/apiv2_oauth_test.go

@@ -288,6 +288,42 @@ func TestAPIv2OAuth_ClientManagementScopes(t *testing.T) {
 	assert.Equal(t, http.StatusForbidden, statusDeny, "auth_keys (no oauth_keys) cannot create a client")
 }
 
+// TestAPIv2OAuth_TaggedAllClientStaysInTags asserts an all-scope client with
+// tags mints tokens only inside its tags, and that a tagged token cannot
+// escape them through an untagged all-scope child, whose tokens may take any
+// tag.
+func TestAPIv2OAuth_TaggedAllClientStaysInTags(t *testing.T) {
+	_, baseURL, admin := newOAuthTestServer(t)
+
+	_, secret := createClient(t, baseURL, admin, []string{"all"}, []string{"tag:a"})
+
+	st, m := mintTokenWithTags(t, baseURL, secret, "tag:b")
+	assert.Equalf(t, http.StatusBadRequest, st, "%v", m)
+	assert.Equal(t, "requested tags [tag:b] are invalid or not permitted", m["error_description"])
+
+	tok := tokenFor(t, baseURL, secret)
+	assert.Equal(t, http.StatusForbidden,
+		createTaggedKey(t, baseURL, tok, []string{"tag:b"}),
+		"the token is held to its tags")
+
+	// A tagged child stays inside its tags.
+	_, childSecret := createClient(t, baseURL, tok, []string{"all"}, []string{"tag:a"})
+	st, m = mintTokenWithTags(t, baseURL, childSecret, "tag:b")
+	assert.Equalf(t, http.StatusBadRequest, st, "tagged child: %v", m)
+
+	createClient(t, baseURL, tok, []string{"auth_keys", "oauth_keys"}, []string{"tag:a"})
+
+	st, body := apiPost(t, baseURL+"/api/v2/tailnet/-/keys", tok,
+		apiv2.CreateKeyRequest{KeyType: "client", Scopes: []string{"all"}})
+	assert.Equalf(t, http.StatusForbidden, st, "untagged all-scope child: %s", body)
+
+	// An admin API key still creates an untagged all-scope client, whose
+	// tokens may take any tag.
+	_, anySecret := createClient(t, baseURL, admin, []string{"all"}, nil)
+	st, m = mintTokenWithTags(t, baseURL, anySecret, "tag:b")
+	assert.Equalf(t, http.StatusOK, st, "%v", m)
+}
+
 func TestAPIv2OAuth_TagOwnedBy(t *testing.T) {
 	app, baseURL, admin := newOAuthTestServer(t)