diff --git a/poetry.lock b/poetry.lock index a33e8b4..1162899 100644 --- a/poetry.lock +++ b/poetry.lock @@ -878,7 +878,6 @@ files = [ {file = "psycopg2_binary-2.9.9-cp311-cp311-win32.whl", hash = "sha256:dc4926288b2a3e9fd7b50dc6a1909a13bbdadfc67d93f3374d984e56f885579d"}, {file = "psycopg2_binary-2.9.9-cp311-cp311-win_amd64.whl", hash = "sha256:b76bedd166805480ab069612119ea636f5ab8f8771e640ae103e05a4aae3e417"}, {file = "psycopg2_binary-2.9.9-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:8532fd6e6e2dc57bcb3bc90b079c60de896d2128c5d9d6f24a63875a95a088cf"}, - {file = "psycopg2_binary-2.9.9-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:b0605eaed3eb239e87df0d5e3c6489daae3f7388d455d0c0b4df899519c6a38d"}, {file = "psycopg2_binary-2.9.9-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:8f8544b092a29a6ddd72f3556a9fcf249ec412e10ad28be6a0c0d948924f2212"}, {file = "psycopg2_binary-2.9.9-cp312-cp312-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:2d423c8d8a3c82d08fe8af900ad5b613ce3632a1249fd6a223941d0735fce493"}, {file = "psycopg2_binary-2.9.9-cp312-cp312-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:2e5afae772c00980525f6d6ecf7cbca55676296b580c0e6abb407f15f3706996"}, @@ -887,8 +886,6 @@ files = [ {file = "psycopg2_binary-2.9.9-cp312-cp312-musllinux_1_1_i686.whl", hash = "sha256:cb16c65dcb648d0a43a2521f2f0a2300f40639f6f8c1ecbc662141e4e3e1ee07"}, {file = "psycopg2_binary-2.9.9-cp312-cp312-musllinux_1_1_ppc64le.whl", hash = "sha256:911dda9c487075abd54e644ccdf5e5c16773470a6a5d3826fda76699410066fb"}, {file = "psycopg2_binary-2.9.9-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:57fede879f08d23c85140a360c6a77709113efd1c993923c59fde17aa27599fe"}, - {file = "psycopg2_binary-2.9.9-cp312-cp312-win32.whl", hash = "sha256:64cf30263844fa208851ebb13b0732ce674d8ec6a0c86a4e160495d299ba3c93"}, - {file = "psycopg2_binary-2.9.9-cp312-cp312-win_amd64.whl", hash = "sha256:81ff62668af011f9a48787564ab7eded4e9fb17a4a6a74af5ffa6a457400d2ab"}, {file = "psycopg2_binary-2.9.9-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:2293b001e319ab0d869d660a704942c9e2cce19745262a8aba2115ef41a0a42a"}, {file = "psycopg2_binary-2.9.9-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:03ef7df18daf2c4c07e2695e8cfd5ee7f748a1d54d802330985a78d2a5a6dca9"}, {file = "psycopg2_binary-2.9.9-cp37-cp37m-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:0a602ea5aff39bb9fac6308e9c9d82b9a35c2bf288e184a816002c9fae930b77"}, @@ -1318,7 +1315,7 @@ requests = "^2.31.0" type = "git" url = "https://github.com/cfpb/regtech-api-commons.git" reference = "HEAD" -resolved_reference = "e8e7c96532f76fcf72b6d864834cdc07d0fbd037" +resolved_reference = "e526c1ee2778c1f96a75d85ac9d22362da67dcb9" [[package]] name = "regtech-regex" diff --git a/src/regtech_user_fi_management/dependencies.py b/src/regtech_user_fi_management/dependencies.py index 76a01f5..512d03f 100644 --- a/src/regtech_user_fi_management/dependencies.py +++ b/src/regtech_user_fi_management/dependencies.py @@ -2,7 +2,7 @@ from http import HTTPStatus from typing import Annotated -from fastapi import Depends, Query, HTTPException, Request, Response +from fastapi import Depends, Query, Request, Response from fastapi.types import DecoratedCallable from sqlalchemy.ext.asyncio import AsyncSession from typing import List, Optional @@ -13,13 +13,18 @@ import regtech_user_fi_management.entities.repos.institutions_repo as repo from starlette.authentication import AuthCredentials from regtech_api_commons.models.auth import AuthenticatedUser +from regtech_api_commons.api.exceptions import RegTechHttpException async def check_domain(request: Request, session: Annotated[AsyncSession, Depends(get_session)]) -> None: if not request.user.is_authenticated: - raise HTTPException(status_code=HTTPStatus.FORBIDDEN) + raise RegTechHttpException( + status_code=HTTPStatus.FORBIDDEN, name="Request Forbidden", detail="unauthenticated user" + ) if await email_domain_denied(session, get_email_domain(request.user.email)): - raise HTTPException(status_code=HTTPStatus.FORBIDDEN, detail="email domain denied") + raise RegTechHttpException( + status_code=HTTPStatus.FORBIDDEN, name="Request Forbidden", detail="email domain denied" + ) async def email_domain_denied(session: AsyncSession, email: str) -> bool: @@ -58,7 +63,9 @@ async def wrapper(request: Request, *args, **kwargs) -> Response: user: AuthenticatedUser = request.user auth: AuthCredentials = request.auth if not is_admin(auth) and lei not in user.institutions: - raise HTTPException(HTTPStatus.FORBIDDEN, detail=f"LEI {lei} is not associated with the user.") + raise RegTechHttpException( + HTTPStatus.FORBIDDEN, name="Request Forbidden", detail=f"LEI {lei} is not associated with the user." + ) return await func(request, *args, **kwargs) return wrapper # type: ignore[return-value] @@ -67,15 +74,17 @@ async def wrapper(request: Request, *args, **kwargs) -> Response: def fi_search_association_check(func: DecoratedCallable) -> DecoratedCallable: def verify_leis(user: AuthenticatedUser, leis: List[str]) -> None: if not set(filter(len, leis)).issubset(set(filter(len, user.institutions))): - raise HTTPException( + raise RegTechHttpException( HTTPStatus.FORBIDDEN, + name="Request Forbidden", detail=f"Institutions query with LEIs ({leis}) not associated with user is forbidden.", ) def verify_domain(user: AuthenticatedUser, domain: str) -> None: if domain != get_email_domain(user.email): - raise HTTPException( + raise RegTechHttpException( HTTPStatus.FORBIDDEN, + name="Request Forbidden", detail=f"Institutions query with domain ({domain}) not associated with user is forbidden.", ) @@ -91,7 +100,11 @@ async def wrapper(request: Request, *args, **kwargs) -> Response: elif domain: verify_domain(user, domain) elif not leis and not domain: - raise HTTPException(HTTPStatus.FORBIDDEN, detail="Retrieving institutions without filter is forbidden.") + raise RegTechHttpException( + HTTPStatus.FORBIDDEN, + name="Request Forbidden", + detail="Retrieving institutions without filter is forbidden.", + ) return await func(request=request, *args, **kwargs) return wrapper # type: ignore[return-value] diff --git a/src/regtech_user_fi_management/main.py b/src/regtech_user_fi_management/main.py index fe88383..8887010 100644 --- a/src/regtech_user_fi_management/main.py +++ b/src/regtech_user_fi_management/main.py @@ -1,17 +1,24 @@ from contextlib import asynccontextmanager import os import logging -from http import HTTPStatus -from fastapi import FastAPI, HTTPException, Request -from fastapi.responses import JSONResponse +from fastapi import FastAPI +from fastapi.exceptions import RequestValidationError from fastapi.security import OAuth2AuthorizationCodeBearer from fastapi.middleware.cors import CORSMiddleware +from starlette.exceptions import HTTPException from starlette.middleware.authentication import AuthenticationMiddleware from alembic.config import Config from alembic import command from regtech_api_commons.oauth2.oauth2_backend import BearerTokenAuthBackend from regtech_api_commons.oauth2.oauth2_admin import OAuth2Admin +from regtech_api_commons.api.exceptions import RegTechHttpException +from regtech_api_commons.api.exception_handlers import ( + regtech_http_exception_handler, + request_validation_error_handler, + http_exception_handler, + general_exception_handler, +) from regtech_user_fi_management.config import kc_settings from regtech_user_fi_management.entities.listeners import setup_dao_listeners @@ -42,19 +49,10 @@ async def lifespan(app_: FastAPI): app = FastAPI(lifespan=lifespan) -@app.exception_handler(HTTPException) -async def http_exception_handler(request: Request, exception: HTTPException) -> JSONResponse: - log.error(exception, exc_info=True, stack_info=True) - return JSONResponse(status_code=exception.status_code, content={"detail": exception.detail}) - - -@app.exception_handler(Exception) -async def general_exception_handler(request: Request, exception: Exception) -> JSONResponse: - log.error(exception, exc_info=True, stack_info=True) - return JSONResponse( - status_code=HTTPStatus.INTERNAL_SERVER_ERROR, - content={"detail": "server error"}, - ) +app.add_exception_handler(RegTechHttpException, regtech_http_exception_handler) # type: ignore[type-arg] # noqa: E501 +app.add_exception_handler(RequestValidationError, request_validation_error_handler) # type: ignore[type-arg] # noqa: E501 +app.add_exception_handler(HTTPException, http_exception_handler) # type: ignore[type-arg] # noqa: E501 +app.add_exception_handler(Exception, general_exception_handler) # type: ignore[type-arg] # noqa: E501 oauth2_scheme = OAuth2AuthorizationCodeBearer( diff --git a/src/regtech_user_fi_management/routers/institutions.py b/src/regtech_user_fi_management/routers/institutions.py index dd534a6..515b94e 100644 --- a/src/regtech_user_fi_management/routers/institutions.py +++ b/src/regtech_user_fi_management/routers/institutions.py @@ -1,4 +1,4 @@ -from fastapi import Depends, Request, HTTPException, Response +from fastapi import Depends, Request, Response from http import HTTPStatus from regtech_api_commons.oauth2.oauth2_admin import OAuth2Admin from regtech_user_fi_management.config import kc_settings @@ -29,6 +29,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from starlette.authentication import requires from regtech_api_commons.models.auth import AuthenticatedUser +from regtech_api_commons.api.exceptions import RegTechHttpException oauth2_admin = OAuth2Admin(kc_settings) @@ -112,7 +113,7 @@ async def get_institution( ): res = await repo.get_institution(request.state.db_session, lei) if not res: - raise HTTPException(HTTPStatus.NOT_FOUND, f"{lei} not found.") + raise RegTechHttpException(HTTPStatus.NOT_FOUND, name="Institution Not Found", detail=f"{lei} not found.") return res @@ -127,7 +128,9 @@ async def get_types(request: Request, response: Response, lei: str, type: Instit else: response.status_code = HTTPStatus.NO_CONTENT case "hmda": - raise HTTPException(status_code=HTTPStatus.NOT_IMPLEMENTED, detail="HMDA type not yet supported") + raise RegTechHttpException( + status_code=HTTPStatus.NOT_IMPLEMENTED, name="Not Supported", detail="HMDA type not yet supported" + ) @router.put("/{lei}/types/{type}", response_model=VersionedData[List[SblTypeAssociationDetailsDto]] | None) @@ -141,11 +144,13 @@ async def update_types( if fi := await repo.update_sbl_types( request.state.db_session, request.user, lei, types_patch.sbl_institution_types ): - return VersionedData(version=fi.version, data=fi.sbl_institution_types) if fi else None + return VersionedData(version=fi.version, data=fi.sbl_institution_types) else: response.status_code = HTTPStatus.NO_CONTENT case "hmda": - raise HTTPException(status_code=HTTPStatus.NOT_IMPLEMENTED, detail="HMDA type not yet supported") + raise RegTechHttpException( + status_code=HTTPStatus.NOT_IMPLEMENTED, name="Not Supported", detail="HMDA type not yet supported" + ) @router.post("/{lei}/domains/", response_model=List[FinancialInsitutionDomainDto], dependencies=[Depends(check_domain)]) diff --git a/tests/api/routers/test_institutions_api.py b/tests/api/routers/test_institutions_api.py index 545749f..ed11e61 100644 --- a/tests/api/routers/test_institutions_api.py +++ b/tests/api/routers/test_institutions_api.py @@ -87,7 +87,8 @@ def test_invalid_tax_id(self, mocker: MockerFixture, app_fixture: FastAPI, authe }, ) assert ( - res.json()["detail"][0]["msg"] == f"Value error, Invalid tax_id 123456789. {regex_configs.tin.error_text}" + res.json()["error_detail"][0]["msg"] + == f"Value error, Invalid tax_id 123456789. {regex_configs.tin.error_text}" ) assert res.status_code == 422 @@ -119,7 +120,9 @@ def test_invalid_lei(self, mocker: MockerFixture, app_fixture: FastAPI, authed_u "top_holder_rssd_id": 123456, }, ) - assert res.json()["detail"][0]["msg"] == f"Value error, Invalid lei test_Lei. {regex_configs.lei.error_text}" + assert ( + res.json()["error_detail"][0]["msg"] == f"Value error, Invalid lei test_Lei. {regex_configs.lei.error_text}" + ) assert res.status_code == 422 def test_create_institution_authed(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock): @@ -254,7 +257,7 @@ def test_create_institution_missing_sbl_type_free_form( }, ) assert res.status_code == 422 - assert "requires additional details." in res.json()["detail"][0]["msg"] + assert "requires additional details." in res.json()["error_detail"][0]["msg"] def test_create_institution_authed_no_permission(self, app_fixture: FastAPI, auth_mock: Mock): claims = { @@ -392,7 +395,7 @@ def test_add_domains_authed_with_denied_email_domain( lei_path = "testLeiPath" res = client.post(f"/v1/institutions/{lei_path}/domains/", json=[{"domain": "testDomain"}]) assert res.status_code == 403 - assert "domain denied" in res.json()["detail"] + assert "domain denied" in res.json()["error_detail"] def test_check_domain_allowed(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock): domain_allowed_mock = mocker.patch( diff --git a/tests/app/test_dependencies.py b/tests/app/test_dependencies.py index 81df785..49ad2e1 100644 --- a/tests/app/test_dependencies.py +++ b/tests/app/test_dependencies.py @@ -4,6 +4,7 @@ from pytest_mock import MockerFixture from sqlalchemy.ext.asyncio import AsyncSession from regtech_user_fi_management.dependencies import lei_association_check, fi_search_association_check +from regtech_api_commons.api.exceptions import RegTechHttpException from starlette.authentication import AuthCredentials import pytest @@ -63,6 +64,7 @@ async def method_to_wrap(request: Request, lei: str): await method_to_wrap(mock_request, lei="NOTMYBANK") assert e.value.status_code == HTTPStatus.FORBIDDEN assert "not associated" in e.value.detail + assert isinstance(e.value, RegTechHttpException) async def test_fi_search_association_check_matching_lei(mock_request: Request): @@ -82,6 +84,7 @@ async def method_to_wrap(request: Request, leis: List[str] = [], domain: str = " await method_to_wrap(mock_request, leis=["NOTMYBANK"]) assert e.value.status_code == HTTPStatus.FORBIDDEN assert "not associated" in e.value.detail + assert isinstance(e.value, RegTechHttpException) async def test_fi_search_association_check_matching_domain(mock_request: Request): @@ -112,6 +115,7 @@ async def method_to_wrap(request: Request, leis: List[str] = [], domain: str = " await method_to_wrap(mock_request) assert e.value.status_code == HTTPStatus.FORBIDDEN assert "without filter" in e.value.detail + assert isinstance(e.value, RegTechHttpException) async def test_fi_search_association_check_lei_admin(mock_request: Request):