diff --git a/internal/api/api_keys_test.go b/internal/api/api_keys_test.go new file mode 100644 index 0000000..3ac06f2 --- /dev/null +++ b/internal/api/api_keys_test.go @@ -0,0 +1,135 @@ +package api_test + +import ( + "net/http" + "testing" + "time" +) + +func TestAPIKey_DefaultsToNeverExpiring(t *testing.T) { + s := newTS(t) + + var key struct { + Key string `json:"key"` + ExpiresAt *string `json:"expires_at"` + } + decode(t, s.req(t, http.MethodPost, "/api/users/1/api-keys", + map[string]string{"name": "no-expiry"}), &key) + + if key.ExpiresAt != nil { + t.Errorf("expires_at = %v, want nil (unset expires_in_days means never expires)", *key.ExpiresAt) + } +} + +func TestAPIKey_ExpiresInDaysSetsExpiresAt(t *testing.T) { + s := newTS(t) + + var key struct { + ID int64 `json:"id"` + ExpiresAt *string `json:"expires_at"` + } + decode(t, s.req(t, http.MethodPost, "/api/users/1/api-keys", + map[string]any{"name": "rotates", "expires_in_days": 30}), &key) + + if key.ExpiresAt == nil { + t.Fatal("expires_at = nil, want a timestamp roughly 30 days out") + } + got, err := time.Parse(time.RFC3339, *key.ExpiresAt) + if err != nil { + t.Fatalf("parse expires_at: %v", err) + } + want := time.Now().AddDate(0, 0, 30) + if diff := want.Sub(got).Abs(); diff > time.Hour { + t.Errorf("expires_at = %v, want close to %v (30 days out)", got, want) + } +} + +func TestAPIKey_ExpiresInDaysRejectsOutOfRange(t *testing.T) { + s := newTS(t) + + for _, days := range []int{-1, 3651} { + resp := s.req(t, http.MethodPost, "/api/users/1/api-keys", + map[string]any{"name": "bad", "expires_in_days": days}) + resp.Body.Close() + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("expires_in_days=%d: status = %d, want %d", days, resp.StatusCode, http.StatusBadRequest) + } + } +} + +func TestAPIKey_AnExpiredKeyCannotAuthenticate(t *testing.T) { + s := newTS(t) + + var key struct { + ID int64 `json:"id"` + Key string `json:"key"` + } + decode(t, s.req(t, http.MethodPost, "/api/users/1/api-keys", + map[string]any{"name": "soon-expired", "expires_in_days": 1}), &key) + + // A fresh key works... + req, _ := http.NewRequest(http.MethodGet, s.URL+"/api/me", nil) + req.Header.Set("Authorization", "Bearer "+key.Key) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("GET /api/me: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("fresh key: status = %d, want %d", resp.StatusCode, http.StatusOK) + } + + // ...and stops working once its expiry has passed. + s.exec(t, "UPDATE api_keys SET expires_at = $1 WHERE id = $2", time.Now().Add(-time.Hour).Unix(), key.ID) + + req2, _ := http.NewRequest(http.MethodGet, s.URL+"/api/me", nil) + req2.Header.Set("Authorization", "Bearer "+key.Key) + resp2, err := http.DefaultClient.Do(req2) + if err != nil { + t.Fatalf("GET /api/me: %v", err) + } + defer resp2.Body.Close() + if resp2.StatusCode != http.StatusUnauthorized { + t.Errorf("expired key: status = %d, want %d", resp2.StatusCode, http.StatusUnauthorized) + } +} + +func TestAPIKey_ListNeverReturnsTheRawKey(t *testing.T) { + s := newTS(t) + + decode(t, s.req(t, http.MethodPost, "/api/users/1/api-keys", + map[string]string{"name": "listed"}), new(struct { + Key string `json:"key"` + })) + + var keys []struct { + ID int64 `json:"id"` + Name string `json:"name"` + Key string `json:"key"` + } + decode(t, s.req(t, http.MethodGet, "/api/users/1/api-keys", nil), &keys) + + found := false + for _, k := range keys { + if k.Name == "listed" { + found = true + } + if k.Key != "" { + t.Errorf("key %d (%s): raw key present in listing", k.ID, k.Name) + } + } + if !found { + t.Error("the key just created does not appear in the listing") + } +} + +func TestAPIKey_ListIsSelfOrAdmin(t *testing.T) { + s := newTS(t) + a := newTeam(t, s, "apikeys-a") + + resp := a.call(http.MethodGet, "/api/users/1/api-keys", nil) + defer resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("status = %d, want %d (not self, not an admin)", resp.StatusCode, http.StatusForbidden) + } +} diff --git a/internal/api/authz_scope_test.go b/internal/api/authz_scope_test.go index 1b0cd58..07778b5 100644 --- a/internal/api/authz_scope_test.go +++ b/internal/api/authz_scope_test.go @@ -165,6 +165,7 @@ func TestAuthzScope_SelfOrAdminRoutesRefuseAnotherNonAdminUser(t *testing.T) { {http.MethodGet, "/api/users/" + bUserID + "/teams"}, {http.MethodPut, "/api/users/" + bUserID + "/notify"}, {http.MethodPut, "/api/users/" + bUserID + "/password"}, + {http.MethodGet, "/api/users/" + bUserID + "/api-keys"}, {http.MethodPost, "/api/users/" + bUserID + "/api-keys"}, {http.MethodDelete, "/api/users/" + bUserID + "/api-keys/1"}, } diff --git a/internal/api/middleware.go b/internal/api/middleware.go index efae813..ae0aee7 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -135,10 +135,16 @@ func requireSelfOrAdmin(w http.ResponseWriter, r *http.Request, targetID int64) } // apiKeyUser resolves an API key to its user and stamps its last use. +// expires_at IS NULL OR > now is part of the lookup itself, the same way +// serveAs's disabled_at check is: an expired key is one that cannot +// authenticate, by construction, rather than one that happens to still +// resolve and has to be caught afterwards. func apiKeyUser(ctx context.Context, db *sql.DB, token string) (int64, bool) { var keyID, userID int64 err := db.QueryRowContext(ctx, - "SELECT id, user_id FROM api_keys WHERE key_hash = $1", hashToken(token), + `SELECT id, user_id FROM api_keys + WHERE key_hash = $1 AND (expires_at IS NULL OR expires_at > $2)`, + hashToken(token), time.Now().Unix(), ).Scan(&keyID, &userID) if err != nil { return 0, false diff --git a/internal/api/router.go b/internal/api/router.go index cfe27b4..f0c5ae5 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -112,6 +112,7 @@ func NewRouter(db *sql.DB, notify NotifyConfig, cfg config.Config, version strin r.Get("/api/users/{id}/teams", handleUserTeams(db)) r.Put("/api/users/{id}/notify", handleSetNotifyTarget(db)) r.Put("/api/users/{id}/password", handleSetPassword(db)) + r.Get("/api/users/{id}/api-keys", handleListAPIKeys(db)) r.Post("/api/users/{id}/api-keys", handleCreateAPIKey(db)) r.Delete("/api/users/{id}/api-keys/{keyID}", handleDeleteAPIKey(db)) diff --git a/internal/api/users.go b/internal/api/users.go index 0fb0d11..93c7c15 100644 --- a/internal/api/users.go +++ b/internal/api/users.go @@ -234,6 +234,11 @@ func handleDeleteUser(db *sql.DB) http.HandlerFunc { } } +// maxAPIKeyExpiryDays bounds expires_in_days: generous enough for any real +// rotation policy, tight enough to reject a typo (a year in hours, say) that +// would otherwise mint a key that outlives the server by decades. +const maxAPIKeyExpiryDays = 3650 // ~10 years + func handleCreateAPIKey(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64) @@ -247,6 +252,11 @@ func handleCreateAPIKey(db *sql.DB) http.HandlerFunc { var req struct { Name string `json:"name"` + // ExpiresInDays is optional and, left zero, means the key never + // expires — the only behavior any key had before this field + // existed, so an existing integration that does not send it is + // unaffected. + ExpiresInDays int64 `json:"expires_in_days,omitempty"` } if err := decodeJSON(r, &req); err != nil { respond(w, http.StatusBadRequest, errResp("invalid request body")) @@ -256,6 +266,10 @@ func handleCreateAPIKey(db *sql.DB) http.HandlerFunc { respond(w, http.StatusBadRequest, errResp("name is required")) return } + if req.ExpiresInDays < 0 || req.ExpiresInDays > maxAPIKeyExpiryDays { + respond(w, http.StatusBadRequest, errResp("expires_in_days must be 0 (never expires) or up to "+strconv.Itoa(maxAPIKeyExpiryDays))) + return + } var exists int if err := db.QueryRowContext(r.Context(), "SELECT 1 FROM users WHERE id = $1", userID).Scan(&exists); err != nil { @@ -268,18 +282,77 @@ func handleCreateAPIKey(db *sql.DB) http.HandlerFunc { respond(w, http.StatusInternalServerError, errResp("internal error")) return } + var expiresAt *int64 + var expiresAtTime *time.Time + if req.ExpiresInDays > 0 { + t := time.Now().AddDate(0, 0, int(req.ExpiresInDays)).UTC() + u := t.Unix() + expiresAt = &u + expiresAtTime = &t + } var keyID int64 if err := db.QueryRowContext(r.Context(), - "INSERT INTO api_keys (user_id, key_hash, name) VALUES ($1, $2, $3) RETURNING id", - userID, hash, req.Name).Scan(&keyID); err != nil { + "INSERT INTO api_keys (user_id, key_hash, name, expires_at) VALUES ($1, $2, $3, $4) RETURNING id", + userID, hash, req.Name, expiresAt).Scan(&keyID); err != nil { respond(w, http.StatusInternalServerError, errResp("internal error")) return } - key := models.APIKey{ID: keyID, UserID: userID, Name: req.Name, Key: raw, CreatedAt: time.Now().UTC()} + key := models.APIKey{ + ID: keyID, UserID: userID, Name: req.Name, Key: raw, + CreatedAt: time.Now().UTC(), ExpiresAt: expiresAtTime, + } respond(w, http.StatusCreated, key) } } +// handleListAPIKeys lists a user's own API keys: never the raw key itself +// (only ever returned once, at creation), just enough to tell them apart, +// see which are stale (last_used_at) and which are about to stop working +// (expires_at) — the data handleCreateAPIKey and apiKeyUser's last-use stamp +// already produce, with no endpoint to read it back until now. +func handleListAPIKeys(db *sql.DB) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64) + if err != nil { + respond(w, http.StatusBadRequest, errResp("invalid user id")) + return + } + if !requireSelfOrAdmin(w, r, userID) { + return + } + + rows, err := db.QueryContext(r.Context(), + `SELECT id, name, created_at, last_used_at, expires_at + FROM api_keys WHERE user_id = $1 ORDER BY created_at DESC`, userID) + if err != nil { + respond(w, http.StatusInternalServerError, errResp("internal error")) + return + } + defer rows.Close() + + keys := []models.APIKey{} + for rows.Next() { + var k models.APIKey + var created int64 + var lastUsed, expires *int64 + if err := rows.Scan(&k.ID, &k.Name, &created, &lastUsed, &expires); err != nil { + respond(w, http.StatusInternalServerError, errResp("internal error")) + return + } + k.UserID = userID + k.CreatedAt = time.Unix(created, 0).UTC() + k.LastUsedAt = unixPtr(lastUsed) + k.ExpiresAt = unixPtr(expires) + keys = append(keys, k) + } + if err := rows.Err(); err != nil { + respond(w, http.StatusInternalServerError, errResp("internal error")) + return + } + respond(w, http.StatusOK, keys) + } +} + func handleDeleteAPIKey(db *sql.DB) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64) diff --git a/internal/db/migrations/017_api_key_expiry.sql b/internal/db/migrations/017_api_key_expiry.sql new file mode 100644 index 0000000..17f8ab5 --- /dev/null +++ b/internal/db/migrations/017_api_key_expiry.sql @@ -0,0 +1,7 @@ +-- Optional expiry on a user's own API keys. NULL (the existing default for +-- every row already in this table) means "never expires" -- the same +-- behavior these keys have always had, so no existing integration breaks. +-- Service account keys are deliberately NOT touched: they are a different +-- table, managed by automation, and already distinguished by their own +-- "tdsa_" prefix. +ALTER TABLE api_keys ADD COLUMN expires_at BIGINT; diff --git a/internal/models/user.go b/internal/models/user.go index 7a93ffa..ce60178 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -37,5 +37,13 @@ type APIKey struct { Name string `json:"name"` CreatedAt time.Time `json:"created_at"` LastUsedAt *time.Time `json:"last_used_at,omitempty"` - Key string `json:"key,omitempty"` // populated only on creation, never stored + + // ExpiresAt is nil for a key that never expires, which is every key + // created before this field existed and still the default for a new one + // unless its creator asks otherwise (see handleCreateAPIKey's + // expires_in_days). AuthMiddleware stops accepting a key once this + // passes; nothing deletes the row for it. + ExpiresAt *time.Time `json:"expires_at,omitempty"` + + Key string `json:"key,omitempty"` // populated only on creation, never stored }