From 408b9b77970896fff05d4cdfa4ca0dcc3d52ac57 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 15:48:14 +0000 Subject: [PATCH] api/v2: check the createKey scope before reading the body createKey has no static scope, so any token got its body parsed and validated before the handler's scope check. --- hscontrol/api/v2/keys.go | 17 +++++++++++++++++ hscontrol/apiv2_compat_test.go | 3 +-- hscontrol/apiv2_oauth_matrix_test.go | 21 +++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/hscontrol/api/v2/keys.go b/hscontrol/api/v2/keys.go index 84a8ce972..2cfbb8f13 100644 --- a/hscontrol/api/v2/keys.go +++ b/hscontrol/api/v2/keys.go @@ -133,6 +133,7 @@ func registerKeys(api huma.API, b Backend) { Description: "Requires the `auth_keys` scope for an auth key, or `oauth_keys` for an OAuth client (an admin API key is all-access).", Tags: keysTags, Security: security, + Middlewares: huma.Middlewares{requireCreateKeyScope(api)}, MaxBodyBytes: maxBodyBytes, Errors: []int{ http.StatusBadRequest, @@ -318,6 +319,22 @@ func requireKeyScope(ctx context.Context, need scope.Scope) error { return nil } +// requireCreateKeyScope refuses, before huma reads the body, a token that may +// create neither kind of key; the handler then checks the kind it got. +func requireCreateKeyScope(api huma.API) func(huma.Context, func(huma.Context)) { + return func(ctx huma.Context, next func(huma.Context)) { + if requireKeyScope(ctx.Context(), scope.AuthKeys) != nil && + requireKeyScope(ctx.Context(), scope.OAuthKeys) != nil { + _ = huma.WriteErr(api, ctx, http.StatusForbidden, + "token is missing the required scope auth_keys or oauth_keys") + + return + } + + next(ctx) + } +} + // createAuthKey creates a machine auth key (pre-auth key). Ownership: tags -> a // tagged key; no tags -> owned by the API key's user. An OAuth access token must // mint tagged keys, and each tag must be within the token's grant. diff --git a/hscontrol/apiv2_compat_test.go b/hscontrol/apiv2_compat_test.go index b3e2915e1..c87ace154 100644 --- a/hscontrol/apiv2_compat_test.go +++ b/hscontrol/apiv2_compat_test.go @@ -51,7 +51,7 @@ var apiv2CompatSkipReasons = map[string]string{ "oauth/R3/3/status": missingScopeNotFound, "oauth/K3/0/status": missingScopeNotFound, "oauth/J2/0/status": missingScopeNotFound, - "oauth/K4/0/status": bodyBeforeScope, + "oauth/K4/0/status": missingScopeNotFound, "oauth/D1/0/message": missingScopeText, "oauth/O1/0/scope": scopeEchoAsSent, @@ -200,7 +200,6 @@ func notCompared(prefixes []string, key string) bool { // Differences a planned change removes. 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" 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" ) diff --git a/hscontrol/apiv2_oauth_matrix_test.go b/hscontrol/apiv2_oauth_matrix_test.go index 49df9a683..83b5197e3 100644 --- a/hscontrol/apiv2_oauth_matrix_test.go +++ b/hscontrol/apiv2_oauth_matrix_test.go @@ -181,6 +181,27 @@ func TestAPIv2OAuthMatrix_ScopeNarrowing(t *testing.T) { } } +// TestAPIv2OAuthMatrix_CreateKeyScopeBeforeBody: createKey declares no static +// scope, so a token that may create neither kind of key must be refused before +// huma reads and validates the body. +func TestAPIv2OAuthMatrix_CreateKeyScopeBeforeBody(t *testing.T) { + _, baseURL, admin := newOAuthTestServer(t) + invalid := map[string]any{"tags": []int{0}} + + for _, s := range scope.Known() { + held := []scope.Scope{s} + mayCreate := scope.Grants(held, scope.AuthKeys) || scope.Grants(held, scope.OAuthKeys) + + status, body := apiReq(t, http.MethodPost, baseURL+"/api/v2/tailnet/-/keys", + mintScopedToken(t, baseURL, admin, s), invalid) + if mayCreate { + assert.Equalf(t, http.StatusUnprocessableEntity, status, "%s: %s", s, body) + } else { + assert.Truef(t, scopeDenied(status, body), "%s: %d %s", s, status, body) + } + } +} + // 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). Only an untagged client with the "all" scope may request any tag.