-
Notifications
You must be signed in to change notification settings - Fork 607
feat: add permissions for ratelimit overrides #2126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
79 commits
Select commit
Hold shift + click to select a range
e622fff
permissions
cea69ff
[autofix.ci] apply automated fixes
autofix-ci[bot] 54be747
step 2
16cd65d
files
9930e56
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
6a14873
Merge branch 'eng-1336-custom-override-permissions' of https://github…
921241d
more route work
a19fa91
First route
35c0355
Routes made and tests. setOverride not working
f30d350
broken tests
ba6e5ce
docs start
4dff8d0
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
93d1051
save changes
258436c
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
3ca0f80
works
ee4131b
[autofix.ci] apply automated fixes
autofix-ci[bot] c921762
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
f812fd3
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
cf1ce1c
[autofix.ci] apply automated fixes
autofix-ci[bot] f6fcd91
remove docs for PR submittal
d516bc7
Merge branch 'eng-1336-custom-override-permissions' of https://github…
775715f
Update apps/api/src/routes/v1_ratelimit_deleteOverride.ts
MichaelUnkey d10d36c
Update apps/api/src/routes/v1_ratelimit_deleteOverride.ts
MichaelUnkey 9fd380f
[autofix.ci] apply automated fixes
autofix-ci[bot] 730d661
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
b6519f1
Update apps/api/src/routes/v1_ratelimit_listOverrides.ts
MichaelUnkey eacb3b6
response message changed
5828b35
Merge branch 'eng-1336-custom-override-permissions' of https://github…
991edce
fixed build error
56b8c75
Merge branch 'main' into eng-1336-custom-override-permissions
MichaelUnkey e961f82
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
1dd6470
Update apps/api/src/routes/v1_ratelimit_deleteOverride.ts
MichaelUnkey 652b5d3
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
1654e11
Git Comment changes
5974580
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
a8818fd
store
2452892
backup
b83f336
back
2c84335
revert stash
c6896e9
set route working
fcbc029
get list and set routes
77093db
mod routes no errors
4c9a657
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
ef342cc
store after fix
e2545dc
minor changes
41c4d20
fmt
8e238d2
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
6d390a4
Pull and Merge
3553bc3
happy tests
45f2732
Merge branch 'main' into eng-1336-custom-override-permissions
MichaelUnkey 61dc582
security tests
f18588d
Merge branch 'eng-1336-custom-override-permissions' of https://github…
7fd98d1
missing bracket
27abeca
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
cb2ca1e
PR changes
754b87b
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
8f850bc
Merge branch 'main' of https://github.com/unkeyed/unkey into eng-1336…
chronark 5bca654
chore: clean up
chronark 67e4cf6
fix: prod url
chronark b2373dd
[autofix.ci] apply automated fixes
autofix-ci[bot] f2336c4
revert: sdk methods
chronark 37dd295
Merge branch 'eng-1336-custom-override-permissions' of https://github…
chronark 9cccdfb
ci: don't log
chronark 0f899c7
fix: put thread in background
chronark 0eb6cfe
chore: clean up
chronark 972f8b0
chore: disable grafana and prometheus
chronark 35bde28
ci: delete unused tools
chronark b028e6b
chore: disable grafana and prometheus
chronark 65b1d6e
ci
chronark d575caa
ci
chronark 92fda9a
ci
chronark 117751e
ci
chronark 81999a7
ci
chronark 4bba3bb
ci
chronark bb27975
ci
chronark 214c61b
ci
chronark fa3684d
[autofix.ci] apply automated fixes
autofix-ci[bot] 24e7238
ci
chronark 8d82f65
Merge branch 'eng-1336-custom-override-permissions' of https://github…
chronark File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
apps/api/src/routes/v1_ratelimits_deleteOverride.error.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { expect, test } from "vitest"; | ||
|
|
||
| import { randomUUID } from "node:crypto"; | ||
| import { IntegrationHarness } from "src/pkg/testutil/integration-harness"; | ||
|
|
||
| import { schema } from "@unkey/db"; | ||
| import { newId } from "@unkey/id"; | ||
| import type { | ||
| V1RatelimitDeleteOverrideRequest, | ||
| V1RatelimitDeleteOverrideResponse, | ||
| } from "./v1_ratelimits_deleteOverride"; | ||
|
|
||
| test("Missing Namespace", async (t) => { | ||
| const h = await IntegrationHarness.init(t); | ||
|
|
||
| const overrideId = newId("test"); | ||
| const identifier = randomUUID(); | ||
| const namespaceId = newId("test"); | ||
| const namespace = { | ||
| id: namespaceId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| createdAt: new Date(), | ||
| name: newId("test"), | ||
| }; | ||
| await h.db.primary.insert(schema.ratelimitNamespaces).values(namespace); | ||
|
|
||
| await h.db.primary.insert(schema.ratelimitOverrides).values({ | ||
| id: overrideId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| namespaceId, | ||
| identifier, | ||
| limit: 1, | ||
| duration: 60_000, | ||
| async: false, | ||
| }); | ||
|
|
||
| const root = await h.createRootKey(["ratelimit.*.delete_override"]); | ||
| const res = await h.post<V1RatelimitDeleteOverrideRequest, V1RatelimitDeleteOverrideResponse>({ | ||
| url: "/v1/ratelimits.deleteOverride", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${root.key}`, | ||
| }, | ||
| body: { | ||
| identifier, | ||
| }, | ||
| }); | ||
|
|
||
| expect(res.status, `expected 400, received: ${JSON.stringify(res, null, 2)}`).toBe(400); | ||
| expect(res.body).toMatchObject({ | ||
| error: { | ||
| code: "BAD_REQUEST", | ||
| docs: "https://unkey.dev/docs/api-reference/errors/code/BAD_REQUEST", | ||
| message: "You must provide a namespaceId or a namespaceName", | ||
| }, | ||
| }); | ||
| }); |
56 changes: 56 additions & 0 deletions
56
apps/api/src/routes/v1_ratelimits_deleteOverride.happy.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { expect, test } from "vitest"; | ||
|
|
||
| import { randomUUID } from "node:crypto"; | ||
| import { IntegrationHarness } from "src/pkg/testutil/integration-harness"; | ||
|
|
||
| import { isNull, schema } from "@unkey/db"; | ||
| import { newId } from "@unkey/id"; | ||
| import type { | ||
| V1RatelimitDeleteOverrideRequest, | ||
| V1RatelimitDeleteOverrideResponse, | ||
| } from "./v1_ratelimits_deleteOverride"; | ||
|
|
||
| test("deletes override", async (t) => { | ||
| const h = await IntegrationHarness.init(t); | ||
|
|
||
| const overrideId = newId("test"); | ||
| const identifier = randomUUID(); | ||
| const namespaceId = newId("test"); | ||
| const namespace = { | ||
| id: namespaceId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| createdAt: new Date(), | ||
| name: newId("test"), | ||
| }; | ||
| await h.db.primary.insert(schema.ratelimitNamespaces).values(namespace); | ||
|
|
||
| await h.db.primary.insert(schema.ratelimitOverrides).values({ | ||
| id: overrideId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| namespaceId, | ||
| identifier, | ||
| limit: 1, | ||
| duration: 60_000, | ||
| async: false, | ||
| }); | ||
|
|
||
| const root = await h.createRootKey(["ratelimit.*.delete_override"]); | ||
| const res = await h.post<V1RatelimitDeleteOverrideRequest, V1RatelimitDeleteOverrideResponse>({ | ||
| url: "/v1/ratelimits.deleteOverride", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${root.key}`, | ||
| }, | ||
| body: { | ||
| namespaceId, | ||
| identifier, | ||
| }, | ||
| }); | ||
|
|
||
| expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200); | ||
|
|
||
| const found = await h.db.primary.query.ratelimitOverrides.findFirst({ | ||
| where: (table, { eq, and }) => and(eq(table.id, overrideId), isNull(table.deletedAt)), | ||
| }); | ||
| expect(found).toBeUndefined(); | ||
| }); |
158 changes: 158 additions & 0 deletions
158
apps/api/src/routes/v1_ratelimits_deleteOverride.security.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| import { randomUUID } from "node:crypto"; | ||
| import { runCommonRouteTests } from "@/pkg/testutil/common-tests"; | ||
| import { IntegrationHarness } from "@/pkg/testutil/integration-harness"; | ||
| import { schema } from "@unkey/db"; | ||
| import { newId } from "@unkey/id"; | ||
| import { describe, expect, test } from "vitest"; | ||
| import type { | ||
| V1RatelimitDeleteOverrideRequest, | ||
| V1RatelimitDeleteOverrideResponse, | ||
| } from "./v1_ratelimits_deleteOverride"; | ||
|
|
||
| runCommonRouteTests<V1RatelimitDeleteOverrideRequest>({ | ||
| prepareRequest: async (rh) => { | ||
| const overrideId = newId("test"); | ||
| const identifier = randomUUID(); | ||
| const namespaceId = newId("test"); | ||
| const namespace = { | ||
| id: namespaceId, | ||
| workspaceId: rh.resources.userWorkspace.id, | ||
| createdAt: new Date(), | ||
| name: newId("test"), | ||
| }; | ||
| await rh.db.primary.insert(schema.ratelimitNamespaces).values(namespace); | ||
| await rh.db.primary.insert(schema.ratelimitOverrides).values({ | ||
| id: overrideId, | ||
| workspaceId: rh.resources.userWorkspace.id, | ||
| namespaceId, | ||
| identifier, | ||
| limit: 1, | ||
| duration: 60_000, | ||
| async: false, | ||
| }); | ||
|
|
||
| return { | ||
| method: "POST", | ||
| url: "/v1/ratelimits.deleteOverride", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: { | ||
| namespaceId, | ||
| identifier, | ||
| }, | ||
| }; | ||
| }, | ||
| }); | ||
| describe("correct roles", () => { | ||
| describe.each([{ name: "delete override", roles: ["ratelimit.*.delete_override"] }])( | ||
| "$name", | ||
| ({ roles }) => { | ||
| test("returns 200", async (t) => { | ||
| const h = await IntegrationHarness.init(t); | ||
| const overrideId = newId("test"); | ||
| const identifier = randomUUID(); | ||
| const namespaceId = newId("test"); | ||
| const namespace = { | ||
| id: namespaceId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| createdAt: new Date(), | ||
| name: newId("test"), | ||
| }; | ||
| await h.db.primary.insert(schema.ratelimitNamespaces).values(namespace); | ||
| await h.db.primary.insert(schema.ratelimitOverrides).values({ | ||
| id: overrideId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| namespaceId, | ||
| identifier, | ||
| limit: 1, | ||
| duration: 60_000, | ||
| async: false, | ||
| }); | ||
| const root = await h.createRootKey(roles); | ||
|
|
||
| const res = await h.post< | ||
| V1RatelimitDeleteOverrideRequest, | ||
| V1RatelimitDeleteOverrideResponse | ||
| >({ | ||
| url: "/v1/ratelimits.deleteOverride", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${root.key}`, | ||
| }, | ||
| body: { | ||
| namespaceId, | ||
| identifier, | ||
| }, | ||
| }); | ||
|
|
||
| expect( | ||
| res.status, | ||
| `expected status 200, received: ${JSON.stringify(res, null, 2)}`, | ||
| ).toEqual(200); | ||
|
|
||
| const found = await h.db.primary.query.ratelimitOverrides.findFirst({ | ||
| where: (table, { eq, and, isNull }) => | ||
| and(isNull(table.deletedAt), eq(table.id, overrideId)), | ||
| }); | ||
| expect(found).toBeUndefined(); | ||
| }); | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| describe("incorrect roles", () => { | ||
| describe.each([{ name: "delete override", roles: ["ratelimit.*.create_override"] }])( | ||
| "$name", | ||
| ({ roles }) => { | ||
| test("returns 403", async (t) => { | ||
| const h = await IntegrationHarness.init(t); | ||
| const overrideId = newId("test"); | ||
| const identifier = randomUUID(); | ||
| const namespaceId = newId("test"); | ||
| const namespace = { | ||
| id: namespaceId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| createdAt: new Date(), | ||
| name: newId("test"), | ||
| }; | ||
| await h.db.primary.insert(schema.ratelimitNamespaces).values(namespace); | ||
| await h.db.primary.insert(schema.ratelimitOverrides).values({ | ||
| id: overrideId, | ||
| workspaceId: h.resources.userWorkspace.id, | ||
| namespaceId, | ||
| identifier, | ||
| limit: 1, | ||
| duration: 60_000, | ||
| async: false, | ||
| }); | ||
| const root = await h.createRootKey(roles); | ||
|
|
||
| const res = await h.post< | ||
| V1RatelimitDeleteOverrideRequest, | ||
| V1RatelimitDeleteOverrideResponse | ||
| >({ | ||
| url: "/v1/ratelimits.deleteOverride", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${root.key}`, | ||
| }, | ||
| body: { | ||
| namespaceId, | ||
| identifier, | ||
| }, | ||
| }); | ||
|
|
||
| expect( | ||
| res.status, | ||
| `expected status 403, received: ${JSON.stringify(res, null, 2)}`, | ||
| ).toEqual(403); | ||
|
|
||
| const found = await h.db.primary.query.ratelimitOverrides.findFirst({ | ||
| where: (table, { eq }) => eq(table.id, overrideId), | ||
| }); | ||
| expect(found?.id).toEqual(overrideId); | ||
| }); | ||
| }, | ||
| ); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve worker startup reliability.
The current approach of using
sleep 15has several issues:Consider implementing a healthcheck or polling mechanism instead.
Example implementation:
📝 Committable suggestion