From 9e37ea73a817e70052d323bfadf2a156edc9d102 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Thu, 10 Sep 2026 05:02:05 -0700 Subject: [PATCH 1/4] feat: add a permanent delete for API keys, DELETE /api-keys/:id/permanent plus a Delete key button under a revoked key in the dashboard drawer and warmbly key purge, taking the key's usage logs with it and refusing any key that could still authenticate so revoking stays the step that records why a credential ended (issue #414) --- cmd/cli/specs.go | 12 ++ docs/content/docs/api/authentication.mdx | 2 + docs/content/docs/api/endpoints.mdx | 3 + docs/content/docs/api/reference/api-keys.mdx | 21 ++ internal/api/handler/api_key.go | 26 +++ internal/api/routes.go | 3 + internal/app/apikey/service.go | 16 ++ internal/models/api_key.go | 11 ++ .../repository/api_key_delete_live_test.go | 184 ++++++++++++++++++ internal/repository/pg_api_key.go | 28 +++ .../api-keys/_components/KeyDetailDrawer.tsx | 51 ++++- .../api/client/app/api-keys/deleteAPIKey.ts | 11 ++ .../api/hooks/app/api-keys/useDeleteAPIKey.ts | 13 ++ 13 files changed, 373 insertions(+), 8 deletions(-) create mode 100644 internal/repository/api_key_delete_live_test.go create mode 100644 web/src/lib/api/client/app/api-keys/deleteAPIKey.ts create mode 100644 web/src/lib/api/hooks/app/api-keys/useDeleteAPIKey.ts diff --git a/cmd/cli/specs.go b/cmd/cli/specs.go index 662f2188..7b0f6e69 100644 --- a/cmd/cli/specs.go +++ b/cmd/cli/specs.go @@ -1614,6 +1614,18 @@ you close the terminal.`, Args: []argSpec{{Name: "id", Help: "The key's id"}}, Success: "API key revoked.", }, + { + Name: "purge", Aliases: []string{"destroy"}, Short: "Delete a revoked API key for good", + Method: http.MethodDelete, Path: "/api-keys/{id}/permanent", + Long: `Remove a revoked or expired key from the workspace, along with its +request log. + +This is the step after ` + "`warmbly key revoke`" + `, not a shortcut past it: a key that +could still authenticate is refused. Note that ` + "`delete`" + ` is an alias of ` + "`revoke`" + `, +which ends a key but leaves it listed; ` + "`purge`" + ` is the one that removes the row.`, + Args: []argSpec{{Name: "id", Help: "The key's id"}}, + Success: "API key deleted.", + }, { Name: "permissions", Aliases: []string{"scopes"}, Short: "Every grantable scope and its bit value", Method: http.MethodGet, Path: "/api-keys/permissions", diff --git a/docs/content/docs/api/authentication.mdx b/docs/content/docs/api/authentication.mdx index def0a1fb..0be2318d 100644 --- a/docs/content/docs/api/authentication.mdx +++ b/docs/content/docs/api/authentication.mdx @@ -39,6 +39,8 @@ Codes expire after ten minutes, both halves are per-IP rate limited, and the `de `DELETE /v1/api-keys/:id` revokes any key in the workspace and needs the `API_KEYS` scope. `DELETE /v1/api-keys/self` revokes the key the call was made with and needs no scope at all, so a narrowly scoped credential can always end itself. This is what `warmbly auth logout` uses. +Revoking leaves the key listed, with the time and reason it ended, and its request history intact. `DELETE /v1/api-keys/:id/permanent` removes the row for good, along with its usage logs, and is how the **Delete key** button under a revoked key in Settings > API keys works. It refuses a key that could still authenticate with a `409`: revoke it first, so what ended the credential is on the record. A key past its `expires_at` can be deleted directly, since it already authenticates nothing. + ## Using your API key Include your API key in the `Authorization` header of every request: diff --git a/docs/content/docs/api/endpoints.mdx b/docs/content/docs/api/endpoints.mdx index 2d1c8ca7..79ba6fe8 100644 --- a/docs/content/docs/api/endpoints.mdx +++ b/docs/content/docs/api/endpoints.mdx @@ -271,8 +271,11 @@ Changing Advisor settings (`PATCH /advisor/settings`) is JWT only, alongside the | GET | `/api-keys/:id` | `API_KEYS` | | PATCH | `/api-keys/:id` | `API_KEYS` | | DELETE | `/api-keys/:id` | `API_KEYS` | +| DELETE | `/api-keys/:id/permanent` | `API_KEYS` | | DELETE | `/api-keys/self` | none | +`DELETE /api-keys/:id/permanent` deletes a key that has already been revoked or has expired, taking its usage logs with it. A key that could still authenticate gets a `409`, because revoking is what records that a credential was ended and why. See [Ending a key](/api/authentication/). + `DELETE /api-keys/self` revokes the key the request was made with, and is the one route here that needs no scope. A credential must always be able to end itself: requiring `API_KEYS` to sign out would leave a read-only key on a laptop someone is handing back live, which is what [`warmbly auth logout`](/api/cli/) promises to prevent. A JWT caller gets a `400`: there is no key in that request to end, only a session, which `POST /auth/logout` ends. ### OAuth apps diff --git a/docs/content/docs/api/reference/api-keys.mdx b/docs/content/docs/api/reference/api-keys.mdx index 1ba46be7..2dcaf800 100644 --- a/docs/content/docs/api/reference/api-keys.mdx +++ b/docs/content/docs/api/reference/api-keys.mdx @@ -262,6 +262,26 @@ A small status envelope. { "status": "revoked" } ``` +## Delete an API key + +`DELETE /api-keys/:id/permanent` + +Removes a revoked or expired key from the workspace for good, along with its usage logs. A key that could still authenticate is refused with a `409`: revoke it first, so what ended the credential stays on the record. A key past its `expires_at` can be deleted directly, since it already authenticates nothing. + +Auth: **Scope** `API_KEYS` · **Org permission** `manage_api_keys` + +| Parameter | In | Type | Description | +| --- | --- | --- | --- | +| `id` | path | uuid | The API key id. | + +### Response + +A small status envelope. + +```json +{ "status": "deleted" } +``` + ## Usage summary `GET /api-keys/usage/summary` @@ -386,5 +406,6 @@ All endpoints use the shared error envelope with stable `code` and `request_id` - `401` when the caller is unauthenticated. - `403` when the caller lacks the `API_KEYS` scope or the `manage_api_keys` org permission. - `404` when the `:id` path value is not a valid UUID or the key does not belong to the organization. +- `409` when deleting a key that can still authenticate. Revoke it first. See [Error codes](/api/error-codes/) for the full list. diff --git a/internal/api/handler/api_key.go b/internal/api/handler/api_key.go index e7d5ed79..1bd3e5f4 100644 --- a/internal/api/handler/api_key.go +++ b/internal/api/handler/api_key.go @@ -171,6 +171,32 @@ func (h *Handler) RevokeAPIKey(c *gin.Context) { c.JSON(http.StatusOK, gin.H{"status": "revoked"}) } +// DeleteAPIKey permanently removes a revoked or expired API key +// DELETE /api-keys/:id/permanent +func (h *Handler) DeleteAPIKey(c *gin.Context) { + orgID := middleware.GetOrganizationID(c) + if orgID == nil { + errx.JSON(c, errx.New(errx.BadRequest, "no organization selected")) + return + } + + keyIDStr := c.Param("id") + keyID, err := uuid.Parse(keyIDStr) + if err != nil { + errx.JSON(c, errx.ErrNotFound) + return + } + + if xerr := h.APIKeyService.Delete(c.Request.Context(), *orgID, keyID); xerr != nil { + errx.JSON(c, xerr) + return + } + + h.auditOrg(c, models.AuditActionDelete, models.AuditEntityAPIKey, &keyID, nil, nil) + + c.JSON(http.StatusOK, gin.H{"status": "deleted"}) +} + // RevokeOwnAPIKey revokes the key the request was made with. // // Deliberately outside the API_KEYS scope gate: a credential must always be diff --git a/internal/api/routes.go b/internal/api/routes.go index 9472139f..61b132ee 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -793,6 +793,9 @@ func Run( apiKeys.GET("/:id", h.GetAPIKey) apiKeys.PATCH("/:id", h.UpdateAPIKey) apiKeys.DELETE("/:id", h.RevokeAPIKey) + // Revoking ends a key; deleting removes the row and its usage + // logs. Separate paths so neither can be reached by accident. + apiKeys.DELETE("/:id/permanent", h.DeleteAPIKey) apiKeys.GET("/:id/analytics", h.GetAPIKeyAnalytics) apiKeys.GET("/:id/logs", h.ListAPIKeyUsageLogs) } diff --git a/internal/app/apikey/service.go b/internal/app/apikey/service.go index 3ab49239..cfc7edff 100644 --- a/internal/app/apikey/service.go +++ b/internal/app/apikey/service.go @@ -55,6 +55,7 @@ type APIKeyService interface { List(ctx context.Context, orgID uuid.UUID, limit int, cursor *uuid.UUID) (*models.APIKeysResult, *errx.Error) Update(ctx context.Context, orgID, keyID uuid.UUID, data *models.UpdateAPIKey) (*models.APIKey, *errx.Error) Revoke(ctx context.Context, orgID, keyID uuid.UUID, reason string) *errx.Error + Delete(ctx context.Context, orgID, keyID uuid.UUID) *errx.Error // Validation ValidateKey(ctx context.Context, rawKey string) (*models.APIKey, *errx.Error) @@ -244,6 +245,21 @@ func (s *apiKeyService) Revoke(ctx context.Context, orgID, keyID uuid.UUID, reas return nil } +// Delete removes a revoked or expired key from the workspace for good, along +// with its usage logs. An active key has to be revoked first: revoking is what +// records that the credential was ended and why, and a delete that also cut +// off live access would end it with nothing saying so. +func (s *apiKeyService) Delete(ctx context.Context, orgID, keyID uuid.UUID) *errx.Error { + key, xerr := s.repo.GetByID(ctx, orgID, keyID) + if xerr != nil { + return xerr + } + if key.CanAuthenticate() { + return errx.New(errx.Conflict, "this key is still active; revoke it before deleting it") + } + return s.repo.Delete(ctx, orgID, keyID) +} + func (s *apiKeyService) ValidateKey(ctx context.Context, rawKey string) (*models.APIKey, *errx.Error) { if !strings.HasPrefix(rawKey, KeyPrefix) { return nil, errx.ErrAuth diff --git a/internal/models/api_key.go b/internal/models/api_key.go index 7b79d8ad..eafa7be8 100644 --- a/internal/models/api_key.go +++ b/internal/models/api_key.go @@ -43,6 +43,17 @@ type APIKey struct { UpdatedAt time.Time `json:"updated_at"` } +// CanAuthenticate reports whether this key would still be accepted on a +// request. The `status` column only ever holds 'active' or 'revoked'; expiry is +// applied when the key is read (GetByHash), so a key past its expires_at is +// already dead without the column saying so. +func (k APIKey) CanAuthenticate() bool { + if k.Status != APIKeyStatusActive { + return false + } + return k.ExpiresAt == nil || k.ExpiresAt.After(time.Now()) +} + type APIKeyWithSecret struct { APIKey Secret string `json:"secret"` // Only returned on creation diff --git a/internal/repository/api_key_delete_live_test.go b/internal/repository/api_key_delete_live_test.go new file mode 100644 index 00000000..f2a8c09f --- /dev/null +++ b/internal/repository/api_key_delete_live_test.go @@ -0,0 +1,184 @@ +package repository + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" +) + +// Deleting an API key removes the row and its usage logs, and refuses a key +// that can still authenticate: ending a live credential is Revoke's job +// (issue #414). +// +// Run against the dev stack: +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \ +// go test ./internal/repository/ -run LiveAPIKeyDelete -v + +type apiKeyFixture struct { + org uuid.UUID + other uuid.UUID + owner uuid.UUID +} + +func newAPIKeyFixture(t *testing.T, pool *pgxpool.Pool) *apiKeyFixture { + t.Helper() + ctx := context.Background() + f := &apiKeyFixture{org: uuid.New(), other: uuid.New(), owner: uuid.New()} + + exec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("fixture %q: %v", sql[:min(60, len(sql))], err) + } + } + tag := f.org.String()[:8] + + exec(`INSERT INTO users (id, first_name, last_name, email, password_hash) + VALUES ($1, 'Keys', 'Live', $2, 'x')`, f.owner, "keys-"+tag+"@fixture.invalid") + for i, org := range []uuid.UUID{f.org, f.other} { + exec(`INSERT INTO organizations (id, name, slug, owner_user_id) + VALUES ($1, 'Keys', $2, $3)`, org, "keys-"+tag+"-"+string(rune('a'+i)), f.owner) + exec(`INSERT INTO organization_members (organization_id, user_id, role, accepted_at) + VALUES ($1, $2, 'owner', NOW())`, org, f.owner) + } + + t.Cleanup(func() { + c := context.Background() + for _, step := range []struct { + sql string + arg any + }{ + {`DELETE FROM api_keys WHERE organization_id = ANY($1)`, []uuid.UUID{f.org, f.other}}, + {`DELETE FROM organization_members WHERE organization_id = ANY($1)`, []uuid.UUID{f.org, f.other}}, + {`DELETE FROM organizations WHERE id = ANY($1)`, []uuid.UUID{f.org, f.other}}, + {`DELETE FROM users WHERE id = $1`, f.owner}, + } { + if _, err := pool.Exec(c, step.sql, step.arg); err != nil { + t.Errorf("cleanup %q: %v", step.sql, err) + } + } + }) + return f +} + +// addKey inserts one key and returns its id. +func (f *apiKeyFixture) addKey(t *testing.T, pool *pgxpool.Pool, org uuid.UUID, status string, expiresAt *time.Time) uuid.UUID { + t.Helper() + id := uuid.New() + _, err := pool.Exec(context.Background(), + `INSERT INTO api_keys (id, user_id, organization_id, name, key_prefix, key_suffix, key_hash, permissions, status, expires_at) + VALUES ($1, $2, $3, 'Live key', 'wmbly_ab', 'wxyz', $4, 1, $5, $6)`, + id, f.owner, org, id.String(), status, expiresAt) + if err != nil { + t.Fatalf("insert key: %v", err) + } + return id +} + +func TestLiveAPIKeyDelete(t *testing.T) { + handle, pool := liveContactDB(t) + f := newAPIKeyFixture(t, pool) + repo := NewAPIKeyRepository(handle) + ctx := context.Background() + + past := time.Now().Add(-time.Hour) + future := time.Now().Add(time.Hour) + + t.Run("a revoked key is deleted with its usage logs", func(t *testing.T) { + id := f.addKey(t, pool, f.org, "revoked", nil) + if _, err := pool.Exec(ctx, + `INSERT INTO api_key_usage_logs (api_key_id, endpoint, method, ip_address, response_status, response_time_ms) + VALUES ($1, '/v1/campaigns', 'GET', '203.0.113.7', 200, 12)`, id); err != nil { + t.Fatalf("insert usage log: %v", err) + } + if xerr := repo.Delete(ctx, f.org, id); xerr != nil { + t.Fatalf("delete: %v", xerr) + } + if _, xerr := repo.GetByID(ctx, f.org, id); xerr != errx.ErrNotFound { + t.Fatalf("key still readable after delete: %v", xerr) + } + var logs int + if err := pool.QueryRow(ctx, `SELECT COUNT(*) FROM api_key_usage_logs WHERE api_key_id = $1`, id).Scan(&logs); err != nil { + t.Fatalf("count logs: %v", err) + } + if logs != 0 { + t.Fatalf("usage logs left behind: %d", logs) + } + }) + + t.Run("an active key is refused", func(t *testing.T) { + id := f.addKey(t, pool, f.org, "active", nil) + if xerr := repo.Delete(ctx, f.org, id); xerr != errx.ErrNotFound { + t.Fatalf("delete of an active key = %v, want not found", xerr) + } + if _, xerr := repo.GetByID(ctx, f.org, id); xerr != nil { + t.Fatalf("active key was deleted anyway: %v", xerr) + } + }) + + t.Run("an unexpired key is refused", func(t *testing.T) { + id := f.addKey(t, pool, f.org, "active", &future) + if xerr := repo.Delete(ctx, f.org, id); xerr != errx.ErrNotFound { + t.Fatalf("delete of an unexpired key = %v, want not found", xerr) + } + }) + + // The status column only ever holds 'active' or 'revoked'; expiry is + // applied when the key is read, so a key past expires_at is already dead + // and deleting it is allowed. + t.Run("a key past its expiry is deleted", func(t *testing.T) { + id := f.addKey(t, pool, f.org, "active", &past) + if xerr := repo.Delete(ctx, f.org, id); xerr != nil { + t.Fatalf("delete of an expired key: %v", xerr) + } + }) + + t.Run("another organization's key is not found", func(t *testing.T) { + id := f.addKey(t, pool, f.other, "revoked", nil) + if xerr := repo.Delete(ctx, f.org, id); xerr != errx.ErrNotFound { + t.Fatalf("cross-org delete = %v, want not found", xerr) + } + if _, xerr := repo.GetByID(ctx, f.other, id); xerr != nil { + t.Fatalf("another org's key was deleted: %v", xerr) + } + }) + + t.Run("deleting twice is not found the second time", func(t *testing.T) { + id := f.addKey(t, pool, f.org, "revoked", nil) + if xerr := repo.Delete(ctx, f.org, id); xerr != nil { + t.Fatalf("first delete: %v", xerr) + } + if xerr := repo.Delete(ctx, f.org, id); xerr != errx.ErrNotFound { + t.Fatalf("second delete = %v, want not found", xerr) + } + }) +} + +func TestAPIKeyCanAuthenticate(t *testing.T) { + past := time.Now().Add(-time.Hour) + future := time.Now().Add(time.Hour) + cases := []struct { + name string + key models.APIKey + want bool + }{ + {"active, no expiry", models.APIKey{Status: models.APIKeyStatusActive}, true}, + {"active, expires later", models.APIKey{Status: models.APIKeyStatusActive, ExpiresAt: &future}, true}, + {"active, already expired", models.APIKey{Status: models.APIKeyStatusActive, ExpiresAt: &past}, false}, + {"revoked", models.APIKey{Status: models.APIKeyStatusRevoked}, false}, + {"revoked, expires later", models.APIKey{Status: models.APIKeyStatusRevoked, ExpiresAt: &future}, false}, + {"expired", models.APIKey{Status: models.APIKeyStatusExpired}, false}, + } + for _, tc := range cases { + if got := tc.key.CanAuthenticate(); got != tc.want { + t.Errorf("%s: CanAuthenticate() = %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/internal/repository/pg_api_key.go b/internal/repository/pg_api_key.go index 0015fa40..2ac2b494 100644 --- a/internal/repository/pg_api_key.go +++ b/internal/repository/pg_api_key.go @@ -22,6 +22,7 @@ type APIKeyRepository interface { List(ctx context.Context, orgID uuid.UUID, limit int, cursor *uuid.UUID) (*models.APIKeysResult, *errx.Error) Update(ctx context.Context, orgID, keyID uuid.UUID, data *models.UpdateAPIKey) (*models.APIKey, *errx.Error) Revoke(ctx context.Context, orgID, keyID uuid.UUID, reason string) *errx.Error + Delete(ctx context.Context, orgID, keyID uuid.UUID) *errx.Error UpdateLastUsed(ctx context.Context, keyID uuid.UUID, ip string) error LogUsage(ctx context.Context, log *models.APIKeyUsageLog) error @@ -282,6 +283,33 @@ func (r *apiKeyRepository) Revoke(ctx context.Context, orgID, keyID uuid.UUID, r return nil } +// Delete removes a key row for good. It refuses a key that can still +// authenticate: ending a live credential is Revoke's job, which leaves the row +// and the reason behind. A key past its expires_at is already dead even though +// the status column still reads 'active', so that one is deletable. The usage +// logs go with the row through the FK's ON DELETE CASCADE. +func (r *apiKeyRepository) Delete(ctx context.Context, orgID, keyID uuid.UUID) *errx.Error { + query := ` + DELETE FROM api_keys + WHERE organization_id = $1 AND id = $2 + AND (status <> 'active' OR (expires_at IS NOT NULL AND expires_at <= now())) + ` + + params := []any{orgID, keyID} + + cmd, err := r.DB.Exec(ctx, query, params...) + if err != nil { + db.CaptureError(err, query, params, "exec") + return errx.InternalError() + } + + if cmd.RowsAffected() == 0 { + return errx.ErrNotFound + } + + return nil +} + func (r *apiKeyRepository) UpdateLastUsed(ctx context.Context, keyID uuid.UUID, ip string) error { // Casting via NULLIF lets the same query handle "no IP available" (worker // background calls, tests) without erroring on an empty INET. diff --git a/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx b/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx index e1564fc0..cd925498 100644 --- a/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx +++ b/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx @@ -6,7 +6,7 @@ // - usage graph (24h request volume, status-code split) // - top endpoints // - recent request log -// - actions (revoke, edit name/description) +// - actions (revoke, delete once revoked, edit name/description) // // Slides in from the right; closes on backdrop click or Escape. @@ -23,6 +23,7 @@ import { NetworkIcon, RefreshCwIcon, ShieldCheckIcon, + Trash2Icon, TrashIcon, XIcon, } from "lucide-react"; @@ -33,9 +34,12 @@ import useAPIKeyAnalytics from "@/lib/api/hooks/app/api-keys/useAPIKeyAnalytics" import useAPIKeyUsageLogs from "@/lib/api/hooks/app/api-keys/useAPIKeyUsageLogs"; import useAPIPermissions from "@/lib/api/hooks/app/api-keys/useAPIPermissions"; import useRevokeAPIKey from "@/lib/api/hooks/app/api-keys/useRevokeAPIKey"; +import useDeleteAPIKey from "@/lib/api/hooks/app/api-keys/useDeleteAPIKey"; import useUpdateAPIKey from "@/lib/api/hooks/app/api-keys/useUpdateAPIKey"; import { StackedBars } from "./Sparkline"; import { useConfirm } from "@/hooks/context/confirm"; +import type { AppError } from "@/lib/api/client/normalizeError"; +import buildError from "@/lib/helper/buildError"; export default function KeyDetailDrawer({ apiKey, @@ -84,6 +88,7 @@ function Inner({ apiKey, onClose }: { apiKey: APIKey; onClose: () => void }) { const logs = useAPIKeyUsageLogs(apiKey.id, { limit: 50 }); const perms = useAPIPermissions(); const revoke = useRevokeAPIKey(); + const remove = useDeleteAPIKey(); const update = useUpdateAPIKey(); const confirm = useConfirm(); @@ -131,6 +136,23 @@ function Inner({ apiKey, onClose }: { apiKey: APIKey; onClose: () => void }) { ); } + // Deleting is the second step after revoking, never a shortcut past it: + // the backend refuses a key that can still authenticate. + function confirmDelete() { + confirm.show( + `Delete "${apiKey.name}" for good? The key and its request history are removed, and that cannot be undone.`, + async () => { + try { + await remove.mutateAsync(apiKey.id); + toast.success("Key deleted"); + onClose(); + } catch (err) { + toast.error(buildError(err as AppError)); + } + }, + ); + } + const grantedPermissions = React.useMemo(() => { if (!perms.data) return []; return perms.data.permissions.filter((p) => (apiKey.permissions & p.value) !== 0); @@ -385,7 +407,7 @@ function Inner({ apiKey, onClose }: { apiKey: APIKey; onClose: () => void }) { {/* Footer */} -
+
{apiKey.status === "active" ? ( ) : ( - - - Revoked {apiKey.revoked_at ? fmtRelative(apiKey.revoked_at) : ""} - {apiKey.revoked_reason ? ` · ${apiKey.revoked_reason}` : ""} - + <> + + + + Revoked {apiKey.revoked_at ? fmtRelative(apiKey.revoked_at) : ""} + {apiKey.revoked_reason ? ` · ${apiKey.revoked_reason}` : ""} + + + + )} - +
); diff --git a/web/src/lib/api/client/app/api-keys/deleteAPIKey.ts b/web/src/lib/api/client/app/api-keys/deleteAPIKey.ts new file mode 100644 index 00000000..2b2a6591 --- /dev/null +++ b/web/src/lib/api/client/app/api-keys/deleteAPIKey.ts @@ -0,0 +1,11 @@ +import Request from "../../Request"; + +// Permanent removal, distinct from revoking: the row and its usage logs go. +// The backend refuses a key that can still authenticate. +export default async function deleteAPIKey(id: string): Promise { + return await Request({ + method: "DELETE", + url: `/api-keys/${id}/permanent`, + authorization: true, + }); +} diff --git a/web/src/lib/api/hooks/app/api-keys/useDeleteAPIKey.ts b/web/src/lib/api/hooks/app/api-keys/useDeleteAPIKey.ts new file mode 100644 index 00000000..38fa6b2b --- /dev/null +++ b/web/src/lib/api/hooks/app/api-keys/useDeleteAPIKey.ts @@ -0,0 +1,13 @@ +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import deleteAPIKey from "@/lib/api/client/app/api-keys/deleteAPIKey"; + +export default function useDeleteAPIKey() { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: (id: string) => deleteAPIKey(id), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["api-keys"] }); + }, + }); +} From 804ac149fd558d94854bb5f25555d5a1d7d5781d Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Thu, 10 Sep 2026 05:13:38 -0700 Subject: [PATCH 2/4] feat: name the API key case in the docs' 409 section, so the error-codes page lists the state-based conflict alongside the duplicate-resource one --- docs/content/docs/api/error-codes.mdx | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index 6b5e533c..57220045 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -217,6 +217,7 @@ Returned when the request conflicts with existing data. **Common causes:** - Trying to create a resource that already exists - Duplicate unique values +- Deleting something whose state does not allow it yet, such as an API key that can still authenticate **Example:** From 1e1231f3b1e2b5365fd5e8de1015bae31fbd7e56 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Thu, 10 Sep 2026 05:23:56 -0700 Subject: [PATCH 3/4] feat: address the review of the API key delete, putting the permanent-delete route in the published OpenAPI contract with its 409, keeping one auto margin in the drawer footer so the Delete button lands on the right, and deciding the dashboard's status pill, its footer and the key-count strip on whether the key can still authenticate rather than on a status column that never says expired --- docs/content/docs/api/reference/api-keys.mdx | 2 + docs/public/openapi.json | 115 ++++++++++++++++++ .../repository/api_key_delete_live_test.go | 30 +++++ internal/repository/pg_api_key.go | 8 +- .../api-keys/_components/KeyDetailDrawer.tsx | 35 ++++-- web/src/app/app/api-keys/page.tsx | 10 +- web/src/lib/api/models/app/apikeys/APIKey.ts | 14 +++ 7 files changed, 198 insertions(+), 16 deletions(-) diff --git a/docs/content/docs/api/reference/api-keys.mdx b/docs/content/docs/api/reference/api-keys.mdx index 2dcaf800..c5d70b97 100644 --- a/docs/content/docs/api/reference/api-keys.mdx +++ b/docs/content/docs/api/reference/api-keys.mdx @@ -21,6 +21,8 @@ When you create a key, the response includes a `secret` field containing the ful Returns the organization's API keys, newest first, with the secret never included. +`status` is `active` or `revoked`. Expiry is applied when a key is read, not written to the row, so a key past its `expires_at` still reports `active` here while authenticating nothing. Compare `expires_at` against the current time if you need to know whether a key is usable. + Auth: **Scope** `API_KEYS` · **Org permission** `manage_api_keys` | Parameter | In | Type | Description | diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 10956702..7a0ca06e 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -11800,6 +11800,107 @@ } } }, + "/api-keys/{id}/permanent": { + "delete": { + "operationId": "api-keys_delete", + "summary": "Delete an API key", + "description": "Removes a revoked or expired key from the workspace for good, along with its usage logs. A key that could still authenticate is refused with a 409: revoke it first, so what ended the credential stays on the record. A key past its expires_at can be deleted directly, since it already authenticates nothing.", + "tags": [ + "api-keys" + ], + "security": [ + { + "bearerAuth": [] + } + ], + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + }, + { + "$ref": "#/components/parameters/IdempotencyKey" + } + ], + "responses": { + "200": { + "description": "Deletion status envelope.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/APIKeyDeleteResult" + } + } + } + }, + "400": { + "description": "No organization selected.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Unauthenticated.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Missing API_KEYS scope or manage_api_keys org permission.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Invalid UUID or key not found in this organization.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "409": { + "description": "The key can still authenticate. Revoke it first.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + } + } + }, "/api-keys/{id}/analytics": { "get": { "operationId": "api-keys_analytics", @@ -25007,6 +25108,20 @@ } } }, + "APIKeyDeleteResult": { + "type": "object", + "required": [ + "status" + ], + "properties": { + "status": { + "type": "string", + "enum": [ + "deleted" + ] + } + } + }, "APIPermission": { "type": "object", "required": [ diff --git a/internal/repository/api_key_delete_live_test.go b/internal/repository/api_key_delete_live_test.go index f2a8c09f..234b97c2 100644 --- a/internal/repository/api_key_delete_live_test.go +++ b/internal/repository/api_key_delete_live_test.go @@ -161,6 +161,36 @@ func TestLiveAPIKeyDelete(t *testing.T) { }) } +// The dashboard's key-count strip has to agree with the status the list shows +// for the same key: one past its expires_at is expired, not active. +func TestLiveAPIKeyUsageSummaryCountsExpiry(t *testing.T) { + handle, pool := liveContactDB(t) + f := newAPIKeyFixture(t, pool) + repo := NewAPIKeyRepository(handle) + ctx := context.Background() + + past := time.Now().Add(-time.Hour) + future := time.Now().Add(time.Hour) + f.addKey(t, pool, f.org, "active", nil) + f.addKey(t, pool, f.org, "active", &future) + f.addKey(t, pool, f.org, "active", &past) + f.addKey(t, pool, f.org, "revoked", nil) + + sum, xerr := repo.GetUsageSummary(ctx, f.org) + if xerr != nil { + t.Fatalf("usage summary: %v", xerr) + } + if sum.ActiveKeys != 2 { + t.Errorf("active_keys = %d, want 2", sum.ActiveKeys) + } + if sum.ExpiredKeys != 1 { + t.Errorf("expired_keys = %d, want 1", sum.ExpiredKeys) + } + if sum.RevokedKeys != 1 { + t.Errorf("revoked_keys = %d, want 1", sum.RevokedKeys) + } +} + func TestAPIKeyCanAuthenticate(t *testing.T) { past := time.Now().Add(-time.Hour) future := time.Now().Add(time.Hour) diff --git a/internal/repository/pg_api_key.go b/internal/repository/pg_api_key.go index 2ac2b494..ef7d7c10 100644 --- a/internal/repository/pg_api_key.go +++ b/internal/repository/pg_api_key.go @@ -350,10 +350,14 @@ func (r *apiKeyRepository) LogUsage(ctx context.Context, log *models.APIKeyUsage func (r *apiKeyRepository) GetUsageSummary(ctx context.Context, orgID uuid.UUID) (*models.APIKeyUsageSummary, *errx.Error) { query := ` WITH key_counts AS ( + -- The status column only ever holds 'active' or 'revoked'; expiry + -- is applied when a key is read, so a key past expires_at counts + -- as expired here rather than inflating the active total. SELECT - COUNT(*) FILTER (WHERE status = 'active') AS active_keys, + COUNT(*) FILTER (WHERE status = 'active' AND (expires_at IS NULL OR expires_at > now())) AS active_keys, COUNT(*) FILTER (WHERE status = 'revoked') AS revoked_keys, - COUNT(*) FILTER (WHERE status = 'expired') AS expired_keys + COUNT(*) FILTER (WHERE status = 'expired' + OR (status = 'active' AND expires_at IS NOT NULL AND expires_at <= now())) AS expired_keys FROM api_keys WHERE organization_id = $1 ), diff --git a/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx b/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx index cd925498..f4543e8d 100644 --- a/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx +++ b/web/src/app/app/api-keys/_components/KeyDetailDrawer.tsx @@ -30,6 +30,7 @@ import { import toast from "react-hot-toast"; import type APIKey from "@/lib/api/models/app/apikeys/APIKey"; +import { keyCanAuthenticate, keyStatus } from "@/lib/api/models/app/apikeys/APIKey"; import useAPIKeyAnalytics from "@/lib/api/hooks/app/api-keys/useAPIKeyAnalytics"; import useAPIKeyUsageLogs from "@/lib/api/hooks/app/api-keys/useAPIKeyUsageLogs"; import useAPIPermissions from "@/lib/api/hooks/app/api-keys/useAPIPermissions"; @@ -169,7 +170,7 @@ function Inner({ apiKey, onClose }: { apiKey: APIKey; onClose: () => void }) { {apiKey.key_prefix}…{apiKey.key_suffix} - + ) : ( <> - - - - Revoked {apiKey.revoked_at ? fmtRelative(apiKey.revoked_at) : ""} - {apiKey.revoked_reason ? ` · ${apiKey.revoked_reason}` : ""} - + {/* flex-1 rather than a second ml-auto: two auto margins + in one row split the free space and park the button + in the middle of the footer. */} + + + {endedNote(apiKey)}