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

feat: create recoverable keys from dashboard #2783

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+131 −4
Open

feat: create recoverable keys from dashboard #2783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24bf8a4
create recoverable keys
harshsbhat Dec 27, 2024
3b36e6c
lint
harshsbhat Dec 28, 2024
3a66720
Add console.error
harshsbhat Dec 28, 2024
f3f5fa4
bot request
harshsbhat Dec 28, 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
97 changes: 94 additions & 3 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 @@ -46,11 +46,18 @@ export const dynamic = "force-dynamic";
type Props = {
apiId: string;
keyAuthId: string;
storeEncryptedKeys: boolean;
defaultBytes: number | null;
defaultPrefix: string | null;
};

export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Props) => {
export const CreateKey = ({
apiId,
keyAuthId,
storeEncryptedKeys,
defaultBytes,
defaultPrefix,
}: Props) => {
const router = useRouter();
const form = useForm<z.infer<typeof formSchema>>({
resolver: async (data, context, options) => {
Expand All @@ -67,6 +74,7 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
expireEnabled: false,
limitEnabled: false,
metaEnabled: false,
recoverEnabled: false,
ratelimitEnabled: false,
},
});
Expand Down Expand Up @@ -105,6 +113,9 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
if (refill?.interval === "monthly" && !refill.refillDay) {
refill.refillDay = 1;
}
if (!values.recoverEnabled) {
setRecoverable(false);
}

await key.mutateAsync({
keyAuthId,
Expand All @@ -114,6 +125,7 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
ownerId: values.ownerId ?? undefined,
remaining: values.limit?.remaining ?? undefined,
refill: refill,
recoverEnabled: values.recoverEnabled,
enabled: true,
});

Expand All @@ -133,6 +145,7 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
: "*".repeat(split.at(0)?.length ?? 0);
const [showKey, setShowKey] = useState(false);
const [showKeyInSnippet, setShowKeyInSnippet] = useState(false);
const [recoverable, setRecoverable] = useState(false);

const resetRateLimit = () => {
// set them to undefined so the form resets properly.
Expand Down Expand Up @@ -170,9 +183,36 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
</div>
<Alert>
<AlertCircle className="w-4 h-4" />
<AlertTitle>This key is only shown once and can not be recovered </AlertTitle>
<AlertTitle>
{recoverable
? "This key can be recovered"
: "This key is only shown once and cannot be recovered"}
</AlertTitle>
<AlertDescription>
Please pass it on to your user or store it somewhere safe.
{recoverable ? (
<>
It can be recovered using endpoints{" "}
<Link
target="_blank"
href="/docs/api-reference/keys/get"
className="font-medium underline"
>
getKey
</Link>{" "}
and{" "}
<Link
target="_blank"
href="/docs/api-reference/apis/list-keys"
className="font-medium underline"
>
listKeys
</Link>
. Although we still recommend you to pass it on to your user or store it
somewhere safe.
</>
) : (
"Please pass it on to your user or store it somewhere safe."
)}
</AlertDescription>
</Alert>
<Code className="flex items-center justify-between w-full gap-4 mt-2 my-8 ph-no-capture max-sm:text-xs sm:overflow-hidden">
Expand Down Expand Up @@ -209,6 +249,7 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
form.setValue("expireEnabled", false);
form.setValue("ratelimitEnabled", false);
form.setValue("metaEnabled", false);
form.setValue("recoverEnabled", false);
form.setValue("limitEnabled", false);
router.refresh();
}}
Expand Down Expand Up @@ -750,7 +791,57 @@ export const CreateKey = ({ apiId, keyAuthId, defaultBytes, defaultPrefix }: Pro
) : null}
</CardContent>
</Card>
{storeEncryptedKeys && (
<Card>
<CardContent className="justify-between w-full p-4 item-center">
<div className="flex items-center justify-between w-full">
<span>Recoverable</span>

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

{form.watch("recoverEnabled") ? (
<>
{form.formState.errors.ratelimit && (
<p className="text-xs text-center text-content-alert">
{form.formState.errors.ratelimit.message}
</p>
)}
</>
) : null}
<p className="text-xs text-content-subtle">
You can choose to recover and display plaintext keys later, though it's
not recommended. Recoverable keys are securely stored in an encrypted
vault. For more, visit{" "}
<Link
className="font-semibold"
href={"unkey.com/docs/security/recovering-keys"}
>
unkey.com/docs/security/recovering-keys.
</Link>
</p>
</CardContent>
</Card>
)}
<div className="w-full">
<Button
className="w-full"
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export default async function CreateKeypage(props: {
<CreateKey
keyAuthId={keyAuth.id}
apiId={props.params.apiId}
storeEncryptedKeys={keyAuth.storeEncryptedKeys}
defaultBytes={keyAuth.defaultBytes}
defaultPrefix={keyAuth.defaultPrefix}
/>
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export const formSchema = z.object({
.min(new Date(new Date().getTime() + 2 * 60000))
.optional(),
ratelimitEnabled: z.boolean().default(false),
recoverEnabled: z.boolean().default(false),
ratelimit: z
.object({
async: z.boolean().default(false),
Expand Down
36 changes: 35 additions & 1 deletion apps/dashboard/lib/trpc/routers/key/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { insertAuditLogs } from "@/lib/audit";
import { db, schema } from "@/lib/db";
import { env } from "@/lib/env";
import { TRPCError } from "@trpc/server";
import { newId } from "@unkey/id";
import { newKey } from "@unkey/keys";
import { type EncryptRequest, type RequestContext, Vault } from "@unkey/vault";
import { z } from "zod";
import { auth, t } from "../../trpc";

Expand Down Expand Up @@ -33,6 +35,7 @@ export const createKey = t.procedure
})
.optional(),
enabled: z.boolean().default(true),
recoverEnabled: z.boolean().optional(),
environment: z.string().optional(),
}),
)
Expand Down Expand Up @@ -110,7 +113,38 @@ export const createKey = t.procedure
enabled: input.enabled,
environment: input.environment,
});

if (input.recoverEnabled && keyAuth?.storeEncryptedKeys) {
const vault = new Vault(env().AGENT_URL, env().AGENT_TOKEN);
const encryptReq: EncryptRequest = {
keyring: workspace?.id,
data: key,
};
const requestId = crypto.randomUUID();
const context: RequestContext = { requestId };
const vaultRes = await vault.encrypt(context, encryptReq).catch((err) => {
console.error(err);
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Encryption Failed. Please contact support using support@unkey.dev",
});
});
Comment on lines +116 to +130
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Strengthen error handling and null checks

A few suggestions to improve the robustness of the encryption implementation:

  1. Validate environment variables before vault initialization
  2. Replace optional chaining with explicit null check for workspace
  3. Consider using more user-friendly error messages

Apply this diff:

 if (input.recoverEnabled && keyAuth?.storeEncryptedKeys) {
+  if (!env().AGENT_URL || !env().AGENT_TOKEN) {
+    throw new TRPCError({
+      code: "INTERNAL_SERVER_ERROR",
+      message: "Unable to process request. Please contact support.",
+    });
+  }
+  if (!workspace) {
+    throw new TRPCError({
+      code: "INTERNAL_SERVER_ERROR",
+      message: "Unable to process request. Please contact support.",
+    });
+  }
   const vault = new Vault(env().AGENT_URL, env().AGENT_TOKEN);
   const encryptReq: EncryptRequest = {
-    keyring: workspace?.id,
+    keyring: workspace.id,
     data: key,
   };
   const requestId = crypto.randomUUID();
   const context: RequestContext = { requestId };
   const vaultRes = await vault.encrypt(context, encryptReq).catch((err) => {
     console.error(err);
     throw new TRPCError({
       code: "INTERNAL_SERVER_ERROR",
-      message: "Encryption Failed. Please contact support using support@unkey.dev",
+      message: "Unable to process request. Please contact support.",
     });
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (input.recoverEnabled && keyAuth?.storeEncryptedKeys) {
const vault = new Vault(env().AGENT_URL, env().AGENT_TOKEN);
const encryptReq: EncryptRequest = {
keyring: workspace?.id,
data: key,
};
const requestId = crypto.randomUUID();
const context: RequestContext = { requestId };
const vaultRes = await vault.encrypt(context, encryptReq).catch((err) => {
console.error(err);
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Encryption Failed. Please contact support using support@unkey.dev",
});
});
if (input.recoverEnabled && keyAuth?.storeEncryptedKeys) {
if (!env().AGENT_URL || !env().AGENT_TOKEN) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Unable to process request. Please contact support.",
});
}
if (!workspace) {
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Unable to process request. Please contact support.",
});
}
const vault = new Vault(env().AGENT_URL, env().AGENT_TOKEN);
const encryptReq: EncryptRequest = {
keyring: workspace.id,
data: key,
};
const requestId = crypto.randomUUID();
const context: RequestContext = { requestId };
const vaultRes = await vault.encrypt(context, encryptReq).catch((err) => {
console.error(err);
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message: "Unable to process request. Please contact support.",
});
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is fine:

  • keyring: workspace?.id,
  • keyring: workspace.id,

I doubt the other two change requests tho. Let's wait for some maintainer

Copy link
Contributor

Choose a reason for hiding this comment

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

@harshsbhat, understood! We'll defer to the maintainer's verdict on those additional changes. Let me know if you need anything else in the meantime.

await tx
.insert(schema.encryptedKeys)
.values({
keyId: keyId,
workspaceId: workspace.id,
encrypted: vaultRes.encrypted,
encryptionKeyId: vaultRes.keyId,
})
.catch((err) => {
console.error(err);
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message:
"We are unable to store encrypted the key. Please contact support using support@unkey.dev",
});
});
}
await insertAuditLogs(tx, {
workspaceId: workspace.id,
actor: { type: "user", id: ctx.user.id },
Expand Down
Loading