Merge pull request #485 from supertokens/fix/cookie-domain-inconsistency

fix: duplicate session token issue when cookieDomain is changed

Author: rishabhpoddar

CHANGELOG.md

@@ -8,6 +8,87 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [unreleased]
 
+## [0.20.0] - 2024-05-08
+
+-   Added `older_cookie_domain` config option in the session recipe. This will allow users to clear cookies from the older domain when the `cookie_domain` is changed.
+-   If `verify_session` detects multiple access tokens in the request, it will return a 401 error, prompting a refresh, even if one of the tokens is valid.
+-   `refresh_post` (`/auth/session/refresh` by default) API changes:
+    -   now returns 500 error if multiple access tokens are present in the request and `config.older_cookie_domain` is not set.
+    -   now clears the access token cookie if it was called without a refresh token (if an access token cookie exists and if using cookie-based sessions).
+    -   now clears cookies from the old domain if `older_cookie_domain` is specified and multiple refresh/access token cookies exist, without updating the front-token or any of the tokens.
+    -   now a 200 response may not include new session tokens.
+-   Fixed a bug in the `normalise_session_scope` util function that caused it to remove leading dots from the scope string.
+
+### Migration
+
+With this update, the second argument in the `session.init` function changes from `cookie_secure` to `older_cookie_domain`. If you're using positional arguments, you need to insert `None` for `older_cookie_domain` as the second argument to maintain the correct order of parameters.
+
+Before:
+```python
+from supertokens_python import init, SupertokensConfig, InputAppInfo
+from supertokens_python.recipe import session
+
+init(
+    supertokens_config=SupertokensConfig("..."),
+    app_info=InputAppInfo("..."),
+    framework="...",
+    recipe_list=[
+        session.init(
+            "example.com" # cookie_domain
+            True, # cookie_secure
+            "strict" # cookie_same_site
+        ),
+    ],
+)
+```
+
+After the update:
+
+```python
+from supertokens_python import init, SupertokensConfig, InputAppInfo
+from supertokens_python.recipe import session
+
+init(
+    supertokens_config=SupertokensConfig("..."),
+    app_info=InputAppInfo("..."),
+    framework="...",
+    recipe_list=[
+        session.init(
+            "example.com" # cookie_domain
+            None, # older_cookie_domain
+            True, # cookie_secure
+            "strict" # cookie_same_site
+        ),
+    ],
+)
+```
+
+### Rationale
+
+This update addresses an edge case where changing the `cookie_domain` config on the server can lead to session integrity issues. For instance, if the API server URL is 'api.example.com' with a cookie domain of '.example.com', and the server updates the cookie domain to 'api.example.com', the client may retain cookies with both '.example.com' and 'api.example.com' domains, resulting in multiple sets of session token cookies existing.
+
+Previously, verify_session would select one of the access tokens from the incoming request. If it chose the older cookie, it would return a 401 status code, prompting a refresh request. However, the `refresh_post` API would then set new session token cookies with the updated `cookie_domain`, but older cookies will persist, leading to repeated 401 errors and refresh loops.
+
+With this update, verify_session will return a 401 error if it detects multiple access tokens in the request, prompting a refresh request. The `refresh_post` API will clear cookies from the old domain if `older_cookie_domain` is specified in the configuration, then return a 200 status. If `older_cookie_domain` is not configured, the `refresh_post` API will return a 500 error with a message instructing to set `older_cookie_domain`.
+
+**Example:**
+
+-   `apiDomain`: 'api.example.com'
+-   `cookie_domain`: 'api.example.com'
+
+**Flow:**
+
+1. After authentication, the frontend has cookies set with `domain=api.example.com`, but the access token has expired.
+2. The server updates `cookie_domain` to `.example.com`.
+3. An API call requiring session with an expired access token (cookie with `domain=api.example.com`) results in a 401 response.
+4. The frontend attempts to refresh the session, generating a new access token saved with `domain=.example.com`.
+5. The original API call is retried, but because it sends both the old and new cookies, it again results in a 401 response.
+6. The frontend tries to refresh the session with multiple access tokens:
+    - If `older_cookie_domain` is not set, the refresh fails with a 500 error.
+        - The user remains stuck until they clear cookies manually or `older_cookie_domain` is set.
+    - If `older_cookie_domain` is set, the refresh clears the older cookie, returning a 200 response.
+        - The frontend retries the original API call, sending only the new cookie (`domain=.example.com`), resulting in a successful request.
+
 ## [0.19.0] - 2024-05-06
 
 -  `create_new_session` now defaults to the value of the `st-auth-mode` header (if available) if the configured `get_token_transfer_method` returns `any`.

setup.py

@@ -82,7 +82,7 @@
 
 setup(
     name="supertokens_python",
-    version="0.19.0",
+    version="0.20.0",
     author="SuperTokens",
     license="Apache 2.0",
     author_email="[email protected]",

supertokens_python/constants.py

@@ -14,7 +14,7 @@
 from __future__ import annotations
 
 SUPPORTED_CDI_VERSIONS = ["3.0"]
-VERSION = "0.19.0"
+VERSION = "0.20.0"
 TELEMETRY = "/telemetry"
 USER_COUNT = "/users/count"
 USER_DELETE = "/user/remove"

supertokens_python/recipe/session/__init__.py

@@ -34,6 +34,7 @@
 
 def init(
     cookie_domain: Union[str, None] = None,
+    older_cookie_domain: Union[str, None] = None,
     cookie_secure: Union[bool, None] = None,
     cookie_same_site: Union[Literal["lax", "none", "strict"], None] = None,
     session_expired_status_code: Union[int, None] = None,
@@ -53,6 +54,7 @@ def init(
 ) -> Callable[[AppInfo], RecipeModule]:
     return SessionRecipe.init(
         cookie_domain,
+        older_cookie_domain,
         cookie_secure,
         cookie_same_site,
         session_expired_status_code,

supertokens_python/recipe/session/cookie_and_header.py

@@ -18,6 +18,11 @@
 
 from typing_extensions import Literal
 
+from supertokens_python.recipe.session.exceptions import (
+    raise_clear_duplicate_session_cookies_exception,
+)
+from supertokens_python.recipe.session.interfaces import ResponseMutator
+
 from .constants import (
     ACCESS_CONTROL_EXPOSE_HEADERS,
     ACCESS_TOKEN_COOKIE_KEY,
@@ -111,9 +116,9 @@ def _set_cookie(
     expires: int,
     path_type: Literal["refresh_token_path", "access_token_path"],
     request: BaseRequest,
+    domain: Optional[str],
     user_context: Dict[str, Any],
 ):
-    domain = config.cookie_domain
     secure = config.cookie_secure
     same_site = config.get_cookie_same_site(request, user_context)
     path = ""
@@ -141,10 +146,21 @@ def set_cookie_response_mutator(
     expires: int,
     path_type: Literal["refresh_token_path", "access_token_path"],
     request: BaseRequest,
+    domain: Optional[str] = None,
 ):
+    domain = domain if domain is not None else config.cookie_domain
+
     def mutator(response: BaseResponse, user_context: Dict[str, Any]):
         return _set_cookie(
-            response, config, key, value, expires, path_type, request, user_context
+            response,
+            config,
+            key,
+            value,
+            expires,
+            path_type,
+            request,
+            domain,
+            user_context,
         )
 
     return mutator
@@ -294,6 +310,7 @@ def _set_token(
             expires,
             "refresh_token_path" if token_type == "refresh" else "access_token_path",
             request,
+            config.cookie_domain,
             user_context,
         )
     elif transfer_method == "header":
@@ -398,3 +415,93 @@ def _set_access_token_in_response(
             request,
             user_context,
         )
+
+
+# This function addresses an edge case where changing the cookie_domain config on the server can
+# lead to session integrity issues. For instance, if the API server URL is 'api.example.com'
+# with a cookie domain of '.example.com', and the server updates the cookie domain to 'api.example.com',
+# the client may retain cookies with both '.example.com' and 'api.example.com' domains.
+
+# Consequently, if the server chooses the older cookie, session invalidation occurs, potentially
+# resulting in an infinite refresh loop. To fix this, users are asked to specify "older_cookie_domain" in
+# the config.
+
+# This function checks for multiple cookies with the same name and clears the cookies for the older domain.
+def clear_session_cookies_from_older_cookie_domain(
+    request: BaseRequest, config: SessionConfig, user_context: Dict[str, Any]
+):
+    allowed_transfer_method = config.get_token_transfer_method(
+        request, False, user_context
+    )
+    # If the transfer method is 'header', there's no need to clear cookies immediately, even if there are multiple in the request.
+    if allowed_transfer_method == "header":
+        return
+
+    did_clear_cookies = False
+    response_mutators: List[ResponseMutator] = []
+
+    token_types: List[TokenType] = ["access", "refresh"]
+    for token_type in token_types:
+        if has_multiple_cookies_for_token_type(request, token_type):
+            # If a request has multiple session cookies and 'older_cookie_domain' is
+            # unset, we can't identify the correct cookie for refreshing the session.
+            # Using the wrong cookie can cause an infinite refresh loop. To avoid this,
+            # we throw a 500 error asking the user to set 'older_cookie_domain''.
+            if config.older_cookie_domain is None:
+                raise Exception(
+                    "The request contains multiple session cookies. This may happen if you've changed the 'cookie_domain' setting in your configuration. To clear tokens from the previous domain, set 'older_cookie_domain' in your config."
+                )
+
+            log_debug_message(
+                "Clearing duplicate %s cookie with domain %s",
+                token_type,
+                config.cookie_domain,
+            )
+            response_mutators.append(
+                set_cookie_response_mutator(
+                    config,
+                    get_cookie_name_from_token_type(token_type),
+                    "",
+                    0,
+                    "refresh_token_path"
+                    if token_type == "refresh"
+                    else "access_token_path",
+                    request,
+                    domain=config.older_cookie_domain,
+                )
+            )
+            did_clear_cookies = True
+    if did_clear_cookies:
+        raise_clear_duplicate_session_cookies_exception(
+            "The request contains multiple session cookies. We are clearing the cookie from older_cookie_domain. Session will be refreshed in the next refresh call.",
+            response_mutators=response_mutators,
+        )
+
+
+def has_multiple_cookies_for_token_type(
+    request: BaseRequest, token_type: TokenType
+) -> bool:
+    cookie_string = request.get_header("cookie")
+    if cookie_string is None:
+        return False
+
+    cookies = _parse_cookie_string_from_request_header_allow_duplicates(cookie_string)
+    cookie_name = get_cookie_name_from_token_type(token_type)
+    return cookie_name in cookies and len(cookies[cookie_name]) > 1
+
+
+def _parse_cookie_string_from_request_header_allow_duplicates(
+    cookie_string: str,
+) -> Dict[str, List[str]]:
+    cookies: Dict[str, List[str]] = {}
+    cookie_pairs = cookie_string.split(";")
+    for cookie_pair in cookie_pairs:
+        name_value = cookie_pair.split("=")
+        if len(name_value) != 2:
+            raise Exception("Invalid cookie string in request header")
+        name, value = unquote(name_value[0].strip()), unquote(name_value[1].strip())
+        if name in cookies:
+            cookies[name].append(value)
+        else:
+            cookies[name] = [value]
+    return cookies

supertokens_python/recipe/session/exceptions.py

@@ -45,6 +45,14 @@ def raise_unauthorised_exception(
     raise err
 
 
+def raise_clear_duplicate_session_cookies_exception(
+    msg: str, response_mutators: List[ResponseMutator]
+) -> NoReturn:
+    err = ClearDuplicateSessionCookiesError(msg)
+    err.response_mutators.extend(response_mutators)
+    raise err
+
+
 class SuperTokensSessionError(SuperTokensError):
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
@@ -89,3 +97,7 @@ def to_json(self):
 
 def raise_invalid_claims_exception(msg: str, payload: List[ClaimValidationError]):
     raise InvalidClaimsError(msg, payload)
+
+
+class ClearDuplicateSessionCookiesError(SuperTokensSessionError):
+    pass

supertokens_python/recipe/session/recipe.py

@@ -23,6 +23,7 @@
     get_cors_allowed_headers,
 )
 from .exceptions import (
+    ClearDuplicateSessionCookiesError,
     SuperTokensSessionError,
     TokenTheftError,
     UnauthorisedError,
@@ -72,6 +73,7 @@ def __init__(
         recipe_id: str,
         app_info: AppInfo,
         cookie_domain: Union[str, None] = None,
+        older_cookie_domain: Union[str, None] = None,
         cookie_secure: Union[bool, None] = None,
         cookie_same_site: Union[Literal["lax", "none", "strict"], None] = None,
         session_expired_status_code: Union[int, None] = None,
@@ -95,6 +97,7 @@ def __init__(
         self.config = validate_and_normalise_user_input(
             app_info,
             cookie_domain,
+            older_cookie_domain,
             cookie_secure,
             cookie_same_site,
             session_expired_status_code,
@@ -265,6 +268,11 @@ async def handle_error(
             return await self.config.error_handlers.on_invalid_claim(
                 self, request, err.payload, response
             )
+        if isinstance(err, ClearDuplicateSessionCookiesError):
+            log_debug_message("errorHandler: returning CLEAR_DUPLICATE_SESSION_COOKIES")
+            return await self.config.error_handlers.on_clear_duplicate_session_cookies(
+                request, str(err), response
+            )
 
         log_debug_message("errorHandler: returning TRY_REFRESH_TOKEN")
         return await self.config.error_handlers.on_try_refresh_token(
@@ -280,6 +288,7 @@ def get_all_cors_headers(self) -> List[str]:
     @staticmethod
     def init(
         cookie_domain: Union[str, None] = None,
+        older_cookie_domain: Union[str, None] = None,
         cookie_secure: Union[bool, None] = None,
         cookie_same_site: Union[Literal["lax", "none", "strict"], None] = None,
         session_expired_status_code: Union[int, None] = None,
@@ -305,6 +314,7 @@ def func(app_info: AppInfo):
                     SessionRecipe.recipe_id,
                     app_info,
                     cookie_domain,
+                    older_cookie_domain,
                     cookie_secure,
                     cookie_same_site,
                     session_expired_status_code,