Add personal API tokens for users - #4466
Conversation
Code reviewFound 3 issues, all fixed in 6c8d397:
integreat-cms/integreat_cms/cms/views/settings/user_settings_view.py Lines 149 to 159 in 322b84a
integreat-cms/integreat_cms/cms/models/users/user_api_token.py Lines 38 to 41 in 322b84a
integreat-cms/integreat_cms/cms/forms/users/user_api_token_form.py Lines 28 to 38 in 322b84a Also addressed: tests moved to 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
andrew8er
left a comment
There was a problem hiding this comment.
General implementation is good, but the column documentation is a bit lacking. There is also an alternative that might be preferable (regarding storage and compute).
Follow-up to the review of #4466: - store `prefix` and `token_hash` as `bytea` instead of hex encoded `varchar`, which halves the storage and drops the encoding step from every lookup - document the encoding of both columns, and derive the hash length from the hash algorithm instead of hard-coding it - rename `hash_token` to `_hash_token` and let it assemble the plaintext from prefix and secret itself, so storage and lookup cannot disagree on the format - reject prefixes of the wrong length or with non-hex characters before the database is queried Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6c8d397 to
0ebfe91
Compare
|
Thanks for the careful look at the columns, @andrew8er — all five points are in 0ebfe91:
Also rebased onto the current |
Introduce a `UserApiToken` model so users can create personal API tokens in their account settings. The tokens authenticate API requests on behalf of their user, so endpoints inherit exactly that user's permissions instead of bypassing the RBAC structure. Only a SHA-256 hash of the token is stored. The plaintext is shown once directly after creation and cannot be recovered afterwards. A random prefix is stored alongside the hash so a token can be looked up without hashing every row, and the comparison itself is constant-time. The new `api_token_required` decorator reads the `Authorization: Bearer` header, resolves and validates the token, rejects tokens of deactivated users, optionally enforces a permission and records the last usage. This is the authentication foundation for the CRM integration (#4138); the region settings and statistics endpoints follow in separate PRs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Do not pass the plaintext token through the messages framework. The project's MESSAGE_STORAGE is MessageLoggerStorage, which logs every message when MESSAGE_LOGGING_ENABLED is set — a setting that defaults to DEBUG and can be switched on in production. Creating a token would therefore have written the usable secret verbatim into the log, which defeats storing only a hash. The plaintext is now handed over via the session and rendered exactly once in the template, the same way TOTPRegisterView passes its secret. A regression test asserts the plaintext never reaches the log. Also: - Rename UserApiToken to ApiToken, following the convention established in #2257 that per-user models in cms/models/users/ carry no User prefix (FidoKey, Organization, Role) - Move the tests to tests/cms/models/users/ and tests/api/ so they mirror the app structure - Drop the unsupported *args from ApiTokenForm.__init__, matching the other forms and CustomModelForm's signature - Document that api_token_required does not check region membership - Correct the prefix-lookup rationale and the token_hash help text - Update the stale UserSettingsView.post docstring Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the review of #4466: - store `prefix` and `token_hash` as `bytea` instead of hex encoded `varchar`, which halves the storage and drops the encoding step from every lookup - document the encoding of both columns, and derive the hash length from the hash algorithm instead of hard-coding it - rename `hash_token` to `_hash_token` and let it assemble the plaintext from prefix and secret itself, so storage and lookup cannot disagree on the format - reject prefixes of the wrong length or with non-hex characters before the database is queried Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0ebfe91 to
37ff6a7
Compare
Short description
First step of the CRM integration (#4138): a personal API token per user, so the upcoming region settings and statistics endpoints can be authenticated without bypassing the RBAC structure.
The CMS API currently has no authentication at all — this PR adds the foundation, not yet any protected endpoint.
Per the discussion in #4138, this deliberately avoids a per-region token: the
region_slugin the URL already provides the CRM↔region mapping, and a token bound to a normal user keeps that user's permissions intact.Implementation
UserApiTokenmodel (modelled onFidoKey): user FK, name,prefix,token_hash,created_at,last_usageprefixallows looking up a token without hashing every row; the comparison itself is constant-time viasecrets.compare_digestapi_token_required(permission=None)decorator inapi/decorators.py: readsAuthorization: Bearer <token>, resolves and validates it, rejects tokens of deactivated users, optionally enforces a permission, recordslast_usageand setsrequest.userrequest.user.api_tokens, so nobody can delete somebody else's token.Testing
11 tests in
tests/cms/test_user_api_token.pycovering: plaintext is never persisted, token lookup (valid / wrong secret / unknown prefix / malformed / empty), successful authentication +last_usageupdate, rejection of missing/malformed/unknown/non-bearer headers, rejection of deactivated users, and permission enforcement (unprivileged user gets 403, privileged passes).ruff,mypy,djlintandcheck_translationsall pass locally.Follow-ups (separate PRs, see #4138)
POST/GET /api/v3/<region_slug>/settings/— generic region settings endpoint + read-only form for CRM-managed regions + removal of themt_midyear_start_monthlogic incl. data migration/api/v3/<region_slug>/statistics/— async webhook replacing the region-condition CSVPart of #4138 · CRM side: digitalfabrik/customcrm#19
🤖 Generated with Claude Code