From 5f250db46d5305a26d8eaeb2bce2977f68e8fae0 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Mon, 6 May 2024 15:02:51 -0700 Subject: [PATCH 1/8] Add manager check --- app/authentication/token.py | 43 +++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/app/authentication/token.py b/app/authentication/token.py index 5e7cc24de..bf6d99cf9 100644 --- a/app/authentication/token.py +++ b/app/authentication/token.py @@ -33,13 +33,31 @@ async def is_service_account(token: str = Depends(oauth2_scheme)) -> bool: return True -async def is_admin(token: str = Depends(oauth2_scheme)) -> bool: +async def assert_admin(token: str = Depends(oauth2_scheme)) -> None: """Calls GFW API to authorize user. User must be ADMIN for gfw app """ - return await is_app_admin(token, "gfw", "Unauthorized") + return await assert_app_role(token, "ADMIN", "gfw", "Unauthorized") + + +async def assert_manager(token: str = Depends(oauth2_scheme)) -> None: + """Calls GFW API to authorize user. + + User must be MANAGER for data-api app. + """ + + return await assert_app_role(token, "MANAGER", "data-api", "Unauthorized") + + +async def is_admin_or_manager(token: str = Depends(oauth2_scheme)) -> bool: + """Calls GFW API to authorize user. + + User must be ADMIN for gfw app or MANAGER for data-api app. + """ + + return (await assert_admin(token)) or (await assert_manager(token)) async def rw_user_id(token: str = Depends(oauth2_scheme)) -> str: @@ -65,8 +83,9 @@ async def is_gfwpro_admin_for_query( status_code=401, detail="Unauthorized query on a restricted dataset" ) else: - return await is_app_admin( + return await is_app_role( cast(str, token), + "ADMIN", "gfw-pro", error_str="Unauthorized query on a restricted dataset", ) @@ -74,21 +93,23 @@ async def is_gfwpro_admin_for_query( return True -async def is_app_admin(token: str, app: str, error_str: str) -> bool: - """Calls GFW API to authorize user. +async def assert_app_role(token: str, role: str, app: str, error_str: str) -> None: + is_authorized = await is_app_role(token, role, app) + + if not is_authorized: + raise HTTPException(status_code=401, detail=error_str) - User must be an ADMIN for the specified app, else it will throw an - exception with the specified error string. - """ + +async def is_app_role(token: str, role: str, app: str) -> bool: + """Calls RW API to authorize user for specific role and app.""" response: Response = await who_am_i(token) if response.status_code == 401 or not ( - response.json()["role"] == "ADMIN" + response.json()["role"] == role and app in response.json()["extraUserData"]["apps"] ): - logger.warning(f"ADMIN privileges required. Unauthorized user: {response.text}") - raise HTTPException(status_code=401, detail=error_str) + return False else: return True From 19542719265310c1abfefd013f2ea4ee09fc0f1d Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Mon, 6 May 2024 15:34:55 -0700 Subject: [PATCH 2/8] Pass user instead of bool flag for authentication check --- app/authentication/token.py | 82 +++++++++------------ app/models/pydantic/authentication.py | 5 +- app/routes/authentication/authentication.py | 21 +++--- app/routes/datasets/dataset.py | 19 ++--- app/utils/rw_api.py | 6 +- tests/__init__.py | 15 +++- tests/conftest.py | 6 +- tests/routes/datasets/test_datasets.py | 3 +- tests_v2/conftest.py | 6 +- tests_v2/utils.py | 13 +++- 10 files changed, 89 insertions(+), 87 deletions(-) diff --git a/app/authentication/token.py b/app/authentication/token.py index bf6d99cf9..1f0837285 100644 --- a/app/authentication/token.py +++ b/app/authentication/token.py @@ -1,10 +1,11 @@ -from typing import Tuple, cast +from typing import cast from fastapi import Depends, HTTPException from fastapi.logger import logger from fastapi.security import OAuth2PasswordBearer from httpx import Response +from ..models.pydantic.authentication import User from ..routes import dataset_dependency from ..settings.globals import PROTECTED_QUERY_DATASETS from ..utils.rw_api import who_am_i @@ -33,37 +34,13 @@ async def is_service_account(token: str = Depends(oauth2_scheme)) -> bool: return True -async def assert_admin(token: str = Depends(oauth2_scheme)) -> None: +async def is_admin(token: str = Depends(oauth2_scheme)) -> None: """Calls GFW API to authorize user. User must be ADMIN for gfw app """ - return await assert_app_role(token, "ADMIN", "gfw", "Unauthorized") - - -async def assert_manager(token: str = Depends(oauth2_scheme)) -> None: - """Calls GFW API to authorize user. - - User must be MANAGER for data-api app. - """ - - return await assert_app_role(token, "MANAGER", "data-api", "Unauthorized") - - -async def is_admin_or_manager(token: str = Depends(oauth2_scheme)) -> bool: - """Calls GFW API to authorize user. - - User must be ADMIN for gfw app or MANAGER for data-api app. - """ - - return (await assert_admin(token)) or (await assert_manager(token)) - - -async def rw_user_id(token: str = Depends(oauth2_scheme)) -> str: - """Gets user ID from token.""" - - return await who_am_i(token).json()["id"] + return await is_app_admin(token, "gfw", "Unauthorized") async def is_gfwpro_admin_for_query( @@ -83,9 +60,8 @@ async def is_gfwpro_admin_for_query( status_code=401, detail="Unauthorized query on a restricted dataset" ) else: - return await is_app_role( + return await is_app_admin( cast(str, token), - "ADMIN", "gfw-pro", error_str="Unauthorized query on a restricted dataset", ) @@ -93,40 +69,48 @@ async def is_gfwpro_admin_for_query( return True -async def assert_app_role(token: str, role: str, app: str, error_str: str) -> None: - is_authorized = await is_app_role(token, role, app) - - if not is_authorized: - raise HTTPException(status_code=401, detail=error_str) - - -async def is_app_role(token: str, role: str, app: str) -> bool: +async def is_app_admin(token: str, app: str, error_str: str) -> bool: """Calls RW API to authorize user for specific role and app.""" response: Response = await who_am_i(token) if response.status_code == 401 or not ( - response.json()["role"] == role + response.json()["role"] == "ADMIN" and app in response.json()["extraUserData"]["apps"] ): - return False + raise HTTPException(status_code=401, detail=error_str) else: return True -async def get_user(token: str = Depends(oauth2_scheme)) -> Tuple[str, str]: - """Calls GFW API to authorize user. - - This functions check is user of any level is associated with the GFW - app and returns the user ID - """ +async def get_user(token: str = Depends(oauth2_scheme)) -> User: + """Get the details for authenticated user.""" response: Response = await who_am_i(token) - if response.status_code == 401 or not ( - "gfw" in response.json()["extraUserData"]["apps"] - ): + if response.status_code == 401: logger.info("Unauthorized user") raise HTTPException(status_code=401, detail="Unauthorized") else: - return response.json()["id"], response.json()["role"] + return User(**response.json()["data"]) + + +async def get_admin(user: User = Depends(get_user)) -> User: + """Get the details for authenticated ADMIN user.""" + + if user.role != "ADMIN": + raise HTTPException(status_code=401, detail="Unauthorized") + + return user + + +async def get_manager(user: User = Depends(get_user)) -> User: + """Get the details for authenticated MANAGER for data-api application or + ADMIN user.""" + + if user.role != "ADMIN" or not ( + user.role == "MANAGER" and "data-api" in user.applications + ): + raise HTTPException(status_code=401, detail="Unauthorized") + + return user diff --git a/app/models/pydantic/authentication.py b/app/models/pydantic/authentication.py index e218bc872..61be0cf5c 100644 --- a/app/models/pydantic/authentication.py +++ b/app/models/pydantic/authentication.py @@ -14,17 +14,18 @@ class SignUpRequestIn(StrictBaseModel): email: EmailStr = Query(..., description="User's email address") -class SignUp(StrictBaseModel): +class User(StrictBaseModel): id: str name: str email: EmailStr createdAt: datetime role: str + applications: List[str] extraUserData: Dict[str, Any] class SignUpResponse(Response): - data: SignUp + data: User class APIKeyRequestIn(StrictBaseModel): diff --git a/app/routes/authentication/authentication.py b/app/routes/authentication/authentication.py index 1014868c8..d9a13744e 100644 --- a/app/routes/authentication/authentication.py +++ b/app/routes/authentication/authentication.py @@ -1,4 +1,4 @@ -from typing import List, Optional, Tuple +from typing import List, Optional from uuid import UUID from fastapi import APIRouter, Depends, HTTPException, Path, Query, Request @@ -18,6 +18,7 @@ ApiKeyValidationResponse, SignUpRequestIn, SignUpResponse, + User, ) from ...models.pydantic.responses import Response from ...settings.globals import ( @@ -58,14 +59,14 @@ async def get_token(form_data: OAuth2PasswordRequestForm = Depends()): async def create_api_key( api_key_data: APIKeyRequestIn, request: Request, - user: Tuple[str, str] = Depends(get_user), + user: User = Depends(get_user), ): """Request a new API key. Default keys are valid for one year """ - user_id, user_role = user + user_id, user_role = user["id"], user["role"] if api_key_data.never_expires and user_role != "ADMIN": raise HTTPException( @@ -90,7 +91,7 @@ async def create_api_key( if key.alias == api_key_data.alias: raise HTTPException( status_code=409, - detail="Key with specified alias already exists; use a different alias" + detail="Key with specified alias already exists; use a different alias", ) row: ORMApiKey = await api_keys.create_api_key(user_id=user_id, **input_data) @@ -117,13 +118,13 @@ async def create_api_key( @router.get("/apikey/{api_key}", tags=["Authentication"]) async def get_api_key( api_key: UUID = Path(..., description="API Key"), - user: Tuple[str, str] = Depends(get_user), + user: User = Depends(get_user), ): """Get details for a specific API Key. User must own API Key or must be Admin to see details. """ - user_id, role = user + user_id, role = user["id"], user["role"] try: row: ORMApiKey = await api_keys.get_api_key(api_key) except RecordNotFoundError: @@ -141,13 +142,13 @@ async def get_api_key( @router.get("/apikeys", tags=["Authentication"]) async def get_api_keys( - user: Tuple[str, str] = Depends(get_user), + user: User = Depends(get_user), ): """Request a new API key. Default keys are valid for one year """ - user_id, _ = user + user_id = user["id"] rows: List[ORMApiKey] = await api_keys.get_api_keys_from_user(user_id) data = [ApiKey.from_orm(row) for row in rows] @@ -184,13 +185,13 @@ async def delete_api_key( api_key: UUID = Path( ..., description="Api Key to delete. Must be owned by authenticated user." ), - user: Tuple[str, str] = Depends(get_user), + user: User = Depends(get_user), ): """Delete existing API key. API Key must belong to user. """ - user_id, _ = user + user_id = user["id"] try: row: ORMApiKey = await api_keys.get_api_key(api_key) except RecordNotFoundError: diff --git a/app/routes/datasets/dataset.py b/app/routes/datasets/dataset.py index d56d7d621..838f86ef8 100644 --- a/app/routes/datasets/dataset.py +++ b/app/routes/datasets/dataset.py @@ -8,11 +8,12 @@ from sqlalchemy.schema import CreateSchema, DropSchema from ...application import db -from ...authentication.token import is_admin, rw_user_id +from ...authentication.token import get_manager, is_admin from ...crud import datasets, versions from ...errors import RecordAlreadyExistsError, RecordNotFoundError from ...models.orm.datasets import Dataset as ORMDataset from ...models.orm.versions import Version as ORMVersion +from ...models.pydantic.authentication import User from ...models.pydantic.datasets import ( Dataset, DatasetCreateIn, @@ -52,21 +53,21 @@ async def create_dataset( *, dataset: str = Depends(dataset_dependency), request: DatasetCreateIn, - is_authorized: bool = Depends(is_admin), - owner_id: str = Depends(rw_user_id), + user: User = Depends(get_manager), response: Response, ) -> DatasetResponse: - """Create a dataset. A “dataset” is largely a metadata concept: it represents - a data product that may have multiple versions or file formats over time. - The user that creates a dataset using this operation becomes the owner of - the dataset, which provides the user with the privileges to do further write - operations on the dataset, including creating and modifying versions and assets. + """Create a dataset. A “dataset” is largely a metadata concept: it + represents a data product that may have multiple versions or file formats + over time. The user that creates a dataset using this operation becomes the + owner of the dataset, which provides the user with the privileges to do + further write operations on the dataset, including creating and modifying + versions and assets. This operation requires a `MANAGER` or an `ADMIN` user role. """ input_data: Dict = request.dict(exclude_none=True, by_alias=True) - input_data["owner_id"] = owner_id + input_data["owner_id"] = user.id try: new_dataset: ORMDataset = await datasets.create_dataset(dataset, **input_data) diff --git a/app/utils/rw_api.py b/app/utils/rw_api.py index 22ff7effc..d580b7a69 100644 --- a/app/utils/rw_api.py +++ b/app/utils/rw_api.py @@ -12,7 +12,7 @@ RecordNotFoundError, UnauthorizedError, ) -from ..models.pydantic.authentication import SignUp +from ..models.pydantic.authentication import User from ..models.pydantic.geostore import Geometry, GeostoreCommon from ..settings.globals import RW_API_URL @@ -116,7 +116,7 @@ async def login(user_name: str, password: str) -> str: return response.json()["data"]["token"] -async def signup(name: str, email: str) -> SignUp: +async def signup(name: str, email: str) -> User: """Obtain a token form RW API using given user name and password.""" headers = {"Content-Type": "application/json"} @@ -153,4 +153,4 @@ async def signup(name: str, email: str) -> SignUp: detail="An error occurred while trying to create a new user account. Please try again.", ) - return SignUp(**response.json()["data"]) + return User(**response.json()["data"]) diff --git a/tests/__init__.py b/tests/__init__.py index e268bda67..16b498565 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -12,6 +12,7 @@ from sqlalchemy import create_engine from sqlalchemy.orm import Session, sessionmaker +from app.models.pydantic.authentication import User from app.settings.globals import ( AWS_REGION, DATA_LAKE_BUCKET, @@ -51,8 +52,6 @@ BUCKET = "test-bucket" PORT = 9000 -RW_USER_ID = "5874bfcca049b7a56ad42771" # pragma: allowlist secret - SessionLocal: Optional[Session] = None @@ -315,8 +314,16 @@ async def get_api_key_mocked() -> Tuple[Optional[str], Optional[str]]: return str(uuid.uuid4()), "localhost" -async def get_rw_user_id() -> str: - return RW_USER_ID +async def get_manager_mocked() -> User: + return User( + id="mr_manager123", + name="Mr. Manager", + email="mr_manager@management.com", + createdAt="2021-06-13T03:18:23.000Z", + role="MANAGER", + applications=["data-api"], + extraUserData={}, + ) def setup_clients(ec2_client, iam_client): diff --git a/tests/conftest.py b/tests/conftest.py index efb5801f6..8d66ad9bb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,7 +16,7 @@ from httpx import AsyncClient from app.authentication.api_keys import get_api_key -from app.authentication.token import is_admin, is_service_account, rw_user_id +from app.authentication.token import get_manager, is_admin, is_service_account from app.settings.globals import ( AURORA_JOB_QUEUE, AURORA_JOB_QUEUE_FAST, @@ -52,7 +52,7 @@ AWSMock, MemoryServer, get_api_key_mocked, - get_rw_user_id, + get_manager_mocked, is_admin_mocked, is_service_account_mocked, setup_clients, @@ -252,7 +252,7 @@ async def async_client(): app.dependency_overrides[is_admin] = is_admin_mocked app.dependency_overrides[is_service_account] = is_service_account_mocked app.dependency_overrides[get_api_key] = get_api_key_mocked - app.dependency_overrides[rw_user_id] = get_rw_user_id + app.dependency_overrides[get_manager] = get_manager_mocked async with AsyncClient(app=app, base_url="http://test", trust_env=False) as client: yield client diff --git a/tests/routes/datasets/test_datasets.py b/tests/routes/datasets/test_datasets.py index f823a9798..5e149ec1e 100644 --- a/tests/routes/datasets/test_datasets.py +++ b/tests/routes/datasets/test_datasets.py @@ -3,7 +3,6 @@ import pytest from app.application import ContextEngine, db -from tests import RW_USER_ID from tests.utils import create_default_asset, dataset_metadata payload = {"metadata": dataset_metadata} @@ -69,7 +68,7 @@ async def test_datasets(async_client): ) assert len(rows) == 1 - assert rows[0][0] == RW_USER_ID + assert rows[0][0] == "mr_manager123" new_payload = {"metadata": {"title": "New Title"}} response = await async_client.patch(f"/dataset/{dataset}", json=new_payload) diff --git a/tests_v2/conftest.py b/tests_v2/conftest.py index bd58a3def..4a9fefce0 100755 --- a/tests_v2/conftest.py +++ b/tests_v2/conftest.py @@ -11,7 +11,7 @@ from fastapi.testclient import TestClient from httpx import AsyncClient -from app.authentication.token import get_user, is_admin, is_service_account, rw_user_id +from app.authentication.token import get_manager, get_user, is_admin, is_service_account from app.crud import api_keys from app.models.enum.change_log import ChangeLogStatus from app.models.pydantic.change_log import ChangeLog @@ -31,7 +31,7 @@ dict_function_closure, get_admin_mocked, get_extent_mocked, - get_rw_user_id_mocked, + get_manager_mocked, get_user_mocked, int_function_closure, void_coroutine, @@ -81,7 +81,7 @@ async def async_client(db, init_db) -> AsyncGenerator[AsyncClient, None]: True, with_args=False ) app.dependency_overrides[get_user] = get_admin_mocked - app.dependency_overrides[rw_user_id] = get_rw_user_id_mocked + app.dependency_overrides[get_manager] = get_manager_mocked async with AsyncClient( app=app, diff --git a/tests_v2/utils.py b/tests_v2/utils.py index 42a6b34ae..af43a336f 100644 --- a/tests_v2/utils.py +++ b/tests_v2/utils.py @@ -8,6 +8,7 @@ from _pytest.monkeypatch import MonkeyPatch from app.application import ContextEngine +from app.models.pydantic.authentication import User from app.models.pydantic.extent import Extent from app.routes.datasets import versions from app.tasks import batch, delete_assets @@ -39,8 +40,16 @@ async def get_admin_mocked() -> Tuple[str, str]: return "adminid_123", "ADMIN" -async def get_rw_user_id_mocked() -> str: - return "userid_123" +async def get_manager_mocked() -> str: + return User( + id="mr_manager123", + name="Mr. Manager", + email="mr_manager@management.com", + createdAt="2021-06-13T03:18:23.000Z", + role="MANAGER", + applications=["data-api"], + extraUserData={}, + ) async def get_api_key_mocked() -> Tuple[Optional[str], Optional[str]]: From 8a602d0aed9f308ecf5afcc6ce9193b471473cf1 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Mon, 6 May 2024 15:39:27 -0700 Subject: [PATCH 3/8] Remove accidental changes --- app/authentication/token.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/authentication/token.py b/app/authentication/token.py index 1f0837285..7f8cd8d24 100644 --- a/app/authentication/token.py +++ b/app/authentication/token.py @@ -34,7 +34,7 @@ async def is_service_account(token: str = Depends(oauth2_scheme)) -> bool: return True -async def is_admin(token: str = Depends(oauth2_scheme)) -> None: +async def is_admin(token: str = Depends(oauth2_scheme)) -> bool: """Calls GFW API to authorize user. User must be ADMIN for gfw app @@ -70,7 +70,11 @@ async def is_gfwpro_admin_for_query( async def is_app_admin(token: str, app: str, error_str: str) -> bool: - """Calls RW API to authorize user for specific role and app.""" + """Calls GFW API to authorize user. + + User must be an ADMIN for the specified app, else it will throw an + exception with the specified error string. + """ response: Response = await who_am_i(token) @@ -78,6 +82,7 @@ async def is_app_admin(token: str, app: str, error_str: str) -> bool: response.json()["role"] == "ADMIN" and app in response.json()["extraUserData"]["apps"] ): + logger.warning(f"ADMIN privileges required. Unauthorized user: {response.text}") raise HTTPException(status_code=401, detail=error_str) else: return True From 97a2ec7f893cb9a5e633775ec81e4227f19353b3 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Tue, 7 May 2024 11:05:13 -0700 Subject: [PATCH 4/8] Fix API key changes --- app/routes/authentication/authentication.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/routes/authentication/authentication.py b/app/routes/authentication/authentication.py index d9a13744e..164cbdbdd 100644 --- a/app/routes/authentication/authentication.py +++ b/app/routes/authentication/authentication.py @@ -66,7 +66,7 @@ async def create_api_key( Default keys are valid for one year """ - user_id, user_role = user["id"], user["role"] + user_id, user_role = user.id, user.role if api_key_data.never_expires and user_role != "ADMIN": raise HTTPException( @@ -124,7 +124,7 @@ async def get_api_key( User must own API Key or must be Admin to see details. """ - user_id, role = user["id"], user["role"] + user_id, role = user.id, user.role try: row: ORMApiKey = await api_keys.get_api_key(api_key) except RecordNotFoundError: @@ -148,7 +148,7 @@ async def get_api_keys( Default keys are valid for one year """ - user_id = user["id"] + user_id = user.id rows: List[ORMApiKey] = await api_keys.get_api_keys_from_user(user_id) data = [ApiKey.from_orm(row) for row in rows] @@ -191,7 +191,7 @@ async def delete_api_key( API Key must belong to user. """ - user_id = user["id"] + user_id = user.id try: row: ORMApiKey = await api_keys.get_api_key(api_key) except RecordNotFoundError: From 23265cd70b0aaba8c20fe03c2e2507ac7365e147 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Tue, 7 May 2024 13:49:04 -0700 Subject: [PATCH 5/8] Update fixtures to return User objects --- app/routes/authentication/authentication.py | 10 ++++------ tests_v2/utils.py | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/routes/authentication/authentication.py b/app/routes/authentication/authentication.py index 164cbdbdd..931a22a9e 100644 --- a/app/routes/authentication/authentication.py +++ b/app/routes/authentication/authentication.py @@ -66,12 +66,10 @@ async def create_api_key( Default keys are valid for one year """ - user_id, user_role = user.id, user.role - - if api_key_data.never_expires and user_role != "ADMIN": + if api_key_data.never_expires and user.role != "ADMIN": raise HTTPException( status_code=400, - detail=f"Users with role {user_role} cannot set `never_expires` to True.", + detail=f"Users with role {user.role} cannot set `never_expires` to True.", ) input_data = api_key_data.dict(by_alias=True) @@ -86,7 +84,7 @@ async def create_api_key( # Give a good error code/message if user is specifying an alias that exists for # another one of his API keys. - prev_keys: List[ORMApiKey] = await api_keys.get_api_keys_from_user(user_id=user_id) + prev_keys: List[ORMApiKey] = await api_keys.get_api_keys_from_user(user_id=user.id) for key in prev_keys: if key.alias == api_key_data.alias: raise HTTPException( @@ -94,7 +92,7 @@ async def create_api_key( detail="Key with specified alias already exists; use a different alias", ) - row: ORMApiKey = await api_keys.create_api_key(user_id=user_id, **input_data) + row: ORMApiKey = await api_keys.create_api_key(user_id=user.id, **input_data) is_internal = api_key_is_internal( api_key_data.domains, user_id=None, origin=origin, referrer=referrer diff --git a/tests_v2/utils.py b/tests_v2/utils.py index af43a336f..99d805688 100644 --- a/tests_v2/utils.py +++ b/tests_v2/utils.py @@ -33,11 +33,27 @@ def submit_batch_job(self, *args, **kwargs) -> uuid.UUID: async def get_user_mocked() -> Tuple[str, str]: - return "userid_123", "USER" + return User( + id="userid_123", + name="Ms. User", + email="ms_user@user.com", + createdAt="2021-06-13T03:18:23.000Z", + role="USER", + applications=[], + extraUserData={}, + ) async def get_admin_mocked() -> Tuple[str, str]: - return "adminid_123", "ADMIN" + return User( + id="adminid_123", + name="Sir Admin", + email="sir_admin@admin.com", + createdAt="2021-06-13T03:18:23.000Z", + role="ADMIN", + applications=[], + extraUserData={}, + ) async def get_manager_mocked() -> str: From 71d6f46253020570ba0c66737baf619b9813baf5 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Tue, 7 May 2024 13:51:29 -0700 Subject: [PATCH 6/8] Update all API key functions to use new interface --- app/routes/authentication/authentication.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/routes/authentication/authentication.py b/app/routes/authentication/authentication.py index 931a22a9e..7e9090798 100644 --- a/app/routes/authentication/authentication.py +++ b/app/routes/authentication/authentication.py @@ -122,13 +122,13 @@ async def get_api_key( User must own API Key or must be Admin to see details. """ - user_id, role = user.id, user.role + try: row: ORMApiKey = await api_keys.get_api_key(api_key) except RecordNotFoundError: raise HTTPException(status_code=404, detail="The API Key does not exist.") - if role != "ADMIN" and row.user_id != user_id: + if user.role != "ADMIN" and row.user_id != user.id: raise HTTPException( status_code=403, detail="API Key is not associated with current user." ) @@ -146,8 +146,7 @@ async def get_api_keys( Default keys are valid for one year """ - user_id = user.id - rows: List[ORMApiKey] = await api_keys.get_api_keys_from_user(user_id) + rows: List[ORMApiKey] = await api_keys.get_api_keys_from_user(user.id) data = [ApiKey.from_orm(row) for row in rows] return ApiKeysResponse(data=data) @@ -189,7 +188,6 @@ async def delete_api_key( API Key must belong to user. """ - user_id = user.id try: row: ORMApiKey = await api_keys.get_api_key(api_key) except RecordNotFoundError: @@ -198,7 +196,7 @@ async def delete_api_key( ) # TODO: we might want to allow admins to delete api keys of other users? - if not row.user_id == user_id: + if not row.user_id == user.id: raise HTTPException( status_code=403, detail="The requested API key does not belong to the current user.", From e5c8e38d8681836ec5845d9b43bc4b6c1db33313 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Wed, 8 May 2024 11:14:46 -0700 Subject: [PATCH 7/8] Fix making User model for check-auth response --- app/authentication/token.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/authentication/token.py b/app/authentication/token.py index 7f8cd8d24..c622a82d7 100644 --- a/app/authentication/token.py +++ b/app/authentication/token.py @@ -97,7 +97,7 @@ async def get_user(token: str = Depends(oauth2_scheme)) -> User: logger.info("Unauthorized user") raise HTTPException(status_code=401, detail="Unauthorized") else: - return User(**response.json()["data"]) + return User(**response.json()) async def get_admin(user: User = Depends(get_user)) -> User: From 4b5b704466f171f45912ba9d0daeee7ef2c43844 Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Wed, 8 May 2024 13:58:51 -0700 Subject: [PATCH 8/8] Don't check for application on auth --- app/authentication/token.py | 4 +--- tests/__init__.py | 2 +- tests_v2/utils.py | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/authentication/token.py b/app/authentication/token.py index c622a82d7..2e7163211 100644 --- a/app/authentication/token.py +++ b/app/authentication/token.py @@ -113,9 +113,7 @@ async def get_manager(user: User = Depends(get_user)) -> User: """Get the details for authenticated MANAGER for data-api application or ADMIN user.""" - if user.role != "ADMIN" or not ( - user.role == "MANAGER" and "data-api" in user.applications - ): + if user.role != "ADMIN" or user.role != "MANAGER": raise HTTPException(status_code=401, detail="Unauthorized") return user diff --git a/tests/__init__.py b/tests/__init__.py index 16b498565..624836f9e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -321,7 +321,7 @@ async def get_manager_mocked() -> User: email="mr_manager@management.com", createdAt="2021-06-13T03:18:23.000Z", role="MANAGER", - applications=["data-api"], + applications=[], extraUserData={}, ) diff --git a/tests_v2/utils.py b/tests_v2/utils.py index 99d805688..8d7ea4b65 100644 --- a/tests_v2/utils.py +++ b/tests_v2/utils.py @@ -63,7 +63,7 @@ async def get_manager_mocked() -> str: email="mr_manager@management.com", createdAt="2021-06-13T03:18:23.000Z", role="MANAGER", - applications=["data-api"], + applications=[], extraUserData={}, )