From dfd56bf84315c5907ed375ea4cdc15fc59f8f30c Mon Sep 17 00:00:00 2001 From: fazeelghafoor Date: Fri, 2 Aug 2024 07:22:37 +0500 Subject: [PATCH 1/6] change token field to TextField in AbstractAccessToken model - add TokenChecksumField field - update middleware, validators, and views to use token checksums for token retrieval and validation - modified test migrations to include token_checksum field in "sampleaccesstoken" model - add test for token checksum field --- AUTHORS | 1 + CHANGELOG.md | 4 +++ oauth2_provider/middleware.py | 4 ++- .../migrations/0011_add_token_checksum.py | 25 +++++++++++++++++++ oauth2_provider/models.py | 15 +++++++++-- oauth2_provider/oauth2_validators.py | 8 +++++- oauth2_provider/views/base.py | 4 ++- oauth2_provider/views/introspect.py | 6 ++++- tests/migrations/0002_swapped_models.py | 12 ++++++--- tests/test_models.py | 13 ++++++++++ 10 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 oauth2_provider/migrations/0011_add_token_checksum.py diff --git a/AUTHORS b/AUTHORS index 357abc2fa..22c671fbf 100644 --- a/AUTHORS +++ b/AUTHORS @@ -51,6 +51,7 @@ Dylan Tack Eduardo Oliveira Egor Poderiagin Emanuele Palazzetti +Fazeel Ghafoor Federico Dolce Florian Demmer Frederico Vieira diff --git a/CHANGELOG.md b/CHANGELOG.md index 362fd74b3..a3c913bdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] ### Added +* Add TokenChecksumField to compute and store SHA-256 checksums for tokens in AbstractAccessToken model. +* Add migration to include `token_checksum` field in AbstractAccessToken model. ### Changed +* Update token to TextField from CharField with 255 character limit in AbstractAccessToken model. +* Update middleware, validators, and views to use token checksums instead of token for token retrieval and validation. ### Deprecated ### Removed * #1425 Remove deprecated `RedirectURIValidator`, `WildcardSet` per #1345; `validate_logout_request` per #1274 diff --git a/oauth2_provider/middleware.py b/oauth2_provider/middleware.py index de1689894..65c9cf03c 100644 --- a/oauth2_provider/middleware.py +++ b/oauth2_provider/middleware.py @@ -1,3 +1,4 @@ +import hashlib import logging from django.contrib.auth import authenticate @@ -55,7 +56,8 @@ def __call__(self, request): tokenstring = authheader.split()[1] AccessToken = get_access_token_model() try: - token = AccessToken.objects.get(token=tokenstring) + token_checksum = hashlib.sha256(tokenstring.encode("utf-8")).hexdigest() + token = AccessToken.objects.get(token_checksum=token_checksum) request.access_token = token except AccessToken.DoesNotExist as e: log.exception(e) diff --git a/oauth2_provider/migrations/0011_add_token_checksum.py b/oauth2_provider/migrations/0011_add_token_checksum.py new file mode 100644 index 000000000..24d9d537c --- /dev/null +++ b/oauth2_provider/migrations/0011_add_token_checksum.py @@ -0,0 +1,25 @@ +# Generated by Django 5.0.7 on 2024-07-29 23:13 + +import oauth2_provider.models +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("oauth2_provider", "0010_application_allowed_origins"), + ] + + operations = [ + migrations.AddField( + model_name="accesstoken", + name="token_checksum", + field=oauth2_provider.models.TokenChecksumField( + blank=True, db_index=True, max_length=64, unique=True + ), + ), + migrations.AlterField( + model_name="accesstoken", + name="token", + field=models.TextField(), + ), + ] diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index 661bd7dfc..79ec56d25 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -1,3 +1,4 @@ +import hashlib import logging import time import uuid @@ -44,6 +45,14 @@ def pre_save(self, model_instance, add): return super().pre_save(model_instance, add) +class TokenChecksumField(models.CharField): + def pre_save(self, model_instance, add): + token = getattr(model_instance, "token") + checksum = hashlib.sha256(token.encode("utf-8")).hexdigest() + setattr(model_instance, self.attname, checksum) + return super().pre_save(model_instance, add) + + class AbstractApplication(models.Model): """ An Application instance represents a Client on the Authorization server. @@ -379,8 +388,10 @@ class AbstractAccessToken(models.Model): null=True, related_name="refreshed_access_token", ) - token = models.CharField( - max_length=255, + token = models.TextField() + token_checksum = TokenChecksumField( + max_length=64, + blank=True, unique=True, db_index=True, ) diff --git a/oauth2_provider/oauth2_validators.py b/oauth2_provider/oauth2_validators.py index 47d65e851..6f359af29 100644 --- a/oauth2_provider/oauth2_validators.py +++ b/oauth2_provider/oauth2_validators.py @@ -1,5 +1,6 @@ import base64 import binascii +import hashlib import http.client import inspect import json @@ -462,7 +463,12 @@ def validate_bearer_token(self, token, scopes, request): return False def _load_access_token(self, token): - return AccessToken.objects.select_related("application", "user").filter(token=token).first() + token_checksum = hashlib.sha256(token.encode("utf-8")).hexdigest() + return ( + AccessToken.objects.select_related("application", "user") + .filter(token_checksum=token_checksum) + .first() + ) def validate_code(self, client_id, code, client, request, *args, **kwargs): try: diff --git a/oauth2_provider/views/base.py b/oauth2_provider/views/base.py index cad36c757..52cb151d5 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -1,3 +1,4 @@ +import hashlib import json import logging from urllib.parse import parse_qsl, urlencode, urlparse @@ -289,7 +290,8 @@ def post(self, request, *args, **kwargs): if status == 200: access_token = json.loads(body).get("access_token") if access_token is not None: - token = get_access_token_model().objects.get(token=access_token) + token_checksum = hashlib.sha256(access_token.encode("utf-8")).hexdigest() + token = get_access_token_model().objects.get(token_checksum=token_checksum) app_authorized.send(sender=self, request=request, token=token) response = HttpResponse(content=body, status=status) diff --git a/oauth2_provider/views/introspect.py b/oauth2_provider/views/introspect.py index 04ca92a38..05a77909f 100644 --- a/oauth2_provider/views/introspect.py +++ b/oauth2_provider/views/introspect.py @@ -1,4 +1,5 @@ import calendar +import hashlib from django.core.exceptions import ObjectDoesNotExist from django.http import JsonResponse @@ -24,8 +25,11 @@ class IntrospectTokenView(ClientProtectedScopedResourceView): @staticmethod def get_token_response(token_value=None): try: + token_checksum = hashlib.sha256(token_value.encode("utf-8")).hexdigest() token = ( - get_access_token_model().objects.select_related("user", "application").get(token=token_value) + get_access_token_model() + .objects.select_related("user", "application") + .get(token_checksum=token_checksum) ) except ObjectDoesNotExist: return JsonResponse({"active": False}, status=200) diff --git a/tests/migrations/0002_swapped_models.py b/tests/migrations/0002_swapped_models.py index 412f19927..e168a053d 100644 --- a/tests/migrations/0002_swapped_models.py +++ b/tests/migrations/0002_swapped_models.py @@ -118,10 +118,14 @@ class Migration(migrations.Migration): field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='s_refreshed_access_token', to=settings.OAUTH2_PROVIDER_REFRESH_TOKEN_MODEL), ), migrations.AddField( - model_name='sampleaccesstoken', - name='token', - field=models.CharField(max_length=255, unique=True), - preserve_default=False, + model_name="sampleaccesstoken", + name="token", + field=models.TextField(), + ), + migrations.AddField( + model_name="sampleaccesstoken", + name="token_checksum", + field=models.CharField(max_length=64, unique=True, db_index=True), ), migrations.AddField( model_name='sampleaccesstoken', diff --git a/tests/test_models.py b/tests/test_models.py index 586bef124..24e4ceafe 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,3 +1,5 @@ +import hashlib +import secrets from datetime import timedelta import pytest @@ -310,6 +312,17 @@ def test_expires_can_be_none(self): self.assertIsNone(access_token.expires) self.assertTrue(access_token.is_expired()) + def test_token_checksum_field(self): + token = secrets.token_urlsafe(32) + access_token = AccessToken.objects.create( + user=self.user, + token=token, + expires=timezone.now() + timedelta(hours=1), + ) + expected_checksum = hashlib.sha256(token.encode()).hexdigest() + + self.assertEqual(access_token.token_checksum, expected_checksum) + class TestRefreshTokenModel(BaseTestModels): def test_str(self): From 94c0415962153566abf9ee0c470bed4c0fb01be8 Mon Sep 17 00:00:00 2001 From: fazeelghafoor <33656455+fazeelghafoor@users.noreply.github.com> Date: Tue, 6 Aug 2024 01:20:29 +0500 Subject: [PATCH 2/6] Update CHANGELOG.md Co-authored-by: Alan Crosswell --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3c913bdb..bb9e75b5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Add TokenChecksumField to compute and store SHA-256 checksums for tokens in AbstractAccessToken model. * Add migration to include `token_checksum` field in AbstractAccessToken model. ### Changed -* Update token to TextField from CharField with 255 character limit in AbstractAccessToken model. +* Update token to TextField from CharField with 255 character limit in AbstractAccessToken model. Removing the 255 character limit enables supporting JWT tokens with additional claims + * Update middleware, validators, and views to use token checksums instead of token for token retrieval and validation. ### Deprecated ### Removed From 68e7f9b34105ef654eac81d4953857c842530e6b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 5 Aug 2024 20:20:39 +0000 Subject: [PATCH 3/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb9e75b5f..51b490a2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Add migration to include `token_checksum` field in AbstractAccessToken model. ### Changed * Update token to TextField from CharField with 255 character limit in AbstractAccessToken model. Removing the 255 character limit enables supporting JWT tokens with additional claims - + * Update middleware, validators, and views to use token checksums instead of token for token retrieval and validation. ### Deprecated ### Removed From 4a4693b252ffc324067492e2f875854f95b7be90 Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Tue, 13 Aug 2024 10:33:08 -0400 Subject: [PATCH 4/6] resolve rebase conflict --- .../{0011_add_token_checksum.py => 0012_add_token_checksum.py} | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) rename oauth2_provider/migrations/{0011_add_token_checksum.py => 0012_add_token_checksum.py} (80%) diff --git a/oauth2_provider/migrations/0011_add_token_checksum.py b/oauth2_provider/migrations/0012_add_token_checksum.py similarity index 80% rename from oauth2_provider/migrations/0011_add_token_checksum.py rename to oauth2_provider/migrations/0012_add_token_checksum.py index 24d9d537c..8e9fd5142 100644 --- a/oauth2_provider/migrations/0011_add_token_checksum.py +++ b/oauth2_provider/migrations/0012_add_token_checksum.py @@ -6,7 +6,8 @@ class Migration(migrations.Migration): dependencies = [ - ("oauth2_provider", "0010_application_allowed_origins"), + ("oauth2_provider", "0011_refreshtoken_token_family"), + migrations.swappable_dependency(settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL), ] operations = [ From 7d152a457156b4ee5ef326b43032f9234cdb688b Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Tue, 13 Aug 2024 10:38:13 -0400 Subject: [PATCH 5/6] fix swappable dependency --- oauth2_provider/migrations/0012_add_token_checksum.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/oauth2_provider/migrations/0012_add_token_checksum.py b/oauth2_provider/migrations/0012_add_token_checksum.py index 8e9fd5142..0a3d5fe6c 100644 --- a/oauth2_provider/migrations/0012_add_token_checksum.py +++ b/oauth2_provider/migrations/0012_add_token_checksum.py @@ -2,12 +2,12 @@ import oauth2_provider.models from django.db import migrations, models - +from oauth2_provider.settings import oauth2_settings class Migration(migrations.Migration): dependencies = [ ("oauth2_provider", "0011_refreshtoken_token_family"), - migrations.swappable_dependency(settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL), + migrations.swappable_dependency(oauth2_settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL), ] operations = [ From aef2d2094abdb0ed23ba50ac742aa12c0bf0b83f Mon Sep 17 00:00:00 2001 From: Alan Crosswell Date: Tue, 13 Aug 2024 10:47:37 -0400 Subject: [PATCH 6/6] fix (again!) incorrect swappable dependency reference --- oauth2_provider/migrations/0012_add_token_checksum.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauth2_provider/migrations/0012_add_token_checksum.py b/oauth2_provider/migrations/0012_add_token_checksum.py index 0a3d5fe6c..7f62955e3 100644 --- a/oauth2_provider/migrations/0012_add_token_checksum.py +++ b/oauth2_provider/migrations/0012_add_token_checksum.py @@ -7,7 +7,7 @@ class Migration(migrations.Migration): dependencies = [ ("oauth2_provider", "0011_refreshtoken_token_family"), - migrations.swappable_dependency(oauth2_settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL), + migrations.swappable_dependency(oauth2_settings.ACCESS_TOKEN_MODEL), ] operations = [