Handle auth exception

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,29 +57,23 @@ 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()
 
         if not auth:
             # 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
 

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