-
Notifications
You must be signed in to change notification settings - Fork 27
Auth Integration V1 #4087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Auth Integration V1 #4087
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
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces ApiKey/server-session auth with JWT access/refresh tokens across backend and frontend: adds get_tokens_for_user, Simple JWT config and refresh endpoint; introduces AuthCookieManager, single‑flight refresh, legacy migration, middleware changes, proxy/header updates, API client updates, types, and test adjustments. 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 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. Comment |
…s' into feat/frontend-auth-refresh-tokens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@metaculus_web/settings.py`:
- Around line 150-155: DEFAULT_AUTHENTICATION_CLASSES currently lists
"rest_framework.authentication.SessionAuthentication" before
"rest_framework_simplejwt.authentication.JWTAuthentication", which causes
SessionAuthentication to run first and block JWT auth (and raise CSRF errors);
reorder the list so "rest_framework_simplejwt.authentication.JWTAuthentication"
appears before "rest_framework.authentication.SessionAuthentication" in
DEFAULT_AUTHENTICATION_CLASSES (leaving
"authentication.auth.FallbackTokenAuthentication" as-is) so JWTAuthentication is
attempted prior to SessionAuthentication.
📜 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). (4)
- GitHub Check: integration-tests
- GitHub Check: Frontend Checks
- GitHub Check: Backend Checks
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
metaculus_web/settings.py (2)
16-23: Imports look fine for the JWT additions.
No concerns here.
166-201: [rewritten comment]
[exactly one classification tag]
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| 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, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note: we’ll use our custom RS256 private key in production for asymmetric encryption. If JWT_PRIVATE_KEY isn’t specified, it will fall back to the Django SECRET_KEY.
…s' into feat/frontend-auth-refresh-tokens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>
cemreinanc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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.