Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: encrypt metadata feature #2059

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

feat: encrypt metadata feature #2059

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
07b1fc8
Encrypted meta for API
harshsbhat Sep 2, 2024
82db90b
Encrypted meta for dashboard
harshsbhat Sep 2, 2024
20acdfd
removal of unused variables for linting
harshsbhat Sep 3, 2024
07ad642
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 4, 2024
7a4f0f7
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 4, 2024
c453788
Merge remote-tracking branch 'origin/main' into harshbhat/encrypted-meta
harshsbhat Sep 5, 2024
5f1c4da
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 8, 2024
7451f9b
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 10, 2024
f65066c
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 11, 2024
342bfe2
Requested changes
harshsbhat Sep 11, 2024
cb44ea9
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 11, 2024
875e826
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 13, 2024
0e9c23d
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 17, 2024
616beb4
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 18, 2024
1770d0c
Merge branch 'main' into harshbhat/encrypted-meta
harshsbhat Sep 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions apps/api/src/routes/v1_keys_createKey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ When validating a key, we will return this back to you, so you can clearly ident
trialEnds: "2023-06-16T17:16:37.161Z",
},
}),
encryptedMeta: z
.record(z.unknown())
.optional()
.openapi({
description:
"This is a place for dynamic meta data, anything that feels useful for you should go here",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to explain that it's encrypted too, as this string is used in sdks and documentation.
I know the field is named encryptedMeta but it doesn't hurt to be obvious.

perkinsjr marked this conversation as resolved.
Show resolved Hide resolved
example: {
billingTier: "PRO",
trialEnds: "2023-06-16T17:16:37.161Z",
},
}),
roles: z
.array(z.string().min(1).max(512))
.optional()
Expand Down Expand Up @@ -336,6 +347,48 @@ export const registerV1KeysCreateKey = (app: App) =>
byteLength: req.byteLength ?? 16,
prefix: req.prefix,
}).toString();
let metaSecret = req.encryptedMeta ? JSON.stringify(req.encryptedMeta) : null;

if (metaSecret) {
const perm = rbac.evaluatePermissions(
buildUnkeyQuery(({ or }) => or("*", "api.*.encrypt_meta", `api.${api.id}.encrypt_meta`)),
auth.permissions,
);

if (perm.err) {
throw new UnkeyApiError({
code: "INTERNAL_SERVER_ERROR",
message: `unable to evaluate permissions: ${perm.err.message}`,
});
}

if (!perm.val.valid) {
throw new UnkeyApiError({
code: "INSUFFICIENT_PERMISSIONS",
message: `insufficient permissions to encrypt metadata: ${perm.val.message}`,
});
}

if (metaSecret) {
const encryptedMeta = await retry(
3,
async () => {
return await vault.encrypt(c, {
keyring: authorizedWorkspaceId,
data: metaSecret as string,
});
},
(attempt, err) =>
logger.warn("vault.encrypt failed", {
attempt,
err: err.message,
}),
);

metaSecret = encryptedMeta.encrypted;
Copy link
Collaborator

Choose a reason for hiding this comment

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

you're ignoring the returned encryption key id, that needs to be stored as well, otherwise we can't reroll the encryption keys

}
}

const start = secret.slice(0, (req.prefix?.length ?? 0) + 5);
const kId = newId("key");
const hash = await sha256(secret.toString());
Expand All @@ -347,6 +400,7 @@ export const registerV1KeysCreateKey = (app: App) =>
start,
ownerId: externalId,
meta: req.meta ? JSON.stringify(req.meta) : null,
encryptedMeta: metaSecret ? metaSecret : null,
workspaceId: authorizedWorkspaceId,
forWorkspaceId: null,
expires: req.expires ? new Date(req.expires) : null,
Expand Down
44 changes: 41 additions & 3 deletions apps/api/src/routes/v1_keys_verifyKey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { rootKeyAuth } from "@/pkg/auth/root_key";
import { UnkeyApiError, openApiErrorResponses } from "@/pkg/errors";
import type { App } from "@/pkg/hono/app";
import { DisabledWorkspaceError, MissingRatelimitError } from "@/pkg/keys/service";
import { retry } from "@/pkg/util/retry";
import { createRoute, z } from "@hono/zod-openapi";
import { SchemaError } from "@unkey/error";
import { permissionQuerySchema } from "@unkey/rbac";
import { buildUnkeyQuery, permissionQuerySchema } from "@unkey/rbac";

const route = createRoute({
tags: ["keys"],
Expand Down Expand Up @@ -176,6 +178,16 @@ A key could be invalid for a number of reasons, for example if it has expired, h
stripeCustomerId: "cus_1234",
},
}),
encryptedMeta: z
.record(z.unknown())
.optional()
.openapi({
description: "Any additional metadata you want to store with the key",
example: {
roles: ["admin", "user"],
stripeCustomerId: "cus_1234",
},
}),
expires: z.number().int().optional().openapi({
description:
"The unix timestamp in milliseconds when the key will expire. If this field is null or undefined, the key is not expiring.",
Expand Down Expand Up @@ -286,8 +298,7 @@ export type V1KeysVerifyKeyResponse = z.infer<
export const registerV1KeysVerifyKey = (app: App) =>
app.openapi(route, async (c) => {
const req = c.req.valid("json");
const { keyService, analytics } = c.get("services");

const { keyService, analytics, vault } = c.get("services");
const { val, err } = await keyService.verifyKey(c, {
key: req.key,
apiId: req.apiId,
Expand Down Expand Up @@ -326,13 +337,40 @@ export const registerV1KeysVerifyKey = (app: App) =>
code: val.code,
});
}
let metaSecret = "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is way too expensive to do uncached. Making a network request to the agent in the critical path is too slow.
We need to move this into the cache's swr callback

if (val.key.encryptedMeta) {
try {
await rootKeyAuth(
Copy link
Collaborator

Choose a reason for hiding this comment

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

what rootKey are you using here?
verifying keys does not require a root key. I'm very surprised this didn't cause issues when you tested it.

c,
buildUnkeyQuery(({ or }) =>
or("*", "api.*.decrypt_meta", `api.${val.api.id}.decrypt_meta`),
),
);
const encryptedMeta =
typeof val.key.encryptedMeta === "string"
Copy link
Collaborator

Choose a reason for hiding this comment

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

what else could it be?
This should always be a string or a falsy value, right?

? val.key.encryptedMeta
: JSON.stringify(val.key.encryptedMeta);

const decryptRes = await retry(3, () =>
vault.decrypt(c, {
keyring: val.key.workspaceId,
encrypted: encryptedMeta,
}),
);

metaSecret = decryptRes.plaintext;
} catch {
metaSecret = "{}";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think undefined and therefore not returning the field in the api response is preferrable

}
}

const responseBody = {
keyId: val.key?.id,
valid: val.valid,
name: val.key?.name ?? undefined,
ownerId: val.key?.ownerId ?? undefined,
meta: val.key?.meta ? JSON.parse(val.key?.meta) : undefined,
encryptedMeta: JSON.parse(metaSecret),
expires: val.key?.expires?.getTime(),
remaining: val.remaining ?? undefined,
ratelimit: val.ratelimit ?? undefined,
Expand Down
116 changes: 116 additions & 0 deletions apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,23 @@ const formSchema = z.object({
},
)
.optional(),
encryptMetaEnabled: z.boolean().default(false),
encryptMeta: z
.string()
.refine(
(s) => {
try {
JSON.parse(s);
return true;
} catch {
return false;
}
},
{
message: "Must be valid json",
},
)
.optional(),
limitEnabled: z.boolean().default(false),
limit: z
.object({
Expand Down Expand Up @@ -163,6 +180,7 @@ export const CreateKey: React.FC<Props> = ({ apiId, keyAuthId }) => {
expireEnabled: false,
limitEnabled: false,
metaEnabled: false,
encryptMetaEnabled: false,
ratelimitEnabled: false,
},
});
Expand All @@ -189,6 +207,9 @@ export const CreateKey: React.FC<Props> = ({ apiId, keyAuthId }) => {
if (!values.metaEnabled) {
delete values.meta;
}
if (!values.encryptMetaEnabled) {
delete values.encryptMeta;
}
if (!values.limitEnabled) {
delete values.limit;
}
Expand All @@ -200,6 +221,7 @@ export const CreateKey: React.FC<Props> = ({ apiId, keyAuthId }) => {
keyAuthId,
...values,
meta: values.meta ? JSON.parse(values.meta) : undefined,
encryptedMeta: values.encryptMeta ? JSON.parse(values.encryptMeta) : undefined,
expires: values.expires?.getTime() ?? undefined,
ownerId: values.ownerId ?? undefined,
remaining: values.limit?.remaining ?? undefined,
Expand Down Expand Up @@ -292,6 +314,7 @@ export const CreateKey: React.FC<Props> = ({ apiId, keyAuthId }) => {
form.setValue("expireEnabled", false);
form.setValue("ratelimitEnabled", false);
form.setValue("metaEnabled", false);
form.setValue("encryptMetaEnabled", false);
form.setValue("limitEnabled", false);
router.refresh();
}}
Expand Down Expand Up @@ -806,7 +829,100 @@ export const CreateKey: React.FC<Props> = ({ apiId, keyAuthId }) => {
) : null}
</CardContent>
</Card>
<Card>
<CardContent className="justify-between w-full p-4 item-center">
<div className="flex items-center justify-between w-full">
<span>Encrypted Metadata</span>

<FormField
control={form.control}
name="encryptMetaEnabled"
render={({ field }) => (
<FormItem>
<FormLabel className="sr-only">Metadata</FormLabel>
<FormControl>
<Switch
onCheckedChange={(e) => {
field.onChange(e);
if (field.value === false) {
resetLimited();
}
}}
/>
</FormControl>
</FormItem>
)}
/>
</div>

{form.watch("encryptMetaEnabled") ? (
<>
<p className="text-xs text-content-subtle">
Encrypt sensitive data before associating it with this key to store it
securely. Whenever you verify this key with the decrypt metadata
Copy link
Collaborator

Choose a reason for hiding this comment

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

As mentioned above, verifying keys does not require a root key (yet), therefore you can't do permission checks

permissions, the encrypted metadata will be returned to you securely,
ensuring confidentiality. Enter custom encrypted metadata as a JSON
object.
</p>

<div className="flex flex-col gap-4 mt-4">
<FormField
control={form.control}
name="encryptMeta"
render={({ field }) => (
<FormItem>
<FormControl>
<Textarea
disabled={!form.watch("encryptMetaEnabled")}
className="m-4 mx-auto border rounded-md shadow-sm"
rows={7}
placeholder={`{"STRIPE_API_KEY" : "sk_test123"}`}
{...field}
value={
form.getValues("encryptMetaEnabled")
? field.value
: undefined
}
/>
</FormControl>
<FormDescription>
Enter custom metadata as a JSON object.
</FormDescription>
<FormMessage />
<Button
variant="secondary"
type="button"
onClick={(_e) => {
try {
if (field.value) {
const parsed = JSON.parse(field.value);
field.onChange(JSON.stringify(parsed, null, 2));
form.clearErrors("encryptMeta");
}
} catch (_e) {
form.setError("encryptMeta", {
type: "manual",
message: "Invalid JSON",
});
}
}}
value={field.value}
>
Format Json
</Button>
</FormItem>
)}
/>
</div>
{form.formState.errors.ratelimit && (
<p className="text-xs text-center text-content-alert">
{form.formState.errors.ratelimit.message}
</p>
)}
</>
) : null}
</CardContent>
</Card>
<div className="w-full">
<Button
className="w-full"
Expand Down
16 changes: 16 additions & 0 deletions apps/dashboard/app/(app)/settings/root-keys/[keyId]/permissions/permissions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ export const workspacePermissions = {
description: "Decrypt keys in this workspace",
permission: "api.*.decrypt_key",
},
encrypt_meta: {
description: "Encrypt metadata associated with the keys in this workspace",
permission: "api.*.encrypt_meta",
},
decrypt_meta: {
description: "Decrypt metadata associated with the keys in this workspace",
permission: "api.*.decrypt_meta",
},
},
Ratelimit: {
create_namespace: {
Expand Down Expand Up @@ -178,6 +186,14 @@ export function apiPermissions(apiId: string): { [category: string]: UnkeyPermis
description: "Decrypt keys belonging to this API",
permission: `api.${apiId}.decrypt_key`,
},
encrypt_meta: {
description: "Encrypt metadata associated with the keys of this API",
permission: `api.${apiId}.encrypt_meta`,
},
decrypt_meta: {
description: "Decrypt metadata associated with the keys of this API",
permission: `api.${apiId}.decrypt_meta`,
},
},
};
}
Loading