Merge pull request #287 from supertokens/fix/access-token-cookie-expiry

fix: Update access token cookie expiry logic and add comments

Author: rishabhpoddar

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

setup.py

@@ -70,7 +70,7 @@
 
 setup(
     name="supertokens_python",
-    version="0.12.1",
+    version="0.12.2",
     author="SuperTokens",
     license="Apache 2.0",
     author_email="[email protected]",

supertokens_python/constants.py

@@ -12,7 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 SUPPORTED_CDI_VERSIONS = ["2.9", "2.10", "2.11", "2.12", "2.13", "2.14", "2.15"]
-VERSION = "0.12.1"
+VERSION = "0.12.2"
 TELEMETRY = "/telemetry"
 USER_COUNT = "/users/count"
 USER_DELETE = "/user/remove"

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

@@ -13,10 +13,10 @@
 # under the License.
 import json
 from math import ceil
-from time import time
 from typing import Any, Dict, Optional
 
 from supertokens_python.framework.response import BaseResponse
+from supertokens_python.utils import get_timestamp_ms
 
 
 class FastApiResponse(BaseResponse):
@@ -49,31 +49,22 @@ def set_cookie(
         httponly: bool = False,
         samesite: str = "lax",
     ):
-        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 - int(time() * 1000)) / 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 - int(time() * 1000)) / 1000),
-                path=path,
-                domain=domain,
-                secure=secure,
-                httponly=httponly,
-                samesite=samesite,
-            )
+        # 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)
+
+        # 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

@@ -14,7 +14,6 @@
 from __future__ import annotations
 
 import json
-from datetime import datetime
 from typing import TYPE_CHECKING, Any, Callable, Dict, Optional
 
 from supertokens_python.framework import BaseRequest
@@ -33,14 +32,14 @@
 from . import session_functions
 from .access_token import validate_access_token_structure
 from .cookie_and_header import (
+    anti_csrf_response_mutator,
+    clear_session_response_mutator,
+    front_token_response_mutator,
     get_anti_csrf_header,
     get_rid_header,
     get_token,
-    token_response_mutator,
-    front_token_response_mutator,
-    anti_csrf_response_mutator,
     set_cookie_response_mutator,
-    clear_session_response_mutator,
+    token_response_mutator,
 )
 from .exceptions import (
     TokenTheftError,
@@ -50,12 +49,12 @@
 )
 from .interfaces import (
     AccessTokenObj,
-    ResponseMutator,
     ClaimsValidationResult,
     GetClaimValueOkResult,
     JSONObject,
     RecipeInterface,
     RegenerateAccessTokenOkResult,
+    ResponseMutator,
     SessionClaim,
     SessionClaimValidator,
     SessionDoesNotExistError,
@@ -64,14 +63,18 @@
 )
 from .jwt import ParsedJWTInfo, parse_jwt_without_signature_verification
 from .session_class import Session
-from .utils import SessionConfig, TokenTransferMethod, validate_claims_in_payload
+from .utils import (
+    HUNDRED_YEARS_IN_MS,
+    SessionConfig,
+    TokenTransferMethod,
+    validate_claims_in_payload,
+)
 
 if TYPE_CHECKING:
     from typing import List, Union
     from supertokens_python import AppInfo
     from supertokens_python.querier import Querier
 
-
 from .constants import available_token_transfer_methods
 from .interfaces import SessionContainer
 
@@ -248,12 +251,16 @@ async def create_new_session(
                 new_session.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 100 years seems enough.
         response_mutators.append(
             token_response_mutator(
                 self.config,
                 "access",
                 new_access_token_info["token"],
-                int(datetime.now().timestamp()) + 3153600000000,
+                get_timestamp_ms() + HUNDRED_YEARS_IN_MS,
                 new_session.transfer_method,
             )
         )
@@ -262,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,
             )
         )
@@ -456,12 +465,16 @@ async def get_session(
                     session.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 100 years seems enough.
             session.response_mutators.append(
                 token_response_mutator(
                     self.config,
                     "access",
                     session.access_token,
-                    int(datetime.now().timestamp()) + 3153600000000,
+                    get_timestamp_ms() + HUNDRED_YEARS_IN_MS,
                     session.transfer_method,
                 )
             )
@@ -603,12 +616,16 @@ async def refresh_session(
                         session.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 100 years seems enough.
                 response_mutators.append(
                     token_response_mutator(
                         self.config,
                         "access",
                         new_access_token_info["token"],
-                        int(datetime.now().timestamp()) + 3153600000000,
+                        get_timestamp_ms() + HUNDRED_YEARS_IN_MS,  # 100 years
                         session.transfer_method,
                     )
                 )
@@ -618,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

@@ -11,20 +11,21 @@
 # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 # License for the specific language governing permissions and limitations
 # under the License.
-from datetime import datetime
 from typing import Any, Dict, List, TypeVar, Union
 
 from supertokens_python.recipe.session.exceptions import (
     raise_invalid_claims_exception,
     raise_unauthorised_exception,
 )
+from supertokens_python.utils import get_timestamp_ms
 
 from .cookie_and_header import (
     clear_session_response_mutator,
     front_token_response_mutator,
     token_response_mutator,
 )
 from .interfaces import SessionClaim, SessionClaimValidator, SessionContainer
+from .utils import HUNDRED_YEARS_IN_MS
 
 _T = TypeVar("_T")
 
@@ -95,12 +96,16 @@ async def update_access_token_payload(
                     self.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 100 years seems enough.
             self.response_mutators.append(
                 token_response_mutator(
                     self.config,
                     "access",
                     result.access_token.token,
-                    int(datetime.now().timestamp()) + 3153600000000,
+                    get_timestamp_ms() + HUNDRED_YEARS_IN_MS,
                     self.transfer_method,
                 )
             )

supertokens_python/recipe/session/utils.py

@@ -17,6 +17,8 @@
 from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, List, Optional, Union
 from urllib.parse import urlparse
 
+from typing_extensions import Literal
+
 from supertokens_python.exceptions import raise_general_exception
 from supertokens_python.framework import BaseResponse
 from supertokens_python.normalised_url_path import NormalisedURLPath
@@ -29,13 +31,10 @@
     send_non_200_response,
     send_non_200_response_with_message,
 )
-from typing_extensions import Literal
 
 from ...types import MaybeAwaitable
-from .constants import SESSION_REFRESH, AUTH_MODE_HEADER_KEY
-from .cookie_and_header import (
-    clear_session_from_all_token_transfer_methods,
-)
+from .constants import AUTH_MODE_HEADER_KEY, SESSION_REFRESH
+from .cookie_and_header import clear_session_from_all_token_transfer_methods
 from .exceptions import ClaimValidationError
 from .with_jwt.constants import (
     ACCESS_TOKEN_PAYLOAD_JWT_PROPERTY_NAME_KEY,
@@ -56,6 +55,8 @@
 
 from supertokens_python.logger import log_debug_message
 
+HUNDRED_YEARS_IN_MS = 3153600000000
+
 
 def normalise_session_scope(session_scope: str) -> str:
     def helper(scope: str) -> str: