Skip to content

Commit

Permalink
feat: delete protection for apis (#2011)
Browse files Browse the repository at this point in the history
* feat: delete protection for apis

* test(integration): add deleteProtection field with false value to test requests
feat(routes): add deleteProtection field to createApi route with default true value
feat(routes): update createApi route to handle deleteProtection field
feat(routes): update openapi documentation for deleteProtection field in createApi route

* test(list_keys.test.ts): add deleteProtection field with value false to test data object

* fix(api tests): remove deleteProtection field from test requests
fix(api routes): remove deleteProtection field from createApi route schema
fix(api routes): remove deleteProtection field from createApi route handler
fix(api routes): remove deleteProtection field from deleteApi route tests
fix(api routes): remove deleteProtection field from deleteApi route security tests
fix(db schema): change default value of deleteProtection to false

* style(createApi.ts): remove unnecessary whitespace and hyphen at the end of the file

* test(v1_apis_createApi.happy.test.ts): remove redundant test case for creating new api without delete protection
  • Loading branch information
chronark authored Aug 5, 2024
1 parent eb85713 commit ebdeca6
Show file tree
Hide file tree
Showing 20 changed files with 421 additions and 57 deletions.
2 changes: 1 addition & 1 deletion apps/agent/fly.production.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ port = 9095


[[vm]]
memory = '1gb'
memory = '2gb'
cpu_kind = 'shared'
cpus = 2

Expand Down
2 changes: 2 additions & 0 deletions apps/api/src/pkg/errors/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const ErrorCode = z.enum([
"INSUFFICIENT_PERMISSIONS",
"METHOD_NOT_ALLOWED",
"EXPIRED",
"DELETE_PROTECTED",
]);

export function errorSchemaFactory(code: z.ZodEnum<any>) {
Expand Down Expand Up @@ -81,6 +82,7 @@ function codeToStatus(code: z.infer<typeof ErrorCode>): StatusCode {
return 405;
case "NOT_UNIQUE":
return 409;
case "DELETE_PROTECTED":
case "PRECONDITION_FAILED":
return 412;
case "RATE_LIMITED":
Expand Down
4 changes: 4 additions & 0 deletions apps/api/src/pkg/testutil/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ export abstract class Harness {
deletedAt: null,
planDowngradeRequest: null,
enabled: true,
deleteProtection: true,
};
const userWorkspace: Workspace = {
id: newId("test"),
Expand All @@ -274,6 +275,7 @@ export abstract class Harness {
deletedAt: null,
planDowngradeRequest: null,
enabled: true,
deleteProtection: true,
};

const unkeyKeyAuth: KeyAuth = {
Expand Down Expand Up @@ -306,6 +308,7 @@ export abstract class Harness {
ipWhitelist: null,
createdAt: new Date(),
deletedAt: null,
deleteProtection: true,
};
const userApi: Api = {
id: newId("test"),
Expand All @@ -316,6 +319,7 @@ export abstract class Harness {
ipWhitelist: null,
createdAt: new Date(),
deletedAt: null,
deleteProtection: true,
};

return {
Expand Down
1 change: 1 addition & 0 deletions apps/api/src/routes/v1_apis_createApi.happy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ test("creates new api", async (t) => {
where: (table, { eq }) => eq(table.id, res.body.apiId),
});
expect(found).toBeDefined();
expect(found!.deleteProtection).toBe(false);
});
90 changes: 66 additions & 24 deletions apps/api/src/routes/v1_apis_deleteApi.happy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,77 @@ import { schema } from "@unkey/db";
import { newId } from "@unkey/id";
import { IntegrationHarness } from "src/pkg/testutil/integration-harness";

import { expect, test } from "vitest";
import { describe, expect, test } from "vitest";
import type { V1ApisDeleteApiRequest, V1ApisDeleteApiResponse } from "./v1_apis_deleteApi";

test("deletes the api", async (t) => {
const h = await IntegrationHarness.init(t);
const apiId = newId("test");
await h.db.primary.insert(schema.apis).values({
id: apiId,
name: randomUUID(),
workspaceId: h.resources.userWorkspace.id,
});
describe("without delete protection", () => {
test("soft deletes the api", async (t) => {
const h = await IntegrationHarness.init(t);
const apiId = newId("test");
await h.db.primary.insert(schema.apis).values({
id: apiId,
name: randomUUID(),
workspaceId: h.resources.userWorkspace.id,
});

const root = await h.createRootKey([`api.${apiId}.delete_api`]);
const res = await h.post<V1ApisDeleteApiRequest, V1ApisDeleteApiResponse>({
url: "/v1/apis.deleteApi",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${root.key}`,
},
body: {
apiId,
},
});

expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200);
expect(res.body).toEqual({});

const root = await h.createRootKey([`api.${apiId}.delete_api`]);
const res = await h.post<V1ApisDeleteApiRequest, V1ApisDeleteApiResponse>({
url: "/v1/apis.deleteApi",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${root.key}`,
},
body: {
apiId,
},
const found = await h.db.primary.query.apis.findFirst({
where: (table, { eq, and }) => and(eq(table.id, apiId)),
});
expect(found).toBeDefined();
expect(found!.deletedAt).not.toBeNull();
});
});
describe("with delete protection", () => {
test("returns an error", async (t) => {
const h = await IntegrationHarness.init(t);
const apiId = newId("test");
await h.db.primary.insert(schema.apis).values({
id: apiId,
name: randomUUID(),
workspaceId: h.resources.userWorkspace.id,
deleteProtection: true,
});

const root = await h.createRootKey([`api.${apiId}.delete_api`]);
const res = await h.post<V1ApisDeleteApiRequest, V1ApisDeleteApiResponse>({
url: "/v1/apis.deleteApi",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${root.key}`,
},
body: {
apiId,
},
});

expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200);
expect(res.body).toEqual({});
expect(res.status, `expected 412, received: ${JSON.stringify(res, null, 2)}`).toBe(412);
expect(res.body).toMatchObject({
error: {
code: "DELETE_PROTECTED",
docs: "https://unkey.dev/docs/api-reference/errors/code/DELETE_PROTECTED",
message: `api ${apiId} is protected from deletions`,
},
});

const found = await h.db.primary.query.apis.findFirst({
where: (table, { eq, and, isNull }) => and(eq(table.id, apiId), isNull(table.deletedAt)),
const found = await h.db.primary.query.apis.findFirst({
where: (table, { eq, and, isNull }) => and(eq(table.id, apiId), isNull(table.deletedAt)),
});
expect(found).toBeDefined();
expect(found!.deletedAt).toBeNull();
});
expect(found).toBeUndefined();
});
42 changes: 26 additions & 16 deletions apps/api/src/routes/v1_apis_deleteApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 { UnkeyApiError, errorSchemaFactory, openApiErrorResponses } from "@/pkg/errors";
import { schema } from "@unkey/db";
import { and, eq } from "@unkey/db";
import { buildUnkeyQuery } from "@unkey/rbac";
Expand Down Expand Up @@ -39,6 +39,14 @@ const route = createRoute({
},
},
...openApiErrorResponses,
429: {
description: "The api is protected from deletions",
content: {
"application/json": {
schema: errorSchemaFactory(z.enum(["DELETE_PROTECTED"])).openapi("ErrDeleteProtected"),
},
},
},
},
});
export type Route = typeof route;
Expand All @@ -60,26 +68,28 @@ export const registerV1ApisDeleteApi = (app: App) =>
buildUnkeyQuery(({ or }) => or("*", "api.*.delete_api", `api.${apiId}.delete_api`)),
);

const { val: api, err } = await cache.apiById.swr(apiId, async () => {
return (
(await db.readonly.query.apis.findFirst({
where: (table, { eq, and, isNull }) => and(eq(table.id, apiId), isNull(table.deletedAt)),
with: {
keyAuth: true,
},
})) ?? null
);
/**
* We do not want to cache this. Deleting the api is a very infrequent operation and
* it's absolutely critical that we don't read a stale value from the cache when checking
* for delete protection.
*/
const api = await db.readonly.query.apis.findFirst({
where: (table, { eq, and, isNull }) => and(eq(table.id, apiId), isNull(table.deletedAt)),
with: {
keyAuth: true,
},
});

if (err) {
throw new UnkeyApiError({
code: "INTERNAL_SERVER_ERROR",
message: `unable to load api: ${err.message}`,
});
}
if (!api || api.workspaceId !== auth.authorizedWorkspaceId) {
throw new UnkeyApiError({ code: "NOT_FOUND", message: `api ${apiId} not found` });
}
if (api.deleteProtection) {
throw new UnkeyApiError({
code: "DELETE_PROTECTED",
message: `api ${apiId} is protected from deletions`,
});
}

const authorizedWorkspaceId = auth.authorizedWorkspaceId;
const rootKeyId = auth.key.id;

Expand Down
24 changes: 20 additions & 4 deletions apps/dashboard/app/(app)/apis/[apiId]/settings/delete-api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { useRouter } from "next/navigation";
import { useForm } from "react-hook-form";
import { z } from "zod";

import { parseTrpcError } from "@/lib/utils";
import { cn, parseTrpcError } from "@/lib/utils";
import { revalidate } from "./actions";

type Props = {
Expand All @@ -40,6 +40,7 @@ type Props = {
id: string;
workspaceId: string;
name: string;
deleteProtection: boolean | null;
};
};

Expand Down Expand Up @@ -86,7 +87,12 @@ export const DeleteApi: React.FC<Props> = ({ api, keys }) => {

return (
<>
<Card className="relative border-2 border-alert">
<Card
className={cn("relative ", {
"borrder-opacity-50": api.deleteProtection,
"border-2 border-alert": !api.deleteProtection,
})}
>
<CardHeader>
<CardTitle>Delete</CardTitle>
<CardDescription>
Expand All @@ -95,10 +101,20 @@ export const DeleteApi: React.FC<Props> = ({ api, keys }) => {
</CardDescription>
</CardHeader>

<CardFooter className="z-10 justify-end">
<Button type="button" onClick={() => setOpen(!open)} variant="alert">
<CardFooter className="z-10 justify-between flex-row-reverse w-full">
<Button
type="button"
disabled={!!api.deleteProtection}
onClick={() => setOpen(!open)}
variant="alert"
>
Delete API
</Button>
{api.deleteProtection ? (
<p className="text-sm text-content">
Deletion protection is enabled, you need to disable it before deleting this API.
</p>
) : null}
</CardFooter>
</Card>
<Dialog open={open} onOpenChange={(o) => setOpen(o)}>
Expand Down
Loading

1 comment on commit ebdeca6

@vercel
Copy link

@vercel vercel bot commented on ebdeca6 Aug 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

planetfall – ./apps/planetfall

planetfall-git-main-unkey.vercel.app
planetfall-unkey.vercel.app
planetfall.unkey.dev
planetfall-two.vercel.app

Please sign in to comment.