feat: Update PKCE_REQUIRED to true by default (#1129)

* feat: default PKCE_REQUIRED  to True

BREAKING CHANGE: set to False to maintain legacy behavior


Co-authored-by: Alan Crosswell <alan@columbia.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

AUTHORS

@@ -73,3 +73,4 @@ Eduardo Oliveira
 Andrea Greco
 Dominik George
 David Hill
+Darrel O'Pry

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

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 <https://oauth.net/2/pkce/>`_ is required.
+
+According to `OAuth 2.0 Security Best Current Practice <https://oauth.net/2/oauth-best-practice/>`_ related to the
+`Authorization Code Grant <https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics#section-2.1.>`_
+
+- Public clients MUST use PKCE `RFC7636 <https://datatracker.ietf.org/doc/html/rfc7636>`_
+- For confidential clients, the use of PKCE `RFC7636 <https://datatracker.ietf.org/doc/html/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

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,

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"]

tests/settings.py

@@ -159,3 +159,4 @@
 
 CLEAR_EXPIRED_TOKENS_BATCH_SIZE = 1
 CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL = 0
+PKCE_REQUIRED = False

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",

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(

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

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