novelly/docs/plans/api/users_and_roles_plan.md

# Users, Roles, and Per-Novel Permissions

## Context

Novelly is single-tenant today: no user entity, no login, no ownership. Anyone who reaches the origin gets every novel and every mutation. `README.md:256` already flags this ("Authentication, if this is ever going to run anywhere but localhost"), and the persistent LAN dev deploy makes it real.

This adds accounts with passwords, four roles, and per-novel access grants, so a writer can hand a novel to an editor or reviewer without handing over the whole instance.

Role model (as specified):

| Role | Scope | Can |
|---|---|---|
| Admin | global | everything, all novels |
| Writer | global + per-novel | create novels; on novels they own or are granted Writer on: read, write, delete content; grant access on **owned** novels |
| Editor | per-novel | change anything in the novel — no create, no delete |
| Reviewer | per-novel | read only (prose comments land in a later phase) |

Self-signup creates a Reviewer. The first account created becomes Admin and adopts every existing owner-less novel. Logins persist 24 hours.

## Decisions already made

- ASP.NET Core Identity (EF store) for the user table and password hashing; cookie auth, 24h expiry.
- Global role on the user + per-project grants in a `ProjectMember` table.
- MCP server and background jobs authenticate with a configured service API key.
- `Project.OwnerId` is nullable in the migration; adoption happens when the first Admin registers.

## Key architectural call: enforce authorization in the services, not the endpoints

`CLAUDE.md` mandates that web, embedded agent, and MCP all go through the same application services. Two facts push enforcement down to the service layer:

1. Agent tools (`NovelAgentToolset`, `ImportAgentToolset`) call services directly and never pass through an endpoint filter. An endpoint-only check would leave the agent as an authorization bypass.
2. The flat route groups — `/api/characters/{id}`, `/api/beats/{id}`, `/api/tags/{id}`, `/api/questions/{id}`, `/api/arc-stages/{id}`, `/api/chapters/{id}`, `/api/conversations/{id}` — carry no `projectId` in the route, so a filter cannot resolve the owning novel without doing the same DB join the service already does.

So: **endpoint groups get `.RequireAuthorization()` for authentication only**; every service method resolves its owning project and calls a shared `ProjectAccessService`. Tests construct services directly through `ServiceTestFixture.cs:46-62`, which is the single choke point for the new constructor parameter.

---

## Chunk 1 — Accounts, Identity schema, auth endpoints

**Package:** `Microsoft.AspNetCore.Identity.EntityFrameworkCore` 10.0.10 into `src/Novelly.Api/Novelly.Api.csproj`.

**New feature folder `src/Novelly.Api/Users/`:**

- `NovellyUser.cs` — `public class NovellyUser : IdentityUser<Guid>` plus `DisplayName`, `GlobalRole` (enum), `CreatedAt`. Entity config at the bottom of the file: `GlobalRole` stored `HasConversion<string>().HasMaxLength(32)`, matching `ProjectEntityTypeConfiguration` in `Projects/Project.cs:37`.
- `GlobalRole.cs` — `Admin, Writer, Editor, Reviewer`. Crosses the wire as a name (the `JsonStringEnumConverter` at `Program.cs:30` already handles this).
- `UserContracts.cs` — `RegisterRequest(string Email, string Password, string DisplayName)`, `LoginRequest(string Email, string Password)`, `UserResponse(Guid Id, string Email, string DisplayName, GlobalRole GlobalRole)`, `IModelValidator<>` implementations, `ToResponse()` mapping. Same shape as `Projects/ProjectContracts.cs`.
- `UserAccountService.cs` — primary-constructor DI over `UserManager<NovellyUser>`, `SignInManager<NovellyUser>`, `INovelDbContext`, `ILogger<>`, validators. Never log passwords or email bodies; log user ids and role names only.
- `UserEndpoints.cs` — `MapGroup("/api/auth")` with both filters; `POST /register`, `POST /login`, `POST /logout`, `GET /me`. `/register` and `/login` get `.AllowAnonymous()`.

**Do not use `MapIdentityApi<T>()`** — it is bearer-token shaped, exposes 2FA/email-confirm routes we do not want, and cannot host the first-user-becomes-Admin rule. Hand-written endpoints over `SignInManager` cost ~40 lines and stay in the repo's endpoint idiom.

**Roles as a claim, not Identity role tables:** register with `AddIdentityCore<NovellyUser>()` and make `NovelDbContext` inherit `IdentityUserContext<NovellyUser, Guid>` (not `IdentityDbContext`). This drops `AspNetRoles`/`AspNetUserRoles` — the global role is one column, emitted as a `ClaimTypes.Role` claim at sign-in so `RequireRole` still works.

**`Data/NovelDbContext.cs`:** change the base to `IdentityUserContext<NovellyUser, Guid>`; `OnModelCreating` must now call `base.OnModelCreating(builder)` *before* `ApplyConfigurationsFromAssembly` or the Identity tables never get configured. Add `DbSet<NovellyUser> Users` to both the class and the `INovelDbContext` interface at the bottom of the file (line 39ff) — every service depends on the interface.

**`Common/NovellyServiceRegistration.cs`:** add `AddIdentityCore` + `AddSignInManager` + `AddEntityFrameworkStores<NovelDbContext>`, `AddAuthentication(IdentityConstants.ApplicationScheme).AddIdentityCookies()`, and `AddScoped<UserAccountService>()`.

**Cookie configuration** (24h, API-shaped):

- `ExpireTimeSpan = TimeSpan.FromHours(24)`, `SlidingExpiration = false` — "persist for 24 hours" read literally.
- `Cookie.HttpOnly = true`, `SameSite = Lax`, `SecurePolicy = SameAsRequest` (dev is same-origin through the Vite proxy at `vite.config.ts:9-14`).
- `Events.OnRedirectToLogin` / `OnRedirectToAccessDenied` must return bare 401/403 instead of a 302 to a login page — this is an API.

**`Program.cs`:** `UseAuthentication()` + `UseAuthorization()` immediately after `app.UseCors()` (line 71). Add `NotAuthorizedException => (403, "Forbidden")` to the exception-handler switch at lines 54-59, else it falls through to 500. CORS at lines 36-39 needs `.AllowCredentials()` — which forbids wildcard origins, already satisfied since `Cors:Origins` is explicit.

**First user + orphan adoption**, inside `UserAccountService.RegisterAsync`, in one transaction:

```
if (!await db.Users.AnyAsync(ct)) → GlobalRole.Admin
                                  → db.Projects.Where(p => p.OwnerId == null)
                                       .ExecuteUpdateAsync(set => set.SetProperty(p => p.OwnerId, newUserId), ct)
else                              → GlobalRole.Reviewer
```

SQLite is single-writer, so the transaction closes the race; a second concurrent registration blocks and then sees a non-empty table. `OwnerId` lands in chunk 2's migration — until then this branch just sets the role; sequence the `ExecuteUpdate` into chunk 2 if chunks land separately.

**Migration:** `dotnet ef migrations add AddUsers -p src/Novelly.Api -o Data/Migrations`.

**Risk — Identity migration over an existing SQLite db:** the model snapshot gains six Identity tables at once. `TestDatabase` uses `EnsureCreated()` (`tests/.../TestDatabase.cs:17`), so migrations are structurally untested by the suite. Verify by copying the dev `novel.db` aside and booting the API against the copy before touching the LAN deploy's volume.

**Tests** — `tests/Novelly.Api.Tests/UserAccountTests.cs`, over `TestDatabase` with a hand-built `UserManager`:

- `The_first_account_created_becomes_an_admin`
- `Accounts_created_after_the_first_are_reviewers`
- `Registering_with_an_email_already_in_use_is_rejected`
- `Signing_in_with_the_wrong_password_is_rejected`

**Verify:**

```
ASPNETCORE_URLS=http://localhost:5080 dotnet run --project src/Novelly.Api
curl -c jar -X POST localhost:5080/api/auth/register -H 'content-type: application/json' \
     -d '{"email":"james@wamp.dev","password":"...","displayName":"James"}'
curl -b jar localhost:5080/api/auth/me          # 200, globalRole "Admin" as a name
curl localhost:5080/api/auth/me                 # 401, no redirect
```

---

## Chunk 2 — Ownership, grants, and permission enforcement

**`Projects/Project.cs`:** add `Guid? OwnerId` + `NovellyUser? Owner` and `List<ProjectMember> Members`. In `ProjectEntityTypeConfiguration`, the owner relationship uses `OnDelete(DeleteBehavior.Restrict)` — deleting a user must not cascade-delete their novels — while `Members` cascades from the project like the four existing collections.

**`src/Novelly.Api/Users/ProjectMember.cs`:** `ProjectId`, `UserId`, `ProjectRole` (`Writer, Editor, Reviewer`), `GrantedAt`, `GrantedByUserId`. Unique index on `(ProjectId, UserId)`. Config at the bottom of the file; add the `DbSet` to both `NovelDbContext` and `INovelDbContext`.

**`src/Novelly.Api/Users/NovelUserContext.cs`** — scoped, reads `IHttpContextAccessor.HttpContext?.User`:

```
Guid? UserId, GlobalRole? GlobalRole, bool IsAuthenticated, bool IsServicePrincipal
```

Interface at the bottom of the file (single implementation, per `CLAUDE.md`). `AddHttpContextAccessor()` goes into `AddNovelly`. Background/agent paths get a non-HTTP implementation in chunk 3.

**`src/Novelly.Api/Users/ProjectAccessService.cs`** — the whole authorization rule in one place:

```
enum ProjectPermission { Read, Write, CreateContent, DeleteContent, ManageAccess }

Task RequireAsync(Guid projectId, ProjectPermission permission, CancellationToken ct)   // throws NotAuthorizedException
Task<IQueryable<Project>> VisibleProjects()                                             // for list filtering
```

Rule table it implements:

| | Read | Write | CreateContent | DeleteContent | ManageAccess |
|---|---|---|---|---|---|
| Admin (global) | ✓ | ✓ | ✓ | ✓ | ✓ |
| Owner (Writer who created it) | ✓ | ✓ | ✓ | ✓ | ✓ |
| Grant: Writer | ✓ | ✓ | ✓ | ✓ | ✗ |
| Grant: Editor | ✓ | ✓ | ✗ | ✗ | ✗ |
| Grant: Reviewer | ✓ | ✗ | ✗ | ✗ | ✗ |

Creating a *novel* is separate from `CreateContent`: `ProjectService.CreateAsync` requires global role Admin or Writer and stamps `OwnerId` from the user context. A Reviewer who is granted Writer on someone's novel still cannot create novels of their own — global role governs that.

**`Common/NotAuthorizedException.cs`** — one-liner beside `AgentNotConfiguredException.cs`.

**Wiring into services:** each of `ProjectService`, `CharacterService`, `CharacterArcService`, `BeatService`, `TagService`, `ChapterService`, `OpenQuestionService`, `NovelAgentService`, `ImportService` takes `ProjectAccessService` in its primary constructor and calls `RequireAsync` after resolving the owning project. Flat-route services already have a private `FindAsync` that loads the entity — resolve `projectId` from the loaded entity there, so the not-found path (`LogWarning` + null return) still runs first and a permission failure never leaks the existence of a novel the caller cannot see.

`ProjectService.ListAsync` (`ProjectService.cs:18`) filters through `VisibleProjects()`; `GenreService` stays open to any authenticated user (it is a static suggestion list).

**Grant endpoints** in `Users/ProjectMemberEndpoints.cs`, group `/api/projects/{projectId:guid}/members`: `GET` list, `POST` grant `{ email, role }`, `DELETE /{userId:guid}` revoke. All require `ManageAccess`.

**Migration:** `AddProjectOwnershipAndMembers`. `OwnerId` nullable — never backfilled by SQL; chunk 1's adoption step claims orphans.

**Tests** — `tests/Novelly.Api.Tests/ProjectAccessTests.cs` (and additions to `ServiceTestFixture.cs:46-62` giving every fixture a default admin context, so existing tests keep passing unchanged):

- `A_writer_sees_only_novels_they_own_or_have_been_granted`
- `An_editor_can_rewrite_a_chapter_but_cannot_delete_it`
- `An_editor_cannot_create_a_new_novel`
- `A_reviewer_can_read_a_chapter_but_not_change_it`
- `A_writer_granted_access_to_someone_elses_novel_still_cannot_grant_access_to_others`
- `An_admin_reaches_every_novel`
- `Deleting_a_user_leaves_their_novels_standing`

**Verify:** two cookie jars (admin + a second account promoted to Writer), then curl the grant endpoints and confirm 403 on the editor's DELETE and 200 on their PATCH.

---

## Chunk 3 — Service principal for MCP and background work

**Config:** `Auth:ServiceApiKey` in `appsettings.json` (empty by default; real value from environment — never committed, same rule as `ANTHROPIC_API_KEY`).

**`src/Novelly.Api/Users/ServiceApiKeyAuthenticationHandler.cs`** — scheme `"ServiceApiKey"`, header `X-Novelly-Api-Key`, constant-time comparison. The key maps to a **seeded service user row** (created on boot when a key is configured) rather than a synthetic principal, so `OwnerId`/`GrantedByUserId` foreign keys stay valid. Service user carries `GlobalRole.Admin`.

The default authorization policy accepts either scheme (`IdentityConstants.ApplicationScheme` or `ServiceApiKey`). When no key is configured the scheme registers but never authenticates — the app must stay fully usable without it, mirroring the agent-optional rule.

**`ImportJobRunner`** (hosted service, no HTTP context — `NovellyServiceRegistration.cs:47`): add `RequestedByUserId` to `ImportJob`, stamped at enqueue time, and have the runner push a `NovelUserContext` for that user into its scope. Background work then runs as the person who asked for it rather than as a god-mode principal. Small migration: `AddImportJobRequestedBy`.

**`src/Novelly.Mcp/Program.cs`:** in the `AddHttpClient<NovelApiClient>` delegate, add `client.DefaultRequestHeaders.Add("X-Novelly-Api-Key", key)` from `NOVELLY_API_KEY`. Add `Unauthorized`/`Forbidden` cases to the status switch in `NovelApiClient.cs:53-58` so the agent sees "Not permitted: …" instead of a bare "API returned 401".

**Tests:** `ServiceApiKeyTests` — `A_request_with_the_configured_service_key_is_admitted_as_the_service_user`, `A_request_with_a_wrong_key_is_rejected`, `The_api_still_serves_signed_in_users_when_no_service_key_is_configured`.

**Verify:** build the MCP server and drive it over stdio (`initialize` → `notifications/initialized` → `tools/list` → `tools/call list_projects`) with the key set, then again with it unset to confirm the failure message is legible.

---

## Chunk 4 — Web client: login, session, grant management

- `src/Novelly.Web/src/api/client.ts` — add `credentials: 'include'` to the `fetch` at line 14. Pairs with `AllowCredentials()` from chunk 1.
- `src/api/types.ts` — `GlobalRole`/`ProjectRole` string-literal unions plus the exported value arrays (the file's existing convention, feeding `<Select>`), `User`, `ProjectMember`.
- `src/api/hooks.ts` — `keys.me`, `keys.members(projectId)`; `useMe`, `useLogin`, `useRegister`, `useLogout`, `useProjectMembers`, `useGrantAccess`, `useRevokeAccess`. `useLogout` clears the whole cache (`qc.clear()`).
- `src/auth/AuthContext.tsx` — provider over `useMe`, exposing `user`, `isPending`, and a `can(permission, project)` helper so pages hide what the role forbids.
- `src/pages/LoginPage.tsx` — email/password, a register toggle, copy noting the first account becomes the admin.
- `src/App.tsx` — `/login` route outside the guard; everything else wrapped in a `RequireAuth` element that renders `<Spinner>` while `useMe` is pending and `<Navigate to="/login">` on 401.
- `src/pages/ProjectLayout.tsx` — user chip + logout in the sticky header (lines 33-55).
- `src/pages/SettingsPage.tsx` — a "People" section listing members with a role `<Select>` and a remove button, visible only with `ManageAccess`. Reuse `ConfirmModal` for revoke.
- Hide, don't just fail: "New novel" hidden for Reviewers/Editors (`ProjectsPage.tsx:30`), delete buttons hidden for Editors and Reviewers, `AutoField`s rendered read-only for Reviewers.

**Verify:** `dotnet run --project src/Novelly.AppHost`, then in the browser register the first account (lands as Admin, adopts existing novels), register a second, promote it to Writer, grant it Editor on a novel, and confirm the delete affordances are gone and a forced PATCH still returns 403.

---

## Cross-cutting risks

- **No endpoint-level tests exist** (`tests/` has zero `WebApplicationFactory` usage). Authentication lives in middleware the current suite structurally cannot reach. Chunk 1 should add `Microsoft.AspNetCore.Mvc.Testing` and a small factory — otherwise the cookie config, the 401-instead-of-302 behavior, and `.RequireAuthorization()` coverage are all unverified.
- **Coverage gate** is 60% line in `scripts/ci/prepush.sh`; a large batch of new `[ExcludeFromCodeCoverage]`-worthy wiring (handlers, registration) should be marked so the floor holds.
- **`check-no-comments.sh`** fails the push on any `//` comment — the Identity boilerplate copied from docs usually carries them.
- **The import feature assumes local-first** ("API and browser share a filesystem", `docs/plans/api/outline_import_agent_plan.md:5`). Multi-user makes that assumption unsafe, but it is out of scope here; flag it rather than fix it.
- **Prose comments for Reviewers** are deliberately not built. The seam is the `ProjectPermission.Read` grant plus `ProjectMember` — a later `Comments/` feature hangs off chapter + character range without touching this model.