Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper function to get flask.request.admin #2179

Merged
merged 2 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/palace/manager/api/admin/controller/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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()
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/palace/manager/api/admin/controller/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/palace/manager/api/admin/controller/library_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/palace/manager/api/admin/controller/quicksight.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/palace/manager/api/admin/controller/sign_in.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down
Empty file.
34 changes: 34 additions & 0 deletions src/palace/manager/api/admin/util/flask.py
Original file line number Diff line number Diff line change
@@ -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)
13 changes: 6 additions & 7 deletions tests/fixtures/api_admin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from contextlib import contextmanager

import flask
import pytest

from palace.manager.api.admin.controller import setup_admin_controllers
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/api/admin/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down