Skip to content

Commit 91f86ad

Browse files
Flo4604mcstepp
Flo4604
authored and
mcstepp
committed
fix: add 403 error when 0 key verification perms (#4483)
* fix: add 403 error when 0 key verification perms * cleanup tests
1 parent 5b77add commit 91f86ad

File tree

6 files changed

+164
-28
lines changed

6 files changed

+164
-28
lines changed

‎go/apps/api/openapi/openapi-generated.yaml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5446,7 +5446,7 @@ paths:
54465446
- `api.*.verify_key` (verify keys in any API)
54475447
- `api.<api_id>.verify_key` (verify keys in specific API)
54485448
5449-
If you are getting a NOT_FOUND error, ensure your root key has the required verify key permissions.
5449+
**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.
54505450
operationId: verifyKey
54515451
requestBody:
54525452
content:
@@ -5500,7 +5500,8 @@ paths:
55005500
application/json:
55015501
schema:
55025502
$ref: '#/components/schemas/ForbiddenErrorResponse'
5503-
description: Forbidden
5503+
description: |
5504+
Forbidden. Returned when the root key has no verify permissions at all (neither `api.*.verify_key` nor `api.<api_id>.verify_key` for any API).
55045505
"404":
55055506
content:
55065507
application/json:

‎go/apps/api/openapi/spec/paths/v2/keys/verifyKey/index.yaml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ post:
2020
- `api.*.verify_key` (verify keys in any API)
2121
- `api.<api_id>.verify_key` (verify keys in specific API)
2222
23-
If you are getting a NOT_FOUND error, ensure your root key has the required verify key permissions.
23+
**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.
2424
operationId: verifyKey
2525
x-speakeasy-name-override: verifyKey
2626
security:
@@ -73,7 +73,8 @@ post:
7373
schema:
7474
"$ref": "../../../../error/UnauthorizedErrorResponse.yaml"
7575
"403":
76-
description: Forbidden
76+
description: |
77+
Forbidden. Returned when the root key has no verify permissions at all (neither `api.*.verify_key` nor `api.<api_id>.verify_key` for any API).
7778
content:
7879
application/json:
7980
schema:

‎go/apps/api/routes/v2_keys_verify_key/200_test.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -707,28 +707,4 @@ func TestSuccess(t *testing.T) {
707707
require.Equal(t, openapi.NOTFOUND, res.Body.Data.Code, "Key should be not found but got %s", res.Body.Data.Code)
708708
require.False(t, res.Body.Data.Valid, "Key should be invalid but got %t", res.Body.Data.Valid)
709709
})
710-
711-
key := h.CreateKey(seed.CreateKeyRequest{
712-
WorkspaceID: workspace.ID,
713-
KeySpaceID: api.KeyAuthID.String,
714-
})
715-
716-
t.Run("root key without sufficient permissions", func(t *testing.T) {
717-
// Create root key with insufficient permissions
718-
limitedRootKey := h.CreateRootKey(workspace.ID, "api.*.read") // Wrong permission
719-
720-
req := handler.Request{
721-
Key: key.Key,
722-
}
723-
724-
headers := http.Header{
725-
"Content-Type": {"application/json"},
726-
"Authorization": {fmt.Sprintf("Bearer %s", limitedRootKey)},
727-
}
728-
729-
res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req)
730-
require.Equal(t, 200, res.Status)
731-
require.NotNil(t, res.Body)
732-
require.Equal(t, openapi.NOTFOUND, res.Body.Data.Code, "Key should be not found but got %s", res.Body.Data.Code)
733-
})
734710
}

‎go/apps/api/routes/v2_keys_verify_key/403_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package handler_test
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
"github.com/unkeyed/unkey/go/apps/api/openapi"
10+
handler "github.com/unkeyed/unkey/go/apps/api/routes/v2_keys_verify_key"
11+
"github.com/unkeyed/unkey/go/pkg/testutil"
12+
"github.com/unkeyed/unkey/go/pkg/testutil/seed"
13+
)
14+
15+
func TestForbidden_NoVerifyPermissions(t *testing.T) {
16+
h := testutil.NewHarness(t)
17+
18+
route := &handler.Handler{
19+
DB: h.DB,
20+
Keys: h.Keys,
21+
Logger: h.Logger,
22+
Auditlogs: h.Auditlogs,
23+
ClickHouse: h.ClickHouse,
24+
}
25+
26+
h.Register(route)
27+
28+
workspace := h.Resources().UserWorkspace
29+
api := h.CreateApi(seed.CreateApiRequest{WorkspaceID: workspace.ID})
30+
key := h.CreateKey(seed.CreateKeyRequest{
31+
WorkspaceID: workspace.ID,
32+
KeySpaceID: api.KeyAuthID.String,
33+
})
34+
35+
t.Run("root key with no verify permissions returns 403", func(t *testing.T) {
36+
// Create root key with a permission that is NOT verify_key
37+
rootKeyWithoutVerify := h.CreateRootKey(workspace.ID, "api.*.read_key")
38+
39+
req := handler.Request{
40+
Key: key.Key,
41+
}
42+
43+
headers := http.Header{
44+
"Content-Type": {"application/json"},
45+
"Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithoutVerify)},
46+
}
47+
48+
res := testutil.CallRoute[handler.Request, openapi.ForbiddenErrorResponse](h, route, headers, req)
49+
require.Equal(t, 403, res.Status, "expected 403, received: %d", res.Status)
50+
require.NotNil(t, res.Body)
51+
require.NotNil(t, res.Body.Error)
52+
require.NotEmpty(t, res.Body.Meta.RequestId, "RequestId should be returned in error response")
53+
54+
// Verify the error message mentions both permission options
55+
require.Contains(t, res.Body.Error.Detail, "api.*.verify_key", "error should mention wildcard permission option")
56+
require.Contains(t, res.Body.Error.Detail, "api.<API_ID>.verify_key", "error should mention specific API permission option")
57+
})
58+
59+
t.Run("root key with verify permission for different api returns 200 NOT_FOUND (not 403)", func(t *testing.T) {
60+
// Create a second API
61+
api2 := h.CreateApi(seed.CreateApiRequest{WorkspaceID: workspace.ID})
62+
63+
// Create root key with verify permission for api2 only
64+
rootKeyForApi2 := h.CreateRootKey(workspace.ID, fmt.Sprintf("api.%s.verify_key", api2.ID))
65+
66+
// Try to verify a key from api1
67+
req := handler.Request{
68+
Key: key.Key,
69+
}
70+
71+
headers := http.Header{
72+
"Content-Type": {"application/json"},
73+
"Authorization": {fmt.Sprintf("Bearer %s", rootKeyForApi2)},
74+
}
75+
76+
// Should return 200 with NOT_FOUND (not 403) to avoid leaking key existence
77+
res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req)
78+
require.Equal(t, 200, res.Status, "expected 200, received: %d", res.Status)
79+
require.NotNil(t, res.Body)
80+
require.Equal(t, openapi.NOTFOUND, res.Body.Data.Code, "should return NOT_FOUND to avoid leaking key existence")
81+
require.False(t, res.Body.Data.Valid)
82+
})
83+
84+
t.Run("root key with wildcard verify permission returns 200 VALID", func(t *testing.T) {
85+
// Create root key with wildcard verify permission
86+
rootKeyWithVerify := h.CreateRootKey(workspace.ID, "api.*.verify_key")
87+
88+
req := handler.Request{
89+
Key: key.Key,
90+
}
91+
92+
headers := http.Header{
93+
"Content-Type": {"application/json"},
94+
"Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithVerify)},
95+
}
96+
97+
res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req)
98+
require.Equal(t, 200, res.Status, "expected 200, received: %d", res.Status)
99+
require.NotNil(t, res.Body)
100+
require.Equal(t, openapi.VALID, res.Body.Data.Code)
101+
require.True(t, res.Body.Data.Valid)
102+
})
103+
104+
t.Run("root key with specific api verify permission returns 200 VALID", func(t *testing.T) {
105+
// Create root key with specific API verify permission
106+
rootKeyWithSpecificVerify := h.CreateRootKey(workspace.ID, fmt.Sprintf("api.%s.verify_key", api.ID))
107+
108+
req := handler.Request{
109+
Key: key.Key,
110+
}
111+
112+
headers := http.Header{
113+
"Content-Type": {"application/json"},
114+
"Authorization": {fmt.Sprintf("Bearer %s", rootKeyWithSpecificVerify)},
115+
}
116+
117+
res := testutil.CallRoute[handler.Request, handler.Response](h, route, headers, req)
118+
require.Equal(t, 200, res.Status, "expected 200, received: %d", res.Status)
119+
require.NotNil(t, res.Body)
120+
require.Equal(t, openapi.VALID, res.Body.Data.Code)
121+
require.True(t, res.Body.Data.Valid)
122+
})
123+
}

‎go/apps/api/routes/v2_keys_verify_key/handler.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,24 @@ func (h *Handler) Handle(ctx context.Context, s *zen.Session) error {
5757
return err
5858
}
5959

60+
// Check if the root key has ANY verify permissions at all.
61+
// If not, return a proper permissions error immediately without looking up the key.
62+
// This prevents returning NOT_FOUND for every request when the root key simply lacks verify permissions entirely.
63+
if !auth.HasAnyPermission(rbac.Api, rbac.VerifyKey) {
64+
return auth.VerifyRootKey(ctx, keys.WithPermissions(rbac.Or(
65+
rbac.T(rbac.Tuple{
66+
ResourceType: rbac.Api,
67+
ResourceID: "*",
68+
Action: rbac.VerifyKey,
69+
}),
70+
rbac.T(rbac.Tuple{
71+
ResourceType: rbac.Api,
72+
ResourceID: "<API_ID>",
73+
Action: rbac.VerifyKey,
74+
}),
75+
)))
76+
}
77+
6078
// Request validation
6179
req, err := zen.BindBody[Request](s)
6280
if err != nil {

‎go/internal/services/keys/validation.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"database/sql"
66
"fmt"
7+
"strings"
78
"time"
89

910
"github.com/unkeyed/unkey/go/apps/api/openapi"
@@ -76,6 +77,22 @@ func (k *KeyVerifier) withIPWhitelist() error {
7677
return nil
7778
}
7879

80+
// HasAnyPermission checks if the key has any permission matching the given action.
81+
// It returns true if the key has at least one permission that ends with the specified action
82+
// for the given resource type (e.g., checking for any "api.*.verify_key" or "api.{apiId}.verify_key").
83+
func (k *KeyVerifier) HasAnyPermission(resourceType rbac.ResourceType, action rbac.ActionType) bool {
84+
prefix := string(resourceType) + "."
85+
suffix := "." + string(action)
86+
87+
for _, perm := range k.Permissions {
88+
if strings.HasPrefix(perm, prefix) && strings.HasSuffix(perm, suffix) {
89+
return true
90+
}
91+
}
92+
93+
return false
94+
}
95+
7996
// withPermissions validates that the key has the required RBAC permissions.
8097
// It uses the configured RBAC system to evaluate the permission query against the key's permissions.
8198
func (k *KeyVerifier) withPermissions(ctx context.Context, query rbac.PermissionQuery) error {

0 commit comments

Comments
 (0)