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/cmd/warmblyctl/api_resources.go b/cmd/warmblyctl/api_resources.go index 98edd48e..690deace 100644 --- a/cmd/warmblyctl/api_resources.go +++ b/cmd/warmblyctl/api_resources.go @@ -146,12 +146,14 @@ var apiSpecs = []apiSpec{ {name: "webhook deliveries", summary: "Recent deliveries across endpoints", method: "GET", path: "/webhooks/deliveries", query: []string{"limit", "cursor"}}, {name: "webhook event-types", summary: "Every event type a webhook can subscribe to", method: "GET", path: "/webhooks/event-types"}, - // API keys (self-service). + // API keys (self-service). `purge` rather than `delete` to match + // `warmbly key purge`, where `delete` is already an alias of `revoke`. {name: "apikey list", summary: "List the organization's API keys", method: "GET", path: "/api-keys"}, {name: "apikey get", summary: "Get one API key", method: "GET", path: "/api-keys/{id}"}, {name: "apikey create", summary: "Create an API key; the secret is only ever in this response", method: "POST", path: "/api-keys", body: bodyRequired}, {name: "apikey update", summary: "Update an API key's name, scopes or restrictions", method: "PATCH", path: "/api-keys/{id}", body: bodyRequired}, {name: "apikey revoke", summary: "Revoke an API key", method: "DELETE", path: "/api-keys/{id}"}, + {name: "apikey purge", summary: "Delete a revoked API key for good, with its usage logs; a key that can still authenticate is refused", method: "DELETE", path: "/api-keys/{id}/permanent"}, {name: "apikey permissions", summary: "Every grantable scope with its bit value", method: "GET", path: "/api-keys/permissions"}, // Templates. 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/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:** diff --git a/docs/content/docs/api/reference/api-keys.mdx b/docs/content/docs/api/reference/api-keys.mdx index 1ba46be7..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 | @@ -262,6 +264,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 +408,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/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/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 4b50cd2c..067d1bff 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -810,6 +810,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..234b97c2 --- /dev/null +++ b/internal/repository/api_key_delete_live_test.go @@ -0,0 +1,214 @@ +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) + } + }) +} + +// 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) + 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..ef7d7c10 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. @@ -322,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 e1564fc0..f4543e8d 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,19 +23,24 @@ import { NetworkIcon, RefreshCwIcon, ShieldCheckIcon, + Trash2Icon, TrashIcon, XIcon, } from "lucide-react"; 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"; 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 +89,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 +137,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); @@ -147,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)} + + + )} - + ); } +// What ended the key, for the footer of a key that can no longer authenticate. +function endedNote(key: APIKey): string { + if (keyStatus(key) === "expired") { + return `Expired ${key.expires_at ? fmtRelative(key.expires_at) : ""}`.trim(); + } + const when = key.revoked_at ? ` ${fmtRelative(key.revoked_at)}` : ""; + const why = key.revoked_reason ? ` · ${key.revoked_reason}` : ""; + return `Revoked${when}${why}`; +} + function StatusBadge({ status }: { status: "active" | "revoked" | "expired" }) { const tone = status === "active" diff --git a/web/src/app/app/api-keys/page.tsx b/web/src/app/app/api-keys/page.tsx index 14117c88..b07fe13c 100644 --- a/web/src/app/app/api-keys/page.tsx +++ b/web/src/app/app/api-keys/page.tsx @@ -48,6 +48,7 @@ import useAPIKeys from "@/lib/api/hooks/app/api-keys/useAPIKeys"; import useAPIKeyUsageSummary from "@/lib/api/hooks/app/api-keys/useAPIKeyUsageSummary"; import useAPIKeyAnalytics from "@/lib/api/hooks/app/api-keys/useAPIKeyAnalytics"; import type APIKey from "@/lib/api/models/app/apikeys/APIKey"; +import { keyCanAuthenticate, keyStatus } from "@/lib/api/models/app/apikeys/APIKey"; import CreateKeyModal from "./_components/CreateKeyModal"; import KeyDetailDrawer from "./_components/KeyDetailDrawer"; import { StackedBars } from "./_components/Sparkline"; @@ -204,7 +205,10 @@ export default function APIKeysPage() { } function KeyRow({ apiKey, onClick }: { apiKey: APIKey; onClick: () => void }) { - const status = apiKey.status; + // A key past its expires_at still reads "active" in the column, so the row + // asks the model rather than the field. + const status = keyStatus(apiKey); + const live = keyCanAuthenticate(apiKey); return (