diff --git a/AUTHORS b/AUTHORS index 962cc7d00..8ec6667e2 100644 --- a/AUTHORS +++ b/AUTHORS @@ -73,3 +73,4 @@ Eduardo Oliveira Andrea Greco Dominik George David Hill +Darrel O'Pry diff --git a/CHANGELOG.md b/CHANGELOG.md index 148d5e50e..baae70de8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 on using Celery to automate clearing expired tokens. ### Changed +* #1129 (**Breaking**) Changed default value of PKCE_REQUIRED to True. This is a **breaking change**. Clients without + PKCE enabled will fail to authenticate. This breaks with [section 5 of RFC7636](https://datatracker.ietf.org/doc/html/rfc7636) + in favor of the [OAuth2 Security Best Practices for Authorization Code Grants](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-2.1). + If you want to retain the pre-2.x behavior, set `PKCE_REQUIRED = False ` in your settings.py + * #1093 (**Breaking**) Changed to implement [hashed](https://docs.djangoproject.com/en/stable/topics/auth/passwords/) client_secret values. This is a **breaking change** that will migrate all your existing cleartext `application.client_secret` values to be hashed with Django's default password hashing algorithm diff --git a/docs/settings.rst b/docs/settings.rst index 0ba12df11..2ac31ccda 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -253,9 +253,21 @@ will be used. PKCE_REQUIRED ~~~~~~~~~~~~~ -Default: ``False`` +Default: ``True`` + +Can be either a bool or a callable that takes a client id and returns a bool. + +Whether or not `Proof Key for Code Exchange `_ is required. + +According to `OAuth 2.0 Security Best Current Practice `_ related to the +`Authorization Code Grant `_ + +- Public clients MUST use PKCE `RFC7636 `_ +- For confidential clients, the use of PKCE `RFC7636 `_ is RECOMMENDED. + + + -Whether or not PKCE is required. Can be either a bool or a callable that takes a client id and returns a bool. OIDC_RSA_PRIVATE_KEY diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 3b7dea3f8..00a4e631c 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -97,7 +97,7 @@ "RESOURCE_SERVER_INTROSPECTION_CREDENTIALS": None, "RESOURCE_SERVER_TOKEN_CACHING_SECONDS": 36000, # Whether or not PKCE is required - "PKCE_REQUIRED": False, + "PKCE_REQUIRED": True, # Whether to re-create OAuthlibCore on every request. # Should only be required in testing. "ALWAYS_RELOAD_OAUTHLIB_CORE": False, diff --git a/tests/presets.py b/tests/presets.py index fa2d7a34c..6411687a4 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -19,6 +19,7 @@ "openid": "OpenID connect", }, "DEFAULT_SCOPES": ["read", "write"], + "PKCE_REQUIRED": False, } OIDC_SETTINGS_RO = deepcopy(OIDC_SETTINGS_RW) OIDC_SETTINGS_RO["DEFAULT_SCOPES"] = ["read"] diff --git a/tests/settings.py b/tests/settings.py index d2fbe6a56..27dcfe9a3 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -159,3 +159,4 @@ CLEAR_EXPIRED_TOKENS_BATCH_SIZE = 1 CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL = 0 +PKCE_REQUIRED = False diff --git a/tests/test_authorization_code.py b/tests/test_authorization_code.py index 25447b9dd..8bface719 100644 --- a/tests/test_authorization_code.py +++ b/tests/test_authorization_code.py @@ -48,6 +48,7 @@ def setUp(self): self.dev_user = UserModel.objects.create_user("dev_user", "dev@example.com", "123456") self.oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES = ["http", "custom-scheme"] + self.oauth2_settings.PKCE_REQUIRED = False self.application = Application.objects.create( name="Test Application", @@ -73,6 +74,7 @@ class TestRegressionIssue315(BaseTest): """ def test_request_is_not_overwritten(self): + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") response = self.client.get( reverse("oauth2_provider:authorize"), @@ -94,6 +96,7 @@ def test_skip_authorization_completely(self): """ If application.skip_authorization = True, should skip the authorization page. """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") self.application.skip_authorization = True self.application.save() @@ -132,6 +135,7 @@ def test_pre_auth_valid_client(self): """ Test response for a valid client_id with response_type: code """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") query_data = { @@ -644,7 +648,6 @@ def get_pkce_auth(self, code_challenge, code_challenge_method): """ Helper method to retrieve a valid authorization code using pkce """ - self.oauth2_settings.PKCE_REQUIRED = True authcode_data = { "client_id": self.application.client_id, "state": "random_state_string", @@ -1115,7 +1118,6 @@ def test_public_pkce_S256_authorize_get(self): self.application.client_type = Application.CLIENT_PUBLIC self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("S256") - self.oauth2_settings.PKCE_REQUIRED = True query_data = { "client_id": self.application.client_id, @@ -1143,7 +1145,6 @@ def test_public_pkce_plain_authorize_get(self): self.application.client_type = Application.CLIENT_PUBLIC self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("plain") - self.oauth2_settings.PKCE_REQUIRED = True query_data = { "client_id": self.application.client_id, @@ -1171,7 +1172,6 @@ def test_public_pkce_S256(self): self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("S256") authorization_code = self.get_pkce_auth(code_challenge, "S256") - self.oauth2_settings.PKCE_REQUIRED = True token_request_data = { "grant_type": "authorization_code", @@ -1200,7 +1200,6 @@ def test_public_pkce_plain(self): self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("plain") authorization_code = self.get_pkce_auth(code_challenge, "plain") - self.oauth2_settings.PKCE_REQUIRED = True token_request_data = { "grant_type": "authorization_code", @@ -1228,7 +1227,6 @@ def test_public_pkce_invalid_algorithm(self): self.application.client_type = Application.CLIENT_PUBLIC self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("invalid") - self.oauth2_settings.PKCE_REQUIRED = True query_data = { "client_id": self.application.client_id, @@ -1250,13 +1248,13 @@ def test_public_pkce_missing_code_challenge(self): Request an access token using client_type: public and PKCE enabled but with the code_challenge missing """ + self.oauth2_settings.PKCE_REQUIRED = True self.client.login(username="test_user", password="123456") self.application.client_type = Application.CLIENT_PUBLIC self.application.skip_authorization = True self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("S256") - self.oauth2_settings.PKCE_REQUIRED = True query_data = { "client_id": self.application.client_id, @@ -1282,7 +1280,6 @@ def test_public_pkce_missing_code_challenge_method(self): self.application.client_type = Application.CLIENT_PUBLIC self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("S256") - self.oauth2_settings.PKCE_REQUIRED = True query_data = { "client_id": self.application.client_id, @@ -1308,7 +1305,6 @@ def test_public_pkce_S256_invalid_code_verifier(self): self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("S256") authorization_code = self.get_pkce_auth(code_challenge, "S256") - self.oauth2_settings.PKCE_REQUIRED = True token_request_data = { "grant_type": "authorization_code", @@ -1332,7 +1328,6 @@ def test_public_pkce_plain_invalid_code_verifier(self): self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("plain") authorization_code = self.get_pkce_auth(code_challenge, "plain") - self.oauth2_settings.PKCE_REQUIRED = True token_request_data = { "grant_type": "authorization_code", @@ -1356,7 +1351,6 @@ def test_public_pkce_S256_missing_code_verifier(self): self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("S256") authorization_code = self.get_pkce_auth(code_challenge, "S256") - self.oauth2_settings.PKCE_REQUIRED = True token_request_data = { "grant_type": "authorization_code", @@ -1379,7 +1373,6 @@ def test_public_pkce_plain_missing_code_verifier(self): self.application.save() code_verifier, code_challenge = self.generate_pkce_codes("plain") authorization_code = self.get_pkce_auth(code_challenge, "plain") - self.oauth2_settings.PKCE_REQUIRED = True token_request_data = { "grant_type": "authorization_code", diff --git a/tests/test_hybrid.py b/tests/test_hybrid.py index 3f4048698..2e85b05b1 100644 --- a/tests/test_hybrid.py +++ b/tests/test_hybrid.py @@ -52,7 +52,7 @@ def setUp(self): self.factory = RequestFactory() self.hy_test_user = UserModel.objects.create_user("hy_test_user", "test_hy@example.com", "123456") self.hy_dev_user = UserModel.objects.create_user("hy_dev_user", "dev_hy@example.com", "123456") - + self.oauth2_settings.PKCE_REQUIRED = False self.oauth2_settings.ALLOWED_REDIRECT_URI_SCHEMES = ["http", "custom-scheme"] self.application = Application( diff --git a/tests/test_scopes.py b/tests/test_scopes.py index 39601ed3b..548cc060c 100644 --- a/tests/test_scopes.py +++ b/tests/test_scopes.py @@ -83,6 +83,7 @@ def test_scopes_saved_in_grant(self): """ Test scopes are properly saved in grant """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code @@ -105,6 +106,7 @@ def test_scopes_save_in_access_token(self): """ Test scopes are properly saved in access token """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code @@ -141,6 +143,7 @@ def test_scopes_protection_valid(self): """ Test access to a scope protected resource with correct scopes provided """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code @@ -183,6 +186,7 @@ def test_scopes_protection_fail(self): """ Test access to a scope protected resource with wrong scopes provided """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code @@ -225,6 +229,7 @@ def test_multi_scope_fail(self): """ Test access to a multi-scope protected resource with wrong scopes provided """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code @@ -267,6 +272,7 @@ def test_multi_scope_valid(self): """ Test access to a multi-scope protected resource with correct scopes provided """ + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code @@ -308,6 +314,7 @@ def test_multi_scope_valid(self): class TestReadWriteScope(BaseTest): def get_access_token(self, scopes): + self.oauth2_settings.PKCE_REQUIRED = False self.client.login(username="test_user", password="123456") # retrieve a valid authorization code diff --git a/tests/test_settings.py b/tests/test_settings.py index 52bdafe03..f9f540339 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -167,3 +167,8 @@ def test_generating_iss_endpoint_type_error(oauth2_settings): with pytest.raises(TypeError) as exc: oauth2_settings.oidc_issuer(None) assert str(exc.value) == "request must be a django or oauthlib request: got None" + + +def test_pkce_required_is_default(): + settings = OAuth2ProviderSettings() + assert settings.PKCE_REQUIRED is True