From 03f3a94f7094b5a3abc182d59f630841e1f7d197 Mon Sep 17 00:00:00 2001
From: Niels Lachat <nlachat@compassion.ch>
Date: Fri, 25 Oct 2024 12:43:14 +0200
Subject: [PATCH] Fix timezone for tokens expiration

This prevents ambiguity and too frequent refreshes from the frontend
---
 auth_external/models/res_users.py     | 23 +++++++++++++++++------
 auth_external/models/tokens_config.py |  7 ++++++-
 auth_external/tests/README.md         |  1 +
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/auth_external/models/res_users.py b/auth_external/models/res_users.py
index fa4f452e..da779bae 100644
--- a/auth_external/models/res_users.py
+++ b/auth_external/models/res_users.py
@@ -147,7 +147,7 @@ def generate_external_auth_token(self, rt_old=None):
                 # gone very wrong because a valid token has been deleted from
                 # the db. In this case we should deny access.
                 raise AccessDenied
-            
+
             if rt_old_model.is_revoked:
                 # if the token which was provided is already marked as revoked,
                 # this means that an attacker is trying to reuse a stolen token
@@ -176,9 +176,12 @@ def generate_external_auth_token(self, rt_old=None):
         # Verification succeeded, we generate tokens.
         tokens_config = self.env["auth_external.tokens_config"].sudo().get_singleton()
 
-        now = datetime.now()
-        at_new_exp = now + timedelta(seconds=tokens_config.access_token_duration_seconds)
-        rt_new_exp = now + timedelta(
+        # To avoid ambiguity, we fix all expiration dates in UTC
+        now_utc = datetime.now(timezone.utc)
+        at_new_exp = now_utc + timedelta(
+            seconds=tokens_config.access_token_duration_seconds
+        )
+        rt_new_exp = now_utc + timedelta(
             seconds=tokens_config.refresh_token_duration_seconds
         )
 
@@ -202,7 +205,13 @@ def generate_external_auth_token(self, rt_old=None):
         # case, the freshly generated refresh_token is the root of a new
         # refresh_token family, we must thus insert it into the database.
         rt_new_model = refresh_tokens.sudo().create(
-            {"jti": rt_new_payload["jti"], "exp": rt_new_exp}
+            {"jti": rt_new_payload["jti"], 
+             # We transform the expiration datetime to a naive version (without
+             # timezone information) because all odoo datetimes are forced to
+             # UTC.
+             # See https://www.odoo.com/documentation/14.0/developer/reference/addons/orm.html?highlight=fields%20many2many#date-time-fields
+             "exp": rt_new_exp.replace(tzinfo=None)
+             }
         )
         if rt_old is not None:
             # If a valid rt was provided and we generated a new rt, we need to
@@ -233,7 +242,9 @@ def _check_refresh_token(self, token: str, sub: any) -> dict:
         :returns: None if the token is valid.
         :raises AccessDenied: if the token is invalid.
         """
-        tokens_config = request.env["auth_external.tokens_config"].sudo().get_singleton()
+        tokens_config = (
+            request.env["auth_external.tokens_config"].sudo().get_singleton()
+        )
         try:
             return self._parse_jwt_token(
                 token,
diff --git a/auth_external/models/tokens_config.py b/auth_external/models/tokens_config.py
index 94f79ab9..a4b38c12 100644
--- a/auth_external/models/tokens_config.py
+++ b/auth_external/models/tokens_config.py
@@ -19,6 +19,8 @@ class TokensConfig(models.Model):
              security risk as a stolen token can be used for longer. A
              reasonable value is a few hours (i.e. 3600 seconds or more).""",
     )
+
+    # TODO: Don't need this, use corresponding field directly!
     access_token_duration_seconds = fields.Integer(compute="_compute_access_token_duration_seconds")
 
     @api.depends("access_token_duration_hours")
@@ -49,4 +51,7 @@ def _compute_refresh_token_duration_seconds(self) -> None:
     def get_singleton(self) -> "TokensConfig":
         singleton = self.search([])
         singleton.ensure_one()
-        return singleton
\ No newline at end of file
+        return singleton
+    
+    # TODO refresh tokens should have longer duration as access tokens 
+    # _sql_constraints = []
\ No newline at end of file
diff --git a/auth_external/tests/README.md b/auth_external/tests/README.md
index 048a4110..9e189fa9 100644
--- a/auth_external/tests/README.md
+++ b/auth_external/tests/README.md
@@ -3,3 +3,4 @@
 odoo/odoo-bin -c etc/dev_t1486.conf -u auth_external -i auth_external --test-tags=auth_external --stop-after-init
 ```
 
+The tests should be run on an ***empty database with only this module installed*** (dependencies break these tests because of constraints on res_users/res_partners).
\ No newline at end of file