From fb1fb200d477aae022e685ca6013492a26dd7f70 Mon Sep 17 00:00:00 2001 From: chronark Date: Thu, 5 Sep 2024 13:35:12 +0200 Subject: [PATCH] fix: updating ratelimits for identities --- .../cluster/docker/ratelimits_test.go | 2 +- .../pkg/api/errors/internal_server_error.go | 2 +- apps/agent/pkg/api/errors/validation_error.go | 2 +- apps/agent/pkg/api/register_routes.go | 2 + .../agent/pkg/api/routes/not_found/handler.go | 2 +- apps/agent/pkg/api/routes/openapi/handler.go | 21 ++ apps/agent/pkg/api/routes/sender.go | 2 +- .../pkg/api/routes/v1_liveness/handler.go | 2 +- .../api/routes/v1_liveness/handler_test.go | 2 +- .../v1_ratelimit_commitLease/handler.go | 2 +- .../v1_ratelimit_commitLease/handler_test.go | 2 +- .../v1_ratelimit_multiRatelimit/handler.go | 2 +- .../routes/v1_ratelimit_ratelimit/handler.go | 2 +- .../v1_ratelimit_ratelimit/handler_test.go | 2 +- .../api/routes/v1_vault_decrypt/handler.go | 2 +- .../api/routes/v1_vault_encrypt/handler.go | 2 +- .../routes/v1_vault_encrypt_bulk/handler.go | 2 +- apps/agent/pkg/api/server.go | 2 +- apps/agent/pkg/api/testutil/harness.go | 2 +- apps/agent/pkg/api/validation/validator.go | 12 +- apps/agent/pkg/openapi/config.yaml | 2 +- apps/agent/{gen => pkg}/openapi/gen.go | 0 apps/agent/pkg/openapi/spec.go | 11 + apps/agent/pkg/tinybird/tinybird.go | 2 +- apps/agent/schema.json | 13 +- apps/agent/services/eventrouter/service.go | 2 +- ...v1_identities_updateIdentity.error.test.ts | 69 +++++- ...v1_identities_updateIdentity.happy.test.ts | 48 +++++ .../routes/v1_identities_updateIdentity.ts | 203 ++++++++++++------ 29 files changed, 310 insertions(+), 109 deletions(-) create mode 100644 apps/agent/pkg/api/routes/openapi/handler.go rename apps/agent/{gen => pkg}/openapi/gen.go (100%) create mode 100644 apps/agent/pkg/openapi/spec.go diff --git a/apps/agent/integration/cluster/docker/ratelimits_test.go b/apps/agent/integration/cluster/docker/ratelimits_test.go index 79fa56599d..b732be6782 100644 --- a/apps/agent/integration/cluster/docker/ratelimits_test.go +++ b/apps/agent/integration/cluster/docker/ratelimits_test.go @@ -10,7 +10,7 @@ import ( "time" "github.com/stretchr/testify/require" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/uid" diff --git a/apps/agent/pkg/api/errors/internal_server_error.go b/apps/agent/pkg/api/errors/internal_server_error.go index d63281cf09..88f21bd9ff 100644 --- a/apps/agent/pkg/api/errors/internal_server_error.go +++ b/apps/agent/pkg/api/errors/internal_server_error.go @@ -5,8 +5,8 @@ import ( "net/http" "github.com/Southclaws/fault/fmsg" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/api/ctxutil" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) // HandleError takes in any unforseen error and returns a BaseError to be sent to the client diff --git a/apps/agent/pkg/api/errors/validation_error.go b/apps/agent/pkg/api/errors/validation_error.go index 0c44abd289..dbcbc64bb0 100644 --- a/apps/agent/pkg/api/errors/validation_error.go +++ b/apps/agent/pkg/api/errors/validation_error.go @@ -5,8 +5,8 @@ import ( "net/http" "github.com/Southclaws/fault/fmsg" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/api/ctxutil" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) func HandleValidationError(ctx context.Context, err error) openapi.ValidationError { diff --git a/apps/agent/pkg/api/register_routes.go b/apps/agent/pkg/api/register_routes.go index 6654a6d667..d004bbeda3 100644 --- a/apps/agent/pkg/api/register_routes.go +++ b/apps/agent/pkg/api/register_routes.go @@ -3,6 +3,7 @@ package api import ( "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" notFound "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/not_found" + openapi "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/openapi" v1Liveness "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_liveness" v1RatelimitCommitLease "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_ratelimit_commitLease" v1RatelimitMultiRatelimit "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_ratelimit_multiRatelimit" @@ -27,6 +28,7 @@ func (s *Server) RegisterRoutes() { staticBearerAuth := newBearerAuthMiddleware(s.authToken) v1Liveness.New(svc).Register(s.mux) + openapi.New(svc).Register(s.mux) v1RatelimitCommitLease.New(svc). WithMiddleware(staticBearerAuth). diff --git a/apps/agent/pkg/api/routes/not_found/handler.go b/apps/agent/pkg/api/routes/not_found/handler.go index 37945d5a1a..5fc75ca31d 100644 --- a/apps/agent/pkg/api/routes/not_found/handler.go +++ b/apps/agent/pkg/api/routes/not_found/handler.go @@ -3,9 +3,9 @@ package notFound import ( "net/http" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/api/ctxutil" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) // This is a hack, because / matches everything, so we need to make sure this is the last route diff --git a/apps/agent/pkg/api/routes/openapi/handler.go b/apps/agent/pkg/api/routes/openapi/handler.go new file mode 100644 index 0000000000..223f914eb6 --- /dev/null +++ b/apps/agent/pkg/api/routes/openapi/handler.go @@ -0,0 +1,21 @@ +package openapi + +import ( + "net/http" + + "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" +) + +func New(svc routes.Services) *routes.Route { + + return routes.NewRoute("GET", "/openapi.json", + func(w http.ResponseWriter, r *http.Request) { + + w.WriteHeader(200) + w.Header().Set("Content-Type", "application/json") + w.Write(openapi.Spec) + + }, + ) +} diff --git a/apps/agent/pkg/api/routes/sender.go b/apps/agent/pkg/api/routes/sender.go index 8ab0c43fc1..ae7fa9f1ce 100644 --- a/apps/agent/pkg/api/routes/sender.go +++ b/apps/agent/pkg/api/routes/sender.go @@ -5,9 +5,9 @@ import ( "encoding/json" "net/http" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/api/ctxutil" "github.com/unkeyed/unkey/apps/agent/pkg/logging" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) type Sender interface { diff --git a/apps/agent/pkg/api/routes/v1_liveness/handler.go b/apps/agent/pkg/api/routes/v1_liveness/handler.go index 82304a447b..1ed160a67a 100644 --- a/apps/agent/pkg/api/routes/v1_liveness/handler.go +++ b/apps/agent/pkg/api/routes/v1_liveness/handler.go @@ -3,8 +3,8 @@ package v1Liveness import ( "net/http" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) func New(svc routes.Services) *routes.Route { diff --git a/apps/agent/pkg/api/routes/v1_liveness/handler_test.go b/apps/agent/pkg/api/routes/v1_liveness/handler_test.go index 2ec47ff64d..1fe7333732 100644 --- a/apps/agent/pkg/api/routes/v1_liveness/handler_test.go +++ b/apps/agent/pkg/api/routes/v1_liveness/handler_test.go @@ -4,9 +4,9 @@ import ( "testing" "github.com/stretchr/testify/require" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" v1Liveness "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_liveness" "github.com/unkeyed/unkey/apps/agent/pkg/api/testutil" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) func TestLiveness(t *testing.T) { diff --git a/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler.go b/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler.go index fdf3865e28..5c10b090d4 100644 --- a/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler.go +++ b/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler.go @@ -6,10 +6,10 @@ import ( "github.com/Southclaws/fault" "github.com/Southclaws/fault/fmsg" "github.com/btcsuite/btcutil/base58" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" ratelimitv1 "github.com/unkeyed/unkey/apps/agent/gen/proto/ratelimit/v1" "github.com/unkeyed/unkey/apps/agent/pkg/api/errors" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "google.golang.org/protobuf/proto" ) diff --git a/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler_test.go b/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler_test.go index 0c022170c6..4da450f70a 100644 --- a/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler_test.go +++ b/apps/agent/pkg/api/routes/v1_ratelimit_commitLease/handler_test.go @@ -5,10 +5,10 @@ import ( "time" "github.com/stretchr/testify/require" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" v1RatelimitCommitLease "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_ratelimit_commitLease" v1RatelimitRatelimit "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit" "github.com/unkeyed/unkey/apps/agent/pkg/api/testutil" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/uid" "github.com/unkeyed/unkey/apps/agent/pkg/util" ) diff --git a/apps/agent/pkg/api/routes/v1_ratelimit_multiRatelimit/handler.go b/apps/agent/pkg/api/routes/v1_ratelimit_multiRatelimit/handler.go index 066a07a468..e5e8fc2b0c 100644 --- a/apps/agent/pkg/api/routes/v1_ratelimit_multiRatelimit/handler.go +++ b/apps/agent/pkg/api/routes/v1_ratelimit_multiRatelimit/handler.go @@ -3,10 +3,10 @@ package v1RatelimitMultiRatelimit import ( "net/http" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" ratelimitv1 "github.com/unkeyed/unkey/apps/agent/gen/proto/ratelimit/v1" "github.com/unkeyed/unkey/apps/agent/pkg/api/errors" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) func New(svc routes.Services) *routes.Route { diff --git a/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler.go b/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler.go index eb3fa105e8..dd1d4e2d70 100644 --- a/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler.go +++ b/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler.go @@ -4,10 +4,10 @@ import ( "net/http" "github.com/btcsuite/btcutil/base58" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" ratelimitv1 "github.com/unkeyed/unkey/apps/agent/gen/proto/ratelimit/v1" "github.com/unkeyed/unkey/apps/agent/pkg/api/errors" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/util" "google.golang.org/protobuf/proto" ) diff --git a/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler_test.go b/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler_test.go index 56f07682ce..1dcbb021d5 100644 --- a/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler_test.go +++ b/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit/handler_test.go @@ -5,9 +5,9 @@ import ( "time" "github.com/stretchr/testify/require" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" v1RatelimitRatelimit "github.com/unkeyed/unkey/apps/agent/pkg/api/routes/v1_ratelimit_ratelimit" "github.com/unkeyed/unkey/apps/agent/pkg/api/testutil" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/uid" ) diff --git a/apps/agent/pkg/api/routes/v1_vault_decrypt/handler.go b/apps/agent/pkg/api/routes/v1_vault_decrypt/handler.go index b4f34b2b10..15c06cddda 100644 --- a/apps/agent/pkg/api/routes/v1_vault_decrypt/handler.go +++ b/apps/agent/pkg/api/routes/v1_vault_decrypt/handler.go @@ -5,10 +5,10 @@ import ( "github.com/Southclaws/fault" "github.com/Southclaws/fault/fmsg" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" vaultv1 "github.com/unkeyed/unkey/apps/agent/gen/proto/vault/v1" "github.com/unkeyed/unkey/apps/agent/pkg/api/errors" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) func New(svc routes.Services) *routes.Route { diff --git a/apps/agent/pkg/api/routes/v1_vault_encrypt/handler.go b/apps/agent/pkg/api/routes/v1_vault_encrypt/handler.go index 694a9b64bf..985bdf090e 100644 --- a/apps/agent/pkg/api/routes/v1_vault_encrypt/handler.go +++ b/apps/agent/pkg/api/routes/v1_vault_encrypt/handler.go @@ -3,10 +3,10 @@ package v1VaultEncrypt import ( "net/http" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" vaultv1 "github.com/unkeyed/unkey/apps/agent/gen/proto/vault/v1" "github.com/unkeyed/unkey/apps/agent/pkg/api/errors" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) func New(svc routes.Services) *routes.Route { diff --git a/apps/agent/pkg/api/routes/v1_vault_encrypt_bulk/handler.go b/apps/agent/pkg/api/routes/v1_vault_encrypt_bulk/handler.go index 1159d10ebe..f71bcb3178 100644 --- a/apps/agent/pkg/api/routes/v1_vault_encrypt_bulk/handler.go +++ b/apps/agent/pkg/api/routes/v1_vault_encrypt_bulk/handler.go @@ -5,10 +5,10 @@ import ( "github.com/Southclaws/fault" "github.com/Southclaws/fault/fmsg" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" vaultv1 "github.com/unkeyed/unkey/apps/agent/gen/proto/vault/v1" "github.com/unkeyed/unkey/apps/agent/pkg/api/errors" "github.com/unkeyed/unkey/apps/agent/pkg/api/routes" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) type Request = openapi.V1EncryptBulkRequestBody diff --git a/apps/agent/pkg/api/server.go b/apps/agent/pkg/api/server.go index 3f3d016a75..d113431aab 100644 --- a/apps/agent/pkg/api/server.go +++ b/apps/agent/pkg/api/server.go @@ -83,7 +83,7 @@ func New(config Config) (*Server, error) { // // validationMiddleware, // ) // s.app.Use(tracingMiddleware) - v, err := validation.New("./pkg/openapi/openapi.json") + v, err := validation.New() if err != nil { return nil, err } diff --git a/apps/agent/pkg/api/testutil/harness.go b/apps/agent/pkg/api/testutil/harness.go index b70b45e2b4..62cd2d3a4c 100644 --- a/apps/agent/pkg/api/testutil/harness.go +++ b/apps/agent/pkg/api/testutil/harness.go @@ -81,7 +81,7 @@ func (h *Harness) Register(route *routes.Route) { func (h *Harness) SetupRoute(constructor func(svc routes.Services) *routes.Route) *routes.Route { - validator, err := validation.New("./pkg/openapi/openapi.json") + validator, err := validation.New() require.NoError(h.t, err) route := constructor(routes.Services{ Logger: h.logger, diff --git a/apps/agent/pkg/api/validation/validator.go b/apps/agent/pkg/api/validation/validator.go index bf3834f3c6..38eb17755e 100644 --- a/apps/agent/pkg/api/validation/validator.go +++ b/apps/agent/pkg/api/validation/validator.go @@ -6,14 +6,13 @@ import ( "fmt" "io" "net/http" - "os" "github.com/Southclaws/fault" "github.com/Southclaws/fault/fmsg" "github.com/pb33f/libopenapi" validator "github.com/pb33f/libopenapi-validator" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/api/ctxutil" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/util" ) @@ -25,12 +24,9 @@ type Validator struct { validator validator.Validator } -func New(specPath string) (*Validator, error) { - b, err := os.ReadFile(specPath) - if err != nil { - return nil, fault.Wrap(err, fmsg.With("failed to read spec file")) - } - document, err := libopenapi.NewDocument(b) +func New() (*Validator, error) { + + document, err := libopenapi.NewDocument(openapi.Spec) if err != nil { return nil, fault.Wrap(err, fmsg.With("failed to create OpenAPI document")) } diff --git a/apps/agent/pkg/openapi/config.yaml b/apps/agent/pkg/openapi/config.yaml index 855596b3ad..c2959e8d1e 100644 --- a/apps/agent/pkg/openapi/config.yaml +++ b/apps/agent/pkg/openapi/config.yaml @@ -1,5 +1,5 @@ package: openapi -output: ./gen/openapi/gen.go +output: ./pkg/openapi/gen.go generate: models: true output-options: diff --git a/apps/agent/gen/openapi/gen.go b/apps/agent/pkg/openapi/gen.go similarity index 100% rename from apps/agent/gen/openapi/gen.go rename to apps/agent/pkg/openapi/gen.go diff --git a/apps/agent/pkg/openapi/spec.go b/apps/agent/pkg/openapi/spec.go new file mode 100644 index 0000000000..77cf03690e --- /dev/null +++ b/apps/agent/pkg/openapi/spec.go @@ -0,0 +1,11 @@ +package openapi + +import ( + _ "embed" +) + +// Spec is the OpenAPI specification for the service +// It's loaded from our openapi file and embedded into the binary +// +//go:embed openapi.json +var Spec []byte diff --git a/apps/agent/pkg/tinybird/tinybird.go b/apps/agent/pkg/tinybird/tinybird.go index a8b11a38d4..c6ffa82463 100644 --- a/apps/agent/pkg/tinybird/tinybird.go +++ b/apps/agent/pkg/tinybird/tinybird.go @@ -8,7 +8,7 @@ import ( "net/http" "strings" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" ) type Client struct { diff --git a/apps/agent/schema.json b/apps/agent/schema.json index e8974390d0..c5a6c5b831 100644 --- a/apps/agent/schema.json +++ b/apps/agent/schema.json @@ -13,19 +13,13 @@ "clickhouse": { "type": "object", "properties": { - "addr": { + "url": { "type": "string", "minLength": 1 - }, - "password": { - "type": "string" - }, - "username": { - "type": "string" } }, "additionalProperties": false, - "required": ["addr", "username", "password"] + "required": ["url"] }, "cluster": { "type": "object", @@ -282,7 +276,8 @@ "required": ["s3Bucket", "s3Url", "s3AccessKeyId", "s3AccessKeySecret", "masterKeys"] } }, - "additionalProperties": false + "additionalProperties": false, + "required": ["vault"] }, "tracing": { "type": "object", diff --git a/apps/agent/services/eventrouter/service.go b/apps/agent/services/eventrouter/service.go index 0a7e13cadd..afd8c0fb3b 100644 --- a/apps/agent/services/eventrouter/service.go +++ b/apps/agent/services/eventrouter/service.go @@ -8,13 +8,13 @@ import ( "net/http" "time" - "github.com/unkeyed/unkey/apps/agent/gen/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/auth" "github.com/unkeyed/unkey/apps/agent/pkg/batch" "github.com/unkeyed/unkey/apps/agent/pkg/clickhouse" "github.com/unkeyed/unkey/apps/agent/pkg/clickhouse/schema" "github.com/unkeyed/unkey/apps/agent/pkg/logging" "github.com/unkeyed/unkey/apps/agent/pkg/metrics" + "github.com/unkeyed/unkey/apps/agent/pkg/openapi" "github.com/unkeyed/unkey/apps/agent/pkg/prometheus" "github.com/unkeyed/unkey/apps/agent/pkg/tinybird" "github.com/unkeyed/unkey/apps/agent/pkg/tracing" diff --git a/apps/api/src/routes/v1_identities_updateIdentity.error.test.ts b/apps/api/src/routes/v1_identities_updateIdentity.error.test.ts index 14938d3a50..d60fbd6fd7 100644 --- a/apps/api/src/routes/v1_identities_updateIdentity.error.test.ts +++ b/apps/api/src/routes/v1_identities_updateIdentity.error.test.ts @@ -1,4 +1,4 @@ -import { expect, test } from "vitest"; +import { describe, expect, test } from "vitest"; import { IntegrationHarness } from "src/pkg/testutil/integration-harness"; @@ -40,3 +40,70 @@ test("empty identityId", async (t) => { }, }); }); + +describe("updating ratelimits", () => { + test("duplicate ratelimits return an error", async (t) => { + const h = await IntegrationHarness.init(t); + const { key: rootKey } = await h.createRootKey(["identity.*.update_identity"]); + + const identity = { + id: newId("test"), + workspaceId: h.resources.userWorkspace.id, + externalId: randomUUID(), + }; + + await h.db.primary.insert(schema.identities).values(identity); + + const ratelimits = [ + { + id: newId("test"), + workspaceId: h.resources.userWorkspace.id, + name: "a", + limit: 10, + duration: 1000, + }, + { + id: newId("test"), + workspaceId: h.resources.userWorkspace.id, + name: "b", + limit: 10, + duration: 1000, + }, + ]; + await h.db.primary.insert(schema.ratelimits).values(ratelimits); + + const res = await h.post( + { + url: "/v1/identities.updateIdentity", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${rootKey}`, + }, + body: { + identityId: identity.id, + ratelimits: [ + { + name: "a", + limit: 10, + duration: 20000, + }, + { + name: "a", + limit: 10, + duration: 124124, + }, + ], + }, + }, + ); + + expect(res.status).toEqual(412); + expect(res.body).toMatchObject({ + error: { + code: "PRECONDITION_FAILED", + docs: "https://unkey.dev/docs/api-reference/errors/code/PRECONDITION_FAILED", + message: "ratelimit names must be unique", + }, + }); + }); +}); diff --git a/apps/api/src/routes/v1_identities_updateIdentity.happy.test.ts b/apps/api/src/routes/v1_identities_updateIdentity.happy.test.ts index d08595ca21..88260819a1 100644 --- a/apps/api/src/routes/v1_identities_updateIdentity.happy.test.ts +++ b/apps/api/src/routes/v1_identities_updateIdentity.happy.test.ts @@ -132,6 +132,54 @@ test("sets new ratelimits", async (t) => { } }); +test("sets the same ratelimits again", async (t) => { + const h = await IntegrationHarness.init(t); + const root = await h.createRootKey(["identity.*.update_identity"]); + + const identity = { + id: newId("test"), + workspaceId: h.resources.userWorkspace.id, + externalId: randomUUID(), + }; + + await h.db.primary.insert(schema.identities).values(identity); + + const ratelimits = new Array(6).fill(null).map((_, i) => ({ + name: randomUUID(), + limit: 10 * (i + 1), + duration: 1000 * (i + 1), + })); + + for (let i = 0; i < 10; i++) { + const res = await h.post( + { + url: "/v1/identities.updateIdentity", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${root.key}`, + }, + body: { + identityId: identity.id, + ratelimits: ratelimits, + }, + }, + ); + + expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200); + + const found = await h.db.primary.query.ratelimits.findMany({ + where: (table, { eq }) => eq(table.identityId, identity.id), + }); + + expect(found.length).toBe(ratelimits.length); + for (const rl of ratelimits) { + expect( + found.some((f) => f.name === rl.name && f.limit === rl.limit && f.duration === rl.duration), + ); + } + } +}); + test("works with thousands of keys", { timeout: 300_000 }, async (t) => { const h = await IntegrationHarness.init(t); const root = await h.createRootKey(["identity.*.update_identity"]); diff --git a/apps/api/src/routes/v1_identities_updateIdentity.ts b/apps/api/src/routes/v1_identities_updateIdentity.ts index c1f29111d2..0a6bc5228e 100644 --- a/apps/api/src/routes/v1_identities_updateIdentity.ts +++ b/apps/api/src/routes/v1_identities_updateIdentity.ts @@ -4,7 +4,7 @@ import { createRoute, z } from "@hono/zod-openapi"; import type { UnkeyAuditLog } from "@/pkg/analytics"; import { rootKeyAuth } from "@/pkg/auth/root_key"; import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors"; -import { eq, schema } from "@unkey/db"; +import { type Ratelimit, eq, schema } from "@unkey/db"; import { newId } from "@unkey/id"; import { buildUnkeyQuery } from "@unkey/rbac"; @@ -148,15 +148,15 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => } if (req.ratelimits && req.ratelimits.length >= 2) { - const ratelimitNames = new Set(); - for (const rl of req.ratelimits) { - ratelimitNames.add(rl.name); - } - if (ratelimitNames.size !== req.ratelimits.length) { - throw new UnkeyApiError({ - code: "PRECONDITION_FAILED", - message: "ratelimit names must be unique", - }); + const uniqueNames = new Set(); + for (const { name } of req.ratelimits) { + if (uniqueNames.has(name)) { + throw new UnkeyApiError({ + code: "PRECONDITION_FAILED", + message: "ratelimit names must be unique", + }); + } + uniqueNames.add(name); } } @@ -198,6 +198,27 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => }); } + auditLogs.push({ + workspaceId: auth.authorizedWorkspaceId, + event: "identity.update", + actor: { + type: "key", + id: auth.key.id, + }, + description: `Updated ${identity.id}`, + resources: [ + { + type: "identity", + id: identity.id, + }, + ], + + context: { + location: c.get("location"), + userAgent: c.get("userAgent"), + }, + }); + if (typeof req.meta !== "undefined") { await tx .update(schema.identities) @@ -208,11 +229,33 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => } if (typeof req.ratelimits !== "undefined") { - if (identity.ratelimits.length > 0) { - await tx.delete(schema.ratelimits).where(eq(schema.ratelimits.identityId, identity.id)); + const deleteRatelimits: Ratelimit[] = []; + const createRatelimits: Required = []; + const updateRatelimits: Ratelimit[] = []; + for (const rl of identity.ratelimits) { + const newRl = req.ratelimits.find((r) => r.name === rl.name); + if (!newRl) { + deleteRatelimits.push(rl); + } else { + updateRatelimits.push({ + ...rl, + limit: newRl.limit, + duration: newRl.duration, + }); + } + } + for (const newRl of req.ratelimits) { + if (!identity.ratelimits.find((r) => r.name === newRl.name)) { + createRatelimits.push(newRl); + } } - for (const rl of identity.ratelimits) { + /** + * Delete undesired ratelimits + */ + + for (const rl of deleteRatelimits) { + await tx.delete(schema.ratelimits).where(eq(schema.ratelimits.id, rl.id)); auditLogs.push({ workspaceId: auth.authorizedWorkspaceId, event: "ratelimit.delete" as const, @@ -229,6 +272,7 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => { type: "ratelimit" as const, id: rl.id, + meta: rl, }, ], context: { @@ -238,45 +282,86 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => }); } - if (req.ratelimits.length > 0) { - const newRatelimits = req.ratelimits.map((r) => ({ - id: newId("ratelimit"), + /** + * Update existing + */ + + for (const rl of updateRatelimits) { + await tx + .update(schema.ratelimits) + .set({ + name: rl.name, + limit: rl.limit, + duration: rl.duration, + }) + .where(eq(schema.ratelimits.id, rl.id)); + auditLogs.push({ workspaceId: auth.authorizedWorkspaceId, - identityId: identity.id, - name: r.name, - limit: r.limit, - duration: r.duration, - })); - await tx.insert(schema.ratelimits).values(newRatelimits); - for (const rl of newRatelimits) { - auditLogs.push({ - workspaceId: auth.authorizedWorkspaceId, - event: "ratelimit.create" as const, - actor: { - type: "key" as const, - id: auth.key.id, + event: "ratelimit.update" as const, + actor: { + type: "key" as const, + id: auth.key.id, + }, + description: `Updated ${rl.id}`, + resources: [ + { + type: "identity" as const, + id: identity.id, }, - description: `Created ${rl.id}`, - resources: [ - { - type: "identity" as const, - id: identity.id, - }, - { - type: "ratelimit" as const, - id: rl.id, - }, - ], + { + type: "ratelimit" as const, + id: rl.id, + meta: rl, + }, + ], + context: { + location: c.get("location"), + userAgent: c.get("userAgent"), + }, + }); + } + + /** + * Create new + */ + + for (const rl of createRatelimits) { + const ratelimitId = newId("ratelimit"); + await tx.insert(schema.ratelimits).values({ + id: ratelimitId, + workspaceId: identity.workspaceId, + identityId: identity.id, + name: rl.name, + limit: rl.limit, + duration: rl.duration, + }); + auditLogs.push({ + workspaceId: auth.authorizedWorkspaceId, + event: "ratelimit.create" as const, + actor: { + type: "key" as const, + id: auth.key.id, + }, - context: { - location: c.get("location"), - userAgent: c.get("userAgent"), + description: `Created ${ratelimitId}`, + resources: [ + { + type: "identity" as const, + id: identity.id, }, - }); - } + { + type: "ratelimit" as const, + id: ratelimitId, + meta: rl, + }, + ], + context: { + location: c.get("location"), + userAgent: c.get("userAgent"), + }, + }); } } - const identityAfterUpdate = await tx.query.identities.findFirst({ where: (table, { eq }) => eq(table.id, identity.id), with: { @@ -300,31 +385,7 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => return identityAfterUpdate!; }); - c.executionCtx.waitUntil( - analytics.ingestUnkeyAuditLogs([ - { - workspaceId: auth.authorizedWorkspaceId, - event: "identity.update", - actor: { - type: "key", - id: auth.key.id, - }, - description: `Updated ${identity.id}`, - resources: [ - { - type: "identity", - id: identity.id, - }, - ], - - context: { - location: c.get("location"), - userAgent: c.get("userAgent"), - }, - }, - ...auditLogs, - ]), - ); + c.executionCtx.waitUntil(analytics.ingestUnkeyAuditLogs(auditLogs)); return c.json({ id: identity.id,