summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--player-server/cmd/player/main.go4
-rw-r--r--player-server/internal/api/handlers.go13
-rw-r--r--player-server/internal/api/handlers_admin.go4
-rw-r--r--player-server/internal/api/handlers_auth.go18
-rw-r--r--player-server/internal/api/handlers_file.go12
-rw-r--r--player-server/internal/api/handlers_media.go56
-rw-r--r--player-server/internal/api/handlers_podcast.go12
-rw-r--r--player-server/internal/api/handlers_share.go13
-rw-r--r--player-server/internal/api/handlers_share_test.go151
-rw-r--r--player-server/internal/api/server.go25
10 files changed, 248 insertions, 60 deletions
diff --git a/player-server/cmd/player/main.go b/player-server/cmd/player/main.go
index e1fd069..1cfb4f7 100644
--- a/player-server/cmd/player/main.go
+++ b/player-server/cmd/player/main.go
@@ -270,6 +270,10 @@ func runWithSignal(args []string, sigCh <-chan os.Signal) error {
},
StaticFS: staticFS,
MediaStreamer: streamer,
+ // Share the already-wired clock so handler-level time arithmetic
+ // (share expiry, session cookie Expires, API token expiry) uses
+ // the same source as the rest of the services (scanner, auth, etc).
+ Clock: deps.clk,
}, logger)
if err != nil {
return fmt.Errorf("failed to create API server: %w", err)
diff --git a/player-server/internal/api/handlers.go b/player-server/internal/api/handlers.go
index 8635c92..08fdbeb 100644
--- a/player-server/internal/api/handlers.go
+++ b/player-server/internal/api/handlers.go
@@ -97,9 +97,16 @@ func readJSON(r *http.Request, dst interface{}) error {
return json.NewDecoder(r.Body).Decode(dst)
}
-func pathID(r *http.Request, name string) int64 {
- id, _ := strconv.ParseInt(r.PathValue(name), 10, 64)
- return id
+// pathID parses a path variable as an int64. It returns the parsed id together
+// with an explicit error so callers can distinguish "missing/malformed" from a
+// legitimately zero value and log the underlying ParseInt failure. The error
+// is wrapped with the variable name to make server logs actionable.
+func pathID(r *http.Request, name string) (int64, error) {
+ id, err := strconv.ParseInt(r.PathValue(name), 10, 64)
+ if err != nil {
+ return 0, fmt.Errorf("invalid %s: %w", name, err)
+ }
+ return id, nil
}
func userIDFromContext(r *http.Request) int64 {
diff --git a/player-server/internal/api/handlers_admin.go b/player-server/internal/api/handlers_admin.go
index d68b07c..0a287f3 100644
--- a/player-server/internal/api/handlers_admin.go
+++ b/player-server/internal/api/handlers_admin.go
@@ -77,8 +77,8 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.adminSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid user id")
return
}
diff --git a/player-server/internal/api/handlers_auth.go b/player-server/internal/api/handlers_auth.go
index c2665a7..3771877 100644
--- a/player-server/internal/api/handlers_auth.go
+++ b/player-server/internal/api/handlers_auth.go
@@ -125,7 +125,7 @@ func (s *Server) handleCreateAPIToken(w http.ResponseWriter, r *http.Request) {
return
}
- expiresAt, ok := apiTokenExpiresAt(req.ExpiresInDays)
+ expiresAt, ok := s.apiTokenExpiresAt(req.ExpiresInDays)
if !ok {
badRequest(w, "expires_in_days must be greater than zero")
return
@@ -166,8 +166,8 @@ func (s *Server) handleRevokeAPIToken(w http.ResponseWriter, r *http.Request) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid token id")
return
}
@@ -198,7 +198,9 @@ func (s *Server) setSessionCookie(w http.ResponseWriter, value string) {
HttpOnly: true,
Secure: s.cfg.SecureCookies,
SameSite: http.SameSiteStrictMode,
- Expires: time.Now().Add(time.Duration(s.cfg.SessionTimeoutHours) * time.Hour),
+ // Use the injected clock so tests can assert the cookie Expires
+ // value deterministically (no flakiness from time.Now()).
+ Expires: s.clk.Now().Add(time.Duration(s.cfg.SessionTimeoutHours) * time.Hour),
})
}
@@ -215,14 +217,18 @@ func (s *Server) clearSessionCookie(w http.ResponseWriter) {
})
}
-func apiTokenExpiresAt(expiresInDays *int) (*time.Time, bool) {
+// apiTokenExpiresAt is a method on Server so it can use the injected clock
+// (s.clk) instead of the wall clock. Returns nil expiry when expiresInDays is
+// nil (i.e. token never expires); false when the value is non-positive (so
+// the caller can return 400).
+func (s *Server) apiTokenExpiresAt(expiresInDays *int) (*time.Time, bool) {
if expiresInDays == nil {
return nil, true
}
if *expiresInDays <= 0 {
return nil, false
}
- expiresAt := time.Now().Add(time.Duration(*expiresInDays) * 24 * time.Hour)
+ expiresAt := s.clk.Now().Add(time.Duration(*expiresInDays) * 24 * time.Hour)
return &expiresAt, true
}
diff --git a/player-server/internal/api/handlers_file.go b/player-server/internal/api/handlers_file.go
index ecf55e0..97de92e 100644
--- a/player-server/internal/api/handlers_file.go
+++ b/player-server/internal/api/handlers_file.go
@@ -14,8 +14,8 @@ import (
func (s *Server) fileHandler(fn func(context.Context, int64, int64) (*service.FileResult, error)) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
http.Error(w, "invalid media id", http.StatusBadRequest)
return
}
@@ -51,8 +51,8 @@ func (s *Server) handleDownload(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.browseSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -88,8 +88,8 @@ func (s *Server) handleRegenThumbnail(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.writeSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
diff --git a/player-server/internal/api/handlers_media.go b/player-server/internal/api/handlers_media.go
index fa7a98d..0570d66 100644
--- a/player-server/internal/api/handlers_media.go
+++ b/player-server/internal/api/handlers_media.go
@@ -34,8 +34,8 @@ func (s *Server) handleGetSetCover(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.browseSvc) {
return
}
- setID := pathID(r, "id")
- if setID == 0 {
+ setID, err := pathID(r, "id")
+ if err != nil || setID == 0 {
badRequest(w, "invalid set id")
return
}
@@ -61,8 +61,8 @@ func (s *Server) handlePostSetCover(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.writeSvc) {
return
}
- setID := pathID(r, "id")
- if setID == 0 {
+ setID, err := pathID(r, "id")
+ if err != nil || setID == 0 {
badRequest(w, "invalid set id")
return
}
@@ -86,8 +86,8 @@ func (s *Server) handleBrowseSet(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.browseSvc) {
return
}
- setID := pathID(r, "id")
- if setID == 0 {
+ setID, err := pathID(r, "id")
+ if err != nil || setID == 0 {
badRequest(w, "invalid set id")
return
}
@@ -108,8 +108,8 @@ func (s *Server) handleUpload(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.writeSvc) {
return
}
- setID := pathID(r, "id")
- if setID == 0 {
+ setID, err := pathID(r, "id")
+ if err != nil || setID == 0 {
badRequest(w, "invalid set id")
return
}
@@ -253,8 +253,8 @@ func (s *Server) handleGetMedia(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.browseSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -274,8 +274,8 @@ func (s *Server) handleFavorite(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.favSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -303,8 +303,8 @@ func (s *Server) handleAddTag(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.tagSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -326,9 +326,9 @@ func (s *Server) handleRemoveTag(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.tagSvc) {
return
}
- id := pathID(r, "id")
+ id, err := pathID(r, "id")
tagName := r.PathValue("tag")
- if id == 0 || tagName == "" {
+ if err != nil || id == 0 || tagName == "" {
badRequest(w, "invalid parameters")
return
}
@@ -343,8 +343,8 @@ func (s *Server) handleSoftDelete(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.writeSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -367,8 +367,8 @@ func (s *Server) handleRestore(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.writeSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -398,8 +398,8 @@ func (s *Server) handlePlaybackHints(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.playbackHintSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -419,8 +419,8 @@ func (s *Server) handleGetNote(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.noteSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -440,8 +440,8 @@ func (s *Server) handleUpsertNote(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.noteSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
@@ -464,8 +464,8 @@ func (s *Server) handleDeleteNote(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.noteSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
diff --git a/player-server/internal/api/handlers_podcast.go b/player-server/internal/api/handlers_podcast.go
index 13ea0d6..a15a055 100644
--- a/player-server/internal/api/handlers_podcast.go
+++ b/player-server/internal/api/handlers_podcast.go
@@ -68,8 +68,8 @@ func (s *Server) handleListEpisodes(w http.ResponseWriter, r *http.Request) {
return
}
- setID := pathID(r, "id")
- if setID == 0 {
+ setID, err := pathID(r, "id")
+ if err != nil || setID == 0 {
badRequest(w, "invalid set id")
return
}
@@ -108,8 +108,8 @@ func (s *Server) handleDownloadEpisode(w http.ResponseWriter, r *http.Request) {
return
}
- episodeID := pathID(r, "episode_id")
- if episodeID == 0 {
+ episodeID, err := pathID(r, "episode_id")
+ if err != nil || episodeID == 0 {
badRequest(w, "invalid episode id")
return
}
@@ -137,8 +137,8 @@ func (s *Server) handleToggleComplete(w http.ResponseWriter, r *http.Request) {
return
}
- episodeID := pathID(r, "episode_id")
- if episodeID == 0 {
+ episodeID, err := pathID(r, "episode_id")
+ if err != nil || episodeID == 0 {
badRequest(w, "invalid episode id")
return
}
diff --git a/player-server/internal/api/handlers_share.go b/player-server/internal/api/handlers_share.go
index 50ad3ef..03ec32a 100644
--- a/player-server/internal/api/handlers_share.go
+++ b/player-server/internal/api/handlers_share.go
@@ -17,12 +17,15 @@ func (s *Server) handleCreateShare(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.shareSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
- expiresAt := time.Now().Add(time.Duration(s.cfg.ShareDefaultExpiryDays) * 24 * time.Hour)
+ // Use the injected clock so tests can pin "now" and assert deterministic
+ // share-expiry semantics (e.g. assert that expiresAt is exactly
+ // ShareDefaultExpiryDays * 24h after the mock clock's T).
+ expiresAt := s.clk.Now().Add(time.Duration(s.cfg.ShareDefaultExpiryDays) * 24 * time.Hour)
share, err := s.shareSvc.CreateShare(r.Context(), userIDFromContext(r), id, expiresAt)
if err != nil {
handleError(w, err)
@@ -35,8 +38,8 @@ func (s *Server) handleListShares(w http.ResponseWriter, r *http.Request) {
if !requireService(w, s.shareSvc) {
return
}
- id := pathID(r, "id")
- if id == 0 {
+ id, err := pathID(r, "id")
+ if err != nil || id == 0 {
badRequest(w, "invalid media id")
return
}
diff --git a/player-server/internal/api/handlers_share_test.go b/player-server/internal/api/handlers_share_test.go
new file mode 100644
index 0000000..58d3257
--- /dev/null
+++ b/player-server/internal/api/handlers_share_test.go
@@ -0,0 +1,151 @@
+package api
+
+import (
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+ "time"
+
+ "codeberg.org/snonux/player/internal"
+ "codeberg.org/snonux/player/internal/auth"
+ "codeberg.org/snonux/player/internal/clock"
+ "codeberg.org/snonux/player/internal/model"
+ "codeberg.org/snonux/player/internal/service"
+)
+
+// TestCreateShare_UsesInjectedClock pins "now" via a clock.MockClock and
+// asserts that handleCreateShare derives expiresAt from the injected clock —
+// not from time.Now(). This guards against the previous flakiness where the
+// expiry was computed off the wall clock and could drift between assertion
+// runs (e.g. when the test goroutine was descheduled).
+func TestCreateShare_UsesInjectedClock(t *testing.T) {
+ // Pin a deterministic instant well in the past so any accidental
+ // time.Now() leak would produce a wildly different expiresAt.
+ fixedNow := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)
+ mockClk := &clock.MockClock{T: fixedNow}
+
+ const expiryDays = 14
+ wantExpiresAt := fixedNow.Add(expiryDays * 24 * time.Hour)
+
+ var capturedExpiresAt time.Time
+ ms := &service.MockMediaService{
+ CreateShareFunc: func(_ context.Context, _, mediaID int64, expiresAt time.Time) (*model.Share, error) {
+ capturedExpiresAt = expiresAt
+ return &model.Share{Token: "tok", MediaID: mediaID}, nil
+ },
+ }
+ // authSvc satisfies the BootstrapRedirect middleware (CountUsers > 0
+ // so requests aren't redirected to /bootstrap.html) and RequireSession
+ // indirectly via session validation — no admin check on this route.
+ authSvc := &service.MockAuthService{
+ CountUsersFunc: func(context.Context) (int, error) { return 1, nil },
+ GetUserByIDFunc: func(_ context.Context, id int64) (*model.User, error) { return &model.User{ID: id}, nil },
+ }
+
+ store := buildSessionStore(1)
+ sm := auth.NewSessionManager(store, mockClk, time.Hour)
+ cfg := &internal.Config{SessionTimeoutHours: 24, ShareDefaultExpiryDays: expiryDays}
+
+ // Build the Server directly so we can inject the mock clock — the
+ // shared newTestServer helper doesn't expose Clock yet, and adding it
+ // there would force every existing test to thread an extra arg.
+ srv, err := NewServer(ServerDeps{
+ Store: buildCountStore(1),
+ SessionManager: sm,
+ Config: cfg,
+ Services: ServerServices{
+ Browse: ms,
+ Write: ms,
+ Share: ms,
+ Tag: ms,
+ Favorite: ms,
+ Note: ms,
+ Auth: authSvc,
+ },
+ StaticFS: newTestFS(map[string]string{"index.html": "x"}),
+ MediaStreamer: service.NewMediaStreamer(nil),
+ Clock: mockClk,
+ })
+ if err != nil {
+ t.Fatalf("NewServer: %v", err)
+ }
+
+ req := httptest.NewRequest(http.MethodPost, "/api/media/1/shares", nil)
+ req.AddCookie(addSessionCookie(t, store, sm, 1))
+ rr := httptest.NewRecorder()
+ srv.ServeHTTP(rr, req)
+
+ if rr.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d (body=%q)", rr.Code, rr.Body.String())
+ }
+ if !capturedExpiresAt.Equal(wantExpiresAt) {
+ t.Fatalf("expected expiresAt %v, got %v", wantExpiresAt, capturedExpiresAt)
+ }
+}
+
+// TestSetSessionCookie_UsesInjectedClock asserts the session-cookie Expires
+// field is derived from s.clk.Now(), not time.Now(). We exercise this via
+// handleLogin (the public Login route), which calls setSessionCookie on
+// success — that's the only handler path that produces a Set-Cookie header
+// with a non-empty Expires.
+func TestSetSessionCookie_UsesInjectedClock(t *testing.T) {
+ fixedNow := time.Date(2024, 6, 1, 8, 0, 0, 0, time.UTC)
+ mockClk := &clock.MockClock{T: fixedNow}
+
+ const sessionHours = 12
+ wantExpires := fixedNow.Add(sessionHours * time.Hour)
+
+ authSvc := &service.MockAuthService{
+ CountUsersFunc: func(context.Context) (int, error) { return 1, nil },
+ LoginFunc: func(_ context.Context, _, _ string) (*service.AuthResult, error) {
+ return &service.AuthResult{
+ SessionID: "sess-xyz",
+ User: &model.User{ID: 1, Username: "alice", IsAdmin: false},
+ }, nil
+ },
+ }
+
+ store := buildSessionStore(1)
+ sm := auth.NewSessionManager(store, mockClk, time.Hour)
+ cfg := &internal.Config{SessionTimeoutHours: sessionHours}
+
+ srv, err := NewServer(ServerDeps{
+ Store: buildCountStore(1),
+ SessionManager: sm,
+ Config: cfg,
+ Services: ServerServices{
+ Auth: authSvc,
+ },
+ StaticFS: newTestFS(map[string]string{"index.html": "x"}),
+ MediaStreamer: service.NewMediaStreamer(nil),
+ Clock: mockClk,
+ })
+ if err != nil {
+ t.Fatalf("NewServer: %v", err)
+ }
+
+ req := httptest.NewRequest(http.MethodPost, "/api/login",
+ strings.NewReader(`{"username":"alice","password":"pw"}`))
+ rr := httptest.NewRecorder()
+ srv.ServeHTTP(rr, req)
+
+ if rr.Code != http.StatusOK {
+ t.Fatalf("expected 200, got %d (body=%q)", rr.Code, rr.Body.String())
+ }
+
+ var sessionCookie *http.Cookie
+ for _, c := range rr.Result().Cookies() {
+ if c.Name == "session" {
+ sessionCookie = c
+ break
+ }
+ }
+ if sessionCookie == nil {
+ t.Fatal("expected session cookie in response")
+ }
+ if !sessionCookie.Expires.Equal(wantExpires) {
+ t.Fatalf("expected cookie Expires %v, got %v", wantExpires, sessionCookie.Expires)
+ }
+}
diff --git a/player-server/internal/api/server.go b/player-server/internal/api/server.go
index e83cecb..c2ee32c 100644
--- a/player-server/internal/api/server.go
+++ b/player-server/internal/api/server.go
@@ -11,6 +11,7 @@ import (
"codeberg.org/snonux/player/internal"
"codeberg.org/snonux/player/internal/auth"
+ "codeberg.org/snonux/player/internal/clock"
"codeberg.org/snonux/player/internal/repository"
"codeberg.org/snonux/player/internal/service"
"codeberg.org/snonux/player/internal/web"
@@ -18,10 +19,15 @@ import (
// Server holds HTTP handlers and dependencies.
type Server struct {
- store repository.Store
- hasher auth.Hasher
- sm auth.SessionManager
- cfg *internal.Config
+ store repository.Store
+ hasher auth.Hasher
+ sm auth.SessionManager
+ cfg *internal.Config
+ // clk is the time source used for time-dependent handler logic (share
+ // expiry, session cookie Expires, API token expiry). Injected so tests
+ // can substitute a clock.MockClock and assert deterministic semantics
+ // instead of racing the wall clock.
+ clk clock.Clock
mux *http.ServeMux
handler http.Handler
browseSvc service.MediaBrowseService
@@ -67,6 +73,11 @@ type ServerDeps struct {
Services ServerServices
StaticFS http.FileSystem
MediaStreamer service.MediaStreamer
+ // Clock is the time source for handler-level time arithmetic (share
+ // expiry, session cookie Expires, API token expiry). If nil it defaults
+ // to clock.RealClock{} so existing production callers and tests that
+ // don't care about deterministic time keep working unchanged.
+ Clock clock.Clock
}
// NewServer creates a Server with routes.
@@ -101,11 +112,17 @@ func NewServerWithLogger(deps ServerDeps, logger *slog.Logger) (*Server, error)
if logger == nil {
logger = slog.Default()
}
+ // Default to the real wall-clock when no clock is injected so production
+ // wiring stays simple and tests that don't pin time keep working.
+ if deps.Clock == nil {
+ deps.Clock = clock.RealClock{}
+ }
s := &Server{
store: deps.Store,
hasher: deps.Hasher,
sm: deps.SessionManager,
cfg: deps.Config,
+ clk: deps.Clock,
mux: http.NewServeMux(),
browseSvc: deps.Services.Browse,
writeSvc: deps.Services.Write,
generated by cgit v1.2.3 (git 2.25.1) at 2026-08-07 05:00:20 +0000