From 60c8097e9fdb79d6b5ecea73d529803cbf297af0 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Tue, 19 May 2026 09:07:47 +0300 Subject: Verify access on progress updates; add S16/S17/S18 scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug fix: POST /api/v1/progress and POST /api/v1/progress/batch did not verify that the supplied media_id belonged to a media row the caller could see. Two failure modes: - Missing media_id triggered an FK violation in UpsertProgress, which fell through handleError to HTTP 500 instead of 404. - Soft-deleted media_id (row still exists, deleted_at != nil) was accepted silently with HTTP 200, recording progress on an item the user could no longer reach. Both now route through accessHelper.verifyAccess in progressService, which returns ErrNotFound (404) for missing/soft-deleted rows and ErrForbidden (403) for unauthorized sets. Verified via curl: POST /progress media_id=999999999 → 404; POST /progress/batch with a bad id → 404. Tests: progress_test.go and no_rows_test.go now seed MediaRepo and UserRepo so the verifyAccess branch finds a real (admin) caller and a real media row. All other tests untouched. S16 covers media list pagination, filtering, sort, and the parser's intentional fail-open behaviour for malformed limit/offset/type. S17 covers podcast list endpoints (GET /podcasts, GET /podcasts/{id}/ episodes) and the admin-only subscribe gate. S18 covers single POST /progress + GET /in-progress, including the new 404 path for missing/forbidden media. Full LLM e2e suite passes 18/18. Co-Authored-By: Claude Opus 4.7 --- player-server/internal/service/progress_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'player-server/internal/service/progress_test.go') diff --git a/player-server/internal/service/progress_test.go b/player-server/internal/service/progress_test.go index 6e6a1a0..9d8e490 100644 --- a/player-server/internal/service/progress_test.go +++ b/player-server/internal/service/progress_test.go @@ -169,11 +169,23 @@ func TestProgressService_UpdateProgress(t *testing.T) { }, }, MediaRepo: repository.MockMediaRepo{ + // GetMediaByID feeds verifyAccess, which UpdateProgress + // now calls to reject missing/deleted/forbidden media. + GetMediaByIDFunc: func(ctx context.Context, id int64) (*model.Media, error) { + return &model.Media{ID: id, SetID: 7}, nil + }, IncrementPlayCountFunc: func(ctx context.Context, id int64) error { incremented = id return tt.incrementErr }, }, + UserRepo: repository.MockUserRepo{ + // Admin user short-circuits checkSetPermission so the + // progress flow doesn't need a permissions fixture. + GetUserByIDFunc: func(ctx context.Context, id int64) (*model.User, error) { + return &model.User{ID: id, IsAdmin: true}, nil + }, + }, } svc := NewProgressService(store, newMockClock()) @@ -229,6 +241,19 @@ func TestProgressService_BatchUpdateProgress_OrdersByObservedAt(t *testing.T) { return nil }, }, + // BatchUpdateProgress now calls verifyAccess per item; the two + // repos below give it real media + an admin user so each item + // passes the access check before reaching applyProgress. + MediaRepo: repository.MockMediaRepo{ + GetMediaByIDFunc: func(ctx context.Context, id int64) (*model.Media, error) { + return &model.Media{ID: id, SetID: 7}, nil + }, + }, + UserRepo: repository.MockUserRepo{ + GetUserByIDFunc: func(ctx context.Context, id int64) (*model.User, error) { + return &model.User{ID: id, IsAdmin: true}, nil + }, + }, } svc := NewProgressService(store, &clock.MockClock{T: observedBase}) -- cgit v1.2.3