Merge pull request #344 from bcgov/feat/335-improve-error-handling

Feat/335 improve error handling

oidc-controller/api/clientConfigurations/crud.py

@@ -16,6 +16,7 @@
 
 logger: structlog.typing.FilteringBoundLogger = structlog.getLogger(__name__)
 
+NOT_FOUND_MSG = "The requested client configuration wasn't found"
 
 class ClientConfigurationCRUD:
     def __init__(self, db: Database):
@@ -40,7 +41,7 @@ async def create(
     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})
-        check_and_raise_not_found_http_exception(obj)
+        check_and_raise_not_found_http_exception(obj, NOT_FOUND_MSG)
 
         return ClientConfiguration(**obj)
 
@@ -57,7 +58,7 @@ async def patch(
             {"$set": data.dict(exclude_unset=True)},
             return_document=ReturnDocument.AFTER,
         )
-        check_and_raise_not_found_http_exception(obj)
+        check_and_raise_not_found_http_exception(obj, NOT_FOUND_MSG)
 
         # remake provider instance to refresh provider client
         await init_provider(self._db)
@@ -66,7 +67,7 @@ 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)
+        check_and_raise_not_found_http_exception(obj, NOT_FOUND_MSG)
 
         # remake provider instance to refresh provider client
         await init_provider(self._db)

oidc-controller/api/core/http_exception_util.py

@@ -5,8 +5,11 @@
 
 logger = structlog.getLogger(__name__)
 
+CONFLICT_DEFAULT_MSG = "The requested resource already exists"
+NOT_FOUND_DEFAULT_MSG = "The requested resource wasn't found"
+UNKNOWN_DEFAULT_MSG = "The server was unable to process the request"
 
-def raise_appropriate_http_exception(err: WriteError, exists_msg: str = None):
+def raise_appropriate_http_exception(err: WriteError, exists_msg: str = CONFLICT_DEFAULT_MSG):
     if err.code == 11000:
         raise HTTPException(
             status_code=http_status.HTTP_409_CONFLICT,
@@ -16,13 +19,13 @@ def raise_appropriate_http_exception(err: WriteError, exists_msg: str = None):
         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",
+            detail=UNKNOWN_DEFAULT_MSG,
         )
 
 
-def check_and_raise_not_found_http_exception(resp):
+def check_and_raise_not_found_http_exception(resp, detail: str = NOT_FOUND_DEFAULT_MSG):
     if resp is None:
         raise HTTPException(
             status_code=http_status.HTTP_404_NOT_FOUND,
-            detail="The requested resource wasn't found",
+            detail=detail,
         )

oidc-controller/api/routers/oidc.py

@@ -5,11 +5,13 @@
 
 import qrcode
 import structlog
-from fastapi import APIRouter, Depends, Request
+from fastapi import APIRouter, Depends, HTTPException, Request
 from fastapi.responses import HTMLResponse, JSONResponse, RedirectResponse
+from fastapi import status as http_status
 from jinja2 import Template
 from oic.oic.message import AccessTokenRequest, AuthorizationRequest
 from pymongo.database import Database
+from pyop.exceptions import InvalidAuthenticationRequest
 
 from ..authSessions.crud import AuthSessionCreate, AuthSessionCRUD
 from ..authSessions.models import AuthSessionPatch, AuthSessionState
@@ -80,9 +82,15 @@ async def get_authorize(request: Request, db: Database = Depends(get_db)):
     model = AuthorizationRequest().from_dict(request.query_params._dict)
     model.verify()
 
-    auth_req = provider.provider.parse_authentication_request(
-        urlencode(request.query_params._dict), request.headers
-    )
+    try:
+        auth_req = provider.provider.parse_authentication_request(
+            urlencode(request.query_params._dict), request.headers
+        )
+    except InvalidAuthenticationRequest as e:
+        raise HTTPException(
+            status_code=http_status.HTTP_400_BAD_REQUEST, 
+            detail=f"Invalid auth request: {e}")
+
     #  fetch placeholder user/model and create proof
     authn_response = provider.provider.authorize(model, "vc-user")
 

oidc-controller/api/verificationConfigs/crud.py

@@ -12,6 +12,7 @@
     VerificationConfigPatch,
 )
 
+NOT_FOUND_MSG = "The requested verifier configuration wasn't found"
 
 class VerificationConfigCRUD:
     _db: Database
@@ -26,13 +27,13 @@ async def create(self, ver_config: VerificationConfig) -> VerificationConfig:
             ver_confs.insert_one(jsonable_encoder(ver_config))
         except Exception as err:
             raise_appropriate_http_exception(
-                err, exists_msg="Verification configuration already exists")
+                err, exists_msg="Verifier 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})
-        check_and_raise_not_found_http_exception(ver_conf)
+        check_and_raise_not_found_http_exception(ver_conf, NOT_FOUND_MSG)
 
         return VerificationConfig(**ver_conf)
 
@@ -52,13 +53,13 @@ async def patch(
             {"$set": data.dict(exclude_unset=True)},
             return_document=ReturnDocument.AFTER,
         )
-        check_and_raise_not_found_http_exception(ver_conf)
+        check_and_raise_not_found_http_exception(ver_conf, NOT_FOUND_MSG)
 
         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})
-        check_and_raise_not_found_http_exception(ver_conf)
+        check_and_raise_not_found_http_exception(ver_conf, NOT_FOUND_MSG)
         return bool(ver_conf)