fix: adds RSA tokens and adjusted logic #102
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe changes refactor Google OAuth authentication handling by shifting the callback GET endpoint to perform frontend redirects and adding a POST endpoint for backend processing. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
User->>Frontend: Initiates Google OAuth login
Frontend->>Backend: Redirects with code and state (GET /auth/google/callback)
Backend->>Frontend: Redirects to frontend callback URL with success/error (GET)
Frontend->>Backend: Sends code and state (POST /auth/google/callback)
Backend->>Frontend: Returns JSON with user info and tokens (POST)
Possibly related PRs
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Redundant Frontend URL Construction ▹ view | |
 |
Over-Generic Error Handling ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| todo/urls.py | ✅ |
| todo/views/task.py | ✅ |
| todo/views/auth.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
todo/views/auth.py (1)
105-150: Refactor repeated frontend callback URL construction.The
frontend_callbackURL is constructed 6 times in this method. This violates the DRY principle and makes maintenance harder.Apply this refactor to eliminate repetition:
def get(self, request: Request): code = request.query_params.get("code") state = request.query_params.get("state") error = request.query_params.get("error") + frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" if error: - frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" return HttpResponseRedirect(f"{frontend_callback}?error={error}") if not code: - frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" return HttpResponseRedirect(f"{frontend_callback}?error=missing_code") if not state: - frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" return HttpResponseRedirect(f"{frontend_callback}?error=missing_state") stored_state = request.session.get("oauth_state") if not stored_state or stored_state != state: - frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" return HttpResponseRedirect(f"{frontend_callback}?error=invalid_state") try: google_data = GoogleOAuthService.handle_callback(code) user = UserService.create_or_update_user(google_data) tokens = generate_google_token_pair( { "user_id": str(user.id), "google_id": user.google_id, "email": user.email_id, "name": user.name, } ) - frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" response = HttpResponseRedirect(f"{frontend_callback}?success=true") self._set_auth_cookies(response, tokens) request.session.pop("oauth_state", None) return response except Exception: - frontend_callback = f"{settings.FRONTEND_URL}/auth/callback" return HttpResponseRedirect(f"{frontend_callback}?error=auth_failed")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.env.example(1 hunks)todo/tests/unit/views/test_auth.py(5 hunks)todo/urls.py(0 hunks)todo/views/auth.py(5 hunks)todo/views/task.py(1 hunks)
💤 Files with no reviewable changes (1)
- todo/urls.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 in the Real-Dev-Squad/todo-backend repository comprehensively tracks user authentication implementation including registration, login, JWT tokens, and making task APIs require authentication. This covers replacing hardcoded user ID placeholders like "system_patch_user" with actual user ID extraction from authenticated requests.
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 tracks the implementation of user authentication in the todo-backend project, which includes extracting user ID from request context to replace hardcoded placeholders like "system_patch_user" in todo/views/task.py.
todo/views/task.py (2)
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:98-112
Timestamp: 2025-06-02T17:02:22.424Z
Learning: The todo-backend project uses a global exception handler that automatically handles TaskNotFoundException, BsonInvalidId, ValueError with ApiErrorResponse, and DRFValidationError. Views should avoid try-catch blocks and let exceptions bubble up to the global handler for consistent error formatting and status codes.
Learnt from: VaibhavSingh8
PR: Real-Dev-Squad/todo-backend#81
File: todo_project/settings/base.py:70-77
Timestamp: 2025-06-16T18:05:07.813Z
Learning: The todo-backend project uses a custom pagination configuration system with `DEFAULT_PAGINATION_SETTINGS` nested under `REST_FRAMEWORK` in Django settings. This is not a DRF setting but a custom implementation used by their `PaginationConfig` class in `task_service.py`, serializers, and throughout their codebase. The configuration includes `DEFAULT_PAGE_LIMIT` and `MAX_PAGE_LIMIT` values.
todo/views/auth.py (4)
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:106-106
Timestamp: 2025-05-29T21:36:27.694Z
Learning: Issue #26 tracks the implementation of user authentication in the todo-backend project, which includes extracting user ID from request context to replace hardcoded placeholders like "system_patch_user" in todo/views/task.py.
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:98-112
Timestamp: 2025-06-02T17:02:22.424Z
Learning: In the todo-backend project, there is a global exception handler in `todo/exceptions/exception_handler.py` that handles exceptions globally, eliminating the need for try-catch blocks in individual view methods. This approach reduces boilerplate code and provides consistent error handling across the application.
Learnt from: Achintya-Chatterjee
PR: Real-Dev-Squad/todo-backend#52
File: todo/views/task.py:98-112
Timestamp: 2025-06-02T17:02:22.424Z
Learning: The todo-backend project uses a global exception handler that automatically handles TaskNotFoundException, BsonInvalidId, ValueError with ApiErrorResponse, and DRFValidationError. Views should avoid try-catch blocks and let exceptions bubble up to the global handler for consistent error formatting and status codes.
Learnt from: VaibhavSingh8
PR: Real-Dev-Squad/todo-backend#81
File: todo_project/settings/base.py:70-77
Timestamp: 2025-06-16T18:05:07.813Z
Learning: The todo-backend project uses a custom pagination configuration system with `DEFAULT_PAGINATION_SETTINGS` nested under `REST_FRAMEWORK` in Django settings. This is not a DRF setting but a custom implementation used by their `PaginationConfig` class in `task_service.py`, serializers, and throughout their codebase. The configuration includes `DEFAULT_PAGE_LIMIT` and `MAX_PAGE_LIMIT` values.
🧬 Code Graph Analysis (1)
todo/views/auth.py (6)
todo/tests/integration/base_mongo_test.py (1)
_set_auth_cookies(66-69)todo/exceptions/google_auth_exceptions.py (2)
GoogleAuthException(10-12)GoogleAPIException(35-37)todo/services/google_oauth_service.py (2)
GoogleOAuthService(10-101)handle_callback(37-53)todo/services/user_service.py (2)
UserService(8-40)create_or_update_user(10-17)todo/utils/google_jwt_utils.py (1)
generate_google_token_pair(101-110)todo/constants/messages.py (1)
AppMessages(2-6)
🪛 Pylint (3.3.7)
todo/views/task.py
[error] 8-8: Unable to import 'drf_spectacular.utils'
(E0401)
todo/tests/unit/views/test_auth.py
[convention] 1-1: Missing module docstring
(C0114)
[error] 1-1: Unable to import 'rest_framework.test'
(E0401)
[convention] 17-17: Missing class docstring
(C0115)
[convention] 62-62: Missing class docstring
(C0115)
[convention] 63-63: Missing function or method docstring
(C0116)
[convention] 63-63: Method name "setUp" doesn't conform to snake_case naming style
(C0103)
[convention] 70-70: Missing function or method docstring
(C0116)
[convention] 77-77: Missing function or method docstring
(C0116)
[convention] 85-85: Missing function or method docstring
(C0116)
[convention] 114-114: Missing function or method docstring
(C0116)
[convention] 120-120: Missing function or method docstring
(C0116)
[convention] 132-132: Missing function or method docstring
(C0116)
[convention] 165-165: Missing class docstring
(C0115)
[convention] 200-200: Missing class docstring
(C0115)
todo/views/auth.py
[error] 5-5: Unable to import 'django.http'
(E0401)
[error] 6-6: Unable to import 'django.conf'
(E0401)
[error] 7-7: Unable to import 'drf_spectacular.utils'
(E0401)
[convention] 199-199: Line too long (102/100)
(C0301)
[warning] 147-147: Catching too general exception Exception
(W0718)
[convention] 162-162: Missing function or method docstring
(C0116)
[warning] 210-210: Consider explicitly re-raising using 'raise GoogleAPIException(str(e)) from e'
(W0707)
🔇 Additional comments (6)
todo/views/task.py (1)
8-8: LGTM! Good cleanup of unused import.Removing the unused
OpenApiExampleimport helps keep the codebase clean.todo/tests/unit/views/test_auth.py (3)
1-1: Good refactoring to use APITestCase for full request/response testing.The migration from
APISimpleTestCasetoAPITestCaseenables proper testing of the full request/response cycle, including session handling and cookies, which is essential for OAuth flow testing.Also applies to: 17-17, 62-62, 165-165, 200-200
70-113: Tests correctly updated to match the new redirect-based GET handling.The test updates properly reflect the refactored behavior where GET requests to the callback endpoint now return HTTP 302 redirects with appropriate query parameters for both error and success cases. Good coverage of all error scenarios.
114-163: Excellent addition of POST method tests for the new callback API.The new POST tests provide comprehensive coverage of the JSON API callback handling, including error cases and successful authentication flow. The tests properly handle session state and verify cookie setting.
todo/views/auth.py (2)
356-363: Good refactoring of cookie clearing logic.The
_clear_auth_cookiesmethod properly consolidates cookie deletion logic and correctly uses only the parameters thatdelete_cookieaccepts (path and domain).
151-211: Well-implemented POST handler for programmatic OAuth callback.The POST method provides a clean API for handling OAuth callbacks programmatically, returning proper JSON responses with user data and token expiry information. This separation of concerns between redirect-based (GET) and API-based (POST) handling is a good architectural decision.
VaibhavSingh8
changed the title
* fix: tests based on updated implementation * fix: lint and format * removed commented lines * changed frontend port number to 3000
Date: 06-Jul-2025
Developer Name: @VaibhavSingh8
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Implement RSA token support, modify authentication logic for Google OAuth, simplify error handling, remove refresh token endpoint, and enhance test configurations.
Why are these changes being made?
To improve security by using RSA tokens, streamline the login flow by handling token refresh internally without a dedicated endpoint, enhance error traceability in token operations, and align server configurations for a more consistent development and testing environment. The changes also ensure improved compatibility and functionality in local and test setups, and they enhance the overall security posture by patching vulnerabilities related to token management.