From 63b9bc5bb69c0d915ad40e7cd1af7ebbc1006694 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Wed, 29 Jan 2025 09:05:47 +0100 Subject: [PATCH 1/9] Add model for JWT refresh token --- changelog.d/3268.added.md | 1 + python/nav/models/api.py | 25 ++++++++ .../nav/models/sql/changes/sc.05.13.0001.sql | 10 ++++ python/nav/web/jwtgen.py | 15 +++++ .../unittests/models/jwtrefreshtoken_test.py | 58 +++++++++++++++++++ 5 files changed, 109 insertions(+) create mode 100644 changelog.d/3268.added.md create mode 100644 python/nav/models/sql/changes/sc.05.13.0001.sql create mode 100644 tests/unittests/models/jwtrefreshtoken_test.py diff --git a/changelog.d/3268.added.md b/changelog.d/3268.added.md new file mode 100644 index 0000000000..0084758571 --- /dev/null +++ b/changelog.d/3268.added.md @@ -0,0 +1 @@ +Add database model for JWT refresh tokens diff --git a/python/nav/models/api.py b/python/nav/models/api.py index d3ac6213c9..7538b53d85 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -23,6 +23,7 @@ from nav.models.fields import VarcharField from nav.models.profiles import Account +from nav.web.jwtgen import is_active class APIToken(models.Model): @@ -66,3 +67,27 @@ def get_absolute_url(self): class Meta(object): db_table = 'apitoken' + + +class JWTRefreshToken(models.Model): + + name = VarcharField(unique=True) + description = models.TextField(null=True, blank=True) + expires = models.DateTimeField() + activates = models.DateTimeField() + last_used = models.DateTimeField(null=True) + revoked = models.BooleanField(default=False) + hash = VarcharField() + + def __str__(self): + return self.name + + def is_active(self) -> bool: + """Returns True if the token is active. A token is considered active when + `expires` is in the future and `activates` is in the past or matches + the current time. + """ + return is_active(self.expires.timestamp(), self.activates.timestamp()) + + class Meta(object): + db_table = 'jwtrefreshtoken' diff --git a/python/nav/models/sql/changes/sc.05.13.0001.sql b/python/nav/models/sql/changes/sc.05.13.0001.sql new file mode 100644 index 0000000000..c0312b14ac --- /dev/null +++ b/python/nav/models/sql/changes/sc.05.13.0001.sql @@ -0,0 +1,10 @@ +CREATE TABLE manage.JWTRefreshToken ( + id SERIAL PRIMARY KEY, + name VARCHAR NOT NULL UNIQUE, + description VARCHAR, + expires TIMESTAMP NOT NULL, + activates TIMESTAMP NOT NULL, + last_used TIMESTAMP, + revoked BOOLEAN NOT NULL DEFAULT FALSE, + hash VARCHAR NOT NULL +); diff --git a/python/nav/web/jwtgen.py b/python/nav/web/jwtgen.py index 93a97b364a..4da2423ac1 100644 --- a/python/nav/web/jwtgen.py +++ b/python/nav/web/jwtgen.py @@ -49,3 +49,18 @@ def _generate_token( new_token, JWTConf().get_nav_private_key(), algorithm="RS256" ) return encoded_token + + +def is_active(exp: float, nbf: float) -> bool: + """ + Takes `exp` and `nbf` as POSIX timestamps. These represent the claims + of a JWT token. `exp` should be the expiration time of the token and + `nbf` should be the time when the token becomes active. + + Returns True if `exp` is in the future and `nbf` is in the past or matches + the current time. + """ + now = datetime.now(timezone.utc) + expires = datetime.fromtimestamp(exp, tz=timezone.utc) + activates = datetime.fromtimestamp(nbf, tz=timezone.utc) + return now >= activates and now < expires diff --git a/tests/unittests/models/jwtrefreshtoken_test.py b/tests/unittests/models/jwtrefreshtoken_test.py new file mode 100644 index 0000000000..d6602b6a98 --- /dev/null +++ b/tests/unittests/models/jwtrefreshtoken_test.py @@ -0,0 +1,58 @@ +from datetime import datetime, timedelta + +from nav.models.api import JWTRefreshToken + + +class TestIsActive: + def test_should_return_false_if_token_activates_in_the_future(self): + now = datetime.now() + token = JWTRefreshToken( + name="testtoken", + hash="dummyhash", + expires=now + timedelta(hours=1), + activates=now + timedelta(hours=1), + ) + assert not token.is_active() + + def test_should_return_false_if_token_expires_in_the_past(self): + now = datetime.now() + token = JWTRefreshToken( + name="testtoken", + hash="dummyhash", + expires=now - timedelta(hours=1), + activates=now - timedelta(hours=1), + ) + assert not token.is_active() + + def test_should_return_true_if_token_activates_in_the_past_and_expires_in_the_future( + self, + ): + now = datetime.now() + token = JWTRefreshToken( + name="testtoken", + hash="dummyhash", + expires=now + timedelta(hours=1), + activates=now - timedelta(hours=1), + ) + assert token.is_active() + + def test_should_return_true_if_token_activates_now_and_expires_in_the_future(self): + now = datetime.now() + token = JWTRefreshToken( + name="testtoken", + hash="dummyhash", + expires=now + timedelta(hours=1), + activates=now, + ) + assert token.is_active() + + +def test_string_representation_should_match_name(): + now = datetime.now() + token = JWTRefreshToken( + name="testtoken", + hash="dummyhash", + expires=now + timedelta(hours=1), + activates=now - timedelta(hours=1), + ) + assert str(token) == token.name From 398d22368232cd5720a6b3e84dba28c32eca6a8f Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 25 Feb 2025 09:07:17 +0100 Subject: [PATCH 2/9] fixup! Add model for JWT refresh token --- python/nav/models/api.py | 7 +++++++ python/nav/web/jwtgen.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 7538b53d85..4b8cd0aecb 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -70,6 +70,11 @@ class Meta(object): class JWTRefreshToken(models.Model): + """Model representing a JWT refresh token. This model does not + contain the token itself, but a hash of the token that can be used + to validate the authenticity of the actual token when it is used to + generate an access token. + """ name = VarcharField(unique=True) description = models.TextField(null=True, blank=True) @@ -90,4 +95,6 @@ def is_active(self) -> bool: return is_active(self.expires.timestamp(), self.activates.timestamp()) class Meta(object): + """Meta class""" + db_table = 'jwtrefreshtoken' diff --git a/python/nav/web/jwtgen.py b/python/nav/web/jwtgen.py index 4da2423ac1..f013843a2c 100644 --- a/python/nav/web/jwtgen.py +++ b/python/nav/web/jwtgen.py @@ -53,7 +53,7 @@ def _generate_token( def is_active(exp: float, nbf: float) -> bool: """ - Takes `exp` and `nbf` as POSIX timestamps. These represent the claims + Takes `exp` (expiration time) and `nbf` (not before time) as POSIX timestamps. These represent the claims of a JWT token. `exp` should be the expiration time of the token and `nbf` should be the time when the token becomes active. From 51a22b1f19add0bfdd06a6125742c31a82db57b8 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 25 Feb 2025 09:46:09 +0100 Subject: [PATCH 3/9] fixup! fixup! Add model for JWT refresh token --- python/nav/web/jwtgen.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/nav/web/jwtgen.py b/python/nav/web/jwtgen.py index f013843a2c..d85f0730ef 100644 --- a/python/nav/web/jwtgen.py +++ b/python/nav/web/jwtgen.py @@ -53,9 +53,9 @@ def _generate_token( def is_active(exp: float, nbf: float) -> bool: """ - Takes `exp` (expiration time) and `nbf` (not before time) as POSIX timestamps. These represent the claims - of a JWT token. `exp` should be the expiration time of the token and - `nbf` should be the time when the token becomes active. + Takes `exp` (expiration time) and `nbf` (not before time) as POSIX timestamps. + These represent the claims of a JWT token. `exp` should be the expiration + time of the token and `nbf` should be the time when the token becomes active. Returns True if `exp` is in the future and `nbf` is in the past or matches the current time. From 559f50c168ff482583f0cb0dec5504a5feff5a96 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 4 Mar 2025 10:44:41 +0100 Subject: [PATCH 4/9] fixup! Add model for JWT refresh token --- python/nav/web/jwtgen.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/nav/web/jwtgen.py b/python/nav/web/jwtgen.py index d85f0730ef..2aa1149b6a 100644 --- a/python/nav/web/jwtgen.py +++ b/python/nav/web/jwtgen.py @@ -60,7 +60,7 @@ def is_active(exp: float, nbf: float) -> bool: Returns True if `exp` is in the future and `nbf` is in the past or matches the current time. """ - now = datetime.now(timezone.utc) - expires = datetime.fromtimestamp(exp, tz=timezone.utc) - activates = datetime.fromtimestamp(nbf, tz=timezone.utc) + now = datetime.now() + expires = datetime.fromtimestamp(exp) + activates = datetime.fromtimestamp(nbf) return now >= activates and now < expires From 8900d52f34ecdc320ec3b43c0de1bf745314a1ca Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 4 Mar 2025 10:45:12 +0100 Subject: [PATCH 5/9] fixup! Add model for JWT refresh token --- python/nav/models/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 4b8cd0aecb..53303735c7 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -80,7 +80,7 @@ class JWTRefreshToken(models.Model): description = models.TextField(null=True, blank=True) expires = models.DateTimeField() activates = models.DateTimeField() - last_used = models.DateTimeField(null=True) + last_used = models.DateTimeField(null=True, blank=True) revoked = models.BooleanField(default=False) hash = VarcharField() From e03278b6a37a2fbd90d7b27cb959fdb21d623d2b Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit <31960753+stveit@users.noreply.github.com> Date: Tue, 4 Mar 2025 10:47:31 +0100 Subject: [PATCH 6/9] Rename tests Co-authored-by: Johanna England --- tests/unittests/models/jwtrefreshtoken_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unittests/models/jwtrefreshtoken_test.py b/tests/unittests/models/jwtrefreshtoken_test.py index d6602b6a98..d086f56204 100644 --- a/tests/unittests/models/jwtrefreshtoken_test.py +++ b/tests/unittests/models/jwtrefreshtoken_test.py @@ -14,7 +14,7 @@ def test_should_return_false_if_token_activates_in_the_future(self): ) assert not token.is_active() - def test_should_return_false_if_token_expires_in_the_past(self): + def test_should_return_false_if_token_expired_in_the_past(self): now = datetime.now() token = JWTRefreshToken( name="testtoken", @@ -24,7 +24,7 @@ def test_should_return_false_if_token_expires_in_the_past(self): ) assert not token.is_active() - def test_should_return_true_if_token_activates_in_the_past_and_expires_in_the_future( + def test_should_return_true_if_token_activated_in_the_past_and_expires_in_the_future( self, ): now = datetime.now() From 934f9add1abff0c4411cac24fc9bbfd9b41fa9ff Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 6 Mar 2025 11:23:54 +0100 Subject: [PATCH 7/9] fixup! Add model for JWT refresh token --- python/nav/web/jwtgen.py | 7 +++-- .../unittests/models/jwtrefreshtoken_test.py | 6 +++- tests/unittests/web/jwtgen_test.py | 31 +++++++++++++++++-- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/python/nav/web/jwtgen.py b/python/nav/web/jwtgen.py index 2aa1149b6a..a7883aec4a 100644 --- a/python/nav/web/jwtgen.py +++ b/python/nav/web/jwtgen.py @@ -5,6 +5,9 @@ from nav.jwtconf import JWTConf, ACCESS_TOKEN_EXPIRE_DELTA, REFRESH_TOKEN_EXPIRE_DELTA +# Alias for datetime.now for mocking purposes +get_now = datetime.now + def generate_access_token(token_data: Optional[dict[str, Any]] = None) -> str: """Generates and returns an access token in JWT format. @@ -34,7 +37,7 @@ def _generate_token( else: new_token = dict(token_data) - now = datetime.now(timezone.utc) + now = get_now(timezone.utc) name = JWTConf().get_nav_name() updated_claims = { 'exp': (now + expiry_delta).timestamp(), @@ -60,7 +63,7 @@ def is_active(exp: float, nbf: float) -> bool: Returns True if `exp` is in the future and `nbf` is in the past or matches the current time. """ - now = datetime.now() + now = get_now() expires = datetime.fromtimestamp(exp) activates = datetime.fromtimestamp(nbf) return now >= activates and now < expires diff --git a/tests/unittests/models/jwtrefreshtoken_test.py b/tests/unittests/models/jwtrefreshtoken_test.py index d086f56204..3f51fc364e 100644 --- a/tests/unittests/models/jwtrefreshtoken_test.py +++ b/tests/unittests/models/jwtrefreshtoken_test.py @@ -1,4 +1,5 @@ from datetime import datetime, timedelta +from unittest.mock import patch from nav.models.api import JWTRefreshToken @@ -44,7 +45,10 @@ def test_should_return_true_if_token_activates_now_and_expires_in_the_future(sel expires=now + timedelta(hours=1), activates=now, ) - assert token.is_active() + # Make sure the value we use for `activates` here matches + # the `now` value in jwtgen.is_active + with patch('nav.web.jwtgen.get_now', return_value=now): + assert token.is_active() def test_string_representation_should_match_name(): diff --git a/tests/unittests/web/jwtgen_test.py b/tests/unittests/web/jwtgen_test.py index 40d30086c2..e90f017bab 100644 --- a/tests/unittests/web/jwtgen_test.py +++ b/tests/unittests/web/jwtgen_test.py @@ -1,10 +1,10 @@ import pytest from unittest.mock import Mock, patch -from datetime import datetime +from datetime import datetime, timedelta import jwt -from nav.web.jwtgen import generate_access_token, generate_refresh_token +from nav.web.jwtgen import generate_access_token, generate_refresh_token, is_active class TestTokenGeneration: @@ -55,6 +55,33 @@ def test_token_type_should_be_refresh_token(self): assert data['token_type'] == "refresh_token" +class TestIsActive: + def test_should_return_false_if_nbf_is_in_the_future(self): + now = datetime.now() + nbf = now + timedelta(hours=1) + exp = now + timedelta(hours=1) + assert not is_active(exp.timestamp(), nbf.timestamp()) + + def test_should_return_false_if_exp_is_in_the_past(self): + now = datetime.now() + nbf = now - timedelta(hours=1) + exp = now - timedelta(hours=1) + assert not is_active(exp.timestamp(), nbf.timestamp()) + + def test_should_return_true_if_nbf_is_in_the_past_and_exp_is_in_the_future(self): + now = datetime.now() + nbf = now - timedelta(hours=1) + exp = now + timedelta(hours=1) + assert is_active(exp.timestamp(), nbf.timestamp()) + + def test_should_return_true_if_nbf_is_now_and_exp_is_in_the_future(self): + now = datetime.now() + exp = now + timedelta(hours=1) + # Make sure the value we use for `nbf` here matches the `now` value in jwtgen.is_active + with patch('nav.web.jwtgen.get_now', return_value=now): + assert is_active(exp.timestamp(), now.timestamp()) + + @pytest.fixture(scope="module", autouse=True) def jwtconf_mock(private_key, nav_name) -> str: """Mocks the get_nav_name and get_nav_private_key functions for From 3896099f8602583124aef18c9be5038ac07150e3 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 6 Mar 2025 13:59:30 +0100 Subject: [PATCH 8/9] fixup! Add model for JWT refresh token --- python/nav/models/sql/changes/sc.05.13.0001.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/nav/models/sql/changes/sc.05.13.0001.sql b/python/nav/models/sql/changes/sc.05.13.0001.sql index c0312b14ac..37b551664b 100644 --- a/python/nav/models/sql/changes/sc.05.13.0001.sql +++ b/python/nav/models/sql/changes/sc.05.13.0001.sql @@ -1,4 +1,4 @@ -CREATE TABLE manage.JWTRefreshToken ( +CREATE TABLE manage.jwtrefreshtoken ( id SERIAL PRIMARY KEY, name VARCHAR NOT NULL UNIQUE, description VARCHAR, From a3db8c2de9b11adee6edb5666520a67dafa142df Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 6 Mar 2025 14:08:17 +0100 Subject: [PATCH 9/9] fixup! Add model for JWT refresh token --- tests/unittests/models/jwtrefreshtoken_test.py | 10 ++++++---- tests/unittests/web/jwtgen_test.py | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/unittests/models/jwtrefreshtoken_test.py b/tests/unittests/models/jwtrefreshtoken_test.py index 3f51fc364e..080b7c6162 100644 --- a/tests/unittests/models/jwtrefreshtoken_test.py +++ b/tests/unittests/models/jwtrefreshtoken_test.py @@ -5,7 +5,7 @@ class TestIsActive: - def test_should_return_false_if_token_activates_in_the_future(self): + def test_when_token_activates_in_the_future_it_should_return_false(self): now = datetime.now() token = JWTRefreshToken( name="testtoken", @@ -15,7 +15,7 @@ def test_should_return_false_if_token_activates_in_the_future(self): ) assert not token.is_active() - def test_should_return_false_if_token_expired_in_the_past(self): + def test_when_token_expired_in_the_past_it_should_return_false(self): now = datetime.now() token = JWTRefreshToken( name="testtoken", @@ -25,7 +25,7 @@ def test_should_return_false_if_token_expired_in_the_past(self): ) assert not token.is_active() - def test_should_return_true_if_token_activated_in_the_past_and_expires_in_the_future( + def test_when_token_activated_in_the_past_and_expires_in_the_future_it_should_return_true( self, ): now = datetime.now() @@ -37,7 +37,9 @@ def test_should_return_true_if_token_activated_in_the_past_and_expires_in_the_fu ) assert token.is_active() - def test_should_return_true_if_token_activates_now_and_expires_in_the_future(self): + def test_when_token_activates_now_and_expires_in_the_future_it_should_return_true( + self, + ): now = datetime.now() token = JWTRefreshToken( name="testtoken", diff --git a/tests/unittests/web/jwtgen_test.py b/tests/unittests/web/jwtgen_test.py index e90f017bab..f2019a069c 100644 --- a/tests/unittests/web/jwtgen_test.py +++ b/tests/unittests/web/jwtgen_test.py @@ -56,25 +56,27 @@ def test_token_type_should_be_refresh_token(self): class TestIsActive: - def test_should_return_false_if_nbf_is_in_the_future(self): + def test_when_nbf_is_in_the_future_it_should_return_false(self): now = datetime.now() nbf = now + timedelta(hours=1) exp = now + timedelta(hours=1) assert not is_active(exp.timestamp(), nbf.timestamp()) - def test_should_return_false_if_exp_is_in_the_past(self): + def test_when_exp_is_in_the_past_it_should_return_false(self): now = datetime.now() nbf = now - timedelta(hours=1) exp = now - timedelta(hours=1) assert not is_active(exp.timestamp(), nbf.timestamp()) - def test_should_return_true_if_nbf_is_in_the_past_and_exp_is_in_the_future(self): + def test_when_nbf_is_in_the_past_and_exp_is_in_the_future_it_should_return_true( + self, + ): now = datetime.now() nbf = now - timedelta(hours=1) exp = now + timedelta(hours=1) assert is_active(exp.timestamp(), nbf.timestamp()) - def test_should_return_true_if_nbf_is_now_and_exp_is_in_the_future(self): + def test_when_nbf_is_now_and_exp_is_in_the_future_it_should_return_true(self): now = datetime.now() exp = now + timedelta(hours=1) # Make sure the value we use for `nbf` here matches the `now` value in jwtgen.is_active