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

New Issue

2026-07-16T23:00:27Z

spikerj commented

2026-07-16 23:00:27 +00:00

Split out from #618. While closing the book read-authorization hole, a code audit found that the intended parent-approval model is mostly not implemented. The intended model (per product owner):

Anyone with privileges can upload a book.
If the uploader is a child: the parent AND staff must approve the upload.
If the uploader is a parent: staff must approve.
If the uploader is staff: staff approval (even self), with audit info.
Staff see all books, with uploader attribution.
Children see their parental-approved books.
Parents see all books they / their children / staff uploaded, and toggle each on/off for their children.

What actually exists today (audit findings)

Upload approval is staff/admin-only. A child's (or anyone's) upload creates an UploadWorkflowState with Status = "pending_approval" that notifies staff (BookController.ApproveUpload is guarded by IsAdminOrStaff() → Forbid()). There is no parent-approval step and no parent notification.
No approval fields on Book. Book has only UploadedBy, Visibility, Status, DeletedAt/By. No ApprovedBy, ApprovedAt, or parental-approval state. Approval audit lives only on the transient UploadWorkflowState.Events list.
No parent "family + staff uploads" view. No query lists "books my children / staff / I uploaded" with uploader attribution. Parents only get a per-child FeaturePermissions.ApprovedBookIds toggle (PUT profile/parental/permissions/{childUserId}).
Nothing auto-makes a book Public. Ingest only flips Status → Available; Visibility is never set to Public in code (appears to be a manual/edit step). So the parent "enable for child" toggle (ApprovedBookIds) only takes effect once a book is independently made Public+Available.
Child parental-gating is enforced in exactly one endpoint (my-books). Direct book-content endpoints historically applied only Public+Available — see the follow-up under #618 that adds the child ApprovedBookIds gate to those endpoints.

Scope of this epic (to design before building)

Parent-approval step in the upload workflow (child upload → parent + staff; parent upload → staff), with role resolution (parent/child are profile flags IsParent/IsChildAccount, not Keycloak roles).
Persist approval + audit on the Book (ApprovedBy, ApprovedAt, approval state) rather than only on the workflow doc.
Parent-facing book list with uploader attribution + the enable-for-child toggle in one place.
Define/automate the Visibility → Public transition and how it relates to approval.
Frontend: parent approval UI, parent family-books view, staff audit view.

This is multi-PR (backend + frontend). #618 is being finished narrowly (close the read hole + child gate on direct access) independently of this epic.

Split out from #618. While closing the book read-authorization hole, a code audit found that the intended **parent-approval model** is mostly **not implemented**. The intended model (per product owner): - Anyone with privileges can upload a book. - If the uploader is a **child**: the **parent AND staff** must approve the upload. - If the uploader is a **parent**: **staff** must approve. - If the uploader is **staff**: staff approval (even self), with audit info. - **Staff** see all books, with uploader attribution. - **Children** see their parental-approved books. - **Parents** see all books they / their children / staff uploaded, and toggle each on/off for their children. ## What actually exists today (audit findings) 1. **Upload approval is staff/admin-only.** A child's (or anyone's) upload creates an `UploadWorkflowState` with `Status = "pending_approval"` that notifies **staff** (`BookController.ApproveUpload` is guarded by `IsAdminOrStaff()` → `Forbid()`). There is **no parent-approval step** and no parent notification. 2. **No approval fields on `Book`.** `Book` has only `UploadedBy`, `Visibility`, `Status`, `DeletedAt/By`. No `ApprovedBy`, `ApprovedAt`, or parental-approval state. Approval audit lives only on the transient `UploadWorkflowState.Events` list. 3. **No parent "family + staff uploads" view.** No query lists "books my children / staff / I uploaded" with uploader attribution. Parents only get a per-child `FeaturePermissions.ApprovedBookIds` toggle (`PUT profile/parental/permissions/{childUserId}`). 4. **Nothing auto-makes a book `Public`.** Ingest only flips `Status → Available`; `Visibility` is never set to `Public` in code (appears to be a manual/edit step). So the parent "enable for child" toggle (`ApprovedBookIds`) only takes effect once a book is independently made Public+Available. 5. **Child parental-gating is enforced in exactly one endpoint** (`my-books`). Direct book-content endpoints historically applied only Public+Available — see the follow-up under #618 that adds the child `ApprovedBookIds` gate to those endpoints. ## Scope of this epic (to design before building) - Parent-approval step in the upload workflow (child upload → parent + staff; parent upload → staff), with role resolution (parent/child are profile flags `IsParent`/`IsChildAccount`, not Keycloak roles). - Persist approval + audit on the Book (`ApprovedBy`, `ApprovedAt`, approval state) rather than only on the workflow doc. - Parent-facing book list with uploader attribution + the enable-for-child toggle in one place. - Define/automate the `Visibility → Public` transition and how it relates to approval. - Frontend: parent approval UI, parent family-books view, staff audit view. This is multi-PR (backend + frontend). #618 is being finished narrowly (close the read hole + child gate on direct access) independently of this epic.

spikerj referenced this issue

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

[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

spikerj referenced this issue

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

[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

spikerj commented

2026-07-22 03:16:33 +00:00

Reconciliation check 2026-07-22 (code vs. epic): partial overlap shipped via #729, core epic items not built — stays open.

Shipped (merged):

Parents can share their uploaded books with their children — BookAccess household-read branch + self-approve family uploads (backend PR #432, angular PR #454, both merged to master ~2026-07-20). This covers a slice of the "parent family-uploads" intent.

Not found in code (the epic's core):

No approval-chain fields on the Book model — SpikerSoft.Common/Models/Book.cs has no ApprovedBy/ApprovalStatus; the only "approved" concept in the Books domain is BookAccess's approvedBookIds (wishlist approvals).
No parent-facing staff-uploads review view, no auto-Public-on-approval logic (grep across SpikerSoft.Business/Domain/Books/ finds nothing).

Reconciliation check 2026-07-22 (code vs. epic): **partial overlap shipped via #729, core epic items not built** — stays open. Shipped (merged): - Parents can share their uploaded books with their children — BookAccess household-read branch + self-approve family uploads (backend PR #432, angular PR #454, both merged to master ~2026-07-20). This covers a slice of the "parent family-uploads" intent. Not found in code (the epic's core): - No approval-chain fields on the Book model — `SpikerSoft.Common/Models/Book.cs` has no `ApprovedBy`/`ApprovalStatus`; the only "approved" concept in the Books domain is `BookAccess`'s `approvedBookIds` (wishlist approvals). - No parent-facing staff-uploads review view, no auto-Public-on-approval logic (`grep` across `SpikerSoft.Business/Domain/Books/` finds nothing).

spikerj commented

2026-07-29 18:25:11 +00:00

Epic status roll-up — audited the five audit findings against origin/master. The read side has been solved by adjacent work; the approval chain this epic exists for has not started.

Audit finding	State
1. Upload approval is staff-only, no parent step	❌ unchanged
2. No `ApprovedBy`/`ApprovedAt` on `Book`	❌ unchanged
3. No parent family/staff-uploads view	❌ unchanged
4. Nothing auto-makes a book `Public`	❌ unchanged
5. Child gating enforced in only one endpoint	✅ solved — see below

Finding 5 is genuinely fixed, and better than the epic proposed. Rather than adding the child gate endpoint-by-endpoint, #618's follow-up introduced BookAccess.CanRead as a single source of truth, now applied at BookController.cs:183, :456 and :609 — with the comment at :609 saying so explicitly ("Single source of truth with the read guards (#618)"). #729 later extended it with the household read branch. So the read-authorization surface this epic worried about is consolidated rather than scattered, which removes the risk of the next endpoint being added without the gate.

Findings 1–4 are untouched. git grep -nE "ApprovedBy|ApprovedAt" over the Book model returns nothing — approval audit still lives only on the transient UploadWorkflowState.Events list. The parental surface that does exist is still just the per-child toggle (ProfileController.cs:394/:398, approved-books/{bookId}), which is the "enable for child" switch, not an approval chain.

So the epic's design question — a child upload requiring parent and staff approval, a parent upload requiring staff — is still entirely unbuilt.

One dependency worth flagging before this is designed. The approval chain will need to read and write parental permission state, and #876 is still open: ApproveParentalLinkCommandHandler.cs:36-46 replaces ParentalControls.Permissions wholesale with a hardcoded object, silently zeroing any field not in that list (currently InfoVault). Any new approval-related permission added before #876 is fixed will be silently reset on the next parental link approval — and because this is a COPPA surface, that failure would be invisible and consequential. Worth fixing #876 first; it's a small change.

Also relevant to finding 4: the Visibility → Public transition is still never set in code, so the existing per-child toggle remains inert until a book is independently made Public+Available. That's arguably the cheapest slice to define, since it's blocking the one piece of parental control that already ships.

**Epic status roll-up** — audited the five audit findings against `origin/master`. **The read side has been solved by adjacent work; the approval chain this epic exists for has not started.** | Audit finding | State | |---|---| | 1. Upload approval is staff-only, no parent step | ❌ unchanged | | 2. No `ApprovedBy`/`ApprovedAt` on `Book` | ❌ unchanged | | 3. No parent family/staff-uploads view | ❌ unchanged | | 4. Nothing auto-makes a book `Public` | ❌ unchanged | | 5. Child gating enforced in only one endpoint | ✅ **solved** — see below | **Finding 5 is genuinely fixed, and better than the epic proposed.** Rather than adding the child gate endpoint-by-endpoint, #618's follow-up introduced `BookAccess.CanRead` as a single source of truth, now applied at `BookController.cs:183`, `:456` and `:609` — with the comment at `:609` saying so explicitly ("Single source of truth with the read guards (#618)"). #729 later extended it with the household read branch. So the read-authorization surface this epic worried about is consolidated rather than scattered, which removes the risk of the next endpoint being added without the gate. **Findings 1–4 are untouched.** `git grep -nE "ApprovedBy|ApprovedAt"` over the Book model returns **nothing** — approval audit still lives only on the transient `UploadWorkflowState.Events` list. The parental surface that does exist is still just the per-child toggle (`ProfileController.cs:394`/`:398`, `approved-books/{bookId}`), which is the "enable for child" switch, not an approval chain. So the epic's design question — a child upload requiring parent **and** staff approval, a parent upload requiring staff — is still entirely unbuilt. **One dependency worth flagging before this is designed.** The approval chain will need to read and write parental permission state, and **#876** is still open: `ApproveParentalLinkCommandHandler.cs:36-46` replaces `ParentalControls.Permissions` wholesale with a hardcoded object, silently zeroing any field not in that list (currently `InfoVault`). Any new approval-related permission added before #876 is fixed will be silently reset on the next parental link approval — and because this is a COPPA surface, that failure would be invisible and consequential. Worth fixing #876 first; it's a small change. Also relevant to finding 4: the `Visibility → Public` transition is still never set in code, so the existing per-child toggle remains inert until a book is independently made Public+Available. That's arguably the cheapest slice to define, since it's blocking the one piece of parental control that already ships.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#621