From 629d51591183b6c11154a24f33627431571ff2b8 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Sun, 6 Sep 2026 19:22:47 +0200 Subject: [PATCH] overrides --- internal/api/store.go | 9 +- internal/db/convert.go | 58 ++++++++++ internal/db/keys.sql.go | 229 ++++++++++++++++++++++++++++++++++++++-- internal/db/models.go | 6 +- queries/keys.sql | 59 ++++++++++- 5 files changed, 340 insertions(+), 21 deletions(-) create mode 100644 internal/db/convert.go diff --git a/internal/api/store.go b/internal/api/store.go index d144127..7ba91b5 100644 --- a/internal/api/store.go +++ b/internal/api/store.go @@ -10,6 +10,7 @@ import ( ) // PgKeyStore adapts the sqlc-generated queries to the KeyStore interface. +// // The indirection earns its keep twice: the middleware is testable without a // database, and adding an unrelated column to api.keys does not ripple into the // auth package. @@ -38,10 +39,10 @@ func (s PgKeyStore) KeyByHash(ctx context.Context, hash []byte) (KeyRecord, erro OwnerStatus: row.OwnerStatus, } - // sqlc with emit_pointers_for_null_types gives *time.Time for a nullable - // timestamptz. If your generated code uses pgtype.Timestamptz instead, - // convert here rather than in the middleware. - rec.ExpiresAt = row.ExpiresAt + // pgx maps a nullable timestamptz to pgtype.Timestamptz, which carries its + // own validity flag rather than being nil. db.Time does the conversion so + // pgtype never leaks past this adapter. + rec.ExpiresAt = db.Time(row.ExpiresAt) return rec, nil } diff --git a/internal/db/convert.go b/internal/db/convert.go new file mode 100644 index 0000000..b9e4691 --- /dev/null +++ b/internal/db/convert.go @@ -0,0 +1,58 @@ +package db + +import ( + "time" + + "github.com/jackc/pgx/v5/pgtype" +) + +// Conversions between pgtype and plain Go types. +// sqlc with sql_package: pgx/v5 maps a nullable timestamptz to +// pgtype.Timestamptz regardless of emit_pointers_for_null_types, and the +// db_type override for it is fiddly to get right across sqlc versions. Doing +// the conversion here is two lines, always works, and keeps pgtype out of +// internal/api and cmd/ entirely. +// This file is hand-written and is NOT regenerated by sqlc. It lives in the db +// package so the pgtype import stays in one place. + +// Time converts a nullable timestamptz to *time.Time. NULL becomes nil. +func Time(t pgtype.Timestamptz) *time.Time { + if !t.Valid { + return nil + } + + v := t.Time + + return &v +} + +// Timestamptz converts *time.Time back for a query parameter. nil becomes NULL. +func Timestamptz(t *time.Time) pgtype.Timestamptz { + if t == nil { + return pgtype.Timestamptz{} + } + + return pgtype.Timestamptz{Time: *t, Valid: true} +} + +// Str converts a nullable text column to *string. +func Str(t pgtype.Text) *string { + if !t.Valid { + return nil + } + + v := t.String + + return &v +} + +// Text converts a string for a nullable text parameter. An empty string becomes +// NULL: an absent label or callsign is absence, not a value, and the +// core.callsign domain would reject "" anyway. +func Text(s string) pgtype.Text { + if s == "" { + return pgtype.Text{} + } + + return pgtype.Text{String: s, Valid: true} +} diff --git a/internal/db/keys.sql.go b/internal/db/keys.sql.go index 62248c7..44755b6 100644 --- a/internal/db/keys.sql.go +++ b/internal/db/keys.sql.go @@ -11,15 +11,102 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) -const getKeyByHash = `-- name: GetKeyByHash :one -SELECT k.id, k.owner_id, k.scopes, k.quota_tier, k.status, k.expires_at, - o.status AS owner_status -FROM api.keys k -JOIN api.owners o ON o.id = k.owner_id -WHERE k.key_hash = $1 AND k.status = 'active' +const createKey = `-- name: CreateKey :one +INSERT INTO api.keys (owner_id, key_hash, key_prefix, label, scopes, quota_tier, expires_at) +VALUES ($1, $2, $3, $4, $5, $6, $7) +RETURNING id, key_prefix, scopes, quota_tier, status, created_at, expires_at ` -type GetKeyByHashRow struct { +type CreateKeyParams struct { + OwnerID int64 + KeyHash []byte + KeyPrefix string + Label *string + Scopes []string + QuotaTier string + ExpiresAt pgtype.Timestamptz +} + +type CreateKeyRow struct { + ID int64 + KeyPrefix string + Scopes []string + QuotaTier string + Status string + CreatedAt pgtype.Timestamptz + ExpiresAt pgtype.Timestamptz +} + +func (q *Queries) CreateKey(ctx context.Context, arg CreateKeyParams) (CreateKeyRow, error) { + row := q.db.QueryRow(ctx, createKey, + arg.OwnerID, + arg.KeyHash, + arg.KeyPrefix, + arg.Label, + arg.Scopes, + arg.QuotaTier, + arg.ExpiresAt, + ) + var i CreateKeyRow + err := row.Scan( + &i.ID, + &i.KeyPrefix, + &i.Scopes, + &i.QuotaTier, + &i.Status, + &i.CreatedAt, + &i.ExpiresAt, + ) + return i, err +} + +const createOwner = `-- name: CreateOwner :one +INSERT INTO api.owners (email, callsign) +VALUES ($1, $2) +RETURNING id, email, callsign, status, created_at +` + +type CreateOwnerParams struct { + Email string + Callsign interface{} +} + +type CreateOwnerRow struct { + ID int64 + Email string + Callsign interface{} + Status string + CreatedAt pgtype.Timestamptz +} + +func (q *Queries) CreateOwner(ctx context.Context, arg CreateOwnerParams) (CreateOwnerRow, error) { + row := q.db.QueryRow(ctx, createOwner, arg.Email, arg.Callsign) + var i CreateOwnerRow + err := row.Scan( + &i.ID, + &i.Email, + &i.Callsign, + &i.Status, + &i.CreatedAt, + ) + return i, err +} + +const keyByHash = `-- name: KeyByHash :one +SELECT + k.id, + k.owner_id, + k.scopes, + k.quota_tier, + k.status, + k.expires_at, + o.status AS owner_status +FROM api.keys k +JOIN api.owners o ON o.id = k.owner_id +WHERE k.key_hash = $1 +` + +type KeyByHashRow struct { ID int64 OwnerID int64 Scopes []string @@ -29,9 +116,15 @@ type GetKeyByHashRow struct { OwnerStatus string } -func (q *Queries) GetKeyByHash(ctx context.Context, keyHash []byte) (GetKeyByHashRow, error) { - row := q.db.QueryRow(ctx, getKeyByHash, keyHash) - var i GetKeyByHashRow +// The authentication hot path. Joins owners so one round trip settles both the +// key's status and the account's. +// +// No filter on status here: the middleware needs to distinguish "revoked" from +// "expired" from "unknown" to give a useful error, and it cannot do that if the +// query has already hidden the row. +func (q *Queries) KeyByHash(ctx context.Context, keyHash []byte) (KeyByHashRow, error) { + row := q.db.QueryRow(ctx, keyByHash, keyHash) + var i KeyByHashRow err := row.Scan( &i.ID, &i.OwnerID, @@ -43,3 +136,119 @@ func (q *Queries) GetKeyByHash(ctx context.Context, keyHash []byte) (GetKeyByHas ) return i, err } + +const listKeysForOwner = `-- name: ListKeysForOwner :many +SELECT id, key_prefix, label, scopes, quota_tier, status, + created_at, expires_at, last_used_at, revoked_at +FROM api.keys +WHERE owner_id = $1 +ORDER BY created_at DESC +` + +type ListKeysForOwnerRow struct { + ID int64 + KeyPrefix string + Label *string + Scopes []string + QuotaTier string + Status string + CreatedAt pgtype.Timestamptz + ExpiresAt pgtype.Timestamptz + LastUsedAt pgtype.Timestamptz + RevokedAt pgtype.Timestamptz +} + +func (q *Queries) ListKeysForOwner(ctx context.Context, ownerID int64) ([]ListKeysForOwnerRow, error) { + rows, err := q.db.Query(ctx, listKeysForOwner, ownerID) + if err != nil { + return nil, err + } + defer rows.Close() + items := []ListKeysForOwnerRow{} + for rows.Next() { + var i ListKeysForOwnerRow + if err := rows.Scan( + &i.ID, + &i.KeyPrefix, + &i.Label, + &i.Scopes, + &i.QuotaTier, + &i.Status, + &i.CreatedAt, + &i.ExpiresAt, + &i.LastUsedAt, + &i.RevokedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const ownerByEmail = `-- name: OwnerByEmail :one +SELECT id, email, email_verified_at, callsign, callsign_verified_at, status, created_at +FROM api.owners +WHERE email = $1 +` + +type OwnerByEmailRow struct { + ID int64 + Email string + EmailVerifiedAt pgtype.Timestamptz + Callsign interface{} + CallsignVerifiedAt pgtype.Timestamptz + Status string + CreatedAt pgtype.Timestamptz +} + +func (q *Queries) OwnerByEmail(ctx context.Context, email string) (OwnerByEmailRow, error) { + row := q.db.QueryRow(ctx, ownerByEmail, email) + var i OwnerByEmailRow + err := row.Scan( + &i.ID, + &i.Email, + &i.EmailVerifiedAt, + &i.Callsign, + &i.CallsignVerifiedAt, + &i.Status, + &i.CreatedAt, + ) + return i, err +} + +const revokeKey = `-- name: RevokeKey :exec +UPDATE api.keys + SET status = 'revoked', revoked_at = now(), revoked_reason = $2 + WHERE id = $1 + AND status <> 'revoked' +` + +type RevokeKeyParams struct { + ID int64 + RevokedReason *string +} + +// The keys_revoked_consistent CHECK requires status and revoked_at to move +// together, so both are set here. +func (q *Queries) RevokeKey(ctx context.Context, arg RevokeKeyParams) error { + _, err := q.db.Exec(ctx, revokeKey, arg.ID, arg.RevokedReason) + return err +} + +const touchKeyLastUsed = `-- name: TouchKeyLastUsed :exec +UPDATE api.keys + SET last_used_at = now() + WHERE id = $1 + AND (last_used_at IS NULL OR last_used_at < now() - interval '5 minutes') +` + +// Debounced by the caller: writing on every request would be one UPDATE per +// request for a field nobody reads in real time. +func (q *Queries) TouchKeyLastUsed(ctx context.Context, id int64) error { + _, err := q.db.Exec(ctx, touchKeyLastUsed, id) + return err +} diff --git a/internal/db/models.go b/internal/db/models.go index 03dbeac..f52a414 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -28,7 +28,7 @@ type ApiKey struct { // Registrants for API keys. Holds PII from an international user base, so the deletion path in 005_privacy.sql applies here too, not just to Discord users. type ApiOwner struct { ID int64 - Email interface{} + Email string EmailVerifiedAt pgtype.Timestamptz Callsign interface{} CallsignVerifiedAt pgtype.Timestamptz @@ -75,7 +75,7 @@ type CoreVerification struct { ID int64 UserID int64 Method string - Callsign interface{} + Callsign string Status string ChallengeHash []byte Detail []byte @@ -103,7 +103,7 @@ type LogbookQso struct { ID int64 UserID int64 ImportID *int64 - Callsign interface{} + Callsign string QsoAt pgtype.Timestamptz // Truncated copy of qso_at used only for duplicate detection. Do not query it directly; use qso_at. QsoMinute pgtype.Timestamp diff --git a/queries/keys.sql b/queries/keys.sql index 1ed6d5f..014d0e2 100644 --- a/queries/keys.sql +++ b/queries/keys.sql @@ -1,6 +1,57 @@ --- name: GetKeyByHash :one -SELECT k.id, k.owner_id, k.scopes, k.quota_tier, k.status, k.expires_at, - o.status AS owner_status +-- name: KeyByHash :one +-- The authentication hot path. Joins owners so one round trip settles both the +-- key's status and the account's. +-- +-- No filter on status here: the middleware needs to distinguish "revoked" from +-- "expired" from "unknown" to give a useful error, and it cannot do that if the +-- query has already hidden the row. +SELECT + k.id, + k.owner_id, + k.scopes, + k.quota_tier, + k.status, + k.expires_at, + o.status AS owner_status FROM api.keys k JOIN api.owners o ON o.id = k.owner_id -WHERE k.key_hash = $1 AND k.status = 'active'; +WHERE k.key_hash = $1; + +-- name: TouchKeyLastUsed :exec +-- Debounced by the caller: writing on every request would be one UPDATE per +-- request for a field nobody reads in real time. +UPDATE api.keys + SET last_used_at = now() + WHERE id = $1 + AND (last_used_at IS NULL OR last_used_at < now() - interval '5 minutes'); + +-- name: CreateOwner :one +INSERT INTO api.owners (email, callsign) +VALUES ($1, $2) +RETURNING id, email, callsign, status, created_at; + +-- name: OwnerByEmail :one +SELECT id, email, email_verified_at, callsign, callsign_verified_at, status, created_at +FROM api.owners +WHERE email = $1; + +-- name: CreateKey :one +INSERT INTO api.keys (owner_id, key_hash, key_prefix, label, scopes, quota_tier, expires_at) +VALUES ($1, $2, $3, $4, $5, $6, $7) +RETURNING id, key_prefix, scopes, quota_tier, status, created_at, expires_at; + +-- name: ListKeysForOwner :many +SELECT id, key_prefix, label, scopes, quota_tier, status, + created_at, expires_at, last_used_at, revoked_at +FROM api.keys +WHERE owner_id = $1 +ORDER BY created_at DESC; + +-- name: RevokeKey :exec +-- The keys_revoked_consistent CHECK requires status and revoked_at to move +-- together, so both are set here. +UPDATE api.keys + SET status = 'revoked', revoked_at = now(), revoked_reason = $2 + WHERE id = $1 + AND status <> 'revoked'; + \ No newline at end of file