Skip to content

Commit

Permalink
Fix timezone for tokens expiration
Browse files Browse the repository at this point in the history
This prevents ambiguity and too frequent refreshes from the frontend
  • Loading branch information
nlachat-compassion committed Oct 25, 2024
1 parent b5fa3f2 commit 03f3a94
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
23 changes: 17 additions & 6 deletions auth_external/models/res_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion auth_external/models/tokens_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
return singleton

# TODO refresh tokens should have longer duration as access tokens
# _sql_constraints = []
1 change: 1 addition & 0 deletions auth_external/tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

0 comments on commit 03f3a94

Please sign in to comment.