From 2e9e1b6a78f2ed8348ca0580c984ebffa7075a5b Mon Sep 17 00:00:00 2001 From: imperosol Date: Sun, 9 Nov 2025 12:46:22 +0100 Subject: [PATCH] remove deprecated api csrf argument --- api/auth.py | 2 ++ api/urls.py | 3 +- com/api.py | 10 ++---- core/tests/test_family.py | 2 +- core/tests/test_files.py | 2 +- counter/api.py | 2 +- counter/tests/test_product_type.py | 55 ++++++++++++++++++++++-------- docs/tutorial/api/dev.md | 15 +++++--- sas/api.py | 15 +++----- sas/tests/test_api.py | 4 +-- 10 files changed, 66 insertions(+), 44 deletions(-) diff --git a/api/auth.py b/api/auth.py index 787234a6..aac8cf40 100644 --- a/api/auth.py +++ b/api/auth.py @@ -6,6 +6,8 @@ from api.models import ApiClient, ApiKey class ApiKeyAuth(APIKeyHeader): + """Authentication through client api keys.""" + param_name = "X-APIKey" def authenticate(self, request: HttpRequest, key: str | None) -> ApiClient | None: diff --git a/api/urls.py b/api/urls.py index 2c3f12ff..50300453 100644 --- a/api/urls.py +++ b/api/urls.py @@ -1,3 +1,4 @@ +from ninja.security import SessionAuth from ninja_extra import NinjaExtraAPI api = NinjaExtraAPI( @@ -5,6 +6,6 @@ api = NinjaExtraAPI( description="Portail Interactif de Communication avec les Outils Numériques", version="0.2.0", urls_namespace="api", - csrf=True, + auth=[SessionAuth()], ) api.auto_discover_controllers() diff --git a/com/api.py b/com/api.py index b01eef0e..d487695e 100644 --- a/com/api.py +++ b/com/api.py @@ -5,7 +5,6 @@ from django.utils.cache import add_never_cache_headers from ninja import Query from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.pagination import PageNumberPaginationExtra -from ninja_extra.permissions import IsAuthenticated from ninja_extra.schemas import PaginatedResponseSchema from api.permissions import HasPerm @@ -17,17 +16,13 @@ from core.views.files import send_raw_file @api_controller("/calendar") class CalendarController(ControllerBase): - @route.get("/internal.ics", url_name="calendar_internal") + @route.get("/internal.ics", auth=None, url_name="calendar_internal") def calendar_internal(self): response = send_raw_file(IcsCalendar.get_internal()) add_never_cache_headers(response) return response - @route.get( - "/unpublished.ics", - permissions=[IsAuthenticated], - url_name="calendar_unpublished", - ) + @route.get("/unpublished.ics", url_name="calendar_unpublished") def calendar_unpublished(self): response = HttpResponse( IcsCalendar.get_unpublished(self.context.request.user), @@ -74,6 +69,7 @@ class NewsController(ControllerBase): @route.get( "/date", + auth=None, url_name="fetch_news_dates", response=PaginatedResponseSchema[NewsDateSchema], ) diff --git a/core/tests/test_family.py b/core/tests/test_family.py index 795de590..69844213 100644 --- a/core/tests/test_family.py +++ b/core/tests/test_family.py @@ -46,7 +46,7 @@ class TestFetchFamilyApi(TestCase): response = self.client.get( reverse("api:family_graph", args=[self.main_user.id]) ) - assert response.status_code == 403 + assert response.status_code == 401 self.client.force_login(baker.make(User)) # unsubscribed user response = self.client.get( diff --git a/core/tests/test_files.py b/core/tests/test_files.py index ac62d88a..4b731898 100644 --- a/core/tests/test_files.py +++ b/core/tests/test_files.py @@ -269,7 +269,7 @@ def test_apply_rights_recursively(): SimpleUploadedFile( "test.jpg", content=RED_PIXEL_PNG, content_type="image/jpg" ), - 403, + 401, ), ( lambda: baker.make(User), diff --git a/counter/api.py b/counter/api.py index c7bcb540..b7995d74 100644 --- a/counter/api.py +++ b/counter/api.py @@ -117,7 +117,7 @@ class ProductTypeController(ControllerBase): def fetch_all(self): return ProductType.objects.order_by("order") - @route.patch("/{type_id}/move") + @route.patch("/{type_id}/move", url_name="reorder_product_type") def reorder(self, type_id: int, other_id: Query[ReorderProductTypeSchema]): """Change the order of a product type. diff --git a/counter/tests/test_product_type.py b/counter/tests/test_product_type.py index 16dc40dd..ad74d619 100644 --- a/counter/tests/test_product_type.py +++ b/counter/tests/test_product_type.py @@ -3,11 +3,9 @@ from django.conf import settings from django.test import Client from django.urls import reverse from model_bakery import baker, seq -from ninja_extra.testing import TestClient from core.baker_recipes import board_user, subscriber_user from core.models import Group, User -from counter.api import ProductTypeController from counter.models import ProductType @@ -19,24 +17,43 @@ def product_types(db) -> list[ProductType]: return baker.make(ProductType, _quantity=5, order=seq(0)) +@pytest.fixture() +def counter_admin_client(db, client: Client) -> Client: + client.force_login( + baker.make( + User, groups=[Group.objects.get(id=settings.SITH_GROUP_COUNTER_ADMIN_ID)] + ) + ) + return client + + @pytest.mark.django_db -def test_fetch_product_types(product_types: list[ProductType]): +def test_fetch_product_types( + counter_admin_client: Client, product_types: list[ProductType] +): """Test that the API returns the right products in the right order""" - client = TestClient(ProductTypeController) - response = client.get("") + response = counter_admin_client.get(reverse("api:fetch_product_types")) assert response.status_code == 200 assert [i["id"] for i in response.json()] == [t.id for t in product_types] @pytest.mark.django_db -def test_move_below_product_type(product_types: list[ProductType]): +def test_move_below_product_type( + counter_admin_client: Client, product_types: list[ProductType] +): """Test that moving a product below another works""" - client = TestClient(ProductTypeController) - response = client.patch( - f"/{product_types[-1].id}/move", query={"below": product_types[0].id} + response = counter_admin_client.patch( + reverse( + "api:reorder_product_type", + kwargs={"type_id": product_types[-1].id}, + query={"below": product_types[0].id}, + ), ) assert response.status_code == 200 - new_order = [i["id"] for i in client.get("").json()] + new_order = [ + i["id"] + for i in counter_admin_client.get(reverse("api:fetch_product_types")).json() + ] assert new_order == [ product_types[0].id, product_types[-1].id, @@ -45,14 +62,22 @@ def test_move_below_product_type(product_types: list[ProductType]): @pytest.mark.django_db -def test_move_above_product_type(product_types: list[ProductType]): +def test_move_above_product_type( + counter_admin_client: Client, product_types: list[ProductType] +): """Test that moving a product above another works""" - client = TestClient(ProductTypeController) - response = client.patch( - f"/{product_types[1].id}/move", query={"above": product_types[0].id} + response = counter_admin_client.patch( + reverse( + "api:reorder_product_type", + kwargs={"type_id": product_types[1].id}, + query={"above": product_types[0].id}, + ), ) assert response.status_code == 200 - new_order = [i["id"] for i in client.get("").json()] + new_order = [ + i["id"] + for i in counter_admin_client.get(reverse("api:fetch_product_types")).json() + ] assert new_order == [ product_types[1].id, product_types[0].id, diff --git a/docs/tutorial/api/dev.md b/docs/tutorial/api/dev.md index 165d605f..c3e00709 100644 --- a/docs/tutorial/api/dev.md +++ b/docs/tutorial/api/dev.md @@ -49,8 +49,9 @@ Notre API offre deux moyens d'authentification : - par clef d'API La plus grande partie des routes de l'API utilisent la méthode par cookie de session. +Cette dernière est donc activée par défaut. -Pour placer une route d'API derrière l'une de ces méthodes (ou bien les deux), +Pour changer la méthode d'authentification, utilisez l'attribut `auth` et les classes `SessionAuth` et [`ApiKeyAuth`][api.auth.ApiKeyAuth]. @@ -60,13 +61,17 @@ utilisez l'attribut `auth` et les classes `SessionAuth` et @api_controller("/foo") class FooController(ControllerBase): # Cette route sera accessible uniquement avec l'authentification - # par cookie de session - @route.get("", auth=[SessionAuth()]) + # par clef d'API + @route.get("", auth=[ApiKeyAuth()]) def fetch_foo(self, club_id: int): ... - # Et celle-ci sera accessible peut importe la méthode d'authentification + # Celle-ci sera accessible peu importe la méthode d'authentification @route.get("/bar", auth=[SessionAuth(), ApiKeyAuth()]) def fetch_bar(self, club_id: int): ... + + # Et celle-ci sera accessible aussi aux utilisateurs non-connectés + @route.get("/public", auth=None) + def fetch_public(self, club_id: int): ... ``` ### Permissions @@ -123,7 +128,7 @@ Ceux-ci incluent notamment un système de à fournir dans les requêtes POST/PUT/PATCH. Ceux-ci sont bien adaptés au cycle requêtes/réponses -typique de l'expérience utilisateur sur un navigateur, +typiques de l'expérience utilisateur sur un navigateur, où les requêtes POST sont toujours effectuées après une requête GET au cours de laquelle on a pu récupérer un token csrf. Cependant, le flux des requêtes sur une API est bien différent ; diff --git a/sas/api.py b/sas/api.py index 6f68bd94..298c4d06 100644 --- a/sas/api.py +++ b/sas/api.py @@ -8,7 +8,6 @@ from ninja.security import SessionAuth from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.exceptions import NotFound, PermissionDenied from ninja_extra.pagination import PageNumberPaginationExtra -from ninja_extra.permissions import IsAuthenticated from ninja_extra.schemas import PaginatedResponseSchema from pydantic import NonNegativeInt @@ -41,7 +40,6 @@ class AlbumController(ControllerBase): @route.get( "/search", response=PaginatedResponseSchema[AlbumSchema], - permissions=[IsAuthenticated], url_name="search-album", ) @paginate(PageNumberPaginationExtra, page_size=50) @@ -74,12 +72,7 @@ class AlbumController(ControllerBase): @api_controller("/sas/picture") class PicturesController(ControllerBase): - @route.get( - "", - response=PaginatedResponseSchema[PictureSchema], - permissions=[IsAuthenticated], - url_name="pictures", - ) + @route.get("", response=PaginatedResponseSchema[PictureSchema], url_name="pictures") @paginate(PageNumberPaginationExtra, page_size=100) def fetch_pictures(self, filters: Query[PictureFilterSchema]): """Find pictures viewable by the user corresponding to the given filters. @@ -141,7 +134,7 @@ class PicturesController(ControllerBase): @route.get( "/{picture_id}/identified", - permissions=[IsAuthenticated, CanView], + permissions=[CanView], response=list[IdentifiedUserSchema], ) def fetch_identifications(self, picture_id: int): @@ -149,7 +142,7 @@ class PicturesController(ControllerBase): picture = self.get_object_or_exception(Picture, pk=picture_id) return picture.people.select_related("user") - @route.put("/{picture_id}/identified", permissions=[IsAuthenticated, CanView]) + @route.put("/{picture_id}/identified", permissions=[CanView]) def identify_users(self, picture_id: NonNegativeInt, users: set[NonNegativeInt]): picture = self.get_object_or_exception( Picture.objects.select_related("parent"), pk=picture_id @@ -209,7 +202,7 @@ class PicturesController(ControllerBase): @api_controller("/sas/relation", tags="User identification on SAS pictures") class UsersIdentifiedController(ControllerBase): - @route.delete("/{relation_id}", permissions=[IsAuthenticated]) + @route.delete("/{relation_id}") def delete_relation(self, relation_id: NonNegativeInt): """Untag a user from a SAS picture. diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py index 6c074dd0..5ac3af1f 100644 --- a/sas/tests/test_api.py +++ b/sas/tests/test_api.py @@ -55,7 +55,7 @@ class TestPictureSearch(TestSas): def test_anonymous_user_forbidden(self): res = self.client.get(self.url) - assert res.status_code == 403 + assert res.status_code == 401 def test_filter_by_album(self): self.client.force_login(self.user_b) @@ -148,7 +148,7 @@ class TestPictureRelation(TestSas): relation = PeoplePictureRelation.objects.exclude(user=self.user_a).first() res = self.client.delete(f"/api/sas/relation/{relation.id}") - assert res.status_code == 403 + assert res.status_code == 401 for user in baker.make(User), self.user_a: self.client.force_login(user)