Conversation
# Conflicts: # poetry.lock
# Conflicts: # authentication/views/common.py # front_end/src/app/(main)/accounts/settings/account/page.tsx # front_end/src/app/(main)/aib/2026/spring/page.tsx
📝 WalkthroughWalkthroughReplaces per-user ApiKey/server-session auth with JWT access/refresh tokens and cookie-based session management across backend and frontend; adds token issuance, refresh, verification, middleware changes, proxy/header updates, single‑flight refresh, legacy migration, types, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser/Client
participant APIProxy as API Proxy Route
participant AuthMgr as AuthCookieManager
participant RefreshSvc as refreshWithSingleFlight
participant Backend as Backend API
Client->>APIProxy: Request to /api/...
APIProxy->>AuthMgr: getAuthCookieManager()
AuthMgr->>AuthMgr: read access/refresh tokens from cookies
APIProxy->>RefreshSvc: refreshWithSingleFlight(refreshToken)
alt Refresh performed
RefreshSvc->>Backend: POST /auth/refresh/
Backend->>RefreshSvc: New tokens
RefreshSvc->>AuthMgr: setAuthTokens(newTokens)
else Cached / no-op
RefreshSvc-->>APIProxy: null or cached result
end
APIProxy->>APIProxy: buildHeaders(accessToken)
APIProxy->>Backend: Proxied request with Authorization header
alt Response 401
APIProxy->>RefreshSvc: refreshWithSingleFlight(refreshToken)
RefreshSvc->>Backend: POST /auth/refresh/
Backend->>RefreshSvc: New tokens
APIProxy->>Backend: Retry request with new token
end
Backend->>APIProxy: Response
APIProxy->>AuthMgr: Write refreshed tokens to cookies
APIProxy->>Client: Response
sequenceDiagram
participant Req as Incoming Request
participant Middleware as Middleware
participant AuthReader as AuthCookieReader
participant AuthMgr as AuthCookieManager
participant ServerAuth as ServerAuthApi
participant Backend as Backend API
Req->>Middleware: Next.js Request
Middleware->>AuthReader: read tokens from request cookies
Middleware->>Middleware: handleLegacyTokenMigration()
alt Legacy token swapped
Middleware->>Backend: POST /auth/exchange-legacy-token/
Backend->>Middleware: New JWT tokens
Middleware->>AuthMgr: setAuthTokens(newTokens)
end
Middleware->>Middleware: refreshTokensIfNeeded()
alt Access expired & refresh exists
Middleware->>ServerAuth: POST /auth/refresh/
ServerAuth->>Backend: validate & rotate
Backend->>ServerAuth: New tokens
Middleware->>AuthMgr: setAuthTokens(newTokens)
else Verify token via backend
Middleware->>Backend: token verification request
end
Middleware->>AuthMgr: Write final tokens to response cookies
Middleware->>Req: Continue to app
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
…s' into feat/frontend-auth-refresh-tokens
…s' into feat/frontend-auth-refresh-tokens
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@metaculus_web/settings.py`:
- Around line 22-23: The settings module imports cryptography symbols
(serialization, load_pem_private_key) but cryptography is only pulled in as an
optional extra of djangorestframework-simplejwt; add cryptography as an explicit
direct dependency in pyproject.toml under [tool.poetry.dependencies] (or
alternatively pin djangorestframework-simplejwt with the crypto extra, e.g.,
djangorestframework-simplejwt[crypto]) so that imports in metaculus_web.settings
(serialization, load_pem_private_key) will always succeed at import time.
♻️ Duplicate comments (1)
metaculus_web/settings.py (1)
149-155: JWT should authenticate before SessionAuthentication to avoid CSRF blocks.Line 150–154: when SessionAuthentication runs first, it can enforce CSRF and short‑circuit JWT auth for SPA calls that include a session cookie. Reorder so JWT runs first.
♻️ Proposed reordering
"DEFAULT_AUTHENTICATION_CLASSES": [ - "rest_framework.authentication.SessionAuthentication", - # Primary auth mechanism for web users - "rest_framework_simplejwt.authentication.JWTAuthentication", + # Primary auth mechanism for web users + "rest_framework_simplejwt.authentication.JWTAuthentication", + "rest_framework.authentication.SessionAuthentication", # Auth Token: should be used for bots only! "authentication.auth.FallbackTokenAuthentication", ],
🧹 Nitpick comments (1)
metaculus_web/settings.py (1)
171-201: Add a clearer failure mode for invalidJWT_PRIVATE_KEY.Line 171–191: if
JWT_PRIVATE_KEYis set but malformed/encrypted, settings import will crash with a low‑signal error. Consider raisingImproperlyConfiguredwith a clear message.🛠️ Suggested hardening
-from django.core.exceptions import DisallowedHost +from django.core.exceptions import DisallowedHost, ImproperlyConfigured ... def get_jwt_encryption_config(): private_key_pem = os.environ.get("JWT_PRIVATE_KEY", "").replace("\\n", "\n") if not private_key_pem: # Fallback to HS256 with SECRET_KEY return {"ALGORITHM": "HS256", "SIGNING_KEY": SECRET_KEY, "VERIFYING_KEY": None} - private_key = load_pem_private_key(private_key_pem.encode(), password=None) + try: + private_key = load_pem_private_key(private_key_pem.encode(), password=None) + except Exception as exc: + raise ImproperlyConfigured( + "JWT_PRIVATE_KEY must be a valid PEM-encoded RSA private key." + ) from exc
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
metaculus_web/settings.py
🧰 Additional context used
🧠 Learnings (2)
📚 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:
metaculus_web/settings.py
📚 Learning: 2026-01-16T20:30:15.454Z
Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4087
File: authentication/views/social.py:47-55
Timestamp: 2026-01-16T20:30:15.454Z
Learning: In the Metaculus codebase, `SocialTokenOnlyAuthView` from the `rest_social_auth` library uses `AllowAny` permission by default, so views inheriting from it (like `SocialCodeAuth` in `authentication/views/social.py`) do not need to explicitly set `permission_classes = (AllowAny,)` for OAuth code exchange to work with unauthenticated requests.
Applied to files:
metaculus_web/settings.py
⏰ 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). (1)
- GitHub Check: integration-tests
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Co-authored-by: Nikita <93587872+ncarazon@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@front_end/src/middleware.ts`:
- Around line 41-47: The current refresh flow in middleware uses
ServerAuthApi.refreshTokens and returns false on any thrown error, which causes
the caller to treat it as verification failure and clear cookies; instead change
the refresh handling to return a tri-state result (e.g., "ok" with tokens,
"failed" for transient refresh errors, and "invalid" for definitive failures) so
callers can distinguish a transient refresh failure from an invalid token;
update the block around ServerAuthApi.refreshTokens to catch errors, log them,
and return a distinct "failed" state (not false) when refresh throws, and ensure
the caller code that checks the returned value skips access-token verification
and cookie clearing in the "failed" case but continues normal flow for "ok" and
"invalid" states.
- Around line 64-74: After a legacy token migration (handled by
handleLegacyTokenMigration) requestAuth still reflects pre-migration cookies so
subsequent refresh/verify logic may run against stale tokens and clear the
newly-set cookies; update the middleware to detect wasMigrated and either skip
the refresh/verify steps when wasMigrated is true or immediately rebuild the
auth reader from response cookies (responseAuth) before calling refresh/verify
so they operate on the migrated tokens; ensure this guard is applied in both the
initial migration block around handleLegacyTokenMigration and the analogous
block later (lines 106-116) where refresh/verify runs.
- Around line 15-22: The verifyToken function currently catches all errors and
unconditionally clears auth cookies; change it to only clear cookies for
explicit auth failures (HTTP 401/403) by detecting ApiError from
ServerAuthApi.verifyToken and inspecting its status/code, while letting other
errors (network/5xx) bubble or be retried; import ApiError from
"@/utils/core/errors" and update the catch block in verifyToken to check for
instances of ApiError (or status properties) and call
responseAuth.clearAuthTokens() only when status === 401 || status === 403,
otherwise rethrow or handle as transient.
In `@metaculus_web/settings.py`:
- Around line 171-201: Remove the unused custom simplejwt keys
CHECK_REVOKE_TOKEN and REVOKE_TOKEN_CLAIM from the SIMPLE_JWT dict in
settings.py (they are not recognized by rest_framework_simplejwt); if you intend
to support token revocation instead, add
rest_framework_simplejwt.token_blacklist to INSTALLED_APPS and use the supported
BLACKLIST_AFTER_ROTATION key inside SIMPLE_JWT (adjust values there) rather than
the removed keys.
♻️ Duplicate comments (1)
front_end/src/app/(api)/api-proxy/[...path]/route.ts (1)
73-92: LGTM -HeadersInittype annotation issue addressed.The explicit
HeadersInittype annotation was correctly removed, allowing TypeScript to infer the properHeaderstype. This enables the.set()and.delete()method calls to type-check correctly.
🧹 Nitpick comments (2)
metaculus_web/settings.py (1)
171-191: Consider adding error handling for malformed JWT private keys.If
JWT_PRIVATE_KEYcontains a malformed PEM key,load_pem_private_keywill raise aValueErrorat Django startup with a cryptic error message. While fail-fast is appropriate, a descriptive error would improve debuggability.Optional: Add descriptive error handling
def get_jwt_encryption_config(): private_key_pem = os.environ.get("JWT_PRIVATE_KEY", "").replace("\\n", "\n") if not private_key_pem: # Fallback to HS256 with SECRET_KEY return {"ALGORITHM": "HS256", "SIGNING_KEY": SECRET_KEY, "VERIFYING_KEY": None} - private_key = load_pem_private_key(private_key_pem.encode(), password=None) + try: + private_key = load_pem_private_key(private_key_pem.encode(), password=None) + except (ValueError, TypeError) as e: + raise ValueError( + f"Invalid JWT_PRIVATE_KEY: {e}. Ensure it's a valid unencrypted PEM-formatted RSA private key." + ) from e public_key_pem = (front_end/src/app/(api)/api-proxy/[...path]/route.ts (1)
110-121: Consider edge case with refresh token rotation.If the proactive refresh (lines 97-104) succeeds but the request still returns 401, the fallback at line 112 retrieves the refresh token from
authManager, which holds the original token from the request cookies—not the potentially rotated token fromrefreshedTokens.refresh.With refresh token rotation policies (where the backend invalidates the old refresh token upon issuing new ones), this second refresh attempt could fail. The current behavior is acceptable (returns 401 to client), but if you want stricter handling:
💡 Optional improvement to use the latest refresh token
// Fallback: retry on 401 (in case proactive check missed edge cases) if (response.status === 401 && shouldPassAuth) { - const refreshToken = authManager.getRefreshToken(); + const refreshToken = refreshedTokens?.refresh ?? authManager.getRefreshToken(); if (refreshToken) { const newTokens = await refreshWithSingleFlight(refreshToken);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
front_end/src/app/(api)/api-proxy/[...path]/route.tsfront_end/src/middleware.tsmetaculus_web/settings.pytests/unit/test_auth/test_views.py
🧰 Additional context used
🧠 Learnings (3)
📚 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:
tests/unit/test_auth/test_views.pymetaculus_web/settings.py
📚 Learning: 2026-01-19T16:13:59.519Z
Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4087
File: metaculus_web/settings.py:150-155
Timestamp: 2026-01-19T16:13:59.519Z
Learning: In the Metaculus codebase `metaculus_web/settings.py`, `SessionAuthentication` is intentionally listed before `JWTAuthentication` in `REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"]` because session auth is expected to be used specifically for the Django admin dashboard, while JWT handles the primary web user authentication.
Applied to files:
metaculus_web/settings.py
📚 Learning: 2026-01-16T20:30:29.385Z
Learnt from: hlbmtc
Repo: Metaculus/metaculus PR: 4087
File: authentication/views/social.py:47-55
Timestamp: 2026-01-16T20:30:29.385Z
Learning: In the Metaculus codebase, `SocialTokenOnlyAuthView` from the `rest_social_auth` library uses `AllowAny` permission by default, so views inheriting from it (like `SocialCodeAuth` in `authentication/views/social.py`) do not need to explicitly set `permission_classes = (AllowAny,)` for OAuth code exchange to work with unauthenticated requests.
Applied to files:
metaculus_web/settings.py
🧬 Code graph analysis (1)
front_end/src/app/(api)/api-proxy/[...path]/route.ts (3)
front_end/src/services/auth_tokens.ts (2)
AuthTokens(8-8)AuthCookieManager(120-156)front_end/src/types/auth.ts (1)
AuthTokens(8-11)front_end/src/services/auth_refresh.ts (1)
refreshWithSingleFlight(26-45)
⏰ 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: Build Docker Image
- GitHub Check: Frontend Checks
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
🔇 Additional comments (11)
tests/unit/test_auth/test_views.py (3)
29-33: Looks good — tokens absent for inactive signup.
53-57: Looks good — tokens present for active signup.
101-104: Looks good — tokens present for invitation signup.metaculus_web/settings.py (3)
16-16: LGTM!Standard library import needed for the JWT token lifetime configuration below.
22-23: LGTM!Cryptography imports are correctly used for RSA key handling in the JWT configuration. The dependency addition was addressed in a previous commit.
150-156: LGTM!The authentication class ordering is intentional per the previous discussion—
SessionAuthenticationfirst for Django admin dashboard, followed byJWTAuthenticationfor primary web user authentication. Based on learnings, this design decision has been confirmed.front_end/src/app/(api)/api-proxy/[...path]/route.ts (4)
4-10: LGTM!The imports are well-organized, bringing in the necessary authentication services (
refreshWithSingleFlight,AuthCookieManager,getAuthCookieManager) to support the new cookie-based JWT auth flow.
94-105: Well-designed proactive token refresh pattern.The proactive refresh check before making the request reduces unnecessary round-trips. Using
refreshWithSingleFlightcorrectly prevents duplicate concurrent refresh requests. The fallback at line 111 handles edge cases where the proactive check might miss.
135-145: LGTM!The response handling correctly persists refreshed tokens to cookies via
AuthCookieManager. UsingnextResponse.cookies(which is writable) to construct the manager ensures tokens are properly set on the outgoing response.
148-167: Unchanged utility function - no concerns.The
processContentDispositionHeaderfunction remains unchanged and continues to correctly handle filename normalization for Content-Disposition headers.front_end/src/middleware.ts (1)
128-135: Matcher exclusion forapp-versionlooks good.Keeps middleware off that path while preserving existing exclusions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| async function verifyToken(responseAuth: AuthCookieManager): Promise<void> { | ||
| try { | ||
| await ServerAuthApi.verifyToken(); | ||
| } catch { | ||
| // Token is invalid (user banned, token revoked, etc.) - clear all auth cookies | ||
| console.error("Token verification failed, clearing auth cookies"); | ||
| responseAuth.clearAuthTokens(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find front_end/src -type f -name "*.ts" -o -name "*.js" | head -20Repository: Metaculus/metaculus
Length of output: 1377
🏁 Script executed:
fd -t f "auth" front_end/src --type f | grep -E "\.(ts|js)$" | head -20Repository: Metaculus/metaculus
Length of output: 262
🏁 Script executed:
rg -l "ServerAuthApi" front_end/src --type ts --type jsRepository: Metaculus/metaculus
Length of output: 637
🏁 Script executed:
cat -n front_end/src/services/api/auth/auth.server.tsRepository: Metaculus/metaculus
Length of output: 5318
🏁 Script executed:
cat -n front_end/src/middleware.tsRepository: Metaculus/metaculus
Length of output: 5549
🏁 Script executed:
rg -n "class ApiService" front_end/src --type ts -A 30Repository: Metaculus/metaculus
Length of output: 2127
🏁 Script executed:
cat -n front_end/src/services/api/api_service.ts | head -100Repository: Metaculus/metaculus
Length of output: 1325
🏁 Script executed:
rg -n "serverFetcher" front_end/src --type ts -lRepository: Metaculus/metaculus
Length of output: 744
🏁 Script executed:
cat -n front_end/src/utils/core/fetch/fetch.server.tsRepository: Metaculus/metaculus
Length of output: 3537
🏁 Script executed:
cat -n front_end/src/utils/core/fetch/fetch.shared.tsRepository: Metaculus/metaculus
Length of output: 4070
🏁 Script executed:
rg -n "class ApiError" front_end/src --type ts -A 20Repository: Metaculus/metaculus
Length of output: 1930
🏁 Script executed:
fd -t f "error" front_end/src/utils/core --type f | grep -E "\.(ts|js)$"Repository: Metaculus/metaculus
Length of output: 97
🏁 Script executed:
cat -n front_end/src/utils/core/errors.tsRepository: Metaculus/metaculus
Length of output: 7718
🏁 Script executed:
rg -n "401|403" front_end/src --type ts -B2 -A2 | head -50Repository: Metaculus/metaculus
Length of output: 4599
🏁 Script executed:
rg -n "isAuthFailure|isAuthError|auth.*error" front_end/src --type tsRepository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
rg -n "instanceof ApiError|ApiError.isApiError" front_end/src --type ts -B2 -A2Repository: Metaculus/metaculus
Length of output: 16149
🏁 Script executed:
rg -n "catch.*error.*status" front_end/src --type ts -B2 -A2 | head -40Repository: Metaculus/metaculus
Length of output: 45
🏁 Script executed:
cat -n front_end/src/middleware.ts | sed -n '29,51p'Repository: Metaculus/metaculus
Length of output: 831
🏁 Script executed:
rg -n "refreshTokens|verifyToken" front_end/src --type ts -B3 -A5 | grep -A10 "catch"Repository: Metaculus/metaculus
Length of output: 2888
Distinguish auth failures from transient errors before clearing cookies.
Catching all errors and clearing tokens can log users out on network errors or 5xx failures. Use the error status code to clear cookies only on explicit auth failures (401/403), letting transient errors be retried:
🛠️ Proposed fix
- } catch {
- // Token is invalid (user banned, token revoked, etc.) - clear all auth cookies
- console.error("Token verification failed, clearing auth cookies");
- responseAuth.clearAuthTokens();
- }
+ } catch (error) {
+ // Clear only for explicit auth failures; keep cookies on transient errors.
+ if (ApiError.isApiError(error) && (error.response.status === 401 || error.response.status === 403)) {
+ console.error("Token verification failed, clearing auth cookies");
+ responseAuth.clearAuthTokens();
+ } else {
+ console.error("Token verification failed (non-auth error):", error);
+ }
+ }You'll need to import ApiError from @/utils/core/errors.
🤖 Prompt for AI Agents
In `@front_end/src/middleware.ts` around lines 15 - 22, The verifyToken function
currently catches all errors and unconditionally clears auth cookies; change it
to only clear cookies for explicit auth failures (HTTP 401/403) by detecting
ApiError from ServerAuthApi.verifyToken and inspecting its status/code, while
letting other errors (network/5xx) bubble or be retried; import ApiError from
"@/utils/core/errors" and update the catch block in verifyToken to check for
instances of ApiError (or status properties) and call
responseAuth.clearAuthTokens() only when status === 401 || status === 403,
otherwise rethrow or handle as transient.
| let tokens; | ||
| try { | ||
| tokens = await ServerAuthApi.refreshTokens(refreshToken); | ||
| } catch (error) { | ||
| console.error("Middleware token refresh failed:", error); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Avoid logging users out when refresh fails.
If refresh throws, the function returns false, which then triggers token verification on an expired access token and clears cookies. That logs users out on transient refresh errors. Return a distinct “failed” state and skip verification in that case so the refresh token can be retried.
🛠️ Proposed fix (tri-state refresh result)
-async function refreshTokensIfNeeded(
+async function refreshTokensIfNeeded(
requestAuth: AuthCookieReader,
responseAuth: AuthCookieManager
-): Promise<boolean> {
+): Promise<"refreshed" | "not_needed" | "failed"> {
const refreshToken = requestAuth.getRefreshToken();
// No refresh token = can't refresh
- if (!refreshToken) return false;
+ if (!refreshToken) return "not_needed";
// Access token still valid = no refresh needed
- if (!requestAuth.isAccessTokenExpired()) return false;
+ if (!requestAuth.isAccessTokenExpired()) return "not_needed";
let tokens;
try {
tokens = await ServerAuthApi.refreshTokens(refreshToken);
} catch (error) {
console.error("Middleware token refresh failed:", error);
- return false;
+ return "failed";
}
responseAuth.setAuthTokens(tokens);
- return true;
+ return "refreshed";
}- if (hasSession) {
- const tokensRefreshed = await refreshTokensIfNeeded(
- requestAuth,
- responseAuth
- );
- // Skip verification if tokens were just refreshed (they're valid by definition)
- // Only verify existing tokens to catch banned users or revoked tokens
- if (!tokensRefreshed) {
- await verifyToken(responseAuth);
- }
- }
+ if (hasSession) {
+ const refreshResult = await refreshTokensIfNeeded(
+ requestAuth,
+ responseAuth
+ );
+ // Skip verification if tokens were just refreshed (they're valid by definition)
+ // Only verify existing tokens to catch banned users or revoked tokens
+ if (refreshResult === "not_needed") {
+ await verifyToken(responseAuth);
+ }
+ }Also applies to: 106-116
🤖 Prompt for AI Agents
In `@front_end/src/middleware.ts` around lines 41 - 47, The current refresh flow
in middleware uses ServerAuthApi.refreshTokens and returns false on any thrown
error, which causes the caller to treat it as verification failure and clear
cookies; instead change the refresh handling to return a tri-state result (e.g.,
"ok" with tokens, "failed" for transient refresh errors, and "invalid" for
definitive failures) so callers can distinguish a transient refresh failure from
an invalid token; update the block around ServerAuthApi.refreshTokens to catch
errors, log them, and return a distinct "failed" state (not false) when refresh
throws, and ensure the caller code that checks the returned value skips
access-token verification and cookie clearing in the "failed" case but continues
normal flow for "ok" and "invalid" states.
| // DEPRECATED: Legacy token migration - remove after 30-day grace period | ||
| // Must run before auth checks so users with legacy tokens aren't rejected | ||
| const wasMigrated = await handleLegacyTokenMigration( | ||
| request, | ||
| response, | ||
| requestAuth, | ||
| responseAuth | ||
| ); | ||
| if (wasMigrated) { | ||
| hasSession = true; | ||
| } |
There was a problem hiding this comment.
Skip refresh/verify immediately after legacy migration.
After a legacy migration, requestAuth still reflects the pre-migration cookies. Refresh/verify can run against stale tokens and clear the newly set cookies. Consider skipping refresh/verify when wasMigrated is true (or rebuild the auth reader from the response cookies if possible).
🛠️ Suggested guard
const wasMigrated = await handleLegacyTokenMigration(
request,
response,
requestAuth,
responseAuth
);
if (wasMigrated) {
hasSession = true;
}
...
- if (hasSession) {
+ if (hasSession) {
const tokensRefreshed = await refreshTokensIfNeeded(
requestAuth,
responseAuth
);
// Skip verification if tokens were just refreshed (they're valid by definition)
// Only verify existing tokens to catch banned users or revoked tokens
- if (!tokensRefreshed) {
+ if (!tokensRefreshed && !wasMigrated) {
await verifyToken(responseAuth);
}
}Also applies to: 106-116
🤖 Prompt for AI Agents
In `@front_end/src/middleware.ts` around lines 64 - 74, After a legacy token
migration (handled by handleLegacyTokenMigration) requestAuth still reflects
pre-migration cookies so subsequent refresh/verify logic may run against stale
tokens and clear the newly-set cookies; update the middleware to detect
wasMigrated and either skip the refresh/verify steps when wasMigrated is true or
immediately rebuild the auth reader from response cookies (responseAuth) before
calling refresh/verify so they operate on the migrated tokens; ensure this guard
is applied in both the initial migration block around handleLegacyTokenMigration
and the analogous block later (lines 106-116) where refresh/verify runs.
| def get_jwt_encryption_config(): | ||
| private_key_pem = os.environ.get("JWT_PRIVATE_KEY", "").replace("\\n", "\n") | ||
|
|
||
| if not private_key_pem: | ||
| # Fallback to HS256 with SECRET_KEY | ||
| return {"ALGORITHM": "HS256", "SIGNING_KEY": SECRET_KEY, "VERIFYING_KEY": None} | ||
|
|
||
| private_key = load_pem_private_key(private_key_pem.encode(), password=None) | ||
| public_key_pem = ( | ||
| private_key.public_key() | ||
| .public_bytes( | ||
| encoding=serialization.Encoding.PEM, | ||
| format=serialization.PublicFormat.SubjectPublicKeyInfo, | ||
| ) | ||
| .decode() | ||
| ) | ||
| return { | ||
| "ALGORITHM": "RS256", | ||
| "SIGNING_KEY": private_key_pem, | ||
| "VERIFYING_KEY": public_key_pem, | ||
| } | ||
|
|
||
|
|
||
| SIMPLE_JWT = { | ||
| "ACCESS_TOKEN_LIFETIME": timedelta(minutes=15), | ||
| "REFRESH_TOKEN_LIFETIME": timedelta(days=30), | ||
| "ROTATE_REFRESH_TOKENS": True, | ||
| "CHECK_REVOKE_TOKEN": True, | ||
| "REVOKE_TOKEN_CLAIM": "hash", | ||
| **get_jwt_encryption_config(), | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for custom token implementations and settings usage
echo "=== Searching for CHECK_REVOKE_TOKEN and REVOKE_TOKEN_CLAIM usage ==="
rg -n "CHECK_REVOKE_TOKEN|REVOKE_TOKEN_CLAIM" --type py
echo -e "\n=== Searching for custom Token classes ==="
rg -n "class.*Token.*:" --type py -g '!*test*' -g '!*migration*' -A 5 | head -50
echo -e "\n=== Searching for simplejwt configuration or settings ==="
rg -n "SIMPLE_JWT" --type py -B 2 -A 15Repository: Metaculus/metaculus
Length of output: 2988
🌐 Web query:
djangorestframework-simplejwt 5.5.1 token revocation settings BLACKLIST
💡 Result:
There is no single "BLACKLIST" boolean setting. To use token revocation in djangorestframework-simplejwt v5.5.1 you must:
- Add the blacklist app to INSTALLED_APPS: 'rest_framework_simplejwt.token_blacklist'. [1][2]
- Control rotation/blacklisting via SIMPLE_JWT settings: ROTATE_REFRESH_TOKENS (issue new refresh tokens) and BLACKLIST_AFTER_ROTATION (add submitted refresh tokens to the blacklist when rotation is enabled). [1]
You can also programmatically blacklist a refresh token with RefreshToken(...).blacklist() and run the provided flushexpiredtokens management command to remove expired entries. [2]
Sources: documentation — Settings and Blacklist app. [1][2]
Remove unused simplejwt settings that don't exist in the library.
The settings CHECK_REVOKE_TOKEN and REVOKE_TOKEN_CLAIM are not part of djangorestframework-simplejwt and are not referenced anywhere in the codebase. They should be removed.
If token revocation is needed, use the standard simplejwt approach: add rest_framework_simplejwt.token_blacklist to INSTALLED_APPS and configure BLACKLIST_AFTER_ROTATION in SIMPLE_JWT instead.
🤖 Prompt for AI Agents
In `@metaculus_web/settings.py` around lines 171 - 201, Remove the unused custom
simplejwt keys CHECK_REVOKE_TOKEN and REVOKE_TOKEN_CLAIM from the SIMPLE_JWT
dict in settings.py (they are not recognized by rest_framework_simplejwt); if
you intend to support token revocation instead, add
rest_framework_simplejwt.token_blacklist to INSTALLED_APPS and use the supported
BLACKLIST_AFTER_ROTATION key inside SIMPLE_JWT (adjust values there) rather than
the removed keys.
3 participants
Details -- https://www.notion.so/metaculus/Auth-migration-roadmap-2e96aaf4f10180728140ddbdb51f5045?source=copy_link#2e96aaf4f10180599e5acc246d55a4ba
Part of #3977
Summary by CodeRabbit
New Features
Bug Fixes / UX
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.