From 405a9fc93729829f7e939f2b5051a2a3231f7c15 Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Tue, 5 May 2026 09:08:19 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat(auth):=20JWT=20TTL=20hot-reloa?= =?UTF-8?q?d=20+=20fix=20hardcoded=2024h=20bug=20(ADR-0023=20Phase=202)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- adr/0023-config-hot-reloading.md | 2 +- pkg/config/config.go | 17 +++++++++----- pkg/config/config_hot_reload_test.go | 33 ++++++++++++++++++++++++++++ pkg/server/server.go | 10 +++++++-- pkg/user/auth_service.go | 21 ++++++++++++++++-- 5 files changed, 73 insertions(+), 10 deletions(-) diff --git a/adr/0023-config-hot-reloading.md b/adr/0023-config-hot-reloading.md index 2440445..bc9b0ef 100644 --- a/adr/0023-config-hot-reloading.md +++ b/adr/0023-config-hot-reloading.md @@ -1,6 +1,6 @@ # Config Hot Reloading Strategy -**Status:** Phase 1 Implemented (2026-05-05 — `logging.level` hot-reloadable via `Config.WatchAndApply` in `pkg/config/config.go`, wired in `pkg/server/server.go Run`. Remaining fields — `api.v2_enabled`, telemetry sampler, `auth.jwt.ttl` — Proposed for follow-up phases following the same pattern.) +**Status:** Phase 1+2 Implemented (2026-05-05 — `logging.level` and `auth.jwt.ttl` hot-reloadable via `Config.WatchAndApply` in `pkg/config/config.go`, wired in `pkg/server/server.go Run`. Phase 2 also fixed a pre-existing bug where the hardcoded 24h TTL ignored `auth.jwt.ttl` from config entirely. Remaining fields — `api.v2_enabled`, telemetry sampler — Proposed for follow-up phases.) **Authors:** Gabriel Radureau, AI Agent **Date:** 2026-04-05 **Last Updated:** 2026-05-05 diff --git a/pkg/config/config.go b/pkg/config/config.go index a5ae6d9..9c6ab3d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -609,11 +609,15 @@ func (c *Config) setupLogOutput() { // WatchAndApply starts watching the config file for changes and applies the // hot-reloadable subset on every change (ADR-0023 selective hot-reload). // -// Phase 1 (this PR) reloads: -// - logging.level — re-applies via SetupLogging on every change. +// Phases shipped: +// - Phase 1: logging.level — re-applied via SetupLogging on every change. +// - Phase 2: auth.jwt.ttl — picked up automatically because the userService +// reads it via JWTConfig.GetTTL (a method value capturing this *Config). +// The reloaded TTL is used on the NEXT token generation; tokens issued +// before the change keep their original expiry. // -// The other fields listed in ADR-0023 (api.v2_enabled, telemetry sampler, -// auth.jwt.ttl) remain restart-only until their handlers land in a follow-up. +// The other fields listed in ADR-0023 (api.v2_enabled, telemetry sampler) +// remain restart-only until their handlers land in subsequent phases. // // Stops when ctx is cancelled. Safe to call once at server startup. // If the config file is absent (ConfigFileNotFoundError at load time), this @@ -641,7 +645,10 @@ func (c *Config) WatchAndApply(ctx context.Context) { // Apply hot-reloadable fields. Order matters: logging first so the // rest of the reload is logged at the right level. c.SetupLogging() - log.Info().Str("logging_level", c.GetLogLevel()).Msg("Hot-reload applied (logging.level)") + log.Info(). + Str("logging_level", c.GetLogLevel()). + Dur("jwt_ttl", c.GetJWTTTL()). + Msg("Hot-reload applied (logging.level + auth.jwt.ttl)") }) c.viper.WatchConfig() diff --git a/pkg/config/config_hot_reload_test.go b/pkg/config/config_hot_reload_test.go index 0e829a7..40da4b9 100644 --- a/pkg/config/config_hot_reload_test.go +++ b/pkg/config/config_hot_reload_test.go @@ -21,6 +21,7 @@ func loadFromFile(t *testing.T, path string) *Config { v.SetConfigFile(path) v.SetConfigType("yaml") v.SetDefault("logging.level", "info") + v.SetDefault("auth.jwt.ttl", time.Hour) require.NoError(t, v.ReadInConfig()) c := &Config{viper: v} @@ -81,3 +82,35 @@ func TestWatchAndApply_NilViperNoOp(t *testing.T) { defer cancel() c.WatchAndApply(ctx) } + +// TestWatchAndApply_JWTTTL proves Phase 2 of ADR-0023: the JWT TTL is +// re-read on every token generation via the GetJWTTTL method value, so +// after a config-file change the new TTL takes effect without restart. +func TestWatchAndApply_JWTTTL(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(path, []byte("auth:\n jwt:\n ttl: 1h\n"), 0644)) + + c := loadFromFile(t, path) + assert.Equal(t, time.Hour, c.GetJWTTTL()) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c.WatchAndApply(ctx) + + require.NoError(t, os.WriteFile(path, []byte("auth:\n jwt:\n ttl: 30m\n"), 0644)) + + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + c.reloadMu.RLock() + ttl := c.GetJWTTTL() + c.reloadMu.RUnlock() + if ttl == 30*time.Minute { + return + } + time.Sleep(20 * time.Millisecond) + } + c.reloadMu.RLock() + defer c.reloadMu.RUnlock() + t.Fatalf("auth.jwt.ttl did not hot-reload to 30m: still %s", c.GetJWTTTL()) +} diff --git a/pkg/server/server.go b/pkg/server/server.go index 1282c88..9255fff 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -139,10 +139,16 @@ func initializeUserServices(cfg *config.Config) (user.UserRepository, user.UserS return nil, nil, fmt.Errorf("failed to create PostgreSQL user repository: %w", err) } - // Create JWT config + // Create JWT config. + // GetTTL is a method value — it captures cfg, so when WatchAndApply + // re-unmarshals into the same Config struct on file changes, every + // subsequent token generation reads the new TTL (ADR-0023 Phase 2). + // ExpirationTime is kept as a static fallback for tests that build + // JWTConfig manually without a Config. jwtConfig := user.JWTConfig{ Secret: cfg.GetJWTSecret(), - ExpirationTime: time.Hour * 24, // 24 hours + ExpirationTime: 24 * time.Hour, + GetTTL: cfg.GetJWTTTL, Issuer: "dance-lessons-coach", } diff --git a/pkg/user/auth_service.go b/pkg/user/auth_service.go index 527efc0..dd4f795 100644 --- a/pkg/user/auth_service.go +++ b/pkg/user/auth_service.go @@ -11,13 +11,30 @@ import ( "golang.org/x/crypto/bcrypt" ) -// JWTConfig holds JWT configuration +// JWTConfig holds JWT configuration. +// +// GetTTL, when non-nil, is called on every token generation to read the +// current TTL — this enables ADR-0023 Phase 2 hot-reload of `auth.jwt.ttl`. +// If nil, ExpirationTime is used as a static fallback. type JWTConfig struct { Secret string ExpirationTime time.Duration + GetTTL func() time.Duration Issuer string } +// effectiveTTL returns the live TTL: GetTTL() when wired, else +// ExpirationTime as a static fallback (used by tests that don't go +// through the server-level wiring). +func (c JWTConfig) effectiveTTL() time.Duration { + if c.GetTTL != nil { + if ttl := c.GetTTL(); ttl > 0 { + return ttl + } + } + return c.ExpirationTime +} + // userServiceImpl implements the unified UserService interface type userServiceImpl struct { repo UserRepository @@ -69,7 +86,7 @@ func (s *userServiceImpl) GenerateJWT(ctx context.Context, user *User) (string, "sub": user.ID, "name": user.Username, "admin": user.IsAdmin, - "exp": time.Now().Add(s.jwtConfig.ExpirationTime).Unix(), + "exp": time.Now().Add(s.jwtConfig.effectiveTTL()).Unix(), "iat": time.Now().Unix(), "iss": s.jwtConfig.Issuer, }