Improve error handling. Fix logout endpoint. Handle sign-in and MFA errors.

.github/workflows/i18n.yml

@@ -20,7 +20,7 @@ jobs:
       - run: pip install -r requirements.txt .
       - name: Update catalogs
         run: |
-          pybabel extract -k '_ i' -o messages.pot .
+          pybabel extract -k '_ i' -o messages.pot app
           pybabel update -N -i messages.pot -d locale
       - name: Count incomplete translations
         shell: bash

app/auth.py

@@ -68,7 +68,7 @@ def verify_jwk_token(self, jwt_credentials: JWTAuthorizationCredentials) -> bool
 
         return alg_obj.verify(msg, prepared_key, sig)
 
-    async def __call__(self, request: Request) -> JWTAuthorizationCredentials | None:
+    async def __call__(self, request: Request) -> JWTAuthorizationCredentials:
         """
         Authenticate and verify the provided JWT token in the request.
 
@@ -86,7 +86,13 @@ async def __call__(self, request: Request) -> JWTAuthorizationCredentials | None
 
             jwt_token = credentials.credentials
 
-            message, signature = jwt_token.rsplit(".", 1)
+            if "." in jwt_token:
+                message, signature = jwt_token.rsplit(".", 1)
+            else:
+                raise HTTPException(
+                    status_code=status.HTTP_403_FORBIDDEN,
+                    detail=_("JWK invalid"),
+                )
 
             try:
                 jwt_credentials = JWTAuthorizationCredentials(

app/dependencies.py

@@ -25,7 +25,7 @@ def get_aws_client() -> Generator[aws.Client, None, None]:
 
 
 async def get_auth_credentials(request: Request) -> auth.JWTAuthorizationCredentials | None:
-    return await auth.JWTAuthorization().__call__(request)
+    return await auth.JWTAuthorization()(request)
 
 
 async def get_current_user(credentials: auth.JWTAuthorizationCredentials = Depends(get_auth_credentials)) -> str:
@@ -57,16 +57,16 @@ async def get_user(username: str = Depends(get_current_user), session: Session =
     user = models.User.first_by(session, "external_id", username)
     if not user:
         raise HTTPException(
-            status_code=status.HTTP_403_FORBIDDEN,
+            status_code=status.HTTP_404_NOT_FOUND,
             detail=_("User not found"),
         )
     return user
 
 
 async def get_admin_user(user: models.User = Depends(get_user)) -> models.User:
-    if not user.is_ocp():
+    if not user.is_admin():
         raise HTTPException(
-            status_code=status.HTTP_401_UNAUTHORIZED,
+            status_code=status.HTTP_403_FORBIDDEN,
             detail=_("Insufficient permissions"),
         )
     return user
@@ -85,7 +85,7 @@ def raise_if_unauthorized(
         for role in roles:
             match role:
                 case models.UserType.OCP:
-                    if user.is_ocp():
+                    if user.is_admin():
                         break
                 case models.UserType.FI:
                     if user.lender_id == application.lender_id:
@@ -102,14 +102,14 @@ def raise_if_unauthorized(
         expired_at = application.expired_at
         if expired_at and expired_at < datetime.now(expired_at.tzinfo):
             raise HTTPException(
-                status_code=status.HTTP_409_CONFLICT,
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                 detail=_("Application expired"),
             )
 
     if statuses:
         if application.status not in statuses:
             raise HTTPException(
-                status_code=status.HTTP_409_CONFLICT,
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                 detail=_("Application status should not be %(status)s", status=_(application.status)),
             )
 
@@ -168,7 +168,7 @@ def _get_application_as_guest_via_uuid(session: Session, uuid: str) -> models.Ap
 
     if application.status == models.ApplicationStatus.LAPSED:
         raise HTTPException(
-            status_code=status.HTTP_409_CONFLICT,
+            status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
             detail=_("Application lapsed"),
         )
 

app/main.py

@@ -1,6 +1,8 @@
-from fastapi import FastAPI
+from fastapi import FastAPI, Request
 from fastapi.middleware.cors import CORSMiddleware
+from fastapi.responses import JSONResponse
 
+from app.i18n import _
 from app.routers import applications, downloads, guest, lenders, statistics, users
 from app.settings import app_settings
 
@@ -21,3 +23,8 @@
 app.include_router(downloads.router)
 app.include_router(lenders.router)
 app.include_router(statistics.router)
+
+
+@app.exception_handler(500)
+async def http_exception_handler(request: Request, exc: Exception) -> JSONResponse:
+    return JSONResponse({"detail": _("An unexpected error occurred")}, status_code=500)

app/models.py

@@ -1015,7 +1015,7 @@ class UserBase(SQLModel):
         default=datetime.utcnow(), sa_column=Column(DateTime(timezone=True), nullable=False, server_default=func.now())
     )
 
-    def is_ocp(self) -> bool:
+    def is_admin(self) -> bool:
         return self.type == UserType.OCP
 
 

app/routers/applications.py

@@ -1,8 +1,6 @@
-import logging
 from datetime import datetime
 from typing import Any, cast
 
-from botocore.exceptions import ClientError
 from fastapi import APIRouter, Depends, HTTPException, Query, status
 from fastapi.encoders import jsonable_encoder
 from sqlalchemy.orm import Session, joinedload
@@ -13,8 +11,6 @@
 from app.i18n import _
 from app.util import SortOrder, get_order_by
 
-logger = logging.getLogger(__name__)
-
 router = APIRouter()
 
 
@@ -171,7 +167,7 @@ async def approve_application(
                 not_validated_fields.append(key)
         if not_validated_fields:
             raise HTTPException(
-                status_code=status.HTTP_409_CONFLICT,
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                 detail=_("Some borrower data field are not verified"),
             )
 
@@ -182,7 +178,7 @@ async def approve_application(
                 not_validated_documents.append(document.type)
         if not_validated_documents:
             raise HTTPException(
-                status_code=status.HTTP_409_CONFLICT,
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
                 detail=_("Some documents are not verified"),
             )
 
@@ -458,7 +454,7 @@ async def get_application(
     Retrieve an application by its ID.
 
     :return: The application with the specified ID and its associated relations.
-    :raise: HTTPException with status code 401 if the user is not authorized to view the application.
+    :raise: HTTPException if the user is not authorized to view the application.
     """
     return util.get_modified_data_fields(session, application)
 
@@ -485,7 +481,7 @@ async def start_application(
 
     :param id: The ID of the application to start.
     :return: The started application with its associated relations.
-    :raise: HTTPException with status code 401 if the user is not authorized to start the application.
+    :raise: HTTPException if the user is not authorized to start the application.
     """
     with rollback_on_error(session):
         application.status = models.ApplicationStatus.STARTED
@@ -576,14 +572,7 @@ async def email_borrower(
             user_id=user.id,
         )
 
-        try:
-            message_id = mail.send_mail_request_to_borrower(client.ses, application, payload.message)
-        except ClientError as e:
-            logger.exception(e)
-            return HTTPException(
-                status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-                detail=_("There was an error"),
-            )
+        message_id = mail.send_mail_request_to_borrower(client.ses, application, payload.message)
         models.Message.create(
             session,
             application_id=application.id,
@@ -612,6 +601,6 @@ async def previous_contracts(
     Get the previous awards associated with an application.
 
     :return: A list of previous awards associated with the application.
-    :raise: HTTPException with status code 401 if the user is not authorized to access the application.
+    :raise: HTTPException if the user is not authorized to access the application.
     """
     return application.previous_awards(session)

app/routers/downloads.py

@@ -41,7 +41,7 @@ async def get_borrower_document(
             session,
             type=(
                 models.ApplicationActionType.OCP_DOWNLOAD_DOCUMENT
-                if user.is_ocp()
+                if user.is_admin()
                 else models.ApplicationActionType.FI_DOWNLOAD_DOCUMENT
             ),
             data={"file_name": document.name},
@@ -116,7 +116,7 @@ async def download_application(
             session,
             type=(
                 models.ApplicationActionType.OCP_DOWNLOAD_APPLICATION
-                if user.is_ocp()
+                if user.is_admin()
                 else models.ApplicationActionType.FI_DOWNLOAD_APPLICATION
             ),
             application_id=application.id,