From 79086f353030ac72eaff00a25b5494b012c866e2 Mon Sep 17 00:00:00 2001 From: jamshale Date: Wed, 13 Sep 2023 11:31:28 -0700 Subject: [PATCH] Fix and refactor open api Signed-off-by: jamshale --- .../api/clientConfigurations/crud.py | 36 ++++++------- .../api/clientConfigurations/examples.py | 2 +- .../api/clientConfigurations/models.py | 16 +++--- .../api/clientConfigurations/router.py | 52 ++++++++++--------- .../api/core/http_exception_util.py | 28 ++++++++++ oidc-controller/api/core/models.py | 4 ++ .../api/verificationConfigs/crud.py | 30 +++++++---- .../api/verificationConfigs/examples.py | 15 +----- .../api/verificationConfigs/models.py | 20 +++---- .../api/verificationConfigs/router.py | 39 +++++++++----- 10 files changed, 134 insertions(+), 108 deletions(-) create mode 100644 oidc-controller/api/core/http_exception_util.py diff --git a/oidc-controller/api/clientConfigurations/crud.py b/oidc-controller/api/clientConfigurations/crud.py index 4f4f7eff..a483722b 100644 --- a/oidc-controller/api/clientConfigurations/crud.py +++ b/oidc-controller/api/clientConfigurations/crud.py @@ -3,19 +3,16 @@ from typing import List from pymongo import ReturnDocument from pymongo.database import Database -from fastapi import HTTPException -from fastapi import status as http_status from fastapi.encoders import jsonable_encoder +from ..core.http_exception_util import raise_appropriate_http_exception, check_and_raise_not_found_http_exception +from ..core.oidc.provider import init_provider +from ..db.session import COLLECTION_NAMES + from .models import ( ClientConfiguration, - ClientConfigurationCreate, ClientConfigurationPatch, - ClientConfigurationRead, ) -from ..db.session import COLLECTION_NAMES -from api.core.oidc.provider import init_provider - logger: structlog.typing.FilteringBoundLogger = structlog.getLogger(__name__) @@ -25,10 +22,14 @@ def __init__(self, db: Database): self._db = db async def create( - self, client_config: ClientConfigurationCreate + self, client_config: ClientConfiguration ) -> ClientConfiguration: col = self._db.get_collection(COLLECTION_NAMES.CLIENT_CONFIGURATIONS) - col.insert_one(jsonable_encoder(client_config)) + try: + col.insert_one(jsonable_encoder(client_config)) + except Exception as err: + raise_appropriate_http_exception( + err, exists_msg="Client configuration already exists") # remake provider instance to refresh provider client await init_provider(self._db) @@ -36,21 +37,16 @@ async def create( **col.find_one({"client_id": client_config.client_id}) ) - async def get(self, client_id: str) -> ClientConfigurationRead: + async def get(self, client_id: str) -> ClientConfiguration: col = self._db.get_collection(COLLECTION_NAMES.CLIENT_CONFIGURATIONS) obj = col.find_one({"client_id": client_id}) - - if obj is None: - raise HTTPException( - status_code=http_status.HTTP_404_NOT_FOUND, - detail="The client_config hasn't been found!", - ) + check_and_raise_not_found_http_exception(obj) return ClientConfiguration(**obj) - async def get_all(self) -> List[ClientConfigurationRead]: + async def get_all(self) -> List[ClientConfiguration]: col = self._db.get_collection(COLLECTION_NAMES.CLIENT_CONFIGURATIONS) - return [ClientConfigurationRead(**cc) for cc in col.find()] + return [ClientConfiguration(**cc) for cc in col.find()] async def patch( self, client_id: str, data: ClientConfigurationPatch @@ -61,6 +57,8 @@ async def patch( {"$set": data.dict(exclude_unset=True)}, return_document=ReturnDocument.AFTER, ) + check_and_raise_not_found_http_exception(obj) + # remake provider instance to refresh provider client await init_provider(self._db) return obj @@ -68,8 +66,8 @@ async def patch( async def delete(self, client_id: str) -> bool: col = self._db.get_collection(COLLECTION_NAMES.CLIENT_CONFIGURATIONS) obj = col.find_one_and_delete({"client_id": client_id}) + check_and_raise_not_found_http_exception(obj) # remake provider instance to refresh provider client await init_provider(self._db) - return bool(obj) diff --git a/oidc-controller/api/clientConfigurations/examples.py b/oidc-controller/api/clientConfigurations/examples.py index 914f7b49..30560035 100644 --- a/oidc-controller/api/clientConfigurations/examples.py +++ b/oidc-controller/api/clientConfigurations/examples.py @@ -1,6 +1,6 @@ from api.core.config import settings -ex_client_config_create = { +ex_client_config = { "client_id": settings.OIDC_CLIENT_ID, "client_name": settings.OIDC_CLIENT_NAME, "client_secret": "**********", diff --git a/oidc-controller/api/clientConfigurations/models.py b/oidc-controller/api/clientConfigurations/models.py index aeb3ea10..9b1aa852 100644 --- a/oidc-controller/api/clientConfigurations/models.py +++ b/oidc-controller/api/clientConfigurations/models.py @@ -1,10 +1,9 @@ from enum import Enum -from typing import List, Optional +from typing import Optional, List from pydantic import BaseModel, Field -from api.core.config import settings - -from .examples import ex_client_config_create +from .examples import ex_client_config +from ..core.config import settings class TOKENENDPOINTAUTHMETHODS(str, Enum): @@ -24,6 +23,7 @@ class ClientConfigurationBase(BaseModel): class Config: allow_population_by_field_name = True + schema_extra = {"example": ex_client_config} class ClientConfiguration(ClientConfigurationBase): @@ -34,16 +34,12 @@ class ClientConfigurationRead(ClientConfigurationBase): pass -class ClientConfigurationCreate(ClientConfigurationBase): - class Config: - schema_extra = {"example": ex_client_config_create} - - class ClientConfigurationPatch(ClientConfigurationBase): client_id: Optional[str] client_name: Optional[str] response_types: Optional[List[str]] redirect_uris: Optional[List[str]] token_endpoint_auth_method: Optional[TOKENENDPOINTAUTHMETHODS] - client_secret: Optional[str] + + pass diff --git a/oidc-controller/api/clientConfigurations/router.py b/oidc-controller/api/clientConfigurations/router.py index 63ea83f2..bac9556b 100644 --- a/oidc-controller/api/clientConfigurations/router.py +++ b/oidc-controller/api/clientConfigurations/router.py @@ -1,39 +1,42 @@ +from typing import List from pymongo.database import Database -from fastapi import APIRouter, HTTPException, Depends +from fastapi import APIRouter, Depends from fastapi import status as http_status -from ..core.models import StatusMessage - from .crud import ClientConfigurationCRUD from .models import ( - ClientConfigurationRead, + ClientConfiguration, ClientConfigurationPatch, - ClientConfigurationCreate, + ClientConfigurationRead, ) from ..core.auth import get_api_key +from ..core.models import GenericErrorMessage, StatusMessage from ..db.session import get_db + router = APIRouter() @router.post( "/", - response_description="Add new verification configuration", + response_description="Add new client configuration", status_code=http_status.HTTP_201_CREATED, - response_model=ClientConfigurationCreate, + response_model=ClientConfiguration, + responses={http_status.HTTP_409_CONFLICT: {"model": GenericErrorMessage}}, response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) async def create_client_config( - ver_config: ClientConfigurationCreate, db: Database = Depends(get_db) + client_config: ClientConfiguration, db: Database = Depends(get_db) ): - return await ClientConfigurationCRUD(db).create(ver_config) + return await ClientConfigurationCRUD(db).create(client_config) @router.get( "/", status_code=http_status.HTTP_200_OK, + response_model=List[ClientConfigurationRead], response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) @@ -42,43 +45,42 @@ async def get_all_client_configs(db: Database = Depends(get_db)): @router.get( - "/{client_config_id}", + "/{client_id}", status_code=http_status.HTTP_200_OK, response_model=ClientConfigurationRead, + responses={http_status.HTTP_404_NOT_FOUND: {"model": GenericErrorMessage}}, response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) -async def get_client_config(client_config_id: str, db: Database = Depends(get_db)): - return await ClientConfigurationCRUD(db).get(client_config_id) +async def get_client_config(client_id: str, db: Database = Depends(get_db)): + return await ClientConfigurationCRUD(db).get(client_id) @router.patch( - "/{client_config_id}", + "/{client_id}", status_code=http_status.HTTP_200_OK, response_model=ClientConfigurationRead, + responses={http_status.HTTP_404_NOT_FOUND: {"model": GenericErrorMessage}}, response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) async def patch_client_config( - client_config_id: str, + client_id: str, data: ClientConfigurationPatch, db: Database = Depends(get_db), ): - return await ClientConfigurationCRUD(db).patch(id=client_config_id, data=data) + return await ClientConfigurationCRUD(db).patch( + client_id=client_id, data=data + ) @router.delete( - "/{client_config_id}", + "/{client_id}", status_code=http_status.HTTP_200_OK, response_model=StatusMessage, + responses={http_status.HTTP_404_NOT_FOUND: {"model": GenericErrorMessage}}, dependencies=[Depends(get_api_key)], ) -async def delete_client_config(client_config_id: str, db: Database = Depends(get_db)): - status = await ClientConfigurationCRUD(db).delete(id=client_config_id) - - if not status: - raise HTTPException( - status_code=http_status.HTTP_404_NOT_FOUND, - detail="client_config does not exist", - ) - return StatusMessage(status=status, message="The client_config was deleted") +async def delete_client_config(client_id: str, db: Database = Depends(get_db)): + status = await ClientConfigurationCRUD(db).delete(client_id) + return StatusMessage(status=status, message="The client configuration was deleted") diff --git a/oidc-controller/api/core/http_exception_util.py b/oidc-controller/api/core/http_exception_util.py new file mode 100644 index 00000000..a57114eb --- /dev/null +++ b/oidc-controller/api/core/http_exception_util.py @@ -0,0 +1,28 @@ +from pymongo.errors import WriteError +from fastapi import HTTPException +from fastapi import status as http_status +import structlog + +logger = structlog.getLogger(__name__) + + +def raise_appropriate_http_exception(err: WriteError, exists_msg: str = None): + if err.code == 11000: + raise HTTPException( + status_code=http_status.HTTP_409_CONFLICT, + detail=exists_msg, + ) + else: + logger.error("Unknown error", err=err) + raise HTTPException( + status_code=http_status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="The server was unable to process the request", + ) + + +def check_and_raise_not_found_http_exception(resp): + if resp is None: + raise HTTPException( + status_code=http_status.HTTP_404_NOT_FOUND, + detail="The requested resource wasn't found", + ) diff --git a/oidc-controller/api/core/models.py b/oidc-controller/api/core/models.py index a9a0a4ff..9f36f42c 100644 --- a/oidc-controller/api/core/models.py +++ b/oidc-controller/api/core/models.py @@ -41,3 +41,7 @@ class Config: class TimestampModel(BaseModel): created_at: datetime = Field(default_factory=datetime.utcnow) updated_at: datetime = Field(default_factory=datetime.utcnow) + + +class GenericErrorMessage(BaseModel): + detail: str diff --git a/oidc-controller/api/verificationConfigs/crud.py b/oidc-controller/api/verificationConfigs/crud.py index c032bfc7..ed5ee421 100644 --- a/oidc-controller/api/verificationConfigs/crud.py +++ b/oidc-controller/api/verificationConfigs/crud.py @@ -1,10 +1,10 @@ -from fastapi import HTTPException -from fastapi import status as http_status +from typing import List from fastapi.encoders import jsonable_encoder from pymongo import ReturnDocument from pymongo.database import Database +from ..core.http_exception_util import raise_appropriate_http_exception, check_and_raise_not_found_http_exception from ..db.session import COLLECTION_NAMES from .models import ( @@ -21,36 +21,44 @@ def __init__(self, db: Database): async def create(self, ver_config: VerificationConfig) -> VerificationConfig: ver_confs = self._db.get_collection(COLLECTION_NAMES.VER_CONFIGS) - ver_confs.insert_one(jsonable_encoder(ver_config)) + + try: + ver_confs.insert_one(jsonable_encoder(ver_config)) + except Exception as err: + raise_appropriate_http_exception( + err, exists_msg="Verification configuration already exists") return ver_confs.find_one({"ver_config_id": ver_config.ver_config_id}) async def get(self, ver_config_id: str) -> VerificationConfig: ver_confs = self._db.get_collection(COLLECTION_NAMES.VER_CONFIGS) ver_conf = ver_confs.find_one({"ver_config_id": ver_config_id}) - - if ver_conf is None: - raise HTTPException( - status_code=http_status.HTTP_404_NOT_FOUND, - detail="The verification_config hasn't been found!", - ) + check_and_raise_not_found_http_exception(ver_conf) return VerificationConfig(**ver_conf) + async def get_all(self) -> List[VerificationConfig]: + ver_confs = self._db.get_collection(COLLECTION_NAMES.VER_CONFIGS) + return [VerificationConfig(**vc) for vc in ver_confs.find()] + async def patch( self, ver_config_id: str, data: VerificationConfigPatch ) -> VerificationConfig: if not isinstance(data, VerificationConfigPatch): - raise Exception("please provide an instance of the PATCH class") + raise Exception( + "please provide an instance of the PATCH class") ver_confs = self._db.get_collection(COLLECTION_NAMES.VER_CONFIGS) ver_conf = ver_confs.find_one_and_update( {"ver_config_id": ver_config_id}, {"$set": data.dict(exclude_unset=True)}, return_document=ReturnDocument.AFTER, ) + check_and_raise_not_found_http_exception(ver_conf) return ver_conf async def delete(self, ver_config_id: str) -> bool: ver_confs = self._db.get_collection(COLLECTION_NAMES.VER_CONFIGS) - ver_conf = ver_confs.find_one_and_delete({"ver_config_id": ver_config_id}) + ver_conf = ver_confs.find_one_and_delete( + {"ver_config_id": ver_config_id}) + check_and_raise_not_found_http_exception(ver_conf) return bool(ver_conf) diff --git a/oidc-controller/api/verificationConfigs/examples.py b/oidc-controller/api/verificationConfigs/examples.py index 5e3304bb..42a9a9f9 100644 --- a/oidc-controller/api/verificationConfigs/examples.py +++ b/oidc-controller/api/verificationConfigs/examples.py @@ -1,17 +1,4 @@ -ex_ver_config_read = { - "ver_config_id": "test-request-config", - "subject_identifier": "first_name", - "proof_request": { - "name": "Basic Proof", - "version": "1.0", - "requested_attributes": [ - {"names": ["first_name", "last_name"], "restrictions": []}, - ], - "requested_predicates": [], - }, -} - -ex_ver_config_create = { +ex_ver_config = { "ver_config_id": "test-request-config", "subject_identifier": "first_name", "proof_request": { diff --git a/oidc-controller/api/verificationConfigs/models.py b/oidc-controller/api/verificationConfigs/models.py index 21f7075a..db654678 100644 --- a/oidc-controller/api/verificationConfigs/models.py +++ b/oidc-controller/api/verificationConfigs/models.py @@ -2,7 +2,7 @@ from typing import Optional, List from pydantic import BaseModel, Field -from .examples import ex_ver_config_read, ex_ver_config_create +from .examples import ex_ver_config from ..core.config import settings @@ -51,7 +51,8 @@ def generate_proof_request(self): } for i, req_attr in enumerate(self.proof_request.requested_attributes): label = req_attr.label or "req_attr_" + str(i) - result["requested_attributes"][label] = req_attr.dict(exclude_none=True) + result["requested_attributes"][label] = req_attr.dict( + exclude_none=True) if settings.SET_NON_REVOKED: result["requested_attributes"][label]["non_revoked"] = { "from": int(time.time()), @@ -60,7 +61,8 @@ def generate_proof_request(self): # TODO add I indexing for req_pred in self.proof_request.requested_predicates: label = req_pred.label or "req_pred_" + str(i) - result["requested_predicates"][label] = req_pred.dict(exclude_none=True) + result["requested_predicates"][label] = req_pred.dict( + exclude_none=True) if settings.SET_NON_REVOKED: result["requested_attributes"][label]["non_revoked"] = { "from": int(time.time()), @@ -69,7 +71,7 @@ def generate_proof_request(self): return result class Config: - schema_extra = {"example": ex_ver_config_create} + schema_extra = {"example": ex_ver_config} class VerificationConfig(VerificationConfigBase): @@ -79,16 +81,6 @@ class VerificationConfig(VerificationConfigBase): class VerificationConfigRead(VerificationConfigBase): ver_config_id: str = Field() - class Config: - schema_extra = {"example": ex_ver_config_read} - - -class VerificationConfigCreate(VerificationConfigBase): - ver_config_id: str = Field() - - class Config: - schema_extra = {"example": ex_ver_config_create} - class VerificationConfigPatch(VerificationConfigBase): subject_identifier: Optional[str] = Field() diff --git a/oidc-controller/api/verificationConfigs/router.py b/oidc-controller/api/verificationConfigs/router.py index 72d7b5bb..5fbfb31d 100644 --- a/oidc-controller/api/verificationConfigs/router.py +++ b/oidc-controller/api/verificationConfigs/router.py @@ -1,17 +1,17 @@ +from typing import List from pymongo.database import Database -from fastapi import APIRouter, HTTPException, Depends +from fastapi import APIRouter, Depends from fastapi import status as http_status -from ..core.models import StatusMessage - from .crud import VerificationConfigCRUD from .models import ( + VerificationConfig, VerificationConfigPatch, VerificationConfigRead, - VerificationConfig, ) from ..core.auth import get_api_key +from ..core.models import GenericErrorMessage, StatusMessage from ..db.session import get_db router = APIRouter() @@ -19,9 +19,10 @@ @router.post( "/", - response_description="Add new verification configuration", + response_description="Add new verifier configuration", status_code=http_status.HTTP_201_CREATED, response_model=VerificationConfig, + responses={http_status.HTTP_409_CONFLICT: {"model": GenericErrorMessage}}, response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) @@ -31,10 +32,22 @@ async def create_ver_config( return await VerificationConfigCRUD(db).create(ver_config) +@router.get( + "/", + status_code=http_status.HTTP_200_OK, + response_model=List[VerificationConfigRead], + response_model_exclude_unset=True, + dependencies=[Depends(get_api_key)], +) +async def get_all_ver_configs(db: Database = Depends(get_db)): + return await VerificationConfigCRUD(db).get_all() + + @router.get( "/{ver_config_id}", status_code=http_status.HTTP_200_OK, response_model=VerificationConfigRead, + responses={http_status.HTTP_404_NOT_FOUND: {"model": GenericErrorMessage}}, response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) @@ -46,11 +59,14 @@ async def get_ver_conf(ver_config_id: str, db: Database = Depends(get_db)): "/{ver_config_id}", status_code=http_status.HTTP_200_OK, response_model=VerificationConfigRead, + responses={http_status.HTTP_404_NOT_FOUND: {"model": GenericErrorMessage}}, response_model_exclude_unset=True, dependencies=[Depends(get_api_key)], ) async def patch_ver_conf( - ver_config_id: str, data: VerificationConfigPatch, db: Database = Depends(get_db) + ver_config_id: str, + data: VerificationConfigPatch, + db: Database = Depends(get_db), ): return await VerificationConfigCRUD(db).patch( ver_config_id=ver_config_id, data=data @@ -61,14 +77,9 @@ async def patch_ver_conf( "/{ver_config_id}", status_code=http_status.HTTP_200_OK, response_model=StatusMessage, + responses={http_status.HTTP_404_NOT_FOUND: {"model": GenericErrorMessage}}, dependencies=[Depends(get_api_key)], ) async def delete_ver_conf_by_uuid(ver_config_id: str, db: Database = Depends(get_db)): - status = await VerificationConfigCRUD(db).delete(ver_config_id=ver_config_id) - - if not status: - raise HTTPException( - status_code=http_status.HTTP_404_NOT_FOUND, - detail="ver_config does not exist", - ) - return StatusMessage(status=status, message="The ver_config was deleted") + status = await VerificationConfigCRUD(db).delete(ver_config_id) + return StatusMessage(status=status, message="The verifier config was deleted")