[Security][Backend][Books] Private books are listable and downloadable by any authenticated user — /api/book/all and /api/book/{id}/file have no visibility/ownership check #618

New Issue

Closed

opened 2026-07-16 18:32:04 +00:00 by spikerj · 4 comments

spikerj commented 2026-07-16 18:32:04 +00:00

Owner

Copy Link

Copy Source

Summary

While verifying the Game Narrative Toolbox PDF (2026-07-16), the e2e.student account was able to:

1. GET /api/book/all → returned both production books, including ones with visibility: "Private" uploaded by another user (title, path, sha256, full metadata).
2. GET /api/book/{id}/file → downloaded the full 29.3 MB private PDF (200 OK).

GetMyBooks/GetPublicBooks filter correctly (Visibility=="Public" && Status=="Available" + parental gating), but all and {id}/file (and presumably /epub-page, /epub-chapters, /epub-resource, /{id} metadata) only require [Authorize] — any logged-in student can enumerate and read every book on the platform regardless of visibility.

Suggested fix

Centralize a per-book read-authorization check (caller is uploader, or book is Public+Available, or caller is staff/admin) and apply it to: GET /{id}, /{id}/file, /{id}/chapters, /{id}/analysis, /epub-chapters, /epub-page, /epub-resource, and restrict GET /all to staff. The upcoming reader-search endpoint (GET /{bookId}/search, reader plan PR-1) and social endpoints should use the same check.

Relates to the reader/Kavita-parity plan (see #617 for the bookmark identity fix from the same session); the social-layer design already gates on Public+Available via SocialVisibilityService, but the base book endpoints need it independently.

Evidence

• e2e.student token (password grant, spikersoft-web client) → /api/book/all 200 with 2 private books; /api/book/6a58f2c2961260a7b51f8705/file 200, content-length: 29329146.

## Summary While verifying the Game Narrative Toolbox PDF (2026-07-16), the **e2e.student** account was able to: 1. `GET /api/book/all` → returned **both** production books, including ones with `visibility: "Private"` uploaded by another user (title, path, sha256, full metadata). 2. `GET /api/book/{id}/file` → downloaded the full 29.3 MB private PDF (200 OK). `GetMyBooks`/`GetPublicBooks` filter correctly (`Visibility=="Public" && Status=="Available"` + parental gating), but `all` and `{id}/file` (and presumably `/epub-page`, `/epub-chapters`, `/epub-resource`, `/{id}` metadata) only require `[Authorize]` — any logged-in student can enumerate and read every book on the platform regardless of visibility. ## Suggested fix Centralize a per-book read-authorization check (caller is uploader, or book is Public+Available, or caller is staff/admin) and apply it to: `GET /{id}`, `/{id}/file`, `/{id}/chapters`, `/{id}/analysis`, `/epub-chapters`, `/epub-page`, `/epub-resource`, and restrict `GET /all` to staff. The upcoming reader-search endpoint (`GET /{bookId}/search`, reader plan PR-1) and social endpoints should use the same check. Relates to the reader/Kavita-parity plan (see #617 for the bookmark identity fix from the same session); the social-layer design already gates on Public+Available via `SocialVisibilityService`, but the base book endpoints need it independently. ## Evidence - e2e.student token (password grant, `spikersoft-web` client) → `/api/book/all` 200 with 2 private books; `/api/book/6a58f2c2961260a7b51f8705/file` 200, `content-length: 29329146`.

spikerj commented 2026-07-16 22:30:26 +00:00

Author

Owner

Copy Link

Copy Source

Part 1 of 2 in progress — backend PR spikersoft-backend#331 (feat/book-read-authz, open, not yet merged).

This ticket has two independent vectors; keeping it open until both are closed:

1. Authenticated API-controller leak (part 1 — PR #331): any logged-in user could read any book's metadata/file/EPUB pages/chapters/resources/search/analysis regardless of ownership or visibility. Fixed with a single-source-of-truth BookAccess.CanRead / ReadableBy guard on every read endpoint (404-no-oracle), and /all filtered by readability (staff → all).

2. Unauthenticated /ebooks static vector (part 2 — not started): the disk PhysicalFileProvider mount and the S3MediaFallbackMiddleware /ebooks route are both registered before UseAuthentication, so raw book bytes (PDF/EPUB content and covers) are served with no auth at all. Per Joey's direction the fix is to migrate all book serving to authorized MinIO and remove both static mounts. Plan coming in a follow-up.

⚠️ Deployment caveat on part 1: CanRead grants non-owner/non-staff read only for Public + Available books. If course/library books students are meant to read are currently Private, they will 404 after deploy. Access model to be confirmed with Joey before merge.

**Part 1 of 2 in progress** — backend PR [spikersoft-backend#331](https://git.spikersoft.com/spikerj/spikersoft-backend/pulls/331) (`feat/book-read-authz`, open, not yet merged). This ticket has **two independent vectors**; keeping it open until both are closed: 1. **Authenticated API-controller leak** *(part 1 — PR #331)*: any logged-in user could read any book's metadata/file/EPUB pages/chapters/resources/search/analysis regardless of ownership or visibility. Fixed with a single-source-of-truth `BookAccess.CanRead` / `ReadableBy` guard on every read endpoint (404-no-oracle), and `/all` filtered by readability (staff → all). 2. **Unauthenticated `/ebooks` static vector** *(part 2 — not started)*: the disk `PhysicalFileProvider` mount **and** the `S3MediaFallbackMiddleware` `/ebooks` route are both registered *before* `UseAuthentication`, so raw book bytes (PDF/EPUB content **and** covers) are served with no auth at all. Per Joey's direction the fix is to migrate all book serving to authorized MinIO and remove both static mounts. Plan coming in a follow-up. ⚠️ **Deployment caveat on part 1:** `CanRead` grants non-owner/non-staff read only for `Public` + `Available` books. If course/library books students are meant to read are currently `Private`, they will 404 after deploy. Access model to be confirmed with Joey before merge.

spikerj referenced this issue 2026-07-16 23:00:27 +00:00

Epic: build the full parent-approval book model (upload approval chain, ApprovedBy audit, parent family/staff-uploads view, auto-Public-on-approval) #621

spikerj commented 2026-07-16 23:16:47 +00:00

Author

Owner

Copy Link

Copy Source

Update on part 1 (authenticated API). After the audit against the intended parent-approval model (see epic #621 for what's actually built vs. aspirational), the authenticated-API vector is closed by two PRs:

• #331 (merged): closed the gross hole — arbitrary logged-in users reading any non-public book. Correct and safe, but it left the child parental gate applied in only my-books, and duplicated the read rule.
• spikersoft-backend#335 (open): makes read-authorization child-aware and unifies the rule. A child now can only directly read (/file, /{id}, epub, search, analysis) and browse (/public) the books their parent approved (ApprovedBookIds) — closing the direct-access parental-gating gap. BookAccess is now the single source of truth (my-books calls it). 7555 + 19 tests green.

Once #335 merges, the authenticated-API side of #618 is fully done. Part 2 (unauthenticated /ebooks static mount → authorized MinIO) remains — keeping this ticket open until that lands. Deploy note on #335: children will 404 on public books their parent hasn't approved (intended tightening).

**Update on part 1 (authenticated API).** After the audit against the intended parent-approval model (see epic #621 for what's actually built vs. aspirational), the authenticated-API vector is closed by two PRs: - **#331 (merged):** closed the gross hole — arbitrary logged-in users reading any non-public book. Correct and safe, but it left the child parental gate applied in only `my-books`, and duplicated the read rule. - **[spikersoft-backend#335](https://git.spikersoft.com/spikerj/spikersoft-backend/pulls/335) (open):** makes read-authorization **child-aware** and unifies the rule. A child now can only directly read (`/file`, `/{id}`, epub, search, analysis) and browse (`/public`) the books their parent approved (`ApprovedBookIds`) — closing the direct-access parental-gating gap. `BookAccess` is now the single source of truth (`my-books` calls it). 7555 + 19 tests green. Once #335 merges, the **authenticated-API** side of #618 is fully done. **Part 2 (unauthenticated `/ebooks` static mount → authorized MinIO)** remains — keeping this ticket open until that lands. Deploy note on #335: children will 404 on public books their parent hasn't approved (intended tightening).

spikerj commented 2026-07-17 00:28:54 +00:00

Author

Owner

Copy Link

Copy Source

Resolved across two vectors, all PRs now merged to master. Closing.

Vector 1 — authenticated API (the reported hole): book read-authorization is centralized in SpikerSoft.Business/Domain/Books/BookAccess.cs (single source of truth: BookReadContext + CanRead/IsInLibrary/ReadableBy) and applied to every read endpoint — /all (staff-only see-all; others filtered), /{id}, /{id}/file, /{id}/analysis, /epub-chapters, /epub-page, /epub-resource, /{bookId}/search; my-books/public now call the same rule instead of divergent inline copies. Child accounts are gated to their parental ApprovedBookIds on direct reads too, not just in my-books. Unauthorized reads collapse into the not-found path (no oracle).

• spikersoft-backend #331 (part 1) + #335 (child-gate + unification) — merged.

Vector 2 — unauthenticated static serving (found while fixing part 1): book bytes were also served with no auth by three routes — the API /ebooks disk mount, the API /ebooks S3 media-fallback route (both before UseAuthentication), and the Angular nginx bind (/spikersoft/ebooks/…, which is what covers actually used). All three removed. Content now streams from the ebooks MinIO bucket only through the authorized /api/book/{id}/file (after CanRead); covers load via short-lived presigned MinIO URLs minted per response after read-authorization.

• spikersoft-backend #336 + spikersoft-angular #215 (part 2) — merged.

⚠️ Deploy dependency (operational, not code): part 2 is a clean cutover with no disk fallback in prod — a book not yet mirrored into the ebooks bucket is unreadable (content and cover) once these deploy. The ebooks bucket must be backfilled (mc mirror /mnt/fusionio/spikersoft/ebooks spiker/ebooks on SERVER, per spikersoft-infrastructure/docs/minio-storage-migration.md) before the release that carries #336/#215 goes out. Closing the ticket since the fix is merged, but flagging so the backfill isn't missed at deploy.

Note: the intended full parent-approval upload model is mostly not built yet — tracked separately as epic #621; #618 was scoped to read-authorization only.

Resolved across two vectors, all PRs now merged to `master`. Closing. **Vector 1 — authenticated API (the reported hole):** book read-authorization is centralized in `SpikerSoft.Business/Domain/Books/BookAccess.cs` (single source of truth: `BookReadContext` + `CanRead`/`IsInLibrary`/`ReadableBy`) and applied to every read endpoint — `/all` (staff-only see-all; others filtered), `/{id}`, `/{id}/file`, `/{id}/analysis`, `/epub-chapters`, `/epub-page`, `/epub-resource`, `/{bookId}/search`; `my-books`/`public` now call the same rule instead of divergent inline copies. Child accounts are gated to their parental `ApprovedBookIds` on direct reads too, not just in `my-books`. Unauthorized reads collapse into the not-found path (no oracle). - spikersoft-backend **#331** (part 1) + **#335** (child-gate + unification) — merged. **Vector 2 — unauthenticated static serving (found while fixing part 1):** book bytes were also served with no auth by three routes — the API `/ebooks` disk mount, the API `/ebooks` S3 media-fallback route (both before `UseAuthentication`), and the Angular nginx bind (`/spikersoft/ebooks/…`, which is what covers actually used). All three removed. Content now streams from the `ebooks` MinIO bucket only through the authorized `/api/book/{id}/file` (after `CanRead`); covers load via short-lived presigned MinIO URLs minted per response after read-authorization. - spikersoft-backend **#336** + spikersoft-angular **#215** (part 2) — merged. ⚠️ **Deploy dependency (operational, not code):** part 2 is a clean cutover with **no disk fallback in prod** — a book not yet mirrored into the `ebooks` bucket is unreadable (content *and* cover) once these deploy. The `ebooks` bucket must be backfilled (`mc mirror /mnt/fusionio/spikersoft/ebooks spiker/ebooks` on SERVER, per `spikersoft-infrastructure/docs/minio-storage-migration.md`) **before** the release that carries #336/#215 goes out. Closing the ticket since the fix is merged, but flagging so the backfill isn't missed at deploy. Note: the intended full parent-approval upload model is mostly not built yet — tracked separately as epic #621; #618 was scoped to read-authorization only.

spikerj closed this issue 2026-07-17 00:28:58 +00:00

spikerj referenced this issue 2026-07-17 00:50:13 +00:00

[CI][Backend] Unit Tests red since Batch S (dcproj NU1105 via Directory.Build.props pins) + ArtPipe per-model images broken since CPM (Dockerfile.model missing Packages.props) #623

spikerj referenced this issue 2026-07-17 00:50:38 +00:00

[Infra][CI] lesson-video-processor never deploys: swarm service name 67 chars exceeds 63-char limit (stack+service name doubled) #624

spikerj referenced this issue 2026-07-17 00:50:38 +00:00

[Infra][CI] every deploy job's /mnt/infrastructure git pull fails under sudo (no credentials) — #584 stale-checkout guard is decorative #625

spikerj commented 2026-07-17 01:04:59 +00:00

Author

Owner

Copy Link

Copy Source

Both vectors are now closed and deployed:

Part 1 — authenticated API leak:

• spikersoft-backend #331 (merged): BookAccess.CanRead/ReadableBy guard on every read endpoint (404-no-oracle), /all filtered by readability.
• spikersoft-backend #335 (merged): read-authorization made child-aware (ApprovedBookIds) and unified as the single source of truth (my-books calls it).

Part 2 — unauthenticated /ebooks static vector:

• spikersoft-backend #336 (merged, 52c67fcc): ebook bytes served only through authorized MinIO paths.
• spikersoft-angular #215 (merged, deb73fa1): covers load via presigned MinIO URLs; the ebooks bind denied.

Post-merge CI on both merge commits has settled: angular fully green (incl. both e2e walks); backend 26/29 workflows green — API and book-management built, published and deployed. The 3 failures are PRE-EXISTING deps-modernization/infra issues unrelated to this fix, now tracked and fixed separately: #623 (Unit Tests dcproj restore + ArtPipe Dockerfile.model → backend PR #337), #624 (lesson-video-processor 67-char swarm service name → infrastructure PR #123), #625 (deploy jobs cannot pull /mnt/infrastructure).

Closing.

Both vectors are now closed and deployed: **Part 1 — authenticated API leak:** - spikersoft-backend #331 (merged): `BookAccess.CanRead`/`ReadableBy` guard on every read endpoint (404-no-oracle), `/all` filtered by readability. - spikersoft-backend #335 (merged): read-authorization made child-aware (`ApprovedBookIds`) and unified as the single source of truth (`my-books` calls it). **Part 2 — unauthenticated `/ebooks` static vector:** - spikersoft-backend #336 (merged, `52c67fcc`): ebook bytes served only through authorized MinIO paths. - spikersoft-angular #215 (merged, `deb73fa1`): covers load via presigned MinIO URLs; the ebooks bind denied. Post-merge CI on both merge commits has settled: angular fully green (incl. both e2e walks); backend 26/29 workflows green — API and book-management built, published and deployed. The 3 failures are PRE-EXISTING deps-modernization/infra issues unrelated to this fix, now tracked and fixed separately: #623 (Unit Tests dcproj restore + ArtPipe Dockerfile.model → backend PR #337), #624 (lesson-video-processor 67-char swarm service name → infrastructure PR #123), #625 (deploy jobs cannot pull /mnt/infrastructure). Closing.

spikerj referenced this issue 2026-07-17 01:06:45 +00:00

[CI][Backend] Unit Tests red since Batch S (dcproj NU1105 via Directory.Build.props pins) + ArtPipe per-model images broken since CPM (Dockerfile.model missing Packages.props) #623

spikerj referenced this issue 2026-07-17 01:48:34 +00:00

[Bug][Prod][Reader] PDFs never open in the reader — pdf.js fetches /api/book/{id}/file tokenless (401); EPUBs unaffected #626

spikerj referenced this issue 2026-07-17 01:52:38 +00:00

[Bug][Prod][Reader] PDFs never open in the reader — pdf.js fetches /api/book/{id}/file tokenless (401); EPUBs unaffected #626

spikerj referenced this issue 2026-07-17 01:53:25 +00:00

[Bug][Prod][Reader] PDFs never open in the reader — pdf.js fetches /api/book/{id}/file tokenless (401); EPUBs unaffected #626

spikerj referenced this issue 2026-07-17 04:35:55 +00:00

[Reader][Social] Backend: shared bookmarks, upvotes, and reading lists (opt-in, child-safe) #631

spikerj referenced this issue 2026-07-18 18:00:46 +00:00

[Infra][MinIO] Uploads-tree coordinated batch cutover — 7 services off /mnt/fusionio (epic #413) #529

spikerj referenced this issue 2026-07-18 18:01:08 +00:00

[Infra][MinIO] Migrate Angular nginx root bind — presigned/proxy, LAST (epic #413) #531

spikerj referenced this issue 2026-07-18 18:01:29 +00:00

[Infra][MinIO] Cut over book/ebook artifacts to MinIO as sole store (ebooks slice of #413) + fix inline-mirror pipeline stall #696

spikerj referenced this issue 2026-07-20 15:26:28 +00:00

Child book read-context never built on real tokens: bare FindFirst("sub") nulled by JWT inbound claim mapping (breaks #729 household grants; #618 child gate fail-opens) #734

spikerj referenced this issue 2026-07-28 01:59:25 +00:00

SW caches /api/book/* JSON for 7d while presigned cover URLs expire in hours — fallback serves dead cover links #879

spikerj referenced this issue 2026-07-28 01:59:46 +00:00

book-upload preview binds bucket-relative coverImage key as <img src> — broken preview for MinIO-served books #881

spikerj referenced this issue 2026-07-29 05:48:20 +00:00

Photo gallery: free-form tags — editor, gallery filter/search, EXIF keyword seeding #893

spikerj referenced this issue 2026-07-29 18:25:11 +00:00

Epic: build the full parent-approval book model (upload approval chain, ApprovedBy audit, parent family/staff-uploads view, auto-Public-on-approval) #621

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

No results found.

Labels

Clear labels

agentic

Actionable surfaced by the agentic architecture & security review

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

sonarqube

Code quality issue surfaced by SonarQube (bug / code smell)

wontfix

This won't be fixed

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

spikerj (Joseph Spiker)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#618