From adbe292bc2971ed1095744eca2a7698acec69d0e Mon Sep 17 00:00:00 2001 From: Flo Date: Fri, 5 Dec 2025 12:04:05 +0100 Subject: [PATCH 1/2] fix: add 403 error when 0 key verification perms --- go/apps/api/openapi/openapi-generated.yaml | 5 +- .../spec/paths/v2/keys/verifyKey/index.yaml | 5 +- .../api/routes/v2_keys_verify_key/200_test.go | 24 --- .../api/routes/v2_keys_verify_key/403_test.go | 161 ++++++++++++++++++ .../api/routes/v2_keys_verify_key/handler.go | 18 ++ go/internal/services/keys/validation.go | 17 ++ 6 files changed, 202 insertions(+), 28 deletions(-) create mode 100644 go/apps/api/routes/v2_keys_verify_key/403_test.go diff --git a/go/apps/api/openapi/openapi-generated.yaml b/go/apps/api/openapi/openapi-generated.yaml index b37277d260..6376b0101c 100644 --- a/go/apps/api/openapi/openapi-generated.yaml +++ b/go/apps/api/openapi/openapi-generated.yaml @@ -5446,7 +5446,7 @@ paths: - `api.*.verify_key` (verify keys in any API) - `api..verify_key` (verify keys in specific API) - If you are getting a NOT_FOUND error, ensure your root key has the required verify key permissions. + **Note**: If your root key has no verify permissions at all, you will receive a `403 Forbidden` error. If your root key has verify permissions for a different API than the key you're verifying, you will receive a `200` response with `code: NOT_FOUND` to avoid leaking key existence. operationId: verifyKey requestBody: content: @@ -5500,7 +5500,8 @@ paths: application/json: schema: $ref: '#/components/schemas/ForbiddenErrorResponse' - description: Forbidden + description: | + Forbidden. Returned when the root key has no verify permissions at all (neither `api.*.verify_key` nor `api..verify_key` for any API). "404": content: application/json: diff --git a/go/apps/api/openapi/spec/paths/v2/keys/verifyKey/index.yaml b/go/apps/api/openapi/spec/paths/v2/keys/verifyKey/index.yaml index 73a737759f..15f7194eea 100644 --- a/go/apps/api/openapi/spec/paths/v2/keys/verifyKey/index.yaml +++ b/go/apps/api/openapi/spec/paths/v2/keys/verifyKey/index.yaml @@ -20,7 +20,7 @@ post: - `api.*.verify_key` (verify keys in any API) - `api..verify_key` (verify keys in specific API) - If you are getting a NOT_FOUND error, ensure your root key has the required verify key permissions. + **Note**: If your root key has no verify permissions at all, you will receive a `403 Forbidden` error. If your root key has verify permissions for a different API than the key you're verifying, you will receive a `200` response with `code: NOT_FOUND` to avoid leaking key existence. operationId: verifyKey x-speakeasy-name-override: verifyKey security: @@ -73,7 +73,8 @@ post: schema: "$ref": "../../../../error/UnauthorizedErrorResponse.yaml" "403": - description: Forbidden + description: | + Forbidden. Returned when the root key has no verify permissions at all (neither `api.*.verify_key` nor `api..verify_key` for any API). content: application/json: schema: diff --git a/go/apps/api/routes/v2_keys_verify_key/200_test.go b/go/apps/api/routes/v2_keys_verify_key/200_test.go index a94b53cb57..e949450ce5 100644 --- a/go/apps/api/routes/v2_keys_verify_key/200_test.go +++ b/go/apps/api/routes/v2_keys_verify_key/200_test.go @@ -707,28 +707,4 @@ func TestSuccess(t *testing.T) { require.Equal(t, openapi.NOTFOUND, res.Body.Data.Code, "Key should be not found but got %s", res.Body.Data.Code) require.False(t, res.Body.Data.Valid, "Key should be invalid but got %t", res.Body.Data.Valid) }) - - key := h.CreateKey(seed.CreateKeyRequest{ - WorkspaceID: workspace.ID, - KeySpaceID: api.KeyAuthID.String, - }) - - t.Run("root key without sufficient permissions", func(t *testing.T) { - // Create root key with insufficient permissions - limitedRootKey := h.CreateRootKey(workspace.ID, "api.*.read") // Wrong permission - - req := handler.Request{ - Key: key.Key, - } - - headers := http.Header{ - "Content-Type": {"application/json"}, - "Authorization": {fmt.Sprintf("Bearer %s", limitedRootKey)}, - } - - res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req) - require.Equal(t, 200, res.Status) - require.NotNil(t, res.Body) - require.Equal(t, openapi.NOTFOUND, res.Body.Data.Code, "Key should be not found but got %s", res.Body.Data.Code) - }) } diff --git a/go/apps/api/routes/v2_keys_verify_key/403_test.go b/go/apps/api/routes/v2_keys_verify_key/403_test.go new file mode 100644 index 0000000000..42a768873c --- /dev/null +++ b/go/apps/api/routes/v2_keys_verify_key/403_test.go @@ -0,0 +1,161 @@ +package handler_test + +import ( + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + "github.com/unkeyed/unkey/go/apps/api/openapi" + handler "github.com/unkeyed/unkey/go/apps/api/routes/v2_keys_verify_key" + "github.com/unkeyed/unkey/go/pkg/testutil" + "github.com/unkeyed/unkey/go/pkg/testutil/seed" +) + +func TestForbidden_NoVerifyPermissions(t *testing.T) { + h := testutil.NewHarness(t) + + route := &handler.Handler{ + DB: h.DB, + Keys: h.Keys, + Logger: h.Logger, + Auditlogs: h.Auditlogs, + ClickHouse: h.ClickHouse, + } + + h.Register(route) + + workspace := h.Resources().UserWorkspace + api := h.CreateApi(seed.CreateApiRequest{WorkspaceID: workspace.ID}) + key := h.CreateKey(seed.CreateKeyRequest{ + WorkspaceID: workspace.ID, + KeySpaceID: api.KeyAuthID.String, + }) + + t.Run("root key with no verify permissions returns 403", func(t *testing.T) { + // Create root key with a permission that is NOT verify_key + rootKeyWithoutVerify := h.CreateRootKey(workspace.ID, "api.*.read_key") + + req := handler.Request{ + Key: key.Key, + } + + headers := http.Header{ + "Content-Type": {"application/json"}, + "Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithoutVerify)}, + } + + res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req) + require.Equal(t, 403, res.Status, "expected 403, received: %d", res.Status) + require.NotNil(t, res.Body) + require.NotNil(t, res.Body.Error) + require.NotEmpty(t, res.Body.Meta.RequestId, "RequestId should be returned in error response") + }) + + t.Run("root key with no verify permissions shows both permission options in error", func(t *testing.T) { + // Create root key with unrelated permission + rootKeyWithoutVerify := h.CreateRootKey(workspace.ID, "api.*.create_key") + + req := handler.Request{ + Key: key.Key, + } + + headers := http.Header{ + "Content-Type": {"application/json"}, + "Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithoutVerify)}, + } + + res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req) + require.Equal(t, 403, res.Status) + require.NotNil(t, res.Body) + require.NotNil(t, res.Body.Error) + + // Verify the error message mentions both permission options + require.Contains(t, res.Body.Error.Detail, "api.*.verify_key", "error should mention wildcard permission option") + require.Contains(t, res.Body.Error.Detail, "api..verify_key", "error should mention specific API permission option") + }) + + t.Run("root key with no permissions at all returns 403", func(t *testing.T) { + // Create root key with no permissions + rootKeyNoPerms := h.CreateRootKey(workspace.ID) + + req := handler.Request{ + Key: key.Key, + } + + headers := http.Header{ + "Content-Type": {"application/json"}, + "Authorization": {fmt.Sprintf("Bearer %s", rootKeyNoPerms)}, + } + + res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req) + require.Equal(t, 403, res.Status, "expected 403, received: %d", res.Status) + require.NotNil(t, res.Body) + require.NotNil(t, res.Body.Error) + }) + + t.Run("root key with verify permission for different api returns 200 NOT_FOUND (not 403)", func(t *testing.T) { + // Create a second API + api2 := h.CreateApi(seed.CreateApiRequest{WorkspaceID: workspace.ID}) + + // Create root key with verify permission for api2 only + rootKeyForApi2 := h.CreateRootKey(workspace.ID, fmt.Sprintf("api.%s.verify_key", api2.ID)) + + // Try to verify a key from api1 + req := handler.Request{ + Key: key.Key, + } + + headers := http.Header{ + "Content-Type": {"application/json"}, + "Authorization": {fmt.Sprintf("Bearer %s", rootKeyForApi2)}, + } + + // Should return 200 with NOT_FOUND (not 403) to avoid leaking key existence + res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req) + require.Equal(t, 200, res.Status, "expected 200, received: %d", res.Status) + require.NotNil(t, res.Body) + require.Equal(t, openapi.NOTFOUND, res.Body.Data.Code, "should return NOT_FOUND to avoid leaking key existence") + require.False(t, res.Body.Data.Valid) + }) + + t.Run("root key with wildcard verify permission returns 200 VALID", func(t *testing.T) { + // Create root key with wildcard verify permission + rootKeyWithVerify := h.CreateRootKey(workspace.ID, "api.*.verify_key") + + req := handler.Request{ + Key: key.Key, + } + + headers := http.Header{ + "Content-Type": {"application/json"}, + "Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithVerify)}, + } + + res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req) + require.Equal(t, 200, res.Status, "expected 200, received: %d", res.Status) + require.NotNil(t, res.Body) + require.Equal(t, openapi.VALID, res.Body.Data.Code) + require.True(t, res.Body.Data.Valid) + }) + + t.Run("root key with specific api verify permission returns 200 VALID", func(t *testing.T) { + // Create root key with specific API verify permission + rootKeyWithSpecificVerify := h.CreateRootKey(workspace.ID, fmt.Sprintf("api.%s.verify_key", api.ID)) + + req := handler.Request{ + Key: key.Key, + } + + headers := http.Header{ + "Content-Type": {"application/json"}, + "Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithSpecificVerify)}, + } + + res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req) + require.Equal(t, 200, res.Status, "expected 200, received: %d", res.Status) + require.NotNil(t, res.Body) + require.Equal(t, openapi.VALID, res.Body.Data.Code) + require.True(t, res.Body.Data.Valid) + }) +} diff --git a/go/apps/api/routes/v2_keys_verify_key/handler.go b/go/apps/api/routes/v2_keys_verify_key/handler.go index 9628fa0f64..5b92c418aa 100644 --- a/go/apps/api/routes/v2_keys_verify_key/handler.go +++ b/go/apps/api/routes/v2_keys_verify_key/handler.go @@ -57,6 +57,24 @@ func (h *Handler) Handle(ctx context.Context, s *zen.Session) error { return err } + // Check if the root key has ANY verify permissions at all. + // If not, return a proper permissions error immediately without looking up the key. + // This prevents returning NOT_FOUND for every request when the root key simply lacks verify permissions entirely. + if !auth.HasAnyPermission(rbac.Api, rbac.VerifyKey) { + return auth.VerifyRootKey(ctx, keys.WithPermissions(rbac.Or( + rbac.T(rbac.Tuple{ + ResourceType: rbac.Api, + ResourceID: "*", + Action: rbac.VerifyKey, + }), + rbac.T(rbac.Tuple{ + ResourceType: rbac.Api, + ResourceID: "", + Action: rbac.VerifyKey, + }), + ))) + } + // Request validation req, err := zen.BindBody[Request](s) if err != nil { diff --git a/go/internal/services/keys/validation.go b/go/internal/services/keys/validation.go index 3343e92bbe..810c188c39 100644 --- a/go/internal/services/keys/validation.go +++ b/go/internal/services/keys/validation.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "strings" "time" "github.com/unkeyed/unkey/go/apps/api/openapi" @@ -76,6 +77,22 @@ func (k *KeyVerifier) withIPWhitelist() error { return nil } +// HasAnyPermission checks if the key has any permission matching the given action. +// It returns true if the key has at least one permission that ends with the specified action +// for the given resource type (e.g., checking for any "api.*.verify_key" or "api.{apiId}.verify_key"). +func (k *KeyVerifier) HasAnyPermission(resourceType rbac.ResourceType, action rbac.ActionType) bool { + prefix := string(resourceType) + "." + suffix := "." + string(action) + + for _, perm := range k.Permissions { + if strings.HasPrefix(perm, prefix) && strings.HasSuffix(perm, suffix) { + return true + } + } + + return false +} + // withPermissions validates that the key has the required RBAC permissions. // It uses the configured RBAC system to evaluate the permission query against the key's permissions. func (k *KeyVerifier) withPermissions(ctx context.Context, query rbac.PermissionQuery) error { From 18d5ec26912a5ab598e603e64322d20b3475d260 Mon Sep 17 00:00:00 2001 From: Flo Date: Fri, 5 Dec 2025 12:08:50 +0100 Subject: [PATCH 2/2] cleanup tests --- .../api/routes/v2_keys_verify_key/403_test.go | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/go/apps/api/routes/v2_keys_verify_key/403_test.go b/go/apps/api/routes/v2_keys_verify_key/403_test.go index 42a768873c..9caee1457b 100644 --- a/go/apps/api/routes/v2_keys_verify_key/403_test.go +++ b/go/apps/api/routes/v2_keys_verify_key/403_test.go @@ -50,50 +50,12 @@ func TestForbidden_NoVerifyPermissions(t *testing.T) { require.NotNil(t, res.Body) require.NotNil(t, res.Body.Error) require.NotEmpty(t, res.Body.Meta.RequestId, "RequestId should be returned in error response") - }) - - t.Run("root key with no verify permissions shows both permission options in error", func(t *testing.T) { - // Create root key with unrelated permission - rootKeyWithoutVerify := h.CreateRootKey(workspace.ID, "api.*.create_key") - - req := handler.Request{ - Key: key.Key, - } - - headers := http.Header{ - "Content-Type": {"application/json"}, - "Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithoutVerify)}, - } - - res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req) - require.Equal(t, 403, res.Status) - require.NotNil(t, res.Body) - require.NotNil(t, res.Body.Error) // Verify the error message mentions both permission options require.Contains(t, res.Body.Error.Detail, "api.*.verify_key", "error should mention wildcard permission option") require.Contains(t, res.Body.Error.Detail, "api..verify_key", "error should mention specific API permission option") }) - t.Run("root key with no permissions at all returns 403", func(t *testing.T) { - // Create root key with no permissions - rootKeyNoPerms := h.CreateRootKey(workspace.ID) - - req := handler.Request{ - Key: key.Key, - } - - headers := http.Header{ - "Content-Type": {"application/json"}, - "Authorization": {fmt.Sprintf("Bearer %s", rootKeyNoPerms)}, - } - - res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req) - require.Equal(t, 403, res.Status, "expected 403, received: %d", res.Status) - require.NotNil(t, res.Body) - require.NotNil(t, res.Body.Error) - }) - t.Run("root key with verify permission for different api returns 200 NOT_FOUND (not 403)", func(t *testing.T) { // Create a second API api2 := h.CreateApi(seed.CreateApiRequest{WorkspaceID: workspace.ID})