From 92edb836d45ccb9e096a4c9a8c2cabcebf133d19 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 19 May 2026 15:14:09 +0300 Subject: Require explicit TokenManager in auth service (DIP) Remove the silent nil-fallback in NewAuthService that fabricated a default auth.TokenManager when callers passed nil. The service now panics on nil, forcing callers to inject their own TokenManager and keeping construction at the composition root. Production wiring (cmd/player/main.go) already constructs the TokenManager explicitly via auth.NewTokenManager(); test call sites in internal/api/handlers_test.go and handlers_podcast_test.go that used to pass nil now pass auth.NewTokenManager() to honour the new contract. Mirrors the pattern set in commit 622827c (http.Client in podcast service). Refs task 7a. Co-Authored-By: Claude Opus 4.7 --- player-server/internal/api/handlers_podcast_test.go | 2 +- player-server/internal/api/handlers_test.go | 16 ++++++++-------- player-server/internal/service/auth.go | 6 +++++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/player-server/internal/api/handlers_podcast_test.go b/player-server/internal/api/handlers_podcast_test.go index a5d15bf..4836218 100644 --- a/player-server/internal/api/handlers_podcast_test.go +++ b/player-server/internal/api/handlers_podcast_test.go @@ -95,7 +95,7 @@ func setupPodcastE2E(t *testing.T) (srv *Server, store repository.Store, sm auth hasher := &staticHasher{fixed: "hashed"} sm = auth.NewSessionManager(dbStore, clk, time.Hour) - authSvc := service.NewAuthService(dbStore, clk, hasher, sm, nil) + authSvc := service.NewAuthService(dbStore, clk, hasher, sm, auth.NewTokenManager()) mediaRoot := t.TempDir() helper := service.NewAccessHelper(dbStore) diff --git a/player-server/internal/api/handlers_test.go b/player-server/internal/api/handlers_test.go index aa15611..fea6861 100644 --- a/player-server/internal/api/handlers_test.go +++ b/player-server/internal/api/handlers_test.go @@ -463,7 +463,7 @@ func TestServer_Bootstrap(t *testing.T) { CreateSessionFunc: func(ctx context.Context, session *model.Session) error { return nil }, } sm := auth.NewSessionManager(&repo, clk, time.Hour) - authSvc := service.NewAuthService(store, clk, hasher, sm, nil) + authSvc := service.NewAuthService(store, clk, hasher, sm, auth.NewTokenManager()) srv := newTestServer(t, store, hasher, sm, cfg, nil, nil, nil, nil, nil, nil, nil, nil, authSvc, nil) body := `{"username":"admin","password":"secret"}` @@ -491,7 +491,7 @@ func TestServer_Bootstrap(t *testing.T) { CountUsersFunc: func(ctx context.Context) (int, error) { return 1, nil }, }, } - authSvc := service.NewAuthService(store, clk, hasher, nil, nil) + authSvc := service.NewAuthService(store, clk, hasher, nil, auth.NewTokenManager()) srv := newTestServer(t, store, hasher, nil, cfg, nil, nil, nil, nil, nil, nil, nil, nil, authSvc, nil) body := `{"username":"admin","password":"secret"}` req := httptest.NewRequest(http.MethodPost, "/api/bootstrap", bytes.NewReader([]byte(body))) @@ -508,7 +508,7 @@ func TestServer_Bootstrap(t *testing.T) { for _, path := range paths { t.Run(path, func(t *testing.T) { store := &repository.MockStore{UserRepo: repository.MockUserRepo{CountUsersFunc: func(ctx context.Context) (int, error) { return 0, nil }}} - authSvc := service.NewAuthService(store, clk, hasher, nil, nil) + authSvc := service.NewAuthService(store, clk, hasher, nil, auth.NewTokenManager()) srv := newTestServer(t, store, hasher, nil, cfg, nil, nil, nil, nil, nil, nil, nil, nil, authSvc, nil) req := httptest.NewRequest(http.MethodPost, path, bytes.NewReader([]byte(`{"username":""}`))) rr := httptest.NewRecorder() @@ -549,7 +549,7 @@ func TestServer_Login(t *testing.T) { CreateSessionFunc: func(ctx context.Context, session *model.Session) error { return nil }, } sm := auth.NewSessionManager(&repo, clk, time.Hour) - authSvc := service.NewAuthService(store, clk, hasher, sm, nil) + authSvc := service.NewAuthService(store, clk, hasher, sm, auth.NewTokenManager()) srv := newTestServer(t, store, hasher, sm, cfg, nil, nil, nil, nil, nil, nil, nil, nil, authSvc, nil) body := `{"username":"alice","password":"correct"}` req := httptest.NewRequest(http.MethodPost, "/api/login", bytes.NewReader([]byte(body))) @@ -578,7 +578,7 @@ func TestServer_Login(t *testing.T) { }, }, } - authSvc := service.NewAuthService(store, clk, hasher, nil, nil) + authSvc := service.NewAuthService(store, clk, hasher, nil, auth.NewTokenManager()) srv := newTestServer(t, store, hasher, nil, cfg, nil, nil, nil, nil, nil, nil, nil, nil, authSvc, nil) body := `{"username":"alice","password":"wrong"}` req := httptest.NewRequest(http.MethodPost, "/api/login", bytes.NewReader([]byte(body))) @@ -599,7 +599,7 @@ func TestServer_Login(t *testing.T) { }, }, } - authSvc := service.NewAuthService(store, clk, hasher, nil, nil) + authSvc := service.NewAuthService(store, clk, hasher, nil, auth.NewTokenManager()) srv := newTestServer(t, store, hasher, nil, cfg, nil, nil, nil, nil, nil, nil, nil, nil, authSvc, nil) body := `{"username":"nobody","password":"pass"}` req := httptest.NewRequest(http.MethodPost, "/api/login", bytes.NewReader([]byte(body))) @@ -626,7 +626,7 @@ func TestServer_SessionCookieSecure(t *testing.T) { } clk := &clock.MockClock{T: time.Now()} sm := auth.NewSessionManager(&repo, clk, time.Hour) - authSvc := service.NewAuthService(store, clk, hasher, sm, nil) + authSvc := service.NewAuthService(store, clk, hasher, sm, auth.NewTokenManager()) t.Run("Secure=true by default", func(t *testing.T) { cfg := &internal.Config{SessionTimeoutHours: 24, SecureCookies: true} @@ -878,7 +878,7 @@ func TestServer_APITokenBearerFlow(t *testing.T) { clk := &clock.MockClock{T: now} hasher := &staticHasher{fixed: "hashed"} sm := auth.NewSessionManager(dbStore, clk, time.Hour) - authSvc := service.NewAuthService(dbStore, clk, hasher, sm, nil) + authSvc := service.NewAuthService(dbStore, clk, hasher, sm, auth.NewTokenManager()) mediaSvc := service.NewMediaService(dbStore, clk, t.TempDir(), nil, nil) cfg := &internal.Config{SessionTimeoutHours: 24, MaxUploadSizeMB: 10} srv := newTestServer(t, dbStore, hasher, sm, cfg, mediaSvc, mediaSvc, mediaSvc, mediaSvc, mediaSvc, mediaSvc, nil, nil, authSvc, nil) diff --git a/player-server/internal/service/auth.go b/player-server/internal/service/auth.go index 1fe468b..c5288ee 100644 --- a/player-server/internal/service/auth.go +++ b/player-server/internal/service/auth.go @@ -21,9 +21,13 @@ type authService struct { } // NewAuthService creates a concrete AuthService. +// tm is required and must not be nil — passing nil will panic. This is +// intentional: the service depends on an injected TokenManager per DIP and +// refuses to silently fabricate one. Production wiring (cmd/player/main.go) +// constructs the TokenManager at the composition root via auth.NewTokenManager. func NewAuthService(store repository.AuthServiceStore, clk clock.Clock, hasher auth.Hasher, sm auth.SessionManager, tm auth.TokenManager) *authService { if tm == nil { - tm = auth.NewTokenManager() + panic("service.NewAuthService: tm (auth.TokenManager) must not be nil") } return &authService{ store: store, -- cgit v1.2.3