diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/ApiKeys.tsx b/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/ApiKeys.tsx index 95534cb9..500d2026 100644 --- a/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/ApiKeys.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/ApiKeys.tsx @@ -2,7 +2,7 @@ import type { FormEvent } from 'react'; import { useState } from 'react'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { toast } from 'sonner'; -import { Copy, Ban, Plus, KeyRound, Loader2 } from 'lucide-react'; +import { Copy, Ban, Plus, KeyRound, Loader2, Trash2 } from 'lucide-react'; import { useClient } from 'hooks/useClient'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; @@ -53,6 +53,7 @@ export const ApiKeys = () => { const queryClient = useQueryClient(); const [name, setName] = useState(''); const [isCreating, setIsCreating] = useState(false); + const [showRevoked, setShowRevoked] = useState(false); const [minted, setMinted] = useState(null); const { data, isLoading } = useQuery<{ keys: ApiKey[] }>({ @@ -79,6 +80,15 @@ export const ApiKeys = () => { } }; + const forget = async (id: number) => { + try { + await client.delete(`/api-keys/${id}/record`); + await refresh(); + } catch { + toast.error('Could not remove it'); + } + }; + const revoke = async (id: number) => { try { await client.delete(`/api-keys/${id}`); @@ -89,7 +99,19 @@ export const ApiKeys = () => { } }; - const keys = data?.keys ?? []; + const allKeys = data?.keys ?? []; + + // Dead means revoked OR past its expiry — the same test the row rendering uses, lifted here so the + // count in the toggle and the rows it reveals can never disagree. + const isDead = (entry: ApiKey) => + !!entry.revokedAt || (!!entry.expiresAt && new Date(entry.expiresAt).getTime() <= Date.now()); + + // Dead keys are HIDDEN by default rather than merely dimmed. Revoking is the normal end of a key's + // life, so on any account that has been used for a while the dead ones outnumber the live ones and the + // screen stops answering the question it exists for — which of my keys still work. The rows are kept + // and reachable; they are just not the first thing shown. + const deadKeys = allKeys.filter(isDead); + const keys = showRevoked ? allKeys : allKeys.filter((entry) => !isDead(entry)); if (isLoading) { return ( @@ -102,8 +124,8 @@ export const ApiKeys = () => { return (

- An API key signs in as you, without a password and without expiring. Give each app or device its own, so you can - cut one off without touching the rest. Send it as{' '} + An API key signs in as you, without a password and without expiring. One key works everywhere — a key is not + tied to an app. Giving each device its own is only so you can cut one off without touching the rest. Send it as{' '} Authorization: Bearer ….

@@ -150,7 +172,9 @@ export const ApiKeys = () => { {keys.length === 0 ? (
- No keys yet. + {/* "No keys yet" would be false on an account whose keys are all revoked, and it is the sentence + that would send somebody looking for a bug in the list. */} + {allKeys.length === 0 ? 'No keys yet.' : 'No keys in use.'}
) : (
@@ -181,11 +205,23 @@ export const ApiKeys = () => { {lastUsed ? `last used ${lastUsed}` : 'never used'}
- {!dead && ( + {dead ? ( + // Only offered once the key is already dead — the server refuses it otherwise, and the + // button matching that rule is what keeps the two from drifting. + ) : ( + + )} ); }; diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/index.tsx b/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/index.tsx index b69c3611..0e154bc6 100644 --- a/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/index.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Settings/ProfileSettings/index.tsx @@ -30,7 +30,7 @@ const sections: SettingsSection[] = [ key: 'api-keys', icon: KeyRound, title: 'API keys', - description: 'Per-app keys that sign in as you, revocable one at a time', + description: 'Keys that sign in as you, revocable one at a time', content: , }, { diff --git a/src/databases/officer_db/src/api-keys/index.ts b/src/databases/officer_db/src/api-keys/index.ts index 4f5abf1a..a0569aae 100644 --- a/src/databases/officer_db/src/api-keys/index.ts +++ b/src/databases/officer_db/src/api-keys/index.ts @@ -3,6 +3,7 @@ export { createApiKey, listApiKeys, revokeApiKey, + deleteApiKey, touchApiKey, type ApiKeyIdentity, } from './queries'; diff --git a/src/databases/officer_db/src/api-keys/queries.ts b/src/databases/officer_db/src/api-keys/queries.ts index f6f37edc..ebe2d31b 100644 --- a/src/databases/officer_db/src/api-keys/queries.ts +++ b/src/databases/officer_db/src/api-keys/queries.ts @@ -1,4 +1,4 @@ -import { eq, and, isNull, sql } from 'drizzle-orm'; +import { eq, and, isNull, isNotNull, sql } from 'drizzle-orm'; import { db } from '../db'; import { apiKeys } from './schema'; import { users } from '../auth/schema'; @@ -99,6 +99,32 @@ export async function revokeApiKey(id: number, userId: number): Promise return rows.length > 0; } +/** + * Delete a revoked key's row for good. + * + * Separate from revoking, and only reachable AFTER it — the `revokedAt is not null` clause is in the + * statement rather than checked before it, so there is no window where a live key is deleted by a request + * that raced a revoke. + * + * ── Why this is a second step and not what revoke does ── + * + * The row IS the record that the key existed. `lastUsedAt` is the only evidence of whether a leaked key + * was used before it was killed, and on what date; deleting on revoke would throw that away at exactly + * the moment somebody most wants it. So revoke keeps everything and this exists for afterwards, when the + * question has been answered and the row is only clutter. + * + * Deleting is safe in a way it would not be for a password: the key can never be honoured again either + * way, because `authenticate` matches on a hash that no longer has a row. Losing the row loses the + * history, not the protection. + */ +export async function deleteApiKey(id: number, userId: number): Promise { + const rows = await db + .delete(apiKeys) + .where(and(eq(apiKeys.id, id), eq(apiKeys.userId, userId), isNotNull(apiKeys.revokedAt))) + .returning({ id: apiKeys.id }); + return rows.length > 0; +} + /** How stale last_used_at is allowed to get. A write per request is not worth this column's value. */ const TOUCH_DEBOUNCE_MS = 60_000; diff --git a/src/servers/api/api-keys/router.ts b/src/servers/api/api-keys/router.ts index fdeea1cc..873d3e71 100644 --- a/src/servers/api/api-keys/router.ts +++ b/src/servers/api/api-keys/router.ts @@ -1,5 +1,5 @@ import { createRouter } from '../../create-router'; -import { createApiKey, listApiKeys, revokeApiKey } from 'officerdb'; +import { createApiKey, listApiKeys, revokeApiKey, deleteApiKey } from 'officerdb'; import { mintApiKey } from '../../auth-token'; import * as errors from '../../custom-errors'; @@ -60,3 +60,26 @@ apiKeysRouter.delete('/:id', async (ctx) => { if (!(await revokeApiKey(id, user.id))) throw errors.NOT_FOUND('no such api key'); return ctx.json({ ok: true }); }); + +// ── Forgetting a key, which is not the same as revoking it ── +// +// DELETE /:id revokes and keeps the row, because the row is the record that the key existed and +// `lastUsedAt` is the only evidence of whether a leaked one was used before it was killed. This removes +// that record, and it is deliberately a SECOND request after the first: `deleteApiKey` refuses a key that +// is not already revoked, in the statement rather than before it, so there is no ordering in which a live +// key is deleted by something that raced a revoke. +// +// Nothing is weakened by allowing it. The key cannot be honoured again either way — authentication +// matches a hash, and a deleted row has none. What goes is the history, not the protection. +apiKeysRouter.delete('/:id/record', async (ctx) => { + const user = ctx.get('user'); + const id = Number(ctx.req.param('id')); + if (!Number.isInteger(id)) throw errors.BAD_REQUEST('invalid id'); + + if (!(await deleteApiKey(id, user.id))) { + // One message for both "not yours / never existed" and "still live", on purpose: the first must not + // be distinguishable from the second by someone probing ids that are not theirs. + throw errors.NOT_FOUND('no such revoked api key'); + } + return ctx.json({ ok: true }); +});