From 49e1785d06b8d056d5df578e19a7a7992c270ea2 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 15 Nov 2024 14:58:25 -0400 Subject: [PATCH] Handle auth exception --- src/palace/manager/api/controller/base.py | 44 ++++++-------- tests/manager/api/controller/test_base.py | 73 ++++++++++++----------- 2 files changed, 57 insertions(+), 60 deletions(-) diff --git a/src/palace/manager/api/controller/base.py b/src/palace/manager/api/controller/base.py index 3a6badbbd4..951bc7e3d4 100644 --- a/src/palace/manager/api/controller/base.py +++ b/src/palace/manager/api/controller/base.py @@ -4,15 +4,11 @@ from werkzeug.datastructures import Authorization from palace.manager.api.circulation_exceptions import RemoteInitiatedServerError -from palace.manager.api.problem_details import ( - INVALID_CREDENTIALS, - LIBRARY_NOT_FOUND, - REMOTE_INTEGRATION_FAILED, -) +from palace.manager.api.problem_details import INVALID_CREDENTIALS, LIBRARY_NOT_FOUND from palace.manager.sqlalchemy.model.library import Library from palace.manager.sqlalchemy.model.patron import Patron from palace.manager.util.log import LoggerMixin -from palace.manager.util.problem_detail import ProblemDetail +from palace.manager.util.problem_detail import BaseProblemDetailException, ProblemDetail class BaseCirculationManagerController(LoggerMixin): @@ -61,7 +57,7 @@ def authenticated_patron_from_request(self) -> ProblemDetail | Patron | Response authentication, a ProblemDetail. """ # Start off by assuming authentication will not work. - flask.request.patron = None # type: ignore[attr-defined] + setattr(flask.request, "patron", None) auth = self.authorization_header() @@ -69,21 +65,15 @@ def authenticated_patron_from_request(self) -> ProblemDetail | Patron | Response # No credentials were provided. return self.authenticate() - try: - patron = self.authenticated_patron(auth) - except RemoteInitiatedServerError as e: - return REMOTE_INTEGRATION_FAILED.detailed( - _("Error in authentication service") - ) - if patron is None: - # Credentials were provided but they turned out not - # to identify anyone in particular. - return self.authenticate() + patron = self.authenticated_patron(auth) if isinstance(patron, Patron): - flask.request.patron = patron # type: ignore[attr-defined] + setattr(flask.request, "patron", patron) + return patron - def authenticated_patron(self, authorization_header: Authorization): + def authenticated_patron( + self, authorization_header: Authorization + ) -> Patron | ProblemDetail: """Look up the patron authenticated by the given authorization header. The header could contain a barcode and pin or a token for an @@ -93,12 +83,16 @@ def authenticated_patron(self, authorization_header: Authorization): If there's no problem, return a Patron object. """ - patron = self.manager.auth.authenticated_patron(self._db, authorization_header) - if not patron: - return INVALID_CREDENTIALS - - if isinstance(patron, ProblemDetail): - return patron + try: + patron = self.manager.auth.authenticated_patron( + self._db, authorization_header + ) + if not patron: + return INVALID_CREDENTIALS + except RemoteInitiatedServerError as e: + return e.problem_detail.detailed(_("Error in authentication service")) + except BaseProblemDetailException as e: + return e.problem_detail return patron diff --git a/tests/manager/api/controller/test_base.py b/tests/manager/api/controller/test_base.py index 470d50b5cd..2050416710 100644 --- a/tests/manager/api/controller/test_base.py +++ b/tests/manager/api/controller/test_base.py @@ -84,52 +84,55 @@ def test_authenticated_patron_from_request( assert circulation_fixture.default_patron == flask.request.patron # type: ignore # No authorization header -> 401 error. - with patch( - "palace.manager.api.controller.base.BaseCirculationManagerController.authorization_header", - lambda x: None, + with ( + patch.object( + circulation_fixture.controller, + "authorization_header", + lambda: None, + ), + circulation_fixture.request_context_with_library("/"), ): - with circulation_fixture.request_context_with_library("/"): - result = ( - circulation_fixture.controller.authenticated_patron_from_request() - ) - assert isinstance(result, Response) - assert 401 == result.status_code - assert None == flask.request.patron # type: ignore + result = circulation_fixture.controller.authenticated_patron_from_request() + assert isinstance(result, Response) + assert 401 == result.status_code + assert None == flask.request.patron # type: ignore # Exception contacting the authentication authority -> ProblemDetail def remote_failure(self, header): raise RemoteInitiatedServerError("argh", "service") - with patch( - "palace.manager.api.controller.base.BaseCirculationManagerController.authenticated_patron", - remote_failure, - ): - with circulation_fixture.request_context_with_library( + with ( + patch.object( + circulation_fixture.manager.auth, + "authenticated_patron", + remote_failure, + ), + circulation_fixture.request_context_with_library( "/", headers=dict(Authorization=circulation_fixture.valid_auth) - ): - result = ( - circulation_fixture.controller.authenticated_patron_from_request() - ) - assert isinstance(result, ProblemDetail) - assert REMOTE_INTEGRATION_FAILED.uri == result.uri - assert "Error in authentication service" == result.detail - assert None == flask.request.patron # type: ignore + ), + ): + result = circulation_fixture.controller.authenticated_patron_from_request() + assert isinstance(result, ProblemDetail) + assert REMOTE_INTEGRATION_FAILED.uri == result.uri + assert "Error in authentication service" == result.detail + assert None == flask.request.patron # type: ignore # Credentials provided but don't identify anyone in particular # -> 401 error. - with patch( - "palace.manager.api.controller.base.BaseCirculationManagerController.authenticated_patron", - lambda self, x: None, - ): - with circulation_fixture.request_context_with_library( + with ( + patch.object( + circulation_fixture.manager.auth, + "authenticated_patron", + lambda db, header: None, + ), + circulation_fixture.request_context_with_library( "/", headers=dict(Authorization=circulation_fixture.valid_auth) - ): - result = ( - circulation_fixture.controller.authenticated_patron_from_request() - ) - assert isinstance(result, Response) - assert 401 == result.status_code - assert None == flask.request.patron # type: ignore + ), + ): + result = circulation_fixture.controller.authenticated_patron_from_request() + assert isinstance(result, ProblemDetail) + assert 401 == result.status_code + assert None == flask.request.patron # type: ignore def test_authenticated_patron_invalid_credentials( self, circulation_fixture: CirculationControllerFixture