Helper function to get flask.request.admin (#2179)

* Function to load flask.request.admin

* Code review feedback

src/palace/manager/api/admin/controller/base.py

@@ -15,6 +15,7 @@
     INVALID_ADMIN_CREDENTIALS,
     INVALID_CSRF_TOKEN,
 )
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.sqlalchemy.model.admin import Admin, AdminRole
 from palace.manager.sqlalchemy.model.library import Library
 from palace.manager.sqlalchemy.util import get_one, get_one_or_create
@@ -45,6 +46,7 @@ def admin_auth_provider(self, type):
 
     def authenticated_admin_from_request(self):
         """Returns an authenticated admin or a problem detail."""
+        setattr(flask.request, "admin", None)
         if not self.admin_auth_providers:
             return ADMIN_AUTH_NOT_CONFIGURED
 
@@ -57,9 +59,8 @@ def authenticated_admin_from_request(self):
             if not auth:
                 return ADMIN_AUTH_MECHANISM_NOT_CONFIGURED
             if admin:
-                flask.request.admin = admin
+                setattr(flask.request, "admin", admin)
                 return admin
-        flask.request.admin = None
         return INVALID_ADMIN_CREDENTIALS
 
     def authenticated_admin(self, admin_details) -> Admin:
@@ -119,21 +120,21 @@ class AdminPermissionsControllerMixin:
     """Mixin that provides methods for verifying an admin's roles."""
 
     def require_system_admin(self) -> None:
-        admin: Admin | None = getattr(flask.request, "admin", None)
+        admin = get_request_admin(default=None)
         if not admin or not admin.is_system_admin():
             raise AdminNotAuthorized()
 
     def require_sitewide_library_manager(self) -> None:
-        admin: Admin | None = getattr(flask.request, "admin", None)
+        admin = get_request_admin(default=None)
         if not admin or not admin.is_sitewide_library_manager():
             raise AdminNotAuthorized()
 
     def require_library_manager(self, library: Library) -> None:
-        admin: Admin | None = getattr(flask.request, "admin", None)
+        admin = get_request_admin(default=None)
         if not admin or not admin.is_library_manager(library):
             raise AdminNotAuthorized()
 
     def require_librarian(self, library: Library) -> None:
-        admin: Admin | None = getattr(flask.request, "admin", None)
+        admin = get_request_admin(default=None)
         if not admin or not admin.is_librarian(library):
             raise AdminNotAuthorized()

src/palace/manager/api/admin/controller/collection_settings.py

@@ -17,6 +17,7 @@
     MISSING_SERVICE,
     PROTOCOL_DOES_NOT_SUPPORT_PARENTS,
 )
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.api.circulation import CirculationApiType
 from palace.manager.celery.tasks.collection_delete import collection_delete
 from palace.manager.core.selftest import HasSelfTests
@@ -40,7 +41,7 @@ def configured_service_info(
         self, service: IntegrationConfiguration
     ) -> dict[str, Any] | None:
         service_info = super().configured_service_info(service)
-        user = getattr(flask.request, "admin", None)
+        user = get_request_admin(default=None)
         if service_info:
             # Add 'marked_for_deletion' to the service info
             service_info["marked_for_deletion"] = service.collection.marked_for_deletion
@@ -60,7 +61,7 @@ def configured_service_library_info(
         self, library_configuration: IntegrationLibraryConfiguration
     ) -> dict[str, Any] | None:
         library_info = super().configured_service_library_info(library_configuration)
-        user = getattr(flask.request, "admin", None)
+        user = get_request_admin(default=None)
         if library_info:
             if user and user.is_librarian(library_configuration.library):
                 return library_info

src/palace/manager/api/admin/controller/dashboard.py

@@ -8,6 +8,7 @@
 from sqlalchemy.orm import Session
 
 from palace.manager.api.admin.model.dashboard_statistics import StatisticsResponse
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.api.controller.circulation_manager import (
     CirculationManagerController,
 )
@@ -26,7 +27,7 @@ class DashboardController(CirculationManagerController):
     def stats(
         self, stats_function: Callable[[Admin, Session], StatisticsResponse]
     ) -> StatisticsResponse:
-        admin: Admin = getattr(flask.request, "admin")
+        admin = get_request_admin()
         return stats_function(admin, self._db)
 
     def circulation_events(self):

src/palace/manager/api/admin/controller/individual_admin_settings.py

@@ -17,6 +17,7 @@
     MISSING_PGCRYPTO_EXTENSION,
     UNKNOWN_ROLE,
 )
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.api.problem_details import LIBRARY_NOT_FOUND
 from palace.manager.sqlalchemy.model.admin import Admin, AdminRole
 from palace.manager.sqlalchemy.model.library import Library
@@ -38,7 +39,7 @@ def _highest_authorized_role(self) -> AdminRole | None:
         highest_role: AdminRole | None = None
         has_auth = False
 
-        admin = getattr(flask.request, "admin", None)
+        admin = get_request_admin(default=None)
 
         if not admin:
             return None
@@ -61,7 +62,7 @@ def _highest_authorized_role(self) -> AdminRole | None:
         return highest_role if has_auth else None
 
     def process_get(self):
-        logged_in_admin: Admin | None = getattr(flask.request, "admin", None)
+        logged_in_admin = get_request_admin(default=None)
         if not logged_in_admin:
             return ADMIN_AUTH_NOT_CONFIGURED
 
@@ -271,7 +272,7 @@ def check_permissions(self, admin, settingUp):
         # which the user is submitting the form in order to create/edit.)
 
         if not settingUp:
-            user = flask.request.admin
+            user = get_request_admin()
 
             # System admin has all permissions.
             if user.is_system_admin():
@@ -336,7 +337,7 @@ def handle_roles(self, admin, roles, settingUp):
             # There are no admins yet; the user and the new system admin are the same person.
             user = admin
         else:
-            user = flask.request.admin
+            user = get_request_admin()
 
         old_roles = admin.roles
         old_roles_set = {(role.role, role.library) for role in old_roles}
@@ -389,7 +390,7 @@ def handle_password(self, password, admin: Admin, is_new, settingUp):
             # There are no admins yet; the user and the new system admin are the same person.
             user = admin
         else:
-            user: Admin = flask.request.admin  # type: ignore
+            user = get_request_admin()
 
         if password:
             # If the admin we're editing has a sitewide manager role, we've already verified

src/palace/manager/api/admin/controller/library_settings.py

@@ -22,6 +22,7 @@
     INVALID_CONFIGURATION_OPTION,
     LIBRARY_SHORT_NAME_ALREADY_IN_USE,
 )
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.api.circulation_manager import CirculationManager
 from palace.manager.api.config import Configuration
 from palace.manager.api.lanes import create_default_lanes
@@ -58,10 +59,11 @@ def process_libraries(self) -> Response | ProblemDetail:
     def process_get(self) -> Response:
         libraries_response = []
         libraries = self._db.query(Library).order_by(Library.name).all()
+        admin = get_request_admin(default=None)
 
         for library in libraries:
             # Only include libraries this admin has librarian access to.
-            if not flask.request.admin or not flask.request.admin.is_librarian(library):  # type: ignore[attr-defined]
+            if not admin or not admin.is_librarian(library):
                 continue
 
             settings = library.settings_dict

src/palace/manager/api/admin/controller/quicksight.py

@@ -8,6 +8,7 @@
     QuicksightGenerateUrlRequest,
     QuicksightGenerateUrlResponse,
 )
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.api.problem_details import NOT_FOUND_ON_REMOTE
 from palace.manager.core.problem_details import INTERNAL_SERVER_ERROR, INVALID_INPUT
 from palace.manager.sqlalchemy.model.admin import Admin
@@ -43,7 +44,7 @@ def dashboard_authorized_arns(self, dashboard_name: str) -> list[str]:
         return arns
 
     def generate_quicksight_url(self, dashboard_name) -> dict:
-        admin: Admin = getattr(flask.request, "admin")
+        admin: Admin = get_request_admin()
         request_data = QuicksightGenerateUrlRequest(**flask.request.args)
 
         authorized_arns = self.dashboard_authorized_arns(dashboard_name)

src/palace/manager/api/admin/controller/sign_in.py

@@ -26,6 +26,7 @@
     section_style,
     small_link_style,
 )
+from palace.manager.api.admin.util.flask import get_request_admin
 from palace.manager.util.problem_detail import ProblemDetail
 
 
@@ -116,7 +117,7 @@ def password_sign_in(self):
         return SanitizedRedirections.redirect(redirect_url)
 
     def change_password(self):
-        admin = flask.request.admin
+        admin = get_request_admin()
         new_password = flask.request.form.get("password")
         if new_password:
             admin.password = new_password

src/palace/manager/api/admin/util/flask.py

@@ -0,0 +1,34 @@
+from typing import Literal, TypeVar, overload
+
+from palace.manager.api.util.flask import get_request_var
+from palace.manager.sqlalchemy.model.admin import Admin
+from palace.manager.util.sentinel import SentinelType
+
+TDefault = TypeVar("TDefault")
+
+
+@overload
+def get_request_admin() -> Admin: ...
+
+
+@overload
+def get_request_admin(*, default: TDefault) -> Admin | TDefault: ...
+
+
+def get_request_admin(
+    *, default: TDefault | Literal[SentinelType.NotGiven] = SentinelType.NotGiven
+) -> Admin | TDefault:
+    """
+    Retrieve the 'admin' attribute from the current Flask request object.
+
+    This attribute should be set by using the @requires_admin decorator on the route
+    or by calling the AdminController.authenticated_admin_from_request
+    method.
+
+    :param default: The default value to return if the 'admin' attribute is not set.
+        If not provided, a `PalaceValueError` will be raised if the attribute is missing
+        or has an incorrect type.
+
+    :return: The `Admin` object from the request, or the default value if provided.
+    """
+    return get_request_var("admin", Admin, default=default)

tests/fixtures/api_admin.py

@@ -1,6 +1,5 @@
 from contextlib import contextmanager
 
-import flask
 import pytest
 
 from palace.manager.api.admin.controller import setup_admin_controllers
@@ -40,10 +39,10 @@ def request_context_with_admin(self, route, *args, **kwargs):
         if "admin" in kwargs:
             admin = kwargs.pop("admin")
         with self.ctrl.app.test_request_context(route, *args, **kwargs) as c:
-            flask.request.form = {}
-            flask.request.files = {}
+            c.request.form = {}
+            c.request.files = {}
             self.ctrl.db.session.begin_nested()
-            flask.request.admin = admin
+            setattr(c.request, "admin", admin)
             try:
                 yield c
             finally:
@@ -55,10 +54,10 @@ def request_context_with_library_and_admin(self, route, *args, **kwargs):
         if "admin" in kwargs:
             admin = kwargs.pop("admin")
         with self.ctrl.request_context_with_library(route, *args, **kwargs) as c:
-            flask.request.form = {}
-            flask.request.files = {}
+            c.request.form = {}
+            c.request.files = {}
             self.ctrl.db.session.begin_nested()
-            flask.request.admin = admin
+            setattr(c.request, "admin", admin)
             try:
                 yield c
             finally:

tests/manager/api/admin/test_routes.py

@@ -50,7 +50,7 @@ class MockAdminController(MockController):
     def authenticated_admin_from_request(self):
         if self.authenticated:
             admin = object()
-            flask.request.admin = self.AUTHENTICATED_ADMIN
+            setattr(flask.request, "admin", self.AUTHENTICATED_ADMIN)
             return self.AUTHENTICATED_ADMIN
         # For the redirect case we want to return a Problem Detail.
         elif self.authenticated_problem_detail: