Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Promoted `adminTTL` const to configurable `cfg.AdminTokenTTL` (TD-010)

- **`internal/admin/admin_svc.go`** — deleted the magic-number const `adminTTL = 300`. Admin JWT TTL is now driven by `cfg.AdminTokenTTL` (seconds), wired through a new `tokenTTL` parameter on `NewAdminSvc`. Operators tune via `AA_ADMIN_TOKEN_TTL` (default 300 / 5 min).
- **`internal/cfg/cfg.go`** — added `AdminTokenTTL int` field and a named const `defaultAdminTokenTTL = 300` (seconds; matches existing int-seconds convention for DefaultTTL, MaxTTL, AppTokenTTL so the cfg package stays internally consistent). Env var `AA_ADMIN_TOKEN_TTL` added to the inline doc comment.
- **`cmd/broker/main.go`** — `NewAdminSvc` wiring updated to pass `c.AdminTokenTTL`.
- **Tests** — `newTestAdminSvc` helpers and direct `NewAdminSvc` calls across `admin_svc_test.go`, `admin_hdl_test.go`, `app_hdl_test.go`, `handler/handler_test.go` now pass an explicit `testAdminTokenTTL = 300` fixture. Assertions that checked `resp.ExpiresIn != adminTTL` now check against the fixture value — the test drives cfg-to-claim TTL flow end-to-end inside the admin package, which is the unit-level equivalent of a config-matrix behavioral test for this field.
- **Rationale for int seconds (not `time.Duration`)** — the existing TTL fields (`DefaultTTL`, `MaxTTL`, `AppTokenTTL`) all use int seconds. Adding one `time.Duration` field would create two conventions in the same cfg package and leak into every caller that passes the field through. A future cleanup can migrate all TTL fields to `time.Duration` together (proposed TD-CFG-003) — but mixing conventions in this PR would be worse than preserving the existing one.

### Removed hardcoded identity literals from cfg + token packages (TD-TOKEN-001, TD-TOKEN-002, TD-CFG-001, TD-CFG-002)

- **`internal/token/tkn_svc.go`** — JWT `iss` claim is now driven by `cfg.Issuer` instead of the hardcoded literal `"agentauth"`. Issuer enforcement moved from `TknClaims.Validate()` (pure structural check) into `TknSvc.Verify()` where config is available. Empty `cfg.Issuer` skips the issuer check (mirrors the Audience contract — operator opt-in).
Expand Down
2 changes: 1 addition & 1 deletion TECH-DEBT.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Full details for each item live in the referenced file. This is the index.
| TD-007 | Resilient logging — audit writes inline, no fallback on store failure | Medium | Future | `internal/audit/audit_log.go` |
| TD-008 | Token predecessor not invalidated on renewal — two valid tokens exist | Medium | B1 (P0) may fix | `internal/token/tkn_svc.go` |
| TD-009 | JTI blocklist never pruned — memory grows indefinitely | Medium | B1 (P0) may fix | `internal/store/sql_store.go` |
| TD-010 | Admin JWT TTL hardcoded (`const adminTTL = 300`) — should be operator-configurable via `AA_ADMIN_TOKEN_TTL` | Low | Future | `internal/admin/admin_svc.go` |
| TD-010 | ~~Admin JWT TTL hardcoded (`const adminTTL = 300`)~~ | ~~Low~~ | **RESOLVED 2026-04-10** — promoted to `cfg.AdminTokenTTL int` (seconds, env `AA_ADMIN_TOKEN_TTL`, default 300 named as `defaultAdminTokenTTL`). `NewAdminSvc` signature extended with `tokenTTL int`; all call sites updated. Branch `fix/td-010-admin-token-ttl-config`. | `internal/admin/admin_svc.go`, `internal/cfg/cfg.go`, `cmd/broker/main.go` |

## New — Documentation Drift (B0 Sidecar Removal)

Expand Down
2 changes: 1 addition & 1 deletion cmd/broker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func main() {
}
idSvc := identity.NewIdSvc(sqlStore, tknSvc, c.TrustDomain, auditLog, c.Audience)
delegSvc := deleg.NewDelegSvc(tknSvc, sqlStore, auditLog, privKey)
adminSvc := admin.NewAdminSvc(c.AdminSecretHash, tknSvc, sqlStore, auditLog, c.Audience)
adminSvc := admin.NewAdminSvc(c.AdminSecretHash, tknSvc, sqlStore, auditLog, c.Audience, c.AdminTokenTTL)
appSvc := app.NewAppSvc(sqlStore, tknSvc, auditLog, c.Audience, c.AppTokenTTL)

// Seed tokens for development (AA_SEED_TOKENS=true)
Expand Down
8 changes: 4 additions & 4 deletions internal/admin/admin_hdl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func newTestHandler(t *testing.T) (*AdminHdl, *AdminSvc, *token.TknSvc) {
}
tknSvc := token.NewTknSvc(priv, pub, cfg.Cfg{DefaultTTL: 300})
st := store.NewSqlStore()
adminSvc := NewAdminSvc(testSecretHash, tknSvc, st, nil, "")
adminSvc := NewAdminSvc(testSecretHash, tknSvc, st, nil, "", testAdminTokenTTL)
valMw := authz.NewValMw(tknSvc, nil, nil, "")
hdl := NewAdminHdl(adminSvc, valMw, nil, nil, st)
return hdl, adminSvc, tknSvc
Expand Down Expand Up @@ -67,8 +67,8 @@ func TestHandleAuth_Success(t *testing.T) {
if resp.TokenType != "Bearer" {
t.Errorf("expected token_type=Bearer, got %s", resp.TokenType)
}
if resp.ExpiresIn != adminTTL {
t.Errorf("expected expires_in=%d, got %d", adminTTL, resp.ExpiresIn)
if resp.ExpiresIn != testAdminTokenTTL {
t.Errorf("expected expires_in=%d, got %d", testAdminTokenTTL, resp.ExpiresIn)
}
}

Expand Down Expand Up @@ -295,7 +295,7 @@ func newAppTestMux(t *testing.T) (*http.ServeMux, *AdminSvc, *token.TknSvc, *sto
}
tknSvc := token.NewTknSvc(priv, pub, cfg.Cfg{DefaultTTL: 300})
al := audit.NewAuditLog(st)
adminSvc := NewAdminSvc(testSecretHash, tknSvc, st, al, "")
adminSvc := NewAdminSvc(testSecretHash, tknSvc, st, al, "", testAdminTokenTTL)
valMw := authz.NewValMw(tknSvc, nil, al, "")
hdl := NewAdminHdl(adminSvc, valMw, al, nil, st)

Expand Down
11 changes: 7 additions & 4 deletions internal/admin/admin_svc.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const (
cmp = "AdminSvc"

adminSub = "admin"
adminTTL = 300
defaultMaxTTL = 300
defaultTokenTTL = 30

Expand Down Expand Up @@ -88,19 +87,23 @@ type AdminSvc struct {
store *store.SqlStore
auditLog *audit.AuditLog
audience string
tokenTTL int // seconds — admin JWT lifetime, from cfg.AdminTokenTTL (TD-010)
}

// NewAdminSvc creates a new admin service. The adminSecretHash is the
// bcrypt hash of the admin secret, derived at config load time. The
// auditLog parameter may be nil to disable audit recording. The audience
// is populated into all issued tokens.
func NewAdminSvc(adminSecretHash string, tknSvc *token.TknSvc, st *store.SqlStore, al *audit.AuditLog, audience string) *AdminSvc {
// is populated into all issued tokens. tokenTTL is the admin JWT lifetime
// in seconds, sourced from cfg.AdminTokenTTL — operators tune this via
// AA_ADMIN_TOKEN_TTL (default 300s / 5 min). Previously hardcoded (TD-010).
func NewAdminSvc(adminSecretHash string, tknSvc *token.TknSvc, st *store.SqlStore, al *audit.AuditLog, audience string, tokenTTL int) *AdminSvc {
return &AdminSvc{
adminSecretHash: []byte(adminSecretHash),
tknSvc: tknSvc,
store: st,
auditLog: al,
audience: audience,
tokenTTL: tokenTTL,
}
}

Expand Down Expand Up @@ -132,7 +135,7 @@ func (s *AdminSvc) Authenticate(secret string) (*token.IssueResp, error) {
Sub: adminSub,
Aud: s.audienceSlice(),
Scope: adminScope,
TTL: adminTTL,
TTL: s.tokenTTL,
})
if err != nil {
obs.Fail(mod, cmp, "failed to issue admin token", "err="+err.Error())
Expand Down
13 changes: 8 additions & 5 deletions internal/admin/admin_svc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import (
"golang.org/x/crypto/bcrypt"
)

const testSecret = "test-admin-secret-32bytes-long!"
const (
testSecret = "test-admin-secret-32bytes-long!"
testAdminTokenTTL = 300 // seconds — test-local value so tests don't depend on prod default
)

var testSecretHash string

Expand All @@ -34,7 +37,7 @@ func newTestAdminSvc(t *testing.T) *AdminSvc {
}
tknSvc := token.NewTknSvc(priv, pub, cfg.Cfg{DefaultTTL: 300, Issuer: "test-issuer", TrustDomain: "test.local"})
st := store.NewSqlStore()
return NewAdminSvc(testSecretHash, tknSvc, st, nil, "")
return NewAdminSvc(testSecretHash, tknSvc, st, nil, "", testAdminTokenTTL)
}

func newTestAdminSvcWithAudit(t *testing.T) (*AdminSvc, *audit.AuditLog) {
Expand All @@ -46,7 +49,7 @@ func newTestAdminSvcWithAudit(t *testing.T) (*AdminSvc, *audit.AuditLog) {
tknSvc := token.NewTknSvc(priv, pub, cfg.Cfg{DefaultTTL: 300, Issuer: "test-issuer", TrustDomain: "test.local"})
st := store.NewSqlStore()
al := audit.NewAuditLog(nil)
return NewAdminSvc(testSecretHash, tknSvc, st, al, ""), al
return NewAdminSvc(testSecretHash, tknSvc, st, al, "", testAdminTokenTTL), al
}

// --- Authenticate ---
Expand All @@ -64,8 +67,8 @@ func TestAuthenticate_Success(t *testing.T) {
if resp.AccessToken == "" {
t.Fatal("expected non-empty access_token")
}
if resp.ExpiresIn != adminTTL {
t.Errorf("expected expires_in=%d, got %d", adminTTL, resp.ExpiresIn)
if resp.ExpiresIn != testAdminTokenTTL {
t.Errorf("expected expires_in=%d, got %d", testAdminTokenTTL, resp.ExpiresIn)
}

// Verify the issued token has correct claims.
Expand Down
4 changes: 2 additions & 2 deletions internal/app/app_hdl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ func newTestAppMux(t *testing.T) (*http.ServeMux, *AppSvc) {
if err != nil {
t.Fatalf("generate key: %v", err)
}
tknSvc := token.NewTknSvc(priv, pub, cfg.Cfg{DefaultTTL: 300})
tknSvc := token.NewTknSvc(priv, pub, cfg.Cfg{DefaultTTL: 300, Issuer: "test-issuer", TrustDomain: "test.local"})
al := audit.NewAuditLog(nil)

adminSvc := admin.NewAdminSvc(testAdminSecretHash, tknSvc, st, al, "")
adminSvc := admin.NewAdminSvc(testAdminSecretHash, tknSvc, st, al, "", 300)
valMw := authz.NewValMw(tknSvc, nil, nil, "")

appSvc := NewAppSvc(st, tknSvc, al, "", 1800)
Expand Down
9 changes: 9 additions & 0 deletions internal/cfg/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
// AA_TLS_CLIENT_CA – path to client CA certificate PEM file (mtls only)
// AA_AUDIENCE – expected token audience claim (empty = skip — operator opt-in)
// AA_APP_TOKEN_TTL – app JWT TTL in seconds (default 1800 / 30 min)
// AA_ADMIN_TOKEN_TTL – admin JWT TTL in seconds (default 300 / 5 min)
package cfg

import (
Expand All @@ -40,6 +41,12 @@ import (
// Cost 12 ≈ 250ms per hash on modern hardware — good balance of security and latency.
const AdminBcryptCost = 12

// defaultAdminTokenTTL is the default lifetime of an admin JWT in seconds.
// 300s = 5 minutes — short enough to limit blast radius of a leaked admin
// token, long enough for interactive operator workflows. Operators override
// via AA_ADMIN_TOKEN_TTL. Named so no magic number leaks into Load().
const defaultAdminTokenTTL = 300

// Cfg holds the complete broker configuration derived from environment
// variables. Use [Load] to create an instance with defaults applied.
type Cfg struct {
Expand All @@ -50,6 +57,7 @@ type Cfg struct {
Issuer string // AA_ISSUER: JWT iss claim — broker identity. Empty = skip issuer check on verify (mirrors Audience contract). Operators set this to a value that uniquely identifies their broker instance.
DefaultTTL int // AA_DEFAULT_TTL (default 300 seconds)
AppTokenTTL int // AA_APP_TOKEN_TTL (default 1800 seconds / 30 min)
AdminTokenTTL int // AA_ADMIN_TOKEN_TTL (default 300 seconds / 5 min)
AdminSecret string // AA_ADMIN_SECRET (required for admin auth)
SeedTokens bool // AA_SEED_TOKENS (dev only, default false)
DBPath string // AA_DB_PATH (default "./data.db")
Expand Down Expand Up @@ -80,6 +88,7 @@ func Load() (Cfg, error) {
Issuer: os.Getenv("AA_ISSUER"),
DefaultTTL: envIntOr("AA_DEFAULT_TTL", 300),
AppTokenTTL: envIntOr("AA_APP_TOKEN_TTL", 1800),
AdminTokenTTL: envIntOr("AA_ADMIN_TOKEN_TTL", defaultAdminTokenTTL),
AdminSecret: os.Getenv("AA_ADMIN_SECRET"),
SeedTokens: envOr("AA_SEED_TOKENS", "false") == "true",
DBPath: envOr("AA_DB_PATH", "./data.db"),
Expand Down
25 changes: 25 additions & 0 deletions internal/cfg/cfg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,31 @@ func TestLoad_AudienceEmpty(t *testing.T) {
}
}

func TestLoad_AdminTokenTTLDefault(t *testing.T) {
t.Setenv("AA_ADMIN_SECRET", "test-cfg-secret")
os.Unsetenv("AA_ADMIN_TOKEN_TTL")
c, err := Load()
if err != nil {
t.Fatal(err)
}
if c.AdminTokenTTL != 300 {
t.Fatalf("expected default AdminTokenTTL 300 (5 min), got %d", c.AdminTokenTTL)
}
}

func TestLoad_AdminTokenTTLCustom(t *testing.T) {
t.Setenv("AA_ADMIN_SECRET", "test-cfg-secret")
os.Setenv("AA_ADMIN_TOKEN_TTL", "600")
defer os.Unsetenv("AA_ADMIN_TOKEN_TTL")
c, err := Load()
if err != nil {
t.Fatal(err)
}
if c.AdminTokenTTL != 600 {
t.Fatalf("expected AdminTokenTTL 600, got %d", c.AdminTokenTTL)
}
}

func TestLoad_AppTokenTTLDefault(t *testing.T) {
t.Setenv("AA_ADMIN_SECRET", "test-cfg-secret")
os.Unsetenv("AA_APP_TOKEN_TTL")
Expand Down
2 changes: 1 addition & 1 deletion internal/handler/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func newTestBroker(t *testing.T) *testBroker {
revSvc := revoke.NewRevSvc(nopRevStore{})
idSvc := identity.NewIdSvc(sqlStore, tknSvc, c.TrustDomain, auditLog, "")
delegSvc := deleg.NewDelegSvc(tknSvc, sqlStore, auditLog, priv)
adminSvc := admin.NewAdminSvc(testAdminSecretHash, tknSvc, sqlStore, auditLog, "")
adminSvc := admin.NewAdminSvc(testAdminSecretHash, tknSvc, sqlStore, auditLog, "", 300)

valMw := authz.NewValMw(tknSvc, revSvc, auditLog, "")

Expand Down