From 95e6112fd6290568f1c29030c0192f2ef57bb77e Mon Sep 17 00:00:00 2001 From: Andreas Thomas Date: Fri, 29 Nov 2024 16:19:23 +0100 Subject: [PATCH] fix: updating name does not affect ratelimits (#2693) * fix: updating name does not affect ratelimits * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .../routes/v1_keys_updateKey.happy.test.ts | 70 +++++++++++ apps/api/src/routes/v1_keys_updateKey.ts | 109 +++++++++++------- 2 files changed, 135 insertions(+), 44 deletions(-) diff --git a/apps/api/src/routes/v1_keys_updateKey.happy.test.ts b/apps/api/src/routes/v1_keys_updateKey.happy.test.ts index f51d034a19..9387711286 100644 --- a/apps/api/src/routes/v1_keys_updateKey.happy.test.ts +++ b/apps/api/src/routes/v1_keys_updateKey.happy.test.ts @@ -1058,3 +1058,73 @@ describe("When refillDay is omitted.", () => { expect(found?.refillDay).toEqual(1); }); }); + +describe("update name", () => { + test("should not affect ratelimit config", async (t) => { + const h = await IntegrationHarness.init(t); + + const root = await h.createRootKey([ + `api.${h.resources.userApi.id}.create_key`, + `api.${h.resources.userApi.id}.update_key`, + ]); + + const key = await h.post({ + url: "/v1/keys.createKey", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${root.key}`, + }, + body: { + apiId: h.resources.userApi.id, + prefix: "prefix", + name: "my key", + ratelimit: { + async: true, + limit: 10, + duration: 1000, + }, + }, + }); + + expect(key.status).toBe(200); + + const update = await h.post({ + url: "/v1/keys.updateKey", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${root.key}`, + }, + body: { + keyId: key.body.keyId, + name: "changed", + }, + }); + + expect(update.status).toBe(200); + + const found = await h.db.primary.query.keys.findFirst({ + where: (table, { eq }) => eq(table.id, key.body.keyId), + }); + + expect(found).toBeDefined(); + expect(found!.ratelimitAsync).toEqual(true); + expect(found!.ratelimitLimit).toEqual(10); + expect(found!.ratelimitDuration).toEqual(1000); + + const verify = await h.post({ + url: "/v1/keys.verifyKey", + headers: { + "Content-Type": "application/json", + }, + body: { + apiId: h.resources.userApi.id, + key: key.body.key, + }, + }); + + expect(verify.status, `expected 200, received: ${JSON.stringify(verify)}`).toBe(200); + expect(verify.body.ratelimit).toBeDefined(); + expect(verify.body.ratelimit!.limit).toBe(10); + expect(verify.body.ratelimit!.remaining).toBe(9); + }); +}); diff --git a/apps/api/src/routes/v1_keys_updateKey.ts b/apps/api/src/routes/v1_keys_updateKey.ts index a6bfe48bac..d12c521896 100644 --- a/apps/api/src/routes/v1_keys_updateKey.ts +++ b/apps/api/src/routes/v1_keys_updateKey.ts @@ -4,7 +4,7 @@ import { createRoute, z } from "@hono/zod-openapi"; import { insertUnkeyAuditLog } from "@/pkg/audit"; import { rootKeyAuth } from "@/pkg/auth/root_key"; import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors"; -import { schema } from "@unkey/db"; +import { type Key, schema } from "@unkey/db"; import { eq } from "@unkey/db"; import { buildUnkeyQuery } from "@unkey/rbac"; import { upsertIdentity } from "./v1_keys_createKey"; @@ -340,50 +340,71 @@ export const registerV1KeysUpdate = (app: App) => const authorizedWorkspaceId = auth.authorizedWorkspaceId; const rootKeyId = auth.key.id; - const externalId = typeof req.externalId !== "undefined" ? req.externalId : req.ownerId; - const identityId = - typeof externalId === "undefined" - ? undefined - : externalId === null - ? null - : (await upsertIdentity(db.primary, authorizedWorkspaceId, externalId)).id; - - await db.primary - .update(schema.keys) - .set({ - name: req.name, - ownerId: req.ownerId, - meta: typeof req.meta === "undefined" ? undefined : JSON.stringify(req.meta ?? {}), - identityId, - expires: - typeof req.expires === "undefined" - ? undefined - : req.expires === null - ? null - : new Date(req.expires), - remaining: req.remaining, - ratelimitAsync: - req.ratelimit === null - ? null - : typeof req.ratelimit === "undefined" - ? undefined - : typeof req.ratelimit.async === "boolean" - ? req.ratelimit.async - : req.ratelimit?.type === "fast", - ratelimitLimit: - req.ratelimit === null ? null : req.ratelimit?.limit ?? req.ratelimit?.refillRate ?? null, - ratelimitDuration: - req.ratelimit === null - ? null - : req.ratelimit?.duration ?? req.ratelimit?.refillInterval ?? null, - refillInterval: req.refill === null ? null : req.refill?.interval, - refillAmount: req.refill === null ? null : req.refill?.amount, - refillDay: req.refill?.interval === "daily" ? null : req?.refill?.refillDay ?? 1, - lastRefillAt: req.refill == null || req.refill?.amount == null ? null : new Date(), - enabled: req.enabled, - }) - .where(eq(schema.keys.id, key.id)); + const changes: Partial = {}; + if (typeof req.name !== "undefined") { + changes.name = req.name; + } + if (typeof req.meta !== "undefined") { + changes.meta = req.meta === null ? null : JSON.stringify(req.meta); + } + if (typeof req.externalId !== "undefined") { + if (req.externalId === null) { + changes.identityId = null; + changes.ownerId = null; + } else { + const identity = await upsertIdentity(db.primary, authorizedWorkspaceId, req.externalId); + changes.identityId = identity.id; + changes.ownerId = req.externalId; + } + } else if (typeof req.ownerId !== "undefined") { + if (req.ownerId === null) { + changes.identityId = null; + changes.ownerId = null; + } else { + const identity = await upsertIdentity(db.primary, authorizedWorkspaceId, req.ownerId); + changes.identityId = identity.id; + changes.ownerId = req.ownerId; + } + } + if (typeof req.expires !== "undefined") { + changes.expires = req.expires === null ? null : new Date(req.expires); + } + if (typeof req.remaining !== "undefined") { + changes.remaining = req.remaining; + } + if (typeof req.ratelimit !== "undefined") { + if (req.ratelimit === null) { + changes.ratelimitAsync = null; + changes.ratelimitLimit = null; + changes.ratelimitDuration = null; + } else { + changes.ratelimitAsync = + typeof req.ratelimit.async === "boolean" + ? req.ratelimit.async + : req.ratelimit.type === "fast"; + changes.ratelimitLimit = req.ratelimit.limit ?? req.ratelimit.refillRate; + changes.ratelimitDuration = req.ratelimit.duration ?? req.ratelimit.refillInterval; + } + } + if (typeof req.refill !== "undefined") { + if (req.refill === null) { + changes.refillInterval = null; + changes.refillAmount = null; + changes.refillDay = null; + changes.lastRefillAt = null; + } else { + changes.refillInterval = req.refill.interval; + changes.refillAmount = req.refill.amount; + changes.refillDay = req.refill.refillDay ?? 1; + } + } + if (typeof req.enabled !== "undefined") { + changes.enabled = req.enabled; + } + if (Object.keys(changes).length > 0) { + await db.primary.update(schema.keys).set(changes).where(eq(schema.keys.id, key.id)); + } await insertUnkeyAuditLog(c, undefined, { workspaceId: authorizedWorkspaceId, event: "key.update",