Implement multi-user annotation support with JWT authentication by samueljackson92 · Pull Request #279 · ukaea/toktagger

samueljackson92 · 2026-06-03T10:26:53Z

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

praksharma · 2026-06-22T07:43:43Z

The login page, user creation, shared annotations seems to work fine for me. I am not sure if the shared annotations are working for video data as there is no "created by" column in the video UI.
The only minor issue I noticed is that once you assign someone a role within a project, the drop-down menu to edit the role, does not get sufficient width and the arrow button to open the drop down menu isn't visible.

Another minor issue. If you do not enter the username/password or both, and pressed the enter key, it should print the same message "invalid username or password"?

wk9874 · 2026-06-22T13:27:16Z

I ran the scripts/setup.py script to generate some projects, and then trying to navigate to the samples table for any of these projects gives me a blank screen with this error in the console:

- 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

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

wk9874

Ok, think I have reviewed backend code and given a rough look over the tests

Need to add end to end tests for:

Login page
Admin user pages
Project admin pages
Non-admin editing their own acount settings
Trying to access pages of the UI which you dont have credentials for directly (eg going to projects/{project_id}. I hope this would give a nice 404 message instead of just a blank page?

wk9874 · 2026-06-22T13:14:29Z

    name: Pytest (Models Disabled)
    runs-on: ubuntu-latest
-    timeout-minutes: 15
+    timeout-minutes: 30


Assume this should be increased in both the models_disabled and models_enabled CI jobs?

wk9874 · 2026-06-22T13:31:26Z

        run: npm --prefix toktagger/ui run build

      - name: Commit built files
+        if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository


Do we need this? Assume its because the CI cannot commit to a fork? If we do need it, maybe we should have it skip the entire build job instead of just the commit step?

wk9874 · 2026-06-22T13:32:57Z

 ```

-This will start a local instance of the application running at `http://localhost:8002`.
+This starts the application at `http://localhost:8002`. On first launch an `admin` account is created automatically and the credentials are printed to the terminal.


Should warn here that they will not see this password again after this point

Should we have a way to either regenerate this password, or set it to a more memorable one (eg, password), and just on first login it forces you to change it before you can log in?

wk9874 · 2026-06-22T13:35:24Z

+| Role | Permissions |
+|---|---|
+| `admin` | Full access: create/edit/delete any project, manage all user accounts, view all annotations |
+| `user` | Access only to projects they are a member of; can annotate and export within those projects |


The PR says users can be either editors or viewers - is this the case? If so, need to update this documentation

From a quick look at the admin panel it looks like you can only be admin or user - is there a reason we dropped viewer / editor?

(unless that is a per-project setting which I have not yet discovered...)

wk9874 · 2026-06-22T13:38:45Z

+### Changing a User's Role
+
+1. Find the user in the table and click **Edit**.
+2. Select the new **Global Role**.


Here, you could reduce all admins down to regular users - maybe it should be enforced that at least one admin must be active, and if you try to change the final admin down to a regular user it doesnt allow it?

wk9874 · 2026-06-22T17:00:28Z

+
+
+@pytest.mark.asyncio
+async def test_update_own_user(auth_setup):


Should test update other user as non-admin, and as admin

wk9874 · 2026-06-22T17:00:49Z

+
+
+@pytest.mark.asyncio
+async def test_delete_user_as_admin(auth_setup):


Test delete user as non admin, and delete own user?

wk9874 · 2026-06-22T17:02:20Z

@@ -1,3 +1,6 @@
+import pytest
+
+pytest.importorskip("pooch")  # scipy.datasets.electrocardiogram requires pooch


Why would pooch ever not be installed? Is it an optional extra to scipy? If so, and if required by the annotators, it should always be included in our pyproject.toml. This importorskip should not be necessary since pooch is not in a toktagger optional dep group

wk9874 · 2026-06-22T17:03:23Z

 from toktagger.api.main import Server
 from toktagger.api.crud.db import MongoDBClient
 import tests.db_definitions as db_definitions
+from testcontainers.mongodb import MongoDbContainer


Not sure why this is making a reappearance - merge gone wrong somewhere?

wk9874 · 2026-06-22T17:03:44Z



+@pytest.fixture(scope="session")
+def mongo_container():


Definitely a merge gone wrong somewhere - this should no longer be present

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>

Both tests queried/called with 'mock_disruption_cnn' but MODEL_3 and the fixture's non-manual annotations use type/created_by 'disruption_cnn'. 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 ukaea#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 requested a review from wk9874 June 3, 2026 10:26

samueljackson92 self-assigned this Jun 3, 2026

samueljackson92 added the enhancement New feature or request label Jun 3, 2026

samueljackson92 marked this pull request as draft June 3, 2026 10:28

samueljackson92 marked this pull request as ready for review June 9, 2026 08:06

claude and others added 8 commits June 22, 2026 16:25

Ignore TypeScript incremental build cache files

d0aa89a

https://claude.ai/code/session_01WETiYYT19bgewacax9qWBW

Add update user management & styling

afda911

wk9874 requested changes Jun 22, 2026

View reviewed changes

claude and others added 14 commits June 23, 2026 08:07

Remove db

818c1d8

Add dep

68acd06

Increase Pytest CI timeout from 15 to 30 minutes

051f2e8

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>

Ruff linting

cb2edcd

Fix CI: add testcontainers to dev deps, remove unused import

6cb20a7

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>

Fix test_model_stop_training and test_model_delete_predictions

c4e8007

Both tests queried/called with 'mock_disruption_cnn' but MODEL_3 and the fixture's non-manual annotations use type/created_by 'disruption_cnn'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Fix issues from review after merge

83438bb

samueljackson92 added 3 commits June 23, 2026 08:10

Update to fix UI issues

eaeafd0

Update docs, fix warnings

172085c

Update to address PR comments

d285b1b

samueljackson92 force-pushed the slj/multi-user-support branch from 655f8e6 to d285b1b Compare June 23, 2026 07:54

samueljackson92 and others added 5 commits June 23, 2026 09:03



		@pytest.mark.asyncio
		async def test_update_own_user(auth_setup):



		@pytest.mark.asyncio
		async def test_delete_user_as_admin(auth_setup):

		@@ -1,3 +1,6 @@
		import pytest

		pytest.importorskip("pooch") # scipy.datasets.electrocardiogram requires pooch

Uh oh!

Conversation

samueljackson92 commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

praksharma commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wk9874 commented Jun 22, 2026

Uh oh!

wk9874 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

samueljackson92 commented Jun 3, 2026 •

edited

Loading

praksharma commented Jun 22, 2026 •

edited

Loading