Skip to content

Commit 1e94e8f

Browse files
committed
fix: API Keys review findings -- security, a11y, error handling
Security: - handleGenerate guards against double-submit (if generating, return) - Enter key also checks !generating before triggering - Dialog prevents backdrop/Escape close when generatedKey is displayed (only explicit 'Done' button clears the one-time key) - Removed CopyInline from key_preview (preview is intentionally incomplete, copying it gives users a broken value) Accessibility: - CopyInline button has dynamic aria-label ('Copy/Copied' + label) - Revoke button has aria-label='Revoke API key {name}' - Removed unused X import Error handling: - useQuery destructures error alongside data/isLoading - Error state renders destructive-styled message instead of empty list - Query key scoped to user: ['api-keys', userId] prevents cross-user cache leakage between sessions - invalidateQueries calls updated to match scoped key Skipped findings (verified not applicable): - Radix alert-dialog removal: not in package.json - Skeleton import: not imported in this file - data.api_key -> data.key: backend returns api_key (line 94)
1 parent a746b0f commit 1e94e8f

1 file changed

Lines changed: 30 additions & 11 deletions

File tree

frontend/src/pages/APIKeysPage.tsx

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useState } from 'react'
2-
import { Plus, Copy, Check, Loader2, X, Clock, Shield, Terminal, Zap } from 'lucide-react'
2+
import { Plus, Copy, Check, Loader2, Clock, Shield, Terminal, Zap } from 'lucide-react'
33
import { useQuery, useQueryClient } from '@tanstack/react-query'
44
import { useAuth } from '@/contexts/AuthContext'
55
import { Button } from '@/components/ui/button'
@@ -67,6 +67,7 @@ function CopyInline({ text, label }: { text: string; label?: string }) {
6767
return (
6868
<button
6969
onClick={handleCopy}
70+
aria-label={copied ? `Copied ${label || ''}` : `Copy ${label || ''}`}
7071
className="inline-flex items-center gap-1.5 text-muted-foreground hover:text-foreground transition-colors"
7172
>
7273
{copied ? <Check className="w-3.5 h-3.5 text-emerald-400" /> : <Copy className="w-3.5 h-3.5" />}
@@ -119,6 +120,7 @@ function KeyCard({
119120
<button
120121
onClick={() => onRevoke(apiKey)}
121122
disabled={revoking}
123+
aria-label={`Revoke API key ${apiKey.name}`}
122124
className={cn(
123125
'opacity-0 group-hover:opacity-100 transition-opacity',
124126
'text-xs text-muted-foreground hover:text-destructive',
@@ -131,12 +133,11 @@ function KeyCard({
131133
)}
132134
</div>
133135

134-
{/* Key preview */}
136+
{/* Key preview (display only, no copy -- preview is intentionally incomplete) */}
135137
<div className="flex items-center gap-2 mb-3">
136-
<code className="text-[13px] font-mono text-muted-foreground tracking-wide select-all">
138+
<code className="text-[13px] font-mono text-muted-foreground tracking-wide">
137139
{apiKey.key_preview}
138140
</code>
139-
<CopyInline text={apiKey.key_preview} label="Key preview" />
140141
</div>
141142

142143
{/* Bottom metadata */}
@@ -293,14 +294,15 @@ export function APIKeysPage() {
293294

294295
const token = session?.access_token || ''
295296

296-
const { data: keys = [], isLoading } = useQuery({
297-
queryKey: ['api-keys'],
297+
const userId = session?.user?.id || ''
298+
const { data: keys = [], isLoading, error } = useQuery({
299+
queryKey: ['api-keys', userId],
298300
queryFn: () => fetchKeys(token),
299301
enabled: !!token,
300302
})
301303

302304
const handleGenerate = async () => {
303-
if (!token || !keyName.trim()) return
305+
if (!token || !keyName.trim() || generating) return
304306
setGenerating(true)
305307
try {
306308
const res = await fetch(`${API_URL}/keys/generate`, {
@@ -318,7 +320,7 @@ export function APIKeysPage() {
318320
const data = await res.json()
319321
setGeneratedKey(data.api_key)
320322
setKeyName('')
321-
queryClient.invalidateQueries({ queryKey: ['api-keys'] })
323+
queryClient.invalidateQueries({ queryKey: ['api-keys', userId] })
322324
} catch (e) {
323325
toast.error(e instanceof Error ? e.message : 'Failed to generate key')
324326
} finally {
@@ -337,7 +339,7 @@ export function APIKeysPage() {
337339
})
338340
if (!res.ok) throw new Error('Failed to revoke key')
339341
toast.success(`"${key.name}" revoked`)
340-
queryClient.invalidateQueries({ queryKey: ['api-keys'] })
342+
queryClient.invalidateQueries({ queryKey: ['api-keys', userId] })
341343
} catch {
342344
toast.error('Failed to revoke key')
343345
} finally {
@@ -367,6 +369,23 @@ export function APIKeysPage() {
367369
)
368370
}
369371

372+
if (error) {
373+
return (
374+
<div className="space-y-8">
375+
<div>
376+
<h1 className="text-2xl font-semibold tracking-tight text-foreground">API Keys</h1>
377+
<p className="text-sm text-muted-foreground mt-1">Authenticate MCP clients, Claude Desktop, and programmatic access.</p>
378+
</div>
379+
<div className="rounded-lg border border-destructive/30 bg-destructive/5 px-5 py-4">
380+
<p className="text-sm font-medium text-destructive">Failed to load API keys</p>
381+
<p className="text-xs text-muted-foreground mt-1">
382+
{error instanceof Error ? error.message : 'An unexpected error occurred.'}
383+
</p>
384+
</div>
385+
</div>
386+
)
387+
}
388+
370389
return (
371390
<div className="space-y-8">
372391
{/* Page header */}
@@ -419,7 +438,7 @@ export function APIKeysPage() {
419438
{activeKeys.length > 0 && <ConnectGuide />}
420439

421440
{/* Generate dialog */}
422-
<Dialog open={generateOpen} onOpenChange={closeGenerateDialog}>
441+
<Dialog open={generateOpen} onOpenChange={(open) => { if (!open && !generatedKey) closeGenerateDialog() }}>
423442
<DialogContent className="sm:max-w-lg border-border/60">
424443
{generatedKey ? (
425444
<>
@@ -462,7 +481,7 @@ export function APIKeysPage() {
462481
placeholder="e.g. Claude Desktop, Development, CI/CD"
463482
value={keyName}
464483
onChange={(e) => setKeyName(e.target.value)}
465-
onKeyDown={(e) => e.key === 'Enter' && keyName.trim() && handleGenerate()}
484+
onKeyDown={(e) => e.key === 'Enter' && keyName.trim() && !generating && handleGenerate()}
466485
className="h-10 bg-background/50"
467486
autoFocus
468487
/>

0 commit comments

Comments
 (0)