arcodange/dance-lessons-coach
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

✨ feat(auth): JWT TTL hot-reload + fix hardcoded 24h bug (ADR-0023 Phase 2) #44

Merged
arcodange merged 1 commits from feat/adr-0023-phase2-jwt-ttl-hot-reload into main 2026-05-05 09:09:23 +02:00
Conversation 0 Commits 1 Files Changed 5 +73 -10
arcodange commented 2026-05-05 09:09:15 +02:00
Owner
Copy Link

Two fixes in one diff

Both touch the same surface (JWTConfig plumbing), so kept together.

1. Bug fix (pre-existing)

pkg/server/server.go was hardcoding ExpirationTime: time.Hour * 24 when building JWTConfig, completely ignoring auth.jwt.ttl from config (default 1h). Production has been signing tokens with 24h TTL regardless of config since the field was added.

2. Hot-reload (ADR-0023 Phase 2)

Extends JWTConfig with GetTTL func() time.Duration callback. New effectiveTTL() helper prefers GetTTL() when set, falls back to ExpirationTime otherwise (test-friendly).

server.go wires GetTTL: cfg.GetJWTTTL — a method value capturing *Config. When WatchAndApply re-unmarshals into the same Config on file change, the next token generation reads the new TTL. Tokens already issued keep their original expiry — only newly-generated tokens are affected.

WatchAndApply now also logs the new jwt_ttl on every reload.

Tests

  • New TestWatchAndApply_JWTTTL in pkg/config/config_hot_reload_test.go: rewrites the config file and asserts c.GetJWTTTL() flips within 2s. Polling (no fixed sleep), race-clean.
  • Existing pkg/user tests (incl. JWT manager + cleanup loop from PR #41) all pass with -race.
  • Full BDD suite (auth/config/greet/health/info/jwt) green.

ADR-0023 status

Was: Phase 1 Implemented (logging.level).
Now: Phase 1+2 Implemented (logging.level + auth.jwt.ttl).

Remaining:

  • Phase 3: telemetry sampler (type / ratio) — needs tracer provider re-init
  • Phase 4: api.v2_enabled — needs router refactor (always-register + middleware gate); deferred

Verifier verdict (skill-driven)

APPROVE

  • Dim A: APPROVE — effectiveTTL() is 6 lines, defensive on nil + non-positive TTL; method value captures cfg cleanly without globals.
  • Dim B: APPROVE — ADR-0023 status updated accurately; bug fix called out in commit message and PR body so it doesn't get lost.
  • Dim C: APPROVE — race detector clean; new test follows the polling pattern from Phase 1; full BDD suite still green.

Test plan

  • go test -race ./pkg/config/... passes
  • go test -race ./pkg/user/... passes
  • Full BDD suite passes
  • go vet ./... clean
  • Reviewer to verify the bug fix in production: server logs at startup should show jwt_ttl=1h0m0s (default) instead of the previous-but-unused 24h
## Two fixes in one diff Both touch the same surface (`JWTConfig` plumbing), so kept together. ### 1. Bug fix (pre-existing) `pkg/server/server.go` was hardcoding `ExpirationTime: time.Hour * 24` when building `JWTConfig`, **completely ignoring `auth.jwt.ttl` from config** (default 1h). Production has been signing tokens with 24h TTL regardless of config since the field was added. ### 2. Hot-reload (ADR-0023 Phase 2) Extends `JWTConfig` with `GetTTL func() time.Duration` callback. New `effectiveTTL()` helper prefers `GetTTL()` when set, falls back to `ExpirationTime` otherwise (test-friendly). `server.go` wires `GetTTL: cfg.GetJWTTTL` — a method value capturing `*Config`. When `WatchAndApply` re-unmarshals into the same Config on file change, the next token generation reads the new TTL. **Tokens already issued keep their original expiry** — only newly-generated tokens are affected. `WatchAndApply` now also logs the new `jwt_ttl` on every reload. ## Tests - New `TestWatchAndApply_JWTTTL` in `pkg/config/config_hot_reload_test.go`: rewrites the config file and asserts `c.GetJWTTTL()` flips within 2s. Polling (no fixed sleep), race-clean. - Existing `pkg/user` tests (incl. JWT manager + cleanup loop from PR #41) all pass with `-race`. - Full BDD suite (auth/config/greet/health/info/jwt) green. ## ADR-0023 status Was: `Phase 1 Implemented (logging.level)`. Now: `Phase 1+2 Implemented (logging.level + auth.jwt.ttl)`. Remaining: - **Phase 3**: telemetry sampler (type / ratio) — needs tracer provider re-init - **Phase 4**: `api.v2_enabled` — needs router refactor (always-register + middleware gate); deferred ## Verifier verdict (skill-driven) **APPROVE** - **Dim A**: APPROVE — `effectiveTTL()` is 6 lines, defensive on nil + non-positive TTL; method value captures `cfg` cleanly without globals. - **Dim B**: APPROVE — ADR-0023 status updated accurately; bug fix called out in commit message and PR body so it doesn't get lost. - **Dim C**: APPROVE — race detector clean; new test follows the polling pattern from Phase 1; full BDD suite still green. ## Test plan - [x] `go test -race ./pkg/config/...` passes - [x] `go test -race ./pkg/user/...` passes - [x] Full BDD suite passes - [x] `go vet ./...` clean - [ ] Reviewer to verify the bug fix in production: server logs at startup should show `jwt_ttl=1h0m0s` (default) instead of the previous-but-unused 24h
arcodange added 1 commit 2026-05-05 09:09:16 +02:00
✨ feat(auth): JWT TTL hot-reload + fix hardcoded 24h bug (ADR-0023 Phase 2) 405a9fc937 
Two changes in one diff because they share the same surface (JWTConfig
plumbing):

1. **Bug fix** : pkg/server/server.go was hardcoding ExpirationTime to
   24h, ignoring the auth.jwt.ttl config value entirely (default 1h).
   Production has been signing tokens with 24h TTL regardless of config
   since the config field was added.

2. **Hot-reload (ADR-0023 Phase 2)** : extends JWTConfig with a GetTTL
   func() time.Duration callback. effectiveTTL() prefers GetTTL when
   set, falls back to ExpirationTime otherwise (test-friendly). server.go
   wires GetTTL = cfg.GetJWTTTL — a method value that captures the
   *Config, so when WatchAndApply re-unmarshals, the next token
   generation reads the new TTL automatically. Tokens already issued
   keep their original expiry.

WatchAndApply now also logs the new jwt_ttl on every reload event.

Tests:
- New TestWatchAndApply_JWTTTL in pkg/config/config_hot_reload_test.go
  rewrites the config file and asserts the in-memory ttl flips within
  2s. Polling (no fixed sleep), race-clean.
- Existing pkg/user tests (including JWT manager + cleanup loop) all
  pass with -race.
- Full BDD suite (auth/config/greet/health/info/jwt) green.

ADR-0023 status: Phase 1+2 Implemented. Phase 3 (telemetry sampler)
and Phase 4 (api.v2_enabled — needs router refactor) remain Proposed.
arcodange merged commit 3c73ca39d6 into main 2026-05-05 09:09:23 +02:00
arcodange deleted branch feat/adr-0023-phase2-jwt-ttl-hot-reload 2026-05-05 09:09:23 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
arcodange
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: arcodange/dance-lessons-coach#44
Delete Branch "feat/adr-0023-phase2-jwt-ttl-hot-reload"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?