From f01f8f99402a3113902db8cc073a03e9aa92f3ce Mon Sep 17 00:00:00 2001 From: Pavel Tvrdik Date: Fri, 11 Dec 2020 14:07:43 +0100 Subject: [PATCH 1/2] Disable 255 chars length limit for redirect uri (#902) RFC 7230 recommends to design system to be capable to work with URI at least to 8000 chars long. This commit allows handle redirect_uri that is over 255 chars. Fixes issue #902. --- .../migrations/0003_auto_20201211_1314.py | 18 ++++++ oauth2_provider/models.py | 2 +- tests/test_models.py | 55 +++++++++++++++---- 3 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 oauth2_provider/migrations/0003_auto_20201211_1314.py diff --git a/oauth2_provider/migrations/0003_auto_20201211_1314.py b/oauth2_provider/migrations/0003_auto_20201211_1314.py new file mode 100644 index 000000000..2787d51a3 --- /dev/null +++ b/oauth2_provider/migrations/0003_auto_20201211_1314.py @@ -0,0 +1,18 @@ +# Generated by Django 3.1.4 on 2020-12-11 13:14 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('oauth2_provider', '0002_auto_20190406_1805'), + ] + + operations = [ + migrations.AlterField( + model_name='grant', + name='redirect_uri', + field=models.TextField(), + ), + ] diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index 77542d35f..112fc00d8 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -220,7 +220,7 @@ class AbstractGrant(models.Model): code = models.CharField(max_length=255, unique=True) # code comes from oauthlib application = models.ForeignKey(oauth2_settings.APPLICATION_MODEL, on_delete=models.CASCADE) expires = models.DateTimeField() - redirect_uri = models.CharField(max_length=255) + redirect_uri = models.TextField() scope = models.TextField(blank=True) created = models.DateTimeField(auto_now_add=True) diff --git a/tests/test_models.py b/tests/test_models.py index c8e06a308..afcd6b419 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -22,10 +22,15 @@ UserModel = get_user_model() -class TestModels(TestCase): +class BaseTestModels(TestCase): def setUp(self): self.user = UserModel.objects.create_user("test_user", "test@example.com", "123456") + def tearDown(self): + self.user.delete() + + +class TestModels(BaseTestModels): def test_allow_scopes(self): self.client.login(username="test_user", password="123456") app = Application.objects.create( @@ -103,10 +108,7 @@ def test_scopes_property(self): OAUTH2_PROVIDER_REFRESH_TOKEN_MODEL="tests.SampleRefreshToken", OAUTH2_PROVIDER_GRANT_MODEL="tests.SampleGrant", ) -class TestCustomModels(TestCase): - def setUp(self): - self.user = UserModel.objects.create_user("test_user", "test@example.com", "123456") - +class TestCustomModels(BaseTestModels): def test_custom_application_model(self): """ If a custom application model is installed, it should be present in @@ -237,7 +239,21 @@ def test_custom_grant_model_not_installed(self): oauth2_settings.GRANT_MODEL = "oauth2_provider.Grant" -class TestGrantModel(TestCase): +class TestGrantModel(BaseTestModels): + def setUp(self): + super().setUp() + self.application = Application.objects.create( + name="Test Application", + redirect_uris="", + user=self.user, + client_type=Application.CLIENT_CONFIDENTIAL, + authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, + ) + + def tearDown(self): + self.application.delete() + super().tearDown() + def test_str(self): grant = Grant(code="test_code") self.assertEqual("%s" % grant, grant.code) @@ -247,11 +263,26 @@ def test_expires_can_be_none(self): self.assertIsNone(grant.expires) self.assertTrue(grant.is_expired()) + def test_redirect_uri_can_be_longer_than_255_chars(self): + long_redirect_uri = "http://example.com/{}".format("authorized/" * 25) + self.assertTrue(len(long_redirect_uri) > 255) + grant = Grant.objects.create( + user=self.user, + code="test_code", + application=self.application, + expires=timezone.now(), + redirect_uri=long_redirect_uri, + scope="", + ) + grant.refresh_from_db() + + # It would be necessary to run test using another DB engine than sqlite + # that transform varchar(255) into text data type. + # https://sqlite.org/datatype3.html#affinity_name_examples + self.assertEqual(grant.redirect_uri, long_redirect_uri) -class TestAccessTokenModel(TestCase): - def setUp(self): - self.user = UserModel.objects.create_user("test_user", "test@example.com", "123456") +class TestAccessTokenModel(BaseTestModels): def test_str(self): access_token = AccessToken(token="test_token") self.assertEqual("%s" % access_token, access_token.token) @@ -273,15 +304,15 @@ def test_expires_can_be_none(self): self.assertTrue(access_token.is_expired()) -class TestRefreshTokenModel(TestCase): +class TestRefreshTokenModel(BaseTestModels): def test_str(self): refresh_token = RefreshToken(token="test_token") self.assertEqual("%s" % refresh_token, refresh_token.token) -class TestClearExpired(TestCase): +class TestClearExpired(BaseTestModels): def setUp(self): - self.user = UserModel.objects.create_user("test_user", "test@example.com", "123456") + super().setUp() # Insert two tokens on database. app = Application.objects.create( name="test_app", From 3af6e03d906b9a65935ead987ce34edea34a2e26 Mon Sep 17 00:00:00 2001 From: Pavel Tvrdik Date: Fri, 18 Dec 2020 10:09:42 +0100 Subject: [PATCH 2/2] fixup! Disable 255 chars length limit for redirect uri (#902) --- AUTHORS | 1 + CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index 7e03b37ed..a1789bf2a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -26,6 +26,7 @@ Jens Timmerman Jerome Leclanche Jim Graham Paul Oswald +Pavel Tvrdík pySilver Rodney Richardson Silvano Cerza diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cb02280a..353776c5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] * #898 Added the ability to customize classes for django admin +* #903 Disable `redirect_uri` field length limit for `AbstractGrant` ### Added * #884 Added support for Python 3.9