rbrooks/Circa
Watch
1
0
Fork
You've already forked Circa
 0
Code Issues 66 Pull requests 1 Projects Releases 2 Packages Wiki Activity Actions

Implement multi-user roles and review attribution #22

New issue
Open
opened 2026-07-28 04:55:06 +00:00 by claude-bot · 1 comment
claude-bot commented 2026-07-28 04:55:06 +00:00
Copy link

Context

UserRole exists and require_role guards routes, but the app is effectively
single-user: there is no user management, and decisions and comments are not surfaced
with their author. Phase 2 is where multiple family members review together, and shared
review without attribution makes disagreements impossible to trace.

Scope

Role management and visible attribution across the review surfaces.

Implementation notes

  • Define the role set concretely against the spec — at minimum viewer, reviewer, admin —
    and document exactly what each may do. Ambiguity here becomes an authorization bug later.
  • Admin-facing user management: list users, change roles, deactivate. Full admin console
    is v0.4.0; this is the minimum needed for a working multi-user deployment.
  • An admin must not be able to remove their own last admin role and lock the collection out.
  • Show the author and timestamp on every decision, comment, note edit, and manual evidence
    row. The data is already there — created_by is populated — it just is not surfaced.
  • Show attribution in the history tab so the sequence of who changed what is readable.
  • Deactivating a user must preserve their prior contributions. Review history is
    append-only; removing a person cannot rewrite what they decided.
  • Extend the authorization tests from #8 to cover the full role matrix.

Done when

  • Roles are documented with an explicit permission matrix
  • An admin can manage users and roles
  • The last admin cannot be demoted or removed
  • Decisions, comments, notes, and evidence display their author
  • Deactivation preserves history
  • The role matrix is covered by tests

References

  • backend/app/models/models.py (UserRole, created_by fields)
  • backend/app/auth/session.py (require_role)
  • docs/circa-spec.md Phase 2

Related: #8 (authorization tests).

## Context `UserRole` exists and `require_role` guards routes, but the app is effectively single-user: there is no user management, and decisions and comments are not surfaced with their author. Phase 2 is where multiple family members review together, and shared review without attribution makes disagreements impossible to trace. ## Scope Role management and visible attribution across the review surfaces. ## Implementation notes - Define the role set concretely against the spec — at minimum viewer, reviewer, admin — and document exactly what each may do. Ambiguity here becomes an authorization bug later. - Admin-facing user management: list users, change roles, deactivate. Full admin console is v0.4.0; this is the minimum needed for a working multi-user deployment. - An admin must not be able to remove their own last admin role and lock the collection out. - Show the author and timestamp on every decision, comment, note edit, and manual evidence row. The data is already there — `created_by` is populated — it just is not surfaced. - Show attribution in the history tab so the sequence of who changed what is readable. - Deactivating a user must preserve their prior contributions. Review history is append-only; removing a person cannot rewrite what they decided. - Extend the authorization tests from #8 to cover the full role matrix. ## Done when - [ ] Roles are documented with an explicit permission matrix - [ ] An admin can manage users and roles - [ ] The last admin cannot be demoted or removed - [ ] Decisions, comments, notes, and evidence display their author - [ ] Deactivation preserves history - [ ] The role matrix is covered by tests ## References - `backend/app/models/models.py` (`UserRole`, `created_by` fields) - `backend/app/auth/session.py` (`require_role`) - `docs/circa-spec.md` Phase 2 Related: #8 (authorization tests).
claude-bot added this to the v0.4.0 milestone 2026-07-28 04:55:06 +00:00
claude-bot commented 2026-07-28 06:07:51 +00:00
Author
Copy link

Amended by the audit of 2026-07-28.

The current default role is a live vulnerability, not just future work (#56). User.role
defaults to UserRole.reviewer and upsert_from_provider passes no role — so every account that
completes OAuth gets write access to the entire collection, with no allowlist and no approval step.
reviewer can also reach POST /api/ingest, which is the arbitrary-write primitive in #55.

That fix belongs in v0.1.1 (#56), ahead of this issue.

Two further splits:

  • Display names (#107) — the attribution slice is small, independently valuable, and needed as
    soon as a second person touches the collection. It also removes the UUID disclosure that #57
    requires anyway.
  • Enforcement gaps that exist today (#73) — zero admin-only routes exist (_ADMIN is defined
    and never used), the viewer role is unreachable, and /api/ingest's docstring says
    "Admin-only in Phase 1" while the code enforces reviewer.
**Amended by the audit of 2026-07-28.** **The current default role is a live vulnerability, not just future work (#56).** `User.role` defaults to `UserRole.reviewer` and `upsert_from_provider` passes no role — so every account that completes OAuth gets write access to the entire collection, with no allowlist and no approval step. `reviewer` can also reach `POST /api/ingest`, which is the arbitrary-write primitive in #55. That fix belongs in v0.1.1 (#56), ahead of this issue. Two further splits: - **Display names (#107)** — the attribution slice is small, independently valuable, and needed as soon as a second person touches the collection. It also removes the UUID disclosure that #57 requires anyway. - **Enforcement gaps that exist today (#73)** — zero admin-only routes exist (`_ADMIN` is defined and never used), the `viewer` role is unreachable, and `/api/ingest`'s docstring says "Admin-only in Phase 1" while the code enforces reviewer.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
renovate/lock-file-maintenance
v0.3.3
v0.3.2
v0.3.1
v0.3.0
v0.2.0
Labels
Clear labels   
ai
AI / OCR evidence backends

  
backend
Backend / API / services

  
bug
Something is not working

  
docs
Documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
frontend
React UI

  
help wanted
Need some help

  
infra
CI, deployment, tooling

  
invalid
Something is wrong

  
question
More information is needed

  
security
Auth, authorization, data safety

  
testing
Automated tests and QA

  
wontfix
This won't be fixed

  
worker
Background jobs and worker runtime

No labels
ai
backend
bug
docs
duplicate
enhancement
frontend
help wanted
infra
invalid
question
security
testing
wontfix
worker
Milestone
Clear milestone
No items
No milestone
v0.4.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Circa#22
No description provided.