Merge pull request #122 from oslokommune/unjwt

Don't rely on the `exp` fields of unsigned JWTs

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Removed dependency on the vulnerable (and seemingly abandoned) python-jose
   library.
+* PyJWT is no longer a dependency.
 
 ## 3.1.0 - 2024-01-10
 

okdata/sdk/auth/auth.py

@@ -1,20 +1,30 @@
 import json
 import logging
+from datetime import datetime, timedelta, timezone
 
 from okdata.sdk.auth.credentials.client_credentials import ClientCredentialsProvider
 from okdata.sdk.auth.credentials.common import (
     TokenProviderNotInitialized,
     TokenRefreshError,
 )
 from okdata.sdk.auth.credentials.password_grant import TokenServiceProvider
-from okdata.sdk.auth.util import is_token_expired
 from okdata.sdk.exceptions import ApiAuthenticateError
 from okdata.sdk.file_cache import FileCache
 
 log = logging.getLogger()
 
 
-class Authenticate(object):
+def _is_expired(timestamp):
+    """Return true if `timestamp` has expired (or is just about to expire)."""
+    return timestamp and (timestamp - datetime.now(timezone.utc)).total_seconds() < 10
+
+
+class Authenticate:
+    _access_token = None
+    _refresh_token = None
+    _expires_at = None
+    _refresh_expires_at = None
+
     def __init__(self, config, token_provider=None, file_cache=None):
         self.token_provider = token_provider
         if not self.token_provider:
@@ -30,9 +40,6 @@ def __init__(self, config, token_provider=None, file_cache=None):
         if not self.file_cache:
             self.file_cache = FileCache(config)
 
-        self._access_token = None
-        self._refresh_token = None
-
     def _resolve_token_provider(self, config):
         # Add more TokenProviders to accept different login methods
         strategies = [ClientCredentialsProvider, TokenServiceProvider]
@@ -54,7 +61,7 @@ def access_token(self):
         if not self._access_token:
             self.login()
         # If expired, refresh
-        if is_token_expired(self._access_token):
+        if _is_expired(self._expires_at):
             self.refresh_access_token()
         return self._access_token
 
@@ -66,8 +73,12 @@ def login(self, force=False):
         if cached:
             self._access_token = cached["access_token"]
             self._refresh_token = cached.get("refresh_token")
+            if expires_at := cached.get("expires_at"):
+                self._expires_at = datetime.fromisoformat(expires_at)
+            if refresh_expires_at := cached.get("refresh_expires_at"):
+                self._refresh_expires_at = datetime.fromisoformat(refresh_expires_at)
 
-        if self._access_token and not is_token_expired(self._access_token):
+        if self._access_token and not _is_expired(self._expires_at):
             log.info("Token not expired, skipping")
             return
         self.refresh_access_token()
@@ -78,7 +89,7 @@ def refresh_access_token(self):
 
         tokens = None
 
-        if self._refresh_token and not is_token_expired(self._refresh_token):
+        if self._refresh_token and not _is_expired(self._refresh_expires_at):
             try:
                 tokens = self.token_provider.refresh_token(self._refresh_token)
             except TokenRefreshError as e:
@@ -89,6 +100,13 @@ def refresh_access_token(self):
             if "access_token" not in tokens:
                 raise ApiAuthenticateError
             self._refresh_token = tokens.get("refresh_token")
+            self._expires_at = datetime.now(timezone.utc) + timedelta(
+                seconds=tokens.get("expires_in")
+            )
+            if refresh_expires_in := tokens.get("refresh_expires_in"):
+                self._refresh_expires_at = datetime.now(timezone.utc) + timedelta(
+                    seconds=refresh_expires_in
+                )
 
         self._access_token = tokens["access_token"]
         self.file_cache.write_credentials(credentials=self)
@@ -99,6 +117,12 @@ def __repr__(self):
                 "provider": self.token_provider.__class__.__name__,
                 "access_token": self._access_token,
                 "refresh_token": self._refresh_token,
+                "expires_at": self._expires_at.isoformat() if self._expires_at else "",
+                "refresh_expires_at": (
+                    self._refresh_expires_at.isoformat()
+                    if self._refresh_expires_at
+                    else ""
+                ),
             }
         )
 

requirements.txt

@@ -26,8 +26,6 @@ packaging==24.0
     # via deprecation
 pycparser==2.22
     # via cffi
-pyjwt==2.4.0
-    # via okdata-sdk (setup.py)
 pyrsistent==0.18.1
     # via jsonschema
 python-keycloak==3.11.1

setup.py

@@ -19,7 +19,6 @@
     namespace_packages=["okdata"],
     install_requires=[
         "jsonschema",
-        "PyJWT>=2.0.0",
         # Versions prior to 3.9.1 depends on the vulnerable (and seemingly
         # abandoned) python-jose library.
         "python-keycloak>=3.9.1,<4",

tests/auth/auth_test.py

@@ -11,8 +11,10 @@
 from freezegun import freeze_time
 
 from tests.auth.client_credentials_test_utils import (
-    from_cache_not_expired_token,
+    expired_time,
     from_cache_expired_token,
+    from_cache_not_expired_token,
+    not_expired_time,
     utc_now,
 )
 from tests.test_utils import (
@@ -71,6 +73,8 @@ def test_authenticate_cached_credentials(self, mock_home_dir):
             "provider": "ClientCredentialsProvider",
             "access_token": from_cache_not_expired_token,
             "refresh_token": from_cache_not_expired_token,
+            "expires_at": not_expired_time.isoformat(),
+            "refresh_expires_at": not_expired_time.isoformat(),
         }
 
         auth.file_cache.write_credentials(json.dumps(cached_credentials))
@@ -88,6 +92,8 @@ def test_authenticate_refresh_credentials(self, requests_mock, mock_home_dir):
             "provider": "ClientCredentialsProvider",
             "access_token": from_cache_not_expired_token,
             "refresh_token": from_cache_not_expired_token,
+            "expires_at": not_expired_time.isoformat(),
+            "refresh_expires_at": not_expired_time.isoformat(),
         }
 
         auth.file_cache.write_credentials(json.dumps(cached_credentials))
@@ -110,6 +116,8 @@ def test_authenticate_expired_tokens(self, requests_mock, mock_home_dir):
             "provider": "TokenServiceProvider",
             "access_token": from_cache_expired_token,
             "refresh_token": from_cache_expired_token,
+            "expires_at": expired_time.isoformat(),
+            "refresh_expires_at": expired_time.isoformat(),
         }
 
         auth.file_cache.write_credentials(json.dumps(cached_credentials))
@@ -134,6 +142,8 @@ def test_authenticate_expired_access_token(self, requests_mock, mock_home_dir):
             "provider": "TokenServiceProvider",
             "access_token": from_cache_expired_token,
             "refresh_token": from_cache_not_expired_token,
+            "expires_at": expired_time.isoformat(),
+            "refresh_expires_at": not_expired_time.isoformat(),
         }
 
         auth.file_cache.write_credentials(json.dumps(cached_credentials))
@@ -173,6 +183,8 @@ def test_refresh_inactive_session(self, requests_mock, mock_home_dir):
             "provider": "TokenServiceProvider",
             "access_token": from_cache_expired_token,
             "refresh_token": from_cache_not_expired_token,
+            "expires_at": expired_time.isoformat(),
+            "refresh_expires_at": not_expired_time.isoformat(),
         }
 
         auth.file_cache.write_credentials(json.dumps(cached_credentials))
@@ -203,6 +215,7 @@ def test_refresh_no_refresh_token(self, requests_mock, mock_home_dir):
         cached_credentials = {
             "provider": "TokenServiceProvider",
             "access_token": from_cache_expired_token,
+            "expires_at": expired_time.isoformat(),
         }
 
         auth.file_cache.write_credentials(json.dumps(cached_credentials))

tox.ini

@@ -4,8 +4,9 @@ envlist = py38,py39,py310,py311,py312,flake8,black,mypy
 [testenv]
 deps=
   freezegun==0.3.12
-  pytest==6.2.5
+  PyJWT==2.8.0
   pytest-mock==1.12.1
+  pytest==6.2.5
   python-dateutil==2.8.0
   requests-mock==1.6.0
   -rrequirements.txt