Skip to content

Multi User Support - #387

Open
samueljackson92 wants to merge 230 commits into
devfrom
slj/multi-user-support
Open

samueljackson92 wants to merge 230 commits into
devfrom
slj/multi-user-support

Conversation

@samueljackson92

Copy link
Copy Markdown
Contributor

This PR implements a first pass at adding multi-user support to toktagger by:

Expanding the framework to support the concept of a "user" and annotations now have a user associated with them
Users have roles associated with them. Currently they are "admin", "editor" and "viewer"

  • Viewers can only view projects, they cannot edit or manage contributors

  • Editors can make and modify annotations, but cannot manage other accounts.

  • Admins can do everything.

  • Added guards to each API enpoint to prevent modifications without being an appropriately authenticated user

  • Added new UI pages to support this framework change

    • Login page - users are now directed to the login page by default if not authenticated
    • Admin control panel - accessible to admins only, can add, delete, and manage accounts
    • User profile page - allows a user to see their login name and change their password.
  • Changed to using guicorn for python api for multi user support.

  • Updated user documentation

  • Updated setup development script

  • Updated test suite covering new functionality.

To test:
On default start-up, the default admin username and password will be printed in the command line terminal of the backend server.

  • check test suite passes successfully
  • check functionality of each type of user
  • check concurrent user annotation

claude and others added 30 commits July 7, 2026 14:09
- JWT-based auth using itsdangerous (stdlib-only, no system crypto dependency)
  with PBKDF2 password hashing; secret auto-persisted to ~/.cache/toktagger/
- First-run mode: when DB has no users, prints admin credentials to terminal
  and runs in passthrough mode (backward compatible with existing installs)
- Concurrent annotation safety: update_annotations now scoped by created_by,
  so two users annotating the same shot no longer overwrite each other
- Project membership (admin/annotator/viewer roles); non-admins see only
  their own projects; project admins manage membership via UI dialog
- Per-user show_others_annotations toggle stored in project_members collection
  and enforced server-side in GET /annotations
- New routes: POST /auth/token, GET /auth/me, CRUD /users, CRUD /projects/{id}/members
- Frontend: Login page, Admin user management page, Project members dialog,
  AuthContext with token persistence, RequireAuth/RequireAdmin route guards,
  user info bar with logout on projects page, apiFetch wrapper on all API calls
- Pre-existing fix: guard ModelRegistry/ray import under models_dependencies_installed()
  in routers/models.py and routers/meta.py

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
Tests (45 total, all passing):
- tests/api/auth/test_core.py — unit tests for hash_password, verify_password,
  create_access_token, decode_token (11 tests)
- tests/api/auth/test_first_run.py — unit tests for ensure_admin_user (5 tests)
- tests/api/auth/test_auth_router.py — /auth/token and /auth/me endpoints (8 tests)
- tests/api/auth/test_users_router.py — /users and /projects/{id}/members CRUD (12 tests)
- tests/api/auth/test_concurrent_annotations.py — concurrent annotation safety,
  identity enforcement, show_others filter, viewer access control (9 tests)

Bugs fixed during test authoring:
- MongoDBClient: file-path mode was ignoring the provided URL and always writing to
  the user cache dir, causing all tests to share a single DB (silent data corruption)
- get_project_members: stored user_id ObjectId was not stringified before Pydantic
  validation, causing ValidationError on every member list call
- GET /samples/{id}/annotations: had no project membership check, allowing any
  authenticated user to read any project's annotations (security gap)

Test infrastructure:
- tests/api/auth/conftest.py: self-contained fixtures using mongita disk client
  with per-test tmp_path isolation (no Docker required)
- tests/conftest.py: guarded ray/ModelRegistry imports behind _models_available flag
  so auth tests can run without ray installed
- pyproject.toml: added [tool.pytest.ini_options] asyncio_mode = "auto"

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
- Add `require_project_viewer` dependency (any project member may read)
- annotations.py: GET /annotations → viewer; PUT /annotations → annotator;
  DELETE /annotations and DELETE /samples/{id}/annotations → admin
- samples.py: all 8 endpoints now guarded (reads→viewer, writes→annotator,
  deletes→admin); remove debug print from add_samples
- data.py: POST /data guarded with viewer check
- annotators.py: GET /annotator → viewer; POST /annotator/{type} → annotator
- models.py: all 12 endpoints guarded alongside existing feature check
- Add 19-test suite (test_endpoint_guards.py) covering non-member/viewer/
  annotator/admin access across samples, project annotations, sample
  annotation delete, and data endpoints — all 64 auth tests pass

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
…import enforcement

- auth/core.py: add get_internal_token() for stable per-process server secret
- auth/dependencies.py: accept internal token as synthetic admin (Ray worker
  callbacks authenticate as __internal__ without a DB round-trip)
- main.py: pass API_TOKEN to Ray workers via runtime_env so sender.py can auth
- core/sender.py: inject Authorization header when API_TOKEN env var is set
- worker.py: prefix model prediction created_by with "model::" to prevent
  username collision (e.g. a user named "disruption_cnn" cannot corrupt
  model predictions via scoped annotation deletes)
- routers/models.py: update delete_predictions filter to match "model::" prefix
- routers/users.py: reject usernames starting with "model::" or "__"
- routers/annotations.py: enforce created_by = current user for non-admin,
  non-internal bulk imports (prevents created_by spoofing)
- modelPredictSample.tsx: update three created_by comparisons to use
  modelCreatedBy() helper that prepends "model::" prefix
- test_model_auth.py: 8 tests covering all of the above (72/72 passing)

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
The MONGO_URL default was accidentally set to "./toktagger_db" during
Phase 1 auth work, placing the database inside the project directory
instead of the intended ~/.cache/toktagger/ukaea/ location. Passing
"default" triggers the existing user_cache_dir fallback in db.py.

Also add toktagger_db/ to .gitignore to prevent accidental future
commits of local database files.

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
Five categories of collection/runtime failures, all pre-existing:

1. tests/db_definitions.py: unconditional `import ray` blocked 4 test
   files. Added a stub (no-op remote decorator) when ray is absent.
   Also changed MODEL_3 type from "disruption_cnn" to
   "mock_disruption_cnn" — the real type is only registered when ray
   is installed, so the Pydantic validator rejected it otherwise.

2. toktagger/api/models/base.py: @ray.remote on WorkerRegistry was
   outside the conditional import guard, causing NameError at import
   time. Added matching stub so the decorator is a no-op without ray.

3. tests/api/routers/test_models.py: direct `import ray` crashes
   collection. Changed to pytest.importorskip("ray") for clean skip.

4. tests/end_to_end/__init__.py: unconditional playwright import
   blocked all 4 e2e test files. Wrapped in try/except; each test
   file gets pytest.importorskip("playwright") for clean skip.

5. tests/conftest.py / test_annotator.py / test_data_loaders.py:
   - Docker not available: mongo_container fixture now skips when
     docker.from_env().ping() fails, turning 56 ERRORs into skips.
   - pooch (optional scipy dep) absent: test_annotator.py gets
     importorskip at module level.
   - FAIR-MAST external endpoint inaccessible: test now calls
     pytest.skip() on any network error rather than failing.

Result: 91 passed, 159 skipped, 0 errors.

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
Ruff lint (24 errors → 0):
- Remove unused imports auto-fixed by ruff --fix
- Rename unused local variables with _ prefix (original_loads, result,
  direction) to satisfy F841
- Add # noqa: E402 to imports after pytest.importorskip() in
  test_models.py (the skip call must precede imports)

Ruff format (27 files → 0):
- Auto-reformatted with ruff format

ESLint (1 error → 0):
- Remove unused useNavigate import from project_id/page.tsx

Prettier (11 files → 0):
- Auto-reformatted with prettier --write

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW
…ur assertions

- tests/conftest.py: set app.state.auth_required=False in legacy api_client
  fixture so unauthenticated router tests pass (ensure_admin_user always returns
  True since the branch requires auth, but legacy tests have no token)
- tests/conftest.py: update setup_model_samples created_by to
  "model::mock_disruption_cnn" to match the model:: namespace prefix
  introduced in commit 893ba2c
- tests/api/crud/test_utils.py: update test_get_models_by_type and
  test_get_models_by_status to reflect MODEL_3 type change from
  "disruption_cnn" → "mock_disruption_cnn" (commit a06385f)
- tests/api/routers/test_annotations.py: update annotation count (8 not 7)
  and skip created_by assertion since the server now overwrites it with the
  authenticated user's identity
- tests/api/routers/test_models.py: update delete/stop tests to use
  "mock_disruption_cnn" throughout; fix test_model_delete_no_predictions to
  use "mock_timeseries_cnn" (type with no seeded predictions)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- toktagger/api/main.py: honour TOKTAGGER_AUTH_REQUIRED=false env var to
  bypass auth in the test server process (auth is always required in
  production, but E2E tests spin up a real server process that was timing
  out trying to get a 200 from /projects — now 401 is also accepted, and
  the test process disables auth via the env var so Playwright tests see
  the same UI they did before auth was added)
- tests/conftest.py: set TOKTAGGER_AUTH_REQUIRED=false in run_server() so
  the E2E server starts in passthrough mode; also accept 401 from
  start_server health check so it doesn't wait 10 min before failing
- .github/workflows/ci.yml: skip "Commit built files" push for fork PRs
  (github-actions[bot] cannot write to ukaea/toktagger from a fork PR)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Unit tests (~3 min) + E2E Playwright tests together now exceed 15 min
since tests actually run rather than fast-failing on 401.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ired mode

When TOKTAGGER_AUTH_REQUIRED=false, the server returns a synthetic admin
from GET /auth/me with no token. The previous AuthContext short-circuited
on mount if no token was in localStorage, so the frontend always redirected
to /ui/login regardless of server auth state.

Now AuthContext always calls /auth/me on mount; if the server returns a user
(auth-not-required passthrough), RequireAuth lets the tests proceed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The server now sets created_by = current_user.username on every annotation
PUT. In the E2E environment (TOKTAGGER_AUTH_REQUIRED=false) the synthetic
user is "admin", so saved annotations get created_by="admin", not "manual".

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
testcontainers was imported in conftest.py but not listed in
pyproject.toml, causing ModuleNotFoundError in both pytest CI jobs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…prefix

The delete-predictions endpoint validates model_type against registered
types and filters by 'model::<type>'. The fixture had created_by='disruption_cnn'
(wrong type, wrong prefix); fix to 'model::mock_disruption_cnn'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The built frontend JS (from ukaea/dev) filters model annotations by bare
model type string. The backend worker was adding a 'model::' prefix that
the JS doesn't know about, so disabling the predict tool never cleared
annotations. Use bare model.type in worker and delete-predictions filter.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- stop_model_training: use require_project_annotator (not admin)
- crud/utils.py: remove unused _direction variable and pymongo import
- test_auth_router: assert exactly 403 for deactivated-user login
- test_model_auth: fix test_user_save_does_not_corrupt_model_prefixed_predictions
  to use a user actually named 'disruption_cnn' (was using 'alice')
- test_users_router: add tests for non-admin update/delete-other-user (403)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The session-scoped settings fixture was present on ukaea/dev but dropped
during the branch rebase. ray_session (models_fixtures.py) depends on it
for MODEL_STORAGE env var and setup_model_db uses config.settings.models.cache_dir.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
os.environ["MODEL_STORAGE"] is only set inside Ray worker processes
(via runtime_env), not in the test process itself. Tests that read
model file paths must use config.settings.models.cache_dir directly,
which is patched to a temp directory by the session settings fixture.

Similarly, DISABLE_LOCAL_MODEL_LOAD is not read by the router; the
router checks config.settings.models.local_load_enabled instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The built frontend JS identifies model predictions by created_by===model::${modelType}.
Commit 1e959c5 incorrectly removed this prefix from the worker, so the Disable Tool
toggle could never match and hide predictions. Restore the prefix in the worker and
the delete_predictions filter, and update the fixture training data to match.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nftest.py

PR #269 (wk9874/use_mongita) replaced testcontainers with mongita on dev.
When this branch merged, the conflict was kept incorrectly and testcontainers
was re-added to pyproject.toml as a workaround. Align with dev:

- Remove MongoDbContainer import and mongo_container fixture
- db_client now depends on settings and uses MongoDBClient(mongo_url, ...)
- api_client now depends on db_client and injects app.state directly
- start_server now depends on settings (no MONGO_URL env var needed)
- Remove testcontainers[mongodb] from dev dependencies

Branch-specific additions are preserved: app.state.auth_required=False in
api_client, TOKTAGGER_AUTH_REQUIRED=false in run_server, 600-step E2E wait.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
samueljackson92 and others added 7 commits September 28, 2026 14:03
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The default admin password was "admin", but a recent change added an
8-character minimum on user passwords, so the bootstrap admin's own
first-login password-change request (and its login in tests) started
failing with 422. Bumped the default to "admin1234" everywhere it's
referenced: first_run.py, the conftest fixtures, the login e2e tests,
and the docs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@wk9874 wk9874 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backend rereviewed

Comment thread tests/api/auth/test_endpoint_guards.py Outdated
Comment thread tests/api/auth/test_endpoint_guards.py Outdated
Comment thread tests/api/auth/test_endpoint_guards.py Outdated
Comment thread tests/api/auth/test_first_run.py Outdated
Comment thread tests/api/auth/test_first_run.py Outdated

Self-scoped, so it needs no role check. The projects list uses it to gate each
row without issuing one `/projects/{id}/members` request per row. A global admin
is unrestricted by membership and gets an empty list.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh i see, yeah makes sense, maybe just clarify this behaviour in the docstring :)

Comment thread toktagger/api/crud/utils.py Outdated
Comment thread toktagger/api/crud/utils.py Outdated
Comment thread toktagger/api/crud/utils.py Outdated
Comment thread toktagger/api/schemas/annotations.py Outdated
Comment thread scripts/setup.py Outdated
Comment thread scripts/create_mock_data.py
<Heading>Confirm Deletion</Heading>
<Divider />
<Content>
Are you sure you want to delete project{" "}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project delete button is available to project annotators when it should be greyed out - trying to press delete does nothing (403 forbidden)

Image

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants