[lichess] Check the CSRF state, rather than ignoring that aspect of OAuth2 for now

Author: olivierphi

src/apps/lichess_bridge/authentication.py

@@ -15,11 +15,14 @@
 from authlib.common.security import generate_token
 from authlib.integrations.requests_client import OAuth2Session
 from django.conf import settings
+from django.core.exceptions import SuspiciousOperation
 from django.urls import reverse
 
 if TYPE_CHECKING:
     from typing import Self
 
+    from django.http import HttpRequest
+
     from .models import LichessAccessToken
 
 LICHESS_OAUTH2_SCOPES = ("board:play",)
@@ -104,7 +107,7 @@ def get_lichess_token_retrieval_via_oauth2_process_starting_url(
 ) -> str:
     lichess_authorization_endpoint = f"{settings.LICHESS_HOST}/oauth"
 
-    client = _get_lichess_client()
+    client = _get_lichess_oauth2_client()
     uri, state = client.create_authorization_url(
         lichess_authorization_endpoint,
         response_type="code",
@@ -117,14 +120,27 @@ def get_lichess_token_retrieval_via_oauth2_process_starting_url(
     return uri
 
 
-def extract_lichess_token_from_oauth2_callback_url(
+def check_csrf_state_from_oauth2_callback(
+    *, request: "HttpRequest", context: LichessTokenRetrievalProcessContext
+):
+    """
+    Raises a SuspiciousOperation if the state from the request's query string
+    doesn't match the state from the short-lived cookie.
+    """
+    csrf_state_from_request = request.GET["state"]
+    csrf_state_from_short_lived_cookie = context.csrf_state
+    if csrf_state_from_short_lived_cookie != csrf_state_from_request:
+        raise SuspiciousOperation("OAuth2 CSRF state mismatch")
+
+
+def fetch_lichess_token_from_oauth2_callback(
     *,
     authorization_callback_response_url: str,
     context: LichessTokenRetrievalProcessContext,
 ) -> LichessToken:
     lichess_token_endpoint = f"{settings.LICHESS_HOST}/api/token"
 
-    client = _get_lichess_client()
+    client = _get_lichess_oauth2_client()
     token_as_dict = client.fetch_token(
         lichess_token_endpoint,
         authorization_response=authorization_callback_response_url,
@@ -146,7 +162,7 @@ def _get_lichess_oauth2_zakuchess_redirect_uri(
     )
 
 
-def _get_lichess_client() -> OAuth2Session:
+def _get_lichess_oauth2_client() -> OAuth2Session:
     return OAuth2Session(
         client_id=settings.LICHESS_CLIENT_ID,
         code_challenge_method="S256",

src/apps/lichess_bridge/tests/test_views.py

@@ -27,7 +27,7 @@ def test_lichess_homepage_with_access_token_smoke_test(
         "username": "ChessChampion"
     }
 
-    client.cookies["lichess.access_token"] = "lio_123456"
+    client.cookies["lichess.access_token"] = "lio_123456789"
     response = client.get("/lichess/")
     assert response.status_code == HTTPStatus.OK
 
@@ -36,4 +36,4 @@ def test_lichess_homepage_with_access_token_smoke_test(
     assert "Log out from Lichess" in response_html
     assert "ChessChampion" in response_html
 
-    create_lichess_api_client_mock.assert_called_once_with("lio_123456")
+    create_lichess_api_client_mock.assert_called_once_with("lio_123456789")

src/apps/lichess_bridge/views.py

@@ -7,7 +7,8 @@
 from . import cookie_helpers
 from .authentication import (
     LichessTokenRetrievalProcessContext,
-    extract_lichess_token_from_oauth2_callback_url,
+    check_csrf_state_from_oauth2_callback,
+    fetch_lichess_token_from_oauth2_callback,
     get_lichess_token_retrieval_via_oauth2_process_starting_url,
 )
 from .components.pages.lichess import (
@@ -69,7 +70,14 @@ def lichess_webhook_oauth2_token_callback(request: "HttpRequest") -> HttpRespons
         # TODO: Do something with that error
         return redirect("lichess_bridge:homepage")
 
-    token = extract_lichess_token_from_oauth2_callback_url(
+    # We have to check the "CSRF state":
+    # ( https://stack-auth.com/blog/oauth-from-first-principles#attack-4 )
+    check_csrf_state_from_oauth2_callback(
+        request=request, context=lichess_oauth2_process_context
+    )
+
+    # Ok, now let's fetch an API access token from Lichess!
+    token = fetch_lichess_token_from_oauth2_callback(
         authorization_callback_response_url=request.get_full_path(),
         context=lichess_oauth2_process_context,
     )
@@ -80,7 +88,7 @@ def lichess_webhook_oauth2_token_callback(request: "HttpRequest") -> HttpRespons
     cookie_helpers.delete_oauth2_token_retrieval_context_from_cookies(response)
 
     # Now that we have an access token to interact with Lichess' API on behalf
-    # of the user, let's store it into a HTTP-only cookie:
+    # of the user, let's store it into a long-lived HTTP-only cookie:
     cookie_helpers.store_lichess_api_access_token_in_response_cookie(
         token=token,
         response=response,