summaryrefslogtreecommitdiff
path: root/player-server
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-05-19 00:15:48 +0300
committerPaul Buetow <paul@buetow.org>2026-05-19 00:15:48 +0300
commit084848864d78012d801fba85b5827206d5eccac5 (patch)
treea650c71aa2ee411c0a665272331fcaee45ea87ab /player-server
parent04f18d339d6ed189082a7d7a3ed9ebd5401f82a4 (diff)
Fix RevokeShare 500 on unknown token; add S14 rescan + S15 auth boundary scenarios
Bug fix: shareService.RevokeShare returned errors.New("share not found") instead of the sentinel ErrShareNotFound, so handleError fell through to HTTP 500 instead of 404 for DELETE /api/v1/shares/{unknown-token}. The sister method ValidateShareToken already used the sentinel; this aligns them. Verified with curl: DELETE on a missing token now returns 404. The bug surfaced while drafting S15 (auth-boundary negative tests), which exercises 401 unauthenticated, 403 non-admin → admin routes, and 404 unknown resources. S14 covers admin rescan + scan-progress polling. All 15 LLM e2e scenarios pass against the fixed server. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Diffstat (limited to 'player-server')
-rw-r--r--player-server/internal/service/share.go5
-rw-r--r--player-server/test/e2e-llm/scenarios/S14-admin-rescan.md79
-rw-r--r--player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md121
3 files changed, 203 insertions, 2 deletions
diff --git a/player-server/internal/service/share.go b/player-server/internal/service/share.go
index 9552031..e218625 100644
--- a/player-server/internal/service/share.go
+++ b/player-server/internal/service/share.go
@@ -4,7 +4,6 @@ import (
"context"
"crypto/rand"
"encoding/hex"
- "errors"
"fmt"
"path/filepath"
"time"
@@ -77,7 +76,9 @@ func (s *shareService) RevokeShare(ctx context.Context, token string, userID int
return fmt.Errorf("get share: %w", err)
}
if share == nil {
- return errors.New("share not found")
+ // Return the sentinel so handleError maps this to HTTP 404.
+ // Returning a plain errors.New here used to fall through to 500.
+ return ErrShareNotFound
}
_, err = s.helper.verifyAccess(ctx, share.MediaID, userID)
diff --git a/player-server/test/e2e-llm/scenarios/S14-admin-rescan.md b/player-server/test/e2e-llm/scenarios/S14-admin-rescan.md
new file mode 100644
index 0000000..4aeb7ae
--- /dev/null
+++ b/player-server/test/e2e-llm/scenarios/S14-admin-rescan.md
@@ -0,0 +1,79 @@
+---
+id: S14
+title: "Admin rescan trigger and scan-progress polling"
+tags: [admin, api, scan, rescan]
+preconditions:
+ server_state: running # server running with an existing admin account and media library
+ fixtures: []
+assertions:
+ - db: "SELECT count(*) FROM media"
+ - status_code: "GET /api/v1/admin/scan-progress 200"
+skip: false
+---
+
+1. Authenticate as an admin user: call `POST /api/v1/auth/login` with body
+ `{"username": "admin", "password": "TestPassw0rd!"}`. Confirm the response is
+ HTTP 200 and save the `session` cookie returned in the response as
+ `admin_session` for all subsequent admin requests.
+
+2. Trigger a media rescan: call `POST /api/v1/admin/rescan` with the
+ `admin_session` cookie and an empty body. Confirm the response is HTTP 200
+ and the returned JSON body is `{"status": "ok"}`. The handler starts the
+ rescan asynchronously and returns immediately — there is no job id; progress
+ is tracked via the next endpoint.
+
+3. Fetch the current scan progress: call `GET /api/v1/admin/scan-progress`
+ with the `admin_session` cookie. Confirm the response is HTTP 200 and the
+ returned JSON object contains the following fields:
+ - `running` (boolean) — `true` if the scan is still in progress, `false` if
+ it has already completed
+ - `sets_total` (integer, >= 0)
+ - `sets_done` (integer, >= 0)
+ - `files_total` (integer, >= 0)
+ - `files_done` (integer, >= 0)
+ The fields `current_set` (string) and `last_error` (string) may be present
+ or omitted depending on state. Do not require a specific value for `running`
+ — for a small `./testmedia` library the scan can finish before the first
+ poll.
+
+4. Poll scan progress a second time: call `GET /api/v1/admin/scan-progress`
+ again with the `admin_session` cookie. Confirm the response is HTTP 200 and
+ that the same set of fields (`running`, `sets_total`, `sets_done`,
+ `files_total`, `files_done`) are present with the same types as in step 3.
+ `files_done` must be greater than or equal to the value observed in step 3
+ (monotonic non-decreasing within a single rescan), and `last_error` must
+ either be absent or be an empty string (no scan error).
+
+5. Poll scan progress a third time to confirm the field shape is stable: call
+ `GET /api/v1/admin/scan-progress` once more with the `admin_session`
+ cookie. Confirm the response is HTTP 200 and the same field shape as steps 3
+ and 4. The harness is asserting API contract stability, not waiting for
+ completion.
+
+6. Create a temporary non-admin user to verify the admin-only authorization
+ boundary: call `POST /api/v1/admin/users` with the `admin_session` cookie
+ and body
+ `{"username": "e2e-rescan-user", "password": "TestPassw0rd!", "is_admin": false}`.
+ Confirm the response is HTTP 200 and the returned JSON has a non-zero `id`
+ field and an `is_admin` field equal to `false`. Save the `id` as
+ `temp_user_id`.
+
+7. Authenticate as the non-admin user: call `POST /api/v1/auth/login` with no
+ session cookie and body
+ `{"username": "e2e-rescan-user", "password": "TestPassw0rd!"}`. Confirm the
+ response is HTTP 200 and save the returned `session` cookie as
+ `user_session`.
+
+8. Confirm a non-admin user gets 403 on the rescan endpoint: call
+ `POST /api/v1/admin/rescan` with the `user_session` cookie and an empty
+ body. Confirm the response is HTTP 403 (Forbidden) — the `RequireAdmin`
+ middleware rejects non-admin sessions before the handler is reached.
+
+9. Confirm a non-admin user gets 403 on the scan-progress endpoint: call
+ `GET /api/v1/admin/scan-progress` with the `user_session` cookie. Confirm
+ the response is HTTP 403 (Forbidden).
+
+10. Cleanup: delete the temporary non-admin user. Call
+ `DELETE /api/v1/admin/users/{temp_user_id}` with the `admin_session`
+ cookie. Confirm the response is HTTP 200 and the returned JSON body is
+ `{"status": "ok"}`.
diff --git a/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md b/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md
new file mode 100644
index 0000000..ace7be9
--- /dev/null
+++ b/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md
@@ -0,0 +1,121 @@
+---
+id: S15
+title: "Auth boundary negative-path tests (401/403/404)"
+tags: [auth, admin, api, security, negative-path]
+preconditions:
+ server_state: running # server running with an existing admin account
+ fixtures: []
+assertions:
+ - status_code: "GET /api/v1/media 401"
+ - status_code: "GET /api/v1/admin/users 403"
+skip: false
+---
+
+# Purpose
+
+This scenario verifies the auth middleware boundaries so future regressions in
+`RequireSession` / `RequireAdmin` (see `internal/api/middleware.go`) are caught:
+
+- Unauthenticated API requests must return HTTP 401 (JSON, NOT a redirect to
+ `/login.html`). The middleware only redirects to login when the request
+ includes `Accept: text/html`. Test these requests WITHOUT an `Accept` header
+ (or with `Accept: application/json`) so the server returns 401 as JSON.
+- Authenticated requests by a non-admin user must return HTTP 403 on any route
+ wrapped by `requireAdmin`.
+- Authenticated admin requests for missing resources must return HTTP 404
+ (NOT 200 and NOT 500).
+
+If any handler returns a different status code than asserted below, treat it
+as a real auth-boundary defect and file a task.
+
+---
+
+## A) Unauthenticated requests must return HTTP 401
+
+1. Send `GET {PLAYER_URL}/api/v1/media` with NO `session` cookie and NO
+ `Authorization` header. Do NOT send an `Accept: text/html` header — use
+ `Accept: application/json` (or no Accept header). Confirm the response is
+ HTTP 401. The body should be JSON-ish text containing `unauthorized`.
+
+2. Send `GET {PLAYER_URL}/api/v1/sets` with no cookie and no Authorization
+ header (and no `Accept: text/html`). Confirm the response is HTTP 401.
+
+3. Send `GET {PLAYER_URL}/api/v1/tags` with no cookie and no Authorization
+ header (and no `Accept: text/html`). Confirm the response is HTTP 401.
+
+4. Send `POST {PLAYER_URL}/api/v1/auth/tokens` with header
+ `Content-Type: application/json`, body `{"name": "should-not-be-created",
+ "expires_in_days": 1}`, no `session` cookie, and no `Authorization` header.
+ Confirm the response is HTTP 401. Token creation requires an existing
+ session and must NOT succeed without one.
+
+## B) Authenticated non-admin user must get HTTP 403 on admin routes
+
+5. Authenticate as the admin user: call `POST /api/v1/auth/login` with body
+ `{"username": "admin", "password": "TestPassw0rd!"}`. Confirm the response
+ is HTTP 200 and save the `session` cookie as `ADMIN_COOKIE`.
+
+6. Create a temporary non-admin user: call `POST /api/v1/admin/users` with
+ `ADMIN_COOKIE` and body
+ `{"username": "e2e-auth-boundaries", "password": "TestPassw0rd!", "is_admin": false}`.
+ Confirm the response is HTTP 200 and the returned JSON has a non-zero `id`,
+ `username` equal to `e2e-auth-boundaries`, and `is_admin` equal to false.
+ Save the `id` as `temp_user_id`.
+
+7. Log in as the non-admin user: call `POST /api/v1/auth/login` with body
+ `{"username": "e2e-auth-boundaries", "password": "TestPassw0rd!"}` and no
+ prior cookie. Confirm the response is HTTP 200 and save the `session`
+ cookie returned as `USER_COOKIE`. From here on, USER_COOKIE and
+ ADMIN_COOKIE must NOT be mixed up — admin operations continue to use
+ ADMIN_COOKIE; the 403 assertions below use USER_COOKIE.
+
+8. Call `GET /api/v1/admin/users` with `USER_COOKIE` (no Accept: text/html).
+ Confirm the response is HTTP 403. The non-admin user is authenticated
+ (session is valid) but not authorized for this route.
+
+9. Call `GET /api/v1/admin/trash` with `USER_COOKIE`. Confirm the response is
+ HTTP 403.
+
+10. Call `GET /api/v1/admin/permissions` with `USER_COOKIE`. Confirm the
+ response is HTTP 403.
+
+11. Call `POST /api/v1/admin/rescan` with `USER_COOKIE` (empty body is fine).
+ Confirm the response is HTTP 403. The non-admin user must not be able to
+ trigger a media library rescan.
+
+## C) Authenticated admin must get HTTP 404 for missing resources
+
+12. Call `GET /api/v1/media/999999999` with `ADMIN_COOKIE`. Confirm the
+ response is HTTP 404. (Admins bypass the per-set permission check, so
+ `verifyAccess` returns `ErrNotFound`, which the handler maps to 404 via
+ `handleError`.)
+
+13. Call `GET /api/v1/sets/999999999/browse` with `ADMIN_COOKIE`. Confirm the
+ response is HTTP 404. (Admins bypass `checkSetPermission`; the service
+ then loads the set, finds it nil, and returns `ErrNotFound` which the
+ handler maps to 404.)
+
+14. Call `DELETE /api/v1/shares/does-not-exist-token-xyz` with `ADMIN_COOKIE`.
+ Confirm the response status code is one of `{404, 500}` and record which
+ one was actually returned. NOTE: At the time of writing,
+ `shareService.RevokeShare` returns a plain `errors.New("share not found")`
+ instead of the sentinel `ErrShareNotFound`, so `handleError` falls into
+ its default branch and returns HTTP 500. The "correct" status is 404; if
+ you see 500 here, file a task — this is a real auth-boundary / error-
+ mapping defect in `internal/service/share.go`.
+
+15. Call `GET {PLAYER_URL}/s/does-not-exist-token-xyz/thumbnail` with no
+ cookie (this is a public share route). Confirm the response is HTTP 404.
+
+## D) Cleanup
+
+16. Delete the temporary non-admin user: call
+ `DELETE /api/v1/admin/users/{temp_user_id}` with `ADMIN_COOKIE`. Confirm
+ the response is HTTP 200 and the body is `{"status": "ok"}`. Do NOT skip
+ this cleanup — leftover users will pollute subsequent runs of S12, S13,
+ and S15.
+
+17. Confirm cleanup succeeded: call `GET /api/v1/admin/users` with
+ `ADMIN_COOKIE`. Confirm the response is HTTP 200 and the array does NOT
+ contain any entry whose `id` matches `temp_user_id` or whose `username`
+ is `e2e-auth-boundaries`.