diff --git a/AUTHORS b/AUTHORS index 92f65ed6e..295bbe6e1 100644 --- a/AUTHORS +++ b/AUTHORS @@ -64,6 +64,5 @@ Jadiel Teófilo pySilver Łukasz Skarżyński Shaheed Haque -Peter Karman Vinay Karanam Eduardo Oliveira diff --git a/CHANGELOG.md b/CHANGELOG.md index e733c3d00..75aa14ef2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * #651 Batch expired token deletions in `cleartokens` management command * Added pt-BR translations. -* #729 Add support for [hashed client_secret values](https://django-oauth-toolkit.readthedocs.io/en/latest/settings.html#client-secret-hasher). ### Fixed * #1012 Return status for introspecting a nonexistent token from 401 to the correct value of 200 per [RFC 7662](https://datatracker.ietf.org/doc/html/rfc7662#section-2.2). diff --git a/docs/settings.rst b/docs/settings.rst index e837d7217..49460bc0e 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -88,13 +88,6 @@ CLIENT_SECRET_GENERATOR_LENGTH The length of the generated secrets, in characters. If this value is too low, secrets may become subject to bruteforce guessing. -CLIENT_SECRET_HASHER -~~~~~~~~~~~~~~~~~~~~ -If set to one of the Django password hasher algorithm names, client_secret values will be -stored as `hashed Django passwords `_. -See the official list in the django.contrib.auth.hashers namespace. -Default is none (stored as plain text). - EXTRA_SERVER_KWARGS ~~~~~~~~~~~~~~~~~~~ A dictionary to be passed to oauthlib's Server class. Three options @@ -104,19 +97,19 @@ of those three can be a callable) must be passed here directly and classes must be instantiated (callables should accept request as their only argument). GRANT_MODEL -~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~ The import string of the class (model) representing your grants. Overwrite this value if you wrote your own implementation (subclass of ``oauth2_provider.models.Grant``). APPLICATION_ADMIN_CLASS -~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~ The import string of the class (model) representing your application admin class. Overwrite this value if you wrote your own implementation (subclass of ``oauth2_provider.admin.ApplicationAdmin``). ACCESS_TOKEN_ADMIN_CLASS -~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~ The import string of the class (model) representing your access token admin class. Overwrite this value if you wrote your own implementation (subclass of ``oauth2_provider.admin.AccessTokenAdmin``). @@ -128,7 +121,7 @@ Overwrite this value if you wrote your own implementation (subclass of ``oauth2_provider.admin.GrantAdmin``). REFRESH_TOKEN_ADMIN_CLASS -~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~ The import string of the class (model) representing your refresh token admin class. Overwrite this value if you wrote your own implementation (subclass of ``oauth2_provider.admin.RefreshTokenAdmin``). @@ -161,7 +154,7 @@ If you don't change the validator code and don't run cleartokens all refresh tokens will last until revoked or the end of time. You should change this. REFRESH_TOKEN_GRACE_PERIOD_SECONDS -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The number of seconds between when a refresh token is first used when it is expired. The most common case of this for this is native mobile applications that run into issues of network connectivity during the refresh cycle and are @@ -185,7 +178,7 @@ See also: validator's rotate_refresh_token method can be overridden to make this when close to expiration, theoretically). REFRESH_TOKEN_GENERATOR -~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~ See `ACCESS_TOKEN_GENERATOR`. This is the same but for refresh tokens. Defaults to access token generator if not provided. @@ -272,7 +265,7 @@ Default: ``""`` The RSA private key used to sign OIDC ID tokens. If not set, OIDC is disabled. OIDC_RSA_PRIVATE_KEYS_INACTIVE -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~ Default: ``[]`` An array of *inactive* RSA private keys. These keys are not used to sign tokens, @@ -283,7 +276,7 @@ This is useful for providing a smooth transition during key rotation. should be retained in this inactive list. OIDC_JWKS_MAX_AGE_SECONDS -~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~ Default: ``3600`` The max-age value for the Cache-Control header on jwks_uri. @@ -358,9 +351,9 @@ Time of sleep in seconds used by ``cleartokens`` management command between batc Settings imported from Django project -------------------------------------- +-------------------------- USE_TZ -~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~ Used to determine whether or not to make token expire dates timezone aware. diff --git a/oauth2_provider/migrations/0006_alter_application_client_secret.py b/oauth2_provider/migrations/0006_alter_application_client_secret.py deleted file mode 100644 index ae6f3d843..000000000 --- a/oauth2_provider/migrations/0006_alter_application_client_secret.py +++ /dev/null @@ -1,20 +0,0 @@ -# Generated by Django 4.0.1 on 2022-01-07 22:40 - -from django.db import migrations -import oauth2_provider.generators -import oauth2_provider.models - - -class Migration(migrations.Migration): - - dependencies = [ - ('oauth2_provider', '0005_auto_20211222_2352'), - ] - - operations = [ - migrations.AlterField( - model_name='application', - name='client_secret', - field=oauth2_provider.models.ClientSecretField(blank=True, db_index=True, default=oauth2_provider.generators.generate_client_secret, max_length=255), - ), - ] diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index 8ca031062..2c9747ce8 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -6,7 +6,6 @@ from django.apps import apps from django.conf import settings -from django.contrib.auth.hashers import make_password from django.core.exceptions import ImproperlyConfigured from django.db import models, transaction from django.urls import reverse @@ -25,19 +24,6 @@ logger = logging.getLogger(__name__) -class ClientSecretField(models.CharField): - def pre_save(self, model_instance, add): - if oauth2_settings.CLIENT_SECRET_HASHER: - plain_secret = getattr(model_instance, self.attname) - if "$" not in plain_secret: # not yet hashed - hashed_secret = make_password( - plain_secret, salt=model_instance.client_id, hasher=oauth2_settings.CLIENT_SECRET_HASHER - ) - setattr(model_instance, self.attname, hashed_secret) - return hashed_secret - return super().pre_save(model_instance, add) - - class AbstractApplication(models.Model): """ An Application instance represents a Client on the Authorization server. @@ -104,7 +90,7 @@ class AbstractApplication(models.Model): ) client_type = models.CharField(max_length=32, choices=CLIENT_TYPES) authorization_grant_type = models.CharField(max_length=32, choices=GRANT_TYPES) - client_secret = ClientSecretField( + client_secret = models.CharField( max_length=255, blank=True, default=generate_client_secret, db_index=True ) name = models.CharField(max_length=255, blank=True) diff --git a/oauth2_provider/oauth2_validators.py b/oauth2_provider/oauth2_validators.py index 06ef64f09..f3a24e258 100644 --- a/oauth2_provider/oauth2_validators.py +++ b/oauth2_provider/oauth2_validators.py @@ -11,7 +11,6 @@ import requests from django.conf import settings from django.contrib.auth import authenticate, get_user_model -from django.contrib.auth.hashers import check_password from django.core.exceptions import ObjectDoesNotExist from django.db import transaction from django.db.models import Q @@ -123,17 +122,6 @@ def _authenticate_basic_auth(self, request): elif request.client.client_id != client_id: log.debug("Failed basic auth: wrong client id %s" % client_id) return False - # we use the "$" as a sentinel character to determine - # whether a secret has been hashed like a Django password or not. - # We can do this because the default oauthlib.common.UNICODE_ASCII_CHARACTER_SET - # used by our default generator does not include the "$" character. - # However, if a different character set was used to generate the secret, this sentinel - # might be a false positive. - elif "$" in request.client.client_secret and request.client.client_secret != client_secret: - if not check_password(client_secret, request.client.client_secret): - log.debug("Failed basic auth: wrong hashed client secret %s" % client_secret) - return False - return True elif request.client.client_secret != client_secret: log.debug("Failed basic auth: wrong client secret %s" % client_secret) return False diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index fd02b0685..22e067716 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -37,7 +37,6 @@ "CLIENT_ID_GENERATOR_CLASS": "oauth2_provider.generators.ClientIdGenerator", "CLIENT_SECRET_GENERATOR_CLASS": "oauth2_provider.generators.ClientSecretGenerator", "CLIENT_SECRET_GENERATOR_LENGTH": 128, - "CLIENT_SECRET_HASHER": None, "ACCESS_TOKEN_GENERATOR": None, "REFRESH_TOKEN_GENERATOR": None, "EXTRA_SERVER_KWARGS": {}, diff --git a/tests/presets.py b/tests/presets.py index efef78c2a..438da1e03 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -44,6 +44,3 @@ "READ_SCOPE": "read", "WRITE_SCOPE": "write", } - -# default django auth hasher as of version 3.2 -CLIENT_SECRET_HASHER = {"CLIENT_SECRET_HASHER": "pbkdf2_sha256"} diff --git a/tests/test_client_credential.py b/tests/test_client_credential.py index 936cfb6ae..8159d55db 100644 --- a/tests/test_client_credential.py +++ b/tests/test_client_credential.py @@ -24,8 +24,6 @@ AccessToken = get_access_token_model() UserModel = get_user_model() -CLIENT_SECRET = "abcdefghijklmnopqrstuvwxyz1234567890" - # mocking a protected resource view class ResourceView(ProtectedResourceView): @@ -46,7 +44,6 @@ def setUp(self): user=self.dev_user, client_type=Application.CLIENT_PUBLIC, authorization_grant_type=Application.GRANT_CLIENT_CREDENTIALS, - client_secret=CLIENT_SECRET, ) def tearDown(self): @@ -82,29 +79,6 @@ def test_client_credential_access_allowed(self): response = view(request) self.assertEqual(response, "This is a protected resource") - @pytest.mark.oauth2_settings(presets.CLIENT_SECRET_HASHER) - def test_client_credential_with_hashed_client_secret(self): - """ - Verify client_secret is hashed before writing to the db, - and comparison on request uses same hashing algo. - """ - self.assertNotEqual(self.application.client_secret, CLIENT_SECRET) - self.assertIn("$", self.application.client_secret) - self.assertIn(presets.CLIENT_SECRET_HASHER["CLIENT_SECRET_HASHER"], self.application.client_secret) - - token_request_data = { - "grant_type": "client_credentials", - } - auth_headers = get_basic_auth_header(self.application.client_id, CLIENT_SECRET) - - response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers) - self.assertEqual(response.status_code, 200) - - # secret mismatch should return a 401 - auth_headers = get_basic_auth_header(self.application.client_id, "not-the-secret") - response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers) - self.assertEqual(response.status_code, 401) - def test_client_credential_does_not_issue_refresh_token(self): token_request_data = { "grant_type": "client_credentials",