[chore] Better cookies management, with a HttpCookieAttributes struct

Author: olivierphi

src/apps/daily_challenge/admin.py

@@ -199,6 +199,7 @@ def play_future_daily_challenge_view(
             lookup_key,
             expires=now() + _FUTURE_DAILY_CHALLENGE_COOKIE_DURATION,
             httponly=True,
+            samesite="Lax",
         )
 
         return response

src/apps/daily_challenge/cookie_helpers.py

@@ -1,10 +1,15 @@
+import datetime as dt
 import logging
 from typing import TYPE_CHECKING, NamedTuple
 
 from django.utils.timezone import now
 from msgspec import MsgspecError
 
 from apps.chess.models import UserPrefs
+from lib.http_cookies_helpers import (
+    HttpCookieAttributes,
+    set_http_cookie_on_django_response,
+)
 
 from .models import PlayerGameState, PlayerSessionContent, PlayerStats
 
@@ -15,10 +20,13 @@
 
 
 _PLAYER_CONTENT_SESSION_KEY = "pc"
-_USER_PREFS_COOKIE = {
-    "name": "uprefs",
-    "max-age": 3600 * 24 * 30 * 6,  # approximately 6 months
-}
+
+_USER_PREFS_COOKIE_ATTRS = HttpCookieAttributes(
+    name="uprefs",
+    max_age=dt.timedelta(days=30 * 6),  # approximately 6 months
+    http_only=True,
+    same_site="Lax",
+)
 
 _logger = logging.getLogger(__name__)
 
@@ -100,7 +108,7 @@ def get_user_prefs_from_request(request: "HttpRequest") -> UserPrefs:
     def new_content():
         return UserPrefs()
 
-    cookie_content: str | None = request.COOKIES.get(_USER_PREFS_COOKIE["name"])
+    cookie_content: str | None = request.COOKIES.get(_USER_PREFS_COOKIE_ATTRS.name)
     if cookie_content is None or len(cookie_content) < 5:
         return new_content()
 
@@ -126,12 +134,10 @@ def save_daily_challenge_state_in_session(
 
 
 def save_user_prefs(*, user_prefs: "UserPrefs", response: "HttpResponse") -> None:
-    response.set_cookie(
-        _USER_PREFS_COOKIE["name"],
-        user_prefs.to_cookie_content(),
-        max_age=_USER_PREFS_COOKIE["max-age"],
-        httponly=True,
-        samesite="Lax",
+    set_http_cookie_on_django_response(
+        response=response,
+        attributes=_USER_PREFS_COOKIE_ATTRS,
+        value=user_prefs.to_cookie_content(),
     )
 
 

src/apps/daily_challenge/models.py

@@ -17,7 +17,7 @@
     PieceRoleBySquare,
     PlayerSide,
 )
-from lib.django_helpers import literal_to_django_choices
+from lib.django_choices_helpers import literal_to_django_choices
 
 from .consts import BOT_SIDE, FACTIONS, PLAYER_SIDE
 

src/apps/lichess_bridge/cookie_helpers.py

@@ -1,30 +1,42 @@
+import datetime as dt
 import logging
 from typing import TYPE_CHECKING, cast
 
 from django.core.exceptions import SuspiciousOperation
 from msgspec import MsgspecError
 
+from lib.http_cookies_helpers import (
+    HttpCookieAttributes,
+    set_http_cookie_on_django_response,
+)
+
 from .authentication import LichessTokenRetrievalProcessContext
-from .models import LICHESS_ACCESS_TOKEN_PREFIX
+from .lichess_api import is_lichess_api_access_token_valid
 
 if TYPE_CHECKING:
     from django.http import HttpRequest, HttpResponse
 
     from .authentication import LichessToken
     from .models import LichessAccessToken
 
-_OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE = {
-    "name": "lichess.oauth2.ctx",
-    # One day should be more than enough to let the user grant their authorisation:
-    "max-age": 3600 * 24,
-}
-
-_API_ACCESS_TOKEN_COOKIE = {
-    "name": "lichess.access_token",
+_OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE_ATTRS = HttpCookieAttributes(
+    name="lichess.oauth2.ctx",
+    # This cookie only has to be valid while the user is redirected to Lichess
+    # and press the "Authorize" button there.
+    max_age=dt.timedelta(hours=1),
+    http_only=True,
+    same_site="Lax",
+)
+
+_API_ACCESS_TOKEN_COOKIE_ATTRS = HttpCookieAttributes(
+    name="lichess.access_token",
     # Access tokens delivered by Lichess "are long-lived (expect one year)".
-    # Let's store them for approximately 6 months, on our end:
-    "max-age": 3600 * 24 * 30 * 6,
-}
+    # As Lichess gives us the expiry date of the tokens it gives us, we can use that
+    # for our own cookie - so no "max-age" entry here, but we'll specify one at runtime.
+    max_age=None,
+    http_only=True,
+    same_site="Lax",
+)
 
 
 _logger = logging.getLogger(__name__)
@@ -37,12 +49,10 @@ def store_oauth2_token_retrieval_context_in_response_cookie(
     Store OAuth2 token retrieval context into a short-lived response cookie.
     """
 
-    response.set_cookie(
-        _OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE["name"],
-        context.to_cookie_content(),
-        max_age=_OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE["max-age"],
-        httponly=True,
-        samesite="Lax",
+    set_http_cookie_on_django_response(
+        response=response,
+        attributes=_OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE_ATTRS,
+        value=context.to_cookie_content(),
     )
 
 
@@ -53,7 +63,7 @@ def get_oauth2_token_retrieval_context_from_request(
     Returns a context created from the "CSRF state" and "code verifier" found in the request's cookies.
     """
     cookie_content: str | None = request.COOKIES.get(
-        _OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE["name"]
+        _OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE_ATTRS.name
     )
     if not cookie_content:
         return None
@@ -73,7 +83,7 @@ def get_oauth2_token_retrieval_context_from_request(
 def delete_oauth2_token_retrieval_context_from_cookies(
     response: "HttpResponse",
 ) -> None:
-    response.delete_cookie(_OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE["name"])
+    response.delete_cookie(_OAUTH2_TOKEN_RETRIEVAL_CONTEXT_COOKIE_ATTRS.name)
 
 
 def store_lichess_api_access_token_in_response_cookie(
@@ -82,15 +92,17 @@ def store_lichess_api_access_token_in_response_cookie(
     """
     Store a Lichess API token into a long-lived response cookie.
     """
+    # TODO: use a secured cookie here?
+
+    # Our cookie will expire when the access token given by Lichess will:
+    cookie_attributes = _API_ACCESS_TOKEN_COOKIE_ATTRS._replace(
+        max_age=dt.timedelta(seconds=token.expires_in),
+    )
 
-    response.set_cookie(
-        _API_ACCESS_TOKEN_COOKIE["name"],
-        token.access_token,
-        # TODO: should we use the token's `expires_in` here, rather than our custom
-        #   expiry period? There are pros and cons, let's decide that later :-)
-        max_age=_API_ACCESS_TOKEN_COOKIE["max-age"],
-        httponly=True,
-        samesite="Lax",
+    set_http_cookie_on_django_response(
+        response=response,
+        attributes=cookie_attributes,
+        value=token.access_token,
     )
 
 
@@ -100,14 +112,13 @@ def get_lichess_api_access_token_from_request(
     """
     Returns a Lichess API token found in the request's cookies.
     """
-    cookie_content: str | None = request.COOKIES.get(_API_ACCESS_TOKEN_COOKIE["name"])
+    cookie_content: str | None = request.COOKIES.get(
+        _API_ACCESS_TOKEN_COOKIE_ATTRS.name
+    )
     if not cookie_content:
         return None
 
-    if (
-        not cookie_content.startswith(LICHESS_ACCESS_TOKEN_PREFIX)
-        or len(cookie_content) < 10
-    ):
+    if not is_lichess_api_access_token_valid(cookie_content):
         raise SuspiciousOperation(
             f"Suspicious Lichess API token value '{cookie_content}'"
         )
@@ -118,4 +129,4 @@ def get_lichess_api_access_token_from_request(
 def delete_lichess_api_access_token_from_cookies(
     response: "HttpResponse",
 ) -> None:
-    response.delete_cookie(_API_ACCESS_TOKEN_COOKIE["name"])
+    response.delete_cookie(_API_ACCESS_TOKEN_COOKIE_ATTRS.name)

src/apps/lichess_bridge/lichess_api.py

@@ -2,10 +2,16 @@
 
 import berserk
 
+from .models import LICHESS_ACCESS_TOKEN_PREFIX
+
 if TYPE_CHECKING:
     from .models import LichessAccessToken
 
 
+def is_lichess_api_access_token_valid(token: str) -> bool:
+    return token.startswith(LICHESS_ACCESS_TOKEN_PREFIX) and len(token) > 10
+
+
 def get_lichess_api_client(access_token: "LichessAccessToken") -> berserk.Client:
     return _create_lichess_api_client(access_token)
 

src/lib/django_helpers.py renamed to src/lib/django_choices_helpers.py

@@ -0,0 +1,26 @@
+from typing import TYPE_CHECKING, Literal, NamedTuple
+
+if TYPE_CHECKING:
+    import datetime as dt
+
+    from django.http import HttpResponse
+
+
+class HttpCookieAttributes(NamedTuple):
+    name: str
+    max_age: "dt.timedelta | None"
+    http_only: bool
+    # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#samesitesamesite-value
+    same_site: Literal["Strict", "Lax", "None", None] = "Lax"
+
+
+def set_http_cookie_on_django_response(
+    *, response: "HttpResponse", attributes: HttpCookieAttributes, value: str
+) -> None:
+    response.set_cookie(
+        attributes.name,
+        value,
+        max_age=attributes.max_age,
+        httponly=attributes.http_only,
+        samesite=attributes.same_site,
+    )

src/lib/http_cookies_helpers.py

@@ -192,5 +192,5 @@
 # > (no client authentication, choose any unique client id).
 # So it's not a kind of API secret we would have created on Lichess' side, but just an
 # arbitrary identifier.
-LICHESS_CLIENT_ID = env.get("LICHESS_CLIENT_ID", "")
+LICHESS_CLIENT_ID = env.get("LICHESS_CLIENT_ID", "zakuchess.com")
 LICHESS_HOST = env.get("LICHESS_HOST", "https://lichess.org")