diff --git a/src/palace/manager/api/admin/controller/base.py b/src/palace/manager/api/admin/controller/base.py index 7f7727705..c18aed9e0 100644 --- a/src/palace/manager/api/admin/controller/base.py +++ b/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() diff --git a/src/palace/manager/api/admin/controller/collection_settings.py b/src/palace/manager/api/admin/controller/collection_settings.py index 984320fe9..439eb09ea 100644 --- a/src/palace/manager/api/admin/controller/collection_settings.py +++ b/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 diff --git a/src/palace/manager/api/admin/controller/dashboard.py b/src/palace/manager/api/admin/controller/dashboard.py index 5115e6b47..723a8b182 100644 --- a/src/palace/manager/api/admin/controller/dashboard.py +++ b/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): diff --git a/src/palace/manager/api/admin/controller/individual_admin_settings.py b/src/palace/manager/api/admin/controller/individual_admin_settings.py index 43369a92a..787e1dd02 100644 --- a/src/palace/manager/api/admin/controller/individual_admin_settings.py +++ b/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 diff --git a/src/palace/manager/api/admin/controller/library_settings.py b/src/palace/manager/api/admin/controller/library_settings.py index 84d0ad8f9..b893ce412 100644 --- a/src/palace/manager/api/admin/controller/library_settings.py +++ b/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 diff --git a/src/palace/manager/api/admin/controller/quicksight.py b/src/palace/manager/api/admin/controller/quicksight.py index d199a3b88..3f119e6f3 100644 --- a/src/palace/manager/api/admin/controller/quicksight.py +++ b/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) diff --git a/src/palace/manager/api/admin/controller/sign_in.py b/src/palace/manager/api/admin/controller/sign_in.py index 1e08f3901..a3dca1891 100644 --- a/src/palace/manager/api/admin/controller/sign_in.py +++ b/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 diff --git a/src/palace/manager/api/admin/util/__init__.py b/src/palace/manager/api/admin/util/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/palace/manager/api/admin/util/flask.py b/src/palace/manager/api/admin/util/flask.py new file mode 100644 index 000000000..98c2f521c --- /dev/null +++ b/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) diff --git a/tests/fixtures/api_admin.py b/tests/fixtures/api_admin.py index 2a2773844..bfeeb675f 100644 --- a/tests/fixtures/api_admin.py +++ b/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: diff --git a/tests/manager/api/admin/test_routes.py b/tests/manager/api/admin/test_routes.py index c3b1aa192..5f25462ed 100644 --- a/tests/manager/api/admin/test_routes.py +++ b/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: