From 8c81ea8df1bd7c847f69dbead357f8f69e5bfd03 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 08:55:50 +0100 Subject: [PATCH 01/27] Add pyjwt dependency --- requirements/base.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/requirements/base.txt b/requirements/base.txt index a5b1a93390..55139b16cf 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -38,3 +38,5 @@ napalm==3.4.1 backports.zoneinfo ; python_version < '3.9' git+https://github.com/Uninett/drf-oidc-auth@v4.0#egg=drf-oidc-auth + +pyjwt>=2.6.0 From 8bde9e3d755de9fe4ea734e86fe8c2e5eb08bd3d Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 08:56:36 +0100 Subject: [PATCH 02/27] Add jwt refresh token model --- python/nav/models/api.py | 40 ++++++++++++++++++- .../nav/models/sql/changes/sc.05.05.0002.sql | 7 ++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 python/nav/models/sql/changes/sc.05.05.0002.sql diff --git a/python/nav/models/api.py b/python/nav/models/api.py index f7495fe9c5..d90a231729 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -15,7 +15,9 @@ # """Models for the NAV API""" -from datetime import datetime +from datetime import datetime, timedelta + +import jwt from django.db import models from django.urls import reverse @@ -66,3 +68,39 @@ def get_absolute_url(self): class Meta(object): db_table = 'apitoken' + + +class JWTRefreshToken(models.Model): + """RefreshTokens are used for generating new access tokens""" + + token = VarcharField() + name = VarcharField(unique=True) + description = models.TextField(null=True, blank=True) + + ACCESS_EXPIRE_DELTA = timedelta(hours=1) + REFRESH_EXPIRE_DELTA = timedelta(days=1) + + def __str__(self): + return self.token + + def data(self): + """Body of token as a dict""" + return jwt.decode(self.token, options={'verify_signature': False}) + + def activates(self): + """Datetime when token activates""" + data = self.data() + return datetime.fromtimestamp(data['nbf']) + + def expires(self): + """Datetime when token expires""" + data = self.data() + return datetime.fromtimestamp(data['exp']) + + def is_active(self): + """True if token is active""" + now = datetime.now() + return now >= self.activates() and now < self.expires() + + class Meta(object): + db_table = 'jwtrefreshtoken' diff --git a/python/nav/models/sql/changes/sc.05.05.0002.sql b/python/nav/models/sql/changes/sc.05.05.0002.sql new file mode 100644 index 0000000000..bfb921175e --- /dev/null +++ b/python/nav/models/sql/changes/sc.05.05.0002.sql @@ -0,0 +1,7 @@ +--- Create table for storing JWT refresh tokens +CREATE TABLE JWTRefreshToken ( + id SERIAL PRIMARY KEY, + token VARCHAR NOT NULL, + name VARCHAR NOT NULL UNIQUE, + description VARCHAR +); From d8f5a642b664c900e085d96379389b492c268f6f Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 08:57:20 +0100 Subject: [PATCH 03/27] Add methods for generating jwt tokens --- python/nav/models/api.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index d90a231729..baa768a21d 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -25,6 +25,7 @@ from nav.adapters import HStoreField from nav.models.fields import VarcharField from nav.models.profiles import Account +from nav.jwtconf import JWTConf class APIToken(models.Model): @@ -102,5 +103,37 @@ def is_active(self): now = datetime.now() return now >= self.activates() and now < self.expires() + @classmethod + def _encode_token(cls, token_data): + return jwt.encode( + token_data, JWTConf().get_nav_private_key(), algorithm="RS256" + ) + + @classmethod + def _generate_token(cls, token_data, expiry_delta, token_type): + new_token = dict(token_data) + now = datetime.now() + name = JWTConf().get_nav_name() + updated_claims = { + 'exp': (now + expiry_delta).timestamp(), + 'nbf': now.timestamp(), + 'iat': now.timestamp(), + 'aud': name, + 'iss': name, + 'token_type': token_type, + } + new_token.update(updated_claims) + return cls._encode_token(new_token) + + @classmethod + def generate_access_token(cls, token_data={}): + return cls._generate_token(token_data, cls.ACCESS_EXPIRE_DELTA, "access_token") + + @classmethod + def generate_refresh_token(cls, token_data={}): + return cls._generate_token( + token_data, cls.REFRESH_EXPIRE_DELTA, "refresh_token" + ) + class Meta(object): db_table = 'jwtrefreshtoken' From 090f8e8dc72185fe6b73dcc7f1bc16b145ff95ed Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 08:57:39 +0100 Subject: [PATCH 04/27] Add function for expiring jwt token --- python/nav/models/api.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index baa768a21d..6deb3c0589 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -103,6 +103,14 @@ def is_active(self): now = datetime.now() return now >= self.activates() and now < self.expires() + def expire(self): + """Expires the token""" + data = self.data() + data['exp'] = datetime.now().timestamp() + data['nbf'] = datetime.now().timestamp() + self.token = self._encode_token(data) + self.save() + @classmethod def _encode_token(cls, token_data): return jwt.encode( From b7a8fa64045c6ad252ea5e89d62bb91af60157cb Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 09:08:33 +0100 Subject: [PATCH 05/27] Put model in manage namespace --- python/nav/models/sql/changes/sc.05.05.0002.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/nav/models/sql/changes/sc.05.05.0002.sql b/python/nav/models/sql/changes/sc.05.05.0002.sql index bfb921175e..8c3defa9c3 100644 --- a/python/nav/models/sql/changes/sc.05.05.0002.sql +++ b/python/nav/models/sql/changes/sc.05.05.0002.sql @@ -1,5 +1,5 @@ --- Create table for storing JWT refresh tokens -CREATE TABLE JWTRefreshToken ( +CREATE TABLE manage.JWTRefreshToken ( id SERIAL PRIMARY KEY, token VARCHAR NOT NULL, name VARCHAR NOT NULL UNIQUE, From b7db4779f3f86e2d05c7b4e56a578974143efcff Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 7 Nov 2023 13:31:30 +0100 Subject: [PATCH 06/27] Mark certain functions as properties --- python/nav/models/api.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 6deb3c0589..500a5b6e52 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -84,31 +84,32 @@ class JWTRefreshToken(models.Model): def __str__(self): return self.token + @property def data(self): """Body of token as a dict""" return jwt.decode(self.token, options={'verify_signature': False}) + @property def activates(self): """Datetime when token activates""" - data = self.data() - return datetime.fromtimestamp(data['nbf']) + return datetime.fromtimestamp(self.data['nbf']) + @property def expires(self): """Datetime when token expires""" - data = self.data() - return datetime.fromtimestamp(data['exp']) + return datetime.fromtimestamp(self.data['exp']) + @property def is_active(self): """True if token is active""" now = datetime.now() - return now >= self.activates() and now < self.expires() + return now >= self.activates and now < self.expires def expire(self): """Expires the token""" - data = self.data() - data['exp'] = datetime.now().timestamp() - data['nbf'] = datetime.now().timestamp() - self.token = self._encode_token(data) + self.data['exp'] = datetime.now().timestamp() + self.data['nbf'] = datetime.now().timestamp() + self.token = self._encode_token(self.data) self.save() @classmethod From 348612d5348ab9d08926596bac61f47582c2967a Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 7 Nov 2023 14:11:31 +0100 Subject: [PATCH 07/27] Make jwtconf data easier to mock --- python/nav/models/api.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 500a5b6e52..2f6290be0d 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -112,17 +112,23 @@ def expire(self): self.token = self._encode_token(self.data) self.save() + @classmethod + def _get_private_key(self): + return JWTConf().get_nav_private_key() + + @classmethod + def _get_nav_name(self): + return JWTConf().get_nav_name() + @classmethod def _encode_token(cls, token_data): - return jwt.encode( - token_data, JWTConf().get_nav_private_key(), algorithm="RS256" - ) + return jwt.encode(token_data, cls._get_private_key(), algorithm="RS256") @classmethod def _generate_token(cls, token_data, expiry_delta, token_type): new_token = dict(token_data) now = datetime.now() - name = JWTConf().get_nav_name() + name = cls._get_nav_name() updated_claims = { 'exp': (now + expiry_delta).timestamp(), 'nbf': now.timestamp(), From 0404d5c34e3cd1c350f56e8cd0cd05c29091a26c Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 7 Nov 2023 14:41:44 +0100 Subject: [PATCH 08/27] fixup! Mark certain functions as properties --- python/nav/models/api.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 2f6290be0d..82276680b2 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -107,9 +107,11 @@ def is_active(self): def expire(self): """Expires the token""" - self.data['exp'] = datetime.now().timestamp() - self.data['nbf'] = datetime.now().timestamp() - self.token = self._encode_token(self.data) + # Base claims for expired token on existing claims + expired_data = self.data + expired_data['exp'] = datetime.now().timestamp() + expired_data['nbf'] = datetime.now().timestamp() + self.token = self._encode_token(expired_data) self.save() @classmethod From 0ca8096fb2e5c07237665b4cb561df9fb25fe3de Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 7 Nov 2023 14:42:11 +0100 Subject: [PATCH 09/27] Rename properties to be more explicit Now shows very clearly that you are getting exp calim or nbf claim --- python/nav/models/api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 82276680b2..d38ea1dfc7 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -90,12 +90,12 @@ def data(self): return jwt.decode(self.token, options={'verify_signature': False}) @property - def activates(self): + def nbf(self): """Datetime when token activates""" return datetime.fromtimestamp(self.data['nbf']) @property - def expires(self): + def exp(self): """Datetime when token expires""" return datetime.fromtimestamp(self.data['exp']) @@ -103,7 +103,7 @@ def expires(self): def is_active(self): """True if token is active""" now = datetime.now() - return now >= self.activates and now < self.expires + return now >= self.nbf and now < self.exp def expire(self): """Expires the token""" From 2103df55bc5033df230a9a49a6a69c4a25b0ef6b Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Tue, 7 Nov 2023 14:49:01 +0100 Subject: [PATCH 10/27] Add static method for decoding token --- python/nav/models/api.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index d38ea1dfc7..8d16087ad9 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -87,7 +87,7 @@ def __str__(self): @property def data(self): """Body of token as a dict""" - return jwt.decode(self.token, options={'verify_signature': False}) + return self.decode_token(self.token) @property def nbf(self): @@ -152,5 +152,9 @@ def generate_refresh_token(cls, token_data={}): token_data, cls.REFRESH_EXPIRE_DELTA, "refresh_token" ) + @classmethod + def decode_token(cls, token): + return jwt.decode(token, options={'verify_signature': False}) + class Meta(object): db_table = 'jwtrefreshtoken' From 6cd5833531fcc36eb1e287cb7c95f8ab694617eb Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 9 Nov 2023 12:24:08 +0100 Subject: [PATCH 11/27] Update docstring --- python/nav/models/api.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 8d16087ad9..92da3e5e7a 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -101,7 +101,9 @@ def exp(self): @property def is_active(self): - """True if token is active""" + """True if token is active. A token is considered active when + the nbf claim is in the past and the exp claim is in the future + """ now = datetime.now() return now >= self.nbf and now < self.exp From 10c3e7c6f7e69390efb7f68bde340536df4bca4c Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 9 Nov 2023 13:17:48 +0100 Subject: [PATCH 12/27] Improve function documentation --- python/nav/models/api.py | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 92da3e5e7a..25da55d201 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -16,6 +16,7 @@ """Models for the NAV API""" from datetime import datetime, timedelta +from typing import Dict, Any import jwt @@ -85,22 +86,22 @@ def __str__(self): return self.token @property - def data(self): + def data(self) -> Dict[str, Any]: """Body of token as a dict""" return self.decode_token(self.token) @property - def nbf(self): + def nbf(self) -> datetime: """Datetime when token activates""" return datetime.fromtimestamp(self.data['nbf']) @property - def exp(self): + def exp(self) -> datetime: """Datetime when token expires""" return datetime.fromtimestamp(self.data['exp']) @property - def is_active(self): + def is_active(self) -> bool: """True if token is active. A token is considered active when the nbf claim is in the past and the exp claim is in the future """ @@ -117,19 +118,27 @@ def expire(self): self.save() @classmethod - def _get_private_key(self): + def _get_private_key(self) -> str: + """Returns private key in PEM format""" return JWTConf().get_nav_private_key() @classmethod - def _get_nav_name(self): + def _get_nav_name(self) -> str: + """Returns the name of this NAV instance. This is used for the iss and aud claim""" return JWTConf().get_nav_name() @classmethod - def _encode_token(cls, token_data): + def _encode_token(cls, token_data: Dict[str, Any]) -> str: + """Returns an encoded token in JWT format""" return jwt.encode(token_data, cls._get_private_key(), algorithm="RS256") @classmethod - def _generate_token(cls, token_data, expiry_delta, token_type): + def _generate_token( + cls, token_data: Dict[str, Any], expiry_delta: timedelta, token_type: str + ) -> str: + """Generates and returns a token in JWT format. Will use `token_data` as a basis + for the new token, but certain claims will be overridden + """ new_token = dict(token_data) now = datetime.now() name = cls._get_nav_name() @@ -145,17 +154,24 @@ def _generate_token(cls, token_data, expiry_delta, token_type): return cls._encode_token(new_token) @classmethod - def generate_access_token(cls, token_data={}): + def generate_access_token(cls, token_data: Dict[str, Any] = {}) -> str: + """Generates and returns an access token in JWT format. Will use `token_data` as a basis + for the new token, but certain claims will be overridden + """ return cls._generate_token(token_data, cls.ACCESS_EXPIRE_DELTA, "access_token") @classmethod - def generate_refresh_token(cls, token_data={}): + def generate_refresh_token(cls, token_data: Dict[str, Any] = {}) -> str: + """Generates and returns a refresh token in JWT format. Will use `token_data` as a basis + for the new token, but certain claims will be overridden + """ return cls._generate_token( token_data, cls.REFRESH_EXPIRE_DELTA, "refresh_token" ) @classmethod - def decode_token(cls, token): + def decode_token(cls, token: str) -> Dict[str, Any]: + """Decodes a token in JWT format and returns the body of the decoded token""" return jwt.decode(token, options={'verify_signature': False}) class Meta(object): From 3386f29624af92afff8233cda892f040977d3e2e Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 9 Nov 2023 13:57:56 +0100 Subject: [PATCH 13/27] Use jwt conf directly no longer needed for testing, found better way --- python/nav/models/api.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 25da55d201..4e6a7544ee 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -117,20 +117,12 @@ def expire(self): self.token = self._encode_token(expired_data) self.save() - @classmethod - def _get_private_key(self) -> str: - """Returns private key in PEM format""" - return JWTConf().get_nav_private_key() - - @classmethod - def _get_nav_name(self) -> str: - """Returns the name of this NAV instance. This is used for the iss and aud claim""" - return JWTConf().get_nav_name() - @classmethod def _encode_token(cls, token_data: Dict[str, Any]) -> str: """Returns an encoded token in JWT format""" - return jwt.encode(token_data, cls._get_private_key(), algorithm="RS256") + return jwt.encode( + token_data, JWTConf().get_nav_private_key(), algorithm="RS256" + ) @classmethod def _generate_token( @@ -141,7 +133,7 @@ def _generate_token( """ new_token = dict(token_data) now = datetime.now() - name = cls._get_nav_name() + name = JWTConf().get_nav_name() updated_claims = { 'exp': (now + expiry_delta).timestamp(), 'nbf': now.timestamp(), From 217cd4a52ee3f9144bac24ca5acb99c61ca5e69b Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Thu, 9 Nov 2023 15:17:37 +0100 Subject: [PATCH 14/27] Mark function as private --- python/nav/models/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 4e6a7544ee..999a4b12d1 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -88,7 +88,7 @@ def __str__(self): @property def data(self) -> Dict[str, Any]: """Body of token as a dict""" - return self.decode_token(self.token) + return self._decode_token(self.token) @property def nbf(self) -> datetime: @@ -162,7 +162,7 @@ def generate_refresh_token(cls, token_data: Dict[str, Any] = {}) -> str: ) @classmethod - def decode_token(cls, token: str) -> Dict[str, Any]: + def _decode_token(cls, token: str) -> Dict[str, Any]: """Decodes a token in JWT format and returns the body of the decoded token""" return jwt.decode(token, options={'verify_signature': False}) From 104ee18e70c331aca6ad284d910b089c97a88bfe Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Fri, 10 Nov 2023 13:04:16 +0100 Subject: [PATCH 15/27] Remove properties and use directly in expire funn --- python/nav/models/api.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 999a4b12d1..f6aec3036c 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -90,23 +90,15 @@ def data(self) -> Dict[str, Any]: """Body of token as a dict""" return self._decode_token(self.token) - @property - def nbf(self) -> datetime: - """Datetime when token activates""" - return datetime.fromtimestamp(self.data['nbf']) - - @property - def exp(self) -> datetime: - """Datetime when token expires""" - return datetime.fromtimestamp(self.data['exp']) - @property def is_active(self) -> bool: """True if token is active. A token is considered active when the nbf claim is in the past and the exp claim is in the future """ now = datetime.now() - return now >= self.nbf and now < self.exp + nbf = datetime.fromtimestamp(self.data['nbf']) + exp = datetime.fromtimestamp(self.data['exp']) + return now >= nbf and now < exp def expire(self): """Expires the token""" From a7554c43c8bc6af7bd2ce5edca2094ec187cb36b Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Fri, 10 Nov 2023 13:05:55 +0100 Subject: [PATCH 16/27] Set exp claim to the past when expiring --- python/nav/models/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index f6aec3036c..52a2cb0d7a 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -104,8 +104,9 @@ def expire(self): """Expires the token""" # Base claims for expired token on existing claims expired_data = self.data - expired_data['exp'] = datetime.now().timestamp() - expired_data['nbf'] = datetime.now().timestamp() + now = datetime.now() + expired_data['exp'] = (now - timedelta(hours=1)).timestamp() + expired_data['nbf'] = now.timestamp() self.token = self._encode_token(expired_data) self.save() From 9eebe3f0bf2495e609c911c9bc6b0289fb9ec0a2 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Fri, 10 Nov 2023 13:06:23 +0100 Subject: [PATCH 17/27] Dont change nbf claim in expire doesnt actually do anything. if you set it to the past, then its valid and if you set it to the future then it will eventually be valid. setting exp to the past is enought to deactivate it for good --- python/nav/models/api.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 52a2cb0d7a..a00a8dd99a 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -104,9 +104,7 @@ def expire(self): """Expires the token""" # Base claims for expired token on existing claims expired_data = self.data - now = datetime.now() - expired_data['exp'] = (now - timedelta(hours=1)).timestamp() - expired_data['nbf'] = now.timestamp() + expired_data['exp'] = (datetime.now() - timedelta(hours=1)).timestamp() self.token = self._encode_token(expired_data) self.save() From 7522e965297c5a8b7fc98b93b88f1e7523227f18 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 10:38:02 +0100 Subject: [PATCH 18/27] Use property once --- python/nav/models/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index a00a8dd99a..a74e430b78 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -96,8 +96,9 @@ def is_active(self) -> bool: the nbf claim is in the past and the exp claim is in the future """ now = datetime.now() - nbf = datetime.fromtimestamp(self.data['nbf']) - exp = datetime.fromtimestamp(self.data['exp']) + data = self.data + nbf = datetime.fromtimestamp(data['nbf']) + exp = datetime.fromtimestamp(data['exp']) return now >= nbf and now < exp def expire(self): From 0a300cf27f8fd9a1d8693e8e97bc3d4ebd9c8035 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 10:46:12 +0100 Subject: [PATCH 19/27] Remove property decorators doesnt really add anything --- python/nav/models/api.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index a74e430b78..19d5ce5442 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -85,26 +85,24 @@ class JWTRefreshToken(models.Model): def __str__(self): return self.token - @property - def data(self) -> Dict[str, Any]: + def get_body(self) -> Dict[str, Any]: """Body of token as a dict""" return self._decode_token(self.token) - @property def is_active(self) -> bool: """True if token is active. A token is considered active when the nbf claim is in the past and the exp claim is in the future """ now = datetime.now() - data = self.data - nbf = datetime.fromtimestamp(data['nbf']) - exp = datetime.fromtimestamp(data['exp']) + body = self.get_body() + nbf = datetime.fromtimestamp(body['nbf']) + exp = datetime.fromtimestamp(body['exp']) return now >= nbf and now < exp def expire(self): """Expires the token""" # Base claims for expired token on existing claims - expired_data = self.data + expired_data = self.body expired_data['exp'] = (datetime.now() - timedelta(hours=1)).timestamp() self.token = self._encode_token(expired_data) self.save() From 3fcbc40c6dc4a6688a7283aeea36b9cd6e7f9466 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 10:59:33 +0100 Subject: [PATCH 20/27] Reorder class --- python/nav/models/api.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index 19d5ce5442..221b665770 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -108,10 +108,19 @@ def expire(self): self.save() @classmethod - def _encode_token(cls, token_data: Dict[str, Any]) -> str: - """Returns an encoded token in JWT format""" - return jwt.encode( - token_data, JWTConf().get_nav_private_key(), algorithm="RS256" + def generate_access_token(cls, token_data: Dict[str, Any] = {}) -> str: + """Generates and returns an access token in JWT format. Will use `token_data` as a basis + for the new token, but certain claims will be overridden + """ + return cls._generate_token(token_data, cls.ACCESS_EXPIRE_DELTA, "access_token") + + @classmethod + def generate_refresh_token(cls, token_data: Dict[str, Any] = {}) -> str: + """Generates and returns a refresh token in JWT format. Will use `token_data` as a basis + for the new token, but certain claims will be overridden + """ + return cls._generate_token( + token_data, cls.REFRESH_EXPIRE_DELTA, "refresh_token" ) @classmethod @@ -136,19 +145,10 @@ def _generate_token( return cls._encode_token(new_token) @classmethod - def generate_access_token(cls, token_data: Dict[str, Any] = {}) -> str: - """Generates and returns an access token in JWT format. Will use `token_data` as a basis - for the new token, but certain claims will be overridden - """ - return cls._generate_token(token_data, cls.ACCESS_EXPIRE_DELTA, "access_token") - - @classmethod - def generate_refresh_token(cls, token_data: Dict[str, Any] = {}) -> str: - """Generates and returns a refresh token in JWT format. Will use `token_data` as a basis - for the new token, but certain claims will be overridden - """ - return cls._generate_token( - token_data, cls.REFRESH_EXPIRE_DELTA, "refresh_token" + def _encode_token(cls, token_data: Dict[str, Any]) -> str: + """Returns an encoded token in JWT format""" + return jwt.encode( + token_data, JWTConf().get_nav_private_key(), algorithm="RS256" ) @classmethod From b0b6e077f5e657ef5afde4a7f7fd622ff7d5d741 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 11:00:43 +0100 Subject: [PATCH 21/27] Fix wrong function name being used --- 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 221b665770..c82592fa80 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -102,7 +102,7 @@ def is_active(self) -> bool: def expire(self): """Expires the token""" # Base claims for expired token on existing claims - expired_data = self.body + expired_data = self.get_body() expired_data['exp'] = (datetime.now() - timedelta(hours=1)).timestamp() self.token = self._encode_token(expired_data) self.save() From 5416307fc78525ce030521f5ae4e968e55e58ec4 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 11:02:18 +0100 Subject: [PATCH 22/27] Add tests --- .../unittests/models/jwtrefreshtoken_test.py | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 tests/unittests/models/jwtrefreshtoken_test.py diff --git a/tests/unittests/models/jwtrefreshtoken_test.py b/tests/unittests/models/jwtrefreshtoken_test.py new file mode 100644 index 0000000000..bc9a7e74bf --- /dev/null +++ b/tests/unittests/models/jwtrefreshtoken_test.py @@ -0,0 +1,209 @@ +import pytest +from unittest.mock import Mock, patch +from datetime import datetime, timedelta +from typing import Dict, Any + +from nav.models.api import JWTRefreshToken + + +class TestGenerateAccessToken: + def test_nbf_should_be_in_the_past(self): + encoded_token = JWTRefreshToken.generate_access_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['nbf'] < datetime.now().timestamp() + + def test_exp_should_be_in_the_future(self): + encoded_token = JWTRefreshToken.generate_access_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['exp'] > datetime.now().timestamp() + + def test_iat_should_be_in_the_past(self): + encoded_token = JWTRefreshToken.generate_access_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['iat'] < datetime.now().timestamp() + + def test_token_type_should_be_access_token(self): + encoded_token = JWTRefreshToken.generate_access_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['token_type'] == "access_token" + + +class TestGenerateRefreshToken: + def test_nbf_should_be_in_the_past(self): + encoded_token = JWTRefreshToken.generate_refresh_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['nbf'] < datetime.now().timestamp() + + def test_exp_should_be_in_the_future(self): + encoded_token = JWTRefreshToken.generate_refresh_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['exp'] > datetime.now().timestamp() + + def test_iat_should_be_in_the_past(self): + encoded_token = JWTRefreshToken.generate_refresh_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['iat'] < datetime.now().timestamp() + + def test_token_type_should_be_refresh_token(self): + encoded_token = JWTRefreshToken.generate_refresh_token() + body = JWTRefreshToken._decode_token(encoded_token) + assert body['token_type'] == "refresh_token" + + +class TestIsActive: + def test_should_return_false_if_nbf_is_in_the_future(self, refresh_token_data): + refresh_token_data['nbf'] = (datetime.now() + timedelta(hours=1)).timestamp() + refresh_token_data['exp'] = (datetime.now() + timedelta(hours=1)).timestamp() + encoded_token = JWTRefreshToken._encode_token(refresh_token_data) + token = JWTRefreshToken(token=encoded_token) + assert not token.is_active() + + def test_should_return_false_if_exp_is_in_the_past(self, refresh_token_data): + refresh_token_data['nbf'] = (datetime.now() - timedelta(hours=1)).timestamp() + refresh_token_data['exp'] = (datetime.now() - timedelta(hours=1)).timestamp() + encoded_token = JWTRefreshToken._encode_token(refresh_token_data) + token = JWTRefreshToken(token=encoded_token) + assert not token.is_active() + + def test_should_return_true_if_nbf_is_in_the_past_and_exp_is_in_the_future( + self, refresh_token_data + ): + now = datetime.now() + refresh_token_data['nbf'] = (now - timedelta(hours=1)).timestamp() + refresh_token_data['exp'] = (now + timedelta(hours=1)).timestamp() + encoded_token = JWTRefreshToken._encode_token(refresh_token_data) + token = JWTRefreshToken(token=encoded_token) + assert token.is_active() + + +class TestExpire: + def test_should_make_active_token_inactive(self, refresh_token_data): + now = datetime.now() + # set claims so the token starts as being active + refresh_token_data['nbf'] = now - timedelta(hours=1) + refresh_token_data['exp'] = now + timedelta(hours=1) + encoded_token = JWTRefreshToken._encode_token(refresh_token_data) + token = JWTRefreshToken(token=encoded_token) + token.save = Mock() + assert token.is_active() + token.expire() + assert not token.is_active() + + +class TestGetBody: + def test_should_return_accurate_representation_of_token_body( + self, refresh_token, refresh_token_data + ): + token = JWTRefreshToken(token=refresh_token) + assert token.get_body() == refresh_token_data + + +class TestDecodeToken: + def test_should_return_same_body_as_when_token_was_encoded( + self, refresh_token, refresh_token_data + ): + decoded_data = JWTRefreshToken._decode_token(refresh_token) + assert decoded_data == refresh_token_data + + +class TestEncodeToken: + def test_should_generate_a_known_token_using_the_same_body( + self, refresh_token, refresh_token_data + ): + encoded_token = JWTRefreshToken._encode_token(refresh_token_data) + assert encoded_token == refresh_token + + +@pytest.fixture() +def refresh_token_data() -> Dict[Any, str]: + """Yields the body of the token in the refresh_token fixture""" + body = { + "exp": 1516339022, + "nbf": 1516239022, + "iat": 1516239022, + "aud": "nav", + "iss": "nav", + "token_type": "refresh_token", + } + yield body + + +@pytest.fixture() +def refresh_token() -> str: + """Yields a refresh token with a body matching the refresh_token_data fixture""" + token = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE1\ +MTYzMzkwMjIsIm5iZiI6MTUxNjIzOTAyMiwiaWF0IjoxNTE2MjM5MDIyLCJh\ +dWQiOiJuYXYiLCJpc3MiOiJuYXYiLCJ0b2tlbl90eXBlIjoicmVmcmVzaF90\ +b2tlbiJ9.LC5YhPTrOQk2q-gPgPHAf9nWF8zjBFBmM6AEh1gSjJgvBdrwqsZ\ +7lqsEAQ09IXBrsZ3UhJDkEh5e31Tcp9afk_5f2dLA5zwcayxEbJ7Bj3M0PPb\ +D_jz5YWqJ1x9YwROh_iOVtTtVze8079rpF_0LIgbibJjJ1BLrvHLtYhTACTx\ +PKfmSJXK60_bg1jRPtlFIilNVdYQ3mnOXjg-9AjsCDH4nzABwiIpAXBR1r-9\ +3AZ_ZYxygwctVQpbIJIr0lTntczZ5sRudpK271JHdvLe-iZFz6MpfNIRBJS8\ +qawbo_kZmetm6zWmrPDcyC95tYfd2JL8XhEGGpB3nfhQipqG8nQ" + yield token + + +@pytest.fixture(scope="module", autouse=True) +def jwtconf_mock(private_key, nav_name) -> str: + """Mocks the get_nave_name and get_nav_private_key functions for + the JWTConf class + """ + with patch("nav.models.api.JWTConf") as _jwtconf_mock: + instance = _jwtconf_mock.return_value + instance.get_nav_name = Mock(return_value=nav_name) + instance.get_nav_private_key = Mock(return_value=private_key) + yield _jwtconf_mock + + +@pytest.fixture(scope="module") +def private_key() -> str: + """Yields a private key in PEM format""" + key = """-----BEGIN PRIVATE KEY----- +MIIEuwIBADANBgkqhkiG9w0BAQEFAASCBKUwggShAgEAAoIBAQCp+4AEZM4uYZKu +/hrKzySMTFFx3/ncWo6XAFpADQHXLOwRB9Xh1/OwigHiqs/wHRAAmnrlkwCCQA8r +xiHBAMjp5ApbkyggQz/DVijrpSba6Tiy1cyBTZC3cvOK2FpJzsakJLhIXD1HaULO +ClyIJB/YrmHmQc8SL3Uzou5mMpdcBC2pzwmEW1cvQURpnvgrDF8V86GrQkjK6nIP +IEeuW6kbD5lWFAPfLf1ohDWex3yxeSFyXNRApJhbF4HrKFemPkOi7acsky38UomQ +jZgAMHPotJNkQvAHcnXHhg0FcWGdohv5bc/Ctt9GwZOzJxwyJLBBsSewbE310TZi +3oLU1TmvAgMBAAECgf8zrhi95+gdMeKRpwV+TnxOK5CXjqvo0vTcnr7Runf/c9On +WeUtRPr83E4LxuMcSGRqdTfoP0loUGb3EsYwZ+IDOnyWWvytfRoQdExSA2RM1PDo +GRiUN4Dy8CrGNqvnb3agG99Ay3Ura6q5T20n9ykM4qKL3yDrO9fmWyMgRJbAOAYm +xzf7H910mDZghXPpq8nzDky0JLNZcaqbxuPQ3+EI4p2dLNXbNqMPs8Y20JKLeOPs +HikRM0zfhHEJSt5IPFQ54/CzscGHGeCleQINWTgvDLMcE5fJMvbLLZixV+YsBfAq +e2JsSubS+9RI2ktMlSKaemr8yeoIpsXfAiJSHkECgYEA0NKU18xK+9w5IXfgNwI4 +peu2tWgwyZSp5R2pdLT7O1dJoLYRoAmcXNePB0VXNARqGxTNypJ9zmMawNmf3YRS +BqG8aKz7qpATlx9OwYlk09fsS6MeVmaur8bHGHP6O+gt7Xg+zhiFPvU9P5LB+C0Z +0d4grEmIxNhJCtJRQOThD8ECgYEA0GKRO9SJdnhw1b6LPLd+o/AX7IEzQDHwdtfi +0h7hKHHGBlUMbIBwwjKmyKm6cSe0PYe96LqrVg+cVf84wbLZPAixhOjyplLznBzF +LqOrfFPfI5lQVhslE1H1CdLlk9eyT96jDgmLAg8EGSMV8aLGj++Gi2l/isujHlWF +BI4YpW8CgYEAsyKyhJzABmbYq5lGQmopZkxapCwJDiP1ypIzd+Z5TmKGytLlM8CK +3iocjEQzlm/jBfBGyWv5eD8UCDOoLEMCiqXcFn+uNJb79zvoN6ZBVGl6TzhTIhNb +73Y5/QQguZtnKrtoRSxLwcJnFE41D0zBRYOjy6gZJ6PSpPHeuiid2QECgYACuZc+ +mgvmIbMQCHrXo2qjiCs364SZDU4gr7gGmWLGXZ6CTLBp5tASqgjmTNnkSumfeFvy +ZCaDbJbVxQ2f8s/GajKwEz/BDwqievnVH0zJxmr/kyyqw5Ybh5HVvA1GfqaVRssJ +DvTjZQDft0a9Lyy7ix1OS2XgkcMjTWj840LNPwKBgDPXMBgL5h41jd7jCsXzPhyr +V96RzQkPcKsoVvrCoNi8eoEYgRd9jwfiU12rlXv+fgVXrrfMoJBoYT6YtrxEJVdM +RAjRpnE8PMqCUA8Rd7RFK9Vp5Uo8RxTNvk9yPvDv1+lHHV7lEltIk5PXuKPHIrc1 +nNUyhzvJs2Qba2L/huNC +-----END PRIVATE KEY-----""" + yield key + + +@pytest.fixture() +def public_key() -> str: + """Yields a public key in PEM format""" + key = """-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqfuABGTOLmGSrv4ays8k +jExRcd/53FqOlwBaQA0B1yzsEQfV4dfzsIoB4qrP8B0QAJp65ZMAgkAPK8YhwQDI +6eQKW5MoIEM/w1Yo66Um2uk4stXMgU2Qt3LzithaSc7GpCS4SFw9R2lCzgpciCQf +2K5h5kHPEi91M6LuZjKXXAQtqc8JhFtXL0FEaZ74KwxfFfOhq0JIyupyDyBHrlup +Gw+ZVhQD3y39aIQ1nsd8sXkhclzUQKSYWxeB6yhXpj5Dou2nLJMt/FKJkI2YADBz +6LSTZELwB3J1x4YNBXFhnaIb+W3PwrbfRsGTsyccMiSwQbEnsGxN9dE2Yt6C1NU5 +rwIDAQAB +-----END PUBLIC KEY-----""" + yield key + + +@pytest.fixture(scope="module") +def nav_name() -> str: + yield "nav" From 17759b38c93a2ee7a764c2370bfec57a1c2fec62 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 11:08:50 +0100 Subject: [PATCH 23/27] Shorten docstring --- python/nav/models/api.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index c82592fa80..e9ba485fee 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -109,15 +109,17 @@ def expire(self): @classmethod def generate_access_token(cls, token_data: Dict[str, Any] = {}) -> str: - """Generates and returns an access token in JWT format. Will use `token_data` as a basis - for the new token, but certain claims will be overridden + """Generates and returns an access token in JWT format. + Will use `token_data` as a basis for the new token, + but certain claims will be overridden. """ return cls._generate_token(token_data, cls.ACCESS_EXPIRE_DELTA, "access_token") @classmethod def generate_refresh_token(cls, token_data: Dict[str, Any] = {}) -> str: - """Generates and returns a refresh token in JWT format. Will use `token_data` as a basis - for the new token, but certain claims will be overridden + """Generates and returns a refresh token in JWT format. + Will use `token_data` as a basis for the new token, + but certain claims will be overridden. """ return cls._generate_token( token_data, cls.REFRESH_EXPIRE_DELTA, "refresh_token" From 5fe8ee29a9dd2253069cef880b7b5a6cc7fba97f Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 13 Nov 2023 11:22:32 +0100 Subject: [PATCH 24/27] Use term "data" instead of "body" "data" and "body" were both used for the same thing, so changed to only use one of the terms for clarity --- python/nav/models/api.py | 14 +++--- .../unittests/models/jwtrefreshtoken_test.py | 50 +++++++++---------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/python/nav/models/api.py b/python/nav/models/api.py index e9ba485fee..0e15a3d91c 100644 --- a/python/nav/models/api.py +++ b/python/nav/models/api.py @@ -85,8 +85,8 @@ class JWTRefreshToken(models.Model): def __str__(self): return self.token - def get_body(self) -> Dict[str, Any]: - """Body of token as a dict""" + def data(self) -> Dict[str, Any]: + """Data of token as a dict""" return self._decode_token(self.token) def is_active(self) -> bool: @@ -94,15 +94,15 @@ def is_active(self) -> bool: the nbf claim is in the past and the exp claim is in the future """ now = datetime.now() - body = self.get_body() - nbf = datetime.fromtimestamp(body['nbf']) - exp = datetime.fromtimestamp(body['exp']) + data = self.data() + nbf = datetime.fromtimestamp(data['nbf']) + exp = datetime.fromtimestamp(data['exp']) return now >= nbf and now < exp def expire(self): """Expires the token""" # Base claims for expired token on existing claims - expired_data = self.get_body() + expired_data = self.data() expired_data['exp'] = (datetime.now() - timedelta(hours=1)).timestamp() self.token = self._encode_token(expired_data) self.save() @@ -155,7 +155,7 @@ def _encode_token(cls, token_data: Dict[str, Any]) -> str: @classmethod def _decode_token(cls, token: str) -> Dict[str, Any]: - """Decodes a token in JWT format and returns the body of the decoded token""" + """Decodes a token in JWT format and returns the data of the decoded token""" return jwt.decode(token, options={'verify_signature': False}) class Meta(object): diff --git a/tests/unittests/models/jwtrefreshtoken_test.py b/tests/unittests/models/jwtrefreshtoken_test.py index bc9a7e74bf..0a94085b13 100644 --- a/tests/unittests/models/jwtrefreshtoken_test.py +++ b/tests/unittests/models/jwtrefreshtoken_test.py @@ -9,45 +9,45 @@ class TestGenerateAccessToken: def test_nbf_should_be_in_the_past(self): encoded_token = JWTRefreshToken.generate_access_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['nbf'] < datetime.now().timestamp() + data = JWTRefreshToken._decode_token(encoded_token) + assert data['nbf'] < datetime.now().timestamp() def test_exp_should_be_in_the_future(self): encoded_token = JWTRefreshToken.generate_access_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['exp'] > datetime.now().timestamp() + data = JWTRefreshToken._decode_token(encoded_token) + assert data['exp'] > datetime.now().timestamp() def test_iat_should_be_in_the_past(self): encoded_token = JWTRefreshToken.generate_access_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['iat'] < datetime.now().timestamp() + data = JWTRefreshToken._decode_token(encoded_token) + assert data['iat'] < datetime.now().timestamp() def test_token_type_should_be_access_token(self): encoded_token = JWTRefreshToken.generate_access_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['token_type'] == "access_token" + data = JWTRefreshToken._decode_token(encoded_token) + assert data['token_type'] == "access_token" class TestGenerateRefreshToken: def test_nbf_should_be_in_the_past(self): encoded_token = JWTRefreshToken.generate_refresh_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['nbf'] < datetime.now().timestamp() + data = JWTRefreshToken._decode_token(encoded_token) + assert data['nbf'] < datetime.now().timestamp() def test_exp_should_be_in_the_future(self): encoded_token = JWTRefreshToken.generate_refresh_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['exp'] > datetime.now().timestamp() + data = JWTRefreshToken._decode_token(encoded_token) + assert data['exp'] > datetime.now().timestamp() def test_iat_should_be_in_the_past(self): encoded_token = JWTRefreshToken.generate_refresh_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['iat'] < datetime.now().timestamp() + data = JWTRefreshToken._decode_token(encoded_token) + assert data['iat'] < datetime.now().timestamp() def test_token_type_should_be_refresh_token(self): encoded_token = JWTRefreshToken.generate_refresh_token() - body = JWTRefreshToken._decode_token(encoded_token) - assert body['token_type'] == "refresh_token" + data = JWTRefreshToken._decode_token(encoded_token) + assert data['token_type'] == "refresh_token" class TestIsActive: @@ -90,16 +90,16 @@ def test_should_make_active_token_inactive(self, refresh_token_data): assert not token.is_active() -class TestGetBody: - def test_should_return_accurate_representation_of_token_body( +class TestData: + def test_should_return_accurate_representation_of_token_data( self, refresh_token, refresh_token_data ): token = JWTRefreshToken(token=refresh_token) - assert token.get_body() == refresh_token_data + assert token.data() == refresh_token_data class TestDecodeToken: - def test_should_return_same_body_as_when_token_was_encoded( + def test_should_return_same_data_as_when_token_was_encoded( self, refresh_token, refresh_token_data ): decoded_data = JWTRefreshToken._decode_token(refresh_token) @@ -107,7 +107,7 @@ def test_should_return_same_body_as_when_token_was_encoded( class TestEncodeToken: - def test_should_generate_a_known_token_using_the_same_body( + def test_should_generate_a_known_token_using_the_same_data( self, refresh_token, refresh_token_data ): encoded_token = JWTRefreshToken._encode_token(refresh_token_data) @@ -116,8 +116,8 @@ def test_should_generate_a_known_token_using_the_same_body( @pytest.fixture() def refresh_token_data() -> Dict[Any, str]: - """Yields the body of the token in the refresh_token fixture""" - body = { + """Yields the data of the token in the refresh_token fixture""" + data = { "exp": 1516339022, "nbf": 1516239022, "iat": 1516239022, @@ -125,12 +125,12 @@ def refresh_token_data() -> Dict[Any, str]: "iss": "nav", "token_type": "refresh_token", } - yield body + yield data @pytest.fixture() def refresh_token() -> str: - """Yields a refresh token with a body matching the refresh_token_data fixture""" + """Yields a refresh token with data matching the refresh_token_data fixture""" token = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE1\ MTYzMzkwMjIsIm5iZiI6MTUxNjIzOTAyMiwiaWF0IjoxNTE2MjM5MDIyLCJh\ dWQiOiJuYXYiLCJpc3MiOiJuYXYiLCJ0b2tlbl90eXBlIjoicmVmcmVzaF90\ From 0163627b886895142a9fec848e4676426fb40452 Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 22:18:03 +0100 Subject: [PATCH 25/27] Add refresh view --- python/nav/web/api/v1/views.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/python/nav/web/api/v1/views.py b/python/nav/web/api/v1/views.py index 9cf02faba5..c3afe5f868 100644 --- a/python/nav/web/api/v1/views.py +++ b/python/nav/web/api/v1/views.py @@ -45,6 +45,7 @@ from oidc_auth.authentication import JSONWebTokenAuthentication from nav.models import manage, event, cabling, rack, profiles +from nav.models.api import JWTRefreshToken from nav.models.fields import INFINITY, UNRESOLVED from nav.web.servicecheckers import load_checker_classes from nav.util import auth_token, is_valid_cidr @@ -1107,6 +1108,33 @@ class RackViewSet(NAVAPIMixin, viewsets.ReadOnlyModelViewSet): search_fields = ['rackname'] +class JWTRefreshViewSet(APIView): + """ + Accepts a valid refresh token. + Returns a new refresh token and an access token. + """ + + def post(self, request): + try: + db_token = JWTRefreshToken.objects.get( + token=request.data.get('refresh_token') + ) + except JWTRefreshToken.DoesNotExist: + return Response("Invalid token", status=status.HTTP_403_FORBIDDEN) + if not db_token.is_active(): + return Response("Inactive token", status=status.HTTP_403_FORBIDDEN) + token_data = db_token.data() + access_token = JWTRefreshToken.generate_access_token(token_data) + refresh_token = JWTRefreshToken.generate_refresh_token(token_data) + db_token.token = refresh_token + db_token.save() + response_data = { + 'access_token': access_token, + 'refresh_token': refresh_token, + } + return Response(response_data) + + def get_or_create_token(request): """Gets an existing token or creates a new one. If the old token has expired, create a new one. From 67d742d5dc24de95f2f026fc7e5df2894fb8c10e Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Mon, 6 Feb 2023 22:18:13 +0100 Subject: [PATCH 26/27] Add url for refresh view --- python/nav/web/api/v1/urls.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/nav/web/api/v1/urls.py b/python/nav/web/api/v1/urls.py index 04a1fe6e31..c48499c61b 100644 --- a/python/nav/web/api/v1/urls.py +++ b/python/nav/web/api/v1/urls.py @@ -73,4 +73,5 @@ name="prefix-usage-detail", ), re_path(r'^', include(router.urls)), + re_path(r'^refresh/$', views.JWTRefreshViewSet.as_view(), name='jwt-refresh'), ] From 35737f8bede9a902502491bee679d2a66655cc5d Mon Sep 17 00:00:00 2001 From: Simon Oliver Tveit Date: Wed, 22 Nov 2023 13:13:05 +0100 Subject: [PATCH 27/27] Add tests dupe fixtures from the model tests. need to find good way to share the fixtures between integration/unittests or maybe not mock here and have an actual testing config file instead --- .../integration/jwt_refresh_endpoint_test.py | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 tests/integration/jwt_refresh_endpoint_test.py diff --git a/tests/integration/jwt_refresh_endpoint_test.py b/tests/integration/jwt_refresh_endpoint_test.py new file mode 100644 index 0000000000..018e68609f --- /dev/null +++ b/tests/integration/jwt_refresh_endpoint_test.py @@ -0,0 +1,153 @@ +import pytest + +from unittest.mock import Mock, patch + +from django.urls import reverse +from rest_framework.reverse import reverse_lazy +from nav.models.api import JWTRefreshToken + + +def test_token_not_in_database_should_be_rejected(db, api_client, url): + token = JWTRefreshToken.generate_refresh_token() + assert not JWTRefreshToken.objects.filter(token=token).exists() + response = api_client.post( + url, + follow=True, + data={ + 'refresh_token': token, + }, + ) + assert response.status_code == 403 + + +def test_expired_token_should_be_rejected(db, api_client, url): + token = JWTRefreshToken.generate_refresh_token() + db_token = JWTRefreshToken(token=token) + db_token.save() + db_token.expire() + response = api_client.post( + url, + follow=True, + data={ + 'refresh_token': db_token.token, + }, + ) + assert response.status_code == 403 + + +def test_valid_token_should_be_accepted(db, api_client, url): + token = JWTRefreshToken.generate_refresh_token() + db_token = JWTRefreshToken(token=token) + db_token.save() + response = api_client.post( + url, + follow=True, + data={ + 'refresh_token': token, + }, + ) + assert response.status_code == 200 + + +def test_valid_token_should_be_replaced_by_new_token_in_db(db, api_client, url): + token = JWTRefreshToken.generate_refresh_token() + db_token = JWTRefreshToken(token=token) + db_token.save() + response = api_client.post( + url, + follow=True, + data={ + 'refresh_token': token, + }, + ) + assert response.status_code == 200 + assert not JWTRefreshToken.objects.filter(token=token).exists() + new_token = response.data.get("refresh_token") + assert JWTRefreshToken.objects.filter(token=new_token).exists() + + +def test_should_include_access_and_refresh_token_in_response(db, api_client, url): + token = JWTRefreshToken.generate_refresh_token() + db_token = JWTRefreshToken(token=token) + db_token.save() + response = api_client.post( + url, + follow=True, + data={ + 'refresh_token': token, + }, + ) + assert response.status_code == 200 + assert "access_token" in response.data + assert "refresh_token" in response.data + + +@pytest.fixture() +def url(): + return reverse('api:1:jwt-refresh') + + +@pytest.fixture(scope="module", autouse=True) +def jwtconf_mock(private_key, nav_name) -> str: + """Mocks the get_nave_name and get_nav_private_key functions for + the JWTConf class + """ + with patch("nav.models.api.JWTConf") as _jwtconf_mock: + instance = _jwtconf_mock.return_value + instance.get_nav_name = Mock(return_value=nav_name) + instance.get_nav_private_key = Mock(return_value=private_key) + yield _jwtconf_mock + + +@pytest.fixture(scope="module") +def private_key() -> str: + """Yields a private key in PEM format""" + key = """-----BEGIN PRIVATE KEY----- +MIIEuwIBADANBgkqhkiG9w0BAQEFAASCBKUwggShAgEAAoIBAQCp+4AEZM4uYZKu +/hrKzySMTFFx3/ncWo6XAFpADQHXLOwRB9Xh1/OwigHiqs/wHRAAmnrlkwCCQA8r +xiHBAMjp5ApbkyggQz/DVijrpSba6Tiy1cyBTZC3cvOK2FpJzsakJLhIXD1HaULO +ClyIJB/YrmHmQc8SL3Uzou5mMpdcBC2pzwmEW1cvQURpnvgrDF8V86GrQkjK6nIP +IEeuW6kbD5lWFAPfLf1ohDWex3yxeSFyXNRApJhbF4HrKFemPkOi7acsky38UomQ +jZgAMHPotJNkQvAHcnXHhg0FcWGdohv5bc/Ctt9GwZOzJxwyJLBBsSewbE310TZi +3oLU1TmvAgMBAAECgf8zrhi95+gdMeKRpwV+TnxOK5CXjqvo0vTcnr7Runf/c9On +WeUtRPr83E4LxuMcSGRqdTfoP0loUGb3EsYwZ+IDOnyWWvytfRoQdExSA2RM1PDo +GRiUN4Dy8CrGNqvnb3agG99Ay3Ura6q5T20n9ykM4qKL3yDrO9fmWyMgRJbAOAYm +xzf7H910mDZghXPpq8nzDky0JLNZcaqbxuPQ3+EI4p2dLNXbNqMPs8Y20JKLeOPs +HikRM0zfhHEJSt5IPFQ54/CzscGHGeCleQINWTgvDLMcE5fJMvbLLZixV+YsBfAq +e2JsSubS+9RI2ktMlSKaemr8yeoIpsXfAiJSHkECgYEA0NKU18xK+9w5IXfgNwI4 +peu2tWgwyZSp5R2pdLT7O1dJoLYRoAmcXNePB0VXNARqGxTNypJ9zmMawNmf3YRS +BqG8aKz7qpATlx9OwYlk09fsS6MeVmaur8bHGHP6O+gt7Xg+zhiFPvU9P5LB+C0Z +0d4grEmIxNhJCtJRQOThD8ECgYEA0GKRO9SJdnhw1b6LPLd+o/AX7IEzQDHwdtfi +0h7hKHHGBlUMbIBwwjKmyKm6cSe0PYe96LqrVg+cVf84wbLZPAixhOjyplLznBzF +LqOrfFPfI5lQVhslE1H1CdLlk9eyT96jDgmLAg8EGSMV8aLGj++Gi2l/isujHlWF +BI4YpW8CgYEAsyKyhJzABmbYq5lGQmopZkxapCwJDiP1ypIzd+Z5TmKGytLlM8CK +3iocjEQzlm/jBfBGyWv5eD8UCDOoLEMCiqXcFn+uNJb79zvoN6ZBVGl6TzhTIhNb +73Y5/QQguZtnKrtoRSxLwcJnFE41D0zBRYOjy6gZJ6PSpPHeuiid2QECgYACuZc+ +mgvmIbMQCHrXo2qjiCs364SZDU4gr7gGmWLGXZ6CTLBp5tASqgjmTNnkSumfeFvy +ZCaDbJbVxQ2f8s/GajKwEz/BDwqievnVH0zJxmr/kyyqw5Ybh5HVvA1GfqaVRssJ +DvTjZQDft0a9Lyy7ix1OS2XgkcMjTWj840LNPwKBgDPXMBgL5h41jd7jCsXzPhyr +V96RzQkPcKsoVvrCoNi8eoEYgRd9jwfiU12rlXv+fgVXrrfMoJBoYT6YtrxEJVdM +RAjRpnE8PMqCUA8Rd7RFK9Vp5Uo8RxTNvk9yPvDv1+lHHV7lEltIk5PXuKPHIrc1 +nNUyhzvJs2Qba2L/huNC +-----END PRIVATE KEY-----""" + yield key + + +@pytest.fixture() +def public_key() -> str: + """Yields a public key in PEM format""" + key = """-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqfuABGTOLmGSrv4ays8k +jExRcd/53FqOlwBaQA0B1yzsEQfV4dfzsIoB4qrP8B0QAJp65ZMAgkAPK8YhwQDI +6eQKW5MoIEM/w1Yo66Um2uk4stXMgU2Qt3LzithaSc7GpCS4SFw9R2lCzgpciCQf +2K5h5kHPEi91M6LuZjKXXAQtqc8JhFtXL0FEaZ74KwxfFfOhq0JIyupyDyBHrlup +Gw+ZVhQD3y39aIQ1nsd8sXkhclzUQKSYWxeB6yhXpj5Dou2nLJMt/FKJkI2YADBz +6LSTZELwB3J1x4YNBXFhnaIb+W3PwrbfRsGTsyccMiSwQbEnsGxN9dE2Yt6C1NU5 +rwIDAQAB +-----END PUBLIC KEY-----""" + yield key + + +@pytest.fixture(scope="module") +def nav_name() -> str: + yield "nav"