diff --git a/apps/api/src/pkg/errors/http.ts b/apps/api/src/pkg/errors/http.ts index 965a3fdd33..6c63aa9408 100644 --- a/apps/api/src/pkg/errors/http.ts +++ b/apps/api/src/pkg/errors/http.ts @@ -13,7 +13,7 @@ const ErrorCode = z.enum([ "USAGE_EXCEEDED", "DISABLED", "NOT_FOUND", - "NOT_UNIQUE", + "CONFLICT", "RATE_LIMITED", "UNAUTHORIZED", "PRECONDITION_FAILED", @@ -80,7 +80,7 @@ function codeToStatus(code: z.infer): StatusCode { return 404; case "METHOD_NOT_ALLOWED": return 405; - case "NOT_UNIQUE": + case "CONFLICT": return 409; case "DELETE_PROTECTED": case "PRECONDITION_FAILED": diff --git a/apps/api/src/pkg/errors/openapi_responses.ts b/apps/api/src/pkg/errors/openapi_responses.ts index 0afa28520b..943469e63e 100644 --- a/apps/api/src/pkg/errors/openapi_responses.ts +++ b/apps/api/src/pkg/errors/openapi_responses.ts @@ -46,6 +46,17 @@ export const openApiErrorResponses = { }, }, }, + 412: { + description: + "The requested operation cannot be completed because certain conditions were not met. This typically occurs when a required resource state or version check fails.", + content: { + "application/json": { + schema: errorSchemaFactory(z.enum(["PRECONDITION_FAILED"])).openapi( + "ErrPreconditionFailed", + ), + }, + }, + }, 429: { description: `The user has sent too many requests in a given amount of time ("rate limiting")`, content: { diff --git a/apps/api/src/routes/v1_identities_createIdentity.error.test.ts b/apps/api/src/routes/v1_identities_createIdentity.error.test.ts index f5cd8defee..083aecff83 100644 --- a/apps/api/src/routes/v1_identities_createIdentity.error.test.ts +++ b/apps/api/src/routes/v1_identities_createIdentity.error.test.ts @@ -62,12 +62,12 @@ describe("when identity exists already", () => { }, }); - expect(res.status).toEqual(412); + expect(res.status, `expected 409, received: ${JSON.stringify(res, null, 2)}`).toEqual(409); expect(res.body).toMatchObject({ error: { - code: "PRECONDITION_FAILED", - docs: "https://unkey.dev/docs/api-reference/errors/code/PRECONDITION_FAILED", - message: "Duplicate identity", + code: "CONFLICT", + docs: "https://unkey.dev/docs/api-reference/errors/code/CONFLICT", + message: `Identity with externalId "${externalId}" already exists in this workspace`, }, }); }); diff --git a/apps/api/src/routes/v1_identities_createIdentity.ts b/apps/api/src/routes/v1_identities_createIdentity.ts index 0c405fc96a..8aac417b7a 100644 --- a/apps/api/src/routes/v1_identities_createIdentity.ts +++ b/apps/api/src/routes/v1_identities_createIdentity.ts @@ -30,7 +30,7 @@ const route = createRoute({ This usually comes from your authentication provider and could be a userId, organisationId or even an email. It does not matter what you use, as long as it uniquely identifies something in your application. -\`externalId\`s are unique across your workspace and therefore a \`PRECONDITION_FAILED\` error is returned when you try to create duplicates. +\`externalId\`s are unique across your workspace and therefore a \`CONFLICT\` error is returned when you try to create duplicates. `, example: "user_123", }), @@ -138,10 +138,12 @@ export const registerV1IdentitiesCreateIdentity = (app: App) => .catch((e) => { if (e instanceof DatabaseError && e.body.message.includes("Duplicate entry")) { throw new UnkeyApiError({ - code: "PRECONDITION_FAILED", - message: "Duplicate identity", + code: "CONFLICT", + message: `Identity with externalId "${identity.externalId}" already exists in this workspace`, }); } + + throw e; }); const ratelimits = req.ratelimits @@ -199,7 +201,10 @@ export const registerV1IdentitiesCreateIdentity = (app: App) => }, ], - context: { location: c.get("location"), userAgent: c.get("userAgent") }, + context: { + location: c.get("location"), + userAgent: c.get("userAgent"), + }, })), ]); }) 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 d60fbd6fd7..8a367ea638 100644 --- a/apps/api/src/routes/v1_identities_updateIdentity.error.test.ts +++ b/apps/api/src/routes/v1_identities_updateIdentity.error.test.ts @@ -72,6 +72,8 @@ describe("updating ratelimits", () => { ]; await h.db.primary.insert(schema.ratelimits).values(ratelimits); + const name = "requests"; + const res = await h.post( { url: "/v1/identities.updateIdentity", @@ -83,12 +85,12 @@ describe("updating ratelimits", () => { identityId: identity.id, ratelimits: [ { - name: "a", + name, limit: 10, duration: 20000, }, { - name: "a", + name, limit: 10, duration: 124124, }, @@ -97,12 +99,12 @@ describe("updating ratelimits", () => { }, ); - expect(res.status).toEqual(412); + expect(res.status, `expected 409, received: ${JSON.stringify(res, null, 2)}`).toEqual(409); 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", + code: "CONFLICT", + docs: "https://unkey.dev/docs/api-reference/errors/code/CONFLICT", + message: `Ratelimit with name "${name}" is already defined in the request`, }, }); }); diff --git a/apps/api/src/routes/v1_identities_updateIdentity.ts b/apps/api/src/routes/v1_identities_updateIdentity.ts index 7101f1b896..8a11a40f34 100644 --- a/apps/api/src/routes/v1_identities_updateIdentity.ts +++ b/apps/api/src/routes/v1_identities_updateIdentity.ts @@ -152,8 +152,8 @@ export const registerV1IdentitiesUpdateIdentity = (app: App) => for (const { name } of req.ratelimits) { if (uniqueNames.has(name)) { throw new UnkeyApiError({ - code: "PRECONDITION_FAILED", - message: "ratelimit names must be unique", + code: "CONFLICT", + message: `Ratelimit with name "${name}" is already defined in the request`, }); } uniqueNames.add(name); diff --git a/apps/api/src/routes/v1_keys_ami.ts b/apps/api/src/routes/v1_keys_ami.ts deleted file mode 100644 index a32b4172d4..0000000000 --- a/apps/api/src/routes/v1_keys_ami.ts +++ /dev/null @@ -1,174 +0,0 @@ -import type { App } from "@/pkg/hono/app"; -import { createRoute, z } from "@hono/zod-openapi"; - -import { rootKeyAuth } from "@/pkg/auth/root_key"; -import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors"; -import { sha256 } from "@unkey/hash"; -import { buildUnkeyQuery } from "@unkey/rbac"; - -const route = createRoute({ - tags: ["keys"], - operationId: "whoami", - method: "post", - path: "/v1/keys.whoami", - security: [{ bearerAuth: [] }], - request: { - body: { - required: true, - content: { - "application/json": { - schema: z.object({ - key: z.string().min(1).openapi({ - description: "The actual key to fetch", - example: "sk_123", - }), - }), - }, - }, - }, - }, - - responses: { - 200: { - description: "The configuration for a single key", - content: { - "application/json": { - schema: z.object({ - id: z.string().openapi({ - description: "The ID of the key", - example: "key_123", - }), - name: z.string().optional().openapi({ - description: "The name of the key", - example: "API Key 1", - }), - remaining: z.number().int().optional().openapi({ - description: "The remaining number of requests for the key", - example: 1000, - }), - identity: z - .object({ - id: z.string().openapi({ - description: "The identity ID associated with the key", - example: "id_123", - }), - externalId: z.string().openapi({ - description: "The external identity ID associated with the key", - example: "ext123", - }), - }) - .optional() - .openapi({ - description: "The identity object associated with the key", - }), - meta: z - .record(z.unknown()) - .optional() - .openapi({ - description: "Metadata associated with the key", - example: { role: "admin", plan: "premium" }, - }), - createdAt: z.number().int().openapi({ - description: "The timestamp in milliseconds when the key was created", - example: 1620000000000, - }), - enabled: z.boolean().openapi({ - description: "Whether the key is enabled", - example: true, - }), - environment: z.string().optional().openapi({ - description: "The environment the key is associated with", - example: "production", - }), - }), - }, - }, - }, - ...openApiErrorResponses, - }, -}); - -export type Route = typeof route; -export type V1KeysWhoAmIRequest = z.infer< - (typeof route.request.body.content)["application/json"]["schema"] ->; -export type V1KeysWhoAmIResponse = z.infer< - (typeof route.responses)[200]["content"]["application/json"]["schema"] ->; - -export const registerV1KeysWhoAmI = (app: App) => - app.openapi(route, async (c) => { - const { key: secret } = c.req.valid("json"); - const { cache, db } = c.get("services"); - const hash = await sha256(secret); - const { val: data, err } = await cache.keyByHash.swr(hash, async () => { - const dbRes = await db.readonly.query.keys.findFirst({ - where: (table, { eq, and, isNull }) => and(eq(table.hash, hash), isNull(table.deletedAt)), - with: { - keyAuth: { - with: { - api: true, - }, - }, - identity: true, - }, - }); - - if (!dbRes) { - return null; - } - - return { - key: { - ...dbRes, - }, - api: dbRes.keyAuth.api, - identity: dbRes.identity, - } as any; // this was necessary so that we don't need to return the workspace and other types defined in keyByHash - }); - - if (err) { - throw new UnkeyApiError({ - code: "INTERNAL_SERVER_ERROR", - message: `unable to load key: ${err.message}`, - }); - } - if (!data) { - throw new UnkeyApiError({ - code: "NOT_FOUND", - message: "Key not found", - }); - } - const { api, key } = data; - const auth = await rootKeyAuth( - c, - buildUnkeyQuery(({ or }) => or("*", "api.*.read_key", `api.${api.id}.read_key`)), - ); - - if (key.workspaceId !== auth.authorizedWorkspaceId) { - throw new UnkeyApiError({ - code: "NOT_FOUND", - message: "Key not found", - }); - } - let meta = key.meta ? JSON.parse(key.meta) : undefined; - if (!meta || Object.keys(meta).length === 0) { - meta = undefined; - } - - return c.json({ - id: key.id, - name: key.name ?? undefined, - remaining: key.remaining ?? undefined, - identity: data.identity - ? { - id: data.identity.id, - externalId: data.identity.externalId, - } - : undefined, - meta: meta, - createdAt: key.createdAt.getTime(), - enabled: key.enabled, - environment: key.environment ?? undefined, - }); - }); diff --git a/apps/api/src/routes/v1_keys_deleteKey.ts b/apps/api/src/routes/v1_keys_deleteKey.ts index 0718b208bf..8c054f519c 100644 --- a/apps/api/src/routes/v1_keys_deleteKey.ts +++ b/apps/api/src/routes/v1_keys_deleteKey.ts @@ -26,7 +26,7 @@ const route = createRoute({ }), permanent: z.boolean().default(false).optional().openapi({ description: - "By default Unkey soft deletes keys, so they may be recovered later. If you want to permanently delete it, set permanent=true. This might be necessary if you run into NOT_UNIQUE errors during key migration.", + "By default Unkey soft deletes keys, so they may be recovered later. If you want to permanently delete it, set permanent=true. This might be necessary if you run into CONFLICT errors during key migration.", }), }), }, diff --git a/apps/api/src/routes/v1_migrations_createKey.happy.test.ts b/apps/api/src/routes/v1_migrations_createKey.happy.test.ts index d15bba2e9d..c67d17636a 100644 --- a/apps/api/src/routes/v1_migrations_createKey.happy.test.ts +++ b/apps/api/src/routes/v1_migrations_createKey.happy.test.ts @@ -488,7 +488,7 @@ test("an error rolls back and does not create any keys", async (t) => { }); expect(res.status).toEqual(409); - expect(res.body.error.code).toEqual("NOT_UNIQUE"); + expect(res.body.error.code).toEqual("CONFLICT"); for (let i = 0; i < req.length; i++) { const key = await h.db.primary.query.keys.findFirst({ diff --git a/apps/api/src/routes/v1_migrations_createKey.ts b/apps/api/src/routes/v1_migrations_createKey.ts index b17de8a501..078858a337 100644 --- a/apps/api/src/routes/v1_migrations_createKey.ts +++ b/apps/api/src/routes/v1_migrations_createKey.ts @@ -559,7 +559,7 @@ export const registerV1MigrationsCreateKeys = (app: App) => workspaceId: authorizedWorkspaceId, }); throw new UnkeyApiError({ - code: "NOT_UNIQUE", + code: "CONFLICT", message: e.body.message, }); } diff --git a/apps/api/src/routes/v1_permissions_createPermission.error.test.ts b/apps/api/src/routes/v1_permissions_createPermission.error.test.ts index 0c6dc3adfb..ed5ba534f5 100644 --- a/apps/api/src/routes/v1_permissions_createPermission.error.test.ts +++ b/apps/api/src/routes/v1_permissions_createPermission.error.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "vitest"; +import { randomUUID } from "node:crypto"; import { IntegrationHarness } from "src/pkg/testutil/integration-harness"; import type { @@ -39,3 +40,46 @@ describe.each([ }); }); }); + +test("creating the same permission twice returns conflict", async (t) => { + const h = await IntegrationHarness.init(t); + const root = await h.createRootKey(["rbac.*.create_permission"]); + + const createPermissionRequest = { + url: "/v1/permissions.createPermission", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${root.key}`, + }, + body: { + name: randomUUID(), + }, + }; + + const successResponse = await h.post< + V1PermissionsCreatePermissionRequest, + V1PermissionsCreatePermissionResponse + >(createPermissionRequest); + + expect( + successResponse.status, + `expected 200, received: ${JSON.stringify(successResponse, null, 2)}`, + ).toBe(200); + + const errorResponse = await h.post< + V1PermissionsCreatePermissionRequest, + V1PermissionsCreatePermissionResponse + >(createPermissionRequest); + + expect( + errorResponse.status, + `expected 409, received: ${JSON.stringify(errorResponse, null, 2)}`, + ).toBe(409); + expect(errorResponse.body).toMatchObject({ + error: { + code: "CONFLICT", + docs: "https://unkey.dev/docs/api-reference/errors/code/CONFLICT", + message: `Permission with name "${createPermissionRequest.body.name}" already exists in this workspace`, + }, + }); +}); diff --git a/apps/api/src/routes/v1_permissions_createPermission.happy.test.ts b/apps/api/src/routes/v1_permissions_createPermission.happy.test.ts index 3db7878711..d9bfc0ad43 100644 --- a/apps/api/src/routes/v1_permissions_createPermission.happy.test.ts +++ b/apps/api/src/routes/v1_permissions_createPermission.happy.test.ts @@ -32,28 +32,3 @@ test("creates new permission", async (t) => { }); expect(found).toBeDefined(); }); - -test("creating the same permission twice does not error", async (t) => { - const h = await IntegrationHarness.init(t); - const root = await h.createRootKey(["rbac.*.create_permission"]); - - const name = randomUUID(); - - for (let i = 0; i < 2; i++) { - const res = await h.post< - V1PermissionsCreatePermissionRequest, - V1PermissionsCreatePermissionResponse - >({ - url: "/v1/permissions.createPermission", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${root.key}`, - }, - body: { - name, - }, - }); - - expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200); - } -}); diff --git a/apps/api/src/routes/v1_permissions_createPermission.ts b/apps/api/src/routes/v1_permissions_createPermission.ts index 70a8823e6a..e3541168f8 100644 --- a/apps/api/src/routes/v1_permissions_createPermission.ts +++ b/apps/api/src/routes/v1_permissions_createPermission.ts @@ -5,7 +5,8 @@ import { validation } from "@unkey/validation"; import { insertUnkeyAuditLog } from "@/pkg/audit"; import { rootKeyAuth } from "@/pkg/auth/root_key"; -import { openApiErrorResponses } from "@/pkg/errors"; +import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors"; +import { DatabaseError } from "@planetscale/database"; import { schema } from "@unkey/db"; import { newId } from "@unkey/id"; import { buildUnkeyQuery } from "@unkey/rbac"; @@ -78,15 +79,20 @@ export const registerV1PermissionsCreatePermission = (app: App) => name: req.name, description: req.description, }; + await db.primary.transaction(async (tx) => { await tx .insert(schema.permissions) .values(permission) - .onDuplicateKeyUpdate({ - set: { - name: req.name, - description: req.description, - }, + .catch((e) => { + if (e instanceof DatabaseError && e.body.message.includes("Duplicate entry")) { + throw new UnkeyApiError({ + code: "CONFLICT", + message: `Permission with name "${permission.name}" already exists in this workspace`, + }); + } + + throw e; }); await insertUnkeyAuditLog(c, tx, { @@ -111,6 +117,7 @@ export const registerV1PermissionsCreatePermission = (app: App) => context: { location: c.get("location"), userAgent: c.get("userAgent") }, }); }); + return c.json({ permissionId: permission.id, }); diff --git a/apps/api/src/routes/v1_permissions_createRole.error.test.ts b/apps/api/src/routes/v1_permissions_createRole.error.test.ts index 4e9448896f..52eb2b3d37 100644 --- a/apps/api/src/routes/v1_permissions_createRole.error.test.ts +++ b/apps/api/src/routes/v1_permissions_createRole.error.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "vitest"; +import { randomUUID } from "node:crypto"; import { IntegrationHarness } from "src/pkg/testutil/integration-harness"; import type { @@ -36,3 +37,46 @@ describe.each([ }); }); }); + +test("creating the same role twice errors", async (t) => { + const h = await IntegrationHarness.init(t); + const root = await h.createRootKey(["rbac.*.create_role"]); + + const createRoleRequest = { + url: "/v1/permissions.createRole", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${root.key}`, + }, + body: { + name: randomUUID(), + }, + }; + + const successResponse = await h.post< + V1PermissionsCreateRoleRequest, + V1PermissionsCreateRoleResponse + >(createRoleRequest); + + expect( + successResponse.status, + `expected 200, received: ${JSON.stringify(successResponse, null, 2)}`, + ).toBe(200); + + const errorResponse = await h.post< + V1PermissionsCreateRoleRequest, + V1PermissionsCreateRoleResponse + >(createRoleRequest); + + expect( + errorResponse.status, + `expected 409, received: ${JSON.stringify(errorResponse, null, 2)}`, + ).toBe(409); + expect(errorResponse.body).toMatchObject({ + error: { + code: "CONFLICT", + docs: "https://unkey.dev/docs/api-reference/errors/code/CONFLICT", + message: `Role with name "${createRoleRequest.body.name}" already exists in this workspace`, + }, + }); +}); diff --git a/apps/api/src/routes/v1_permissions_createRole.happy.test.ts b/apps/api/src/routes/v1_permissions_createRole.happy.test.ts index 199b406083..59c5d89ec1 100644 --- a/apps/api/src/routes/v1_permissions_createRole.happy.test.ts +++ b/apps/api/src/routes/v1_permissions_createRole.happy.test.ts @@ -29,25 +29,3 @@ test("creates new role", async (t) => { }); expect(found).toBeDefined(); }); - -test("creating the same role twice does not error", async (t) => { - const h = await IntegrationHarness.init(t); - const root = await h.createRootKey(["rbac.*.create_role"]); - - const name = randomUUID(); - - for (let i = 0; i < 2; i++) { - const res = await h.post({ - url: "/v1/permissions.createRole", - headers: { - "Content-Type": "application/json", - Authorization: `Bearer ${root.key}`, - }, - body: { - name, - }, - }); - - expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200); - } -}); diff --git a/apps/api/src/routes/v1_permissions_createRole.ts b/apps/api/src/routes/v1_permissions_createRole.ts index dc106dda7d..48a055cabd 100644 --- a/apps/api/src/routes/v1_permissions_createRole.ts +++ b/apps/api/src/routes/v1_permissions_createRole.ts @@ -3,7 +3,8 @@ import { createRoute, z } from "@hono/zod-openapi"; import { insertUnkeyAuditLog } from "@/pkg/audit"; import { rootKeyAuth } from "@/pkg/auth/root_key"; -import { openApiErrorResponses } from "@/pkg/errors"; +import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors"; +import { DatabaseError } from "@planetscale/database"; import { schema } from "@unkey/db"; import { newId } from "@unkey/id"; import { buildUnkeyQuery } from "@unkey/rbac"; @@ -82,12 +83,17 @@ export const registerV1PermissionsCreateRole = (app: App) => await tx .insert(schema.roles) .values(role) - .onDuplicateKeyUpdate({ - set: { - name: req.name, - description: req.description, - }, + .catch((e) => { + if (e instanceof DatabaseError && e.body.message.includes("Duplicate entry")) { + throw new UnkeyApiError({ + code: "CONFLICT", + message: `Role with name "${role.name}" already exists in this workspace`, + }); + } + + throw e; }); + await insertUnkeyAuditLog(c, tx, { workspaceId: auth.authorizedWorkspaceId, event: "role.create", @@ -106,7 +112,6 @@ export const registerV1PermissionsCreateRole = (app: App) => }, }, ], - context: { location: c.get("location"), userAgent: c.get("userAgent") }, }); }); diff --git a/apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts b/apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts index 1655415595..a2c16341a7 100644 --- a/apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts +++ b/apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts @@ -47,14 +47,14 @@ export const createNamespace = t.procedure .catch((e) => { if (e instanceof DatabaseError && e.body.message.includes("desc = Duplicate entry")) { throw new TRPCError({ - code: "PRECONDITION_FAILED", - message: "duplicate namespace name. Please use a unique name for each namespace.", + code: "CONFLICT", + message: `A namespace with name "${input.name}" already exists in this workspace. Please choose a different name.`, }); } throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: - "We are unable to create namspace. Please try again or contact support@unkey.dev", + "We are unable to create namespace. Please try again or contact support@unkey.dev", }); }); diff --git a/apps/docs/api-reference/errors/code/PRECONDITION_FAILED.mdx b/apps/docs/api-reference/errors/code/PRECONDITION_FAILED.mdx new file mode 100644 index 0000000000..70ab454f43 --- /dev/null +++ b/apps/docs/api-reference/errors/code/PRECONDITION_FAILED.mdx @@ -0,0 +1,15 @@ +--- +title: PRECONDITION_FAILED +openapi-schema: ErrPreconditionFailed + +--- + +## Problem + +The request does not meet one or more preconditions required for fulfillment. For example, that you are not flagged into a beta feature. + +## Solution + +Please check if you meet all conditions outlined in the error message. + +If that doesn't help, ask for help on [Discord](https://unkey.com/discord) diff --git a/go/pkg/error_code/unkey_database_not_unique.go b/go/pkg/error_code/unkey_database_not_unique.go index 2d04e1e106..2879b562e0 100644 --- a/go/pkg/error_code/unkey_database_not_unique.go +++ b/go/pkg/error_code/unkey_database_not_unique.go @@ -10,8 +10,8 @@ func NewUnkeyDatabaseNotUniqueError(err error) UnkeyDatabaseNotUniqueError { err, SystemUnkey, NamespaceKey, - "NOT_UNIQUE", - "The resource identifier msut be unique.", + "CONFLICT", + "The resource identifier must be unique.", ), }