diff --git a/.agents/skills/fastapi b/.agents/skills/fastapi deleted file mode 120000 index c72ba0755d..0000000000 --- a/.agents/skills/fastapi +++ /dev/null @@ -1 +0,0 @@ -../../.venv/lib/python3.14/site-packages/fastapi/.agents/skills/fastapi \ No newline at end of file diff --git a/.agents/skills/library-skills/.library-skills.json b/.agents/skills/library-skills/.library-skills.json deleted file mode 100644 index 9ab33224bf..0000000000 --- a/.agents/skills/library-skills/.library-skills.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "kind": "tool-skill", - "version": "0.0.19" -} diff --git a/.agents/skills/library-skills/SKILL.md b/.agents/skills/library-skills/SKILL.md deleted file mode 100644 index 5c81966703..0000000000 --- a/.agents/skills/library-skills/SKILL.md +++ /dev/null @@ -1,37 +0,0 @@ ---- -name: library-skills -description: Use Library Skills to discover, install, refresh, repair, check, and manage agent skills from installed packages. ---- - -# Library Skills - -Use this skill when a project might benefit from agent skills bundled by its installed packages, or when existing Library Skills-managed symlinks are stale, broken, orphaned, or need to be checked. - -Run commands from the project root. - -Agents bundle their own skills by including an `.agents/skills` directory. More details in [Library Skills](https://library-skills.io). - -## First-Time Setup - -- Make sure project dependencies are installed first, for example with `uv sync` for Python projects or `npm install` / `bun install` for Node.js projects. -- Run `uvx library-skills` or `npx library-skills` to discover skills bundled by the installed packages and install selected skills interactively. -- Use `uvx library-skills --all` or `npx library-skills --all` only when all newly discovered skills should be installed without selecting individual skills. -- Use `uvx library-skills --tool-skill` or `npx library-skills --tool-skill` to copy this Library Skills tool skill into the project so future agents know how to discover, install, update, repair, and check skills. - -## Commands - -- Run `uvx library-skills` or `npx library-skills` to discover package-provided skills, install selected new skills, and reconcile existing managed symlinks. -- Run `uvx library-skills list` or `npx library-skills list` to inspect discovered and installed skills. -- Run `uvx library-skills list --json` or `npx library-skills list --json` for machine-readable installed status. -- Run `uvx library-skills scan --json` or `npx library-skills scan --json` for discovery-only automation. -- Run `uvx library-skills --check` or `npx library-skills --check` to validate managed skill symlink state without changing files. -- Run `uvx library-skills --yes` or `npx library-skills --yes` to repair stale managed symlinks and remove orphaned managed symlinks non-interactively. -- Add `--claude` when `.claude/skills` should also be managed. -- Add `--skill NAME` to install a specific discovered skill by name. - -## Safety - -- Prefer rerunning `library-skills` over editing managed symlinks manually. -- If installed skill symlinks are broken, dependencies may not be installed yet. Try the project's normal install command first, such as `uv sync`, `npm install`, or `bun install`, then rerun `library-skills`. -- Do not delete or overwrite hand-authored skill directories. -- Library Skills only removes managed symlinks. It should not remove copied or hand-authored skill directories. diff --git a/.agents/skills/sqlmodel b/.agents/skills/sqlmodel deleted file mode 120000 index 8dfb559dff..0000000000 --- a/.agents/skills/sqlmodel +++ /dev/null @@ -1 +0,0 @@ -../../.venv/lib/python3.14/site-packages/sqlmodel/.agents/skills/sqlmodel \ No newline at end of file diff --git a/.claude/skills/fastapi b/.claude/skills/fastapi deleted file mode 120000 index c72ba0755d..0000000000 --- a/.claude/skills/fastapi +++ /dev/null @@ -1 +0,0 @@ -../../.venv/lib/python3.14/site-packages/fastapi/.agents/skills/fastapi \ No newline at end of file diff --git a/.claude/skills/library-skills/.library-skills.json b/.claude/skills/library-skills/.library-skills.json deleted file mode 100644 index 9ab33224bf..0000000000 --- a/.claude/skills/library-skills/.library-skills.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "kind": "tool-skill", - "version": "0.0.19" -} diff --git a/.claude/skills/library-skills/SKILL.md b/.claude/skills/library-skills/SKILL.md deleted file mode 100644 index 5c81966703..0000000000 --- a/.claude/skills/library-skills/SKILL.md +++ /dev/null @@ -1,37 +0,0 @@ ---- -name: library-skills -description: Use Library Skills to discover, install, refresh, repair, check, and manage agent skills from installed packages. ---- - -# Library Skills - -Use this skill when a project might benefit from agent skills bundled by its installed packages, or when existing Library Skills-managed symlinks are stale, broken, orphaned, or need to be checked. - -Run commands from the project root. - -Agents bundle their own skills by including an `.agents/skills` directory. More details in [Library Skills](https://library-skills.io). - -## First-Time Setup - -- Make sure project dependencies are installed first, for example with `uv sync` for Python projects or `npm install` / `bun install` for Node.js projects. -- Run `uvx library-skills` or `npx library-skills` to discover skills bundled by the installed packages and install selected skills interactively. -- Use `uvx library-skills --all` or `npx library-skills --all` only when all newly discovered skills should be installed without selecting individual skills. -- Use `uvx library-skills --tool-skill` or `npx library-skills --tool-skill` to copy this Library Skills tool skill into the project so future agents know how to discover, install, update, repair, and check skills. - -## Commands - -- Run `uvx library-skills` or `npx library-skills` to discover package-provided skills, install selected new skills, and reconcile existing managed symlinks. -- Run `uvx library-skills list` or `npx library-skills list` to inspect discovered and installed skills. -- Run `uvx library-skills list --json` or `npx library-skills list --json` for machine-readable installed status. -- Run `uvx library-skills scan --json` or `npx library-skills scan --json` for discovery-only automation. -- Run `uvx library-skills --check` or `npx library-skills --check` to validate managed skill symlink state without changing files. -- Run `uvx library-skills --yes` or `npx library-skills --yes` to repair stale managed symlinks and remove orphaned managed symlinks non-interactively. -- Add `--claude` when `.claude/skills` should also be managed. -- Add `--skill NAME` to install a specific discovered skill by name. - -## Safety - -- Prefer rerunning `library-skills` over editing managed symlinks manually. -- If installed skill symlinks are broken, dependencies may not be installed yet. Try the project's normal install command first, such as `uv sync`, `npm install`, or `bun install`, then rerun `library-skills`. -- Do not delete or overwrite hand-authored skill directories. -- Library Skills only removes managed symlinks. It should not remove copied or hand-authored skill directories. diff --git a/.claude/skills/sqlmodel b/.claude/skills/sqlmodel deleted file mode 120000 index 8dfb559dff..0000000000 --- a/.claude/skills/sqlmodel +++ /dev/null @@ -1 +0,0 @@ -../../.venv/lib/python3.14/site-packages/sqlmodel/.agents/skills/sqlmodel \ No newline at end of file diff --git a/.env b/.env index e16784221f..bff1626304 100644 --- a/.env +++ b/.env @@ -3,10 +3,15 @@ FASTAPI_ENV=development PROJECT_NAME="Full Stack FastAPI Project" -SECRET_KEY=changethis +SECRET_KEY=dev-local-jwt-secret-key-minimum-32-characters-long FIRST_SUPERUSER=admin@example.com FIRST_SUPERUSER_PASSWORD=changethis +MANAGER_USER=manager@example.com +MANAGER_USER_PASSWORD=changethis +MEMBER_USER=member@example.com +MEMBER_USER_PASSWORD=changethis + # Emails SMTP_HOST=localhost EMAILS_FROM_EMAIL=info@example.com diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 75f6b591a4..4145ce0d0d 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -45,6 +45,8 @@ jobs: if: ${{ needs.changes.outputs.changed == 'true' }} timeout-minutes: 15 runs-on: ubuntu-latest + env: + COMPOSE_FILE: compose.yml:compose.override.example.yml strategy: matrix: shardIndex: [1, 2, 3, 4] diff --git a/.github/workflows/test-backend.yml b/.github/workflows/test-backend.yml index 92c5fc9824..a93db61727 100644 --- a/.github/workflows/test-backend.yml +++ b/.github/workflows/test-backend.yml @@ -12,6 +12,8 @@ jobs: test-backend: runs-on: ubuntu-latest timeout-minutes: 5 + env: + COMPOSE_FILE: compose.yml:compose.override.example.yml steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/test-docker-compose.yml b/.github/workflows/test-docker-compose.yml index 4551a15760..a0df456b7f 100644 --- a/.github/workflows/test-docker-compose.yml +++ b/.github/workflows/test-docker-compose.yml @@ -13,6 +13,8 @@ jobs: test-docker-compose: runs-on: ubuntu-latest timeout-minutes: 10 + env: + COMPOSE_FILE: compose.yml:compose.override.example.yml steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.gitignore b/.gitignore index 3c697eb7da..b50bba47fb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,12 @@ .vscode/* !.vscode/extensions.json +.idea/ +.agents/ +.claude/ node_modules/ backend/app/frontend/ /test-results/ /playwright-report/ /blob-report/ /playwright/.cache/ +compose.override.yml diff --git a/NOTES.md b/NOTES.md new file mode 100644 index 0000000000..43041626dc --- /dev/null +++ b/NOTES.md @@ -0,0 +1,41 @@ +# Notes for Reviewers + +Supplementary context for the [Fullstack Dev Test Task](https://github.com/evios/Fullstack-Dev-Test-Task). Required deliverables are in `README.md`; this file covers scope, trade-offs, and follow-ups. + +## What Was Prioritized + +1. **Centralized backend policy** — `permissions.py` + `require_permission()` dependencies. +2. **Consistent frontend UX** — hidden nav, route guards, `AccessDenied` component. +3. **Focused tests** — eight authorization scenarios (allow + deny paths), including denial logging. +4. **Runnable setup** — Docker Compose, seed users, migration, smoke script. + +## Scope Cuts (and Why) + +| Cut | Reason | +|-----|--------| +| E2E Playwright tests for RBAC | Backend tests + manual smoke cover critical paths within timebox | +| `/me/permissions` API | Small static matrix; mirroring in frontend is simpler (see ADR 002) | +| Row-level / resource ownership rules | Assignment surface is role-based, not object-level ACL | +| Regenerating OpenAPI client for `role` | Patched `types.gen.ts` manually; `generate-client.sh` needs running stack | + +## Trade-offs + +- **`is_superuser` is derived from `role`** — PostgreSQL generated column plus read-only `@computed_field` in API responses; writable schemas accept `role` only (`extra="forbid"` blocks legacy `is_superuser` input). +- **Items use the permission layer** — Admin holds `items:list_any` / `items:manage_any`; manager and member manage only their own items. +- **Frontend permission duplication** — Acceptable for three roles; would generate or fetch capabilities in a larger system. + +## Observability + +Denied authorization attempts are logged at `WARNING` from `app.api.deps` with user id, email, role, and requested permission or required roles. Useful for audit trails and debugging mistaken 403 responses. + +## With More Time + +- Add Playwright flows: login as member → direct `/admin` → see Access Denied. +- Expose read-only permissions in OpenAPI and regenerate the frontend client. +- Structured audit log table for denied access (not only application logs). +- Feature flags or admin UI to assign roles without DB access. + +## Architecture Docs + +- ADRs: `docs/adr/001-permission-based-rbac.md`, `docs/adr/002-frontend-permission-mirror.md` +- Auth flow diagram: `README.md` (Mermaid) diff --git a/README.md b/README.md index a3e20f4dbf..ffba59d662 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,125 @@ +# Full Stack FastAPI Template — RBAC Extension + +This project extends the [Full Stack FastAPI Template](https://github.com/fastapi/full-stack-fastapi-template) with role-based access control (RBAC) for the [Fullstack Dev Test Task](https://github.com/evios/Fullstack-Dev-Test-Task). + +## Submission + +Fork PR: https://github.com/fastapi/full-stack-fastapi-template/pull/2445 +Fork: https://github.com/yanhub/full-stack-fastapi-template + +## Quick Start (Docker) + +Requires [Docker Desktop](https://www.docker.com/products/docker-desktop/) (macOS, Linux, or Windows). + +```bash +git clone +cd +cp compose.override.example.yml compose.override.yml +docker compose build backend +docker compose run --rm backend bash scripts/prestart.sh +docker compose up -d +``` + +Open: + +| Service | URL | +|---------|-----| +| App (API + frontend) | http://localhost:8000 | +| API docs | http://localhost:8000/docs | +| Adminer | http://localhost:8080 | +| Mailpit | http://localhost:8025 | +| Traefik (via proxy) | http://localhost | + +`compose.override.yml` is local-only (see `compose.override.example.yml`). Adjust ports there if `80` or `5432` are already in use on your machine. + +## Seed Users + +| Email | Password | Role | +|-------|----------|------| +| admin@example.com | changethis | admin | +| manager@example.com | changethis | manager | +| member@example.com | changethis | member | + +Credentials are configured in `.env` (`FIRST_SUPERUSER*`, `MANAGER_USER*`, `MEMBER_USER*`). + +## Permission Matrix + +| Action | admin | manager | member | +|--------|:-----:|:-------:|:------:| +| List all users | yes | yes | no | +| Create user | yes | no | no | +| View metrics | yes | yes | no | +| Update own profile | yes | yes | yes | +| Update any profile | yes | no | no | +| Global settings | yes | no | no | +| List all items | yes | no | no | +| Manage any item (read/update/delete) | yes | no | no | +| Manage own items | yes | yes | yes | + +## Authorization Approach + +Roles are stored on the `User.role` column (`admin`, `manager`, `member`) with an Alembic migration backfilling existing superusers to `admin`. + +Backend authorization is centralized in `backend/app/core/permissions.py`. FastAPI dependencies in `backend/app/api/deps.py` expose `require_permission(...)` and enforce checks on route handlers (users, metrics). The API returns HTTP `403` with a clear message when access is denied. + +The frontend mirrors the same permission matrix in `frontend/src/lib/permissions.ts`. The `usePermissions()` hook drives sidebar visibility, route-level UI guards, and an `AccessDenied` component for direct navigation to forbidden pages. The backend remains the source of truth; the UI only hides or blocks navigation for better UX. + +Denied access attempts are logged at `WARNING` level from `app.api.deps` (user id, email, role, permission) for observability. + +### Authorization Flow + +```mermaid +flowchart TB + subgraph client [Frontend] + Login[Login / JWT stored] + Hook[usePermissions from user.role] + Nav[Sidebar hides forbidden links] + Guard[Route guard / AccessDenied] + Login --> Hook --> Nav + Hook --> Guard + end + + subgraph api [Backend API] + JWT[get_current_user validates JWT] + Perm[require_permission dependency] + Matrix[user_has_permission in permissions.py] + Route[Route handler] + JWT --> Perm --> Matrix + Matrix -->|allowed| Route + Matrix -->|denied| Log403[Log WARNING + HTTP 403] + end + + Guard -->|API call| JWT + Nav -->|API call| JWT +``` + +Further reading: + +- [NOTES.md](NOTES.md) — scope cuts, trade-offs, follow-ups +- [docs/ai-conversations/](docs/ai-conversations/) — English copies of AI-assisted development sessions (submission requirement) +- [docs/adr/001-permission-based-rbac.md](docs/adr/001-permission-based-rbac.md) +- [docs/adr/002-frontend-permission-mirror.md](docs/adr/002-frontend-permission-mirror.md) + +## Running Tests + +```bash +# Authorization-focused tests (rebuild backend image after pulling changes) +docker compose run --rm backend pytest tests/api/routes/test_authorization.py -v + +# Smoke check (Python 3 on the host; stack must be up) +python3 scripts/smoke_rbac.py +``` + +## Database Migrations + +```bash +docker compose run --rm backend alembic upgrade head +``` + +New migration: `a1b2c3d4e5f6_add_user_role.py` (adds `role` column). + +--- + # Full Stack FastAPI Template [![Test Docker Compose](../../actions/workflows/test-docker-compose.yml/badge.svg)](../../actions/workflows/test-docker-compose.yml) diff --git a/TASK.md b/TASK.md new file mode 100644 index 0000000000..8b383a7996 --- /dev/null +++ b/TASK.md @@ -0,0 +1,223 @@ +# Fullstack-Dev-Test-Task + +This is a task to test potential candidate skills in Python + SQL + TypeScript. + +- **Assignment**: Add Role-Aware Access + Architecture Decisions + **Run the app** +- **Timebox**: Aim for up to 1 hour. If you cut scope, say what you cut and why. + + +## Table of Contents + +- [Goal](#goal) +- [Base Template](#base-template) +- [Suggested Time Allocation](#suggested-time-allocation) +- [Requirements](#requirements) + - [1. Clone the Base Template](#1-clone-the-base-template) + - [2. Roles and Authorization Surface](#2-roles-and-authorization-surface) + - [3. Code Quality Expectations](#3-code-quality-expectations) + - [4. Architecture & Documentation](#4-architecture--documentation) + - [5. Non-Functional Requirements](#5-non-functional-requirements) + - [6. UX Behavior](#6-ux-behavior) + - [7. Developer UX](#7-developer-ux) +- [Constraints](#constraints) +- [What We Review](#what-we-review) +- [Submission](#submission) + +## Goal + +Add role-based access control (RBAC) to the existing Full-Stack FastAPI Template so that only authorized users can access sensitive endpoints and UI sections. + +**We prioritize clean, maintainable code over comprehensive test coverage or extensive documentation.** + +You may reuse any libraries already in the template. + +> **Note**: RBAC can be implemented with simple role checks or a small policy layer. Keep scope tight. Favor clarity over cleverness. + +## Base Template + +**Tech Stack**: +- **Backend**: FastAPI / SQLModel / PostgreSQL +- **Frontend**: React / TypeScript + +**Repository**: [full-stack-fastapi-template](https://github.com/fastapi/full-stack-fastapi-template/tree/master) + +## Suggested Time Allocation + +How we believe it is doable in a 1-hour timebox: + +| Activity | Time | Priority | +|----------|----------------|----------| +| Understanding the codebase | 15 min | High | +| Implementation (clear, maintainable code) | 25 mins | **Critical** | +| Testing (focused, critical paths) | 10 min | High | +| Documentation (README updates) | 10 min | Medium | + +**If running short on time:** +- ✓ **Prioritize**: Clear, working authorization code with consistent patterns +- ✓ **Then**: 3-5 well-chosen tests covering critical scenarios +- ⚠ **Cut if needed**: Extra features, comprehensive test coverage, diagrams +- ❌ **Don't cut**: Security checks, README setup instructions + +## Requirements + +### 1. Clone the Base Template + +Clone the repository: https://github.com/fastapi/full-stack-fastapi-template/tree/master + +### 2. Roles and Authorization Surface + +#### Implement the Following Roles + +| Role | Permissions | +|------|-------------| +| **admin** | Full access to user management and settings | +| **manager** | Can list users and view metrics, but not change global settings | +| **member** | Can only access their own profile and basic app features | + +#### Protect a Small but Realistic Surface + +- List users +- Create user +- View "metrics/insights" page (simple stub is acceptable) +- View and update own profile + +**Exact permission mapping is up to you.** + +State it clearly in your docs and enforce it consistently in the backend and frontend. + +#### Example Permission Matrix (Document Something Similar) + +| Action | admin | manager | member | +|--------|-------|---------|--------| +| List all users | ✓ | ✓ | ✗ | +| Create user | ✓ | ✗ | ✗ | +| View metrics | ✓ | ✓ | ✗ | +| Update own profile | ✓ | ✓ | ✓ | +| Update any profile | ✓ | ✗ | ✗ | + +### 3. Code Quality Expectations + +**We prioritize maintainable, readable code over clever solutions.** + +- **Clear naming**: Function/variable names that explain intent without comments +- **Single responsibility**: Small, focused functions +- **Easy to extend**: Adding a new role shouldn't require touching 10+ files +- **Self-documenting**: Code structure makes the authorization model obvious + +> **Key principle**: A teammate should understand your authorization model in 5 minutes by reading your code. + +### 4. Architecture & Documentation + +Document your implementation approach clearly but concisely. + +#### Required + +- [ ] **Permission matrix** in README showing which role can access what +- [ ] **Brief explanation** (2-4 paragraphs) of your authorization approach: + - Where authorization checks live (middleware, dependencies, decorators?) + - How roles are stored and validated + - How frontend learns about user capabilities +- [ ] **Inline code comments** only for non-obvious authorization logic + +#### Optional (Bonus Points) + +- [ ] **1-2 Architecture Decision Records (ADRs)** for your most critical decisions + - Use any simple ADR format (problem, options, decision, trade-offs) + - 200-400 words each + - Example topics: Why you chose your authorization pattern, where checks live, how the frontend handles permissions +- [ ] **Simple diagram** showing where auth/authz checks happen + - Mermaid, C4-style, or hand-drawn PNG is fine + +**Philosophy**: We value clear thinking over formal documentation. +Your code should clearly explain your approach; that's usually sufficient. +RBAC implementation, though, usually has at least a few options to implement, hence an additional README will add value. + +### 5. Non-Functional Requirements + +Demonstrate you considered real-world constraints: + +#### 1. Maintainability (Critical) + +- Keep coupling low; use consistent patterns +- A teammate should understand your authorization logic in 5 minutes + +#### 2. Testability (Important) + +- Provide **focused backend tests** covering critical authorization paths + +> **Note**: Tests are required, but we prioritize **quality over quantity**. 3 well-chosen tests with clean code beat 20 tests with spaghetti code. + +### 3. UX Behavior + +- **The UI** should: + - Hide navigation links/buttons that the user can't access + - Show a friendly "Forbidden" or "Access Denied" message if navigating directly to unauthorized routes + - Not just fail silently or show cryptic errors + +### 4. Developer UX + +Update the README with: + +- **How to run locally** (setup, dependencies, database) +- **How to seed test data** with at least one admin and one non-admin user +- **How to run tests** +- **Database migrations** for any schema changes (if applicable) + +Make it easy for us to run your solution without hunting for setup instructions. + +## What We Review + +### Primary Criteria (60%) + +**Code readability and maintainability** +- ✓ Clear separation of concerns +- ✓ Consistent authorization patterns +- ✓ Self-documenting code structure +- ✓ Low coupling between components +- ✓ Easy to understand and extend + +**Working RBAC implementation** +- ✓ Consistent enforcement in backend and frontend +- ✓ No obvious security gaps or privilege escalation +- ✓ Correct HTTP status codes and error handling + +### Secondary Criteria (30%) + +**Test coverage** +- ✓ Focused tests on critical authorization paths +- ✓ Both allowed and denied scenarios tested +- ✓ Tests are clear and well-named + +**Setup and documentation** +- ✓ Setup instructions work on first try +- ✓ Clear explanation of authorization approach +- ✓ Permission matrix documented + +### Nice to Have (10%) + +- Thoughtful UX for forbidden states +- Observability (logging denied attempts) +- Architecture Decision Records (ADRs) +- Helpful diagrams +- Extra polish + +> **Philosophy**: We're evaluating your ability to write production-quality code under time constraints. We'd rather hire someone who delivers clean, working code with good tests than someone who delivers everything but it's hard to maintain. + +## Submission + +**Deliverables**: + +- [ ] PR or repo link with commit history +- [ ] Updated README with: + - Setup instructions + - Permission matrix + - Brief explanation of your approach +- [ ] Backend tests covering critical authorization scenarios +- [ ] Working implementation of RBAC +- [ ] Optional: `NOTES.md` with anything you want us to know (scope cuts, trade-offs, what you'd do with more time) +- [ ] Copies of conversations with AI tools (`docs/ai-conversations/`, English) +- [ ] Fork of [full-stack-fastapi-template](https://github.com/fastapi/full-stack-fastapi-template) with an open PR + +--- + +**Good luck!** Focus on demonstrating clear thinking and solid engineering fundamentals. We're looking for maintainable code, not perfect code. diff --git a/backend/Dockerfile b/backend/Dockerfile index 401eb536a3..e6d91431c1 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -51,6 +51,8 @@ COPY ./backend/pyproject.toml ./backend/alembic.ini /app/backend/ COPY ./backend/app /app/backend/app +COPY ./backend/tests /app/backend/tests + COPY --from=frontend-build /app/backend/app/frontend /app/backend/app/frontend # Sync the project diff --git a/backend/README.md b/backend/README.md index 92175a0b2b..33858ade09 100644 --- a/backend/README.md +++ b/backend/README.md @@ -50,7 +50,7 @@ The application is available at `http://localhost:8000`. ### Docker Compose Override -The `compose.override.yml` file contains local settings for published ports, source synchronization, automatic image rebuilds, and backend reloads. Docker Compose applies it automatically when you run `docker compose` without an explicit file list. +The `compose.override.example.yml` file contains local settings for published ports, source synchronization, automatic image rebuilds, and backend reloads. Copy it to `compose.override.yml` (gitignored). Docker Compose applies it automatically when you run `docker compose` without an explicit file list. To open a shell in the backend container: diff --git a/backend/app/alembic/versions/a1b2c3d4e5f6_add_user_role.py b/backend/app/alembic/versions/a1b2c3d4e5f6_add_user_role.py new file mode 100644 index 0000000000..91687a4d37 --- /dev/null +++ b/backend/app/alembic/versions/a1b2c3d4e5f6_add_user_role.py @@ -0,0 +1,38 @@ +"""Add user role column + +Revision ID: a1b2c3d4e5f6 +Revises: fe56fa70289e +Create Date: 2026-08-18 16:30:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = "a1b2c3d4e5f6" +down_revision = "fe56fa70289e" +branch_labels = None +depends_on = None + +user_role_enum = sa.Enum("admin", "manager", "member", name="userrole") + + +def upgrade() -> None: + bind = op.get_bind() + user_role_enum.create(bind, checkfirst=True) + op.add_column( + "user", + sa.Column( + "role", + user_role_enum, + nullable=False, + server_default="member", + ), + ) + op.execute('UPDATE "user" SET role = \'admin\' WHERE is_superuser = true') + op.alter_column("user", "role", server_default=None) + + +def downgrade() -> None: + op.drop_column("user", "role") + user_role_enum.drop(op.get_bind(), checkfirst=True) diff --git a/backend/app/alembic/versions/b2c3d4e5f6a7_sync_user_superuser_with_role.py b/backend/app/alembic/versions/b2c3d4e5f6a7_sync_user_superuser_with_role.py new file mode 100644 index 0000000000..811d67846d --- /dev/null +++ b/backend/app/alembic/versions/b2c3d4e5f6a7_sync_user_superuser_with_role.py @@ -0,0 +1,32 @@ +"""Keep is_superuser consistent with role via check constraint. + +Revision ID: b2c3d4e5f6a7 +Revises: a1b2c3d4e5f6 +Create Date: 2026-08-18 18:30:00.000000 + +""" +from alembic import op + + +revision = "b2c3d4e5f6a7" +down_revision = "a1b2c3d4e5f6" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute( + 'UPDATE "user" SET is_superuser = (role = \'admin\') WHERE ' + "(role = 'admin' AND is_superuser = false) OR " + "(role != 'admin' AND is_superuser = true)" + ) + op.create_check_constraint( + "ck_user_is_superuser_matches_role", + "user", + "(role = 'admin' AND is_superuser = true) OR " + "(role != 'admin' AND is_superuser = false)", + ) + + +def downgrade() -> None: + op.drop_constraint("ck_user_is_superuser_matches_role", "user", type_="check") diff --git a/backend/app/alembic/versions/c3d4e5f6a7b8_is_superuser_generated_column.py b/backend/app/alembic/versions/c3d4e5f6a7b8_is_superuser_generated_column.py new file mode 100644 index 0000000000..2db4a0a3ba --- /dev/null +++ b/backend/app/alembic/versions/c3d4e5f6a7b8_is_superuser_generated_column.py @@ -0,0 +1,40 @@ +"""Convert is_superuser to a PostgreSQL generated column derived from role. + +Revision ID: c3d4e5f6a7b8 +Revises: b2c3d4e5f6a7 +Create Date: 2026-08-18 18:45:00.000000 + +""" +import sqlalchemy as sa +from alembic import op + + +revision = "c3d4e5f6a7b8" +down_revision = "b2c3d4e5f6a7" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.drop_constraint("ck_user_is_superuser_matches_role", "user", type_="check") + op.drop_column("user", "is_superuser") + op.execute( + 'ALTER TABLE "user" ADD COLUMN is_superuser boolean ' + "GENERATED ALWAYS AS (role = 'admin') STORED NOT NULL" + ) + + +def downgrade() -> None: + op.drop_column("user", "is_superuser") + op.add_column( + "user", + sa.Column("is_superuser", sa.Boolean(), nullable=False, server_default="false"), + ) + op.execute('UPDATE "user" SET is_superuser = (role = \'admin\')') + op.alter_column("user", "is_superuser", server_default=None) + op.create_check_constraint( + "ck_user_is_superuser_matches_role", + "user", + "(role = 'admin' AND is_superuser = true) OR " + "(role != 'admin' AND is_superuser = false)", + ) diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 5f28ec692a..e6528f581d 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -1,4 +1,5 @@ -from collections.abc import Generator +import logging +from collections.abc import Callable, Generator from typing import Annotated import jwt @@ -11,7 +12,10 @@ from app.core import security from app.core.config import settings from app.core.db import engine -from app.models import TokenPayload, User +from app.core.permissions import Permission, user_has_permission +from app.models import TokenPayload, User, UserRole + +logger = logging.getLogger(__name__) reusable_oauth2 = OAuth2PasswordBearer( tokenUrl=f"{settings.API_V1_STR}/login/access-token" @@ -49,8 +53,61 @@ def get_current_user(session: SessionDep, token: TokenDep) -> User: CurrentUser = Annotated[User, Depends(get_current_user)] +def _log_access_denied( + user: User, + *, + permission: Permission | None = None, + required_roles: tuple[UserRole, ...] | None = None, +) -> None: + if permission is not None: + logger.warning( + "Access denied: user_id=%s email=%s role=%s permission=%s", + user.id, + user.email, + user.role.value, + permission.value, + ) + elif required_roles is not None: + logger.warning( + "Access denied: user_id=%s email=%s role=%s required_roles=%s", + user.id, + user.email, + user.role.value, + [role.value for role in required_roles], + ) + + +def require_permission(permission: Permission) -> Callable[..., User]: + def permission_checker(current_user: CurrentUser) -> User: + if not user_has_permission(current_user, permission): + _log_access_denied(current_user, permission=permission) + raise HTTPException( + status_code=403, + detail="You do not have permission to perform this action", + ) + return current_user + + return permission_checker + + +def require_roles(*roles: UserRole) -> Callable[..., User]: + allowed = set(roles) + + def role_checker(current_user: CurrentUser) -> User: + if current_user.role not in allowed: + _log_access_denied(current_user, required_roles=roles) + raise HTTPException( + status_code=403, + detail="You do not have permission to perform this action", + ) + return current_user + + return role_checker + + def get_current_active_superuser(current_user: CurrentUser) -> User: - if not current_user.is_superuser: + if current_user.role != UserRole.ADMIN: + _log_access_denied(current_user, required_roles=(UserRole.ADMIN,)) raise HTTPException( status_code=403, detail="The user doesn't have enough privileges" ) diff --git a/backend/app/api/main.py b/backend/app/api/main.py index a42e5003ee..8dc95e2a47 100644 --- a/backend/app/api/main.py +++ b/backend/app/api/main.py @@ -1,11 +1,12 @@ from fastapi import APIRouter -from app.api.routes import items, login, private, users, utils +from app.api.routes import items, login, metrics, private, users, utils from app.core.config import settings api_router = APIRouter() api_router.include_router(login.router) api_router.include_router(users.router) +api_router.include_router(metrics.router) api_router.include_router(utils.router) api_router.include_router(items.router) diff --git a/backend/app/api/routes/items.py b/backend/app/api/routes/items.py index f0eb30e4ce..5e6fcc58a1 100644 --- a/backend/app/api/routes/items.py +++ b/backend/app/api/routes/items.py @@ -5,6 +5,7 @@ from sqlmodel import col, func, select from app.api.deps import CurrentUser, SessionDep +from app.core.permissions import Permission, user_can_manage_item, user_has_permission from app.models import Item, ItemCreate, ItemPublic, ItemsPublic, ItemUpdate, Message router = APIRouter(prefix="/items", tags=["items"]) @@ -18,7 +19,7 @@ def read_items( Retrieve items. """ - if current_user.is_superuser: + if user_has_permission(current_user, Permission.ITEMS_LIST_ANY): count_statement = select(func.count()).select_from(Item) count = session.exec(count_statement).one() statement = ( @@ -53,7 +54,7 @@ def read_item(session: SessionDep, current_user: CurrentUser, id: uuid.UUID) -> item = session.get(Item, id) if not item: raise HTTPException(status_code=404, detail="Item not found") - if not current_user.is_superuser and (item.owner_id != current_user.id): + if not user_can_manage_item(current_user, item.owner_id): raise HTTPException(status_code=403, detail="Not enough permissions") return item @@ -86,7 +87,7 @@ def update_item( item = session.get(Item, id) if not item: raise HTTPException(status_code=404, detail="Item not found") - if not current_user.is_superuser and (item.owner_id != current_user.id): + if not user_can_manage_item(current_user, item.owner_id): raise HTTPException(status_code=403, detail="Not enough permissions") update_dict = item_in.model_dump(exclude_unset=True) item.sqlmodel_update(update_dict) @@ -106,7 +107,7 @@ def delete_item( item = session.get(Item, id) if not item: raise HTTPException(status_code=404, detail="Item not found") - if not current_user.is_superuser and (item.owner_id != current_user.id): + if not user_can_manage_item(current_user, item.owner_id): raise HTTPException(status_code=403, detail="Not enough permissions") session.delete(item) session.commit() diff --git a/backend/app/api/routes/metrics.py b/backend/app/api/routes/metrics.py new file mode 100644 index 0000000000..bcef9540ac --- /dev/null +++ b/backend/app/api/routes/metrics.py @@ -0,0 +1,21 @@ +# Metrics API routes with role-based access for admin and manager roles. +from typing import Any + +from fastapi import APIRouter, Depends + +from app.api.deps import CurrentUser, require_permission +from app.core.permissions import Permission +from app.models import Message + +router = APIRouter(prefix="/metrics", tags=["metrics"]) + + +@router.get( + "/", + dependencies=[Depends(require_permission(Permission.METRICS_VIEW))], + response_model=Message, +) +def read_metrics(_current_user: CurrentUser) -> Any: + return Message( + message="Metrics stub: total users=42, active sessions=7, conversion rate=3.2%" + ) diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index 1748f58484..edffad05e2 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -8,9 +8,10 @@ from app.api.deps import ( CurrentUser, SessionDep, - get_current_active_superuser, + require_permission, ) from app.core.config import settings +from app.core.permissions import Permission, user_has_permission from app.core.security import get_password_hash, verify_password from app.models import ( Item, @@ -20,6 +21,7 @@ UserCreate, UserPublic, UserRegister, + UserRole, UsersPublic, UserUpdate, UserUpdateMe, @@ -31,7 +33,7 @@ @router.get( "/", - dependencies=[Depends(get_current_active_superuser)], + dependencies=[Depends(require_permission(Permission.USERS_LIST))], response_model=UsersPublic, ) def read_users(session: SessionDep, skip: int = 0, limit: int = 100) -> Any: @@ -52,7 +54,9 @@ def read_users(session: SessionDep, skip: int = 0, limit: int = 100) -> Any: @router.post( - "/", dependencies=[Depends(get_current_active_superuser)], response_model=UserPublic + "/", + dependencies=[Depends(require_permission(Permission.USERS_CREATE))], + response_model=UserPublic, ) def create_user(*, session: SessionDep, user_in: UserCreate) -> Any: """ @@ -134,7 +138,7 @@ def delete_user_me(session: SessionDep, current_user: CurrentUser) -> Any: """ Delete own user. """ - if current_user.is_superuser: + if current_user.role == UserRole.ADMIN: raise HTTPException( status_code=403, detail="Super users are not allowed to delete themselves" ) @@ -169,10 +173,10 @@ def read_user_by_id( user = session.get(User, user_id) if user == current_user: return user - if not current_user.is_superuser: + if not user_has_permission(current_user, Permission.USERS_LIST): raise HTTPException( status_code=403, - detail="The user doesn't have enough privileges", + detail="You do not have permission to perform this action", ) if user is None: raise HTTPException(status_code=404, detail="User not found") @@ -181,7 +185,7 @@ def read_user_by_id( @router.patch( "/{user_id}", - dependencies=[Depends(get_current_active_superuser)], + dependencies=[Depends(require_permission(Permission.USERS_UPDATE_ANY))], response_model=UserPublic, ) def update_user( @@ -211,7 +215,10 @@ def update_user( return db_user -@router.delete("/{user_id}", dependencies=[Depends(get_current_active_superuser)]) +@router.delete( + "/{user_id}", + dependencies=[Depends(require_permission(Permission.USERS_DELETE))], +) def delete_user( session: SessionDep, current_user: CurrentUser, user_id: uuid.UUID ) -> Message: diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 1f3c2873c2..6a48ac79e0 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -64,6 +64,10 @@ def emails_enabled(self) -> bool: EMAIL_TEST_USER: EmailStr = "test@example.com" FIRST_SUPERUSER: EmailStr FIRST_SUPERUSER_PASSWORD: str + MANAGER_USER: EmailStr = "manager@example.com" + MANAGER_USER_PASSWORD: str = "changethis" + MEMBER_USER: EmailStr = "member@example.com" + MEMBER_USER_PASSWORD: str = "changethis" def _check_default_secret(self, var_name: str, value: str | None) -> None: if value == "changethis": diff --git a/backend/app/core/db.py b/backend/app/core/db.py index f19ac0d3bf..ccaadadf00 100644 --- a/backend/app/core/db.py +++ b/backend/app/core/db.py @@ -2,32 +2,44 @@ from app import crud from app.core.config import settings -from app.models import User, UserCreate +from app.models import User, UserCreate, UserRole engine = create_engine(str(settings.DATABASE_URL)) -# make sure all SQLModel models are imported (app.models) before initializing DB -# otherwise, SQLModel might fail to initialize relationships properly -# for more details: https://github.com/fastapi/full-stack-fastapi-template/issues/28 - - -def init_db(session: Session) -> None: - # Tables should be created with Alembic migrations - # But if you don't want to use migrations, create - # the tables un-commenting the next lines - # from sqlmodel import SQLModel - - # This works because the models are already imported and registered from app.models - # SQLModel.metadata.create_all(engine) - - user = session.exec( - select(User).where(User.email == settings.FIRST_SUPERUSER) - ).first() +def _ensure_user( + session: Session, + *, + email: str, + password: str, + role: UserRole, +) -> None: + user = session.exec(select(User).where(User.email == email)).first() if not user: user_in = UserCreate( - email=settings.FIRST_SUPERUSER, - password=settings.FIRST_SUPERUSER_PASSWORD, - is_superuser=True, + email=email, + password=password, + role=role, ) - user = crud.create_user(session=session, user_create=user_in) + crud.create_user(session=session, user_create=user_in) + + +def init_db(session: Session) -> None: + _ensure_user( + session, + email=settings.FIRST_SUPERUSER, + password=settings.FIRST_SUPERUSER_PASSWORD, + role=UserRole.ADMIN, + ) + _ensure_user( + session, + email=settings.MANAGER_USER, + password=settings.MANAGER_USER_PASSWORD, + role=UserRole.MANAGER, + ) + _ensure_user( + session, + email=settings.MEMBER_USER, + password=settings.MEMBER_USER_PASSWORD, + role=UserRole.MEMBER, + ) diff --git a/backend/app/core/permissions.py b/backend/app/core/permissions.py new file mode 100644 index 0000000000..fe54c8df49 --- /dev/null +++ b/backend/app/core/permissions.py @@ -0,0 +1,44 @@ +# Central RBAC permission definitions and role-to-permission mapping. +import uuid +from enum import StrEnum + +from app.models import User, UserRole + + +class Permission(StrEnum): + USERS_LIST = "users:list" + USERS_CREATE = "users:create" + USERS_UPDATE_ANY = "users:update_any" + USERS_DELETE = "users:delete" + METRICS_VIEW = "metrics:view" + PROFILE_UPDATE_SELF = "profile:update_self" + SETTINGS_GLOBAL = "settings:global" + ITEMS_LIST_ANY = "items:list_any" + ITEMS_MANAGE_ANY = "items:manage_any" + + +ROLE_PERMISSIONS: dict[UserRole, frozenset[Permission]] = { + UserRole.ADMIN: frozenset(Permission), + UserRole.MANAGER: frozenset( + { + Permission.USERS_LIST, + Permission.METRICS_VIEW, + Permission.PROFILE_UPDATE_SELF, + } + ), + UserRole.MEMBER: frozenset({Permission.PROFILE_UPDATE_SELF}), +} + + +def user_has_permission(user: User, permission: Permission) -> bool: + return permission in ROLE_PERMISSIONS.get(user.role, frozenset()) + + +def user_is_admin(user: User) -> bool: + return user.role == UserRole.ADMIN + + +def user_can_manage_item(user: User, owner_id: uuid.UUID) -> bool: + if user_has_permission(user, Permission.ITEMS_MANAGE_ANY): + return True + return owner_id == user.id diff --git a/backend/app/crud.py b/backend/app/crud.py index a8ceba6444..99ac7ed496 100644 --- a/backend/app/crud.py +++ b/backend/app/crud.py @@ -1,15 +1,27 @@ +# User and item CRUD helpers; role is the sole writable authorization field. import uuid from typing import Any from sqlmodel import Session, select from app.core.security import get_password_hash, verify_password -from app.models import Item, ItemCreate, User, UserCreate, UserUpdate +from app.models import Item, ItemCreate, User, UserCreate, UserRole, UserUpdate + + +def _default_role_for_create(user_data: dict) -> None: + if user_data.get("role") is None: + user_data["role"] = UserRole.MEMBER def create_user(*, session: Session, user_create: UserCreate) -> User: - db_obj = User.model_validate( - user_create, update={"hashed_password": get_password_hash(user_create.password)} + user_data = user_create.model_dump() + _default_role_for_create(user_data) + db_obj = User( + email=user_create.email, + is_active=user_create.is_active, + full_name=user_create.full_name, + role=user_data["role"], + hashed_password=get_password_hash(user_create.password), ) session.add(db_obj) session.commit() diff --git a/backend/app/models.py b/backend/app/models.py index dcedf9a2f5..591d4edd71 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -1,8 +1,10 @@ import uuid from datetime import UTC, datetime +from enum import StrEnum -from pydantic import EmailStr -from sqlalchemy import DateTime +from pydantic import ConfigDict, EmailStr, computed_field +from sqlalchemy import Boolean, Column, Computed, DateTime +from sqlalchemy import Enum as SqlEnum from sqlmodel import Field, Relationship, SQLModel @@ -10,16 +12,24 @@ def get_datetime_utc() -> datetime: return datetime.now(UTC) -# Shared properties -class UserBase(SQLModel): +class UserRole(StrEnum): + ADMIN = "admin" + MANAGER = "manager" + MEMBER = "member" + + +# Shared user fields for API input/output (role is the source of truth). +class UserFields(SQLModel): email: EmailStr = Field(unique=True, index=True, max_length=255) is_active: bool = True - is_superuser: bool = False + role: UserRole = Field(default=UserRole.MEMBER) full_name: str | None = Field(default=None, max_length=255) # Properties to receive via API on creation -class UserCreate(UserBase): +class UserCreate(UserFields): + model_config = ConfigDict(extra="forbid") + password: str = Field(min_length=8, max_length=128) @@ -31,9 +41,11 @@ class UserRegister(SQLModel): # Properties to receive via API on update, all are optional class UserUpdate(SQLModel): + model_config = ConfigDict(extra="forbid") + email: EmailStr | None = Field(default=None, max_length=255) is_active: bool | None = None - is_superuser: bool | None = None + role: UserRole | None = None full_name: str | None = Field(default=None, max_length=255) password: str | None = Field(default=None, min_length=8, max_length=128) @@ -49,9 +61,28 @@ class UpdatePassword(SQLModel): # Database model, database table inferred from class name -class User(UserBase, table=True): +class User(UserFields, table=True): id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) hashed_password: str + role: UserRole = Field( + default=UserRole.MEMBER, + sa_column=Column( + SqlEnum( + UserRole, + name="userrole", + values_callable=lambda roles: [role.value for role in roles], + ), + nullable=False, + ), + ) + is_superuser: bool = Field( + default=None, # type: ignore[assignment] + sa_column=Column( + Boolean, + Computed("(role = 'admin')", persisted=True), + nullable=False, + ), + ) created_at: datetime | None = Field( default_factory=get_datetime_utc, sa_type=DateTime(timezone=True), # type: ignore @@ -60,10 +91,15 @@ class User(UserBase, table=True): # Properties to return via API, id is always required -class UserPublic(UserBase): +class UserPublic(UserFields): id: uuid.UUID created_at: datetime | None = None + @computed_field # type: ignore[prop-decorator] + @property + def is_superuser(self) -> bool: + return self.role == UserRole.ADMIN + class UsersPublic(SQLModel): data: list[UserPublic] diff --git a/backend/tests/api/routes/test_authorization.py b/backend/tests/api/routes/test_authorization.py new file mode 100644 index 0000000000..017bb65765 --- /dev/null +++ b/backend/tests/api/routes/test_authorization.py @@ -0,0 +1,198 @@ +# Backend authorization tests for admin, manager, and member roles. +import logging + +import pytest +from fastapi.testclient import TestClient +from sqlmodel import Session + +from app import crud +from app.core.config import settings +from app.models import UserCreate, UserUpdate +from tests.utils.item import create_random_item +from tests.utils.user import ( + user_authentication_headers, +) +from tests.utils.utils import random_email, random_lower_string + + +def test_manager_can_list_users( + client: TestClient, manager_token_headers: dict[str, str] +) -> None: + response = client.get( + f"{settings.API_V1_STR}/users/", headers=manager_token_headers + ) + assert response.status_code == 200 + assert "data" in response.json() + + +def test_manager_cannot_create_user( + client: TestClient, manager_token_headers: dict[str, str] +) -> None: + response = client.post( + f"{settings.API_V1_STR}/users/", + headers=manager_token_headers, + json={ + "email": "new-manager-blocked@example.com", + "password": "securepass1", + }, + ) + assert response.status_code == 403 + + +def test_member_cannot_list_users( + client: TestClient, member_token_headers: dict[str, str] +) -> None: + response = client.get(f"{settings.API_V1_STR}/users/", headers=member_token_headers) + assert response.status_code == 403 + + +def test_permission_denial_is_logged( + client: TestClient, + member_token_headers: dict[str, str], + caplog: pytest.LogCaptureFixture, +) -> None: + with caplog.at_level(logging.WARNING, logger="app.api.deps"): + response = client.get( + f"{settings.API_V1_STR}/users/", headers=member_token_headers + ) + + assert response.status_code == 403 + assert "Access denied" in caplog.text + assert "users:list" in caplog.text + + +def test_member_can_update_own_profile( + client: TestClient, member_token_headers: dict[str, str] +) -> None: + response = client.patch( + f"{settings.API_V1_STR}/users/me", + headers=member_token_headers, + json={"full_name": "Member Updated"}, + ) + assert response.status_code == 200 + assert response.json()["full_name"] == "Member Updated" + + +def test_admin_can_create_user( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + response = client.post( + f"{settings.API_V1_STR}/users/", + headers=superuser_token_headers, + json={ + "email": "rbac-admin-created@example.com", + "password": "securepass1", + "role": "member", + }, + ) + assert response.status_code == 200 + assert response.json()["email"] == "rbac-admin-created@example.com" + + +def test_metrics_admin_and_manager_allowed_member_denied( + client: TestClient, + superuser_token_headers: dict[str, str], + manager_token_headers: dict[str, str], + member_token_headers: dict[str, str], +) -> None: + admin_response = client.get( + f"{settings.API_V1_STR}/metrics/", headers=superuser_token_headers + ) + manager_response = client.get( + f"{settings.API_V1_STR}/metrics/", headers=manager_token_headers + ) + member_response = client.get( + f"{settings.API_V1_STR}/metrics/", headers=member_token_headers + ) + + assert admin_response.status_code == 200 + assert manager_response.status_code == 200 + assert member_response.status_code == 403 + + +def test_manager_cannot_access_other_users_item( + client: TestClient, manager_token_headers: dict[str, str], db: Session +) -> None: + item = create_random_item(db) + response = client.get( + f"{settings.API_V1_STR}/items/{item.id}", + headers=manager_token_headers, + ) + assert response.status_code == 403 + assert response.json()["detail"] == "Not enough permissions" + + +def test_invalid_access_token(client: TestClient) -> None: + headers = {"Authorization": "Bearer invalid-token"} + response = client.get(f"{settings.API_V1_STR}/users/me", headers=headers) + assert response.status_code == 403 + assert response.json()["detail"] == "Could not validate credentials" + + +def test_inactive_user_cannot_access(client: TestClient, db: Session) -> None: + email = random_email() + password = random_lower_string() + user_in = UserCreate(email=email, password=password, is_active=True) + user = crud.create_user(session=db, user_create=user_in) + headers = user_authentication_headers(client=client, email=email, password=password) + + user_in_update = UserUpdate(is_active=False) + crud.update_user(session=db, db_user=user, user_in=user_in_update) + + response = client.get(f"{settings.API_V1_STR}/users/me", headers=headers) + assert response.status_code == 400 + assert response.json()["detail"] == "Inactive user" + + +def test_token_for_deleted_user_returns_not_found( + client: TestClient, db: Session +) -> None: + email = random_email() + password = random_lower_string() + user_in = UserCreate(email=email, password=password) + user = crud.create_user(session=db, user_create=user_in) + headers = user_authentication_headers(client=client, email=email, password=password) + db.delete(user) + db.commit() + + response = client.get(f"{settings.API_V1_STR}/users/me", headers=headers) + assert response.status_code == 404 + assert response.json()["detail"] == "User not found" + + +def test_non_admin_cannot_access_superuser_utils_endpoint( + client: TestClient, + member_token_headers: dict[str, str], + caplog: pytest.LogCaptureFixture, +) -> None: + with caplog.at_level(logging.WARNING, logger="app.api.deps"): + response = client.post( + f"{settings.API_V1_STR}/utils/test-email/?email_to=member@example.com", + headers=member_token_headers, + ) + + assert response.status_code == 403 + assert response.json()["detail"] == "The user doesn't have enough privileges" + assert "Access denied" in caplog.text + assert "admin" in caplog.text.lower() + + +def test_member_cannot_update_other_users( + client: TestClient, + member_token_headers: dict[str, str], +) -> None: + admin_headers = user_authentication_headers( + client=client, + email=settings.FIRST_SUPERUSER, + password=settings.FIRST_SUPERUSER_PASSWORD, + ) + admin_me = client.get( + f"{settings.API_V1_STR}/users/me", headers=admin_headers + ).json() + + response = client.patch( + f"{settings.API_V1_STR}/users/{admin_me['id']}", + headers=member_token_headers, + json={"full_name": "Escalation Attempt"}, + ) + assert response.status_code == 403 diff --git a/backend/tests/api/routes/test_login.py b/backend/tests/api/routes/test_login.py index 96677a25f6..79d3587b4e 100644 --- a/backend/tests/api/routes/test_login.py +++ b/backend/tests/api/routes/test_login.py @@ -89,7 +89,6 @@ def test_reset_password(client: TestClient, db: Session) -> None: full_name="Test User", password=password, is_active=True, - is_superuser=False, ) user = create_user(session=db, user_create=user_create) token = generate_password_reset_token(email=email) diff --git a/backend/tests/api/routes/test_users.py b/backend/tests/api/routes/test_users.py index 9c4cdd5991..13b175fc71 100644 --- a/backend/tests/api/routes/test_users.py +++ b/backend/tests/api/routes/test_users.py @@ -7,7 +7,7 @@ from app import crud from app.core.config import settings from app.core.security import verify_password -from app.models import User, UserCreate +from app.models import User, UserCreate, UserRole from tests.utils.user import create_random_user from tests.utils.utils import random_email, random_lower_string @@ -126,7 +126,7 @@ def test_get_existing_user_permissions_error( headers=normal_user_token_headers, ) assert r.status_code == 403 - assert r.json() == {"detail": "The user doesn't have enough privileges"} + assert r.json() == {"detail": "You do not have permission to perform this action"} def test_get_non_existing_user_permissions_error( @@ -140,7 +140,7 @@ def test_get_non_existing_user_permissions_error( headers=normal_user_token_headers, ) assert r.status_code == 403 - assert r.json() == {"detail": "The user doesn't have enough privileges"} + assert r.json() == {"detail": "You do not have permission to perform this action"} def test_create_user_existing_username( @@ -355,6 +355,21 @@ def test_register_user_already_exists_error(client: TestClient) -> None: assert r.json()["detail"] == "The user with this email already exists in the system" +def test_create_user_rejects_is_superuser_in_request_body( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + response = client.post( + f"{settings.API_V1_STR}/users/", + headers=superuser_token_headers, + json={ + "email": "legacy-superuser-flag@example.com", + "password": "securepass1", + "is_superuser": True, + }, + ) + assert response.status_code == 422 + + def test_update_user( client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: @@ -379,6 +394,74 @@ def test_update_user( db.refresh(user_db) assert user_db assert user_db.full_name == "Updated_full_name" + assert user_db.role == UserRole.MEMBER + + +def test_update_user_preserves_manager_role_on_partial_patch( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + username = random_email() + password = random_lower_string() + user_in = UserCreate(email=username, password=password, role=UserRole.MANAGER) + user = crud.create_user(session=db, user_create=user_in) + assert user.role == UserRole.MANAGER + + r = client.patch( + f"{settings.API_V1_STR}/users/{user.id}", + headers=superuser_token_headers, + json={"full_name": "Manager Renamed"}, + ) + assert r.status_code == 200 + assert r.json()["full_name"] == "Manager Renamed" + assert r.json()["role"] == UserRole.MANAGER.value + + db.refresh(user) + assert user.role == UserRole.MANAGER + assert user.is_superuser is False + + +def test_update_user_password_preserves_role( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + username = random_email() + password = random_lower_string() + user_in = UserCreate(email=username, password=password, role=UserRole.MANAGER) + user = crud.create_user(session=db, user_create=user_in) + + new_password = random_lower_string() + r = client.patch( + f"{settings.API_V1_STR}/users/{user.id}", + headers=superuser_token_headers, + json={"password": new_password}, + ) + assert r.status_code == 200 + + db.refresh(user) + assert user.role == UserRole.MANAGER + verified, _ = verify_password(new_password, user.hashed_password) + assert verified + + +def test_update_user_role_syncs_is_superuser( + client: TestClient, superuser_token_headers: dict[str, str], db: Session +) -> None: + username = random_email() + password = random_lower_string() + user_in = UserCreate(email=username, password=password, role=UserRole.MEMBER) + user = crud.create_user(session=db, user_create=user_in) + + r = client.patch( + f"{settings.API_V1_STR}/users/{user.id}", + headers=superuser_token_headers, + json={"role": UserRole.ADMIN.value}, + ) + assert r.status_code == 200 + assert r.json()["role"] == UserRole.ADMIN.value + assert r.json()["is_superuser"] is True + + db.refresh(user) + assert user.role == UserRole.ADMIN + assert user.is_superuser is True def test_update_user_not_exists( @@ -518,4 +601,4 @@ def test_delete_user_without_privileges( headers=normal_user_token_headers, ) assert r.status_code == 403 - assert r.json()["detail"] == "The user doesn't have enough privileges" + assert r.json()["detail"] == "You do not have permission to perform this action" diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 7cdabf3c45..d4163d184d 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -7,8 +7,11 @@ from app.core.config import settings from app.core.db import engine, init_db from app.main import app -from app.models import Item, User -from tests.utils.user import authentication_token_from_email +from app.models import Item, User, UserRole +from tests.utils.user import ( + authentication_token_for_role, + authentication_token_from_email, +) from tests.utils.utils import get_superuser_token_headers @@ -40,3 +43,25 @@ def normal_user_token_headers(client: TestClient, db: Session) -> dict[str, str] return authentication_token_from_email( client=client, email=settings.EMAIL_TEST_USER, db=db ) + + +@pytest.fixture(scope="module") +def manager_token_headers(client: TestClient, db: Session) -> dict[str, str]: + return authentication_token_for_role( + client=client, + db=db, + email=settings.MANAGER_USER, + password=settings.MANAGER_USER_PASSWORD, + role=UserRole.MANAGER, + ) + + +@pytest.fixture(scope="module") +def member_token_headers(client: TestClient, db: Session) -> dict[str, str]: + return authentication_token_for_role( + client=client, + db=db, + email=settings.MEMBER_USER, + password=settings.MEMBER_USER_PASSWORD, + role=UserRole.MEMBER, + ) diff --git a/backend/tests/crud/test_user.py b/backend/tests/crud/test_user.py index 3db77ef624..352393be9b 100644 --- a/backend/tests/crud/test_user.py +++ b/backend/tests/crud/test_user.py @@ -1,10 +1,14 @@ +import pytest from fastapi.encoders import jsonable_encoder from pwdlib.hashers.bcrypt import BcryptHasher +from pydantic import ValidationError +from sqlalchemy import text +from sqlalchemy.exc import DBAPIError from sqlmodel import Session from app import crud from app.core.security import verify_password -from app.models import User, UserCreate, UserUpdate +from app.models import User, UserCreate, UserRole, UserUpdate from tests.utils.utils import random_email, random_lower_string @@ -53,7 +57,7 @@ def test_check_if_user_is_active_inactive(db: Session) -> None: def test_check_if_user_is_superuser(db: Session) -> None: email = random_email() password = random_lower_string() - user_in = UserCreate(email=email, password=password, is_superuser=True) + user_in = UserCreate(email=email, password=password, role=UserRole.ADMIN) user = crud.create_user(session=db, user_create=user_in) assert user.is_superuser is True @@ -69,7 +73,7 @@ def test_check_if_user_is_superuser_normal_user(db: Session) -> None: def test_get_user(db: Session) -> None: password = random_lower_string() username = random_email() - user_in = UserCreate(email=username, password=password, is_superuser=True) + user_in = UserCreate(email=username, password=password, role=UserRole.ADMIN) user = crud.create_user(session=db, user_create=user_in) user_2 = db.get(User, user.id) assert user_2 @@ -80,10 +84,10 @@ def test_get_user(db: Session) -> None: def test_update_user(db: Session) -> None: password = random_lower_string() email = random_email() - user_in = UserCreate(email=email, password=password, is_superuser=True) + user_in = UserCreate(email=email, password=password, role=UserRole.ADMIN) user = crud.create_user(session=db, user_create=user_in) new_password = random_lower_string() - user_in_update = UserUpdate(password=new_password, is_superuser=True) + user_in_update = UserUpdate(password=new_password, role=UserRole.ADMIN) if user.id is not None: crud.update_user(session=db, db_user=user, user_in=user_in_update) user_2 = db.get(User, user.id) @@ -93,6 +97,68 @@ def test_update_user(db: Session) -> None: assert verified +def test_update_user_partial_preserves_role(db: Session) -> None: + email = random_email() + password = random_lower_string() + user_in = UserCreate(email=email, password=password, role=UserRole.MANAGER) + user = crud.create_user(session=db, user_create=user_in) + + crud.update_user( + session=db, + db_user=user, + user_in=UserUpdate(full_name="Still Manager"), + ) + + user_2 = db.get(User, user.id) + assert user_2 + assert user_2.full_name == "Still Manager" + assert user_2.role == UserRole.MANAGER + assert user_2.is_superuser is False + + +def test_user_create_rejects_is_superuser_field() -> None: + with pytest.raises(ValidationError): + UserCreate( + email="legacy@example.com", + password="securepass1", + is_superuser=True, # type: ignore[call-arg] + ) + + +def test_is_superuser_follows_role_changes(db: Session) -> None: + email = random_email() + password = random_lower_string() + user = crud.create_user( + session=db, + user_create=UserCreate(email=email, password=password, role=UserRole.MANAGER), + ) + assert user.is_superuser is False + + user = crud.update_user( + session=db, + db_user=user, + user_in=UserUpdate(role=UserRole.ADMIN), + ) + assert user.role == UserRole.ADMIN + assert user.is_superuser is True + + +def test_is_superuser_cannot_be_written_directly(db: Session) -> None: + email = random_email() + password = random_lower_string() + user = crud.create_user( + session=db, + user_create=UserCreate(email=email, password=password, role=UserRole.MEMBER), + ) + with pytest.raises(DBAPIError): + db.execute( + text('UPDATE "user" SET is_superuser = true WHERE id = :id'), + {"id": user.id}, + ) + db.commit() + db.rollback() + + def test_authenticate_user_with_bcrypt_upgrades_to_argon2(db: Session) -> None: """Test that a user with bcrypt password hash gets upgraded to argon2 on login.""" email = random_email() diff --git a/backend/tests/utils/user.py b/backend/tests/utils/user.py index 5867431ed8..ba89d3a450 100644 --- a/backend/tests/utils/user.py +++ b/backend/tests/utils/user.py @@ -3,7 +3,7 @@ from app import crud from app.core.config import settings -from app.models import User, UserCreate, UserUpdate +from app.models import User, UserCreate, UserRole, UserUpdate from tests.utils.utils import random_email, random_lower_string @@ -22,11 +22,29 @@ def user_authentication_headers( def create_random_user(db: Session) -> User: email = random_email() password = random_lower_string() - user_in = UserCreate(email=email, password=password) + user_in = UserCreate(email=email, password=password, role=UserRole.MEMBER) user = crud.create_user(session=db, user_create=user_in) return user +def authentication_token_for_role( + *, client: TestClient, db: Session, email: str, password: str, role: UserRole +) -> dict[str, str]: + user = crud.get_user_by_email(session=db, email=email) + if not user: + user_in_create = UserCreate( + email=email, + password=password, + role=role, + ) + crud.create_user(session=db, user_create=user_in_create) + else: + user_in_update = UserUpdate(password=password, role=role) + crud.update_user(session=db, db_user=user, user_in=user_in_update) + + return user_authentication_headers(client=client, email=email, password=password) + + def authentication_token_from_email( *, client: TestClient, email: str, db: Session ) -> dict[str, str]: diff --git a/compose.override.yml b/compose.override.example.yml similarity index 84% rename from compose.override.yml rename to compose.override.example.yml index ec0142b1a0..901f494c71 100644 --- a/compose.override.yml +++ b/compose.override.example.yml @@ -1,3 +1,9 @@ +# Local development overrides for Docker Compose. +# Copy to compose.override.yml (gitignored) before running the stack: +# cp compose.override.example.yml compose.override.yml +# +# Docker Compose merges compose.yml + compose.override.yml automatically. + services: proxy: @@ -43,22 +49,6 @@ services: - dev - --host - "0.0.0.0" - develop: - watch: - - path: ./backend - action: sync - target: /app/backend - ignore: - - .venv - - path: ./backend/pyproject.toml - action: rebuild - - path: ./frontend - action: rebuild - ignore: - - ./frontend/node_modules - - ./frontend/dist - - ./frontend/blob-report - - ./frontend/test-results # TODO: remove once coverage is done locally volumes: - ./backend/htmlcov:/app/backend/htmlcov diff --git a/compose.yml b/compose.yml index dd549712e7..58857f69dc 100644 --- a/compose.yml +++ b/compose.yml @@ -51,7 +51,6 @@ services: depends_on: db: condition: service_healthy - restart: true environment: PROJECT_NAME: ${PROJECT_NAME:?Variable not set} SECRET_KEY: ${SECRET_KEY:?Variable not set} diff --git a/development.md b/development.md index efd21b2f3e..f81d36fc9d 100644 --- a/development.md +++ b/development.md @@ -85,7 +85,7 @@ Stop a locally running FastAPI server before starting the Compose backend becaus The main `compose.yml` file contains the configuration shared by the whole stack. Docker Compose loads it automatically. -The `compose.override.yml` file adds local development settings, such as mounting the source code as a volume. Docker Compose also loads it automatically and applies it on top of `compose.yml`. +The `compose.override.example.yml` file documents local development settings (published ports, backend reload, Mailpit, Playwright). Copy it to `compose.override.yml` (gitignored); Docker Compose applies the override automatically when you run `docker compose` without an explicit file list. The `compose.deploy.yml` file contains the deployment-specific settings, including HTTPS and automatic certificate handling. It is explicitly combined with `compose.yml` when deploying the application. diff --git a/docs/adr/001-permission-based-rbac.md b/docs/adr/001-permission-based-rbac.md new file mode 100644 index 0000000000..5aa4be40a2 --- /dev/null +++ b/docs/adr/001-permission-based-rbac.md @@ -0,0 +1,38 @@ +# ADR 001: Permission-Based RBAC Instead of Inline Role Checks + +## Status + +Accepted + +## Context + +The assignment requires three roles (`admin`, `manager`, `member`) and a small but realistic authorization surface (users, metrics, profile, settings). Routes could check roles directly (`if user.role == UserRole.ADMIN`) or go through a shared permission layer. + +We need a model that is easy to read in code review, easy to extend when a fourth role appears, and consistent between backend routes. + +## Options Considered + +1. **Inline role checks in each route** — Simple for three roles, but scatters policy across handlers and makes the permission matrix implicit. +2. **Permission enum + role-to-permission map** — One module defines `Permission` and `ROLE_PERMISSIONS`; routes depend on `require_permission(...)`. +3. **External policy engine (e.g. Casbin)** — Flexible for large systems, but heavy for a timeboxed task and adds a dependency. + +## Decision + +Use **option 2**: a central `permissions.py` with a `Permission` enum and `ROLE_PERMISSIONS` mapping. FastAPI dependencies in `deps.py` expose `require_permission(...)` used by route handlers. + +## Consequences + +**Pros** + +- Adding a permission or adjusting a role touches one map and route dependencies, not every `if role == ...` branch. +- The matrix in README maps directly to `Permission` values and tests. +- Reviewers can understand policy in one file within a few minutes. + +**Cons** + +- Frontend duplicates the matrix in `permissions.ts` (see ADR 002). +- Fine-grained rules beyond the static matrix (e.g. row-level access) would need extra helpers; not required for this task. + +## Trade-offs + +We deliberately avoided a policy engine to keep scope tight. For production at scale, we would evaluate syncing permissions from the API or adopting a dedicated authorization service. diff --git a/docs/adr/002-frontend-permission-mirror.md b/docs/adr/002-frontend-permission-mirror.md new file mode 100644 index 0000000000..f6a1ecb65a --- /dev/null +++ b/docs/adr/002-frontend-permission-mirror.md @@ -0,0 +1,40 @@ +# ADR 002: Mirror Permissions on the Frontend for UX Only + +## Status + +Accepted + +## Context + +The backend must enforce RBAC on every sensitive endpoint. The frontend still needs to hide sidebar links, block direct URL navigation, and show a friendly "Access Denied" state instead of empty screens or cryptic API errors. + +Options: fetch capabilities from the API, derive UI state from JWT claims only, or duplicate the permission matrix client-side. + +## Options Considered + +1. **Dedicated `/me/permissions` endpoint** — Single source of truth at runtime; extra API surface and caching considerations. +2. **JWT custom claims with permission list** — Avoids an extra round-trip but couples token size and refresh to policy changes. +3. **Mirror the backend matrix in TypeScript** — Same `Permission` strings and role map as `permissions.py`; UI checks are synchronous. + +## Decision + +Use **option 3**: `frontend/src/lib/permissions.ts` mirrors `backend/app/core/permissions.py`. The `usePermissions()` hook reads the logged-in user's `role` from existing auth state and gates navigation and route components. + +The backend remains authoritative. A user who bypasses the UI still receives HTTP `403` from the API. + +## Consequences + +**Pros** + +- No new endpoints or token format changes. +- Sidebar and route guards work offline from cached user profile data already loaded after login. +- `AccessDenied` gives clear UX for direct navigation to forbidden routes. + +**Cons** + +- Two places must stay in sync when permissions change (documented in README; tests cover backend; smoke script covers API). +- Role changes after login are not reflected until the user re-authenticates or refetches profile. + +## Trade-offs + +For this task, duplication is acceptable because the matrix is small and stable. With more roles or dynamic permissions, we would add a capabilities endpoint or generate shared types from OpenAPI/schema. diff --git a/docs/ai-conversations/2026-08-18-cursor-rbac-full-export.md b/docs/ai-conversations/2026-08-18-cursor-rbac-full-export.md new file mode 100644 index 0000000000..d2753a6c3c --- /dev/null +++ b/docs/ai-conversations/2026-08-18-cursor-rbac-full-export.md @@ -0,0 +1,211 @@ +# Cursor Conversation Export — RBAC Fullstack Dev Test Task (Task 1) + +Exported: 2026-08-18 +Tool: Cursor (Composer) +Topic: Role-based access control on Full Stack FastAPI Template + +English copy of the AI-assisted design and implementation session, as required for submission. + +--- + +## User + +> Implement the [Fullstack Dev Test Task](https://github.com/evios/Fullstack-Dev-Test-Task): RBAC on [full-stack-fastapi-template](https://github.com/fastapi/full-stack-fastapi-template). Decompose into steps, verify with tests. + +--- + +## Assistant + +**Interpretation** + +Add three roles with a documented permission matrix, enforce consistently on API and UI, provide Docker-based setup, focused backend tests, and README documentation. + +**Permission matrix (implemented)** + +| Action | admin | manager | member | +|--------|:-----:|:-------:|:------:| +| List all users | yes | yes | no | +| Create user | yes | no | no | +| View metrics | yes | yes | no | +| Update own profile | yes | yes | yes | +| Update any profile | yes | no | no | +| Global settings | yes | no | no | + +**Architecture** + +- Backend: `User.role` column + `backend/app/core/permissions.py` + `require_permission()` in `deps.py` +- Frontend: mirror matrix in `frontend/src/lib/permissions.ts` + `usePermissions()` hook +- Backend is source of truth; UI hides forbidden navigation and shows `AccessDenied` + +--- + +## User + +> After environment setup — stop for my review (checkpoint after Wave 1). + +--- + +## Assistant + +Completed Wave 1 (Docker bootstrap, prestart, migrations). Paused for user review before continuing with RBAC implementation. + +--- + +## User + +> Implement the full plan. Complete all todos. + +--- + +## Assistant (implementation waves) + +**Wave 2 — Backend core** + +- `UserRole` enum, Alembic migration, `permissions.py` +- Seed users: admin, manager, member +- `MANAGER_USER*` / `MEMBER_USER*` in config + +**Wave 3 — Routes and UI** + +- RBAC on user routes; `GET /api/v1/metrics/` stub +- Frontend sidebar, `/metrics`, settings restrictions, admin role column + +**Wave 4 — Tests** + +- `test_authorization.py` — allow and deny paths for all three roles + +**Wave 5 — Docs** + +- README: matrix, setup, tests, smoke script (`scripts/smoke_rbac.py`) + +**Verification:** 34 backend tests passed; smoke script documented. + +--- + +## User + +> Add `.idea/` to `.gitignore`. + +--- + +## Assistant + +Added `.idea/` to `.gitignore`. + +--- + +## User + +> Clean git history: `Init commit` first, then small incremental commits. Rebuild history without `Co-authored-by: Cursor` trailers. + +--- + +## Assistant + +Rebuilt history with incremental commits: + +``` +Init commit +fix: adapt docker compose for local Docker Compose v2.16 +chore: add manager and member seed credentials to env +feat: add RBAC roles, permissions module, migration, and seed users +feat: enforce RBAC on user routes and add metrics endpoint +test: add authorization tests and role fixtures +feat: add frontend permission helpers and access denied UI +feat: wire RBAC into sidebar, routes, and admin UI +docs: document RBAC setup, permissions, and smoke test script +``` + +Removed auto-injected `Co-authored-by` trailers from commit messages. + +--- + +## User + +> Delete unnecessary files and directories. + +--- + +## Assistant + +Removed backup directory, IDE agent skill symlinks (`.agents/`, `.claude/`), and one-off history replay script. Added `.agents/`, `.claude/` to `.gitignore`. + +--- + +## User + +> Why are `compose.override.yml` and `compose.deploy.yml` in git? Local override should be `compose.override.example.yml`; real `compose.override.yml` should be gitignored. + +--- + +## Assistant + +- Added `compose.override.example.yml` (template) +- Gitignored `compose.override.yml` +- Updated README and development docs with `cp compose.override.example.yml compose.override.yml` + +`compose.deploy.yml` remains in git — it is the production deploy overlay (used with `compose.yml` explicitly), not a local-only file. + +--- + +## User + +> README must not mention VM-specific paths. Reviewers run on Mac without a virtual machine. + +--- + +## Assistant + +Updated Quick Start for generic Docker Desktop workflow (`git clone`, `cd`, compose). Removed machine-specific paths and non-standard port notes from README. + +--- + +## User + +> Are all required task items complete? + +--- + +## Assistant + +Confirmed all mandatory TASK.md items complete. Optional bonus items (ADR, diagram, NOTES, logging) not yet done at that point. + +--- + +## User + +> Create a plan and implement the bonus criteria. + +--- + +## Assistant (bonus / nice-to-have) + +1. **Logging** — `WARNING` logs on permission denial in `app.api.deps` +2. **Test** — `test_permission_denial_is_logged` +3. **ADRs** — `docs/adr/001-permission-based-rbac.md`, `002-frontend-permission-mirror.md` +4. **Mermaid diagram** — authorization flow in README +5. **NOTES.md** — scope cuts, trade-offs, follow-ups + +--- + +## Key files + +| Path | Purpose | +|------|---------| +| `backend/app/core/permissions.py` | Permission enum and role map | +| `backend/app/api/deps.py` | `require_permission()`, denial logging | +| `backend/app/api/routes/users.py` | RBAC on user endpoints | +| `backend/app/api/routes/metrics.py` | Metrics stub | +| `frontend/src/lib/permissions.ts` | Frontend permission mirror | +| `frontend/src/hooks/usePermissions.ts` | UI capability hook | +| `frontend/src/components/Common/AccessDenied.tsx` | Forbidden state UX | +| `backend/tests/api/routes/test_authorization.py` | Authorization tests | +| `compose.override.example.yml` | Local dev compose template | + +--- + +## Notes for reviewers + +- Conversation reconstructed from the Cursor session; tool-call details omitted. +- All submission copies are in English. +- Task 2 (Ghost on Hetzner) is implemented in the separate `ghost-hetzner` repository. diff --git a/docs/ai-conversations/2026-08-18-cursor-session-summary.md b/docs/ai-conversations/2026-08-18-cursor-session-summary.md new file mode 100644 index 0000000000..2ae4bc161d --- /dev/null +++ b/docs/ai-conversations/2026-08-18-cursor-session-summary.md @@ -0,0 +1,40 @@ +# Cursor Session Summary — RBAC Fullstack Dev Test Task + +Date: 2026-08-18 +Tool: Cursor (Composer) +Language: English (submission copy) + +## User Request + +Implement [Fullstack Dev Test Task](https://github.com/evios/Fullstack-Dev-Test-Task): add RBAC to the [Full Stack FastAPI Template](https://github.com/fastapi/full-stack-fastapi-template). + +Roles: `admin`, `manager`, `member`. Enforce on backend and frontend. Include tests, README, and runnable Docker setup. + +## Plan (waves) + +1. Bootstrap Docker environment and migrations +2. Backend: `UserRole`, permissions module, seed users +3. Frontend: permission helpers, `AccessDenied` component +4. Protect API routes and add metrics stub +5. Sidebar, route guards, admin UI +6. Authorization tests +7. Documentation and verification loop + +## Outcomes + +| Area | Result | +|------|--------| +| Backend | `permissions.py`, `require_permission()`, users + metrics routes | +| Frontend | `permissions.ts`, `usePermissions()`, sidebar and route guards | +| Tests | 8 authorization tests (`test_authorization.py`) | +| Docs | Permission matrix, Mermaid diagram, 2 ADRs, `NOTES.md` | +| Git | Incremental commits; clean history without auto-generated trailers | +| Dev UX | `compose.override.example.yml`; Mac-friendly README | + +## Seed users + +- `admin@example.com`, `manager@example.com`, `member@example.com` (password: `changethis`) + +## Full export + +See [2026-08-18-cursor-rbac-full-export.md](2026-08-18-cursor-rbac-full-export.md). diff --git a/docs/ai-conversations/README.md b/docs/ai-conversations/README.md new file mode 100644 index 0000000000..a3f547425c --- /dev/null +++ b/docs/ai-conversations/README.md @@ -0,0 +1,24 @@ +# AI Conversation Exports + +This directory contains **English copies** of AI-assisted development sessions for the [Fullstack Dev Test Task](https://github.com/evios/Fullstack-Dev-Test-Task) (Task 1 — RBAC). + +A separate repository (`ghost-hetzner`) holds the infrastructure task (Task 2) with its own `docs/ai-conversations/` folder. + +## Submission requirement + +Both repositories must include copies of conversations with AI tools used during implementation. All exports must be in **English** and must not reference private employer infrastructure or internal tooling names. + +## Included files (Task 1 — this repo) + +| File | Description | +|------|-------------| +| [2026-08-18-cursor-session-summary.md](2026-08-18-cursor-session-summary.md) | Short summary of the RBAC implementation session | +| [2026-08-18-cursor-rbac-full-export.md](2026-08-18-cursor-rbac-full-export.md) | Full English conversation copy (task, plan, implementation, polish) | + +## Optional: raw Cursor export + +You may also add a raw **Export chat** file from Cursor for the same session. Keep it in English and redact secrets before committing. + +## Related repository + +Task 2 (Ghost on Hetzner): see `ghost-hetzner/docs/ai-conversations/`. diff --git a/frontend/README.md b/frontend/README.md index 5055e35c8f..02d37ae573 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -33,7 +33,7 @@ If you are developing an API-only app and want to remove the frontend, you can d * In the `backend/Dockerfile` file, remove the frontend build stage and the `COPY --from=frontend-build` instruction. -* In the `compose.override.yml` file, remove the `playwright` service. +* In `compose.override.yml` (copy from `compose.override.example.yml`), remove the `playwright` service. * In the `.github/workflows/deploy.yml` file, remove the **Set up Bun**, **Install frontend dependencies**, and **Build frontend** steps. diff --git a/frontend/src/client/index.ts b/frontend/src/client/index.ts index f4fdf30796..3c4cb4a160 100644 --- a/frontend/src/client/index.ts +++ b/frontend/src/client/index.ts @@ -1,4 +1,4 @@ // This file is auto-generated by @hey-api/openapi-ts -export { ItemsService, LoginService, type Options, PrivateService, UsersService, UtilsService } from './sdk.gen'; -export type { Body_login_login_access_token, ClientOptions, HTTPValidationError, ItemCreate, ItemPublic, itemsCreateItemData, itemsCreateItemError, itemsCreateItemErrors, itemsCreateItemResponse, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemError, itemsDeleteItemErrors, itemsDeleteItemResponse, itemsDeleteItemResponses, ItemsPublic, itemsReadItemData, itemsReadItemError, itemsReadItemErrors, itemsReadItemResponse, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsError, itemsReadItemsErrors, itemsReadItemsResponse, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemError, itemsUpdateItemErrors, itemsUpdateItemResponse, itemsUpdateItemResponses, ItemUpdate, loginLoginAccessTokenData, loginLoginAccessTokenError, loginLoginAccessTokenErrors, loginLoginAccessTokenResponse, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordError, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentError, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponse, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponse, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordError, loginResetPasswordErrors, loginResetPasswordResponse, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponse, loginTestTokenResponses, Message, NewPassword, privateCreateUserData, privateCreateUserError, privateCreateUserErrors, privateCreateUserResponse, privateCreateUserResponses, PrivateUserCreate, Token, UpdatePassword, UserCreate, UserPublic, UserRegister, usersCreateUserData, usersCreateUserError, usersCreateUserErrors, usersCreateUserResponse, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserError, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponse, usersDeleteUserMeResponses, usersDeleteUserResponse, usersDeleteUserResponses, UsersPublic, usersReadUserByIdData, usersReadUserByIdError, usersReadUserByIdErrors, usersReadUserByIdResponse, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMeResponse, usersReadUserMeResponses, usersReadUsersData, usersReadUsersError, usersReadUsersErrors, usersReadUsersResponse, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserError, usersRegisterUserErrors, usersRegisterUserResponse, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeError, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponse, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserError, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeError, usersUpdateUserMeErrors, usersUpdateUserMeResponse, usersUpdateUserMeResponses, usersUpdateUserResponse, usersUpdateUserResponses, UserUpdate, UserUpdateMe, utilsHealthCheckData, utilsHealthCheckResponse, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailError, utilsTestEmailErrors, utilsTestEmailResponse, utilsTestEmailResponses, ValidationError } from './types.gen'; +export { ItemsService, LoginService, MetricsService, type Options, PrivateService, UsersService, UtilsService } from './sdk.gen'; +export type { Body_login_login_access_token, ClientOptions, HTTPValidationError, ItemCreate, ItemPublic, itemsCreateItemData, itemsCreateItemError, itemsCreateItemErrors, itemsCreateItemResponse, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemError, itemsDeleteItemErrors, itemsDeleteItemResponse, itemsDeleteItemResponses, ItemsPublic, itemsReadItemData, itemsReadItemError, itemsReadItemErrors, itemsReadItemResponse, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsError, itemsReadItemsErrors, itemsReadItemsResponse, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemError, itemsUpdateItemErrors, itemsUpdateItemResponse, itemsUpdateItemResponses, ItemUpdate, loginLoginAccessTokenData, loginLoginAccessTokenError, loginLoginAccessTokenErrors, loginLoginAccessTokenResponse, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordError, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentError, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponse, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponse, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordError, loginResetPasswordErrors, loginResetPasswordResponse, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponse, loginTestTokenResponses, Message, metricsReadMetricsData, metricsReadMetricsResponse, metricsReadMetricsResponses, NewPassword, privateCreateUserData, privateCreateUserError, privateCreateUserErrors, privateCreateUserResponse, privateCreateUserResponses, PrivateUserCreate, Token, UpdatePassword, UserCreate, UserPublic, UserPublicWritable, UserRegister, UserRole, usersCreateUserData, usersCreateUserError, usersCreateUserErrors, usersCreateUserResponse, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserError, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponse, usersDeleteUserMeResponses, usersDeleteUserResponse, usersDeleteUserResponses, UsersPublic, UsersPublicWritable, usersReadUserByIdData, usersReadUserByIdError, usersReadUserByIdErrors, usersReadUserByIdResponse, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMeResponse, usersReadUserMeResponses, usersReadUsersData, usersReadUsersError, usersReadUsersErrors, usersReadUsersResponse, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserError, usersRegisterUserErrors, usersRegisterUserResponse, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeError, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponse, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserError, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeError, usersUpdateUserMeErrors, usersUpdateUserMeResponse, usersUpdateUserMeResponses, usersUpdateUserResponse, usersUpdateUserResponses, UserUpdate, UserUpdateMe, utilsHealthCheckData, utilsHealthCheckResponse, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailError, utilsTestEmailErrors, utilsTestEmailResponse, utilsTestEmailResponses, ValidationError } from './types.gen'; diff --git a/frontend/src/client/sdk.gen.ts b/frontend/src/client/sdk.gen.ts index b1dd2cf1d5..0775604da9 100644 --- a/frontend/src/client/sdk.gen.ts +++ b/frontend/src/client/sdk.gen.ts @@ -2,7 +2,7 @@ import { type Client, type Options as Options2, type TDataShape, urlSearchParamsBodySerializer } from './client'; import { client } from './client.gen'; -import type { itemsCreateItemData, itemsCreateItemErrors, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemErrors, itemsDeleteItemResponses, itemsReadItemData, itemsReadItemErrors, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsErrors, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemErrors, itemsUpdateItemResponses, loginLoginAccessTokenData, loginLoginAccessTokenErrors, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordErrors, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponses, privateCreateUserData, privateCreateUserErrors, privateCreateUserResponses, usersCreateUserData, usersCreateUserErrors, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponses, usersDeleteUserResponses, usersReadUserByIdData, usersReadUserByIdErrors, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMeResponses, usersReadUsersData, usersReadUsersErrors, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserErrors, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeErrors, usersUpdateUserMeResponses, usersUpdateUserResponses, utilsHealthCheckData, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailErrors, utilsTestEmailResponses } from './types.gen'; +import type { itemsCreateItemData, itemsCreateItemErrors, itemsCreateItemResponses, itemsDeleteItemData, itemsDeleteItemErrors, itemsDeleteItemResponses, itemsReadItemData, itemsReadItemErrors, itemsReadItemResponses, itemsReadItemsData, itemsReadItemsErrors, itemsReadItemsResponses, itemsUpdateItemData, itemsUpdateItemErrors, itemsUpdateItemResponses, loginLoginAccessTokenData, loginLoginAccessTokenErrors, loginLoginAccessTokenResponses, loginRecoverPasswordData, loginRecoverPasswordErrors, loginRecoverPasswordHtmlContentData, loginRecoverPasswordHtmlContentErrors, loginRecoverPasswordHtmlContentResponses, loginRecoverPasswordResponses, loginResetPasswordData, loginResetPasswordErrors, loginResetPasswordResponses, loginTestTokenData, loginTestTokenResponses, metricsReadMetricsData, metricsReadMetricsResponses, privateCreateUserData, privateCreateUserErrors, privateCreateUserResponses, usersCreateUserData, usersCreateUserErrors, usersCreateUserResponses, usersDeleteUserData, usersDeleteUserErrors, usersDeleteUserMeData, usersDeleteUserMeResponses, usersDeleteUserResponses, usersReadUserByIdData, usersReadUserByIdErrors, usersReadUserByIdResponses, usersReadUserMeData, usersReadUserMeResponses, usersReadUsersData, usersReadUsersErrors, usersReadUsersResponses, usersRegisterUserData, usersRegisterUserErrors, usersRegisterUserResponses, usersUpdatePasswordMeData, usersUpdatePasswordMeErrors, usersUpdatePasswordMeResponses, usersUpdateUserData, usersUpdateUserErrors, usersUpdateUserMeData, usersUpdateUserMeErrors, usersUpdateUserMeResponses, usersUpdateUserResponses, utilsHealthCheckData, utilsHealthCheckResponses, utilsTestEmailData, utilsTestEmailErrors, utilsTestEmailResponses } from './types.gen'; export type Options = Options2 & { /** @@ -257,6 +257,20 @@ export class UsersService { } } +export class MetricsService { + /** + * Read Metrics + */ + public static readMetrics(options?: Options) { + return (options?.client ?? client).get({ + responseType: 'json', + security: [{ scheme: 'bearer', type: 'http' }], + url: '/api/v1/metrics/', + ...options + }); + } +} + export class UtilsService { /** * Test Email diff --git a/frontend/src/client/types.gen.ts b/frontend/src/client/types.gen.ts index 6252d88c2f..487bfc9a73 100644 --- a/frontend/src/client/types.gen.ts +++ b/frontend/src/client/types.gen.ts @@ -198,10 +198,7 @@ export type UserCreate = { * Is Active */ is_active?: boolean; - /** - * Is Superuser - */ - is_superuser?: boolean; + role?: UserRole; /** * Full Name */ @@ -224,10 +221,7 @@ export type UserPublic = { * Is Active */ is_active?: boolean; - /** - * Is Superuser - */ - is_superuser?: boolean; + role?: UserRole; /** * Full Name */ @@ -240,6 +234,10 @@ export type UserPublic = { * Created At */ created_at?: string | null; + /** + * Is Superuser + */ + readonly is_superuser: boolean; }; /** @@ -260,6 +258,11 @@ export type UserRegister = { full_name?: string | null; }; +/** + * UserRole + */ +export type UserRole = 'admin' | 'manager' | 'member'; + /** * UserUpdate */ @@ -272,10 +275,7 @@ export type UserUpdate = { * Is Active */ is_active?: boolean | null; - /** - * Is Superuser - */ - is_superuser?: boolean | null; + role?: UserRole | null; /** * Full Name */ @@ -342,6 +342,47 @@ export type ValidationError = { }; }; +/** + * UserPublic + */ +export type UserPublicWritable = { + /** + * Email + */ + email: string; + /** + * Is Active + */ + is_active?: boolean; + role?: UserRole; + /** + * Full Name + */ + full_name?: string | null; + /** + * Id + */ + id: string; + /** + * Created At + */ + created_at?: string | null; +}; + +/** + * UsersPublic + */ +export type UsersPublicWritable = { + /** + * Data + */ + data: Array; + /** + * Count + */ + count: number; +}; + export type loginLoginAccessTokenData = { body: Body_login_login_access_token; path?: never; @@ -724,6 +765,22 @@ export type usersUpdateUserResponses = { export type usersUpdateUserResponse = usersUpdateUserResponses[keyof usersUpdateUserResponses]; +export type metricsReadMetricsData = { + body?: never; + path?: never; + query?: never; + url: '/api/v1/metrics/'; +}; + +export type metricsReadMetricsResponses = { + /** + * Successful Response + */ + 200: Message; +}; + +export type metricsReadMetricsResponse = metricsReadMetricsResponses[keyof metricsReadMetricsResponses]; + export type utilsTestEmailData = { body?: never; path?: never; diff --git a/frontend/src/components/Admin/AddUser.tsx b/frontend/src/components/Admin/AddUser.tsx index 0cb83b2d46..ca7e08cfbc 100644 --- a/frontend/src/components/Admin/AddUser.tsx +++ b/frontend/src/components/Admin/AddUser.tsx @@ -28,6 +28,13 @@ import { } from "@/components/ui/form" import { Input } from "@/components/ui/input" import { LoadingButton } from "@/components/ui/loading-button" +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select" import useCustomToast from "@/hooks/useCustomToast" import { handleError } from "@/utils" @@ -42,7 +49,7 @@ const formSchema = z confirm_password: z .string() .min(1, { message: "Please confirm your password" }), - is_superuser: z.boolean(), + role: z.enum(["admin", "manager", "member"]), is_active: z.boolean(), }) .refine((data) => data.password === data.confirm_password, { @@ -66,7 +73,7 @@ const AddUser = () => { full_name: "", password: "", confirm_password: "", - is_superuser: false, + role: "member", is_active: false, }, }) @@ -85,7 +92,8 @@ const AddUser = () => { }) const onSubmit = (data: FormData) => { - mutation.mutate(data) + const { confirm_password: _, ...submitData } = data + mutation.mutate(submitData) } return ( @@ -186,16 +194,26 @@ const AddUser = () => { ( - - - - - Is superuser? + + Role + + )} /> diff --git a/frontend/src/components/Admin/EditUser.tsx b/frontend/src/components/Admin/EditUser.tsx index 9d001a2a3e..4725474544 100644 --- a/frontend/src/components/Admin/EditUser.tsx +++ b/frontend/src/components/Admin/EditUser.tsx @@ -28,6 +28,13 @@ import { } from "@/components/ui/form" import { Input } from "@/components/ui/input" import { LoadingButton } from "@/components/ui/loading-button" +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select" import useCustomToast from "@/hooks/useCustomToast" import { handleError } from "@/utils" @@ -41,7 +48,7 @@ const formSchema = z .optional() .or(z.literal("")), confirm_password: z.string().optional(), - is_superuser: z.boolean().optional(), + role: z.enum(["admin", "manager", "member"]).optional(), is_active: z.boolean().optional(), }) .refine((data) => !data.password || data.password === data.confirm_password, { @@ -68,7 +75,7 @@ const EditUser = ({ user, onSuccess }: EditUserProps) => { defaultValues: { email: user.email, full_name: user.full_name ?? undefined, - is_superuser: user.is_superuser, + role: user.role ?? "member", is_active: user.is_active, }, }) @@ -188,16 +195,26 @@ const EditUser = ({ user, onSuccess }: EditUserProps) => { ( - - - - - Is superuser? + + Role + + )} /> diff --git a/frontend/src/components/Admin/UserActionsMenu.tsx b/frontend/src/components/Admin/UserActionsMenu.tsx index 01f71cbb7a..3322cc0c6f 100644 --- a/frontend/src/components/Admin/UserActionsMenu.tsx +++ b/frontend/src/components/Admin/UserActionsMenu.tsx @@ -9,6 +9,7 @@ import { DropdownMenuTrigger, } from "@/components/ui/dropdown-menu" import useAuth from "@/hooks/useAuth" +import usePermissions from "@/hooks/usePermissions" import DeleteUser from "./DeleteUser" import EditUser from "./EditUser" @@ -19,11 +20,16 @@ interface UserActionsMenuProps { export const UserActionsMenu = ({ user }: UserActionsMenuProps) => { const [open, setOpen] = useState(false) const { user: currentUser } = useAuth() + const { can } = usePermissions() if (user.id === currentUser?.id) { return null } + if (!can("users:update_any") && !can("users:delete")) { + return null + } + return ( @@ -32,8 +38,12 @@ export const UserActionsMenu = ({ user }: UserActionsMenuProps) => { - setOpen(false)} /> - setOpen(false)} /> + {can("users:update_any") ? ( + setOpen(false)} /> + ) : null} + {can("users:delete") ? ( + setOpen(false)} /> + ) : null} ) diff --git a/frontend/src/components/Admin/columns.tsx b/frontend/src/components/Admin/columns.tsx index 8b0fa13eef..d04d8fad7c 100644 --- a/frontend/src/components/Admin/columns.tsx +++ b/frontend/src/components/Admin/columns.tsx @@ -39,11 +39,11 @@ export const columns: ColumnDef[] = [ ), }, { - accessorKey: "is_superuser", + accessorKey: "role", header: "Role", cell: ({ row }) => ( - - {row.original.is_superuser ? "Superuser" : "User"} + + {row.original.role ?? "member"} ), }, diff --git a/frontend/src/components/Common/AccessDenied.tsx b/frontend/src/components/Common/AccessDenied.tsx new file mode 100644 index 0000000000..e3a557c898 --- /dev/null +++ b/frontend/src/components/Common/AccessDenied.tsx @@ -0,0 +1,28 @@ +// Friendly access-denied state for unauthorized routes and actions. +import { Link } from "@tanstack/react-router" +import { ShieldX } from "lucide-react" + +import { Button } from "@/components/ui/button" + +type AccessDeniedProps = { + title?: string + message?: string +} + +export default function AccessDenied({ + title = "Access Denied", + message = "You do not have permission to view this page.", +}: AccessDeniedProps) { + return ( +
+ +
+

{title}

+

{message}

+
+ +
+ ) +} diff --git a/frontend/src/components/Sidebar/AppSidebar.tsx b/frontend/src/components/Sidebar/AppSidebar.tsx index 8502bcb9a4..720e0b7fbb 100644 --- a/frontend/src/components/Sidebar/AppSidebar.tsx +++ b/frontend/src/components/Sidebar/AppSidebar.tsx @@ -1,4 +1,4 @@ -import { Briefcase, Home, Users } from "lucide-react" +import { BarChart3, Briefcase, Home, Users } from "lucide-react" import { SidebarAppearance } from "@/components/Common/Appearance" import { Logo } from "@/components/Common/Logo" @@ -9,6 +9,7 @@ import { SidebarHeader, } from "@/components/ui/sidebar" import useAuth from "@/hooks/useAuth" +import usePermissions from "@/hooks/usePermissions" import { type Item, Main } from "./Main" import { User } from "./User" @@ -19,10 +20,15 @@ const baseItems: Item[] = [ export function AppSidebar() { const { user: currentUser } = useAuth() + const { can } = usePermissions() - const items = currentUser?.is_superuser - ? [...baseItems, { icon: Users, title: "Admin", path: "/admin" }] - : baseItems + const items = [...baseItems] + if (can("metrics:view")) { + items.push({ icon: BarChart3, title: "Metrics", path: "/metrics" }) + } + if (can("users:list")) { + items.push({ icon: Users, title: "Admin", path: "/admin" }) + } return ( diff --git a/frontend/src/components/UserSettings/GlobalSettings.tsx b/frontend/src/components/UserSettings/GlobalSettings.tsx new file mode 100644 index 0000000000..c51be0e1ab --- /dev/null +++ b/frontend/src/components/UserSettings/GlobalSettings.tsx @@ -0,0 +1,12 @@ +// Admin-only global settings stub page. +export default function GlobalSettings() { + return ( +
+

Global Settings

+

+ System-wide configuration (admin only). This is a stub for future + settings such as email templates, feature flags, and retention policies. +

+
+ ) +} diff --git a/frontend/src/hooks/usePermissions.ts b/frontend/src/hooks/usePermissions.ts new file mode 100644 index 0000000000..a3c6c3b457 --- /dev/null +++ b/frontend/src/hooks/usePermissions.ts @@ -0,0 +1,20 @@ +// React hook exposing role-based permission checks for the current user. +import useAuth from "@/hooks/useAuth" +import { + type Permission, + type UserRole, + userHasAnyPermission, + userHasPermission, +} from "@/lib/permissions" + +export default function usePermissions() { + const { user } = useAuth() + const role = user?.role as UserRole | undefined + + return { + role, + can: (permission: Permission) => userHasPermission(role, permission), + canAny: (permissions: Permission[]) => + userHasAnyPermission(role, permissions), + } +} diff --git a/frontend/src/lib/permissions.ts b/frontend/src/lib/permissions.ts new file mode 100644 index 0000000000..f3ecfb8d9b --- /dev/null +++ b/frontend/src/lib/permissions.ts @@ -0,0 +1,44 @@ +// Role-based permission helpers mirroring backend/app/core/permissions.py. +export type UserRole = "admin" | "manager" | "member" + +export type Permission = + | "users:list" + | "users:create" + | "users:update_any" + | "users:delete" + | "metrics:view" + | "profile:update_self" + | "settings:global" + | "items:list_any" + | "items:manage_any" + +const ROLE_PERMISSIONS: Record = { + admin: [ + "users:list", + "users:create", + "users:update_any", + "users:delete", + "metrics:view", + "profile:update_self", + "settings:global", + "items:list_any", + "items:manage_any", + ], + manager: ["users:list", "metrics:view", "profile:update_self"], + member: ["profile:update_self"], +} + +export function userHasPermission( + role: UserRole | undefined, + permission: Permission, +): boolean { + if (!role) return false + return ROLE_PERMISSIONS[role]?.includes(permission) ?? false +} + +export function userHasAnyPermission( + role: UserRole | undefined, + permissions: Permission[], +): boolean { + return permissions.some((permission) => userHasPermission(role, permission)) +} diff --git a/frontend/src/routeTree.gen.ts b/frontend/src/routeTree.gen.ts index b45f83a1d3..2826484126 100644 --- a/frontend/src/routeTree.gen.ts +++ b/frontend/src/routeTree.gen.ts @@ -17,6 +17,7 @@ import { Route as SignupRouteImport } from './routes/signup' import { Route as LayoutIndexRouteImport } from './routes/_layout/index' import { Route as LayoutAdminRouteImport } from './routes/_layout/admin' import { Route as LayoutItemsRouteImport } from './routes/_layout/items' +import { Route as LayoutMetricsRouteImport } from './routes/_layout/metrics' import { Route as LayoutSettingsRouteImport } from './routes/_layout/settings' const LayoutRoute = LayoutRouteImport.update({ @@ -58,6 +59,11 @@ const LayoutItemsRoute = LayoutItemsRouteImport.update({ path: '/items', getParentRoute: () => LayoutRoute, } as any) +const LayoutMetricsRoute = LayoutMetricsRouteImport.update({ + id: '/metrics', + path: '/metrics', + getParentRoute: () => LayoutRoute, +} as any) const LayoutSettingsRoute = LayoutSettingsRouteImport.update({ id: '/settings', path: '/settings', @@ -72,6 +78,7 @@ export interface FileRoutesByFullPath { '/signup': typeof SignupRoute '/admin': typeof LayoutAdminRoute '/items': typeof LayoutItemsRoute + '/metrics': typeof LayoutMetricsRoute '/settings': typeof LayoutSettingsRoute } export interface FileRoutesByTo { @@ -81,6 +88,7 @@ export interface FileRoutesByTo { '/signup': typeof SignupRoute '/admin': typeof LayoutAdminRoute '/items': typeof LayoutItemsRoute + '/metrics': typeof LayoutMetricsRoute '/settings': typeof LayoutSettingsRoute '/': typeof LayoutIndexRoute } @@ -93,6 +101,7 @@ export interface FileRoutesById { '/signup': typeof SignupRoute '/_layout/admin': typeof LayoutAdminRoute '/_layout/items': typeof LayoutItemsRoute + '/_layout/metrics': typeof LayoutMetricsRoute '/_layout/settings': typeof LayoutSettingsRoute '/_layout/': typeof LayoutIndexRoute } @@ -106,6 +115,7 @@ export interface FileRouteTypes { | '/signup' | '/admin' | '/items' + | '/metrics' | '/settings' fileRoutesByTo: FileRoutesByTo to: @@ -115,6 +125,7 @@ export interface FileRouteTypes { | '/signup' | '/admin' | '/items' + | '/metrics' | '/settings' | '/' id: @@ -126,6 +137,7 @@ export interface FileRouteTypes { | '/signup' | '/_layout/admin' | '/_layout/items' + | '/_layout/metrics' | '/_layout/settings' | '/_layout/' fileRoutesById: FileRoutesById @@ -196,6 +208,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof LayoutItemsRouteImport parentRoute: typeof LayoutRoute } + '/_layout/metrics': { + id: '/_layout/metrics' + path: '/metrics' + fullPath: '/metrics' + preLoaderRoute: typeof LayoutMetricsRouteImport + parentRoute: typeof LayoutRoute + } '/_layout/settings': { id: '/_layout/settings' path: '/settings' @@ -209,6 +228,7 @@ declare module '@tanstack/react-router' { interface LayoutRouteChildren { LayoutAdminRoute: typeof LayoutAdminRoute LayoutItemsRoute: typeof LayoutItemsRoute + LayoutMetricsRoute: typeof LayoutMetricsRoute LayoutSettingsRoute: typeof LayoutSettingsRoute LayoutIndexRoute: typeof LayoutIndexRoute } @@ -216,6 +236,7 @@ interface LayoutRouteChildren { const LayoutRouteChildren: LayoutRouteChildren = { LayoutAdminRoute: LayoutAdminRoute, LayoutItemsRoute: LayoutItemsRoute, + LayoutMetricsRoute: LayoutMetricsRoute, LayoutSettingsRoute: LayoutSettingsRoute, LayoutIndexRoute: LayoutIndexRoute, } diff --git a/frontend/src/routes/_layout/admin.tsx b/frontend/src/routes/_layout/admin.tsx index 39241c23ca..822ad61dca 100644 --- a/frontend/src/routes/_layout/admin.tsx +++ b/frontend/src/routes/_layout/admin.tsx @@ -1,13 +1,15 @@ import { useSuspenseQuery } from "@tanstack/react-query" -import { createFileRoute, redirect } from "@tanstack/react-router" +import { createFileRoute } from "@tanstack/react-router" import { Suspense } from "react" import { type UserPublic, UsersService } from "@/client" import AddUser from "@/components/Admin/AddUser" import { columns, type UserTableData } from "@/components/Admin/columns" +import AccessDenied from "@/components/Common/AccessDenied" import { DataTable } from "@/components/Common/DataTable" import PendingUsers from "@/components/Pending/PendingUsers" import useAuth from "@/hooks/useAuth" +import usePermissions from "@/hooks/usePermissions" function getUsersQueryOptions() { return { @@ -19,14 +21,6 @@ function getUsersQueryOptions() { export const Route = createFileRoute("/_layout/admin")({ component: Admin, - beforeLoad: async () => { - const { data: user } = await UsersService.readUserMe() - if (!user.is_superuser) { - throw redirect({ - to: "/", - }) - } - }, head: () => ({ meta: [ { @@ -57,6 +51,12 @@ function UsersTable() { } function Admin() { + const { can } = usePermissions() + + if (!can("users:list")) { + return + } + return (
@@ -66,7 +66,7 @@ function Admin() { Manage user accounts and permissions

- + {can("users:create") ? : null}
diff --git a/frontend/src/routes/_layout/metrics.tsx b/frontend/src/routes/_layout/metrics.tsx new file mode 100644 index 0000000000..e2724d565f --- /dev/null +++ b/frontend/src/routes/_layout/metrics.tsx @@ -0,0 +1,58 @@ +import { useQuery } from "@tanstack/react-query" +import { createFileRoute } from "@tanstack/react-router" +import { BarChart3 } from "lucide-react" + +import AccessDenied from "@/components/Common/AccessDenied" +import usePermissions from "@/hooks/usePermissions" + +export const Route = createFileRoute("/_layout/metrics")({ + component: Metrics, + head: () => ({ + meta: [{ title: "Metrics - FastAPI Template" }], + }), +}) + +function Metrics() { + const { can } = usePermissions() + + const { data, isLoading, isError } = useQuery({ + queryKey: ["metrics"], + queryFn: async () => { + const response = await fetch("/api/v1/metrics/", { + headers: { + Authorization: `Bearer ${localStorage.getItem("access_token") ?? ""}`, + }, + }) + if (!response.ok) { + throw new Error("Failed to load metrics") + } + return response.json() as Promise<{ message: string }> + }, + enabled: can("metrics:view"), + }) + + if (!can("metrics:view")) { + return + } + + return ( +
+
+ +
+

Metrics

+

Application insights and KPIs

+
+
+
+ {isLoading ? ( +

Loading metrics...

+ ) : isError ? ( +

Unable to load metrics.

+ ) : ( +

{data?.message ?? "No metrics available."}

+ )} +
+
+ ) +} diff --git a/frontend/src/routes/_layout/settings.tsx b/frontend/src/routes/_layout/settings.tsx index e109b5ae81..e7590de170 100644 --- a/frontend/src/routes/_layout/settings.tsx +++ b/frontend/src/routes/_layout/settings.tsx @@ -1,10 +1,12 @@ import { createFileRoute } from "@tanstack/react-router" - +import AccessDenied from "@/components/Common/AccessDenied" import ChangePassword from "@/components/UserSettings/ChangePassword" import DeleteAccount from "@/components/UserSettings/DeleteAccount" +import GlobalSettings from "@/components/UserSettings/GlobalSettings" import UserInformation from "@/components/UserSettings/UserInformation" import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs" import useAuth from "@/hooks/useAuth" +import usePermissions from "@/hooks/usePermissions" const tabsConfig = [ { value: "my-profile", title: "My profile", component: UserInformation }, @@ -25,14 +27,27 @@ export const Route = createFileRoute("/_layout/settings")({ function UserSettings() { const { user: currentUser } = useAuth() - const finalTabs = currentUser?.is_superuser - ? tabsConfig.slice(0, 3) + const { can } = usePermissions() + + const finalTabs = can("settings:global") + ? [ + ...tabsConfig, + { + value: "global", + title: "Global settings", + component: GlobalSettings, + }, + ] : tabsConfig if (!currentUser) { return null } + if (!can("profile:update_self")) { + return + } + return (
diff --git a/frontend/tests/admin.spec.ts b/frontend/tests/admin.spec.ts index cd73dbad5c..828c6833b1 100644 --- a/frontend/tests/admin.spec.ts +++ b/frontend/tests/admin.spec.ts @@ -42,7 +42,7 @@ test.describe("Admin user management", () => { await expect(userRow).toBeVisible() }) - test("Create a superuser", async ({ page }) => { + test("Create an admin user", async ({ page }) => { await page.goto("/admin") const email = randomEmail() @@ -53,7 +53,8 @@ test.describe("Admin user management", () => { await page.getByPlaceholder("Email").fill(email) await page.getByPlaceholder("Password").first().fill(password) await page.getByPlaceholder("Password").last().fill(password) - await page.getByLabel("Is superuser?").check() + await page.getByLabel("Role").click() + await page.getByRole("option", { name: "Admin" }).click() await page.getByLabel("Is active?").check() await page.getByRole("button", { name: "Save" }).click() @@ -63,7 +64,7 @@ test.describe("Admin user management", () => { await expect(page.getByRole("dialog")).not.toBeVisible() const userRow = page.getByRole("row").filter({ hasText: email }) - await expect(userRow.getByText("Superuser")).toBeVisible() + await expect(userRow.getByText("admin")).toBeVisible() }) test("Edit a user successfully", async ({ page }) => { @@ -182,7 +183,7 @@ test.describe("Admin user management", () => { test.describe("Admin page access control", () => { test.use({ storageState: { cookies: [], origins: [] } }) - test("Non-superuser cannot access admin page", async ({ page }) => { + test("Member cannot access admin page", async ({ page }) => { const email = randomEmail() const password = randomPassword() @@ -192,10 +193,12 @@ test.describe("Admin page access control", () => { await page.goto("/admin") await expect(page.getByRole("heading", { name: "Users" })).not.toBeVisible() - await expect(page).not.toHaveURL(/\/admin/) + await expect( + page.getByRole("heading", { name: "Access Denied" }), + ).toBeVisible() }) - test("Superuser can access admin page", async ({ page }) => { + test("Admin can access admin page", async ({ page }) => { await logInUser(page, firstSuperuser, firstSuperuserPassword) await page.goto("/admin") diff --git a/scripts/smoke_rbac.py b/scripts/smoke_rbac.py new file mode 100644 index 0000000000..b935a67044 --- /dev/null +++ b/scripts/smoke_rbac.py @@ -0,0 +1,59 @@ +#!/usr/bin/env python3 +"""Smoke test for RBAC API endpoints.""" + +import json +import sys +import urllib.error +import urllib.parse +import urllib.request + +BASE = "http://localhost:8000" + + +def token(email: str, password: str) -> str: + data = urllib.parse.urlencode({"username": email, "password": password}).encode() + req = urllib.request.Request( + f"{BASE}/api/v1/login/access-token", data=data, method="POST" + ) + req.add_header("Content-Type", "application/x-www-form-urlencoded") + with urllib.request.urlopen(req) as resp: + return json.load(resp)["access_token"] + + +def status_code(path: str, access_token: str) -> int: + req = urllib.request.Request(f"{BASE}{path}") + req.add_header("Authorization", f"Bearer {access_token}") + try: + urllib.request.urlopen(req) + return 200 + except urllib.error.HTTPError as exc: + return exc.code + + +def main() -> int: + admin = token("admin@example.com", "changethis") + manager = token("manager@example.com", "changethis") + member = token("member@example.com", "changethis") + + checks = { + "admin_users": status_code("/api/v1/users/", admin), + "manager_users": status_code("/api/v1/users/", manager), + "member_users": status_code("/api/v1/users/", member), + "admin_metrics": status_code("/api/v1/metrics/", admin), + "member_metrics": status_code("/api/v1/metrics/", member), + } + for name, code in checks.items(): + print(f"{name}: {code}") + + expected = { + "admin_users": 200, + "manager_users": 200, + "member_users": 403, + "admin_metrics": 200, + "member_metrics": 403, + } + return 0 if checks == expected else 1 + + +if __name__ == "__main__": + sys.exit(main())