Conversation
… to control table columns - Added `last_used_at` column to track last api call timestamp - Renamed `Token` to `ApiToken` - Added endpoint to retrieve user's Api Token -- `/api/auth/api-token/` - Small fixes
- Added ApiKey rotation endpoint
…to feat/api-tokens-last-used-at
📝 WalkthroughWalkthroughReplaces DRF Token with a new ApiKey model (tracks last_used_at), migrates existing tokens, updates authentication to persist last_used_at, removes authtoken/admin registration, adds GET/POST API endpoints to fetch/rotate keys, updates backend/frontend usages, and adds UI + i18n for API key management. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Frontend
participant Server
participant DB
Client->>Frontend: Click "Rotate API Key"
Frontend->>Server: POST /auth/api-key/rotate/
Server->>DB: DELETE FROM authentication_apikey WHERE user_id=...
Server->>DB: INSERT INTO authentication_apikey (key,user_id,created,last_used_at)
DB-->>Server: new key row
Server-->>Frontend: 201 { key }
Frontend-->>Client: show new key / toast
sequenceDiagram
participant Client
participant Frontend
participant Server
participant Authenticator
participant DB
Client->>Frontend: Request with Authorization: Token <key>
Frontend->>Server: API request
Server->>Authenticator: FallbackTokenAuthentication.authenticate_credentials(key)
Authenticator->>DB: SELECT * FROM authentication_apikey WHERE key=...
DB-->>Authenticator: ApiKey (user)
Authenticator->>DB: UPDATE authentication_apikey SET last_used_at=now() WHERE key=...
DB-->>Authenticator: update OK
Authenticator-->>Server: (user, token)
Server-->>Frontend: 200 / resource
Frontend-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@authentication/migrations/0001_add_token_model.py`:
- Around line 8-26: The reverse migration hardcodes the user table as users_user
which breaks rollbacks for custom AUTH_USER_MODEL; in
migrate_tokens_and_drop_old_table build the real user table name dynamically by
resolving settings.AUTH_USER_MODEL via apps.get_model(...) and using its
_meta.db_table (or schema_editor.quote_name/_meta.db_table) when constructing
the SQL that creates/references the user FK instead of the literal 'users_user';
update the SQL string assembly to interpolate that resolved table name (using
schema_editor or connection quoting APIs to avoid SQL injection) so rollback
works with custom user models.
In `@authentication/urls.py`:
- Around line 24-26: The API key endpoints are missing authentication
enforcement; add the DRF permission decorator
`@permission_classes`([IsAuthenticated]) to the view functions api_key_api_view
and api_key_rotate_api_view (same pattern used for invite_user_api_view) so both
endpoints require authenticated users; update the corresponding view definitions
in common to import and apply IsAuthenticated/@permission_classes to each
function.
In
`@front_end/src/app/`(main)/accounts/settings/account/components/api_access.tsx:
- Around line 57-69: handleCreateKey currently lacks error handling similar to
handleRotate; wrap the rotateApiKeyAction call in a try/catch (or add a catch
block) to catch failures, log the error and call toast with a failure message
(using t for translation) so the user gets feedback, while leaving the existing
finally block that calls setIsRotating(false); refer to handleCreateKey,
rotateApiKeyAction, setApiKey, setIsVisible, toast and t when applying the
change.
- Around line 142-149: The ConfirmModal remains open after a successful API key
rotation because handleRotate does not close the modal; update the rotation flow
so that after handleRotate completes successfully you call
setShowRotateModal(false) to close the modal. Locate the ConfirmModal usage and
the handleRotate function (and any async rotation helper it calls) and add logic
to setShowRotateModal(false) on success (e.g., in the .then/await success path
or after try block) while keeping error handling unchanged.
- Around line 43-55: handleRotate currently only checks for response.key and
ignores failure cases from rotateApiKeyAction; update handleRotate to detect
when the action returns errors (response.errors) or a falsy key, call
setIsRotating appropriately, and surface the failure to the user (e.g., call
toast with an error message and optionally log the error). Specifically, inside
the try block after awaiting rotateApiKeyAction(), if response.errors exists or
response.key is falsy, call toast(t("apiKeyRotateFailed") or a suitable message)
and log the error, otherwise proceed to setApiKey(response.key) and
setIsVisible(true); ensure setIsRotating(false) still runs in the finally block.
🧹 Nitpick comments (6)
tests/unit/conftest.py (1)
72-73: Consider renaming the variable for clarity.The code is functionally correct. The
"Token"prefix in the Authorization header is the authentication scheme name (not the model name), so it correctly remains unchanged.However, the variable name
tokencould be renamed toapi_keyfor consistency with the new model name, improving code readability.♻️ Optional: Rename variable for clarity
if user: - token = ApiKey.objects.create(user=user) - client.credentials(HTTP_AUTHORIZATION=f"Token {token.key}") + api_key = ApiKey.objects.create(user=user) + client.credentials(HTTP_AUTHORIZATION=f"Token {api_key.key}")authentication/migrations/0001_add_token_model.py (1)
29-49: Hardcoded user table reference in rollback may cause issues.The rollback function hardcodes
users_user(id)as the foreign key reference (line 37). IfAUTH_USER_MODELis ever changed or differs in some deployment, this rollback would fail.Consider dynamically resolving the user table name:
♻️ Suggested improvement
def restore_old_table_from_new(apps, schema_editor): """Recreate old authtoken_token table and copy data back for rollback.""" + User = apps.get_model(settings.AUTH_USER_MODEL) + user_table = User._meta.db_table with schema_editor.connection.cursor() as cursor: cursor.execute( - """ + f""" CREATE TABLE IF NOT EXISTS authtoken_token ( key VARCHAR(40) PRIMARY KEY, created TIMESTAMP WITH TIME ZONE NOT NULL, - user_id INTEGER NOT NULL UNIQUE REFERENCES users_user(id) + user_id INTEGER NOT NULL UNIQUE REFERENCES {user_table}(id) ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED ) """ )front_end/src/app/(main)/aib/2026/spring/page.tsx (1)
19-22: Consider adding error handling for the API key fetch.If
ServerAuthApi.getApiKey()throws an error (e.g., network failure, authentication issue), this will cause an unhandled exception. The non-bot path withServerProfileApi.getBotToken()could have similar issues, but adding error handling here would improve robustness.♻️ Suggested improvement
if (user.is_bot) { - const { key } = await ServerAuthApi.getApiKey(); - return key; + try { + const { key } = await ServerAuthApi.getApiKey(); + return key; + } catch { + return null; + } }users/views.py (1)
6-7: Import ordering could be improved.The
ApiKeyimport fromauthentication.modelsis placed betweenrest_frameworkimports, which breaks the standard import grouping convention (stdlib → third-party → local). Consider moving it to the local imports section.♻️ Suggested reordering
from django.contrib.auth.password_validation import validate_password from django.utils import timezone from rest_framework import serializers, status -from authentication.models import ApiKey from rest_framework.decorators import api_view, permission_classes from rest_framework.exceptions import ValidationError from rest_framework.generics import get_object_or_404 from rest_framework.permissions import AllowAny, IsAdminUser from rest_framework.request import Request from rest_framework.response import Response +from authentication.models import ApiKey from users.models import User, UserSpamActivityauthentication/auth.py (1)
33-37: Consider debouncinglast_used_atupdates for high-traffic scenarios.The current implementation issues a database write on every authenticated API request. While the
filter().update()approach is efficient (single query, no model instantiation), this could become a performance bottleneck under high load.Consider one of these alternatives:
- Time-based debouncing: Only update if
last_used_atis older than a threshold (e.g., 1 hour)- Async update: Queue the update to happen asynchronously
- Sampling: Update on a percentage of requests
Example: Time-based debouncing approach
def authenticate_credentials(self, key): user, token = super().authenticate_credentials(key) - ApiKey.objects.filter(key=key).update(last_used_at=timezone.now()) + # Only update if last_used_at is older than 1 hour to reduce DB writes + one_hour_ago = timezone.now() - timezone.timedelta(hours=1) + ApiKey.objects.filter( + key=key, + last_used_at__lt=one_hour_ago + ).update(last_used_at=timezone.now()) return user, tokenNote: This would require handling the case where
last_used_atisNULL(first use):from django.db.models import Q ApiKey.objects.filter( Q(key=key) & (Q(last_used_at__isnull=True) | Q(last_used_at__lt=one_hour_ago)) ).update(last_used_at=timezone.now())authentication/views/common.py (1)
276-283: Add explicit permission decorator for consistency.This endpoint retrieves sensitive API key information and lacks an explicit
@permission_classesdecorator, which is inconsistent with the codebase pattern where all other endpoints declare their permission requirements explicitly. Add@permission_classes([IsAuthenticated])to improve clarity and align with project conventions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
authentication/admin.pyauthentication/auth.pyauthentication/migrations/0001_add_token_model.pyauthentication/migrations/__init__.pyauthentication/models.pyauthentication/urls.pyauthentication/views/common.pyfront_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.jsonfront_end/src/app/(main)/accounts/settings/account/components/api_access.tsxfront_end/src/app/(main)/accounts/settings/account/page.tsxfront_end/src/app/(main)/accounts/settings/actions.tsxfront_end/src/app/(main)/aib/2026/spring/page.tsxfront_end/src/services/api/auth/auth.server.tsmetaculus_web/settings.pyprojects/admin.pytests/unit/conftest.pyusers/models.pyusers/views.py
💤 Files with no reviewable changes (2)
- metaculus_web/settings.py
- authentication/admin.py
🧰 Additional context used
🧬 Code graph analysis (9)
front_end/src/app/(main)/accounts/settings/actions.tsx (1)
front_end/src/utils/core/errors.ts (1)
ApiError(22-74)
authentication/views/common.py (1)
authentication/models.py (1)
ApiKey(8-34)
authentication/urls.py (1)
authentication/views/common.py (2)
api_key_api_view(277-283)api_key_rotate_api_view(287-292)
front_end/src/app/(main)/accounts/settings/account/components/api_access.tsx (2)
front_end/src/app/(main)/accounts/settings/actions.tsx (1)
rotateApiKeyAction(138-154)front_end/src/components/ui/form_field.tsx (1)
Input(111-128)
authentication/auth.py (1)
authentication/models.py (1)
ApiKey(8-34)
users/models.py (1)
authentication/models.py (1)
ApiKey(8-34)
projects/admin.py (1)
authentication/models.py (1)
ApiKey(8-34)
tests/unit/conftest.py (1)
authentication/models.py (1)
ApiKey(8-34)
users/views.py (1)
authentication/models.py (1)
ApiKey(8-34)
🪛 Ruff (0.14.11)
authentication/migrations/0001_add_token_model.py
8-8: Unused function argument: apps
(ARG001)
29-29: Unused function argument: apps
(ARG001)
56-58: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
60-84: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Frontend Checks
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
🔇 Additional comments (24)
front_end/messages/pt.json (1)
1783-1790: LGTM!The Portuguese translations for the new API key management features are grammatically correct and consistent with the feature requirements. The translations appropriately convey the rotation and creation workflows.
authentication/models.py (1)
8-34: LGTM!The
ApiKeymodel implementation is sound:
- Uses cryptographically secure
os.urandom(20)for key generation (160 bits of entropy)OneToOneFieldcorrectly enforces one API key per userCASCADEdelete behavior is appropriate for user cleanup- Auto-generation in
save()is a clean patternThe implementation correctly mirrors DRF's Token model while adding the
last_used_attracking capability.users/models.py (2)
11-11: LGTM!Import correctly updated to use the new
ApiKeymodel from the authentication app.
250-251: LGTM!The cleanup logic correctly uses
ApiKey.objects.filter(user=self).delete()to remove the user's API key during data deletion. This pattern safely handles cases where no API key exists.projects/admin.py (3)
13-13: LGTM!Import correctly updated to use the new
ApiKeymodel.
564-564: LGTM!API key creation correctly uses the new
ApiKeymodel. The key will be auto-generated via the model'ssave()method.
622-622: LGTM!Consistent with the change on line 564 - correctly uses
ApiKeyfor the complex password user generation action.tests/unit/conftest.py (1)
5-5: LGTM!Import correctly updated to use the new
ApiKeymodel.front_end/messages/zh.json (1)
1787-1794: LGTM!The Simplified Chinese translations for API key management are accurate and well-formed. The terminology is consistent throughout (using "轮换" for rotate and "API 密钥" for API key), and the warning message in
rotateApiKeyDescriptionproperly conveys the impact of rotating the key.front_end/messages/cs.json (1)
1785-1792: LGTM!The Czech translations for API key management are accurate and grammatically correct. The terminology is consistent throughout (using "Rotovat" for rotate and "API klíč" for API key), and the warning message in
rotateApiKeyDescriptionproperly conveys the impact of key rotation.front_end/messages/en.json (1)
1809-1816: LGTM!The new internationalization entries for API key management are clear, descriptive, and follow the existing naming conventions. The warning message in
rotateApiKeyDescriptionappropriately informs users about the consequences of rotating their API key.authentication/migrations/0001_add_token_model.py (1)
60-84: LGTM!The migration structure is correct:
- Creates the
ApiKeymodel first with appropriate fields matching the DRF Token structure plus the newlast_used_atfield- Then runs the data migration to copy existing tokens
The
appsparameter in RunPython functions is required by Django's signature even when unused, so the static analysis hints are false positives.users/views.py (2)
236-239: LGTM!The migration from
TokentoApiKeyis correctly implemented. Theget_or_createpattern and response format remain unchanged, ensuring backward compatibility for API consumers.
286-289: LGTM!Same pattern as
create_bot_api_view- clean replacement ofTokenwithApiKeywhile maintaining the same interface.front_end/messages/es.json (1)
1785-1792: LGTM!The Spanish translations for the API key management UI are complete and grammatically correct. The keys align with the frontend components that handle API key creation and rotation flows.
front_end/src/app/(main)/accounts/settings/account/page.tsx (1)
16-29: LGTM!The API key fetching is correctly integrated. The
apiKeycan benullwhen a user hasn't created one yet, which aligns with the UI flow that allows key creation. The server component context ensures the user is authenticated before reaching this code.front_end/src/app/(main)/accounts/settings/actions.tsx (1)
120-154: LGTM!Both
getApiKeyActionandrotateApiKeyActionfollow the established error handling pattern used throughout this file. The consistent approach of returning{ key }on success and{ errors: err.data }onApiErrormaintains uniformity with other actions likechangePasswordandcreateBot.front_end/src/services/api/auth/auth.server.ts (1)
131-138: LGTM!The new API methods are well-typed and follow the existing patterns in
ServerAuthApiClass. The return type distinction (key: string | nullforgetApiKeyvskey: stringforrotateApiKey) correctly reflects the backend behavior where retrieval may return null for users without a key, while rotation always produces a valid key.authentication/auth.py (1)
8-14: LGTM!Setting
model = ApiKeycorrectly configures theTokenAuthenticationbase class to use the customApiKeymodel instead of DRF's defaultToken. This is the standard approach for customizing token models in Django REST Framework.front_end/messages/zh-TW.json (1)
1782-1789: LGTM!The new translation keys for API key management are complete and align well with the UI functionality. The Traditional Chinese translations appear semantically correct for the create/rotate key flows.
authentication/views/common.py (3)
16-16: LGTM!The import change from
TokentoApiKeyis consistent with the model migration, and all usages throughout the file have been updated accordingly.
65-67: LGTM!The migration from
TokentoApiKeymaintains backward compatibility by preserving thetokenkey in the response payload while using the newApiKeymodel internally.
286-291: Rotation logic is correct; consider explicit permission decorator.The delete-then-create pattern correctly handles both creating a new key (when none exists) and rotating an existing key. The
filter().delete()is safe when no key exists. Same suggestion as above: consider adding explicit@permission_classes([IsAuthenticated])for this security-sensitive endpoint.front_end/src/app/(main)/accounts/settings/account/components/api_access.tsx (1)
82-139: LGTM!The conditional UI rendering is well-structured with appropriate accessibility labels. The confirmation modal for key rotation provides good UX for this potentially disruptive action. The visibility toggle and copy functionality follow expected patterns.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
front_end/src/app/(main)/accounts/settings/account/components/api_access.tsx
Show resolved
Hide resolved
front_end/src/app/(main)/accounts/settings/account/components/api_access.tsx
Show resolved
Hide resolved
front_end/src/app/(main)/accounts/settings/account/components/api_access.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@authentication/migrations/0001_add_token_model.py`:
- Around line 37-59: The rollback creates authtoken_token with user_id hardcoded
as INTEGER which mismatches a BigAutoField primary key; update
restore_old_table_from_new to dynamically determine the user PK DB type (use
apps.get_model(...) to get the user model and obtain its _meta.pk or use
schema_editor.connection.introspection or the PK field's
db_type(schema_editor.connection)) and interpolate that resolved type into the
CREATE TABLE user_id column and its REFERENCES clause, and also update
migrate_tokens_and_drop_old_table to call get_user_table_name (and/or return the
resolved PK type) accordingly so the created foreign key uses the correct BIGINT
(or actual) type instead of INTEGER.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
authentication/migrations/0001_add_token_model.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-15T19:29:58.940Z
Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4075
File: authentication/urls.py:24-26
Timestamp: 2026-01-15T19:29:58.940Z
Learning: In this codebase, DRF is configured to use IsAuthenticated as the default in REST_FRAMEWORK['DEFAULT_PERMISSION_CLASSES'] within metaculus_web/settings.py. Therefore, explicit permission_classes([IsAuthenticated]) decorators are unnecessary on DRF views unless a view needs to override the default. When reviewing Python files, verify that views relying on the default are not redundantly decorated, and flag cases where permissions are being over-specified or when a non-default permission is explicitly required.
Applied to files:
authentication/migrations/0001_add_token_model.py
🪛 Ruff (0.14.11)
authentication/migrations/0001_add_token_model.py
8-8: Unused function argument: apps
(ARG001)
29-29: Unused function argument: apps
(ARG001)
56-58: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
60-84: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
- GitHub Check: Frontend Checks
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
authentication/migrations/0001_add_token_model.py (3)
8-12: LGTM - Dynamic user table resolution is properly implemented.This helper correctly resolves the user model's table name from
settings.AUTH_USER_MODELand properly quotes it using Django's connection ops, ensuring compatibility with custom user models and preventing SQL injection.Note: The static analysis warning (ARG001) about unused
appsis a false positive—appsis used on line 11.
15-34: LGTM - Forward migration handles data transfer correctly.The forward migration properly:
- Checks for the existence of
authtoken_tokenbefore operating (supports fresh installs).- Copies tokens with
ON CONFLICT DO NOTHINGto handle any edge cases.- Drops the old table after successful data transfer.
The unused
appsparameter (flagged by Ruff) is required by Django'sRunPythonsignature and is not a concern.
62-94: LGTM - Migration class structure is correct.The migration:
- Properly declares
initial = Truefor the first migration in the app.- Correctly specifies
swappable_dependencyto handle custom user models.- Orders operations correctly—
CreateModelruns beforeRunPython, ensuring theauthentication_apikeytable exists before data migration.- The
ApiKeymodel definition includes all required fields including the newlast_used_atfield.The Ruff warnings (RUF012) about mutable class attributes are false positives—
dependenciesandoperationsare standard Django migration patterns.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
elisescu
left a comment
elisescu
left a comment
There was a problem hiding this comment.
LGTM, only had one minor comment. Pls have a look
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@authentication/migrations/0001_add_token_model.py`:
- Around line 29-38: The reverse migration function restore_old_table_from_new
should guard against the absence of authtoken_token before running the INSERT;
update restore_old_table_from_new to first check for the existence of the
authtoken_token table (using the same approach as the forward migration, e.g.
inspect schema_editor.connection or run a quick existence query such as checking
to_regclass('public.authtoken_token')) and only execute the INSERT INTO
authtoken_token ... SELECT ... WHEN the table exists; if the table is missing,
simply return/skip to avoid raising an error during rollback.
🧹 Nitpick comments (1)
authentication/migrations/0001_add_token_model.py (1)
8-10: Function name/docstring implies a drop that never happens.
migrate_tokens_and_drop_old_tableonly copies data; there is no DROP. Consider renaming to match behavior (and update the RunPython reference) or implement the drop if that’s intended.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
authentication/migrations/0001_add_token_model.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-15T19:29:58.940Z
Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4075
File: authentication/urls.py:24-26
Timestamp: 2026-01-15T19:29:58.940Z
Learning: In this codebase, DRF is configured to use IsAuthenticated as the default in REST_FRAMEWORK['DEFAULT_PERMISSION_CLASSES'] within metaculus_web/settings.py. Therefore, explicit permission_classes([IsAuthenticated]) decorators are unnecessary on DRF views unless a view needs to override the default. When reviewing Python files, verify that views relying on the default are not redundantly decorated, and flag cases where permissions are being over-specified or when a non-default permission is explicitly required.
Applied to files:
authentication/migrations/0001_add_token_model.py
🪛 Ruff (0.14.11)
authentication/migrations/0001_add_token_model.py
8-8: Unused function argument: apps
(ARG001)
29-29: Unused function argument: apps
(ARG001)
45-47: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
49-73: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Frontend Checks
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
- GitHub Check: Analyze (javascript-typescript)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
3 participants
Tokenmodel into the Authentication app and renamed it toApiKeylast_used_atfield to track token usageThe migration handles existing
authtoken_tokendata and supports rollback. Translations were added for all supported languages.Part of #3977
Summary by CodeRabbit
New Features
UI / Localization
Chores / Migration
✏️ Tip: You can customize this high-level summary in your review settings.