From 69bb09ef34cf212104878ca936cedb3b478db8b2 Mon Sep 17 00:00:00 2001 From: Vaibhav Singh Date: Sun, 6 Jul 2025 15:50:46 +0530 Subject: [PATCH 1/2] refactored google auth callback --- todo/tests/unit/views/test_auth.py | 44 ----- todo/urls.py | 2 - todo/views/auth.py | 270 +++++------------------------ todo/views/task.py | 2 +- 4 files changed, 45 insertions(+), 273 deletions(-) diff --git a/todo/tests/unit/views/test_auth.py b/todo/tests/unit/views/test_auth.py index b1545896..0f2fb3c6 100644 --- a/todo/tests/unit/views/test_auth.py +++ b/todo/tests/unit/views/test_auth.py @@ -131,51 +131,7 @@ def test_get_handles_callback_successfully(self, mock_create_user, mock_handle_c self.assertNotIn("oauth_state", request.session) -class GoogleAuthStatusViewTests(APISimpleTestCase): - def setUp(self): - super().setUp() - self.client = APIClient() - self.url = reverse("google_status") - - def test_get_returns_401_when_no_access_token(self): - response = self.client.get(self.url) - - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) - self.assertEqual(response.data["message"], AuthErrorMessages.NO_ACCESS_TOKEN) - self.assertEqual(response.data["authenticated"], False) - self.assertEqual(response.data["statusCode"], status.HTTP_401_UNAUTHORIZED) - - @patch("todo.utils.google_jwt_utils.validate_google_access_token") - @patch("todo.services.user_service.UserService.get_user_by_id") - def test_get_returns_user_info_when_authenticated(self, mock_get_user, mock_validate_token): - user_id = str(ObjectId()) - user_data = { - "user_id": user_id, - "google_id": "test_google_id", - "email": "test@example.com", - "name": "Test User", - } - mock_validate_token.return_value = user_data - - mock_user = Mock() - mock_user.id = ObjectId(user_id) - mock_user.google_id = "test_google_id" - mock_user.email_id = "test@example.com" - mock_user.name = "Test User" - type(mock_user).id = PropertyMock(return_value=ObjectId(user_id)) - mock_get_user.return_value = mock_user - - tokens = generate_google_token_pair(user_data) - self.client.cookies["ext-access"] = tokens["access_token"] - - response = self.client.get(self.url, HTTP_ACCEPT="application/json") - - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data["data"]["user"]["id"], user_id) - self.assertEqual(response.data["data"]["user"]["email"], mock_user.email_id) - self.assertEqual(response.data["data"]["user"]["name"], mock_user.name) - self.assertEqual(response.data["data"]["user"]["google_id"], mock_user.google_id) class GoogleRefreshViewTests(APISimpleTestCase): diff --git a/todo/urls.py b/todo/urls.py index d370c6af..6827dad3 100644 --- a/todo/urls.py +++ b/todo/urls.py @@ -6,7 +6,6 @@ GoogleCallbackView, GoogleRefreshView, GoogleLogoutView, - GoogleAuthStatusView, ) urlpatterns = [ @@ -15,7 +14,6 @@ path("health", HealthView.as_view(), name="health"), path("auth/google/login/", GoogleLoginView.as_view(), name="google_login"), path("auth/google/callback/", GoogleCallbackView.as_view(), name="google_callback"), - path("auth/google/status/", GoogleAuthStatusView.as_view(), name="google_status"), path("auth/google/refresh/", GoogleRefreshView.as_view(), name="google_refresh"), path("auth/google/logout/", GoogleLogoutView.as_view(), name="google_logout"), ] diff --git a/todo/views/auth.py b/todo/views/auth.py index 40e8cc42..3df5c7fb 100644 --- a/todo/views/auth.py +++ b/todo/views/auth.py @@ -2,15 +2,14 @@ from rest_framework.response import Response from rest_framework.request import Request from rest_framework import status -from django.http import HttpResponseRedirect, HttpResponse +from django.http import HttpResponseRedirect from django.conf import settings -from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiExample, OpenApiResponse +from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse from drf_spectacular.types import OpenApiTypes from todo.services.google_oauth_service import GoogleOAuthService from todo.services.user_service import UserService from todo.utils.google_jwt_utils import ( validate_google_refresh_token, - validate_google_access_token, generate_google_access_token, generate_google_token_pair, ) @@ -19,7 +18,6 @@ from todo.exceptions.google_auth_exceptions import ( GoogleAuthException, GoogleTokenExpiredError, - GoogleTokenInvalidError, GoogleTokenMissingError, GoogleAPIException, ) @@ -70,15 +68,6 @@ def get(self, request: Request): class GoogleCallbackView(APIView): - """ - This class has two implementations: - 1. Current active implementation (temporary) - For testing and development - 2. Commented implementation - For frontend integration (to be used later) - - The temporary implementation processes the OAuth callback directly and shows a success page. - The frontend implementation will redirect to the frontend and process the callback via POST request. - """ - @extend_schema( operation_id="google_callback", summary="Handle Google OAuth callback", @@ -114,12 +103,33 @@ class GoogleCallbackView(APIView): }, ) def get(self, request: Request): - if "error" in request.query_params: - error = request.query_params.get("error") - raise GoogleAuthException(error) - 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: + return HttpResponseRedirect(f"{frontend_callback}?error={error}") + elif code and state: + return HttpResponseRedirect(f"{frontend_callback}?code={code}&state={state}") + else: + return HttpResponseRedirect(f"{frontend_callback}?error=missing_parameters") + + @extend_schema( + operation_id="google_callback_post", + summary="Handle Google OAuth callback (POST)", + description="Processes the OAuth callback from Google via POST request", + tags=["auth"], + responses={ + 200: OpenApiResponse(description="OAuth callback processed successfully"), + 400: OpenApiResponse(description="Bad request - invalid parameters"), + 500: OpenApiResponse(description="Internal server error"), + }, + ) + def post(self, request: Request): + code = request.data.get("code") + state = request.data.get("state") if not code: raise GoogleAuthException("No authorization code received from Google") @@ -128,9 +138,6 @@ def get(self, request: Request): if not stored_state or stored_state != state: raise GoogleAuthException("Invalid state parameter") - return self._handle_callback_directly(code, request) - - def _handle_callback_directly(self, code, request): try: google_data = GoogleOAuthService.handle_callback(code) user = UserService.create_or_update_user(google_data) @@ -144,62 +151,24 @@ def _handle_callback_directly(self, code, request): } ) - wants_json = ( - "application/json" in request.headers.get("Accept", "").lower() - or request.query_params.get("format") == "json" - ) - - if wants_json: - response = Response( - { - "statusCode": status.HTTP_200_OK, - "message": AppMessages.GOOGLE_LOGIN_SUCCESS, - "data": { - "user": { - "id": str(user.id), - "name": user.name, - "email": user.email_id, - "google_id": user.google_id, - }, - "tokens": { - "access_token_expires_in": tokens["expires_in"], - "refresh_token_expires_in": settings.GOOGLE_JWT["REFRESH_TOKEN_LIFETIME"], - }, + response = Response( + { + "statusCode": status.HTTP_200_OK, + "message": AppMessages.GOOGLE_LOGIN_SUCCESS, + "data": { + "user": { + "id": str(user.id), + "name": user.name, + "email": user.email_id, + "google_id": user.google_id, + }, + "tokens": { + "access_token_expires_in": tokens["expires_in"], + "refresh_token_expires_in": settings.GOOGLE_JWT["REFRESH_TOKEN_LIFETIME"], }, - } - ) - else: - response = HttpResponse(f""" - - โœ… Login Successful - -

โœ… Google OAuth Login Successful!

- -

๐Ÿง‘โ€๐Ÿ’ป User Info:

- - -

๐Ÿช Authentication Cookies Set:

- - -

๐Ÿงช Test Other Endpoints:

- - -

Google OAuth integration is working perfectly!

- - - """) + }, + } + ) self._set_auth_cookies(response, tokens) request.session.pop("oauth_state", None) @@ -225,157 +194,6 @@ def _set_auth_cookies(self, response, tokens): ) -# Frontend integration implementation (to be used later) -""" -class GoogleCallbackViewFrontend(APIView): - 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: - return HttpResponseRedirect(f"{frontend_callback}?error={error}") - elif code and state: - return HttpResponseRedirect(f"{frontend_callback}?code={code}&state={state}") - else: - return HttpResponseRedirect(f"{frontend_callback}?error=missing_parameters") - - def post(self, request: Request): - code = request.data.get("code") - state = request.data.get("state") - - if not code: - formatted_errors = [ - ApiErrorDetail( - source={ApiErrorSource.PARAMETER: "code"}, - title=ApiErrors.VALIDATION_ERROR, - detail=ApiErrors.INVALID_AUTH_CODE, - ) - ] - error_response = ApiErrorResponse( - statusCode=400, - message=ApiErrors.INVALID_AUTH_CODE, - errors=formatted_errors - ) - return Response( - data=error_response.model_dump(mode="json", exclude_none=True), - status=status.HTTP_400_BAD_REQUEST - ) - - stored_state = request.session.get("oauth_state") - if not stored_state or stored_state != state: - formatted_errors = [ - ApiErrorDetail( - source={ApiErrorSource.PARAMETER: "state"}, - title=ApiErrors.VALIDATION_ERROR, - detail=ApiErrors.INVALID_STATE_PARAMETER, - ) - ] - error_response = ApiErrorResponse( - statusCode=400, - message=ApiErrors.INVALID_STATE_PARAMETER, - errors=formatted_errors - ) - return Response( - data=error_response.model_dump(mode="json", exclude_none=True), - status=status.HTTP_400_BAD_REQUEST - ) - - 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, - } - ) - - response = Response({ - "statusCode": status.HTTP_200_OK, - "message": AppMessages.GOOGLE_LOGIN_SUCCESS, - "data": { - "user": { - "id": str(user.id), - "name": user.name, - "email": user.email_id, - "google_id": user.google_id, - }, - "tokens": { - "access_token_expires_in": tokens["expires_in"], - "refresh_token_expires_in": settings.GOOGLE_JWT["REFRESH_TOKEN_LIFETIME"] - } - } - }) - - self._set_auth_cookies(response, tokens) - request.session.pop("oauth_state", None) - - return response - - def _get_cookie_config(self): - return { - "path": "/", - "domain": settings.GOOGLE_COOKIE_SETTINGS.get("COOKIE_DOMAIN"), - "secure": settings.GOOGLE_COOKIE_SETTINGS.get("COOKIE_SECURE", False), - "httponly": True, - "samesite": settings.GOOGLE_COOKIE_SETTINGS.get("COOKIE_SAMESITE", "Lax"), - } - - def _set_auth_cookies(self, response, tokens): - config = self._get_cookie_config() - response.set_cookie("ext-access", tokens["access_token"], max_age=tokens["expires_in"], **config) - response.set_cookie( - "ext-refresh", tokens["refresh_token"], max_age=settings.GOOGLE_JWT["REFRESH_TOKEN_LIFETIME"], **config - ) -""" - - -class GoogleAuthStatusView(APIView): - @extend_schema( - operation_id="google_auth_status", - summary="Check authentication status", - description="Check if the user is authenticated and return user information", - tags=["auth"], - responses={ - 200: OpenApiResponse(description="Authentication status retrieved successfully"), - 401: OpenApiResponse(description="Unauthorized - invalid or missing token"), - 500: OpenApiResponse(description="Internal server error"), - }, - ) - def get(self, request: Request): - access_token = request.COOKIES.get("ext-access") - - if not access_token: - raise GoogleTokenMissingError(AuthErrorMessages.NO_ACCESS_TOKEN) - - try: - payload = validate_google_access_token(access_token) - user = UserService.get_user_by_id(payload["user_id"]) - except Exception as e: - raise GoogleTokenInvalidError(str(e)) - - return Response( - { - "statusCode": status.HTTP_200_OK, - "message": "Authentication status retrieved successfully", - "data": { - "authenticated": True, - "user": { - "id": str(user.id), - "email": user.email_id, - "name": user.name, - "google_id": user.google_id, - }, - }, - } - ) - - class GoogleRefreshView(APIView): @extend_schema( operation_id="google_refresh_token", diff --git a/todo/views/task.py b/todo/views/task.py index bbcaf353..4bad214a 100644 --- a/todo/views/task.py +++ b/todo/views/task.py @@ -5,7 +5,7 @@ from rest_framework.request import Request from rest_framework.exceptions import ValidationError from django.conf import settings -from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiExample, OpenApiResponse +from drf_spectacular.utils import extend_schema, OpenApiParameter, OpenApiResponse from drf_spectacular.types import OpenApiTypes from todo.middlewares.jwt_auth import get_current_user_info from todo.serializers.get_tasks_serializer import GetTaskQueryParamsSerializer From 83f151264deb46cbd69b15410114bd337157e405 Mon Sep 17 00:00:00 2001 From: Vaibhav Singh Date: Sun, 6 Jul 2025 16:03:41 +0530 Subject: [PATCH 2/2] fix: failing tests --- todo/tests/unit/views/test_auth.py | 70 +++++++++++++++++------------- todo/views/auth.py | 13 ++++-- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/todo/tests/unit/views/test_auth.py b/todo/tests/unit/views/test_auth.py index 0f2fb3c6..4ffe8263 100644 --- a/todo/tests/unit/views/test_auth.py +++ b/todo/tests/unit/views/test_auth.py @@ -1,4 +1,4 @@ -from rest_framework.test import APISimpleTestCase, APIClient, APIRequestFactory +from rest_framework.test import APITestCase, APIClient, APIRequestFactory from rest_framework.reverse import reverse from rest_framework import status from unittest.mock import patch, Mock, PropertyMock @@ -14,7 +14,7 @@ from todo.constants.messages import AppMessages, AuthErrorMessages -class GoogleLoginViewTests(APISimpleTestCase): +class GoogleLoginViewTests(APITestCase): def setUp(self): super().setUp() self.client = APIClient() @@ -59,7 +59,7 @@ def test_get_with_redirect_url(self, mock_get_auth_url): mock_get_auth_url.assert_called_once_with(redirect_url) -class GoogleCallbackViewTests(APISimpleTestCase): +class GoogleCallbackViewTests(APITestCase): def setUp(self): super().setUp() self.client = APIClient() @@ -67,38 +67,46 @@ def setUp(self): self.factory = APIRequestFactory() self.view = GoogleCallbackView.as_view() - def test_get_returns_error_for_oauth_error(self): + def test_get_redirects_for_oauth_error(self): error = "access_denied" - request = self.factory.get(f"{self.url}?error={error}") + response = self.client.get(f"{self.url}?error={error}") - response = self.view(request) + self.assertEqual(response.status_code, status.HTTP_302_FOUND) + self.assertIn("error=access_denied", response.url) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.data["message"], error) - self.assertEqual(response.data["errors"][0]["detail"], error) + def test_get_redirects_for_missing_code(self): + response = self.client.get(self.url) - def test_get_returns_error_for_missing_code(self): - request = self.factory.get(self.url) + self.assertEqual(response.status_code, status.HTTP_302_FOUND) + self.assertIn("error=missing_parameters", response.url) - response = self.view(request) + def test_get_redirects_for_valid_code_and_state(self): + response = self.client.get(f"{self.url}?code=test_code&state=test_state") + + self.assertEqual(response.status_code, status.HTTP_302_FOUND) + self.assertIn("code=test_code", response.url) + self.assertIn("state=test_state", response.url) + + def test_post_returns_error_for_missing_code(self): + response = self.client.post(self.url, {}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.data["message"], "No authorization code received from Google") - self.assertEqual(response.data["errors"][0]["detail"], "No authorization code received from Google") - def test_get_returns_error_for_invalid_state(self): - request = self.factory.get(f"{self.url}?code=test_code&state=invalid_state") - request.session = {"oauth_state": "different_state"} + def test_post_returns_error_for_invalid_state(self): - response = self.view(request) + session = self.client.session + session["oauth_state"] = "different_state" + session.save() + + response = self.client.post(self.url, {"code": "test_code", "state": "invalid_state"}) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.data["message"], "Invalid state parameter") - self.assertEqual(response.data["errors"][0]["detail"], "Invalid state parameter") @patch("todo.services.google_oauth_service.GoogleOAuthService.handle_callback") @patch("todo.services.user_service.UserService.create_or_update_user") - def test_get_handles_callback_successfully(self, mock_create_user, mock_handle_callback): + def test_post_handles_callback_successfully(self, mock_create_user, mock_handle_callback): mock_google_data = { "id": "test_google_id", "email": "test@example.com", @@ -115,26 +123,26 @@ def test_get_handles_callback_successfully(self, mock_create_user, mock_handle_c mock_handle_callback.return_value = mock_google_data mock_create_user.return_value = mock_user - request = self.factory.get(f"{self.url}?code=test_code&state=test_state") - request.session = {"oauth_state": "test_state"} + session = self.client.session + session["oauth_state"] = "test_state" + session.save() - response = self.view(request) + response = self.client.post(self.url, {"code": "test_code", "state": "test_state"}) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("โœ… Google OAuth Login Successful!", response.content.decode()) - self.assertIn(str(mock_user.id), response.content.decode()) - self.assertIn(mock_user.name, response.content.decode()) - self.assertIn(mock_user.email_id, response.content.decode()) - self.assertIn(mock_user.google_id, response.content.decode()) + self.assertEqual(response.data["data"]["user"]["id"], user_id) + self.assertEqual(response.data["data"]["user"]["name"], mock_user.name) + self.assertEqual(response.data["data"]["user"]["email"], mock_user.email_id) + self.assertEqual(response.data["data"]["user"]["google_id"], mock_user.google_id) self.assertIn("ext-access", response.cookies) self.assertIn("ext-refresh", response.cookies) - self.assertNotIn("oauth_state", request.session) + self.assertNotIn("oauth_state", self.client.session) -class GoogleRefreshViewTests(APISimpleTestCase): +class GoogleRefreshViewTests(APITestCase): def setUp(self): super().setUp() self.client = APIClient() @@ -169,7 +177,7 @@ def test_get_refreshes_token_successfully(self, mock_validate_token): self.assertIn("ext-access", response.cookies) -class GoogleLogoutViewTests(APISimpleTestCase): +class GoogleLogoutViewTests(APITestCase): def setUp(self): super().setUp() self.client = APIClient() @@ -187,7 +195,7 @@ def test_get_returns_success_and_clears_cookies(self): self.client.cookies["ext-refresh"] = tokens["refresh_token"] response = self.client.get(self.url, HTTP_ACCEPT="application/json") - + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data["data"]["success"], True) self.assertEqual(response.data["message"], AppMessages.GOOGLE_LOGOUT_SUCCESS) diff --git a/todo/views/auth.py b/todo/views/auth.py index 3df5c7fb..3015c5bc 100644 --- a/todo/views/auth.py +++ b/todo/views/auth.py @@ -308,9 +308,7 @@ def _handle_logout(self, request: Request): redirect_url = redirect_url or "/" response = HttpResponseRedirect(redirect_url) - config = self._get_cookie_config() - response.delete_cookie("ext-access", **config) - response.delete_cookie("ext-refresh", **config) + self._clear_auth_cookies(response) return response @@ -322,3 +320,12 @@ def _get_cookie_config(self): "httponly": True, "samesite": settings.GOOGLE_COOKIE_SETTINGS.get("COOKIE_SAMESITE", "Lax"), } + + def _clear_auth_cookies(self, response): + """Clear authentication cookies with only the parameters that delete_cookie accepts""" + delete_config = { + "path": "/", + "domain": settings.GOOGLE_COOKIE_SETTINGS.get("COOKIE_DOMAIN"), + } + response.delete_cookie("ext-access", **delete_config) + response.delete_cookie("ext-refresh", **delete_config)