[DOP-21576] Move /auth/me -> /users/me; Remove state from redirect

Author: TiGrib

.env.local

@@ -20,7 +20,7 @@ export DATA_RENTGEN__AUTH__KEYCLOAK__SERVER_URL=http://localhost:8080
 export DATA_RENTGEN__AUTH__KEYCLOAK__REALM_NAME=manually_created
 export DATA_RENTGEN__AUTH__KEYCLOAK__CLIENT_ID=manually_created
 export DATA_RENTGEN__AUTH__KEYCLOAK__CLIENT_SECRET=change_me
-export DATA_RENTGEN__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:3000/callback
+export DATA_RENTGEN__AUTH__KEYCLOAK__REDIRECT_URI=http://localhost:3000/auth/callback
 export DATA_RENTGEN__AUTH__KEYCLOAK__SCOPE=email
 export DATA_RENTGEN__AUTH__KEYCLOAK__VERIFY_SSL=False
 export DATA_RENTGEN__AUTH__PROVIDER=data_rentgen.server.providers.auth.keycloak_provider.KeycloakAuthProvider

data_rentgen/server/api/v1/router/__init__.py

@@ -8,6 +8,7 @@
 from data_rentgen.server.api.v1.router.location import router as location_router
 from data_rentgen.server.api.v1.router.operation import router as operation_router
 from data_rentgen.server.api.v1.router.run import router as run_router
+from data_rentgen.server.api.v1.router.user import router as user_router
 
 router = APIRouter(prefix="/v1")
 router.include_router(auth_router)
@@ -16,3 +17,4 @@
 router.include_router(location_router)
 router.include_router(operation_router)
 router.include_router(run_router)
+router.include_router(user_router)

data_rentgen/server/api/v1/router/auth.py

@@ -1,14 +1,11 @@
 # SPDX-FileCopyrightText: 2024 MTS PJSC
 # SPDX-License-Identifier: Apache-2.0
-import base64
 from http import HTTPStatus
-from logging import getLogger
 from typing import Annotated
 
-from fastapi import APIRouter, Depends, HTTPException, Request
+from fastapi import APIRouter, Depends, Request
 from fastapi.security import OAuth2PasswordRequestForm
 
-from data_rentgen.db.models.user import User
 from data_rentgen.dependencies import Stub
 from data_rentgen.server.errors.registration import get_error_responses
 from data_rentgen.server.errors.schemas.invalid_request import InvalidRequestSchema
@@ -22,11 +19,6 @@
     KeycloakAuthProvider,
 )
 from data_rentgen.server.schemas.v1.auth import AuthTokenSchema
-from data_rentgen.server.schemas.v1.user import UserResponseV1
-from data_rentgen.server.services import get_user
-
-logger = getLogger(__name__)
-
 
 router = APIRouter(
     prefix="/auth",
@@ -49,15 +41,9 @@ async def token(
 @router.get("/callback")
 async def auth_callback(
     request: Request,
-    state: str,
     code: str,
     auth_provider: Annotated[KeycloakAuthProvider, Depends(Stub(AuthProvider))],
 ):
-    state = base64.b64decode(state.encode("utf-8"))  # type: ignore[assignment]
-    original_url = state.decode("utf-8")  # type: ignore[attr-defined]
-
-    if not original_url:
-        raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid original_url")
     code_grant = await auth_provider.get_token_authorization_code_grant(
         code=code,
         redirect_uri=auth_provider.settings.keycloak.redirect_uri,
@@ -66,11 +52,3 @@ async def auth_callback(
     request.session["refresh_token"] = code_grant["refresh_token"]
 
     return HTTPStatus.OK
-
-
-@router.get("/me")
-async def check_auth(
-    current_user: User = Depends(get_user()),
-):
-    logger.info("User check: %s", current_user.name)
-    return UserResponseV1.model_validate(current_user)

data_rentgen/server/api/v1/router/user.py

@@ -0,0 +1,32 @@
+# SPDX-FileCopyrightText: 2024 MTS PJSC
+# SPDX-License-Identifier: Apache-2.0
+from logging import getLogger
+
+from fastapi import APIRouter, Depends
+
+from data_rentgen.db.models.user import User
+from data_rentgen.server.errors.registration import get_error_responses
+from data_rentgen.server.errors.schemas.invalid_request import InvalidRequestSchema
+from data_rentgen.server.errors.schemas.not_authorized import (
+    NotAuthorizedRedirectSchema,
+    NotAuthorizedSchema,
+)
+from data_rentgen.server.schemas.v1.user import UserResponseV1
+from data_rentgen.server.services import get_user
+
+logger = getLogger(__name__)
+
+
+router = APIRouter(
+    prefix="/users",
+    tags=["User"],
+    responses=get_error_responses(include={NotAuthorizedSchema, InvalidRequestSchema, NotAuthorizedRedirectSchema}),
+)
+
+
+@router.get("/me")
+async def check_auth(
+    current_user: User = Depends(get_user()),
+):
+    logger.info("User check: %s", current_user.name)
+    return UserResponseV1.model_validate(current_user)

data_rentgen/server/providers/auth/keycloak_provider.py

@@ -1,13 +1,10 @@
 # SPDX-FileCopyrightText: 2024 MTS PJSC
 # SPDX-License-Identifier: Apache-2.0
-import base64
 import logging
-from http import HTTPStatus
 from typing import Annotated, Any
 
-from fastapi import Depends, FastAPI, HTTPException, Request
+from fastapi import Depends, FastAPI, Request
 from keycloak import KeycloakOpenID
-from keycloak.exceptions import KeycloakConnectionError
 
 from data_rentgen.db.models import User
 from data_rentgen.dependencies import Stub
@@ -81,7 +78,7 @@ async def get_current_user(self, access_token: str, *args, **kwargs) -> User:
 
         if not access_token:
             logger.debug("No access token found in session.")
-            self.redirect_to_auth(str(request.url))
+            self.redirect_to_auth()
 
         # if user is disabled or blocked in Keycloak after the token is issued, he will
         # remain authorized until the token expires (not more than 15 minutes in MTS SSO)
@@ -96,7 +93,7 @@ async def get_current_user(self, access_token: str, *args, **kwargs) -> User:
 
             if token_info is None:
                 # If there is no token_info after refresh user get redirect
-                self.redirect_to_auth(str(request.url))
+                self.redirect_to_auth()
 
         # these names are hardcoded in keycloak:
         # https://github.com/keycloak/keycloak/blob/3ca3a4ad349b4d457f6829eaf2ae05f1e01408be/core/src/main/java/org/keycloak/representations/IDToken.java
@@ -107,11 +104,7 @@ async def get_current_user(self, access_token: str, *args, **kwargs) -> User:
         return await self._uow.user.get_or_create(UserDTO(name=login))  # type: ignore[arg-type]
 
     async def logout(self, refresh_token: str):
-        try:
-            return self.keycloak_openid.logout(refresh_token)
-        except KeycloakConnectionError as err:
-            logger.error("Error when trying to get token: %s", err)
-            return None
+        return self.keycloak_openid.logout(refresh_token)
 
     def decode_token(self, access_token: str) -> dict[str, Any] | None:
         try:
@@ -127,21 +120,14 @@ def refresh_access_token(self, refresh_token: str, origin_url: str) -> tuple[str
             return new_tokens.get("access_token"), new_tokens.get("refresh_token")
         except Exception as err:
             logger.debug("Failed to refresh access token: %s", err)
-            self.redirect_to_auth(origin_url)
+            self.redirect_to_auth()
 
-    def redirect_to_auth(self, state: str = ""):
-        try:
-            state = base64.b64encode(state.encode("utf-8"))  # type: ignore[assignment]
+    def redirect_to_auth(self):
 
-            auth_url = self.keycloak_openid.auth_url(
-                redirect_uri=self.settings.keycloak.redirect_uri,
-                scope=self.settings.keycloak.scope,
-                state=state.decode("utf-8"),  # type: ignore[attr-defined]
-            )
+        auth_url = self.keycloak_openid.auth_url(
+            redirect_uri=self.settings.keycloak.redirect_uri,
+            scope=self.settings.keycloak.scope,
+        )
 
-        except KeycloakConnectionError as err:
-            logger.error("Failed connect to Keycloak: %s", err)
-            # TODO: What exception should we raise here?
-            raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail=err)
         logger.info("Raising redirect error with url: %s", auth_url)
-        raise RedirectError(message="Authorize on provided url", details=auth_url)
+        raise RedirectError(message="Please authorize using provided URL", details=auth_url)

tests/test_server/test_auth/test_keycloak_auth.py

@@ -1,4 +1,3 @@
-import base64
 import logging
 
 import pytest
@@ -9,7 +8,6 @@
 from data_rentgen.db.models import Dataset, User
 from data_rentgen.server.settings import ServerApplicationSettings as Settings
 from data_rentgen.server.settings.auth.keycloak import KeycloakSettings
-from tests.test_server.utils.enrich import enrich_datasets
 
 KEYCLOAK_PROVIDER = "data_rentgen.server.providers.auth.keycloak_provider.KeycloakAuthProvider"
 pytestmark = [pytest.mark.asyncio, pytest.mark.server]
@@ -33,23 +31,21 @@ async def test_get_keycloak_user_unauthorized(
     caplog,
     server_app_settings: Settings,
 ):
-    k_settings = KeycloakSettings.model_validate(server_app_settings.auth.keycloak)
+    settings = KeycloakSettings.model_validate(server_app_settings.auth.keycloak)
 
-    response = await test_client.get("/v1/datasets")
+    response = await test_client.get("/v1/users/me")
 
     # redirect unauthorized user to Keycloak
-    state = str(response.url).encode("utf-8")
-    state = base64.b64encode(state)
     redirect_url = (
-        f"{k_settings.server_url}/realms/{k_settings.realm_name}/protocol/openid-connect/auth?client_id="
-        f"{k_settings.client_id}&response_type=code&redirect_uri={k_settings.redirect_uri}"
-        f"&scope={k_settings.scope}&state={state.decode('utf-8')}&nonce="
+        f"{settings.server_url}/realms/{settings.realm_name}/protocol/openid-connect/auth?client_id="
+        f"{settings.client_id}&response_type=code&redirect_uri={settings.redirect_uri}"
+        f"&scope={settings.scope}&state=&nonce="
     )
     assert response.status_code == 401
     assert response.json() == {
         "error": {
             "code": "auth_redirect",
-            "message": "Authorize on provided url",
+            "message": "Please authorize using provided URL",
             "details": redirect_url,
         },
     }
@@ -71,53 +67,24 @@ async def test_get_keycloak_user_authorized(
     test_client: AsyncClient,
     async_session: AsyncSession,
     user: User,
-    datasets: list[Dataset],
     server_app_settings: Settings,
     create_session_cookie,
     mock_keycloak_well_known,
     mock_keycloak_realm,
 ):
-    datasets = await enrich_datasets(datasets, async_session)
     session_cookie = create_session_cookie(user)
     headers = {
         "Cookie": f"session={session_cookie}",
     }
 
     response = await test_client.get(
-        "/v1/datasets",
+        "/v1/users/me",
         headers=headers,
     )
 
     assert response.cookies.get("session") == session_cookie
     assert response.status_code == 200
-    assert response.json() == {
-        "meta": {
-            "page": 1,
-            "page_size": 20,
-            "total_count": len(datasets),
-            "pages_count": 1,
-            "has_next": False,
-            "has_previous": False,
-            "next_page": None,
-            "previous_page": None,
-        },
-        "items": [
-            {
-                "kind": "DATASET",
-                "id": dataset.id,
-                "format": dataset.format,
-                "name": dataset.name,
-                "location": {
-                    "id": dataset.location.id,
-                    "name": dataset.location.name,
-                    "type": dataset.location.type,
-                    "addresses": [{"url": address.url} for address in dataset.location.addresses],
-                    "external_id": dataset.location.external_id,
-                },
-            }
-            for dataset in sorted(datasets, key=lambda x: x.name)
-        ],
-    }
+    assert response.json() == {"name": user.name}
 
 
 @responses.activate
@@ -135,7 +102,6 @@ async def test_get_keycloak_user_authorized(
 async def test_get_keycloak_user_expired_access_token(
     caplog,
     user: User,
-    datasets: list[Dataset],
     test_client: AsyncClient,
     async_session: AsyncSession,
     server_app_settings: Settings,
@@ -144,45 +110,17 @@ async def test_get_keycloak_user_expired_access_token(
     mock_keycloak_realm,
     mock_keycloak_token_refresh,
 ):
-    datasets = await enrich_datasets(datasets, async_session)
     session_cookie = create_session_cookie(user, expire_in_msec=-100000000)  # expired access token
     headers = {
         "Cookie": f"session={session_cookie}",
     }
 
     with caplog.at_level(logging.DEBUG):
         response = await test_client.get(
-            "/v1/datasets",
+            "/v1/users/me",
             headers=headers,
         )
 
     assert response.cookies.get("session") != session_cookie, caplog.text  # cookie is updated
     assert response.status_code == 200
-    assert response.json() == {
-        "meta": {
-            "page": 1,
-            "page_size": 20,
-            "total_count": len(datasets),
-            "pages_count": 1,
-            "has_next": False,
-            "has_previous": False,
-            "next_page": None,
-            "previous_page": None,
-        },
-        "items": [
-            {
-                "kind": "DATASET",
-                "id": dataset.id,
-                "format": dataset.format,
-                "name": dataset.name,
-                "location": {
-                    "id": dataset.location.id,
-                    "name": dataset.location.name,
-                    "type": dataset.location.type,
-                    "addresses": [{"url": address.url} for address in dataset.location.addresses],
-                    "external_id": dataset.location.external_id,
-                },
-            }
-            for dataset in sorted(datasets, key=lambda x: x.name)
-        ],
-    }
+    assert response.json() == {"name": user.name}