test: Ensure access token expiry time is correct

Author: KShivendu

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## unreleased
 
+## [0.12.2] - 2023-02-23
+- Fix expiry time of access token cookie.
+
+
 ## [0.12.1] - 2023-02-06
 
 -   Email template updates

supertokens_python/framework/fastapi/fastapi_request.py

@@ -45,6 +45,8 @@ def method(self) -> str:
         return self.request.method
 
     def get_cookie(self, key: str) -> Union[str, None]:
+        # Note: Unlike other frameworks, FastAPI wraps the value in quotes in Set-Cookie header
+        # It also takes care of escaping the quotes while fetching the value
         return self.request.cookies.get(key)
 
     def get_header(self, key: str) -> Union[str, None]:

supertokens_python/framework/fastapi/fastapi_response.py

@@ -52,31 +52,19 @@ def set_cookie(
         # Note: For FastAPI response object, the expires value
         # doesn't mean the absolute time in ms, but the duration in seconds
         # So we need to convert our absolute expiry time (ms) to a duration (seconds)
-        if domain is None:
-            # we do ceil because if we do floor, we tests may fail where the access
-            # token lifetime is set to 1 second
-            self.response.set_cookie(
-                key=key,
-                value=value,
-                expires=ceil((expires - get_timestamp_ms()) / 1000),
-                path=path,
-                secure=secure,
-                httponly=httponly,
-                samesite=samesite,
-            )
-        else:
-            # we do ceil because if we do floor, we tests may fail where the access
-            # token lifetime is set to 1 second
-            self.response.set_cookie(
-                key=key,
-                value=value,
-                expires=ceil((expires - get_timestamp_ms()) / 1000),
-                path=path,
-                domain=domain,
-                secure=secure,
-                httponly=httponly,
-                samesite=samesite,
-            )
+
+        # we do ceil because if we do floor, we tests may fail where the access
+        # token lifetime is set to 1 second
+        self.response.set_cookie(
+            key=key,
+            value=value,  # Note: Unlike other frameworks, FastAPI wraps the value in quotes in Set-Cookie header
+            expires=ceil((expires - get_timestamp_ms()) / 1000),
+            path=path,
+            domain=domain,  # type: ignore # starlette didn't set domain as optional type but their default value is None anyways
+            secure=secure,
+            httponly=httponly,
+            samesite=samesite,
+        )
 
     def set_header(self, key: str, value: str):
         self.response.headers[key] = value

supertokens_python/recipe/session/recipe_implementation.py

@@ -254,7 +254,7 @@ async def create_new_session(
         # We set the expiration to 100 years, because we can't really access the expiration of the refresh token everywhere we are setting it.
         # This should be safe to do, since this is only the validity of the cookie (set here or on the frontend) but we check the expiration of the JWT anyway.
         # Even if the token is expired the presence of the token indicates that the user could have a valid refresh
-        # Setting them to infinity would require special case handling on the frontend and just adding 10 years seems enough.
+        # Setting them to infinity would require special case handling on the frontend and just adding 100 years seems enough.
         response_mutators.append(
             token_response_mutator(
                 self.config,
@@ -269,7 +269,9 @@ async def create_new_session(
                 self.config,
                 "refresh",
                 new_refresh_token_info["token"],
-                new_refresh_token_info["expiry"],
+                new_refresh_token_info[
+                    "expiry"
+                ],  # This comes from the core and is 100 days
                 new_session.transfer_method,
             )
         )
@@ -466,7 +468,7 @@ async def get_session(
             # We set the expiration to 100 years, because we can't really access the expiration of the refresh token everywhere we are setting it.
             # This should be safe to do, since this is only the validity of the cookie (set here or on the frontend) but we check the expiration of the JWT anyway.
             # Even if the token is expired the presence of the token indicates that the user could have a valid refresh
-            # Setting them to infinity would require special case handling on the frontend and just adding 10 years seems enough.
+            # Setting them to infinity would require special case handling on the frontend and just adding 100 years seems enough.
             session.response_mutators.append(
                 token_response_mutator(
                     self.config,
@@ -617,7 +619,7 @@ async def refresh_session(
                 # We set the expiration to 100 years, because we can't really access the expiration of the refresh token everywhere we are setting it.
                 # This should be safe to do, since this is only the validity of the cookie (set here or on the frontend) but we check the expiration of the JWT anyway.
                 # Even if the token is expired the presence of the token indicates that the user could have a valid refresh
-                # Setting them to infinity would require special case handling on the frontend and just adding 10 years seems enough.
+                # Setting them to infinity would require special case handling on the frontend and just adding 100 years seems enough.
                 response_mutators.append(
                     token_response_mutator(
                         self.config,
@@ -633,7 +635,9 @@ async def refresh_session(
                         self.config,
                         "refresh",
                         new_refresh_token_info["token"],
-                        new_refresh_token_info["expiry"],
+                        new_refresh_token_info[
+                            "expiry"
+                        ],  # This comes from the core and is 100 days
                         session.transfer_method,
                     )
                 )

supertokens_python/recipe/session/session_class.py

@@ -99,7 +99,7 @@ async def update_access_token_payload(
             # We set the expiration to 100 years, because we can't really access the expiration of the refresh token everywhere we are setting it.
             # This should be safe to do, since this is only the validity of the cookie (set here or on the frontend) but we check the expiration of the JWT anyway.
             # Even if the token is expired the presence of the token indicates that the user could have a valid refresh
-            # Setting them to infinity would require special case handling on the frontend and just adding 10 years seems enough.
+            # Setting them to infinity would require special case handling on the frontend and just adding 100 years seems enough.
             self.response_mutators.append(
                 token_response_mutator(
                     self.config,

tests/test_session.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from datetime import datetime, timedelta
 from typing import Any, Dict, List
 from unittest.mock import MagicMock
 
@@ -28,9 +29,6 @@
 from supertokens_python.recipe.session.asyncio import (
     create_new_session as async_create_new_session,
 )
-from supertokens_python.recipe.session.jwt import (
-    parse_jwt_without_signature_verification,
-)
 from supertokens_python.recipe.session.asyncio import (
     get_all_session_handles_for_user,
     get_session_information,
@@ -44,6 +42,9 @@
     update_session_data,
 )
 from supertokens_python.recipe.session.interfaces import RecipeInterface
+from supertokens_python.recipe.session.jwt import (
+    parse_jwt_without_signature_verification,
+)
 from supertokens_python.recipe.session.recipe_implementation import RecipeImplementation
 from supertokens_python.recipe.session.session_functions import (
     create_new_session,
@@ -356,10 +357,10 @@ async def get_session_information(
 from supertokens_python.recipe.session.exceptions import raise_unauthorised_exception
 from supertokens_python.recipe.session.interfaces import APIInterface, APIOptions
 from tests.utils import (
+    assert_info_clears_tokens,
     extract_all_cookies,
-    get_st_init_args,
     extract_info,
-    assert_info_clears_tokens,
+    get_st_init_args,
 )
 
 
@@ -578,3 +579,72 @@ async def refresh_post(api_options: APIOptions, user_context: Dict[str, Any]):
 
     assert cookies["sAccessToken"]["value"] != ""
     assert cookies["sRefreshToken"]["value"] != ""
+
+
+async def test_token_cookie_expires(
+    driver_config_client: TestClient,
+):
+    init_args = get_st_init_args(
+        [
+            session.init(
+                anti_csrf="VIA_TOKEN",
+                get_token_transfer_method=lambda _, __, ___: "cookie",
+            ),
+        ]
+    )
+    init(**init_args)
+    start_st()
+
+    response = driver_config_client.post("/create")
+    assert response.status_code == 200
+
+    cookies = extract_all_cookies(response)
+
+    assert "sAccessToken" in cookies
+    assert "sRefreshToken" in cookies
+
+    for c in response.cookies:
+        if c.name == "sAccessToken":  # 100 years (set by the SDK)
+            # some time must have elasped since the cookie was set. So less than current time
+            assert (
+                datetime.fromtimestamp(c.expires or 0) - timedelta(days=365.25 * 100)
+                < datetime.now()
+            )
+        if c.name == "sRefreshToken":  # 100 days (set by the core)
+            assert (
+                datetime.fromtimestamp(c.expires or 0) - timedelta(days=100)
+                < datetime.now()
+            )
+
+    assert response.headers["anti-csrf"] != ""
+    assert response.headers["front-token"] != ""
+
+    response = driver_config_client.post(
+        "/auth/session/refresh",
+        cookies={
+            "sRefreshToken": cookies["sRefreshToken"]["value"],
+        },
+        headers={"anti-csrf": response.headers["anti-csrf"]},
+    )
+
+    assert response.status_code == 200
+    cookies = extract_all_cookies(response)
+
+    assert "sAccessToken" in cookies
+    assert "sRefreshToken" in cookies
+
+    for c in response.cookies:
+        if c.name == "sAccessToken":  # 100 years (set by the SDK)
+            # some time must have elasped since the cookie was set. So less than current time
+            assert (
+                datetime.fromtimestamp(c.expires or 0) - timedelta(days=365.25 * 100)
+                < datetime.now()
+            )
+        if c.name == "sRefreshToken":  # 100 days (set by the core)
+            assert (
+                datetime.fromtimestamp(c.expires or 0) - timedelta(days=100)
+                < datetime.now()
+            )
+
+    assert response.headers["anti-csrf"] != ""
+    assert response.headers["front-token"] != ""