Add optional API key expiry and a missing way to list them
Part of the same security-hardening pass as the last five commits, and
the last item in its backlog. User API keys had no expiry at all --
unlike service-account keys, visibly distinct only by their "tdsa_"
prefix -- and, it turns out while implementing this, no way to list
them either: only create (returns the raw key once) and delete-by-id
existed, so a key's owner had no way to even discover what keys they
had short of remembering IDs from creation time.
handleCreateAPIKey takes an optional expires_in_days (0, the default,
keeps today's behavior: never expires, so no existing integration is
affected). apiKeyUser's lookup now carries `expires_at IS NULL OR
expires_at > now` as part of the query itself, the same way serveAs's
disabled_at check already works -- an expired key simply fails to
resolve, like a wrong one, rather than resolving and being caught
after the fact. New GET /api/users/{id}/api-keys (requireSelfOrAdmin,
same as create/delete) lists id/name/created_at/last_used_at/expires_at,
never the raw key.
Scoped down from the original plan on request: no web UI change, since
there turned out to be no existing API-keys UI at all to extend --
building one from scratch would have been a real feature addition, not
a hardening tweak.
Mirrored the additive expires_at field in terdut-tui's APIKey struct
(separate commit, separate repo) per this workspace's version-coupling
rule; the TUI does not create or list expiring keys itself yet.
New tests (api_keys_test.go): default never-expires, expires_in_days
sets expires_at, out-of-range values rejected, an expired key fails
auth after a fresh one worked, the listing never includes the raw key.
Also added the new GET route to authz_scope_test.go's self-or-admin
table from the previous commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -165,6 +165,7 @@ func TestAuthzScope_SelfOrAdminRoutesRefuseAnotherNonAdminUser(t *testing.T) {
|
|||||||
{http.MethodGet, "/api/users/" + bUserID + "/teams"},
|
{http.MethodGet, "/api/users/" + bUserID + "/teams"},
|
||||||
{http.MethodPut, "/api/users/" + bUserID + "/notify"},
|
{http.MethodPut, "/api/users/" + bUserID + "/notify"},
|
||||||
{http.MethodPut, "/api/users/" + bUserID + "/password"},
|
{http.MethodPut, "/api/users/" + bUserID + "/password"},
|
||||||
|
{http.MethodGet, "/api/users/" + bUserID + "/api-keys"},
|
||||||
{http.MethodPost, "/api/users/" + bUserID + "/api-keys"},
|
{http.MethodPost, "/api/users/" + bUserID + "/api-keys"},
|
||||||
{http.MethodDelete, "/api/users/" + bUserID + "/api-keys/1"},
|
{http.MethodDelete, "/api/users/" + bUserID + "/api-keys/1"},
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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.
|
// 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) {
|
func apiKeyUser(ctx context.Context, db *sql.DB, token string) (int64, bool) {
|
||||||
var keyID, userID int64
|
var keyID, userID int64
|
||||||
err := db.QueryRowContext(ctx,
|
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)
|
).Scan(&keyID, &userID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, false
|
return 0, false
|
||||||
|
|||||||
@@ -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.Get("/api/users/{id}/teams", handleUserTeams(db))
|
||||||
r.Put("/api/users/{id}/notify", handleSetNotifyTarget(db))
|
r.Put("/api/users/{id}/notify", handleSetNotifyTarget(db))
|
||||||
r.Put("/api/users/{id}/password", handleSetPassword(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.Post("/api/users/{id}/api-keys", handleCreateAPIKey(db))
|
||||||
r.Delete("/api/users/{id}/api-keys/{keyID}", handleDeleteAPIKey(db))
|
r.Delete("/api/users/{id}/api-keys/{keyID}", handleDeleteAPIKey(db))
|
||||||
|
|
||||||
|
|||||||
+76
-3
@@ -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 {
|
func handleCreateAPIKey(db *sql.DB) http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64)
|
userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64)
|
||||||
@@ -247,6 +252,11 @@ func handleCreateAPIKey(db *sql.DB) http.HandlerFunc {
|
|||||||
|
|
||||||
var req struct {
|
var req struct {
|
||||||
Name string `json:"name"`
|
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 {
|
if err := decodeJSON(r, &req); err != nil {
|
||||||
respond(w, http.StatusBadRequest, errResp("invalid request body"))
|
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"))
|
respond(w, http.StatusBadRequest, errResp("name is required"))
|
||||||
return
|
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
|
var exists int
|
||||||
if err := db.QueryRowContext(r.Context(), "SELECT 1 FROM users WHERE id = $1", userID).Scan(&exists); err != nil {
|
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"))
|
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||||
return
|
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
|
var keyID int64
|
||||||
if err := db.QueryRowContext(r.Context(),
|
if err := db.QueryRowContext(r.Context(),
|
||||||
"INSERT INTO api_keys (user_id, key_hash, name) VALUES ($1, $2, $3) RETURNING id",
|
"INSERT INTO api_keys (user_id, key_hash, name, expires_at) VALUES ($1, $2, $3, $4) RETURNING id",
|
||||||
userID, hash, req.Name).Scan(&keyID); err != nil {
|
userID, hash, req.Name, expiresAt).Scan(&keyID); err != nil {
|
||||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||||
return
|
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)
|
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 {
|
func handleDeleteAPIKey(db *sql.DB) http.HandlerFunc {
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64)
|
userID, err := strconv.ParseInt(chi.URLParam(r, "id"), 10, 64)
|
||||||
|
|||||||
@@ -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;
|
||||||
@@ -37,5 +37,13 @@ type APIKey struct {
|
|||||||
Name string `json:"name"`
|
Name string `json:"name"`
|
||||||
CreatedAt time.Time `json:"created_at"`
|
CreatedAt time.Time `json:"created_at"`
|
||||||
LastUsedAt *time.Time `json:"last_used_at,omitempty"`
|
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
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user