make ApiKeyAuth work on non-safe routes

api/tests/test_mixed_auth.py

@@ -0,0 +1,48 @@
+import pytest
+from django.test import Client
+from django.urls import path
+from model_bakery import baker
+from ninja import NinjaAPI
+from ninja.security import SessionAuth
+
+from api.auth import ApiKeyAuth
+from api.hashers import generate_key
+from api.models import ApiClient, ApiKey
+
+api = NinjaAPI()
+
+
+@api.post("", auth=[ApiKeyAuth(), SessionAuth()])
+def post_method(*args, **kwargs) -> None:
+    """Dummy POST route authenticated by either api key or session cookie."""
+    pass
+
+
+urlpatterns = [path("", api.urls)]
+
+
+@pytest.mark.django_db
+@pytest.mark.urls(__name__)
+@pytest.mark.parametrize("user_logged_in", [False, True])
+def test_csrf_token(user_logged_in):
+    """Test that CSRF check happens only when no api key is used."""
+    client = Client(enforce_csrf_checks=True)
+    key, hashed = generate_key()
+    api_client = baker.make(ApiClient)
+    baker.make(ApiKey, client=api_client, hashed_key=hashed)
+    if user_logged_in:
+        client.force_login(api_client.owner)
+
+    response = client.post("")
+    assert response.status_code == 403
+    assert response.json()["detail"] == "CSRF check Failed"
+
+    # if using a valid API key, CSRF check should not occur
+    response = client.post("", headers={"X-APIKey": key})
+    assert response.status_code == 200
+
+    # if using a wrong API key, ApiKeyAuth should fail,
+    # leading to a fallback into SessionAuth and a CSRF check
+    response = client.post("", headers={"X-APIKey": generate_key()[0]})
+    assert response.status_code == 403
+    assert response.json()["detail"] == "CSRF check Failed"

club/api.py

@@ -16,7 +16,7 @@ class ClubController(ControllerBase):
     @route.get(
         "/search",
         response=PaginatedResponseSchema[SimpleClubSchema],
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[CanAccessLookup],
         url_name="search_club",
     )
@@ -27,7 +27,7 @@ class ClubController(ControllerBase):
     @route.get(
         "/{int:club_id}",
         response=ClubSchema,
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[HasPerm("club.view_club")],
         url_name="fetch_club",
     )

core/api.py

@@ -99,7 +99,7 @@ class SithFileController(ControllerBase):
     @route.get(
         "/search",
         response=PaginatedResponseSchema[SithFileSchema],
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)
@@ -112,7 +112,7 @@ class GroupController(ControllerBase):
     @route.get(
         "/search",
         response=PaginatedResponseSchema[GroupSchema],
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)

counter/api.py

@@ -64,7 +64,7 @@ class CounterController(ControllerBase):
     @route.get(
         "/search",
         response=PaginatedResponseSchema[SimplifiedCounterSchema],
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)
@@ -77,7 +77,7 @@ class ProductController(ControllerBase):
     @route.get(
         "/search",
         response=PaginatedResponseSchema[SimpleProductSchema],
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)

docs/tutorial/api/dev.md

@@ -1,4 +1,3 @@
-
 Pour l'API, nous utilisons `django-ninja` et sa surcouche `django-ninja-extra`.
 Ce sont des librairies relativement simples et qui présentent
 l'immense avantage d'offrir des mécanismes de validation et de sérialisation
@@ -65,8 +64,8 @@ utilisez l'attribut `auth` et les classes `SessionAuth` et
         @route.get("", auth=[ApiKeyAuth()])
         def fetch_foo(self, club_id: int): ...
 
-        # Celle-ci sera accessible peu importe la méthode d'authentification
+        # Celle-ci sera accessible avec les deux méthodes d'authentification
-        @route.get("/bar", auth=[SessionAuth(), ApiKeyAuth()])
+        @route.get("/bar", auth=[ApiKeyAuth(), SessionAuth()])
         def fetch_bar(self, club_id: int): ...
 
         # Et celle-ci sera accessible aussi aux utilisateurs non-connectés
@@ -84,9 +83,7 @@ par-dessus `django-ninja`, le système de permissions de django
 et notre propre système.
 Cette dernière est documentée [ici](../perms.md).
 
-### Limites des clefs d'API
+### Incompatibilité avec certaines permissions
-
-#### Incompatibilité avec certaines permissions
 
 Le système des clefs d'API est apparu très tard dans l'histoire du site
 (en P25, 10 ans après le début du développement).
@@ -117,10 +114,33 @@ Les principaux points de friction sont :
 - `IsLoggedInCounter`, qui utilise encore un autre système 
   d'authentification maison et qui n'est pas fait pour être utilisé en dehors du site.
 
-#### Incompatibilité avec les tokens csrf
+### CSRF
+
+!!!info "A propos du csrf"
 
     Le [CSRF (*cross-site request forgery*)](https://fr.wikipedia.org/wiki/Cross-site_request_forgery)
-est un des multiples facteurs d'attaque sur le web.
+    est un vecteur d'attaque sur le web consistant
+    à soumettre des données au serveur à l'insu
+    de l'utilisateur, en profitant de sa session.
+
+    C'est une attaque qui peut se produire lorsque l'utilisateur
+    est authentifié par cookie de session.
+    En effet, les cookies sont joints automatiquement à
+    toutes les requêtes ;
+    en l'absence de protection contre le CSRF, 
+    un attaquant parvenant à insérer un formulaire 
+    dans la page de l'utilisateur serait en mesure
+    de faire presque n'importe quoi en son nom,
+    et ce sans même que l'utilisateur ni les administrateurs
+    ne s'en rendent compte avant qu'il ne soit largement trop tard !
+
+    Sur le CSRF et les moyens de s'en prémunir, voir :
+
+    - [https://owasp.org/www-community/attacks/csrf]()
+    - [https://security.stackexchange.com/questions/166724/should-i-use-csrf-protection-on-rest-api-endpoints]()
+    - [https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html]()
+    
+Le CSRF, c'est dangereux.
 Heureusement, Django vient encore une fois à notre aide,
 avec des mécanismes intégrés pour s'en protéger.
 Ceux-ci incluent notamment un système de 
@@ -131,13 +151,36 @@ Ceux-ci sont bien adaptés au cycle requêtes/réponses
 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 ;
+Cependant, ils sont également gênants et moins utiles
-de ce fait, il est à attendre que les requêtes POST envoyées à l'API
+dans le cadre d'une API REST, étant donné
-par un client externe n'aient pas de token CSRF et se retrouvent 
+que l'authentification cesse d'être implicite :
-donc bloquées.
+la clef d'API doit être explicitement jointe aux headers,
+pour chaque requête.
 
-Pour ces raisons, l'accès aux requêtes POST/PUT/PATCH de l'API
+Pour ces raisons, la vérification CSRF ne prend place
-par un client externe ne marche pas.
+que pour la vérification de l'authentification
+par cookie de session.
+
+!!!warning "L'ordre est important"
+
+    Si vous écrivez le code suivant, l'authentification par clef d'API
+    ne marchera plus :
+    
+    ```python
+    @api_controller("/foo")
+    class FooController(ControllerBase):
+        @route.post("/bar", auth=[SessionAuth(), ApiKeyAuth()])
+        def post_bar(self, club_id: int): ...
+    ```
+    
+    En effet, la vérification du cookie de session intègrera
+    toujours la vérification CSRF.
+    Or, un échec de cette dernière est traduit par django en un code HTTP 403
+    au lieu d'un HTTP 401.
+    L'authentification se retrouve alors court-circuitée,
+    faisant que la vérification de la clef d'API ne sera jamais appelée.
+    
+    `SessionAuth` doit donc être déclaré **après** `ApiKeyAuth`.
 
 ## Créer un client et une clef d'API
 
@@ -176,5 +219,3 @@ qui en a besoin.
 Dites-lui bien de garder cette clef en lieu sûr !
 Si la clef est perdue, il n'y a pas moyen de la récupérer,
 vous devrez en recréer une.
-
-

pedagogy/api.py

@@ -19,7 +19,7 @@ from pedagogy.utbm_api import UtbmApiClient
 class UvController(ControllerBase):
     @route.get(
         "/{code}",
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[
             # this route will almost always be called in the context
             # of a UV creation/edition
@@ -45,7 +45,7 @@ class UvController(ControllerBase):
         "",
         response=PaginatedResponseSchema[SimpleUvSchema],
         url_name="fetch_uvs",
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[HasPerm("pedagogy.view_uv")],
     )
     @paginate(PageNumberPaginationExtra, page_size=100)

sas/api.py

@@ -52,7 +52,7 @@ class AlbumController(ControllerBase):
     @route.get(
         "/autocomplete-search",
         response=PaginatedResponseSchema[AlbumAutocompleteSchema],
-        auth=[SessionAuth(), ApiKeyAuth()],
+        auth=[ApiKeyAuth(), SessionAuth()],
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)