Merge pull request #474 from akatsoulas/support-pkce

Support for PKCE

HISTORY.rst

@@ -3,6 +3,13 @@
 History
 -------
 
+
+pending
+=======
+
+* Added PKCE support in the authorization code flow.
+
+
 3.0.0 (2022-11-14)
 ==================
 * Gracefully handle ``www-authenticate`` header with missing ``error_description``.
@@ -65,7 +72,7 @@ Backwards-incompatible changes:
   ``SessionMiddleware`` for URLs matching the pattern
   Thanks `@jwhitlock <https://github.com/jwhitlock>`_
 * Move nonce outside of add_state_and_noce_to_session method.
-* Change log level to info for the add_state_and_nonce_to_session.
+* Change log level to info for the add_state_and_verifier_and_nonce_to_session.
 * Session save/load management
   Thanks `@Flor1an-dev <https://github.com/Flor1an-dev>`_
 * Allow multiple parallel login sessions

docs/settings.rst

@@ -270,4 +270,51 @@ of ``mozilla-django-oidc``.
 
    :default: False
 
-   Allow using GET method to logout user
+   Allow using GET method to logout user
+
+.. py:attribute:: OIDC_USE_PKCE
+
+   :default: ``True``
+
+   Controls whether the authentication backend uses PKCE (Proof Key For Code Exchange) during the authorization code flow.
+
+   .. seealso::
+
+      https://datatracker.ietf.org/doc/html/rfc7636
+
+.. py:attribute:: OIDC_PKCE_CODE_CHALLENGE_METHOD
+
+   :default: ``S256``
+
+   Sets the method used to generate the PKCE code challenge.
+
+   Supported methods are:
+
+   * **plain**:
+      ``code_challenge = code_verifier``
+
+   * **S256**:
+      ``code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))``
+
+   .. note::
+
+      This only has an effect if ``OIDC_USE_PKCE`` is ``True``.
+
+   .. seealso::
+
+      https://datatracker.ietf.org/doc/html/rfc7636#section-4.2
+
+.. py:attribute:: OIDC_PKCE_CODE_VERIFIER_SIZE
+
+   :default: ``64``
+
+   Sets the length of the random string used for the PKCE code verifier.  Must be between ``43`` and ``128`` inclusive.
+
+   .. note::
+
+      This only has an effect if ``OIDC_USE_PKCE`` is ``True``.
+
+   .. seealso::
+
+      https://datatracker.ietf.org/doc/html/rfc7636#section-4.1
+

mozilla_django_oidc/auth.py

@@ -2,19 +2,18 @@
 import hashlib
 import json
 import logging
-import requests
-from requests.auth import HTTPBasicAuth
 
+import requests
 from django.contrib.auth import get_user_model
 from django.contrib.auth.backends import ModelBackend
-from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured
+from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
 from django.urls import reverse
-from django.utils.encoding import force_bytes, smart_str, smart_bytes
+from django.utils.encoding import force_bytes, smart_bytes, smart_str
 from django.utils.module_loading import import_string
-
 from josepy.b64 import b64decode
 from josepy.jwk import JWK
 from josepy.jws import JWS, Header
+from requests.auth import HTTPBasicAuth
 
 from mozilla_django_oidc.utils import absolutify, import_from_settings
 
@@ -263,6 +262,7 @@ def authenticate(self, request, **kwargs):
         state = self.request.GET.get("state")
         code = self.request.GET.get("code")
         nonce = kwargs.pop("nonce", None)
+        code_verifier = kwargs.pop("code_verifier", None)
 
         if not code or not state:
             return None
@@ -279,6 +279,10 @@ def authenticate(self, request, **kwargs):
             "redirect_uri": absolutify(self.request, reverse(reverse_url)),
         }
 
+        # Send code_verifier with token request if using PKCE
+        if code_verifier is not None:
+            token_payload.update({"code_verifier": code_verifier})
+
         # Get the token
         token_info = self.get_token(token_payload)
         id_token = token_info.get("id_token")

mozilla_django_oidc/middleware.py

@@ -14,7 +14,7 @@
 from mozilla_django_oidc.auth import OIDCAuthenticationBackend
 from mozilla_django_oidc.utils import (
     absolutify,
-    add_state_and_nonce_to_session,
+    add_state_and_verifier_and_nonce_to_session,
     import_from_settings,
 )
 
@@ -152,7 +152,7 @@ def process_request(self, request):
             nonce = get_random_string(self.OIDC_NONCE_SIZE)
             params.update({"nonce": nonce})
 
-        add_state_and_nonce_to_session(request, state, params)
+        add_state_and_verifier_and_nonce_to_session(request, state, params)
 
         request.session["oidc_login_next"] = request.get_full_path()
 

mozilla_django_oidc/utils.py

@@ -1,13 +1,14 @@
 import logging
 import time
 import warnings
+from hashlib import sha256
+from urllib.request import parse_http_list, parse_keqv_list
 
+# Make it obvious that these aren't the usual base64 functions
+import josepy.b64
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 
-from urllib.request import parse_http_list, parse_keqv_list
-
-
 LOGGER = logging.getLogger(__name__)
 
 
@@ -54,16 +55,75 @@ def is_authenticated(user):
     return user.is_authenticated
 
 
-def add_state_and_nonce_to_session(request, state, params):
+def base64_url_encode(bytes_like_obj):
+    """Return a URL-Safe, base64 encoded version of bytes_like_obj
+
+    Implements base64urlencode as described in
+    https://datatracker.ietf.org/doc/html/rfc7636#appendix-A
+    """
+
+    s = josepy.b64.b64encode(bytes_like_obj).decode("ascii")  # base64 encode
+    # the josepy base64 encoder (strips '='s padding) automatically
+    s = s.replace("+", "-")  # 62nd char of encoding
+    s = s.replace("/", "_")  # 63rd char of encoding
+
+    return s
+
+
+def base64_url_decode(string_like_obj):
+    """Return the bytes encoded in a URL-Safe, base64 encoded string
+    Implements inverse of base64urlencode as described in
+    https://datatracker.ietf.org/doc/html/rfc7636#appendix-A
+    This function is not used by the OpenID client; it's just for testing PKCE related functions.
+    """
+    s = string_like_obj
+
+    s = s.replace("_", "/")  # 63rd char of encoding
+    s = s.replace("-", "+")  # 62nd char of encoding
+    b = josepy.b64.b64decode(s)  # josepy base64 encoder (decodes without '='s padding)
+
+    return b
+
+
+def generate_code_challenge(code_verifier, method):
+    """Return a code_challege, which proves knowledge of the code_verifier.
+    The code challenge is generated according to method which must be one
+    of the methods defined in https://datatracker.ietf.org/doc/html/rfc7636#section-4.2:
+    - plain:
+      code_challenge = code_verifier
+    - S256:
+      code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))
     """
-    Stores the `state` and `nonce` parameters in a session dictionary including the time when it
-    was added. The dictionary can contain multiple state/nonce combinations to allow parallel
-    logins with multiple browser sessions.
-    To keep the session space to a reasonable size, the dictionary is kept at 50 state/nonce
-    combinations maximum.
+
+    if method == "plain":
+        return code_verifier
+
+    elif method == "S256":
+        return base64_url_encode(sha256(code_verifier.encode("ascii")).digest())
+
+    else:
+        raise ValueError("code challenge method must be 'plain' or 'S256'.")
+
+
+def add_state_and_verifier_and_nonce_to_session(
+    request, state, params, code_verifier=None
+):
+    """
+    Stores the `state` and `nonce` parameters and an optional `code_verifier` (for PKCE) in a
+    session dictionary which maps `state` -> {nonce, code_verifier}.  Each entry includes
+    the time when it was added. The dictionary can contain multiple state -> {nonce, code_verifier}
+    mappings to allow parallel logins with multiple browser sessions.
+    To keep the session space to a reasonable size, the dictionary is kept at 50
+    state -> {nonce, code_verifier} mappings maximum.
     """
     nonce = params.get("nonce")
 
+    # OPs supporting PKCE will require `code_verifier` to be sent to the token
+    # endpoint if `code_challenge` is sent to the authentication endpoint.
+    # Make sure that `code_challenge` and `code_verifier` are both specified
+    # or neither is.
+    assert ("code_challenge" in params) == (code_verifier is not None)
+
     # Store Nonce with the State parameter in the session "oidc_states" dictionary.
     # The dictionary can store multiple State/Nonce combinations to allow parallel
     # authentication flows which would otherwise overwrite State/Nonce values!
@@ -95,6 +155,7 @@ def add_state_and_nonce_to_session(request, state, params):
             del request.session["oidc_states"][oldest_state]
 
     request.session["oidc_states"][state] = {
+        "code_verifier": code_verifier,
         "nonce": nonce,
         "added_on": time.time(),
     }

mozilla_django_oidc/views.py

@@ -13,7 +13,8 @@
 
 from mozilla_django_oidc.utils import (
     absolutify,
-    add_state_and_nonce_to_session,
+    add_state_and_verifier_and_nonce_to_session,
+    generate_code_challenge,
     import_from_settings,
 )
 
@@ -92,15 +93,18 @@ def get(self, request):
             if "oidc_states" not in request.session:
                 return self.login_failure()
 
-            # State and Nonce are stored in the session "oidc_states" dictionary.
-            # State is the key, the value is a dictionary with the Nonce in the "nonce" field.
+            # State, Nonce and PKCE Code Verifier are stored in the session "oidc_states"
+            # dictionary.
+            # State is the key, the value is a dictionary with the Nonce in the "nonce" field, and
+            # Code Verifier or None in the "code_verifier" field.
             state = request.GET.get("state")
             if state not in request.session["oidc_states"]:
                 msg = "OIDC callback state not found in session `oidc_states`!"
                 raise SuspiciousOperation(msg)
 
-            # Get the nonce from the dictionary for further processing and delete the entry to
-            # prevent replay attacks.
+            # Get the nonce and optional code verifier from the dictionary for further processing
+            # and delete the entry to prevent replay attacks.
+            code_verifier = request.session["oidc_states"][state].get("code_verifier")
             nonce = request.session["oidc_states"][state]["nonce"]
             del request.session["oidc_states"][state]
 
@@ -115,6 +119,7 @@ def get(self, request):
             kwargs = {
                 "request": request,
                 "nonce": nonce,
+                "code_verifier": code_verifier,
             }
 
             self.user = auth.authenticate(**kwargs)
@@ -192,7 +197,36 @@ def get(self, request):
             nonce = get_random_string(self.get_settings("OIDC_NONCE_SIZE", 32))
             params.update({"nonce": nonce})
 
-        add_state_and_nonce_to_session(request, state, params)
+        if self.get_settings("OIDC_USE_PKCE", True):
+            code_verifier_length = self.get_settings("OIDC_PKCE_CODE_VERIFIER_SIZE", 64)
+            # Check that code_verifier_length is between the min and max length
+            # defined in https://datatracker.ietf.org/doc/html/rfc7636#section-4.1
+            if not (43 <= code_verifier_length <= 128):
+                raise ValueError("code_verifier_length must be between 43 and 128")
+
+            # Generate code_verifier and code_challenge pair
+            code_verifier = get_random_string(code_verifier_length)
+            code_challenge_method = self.get_settings(
+                "OIDC_PKCE_CODE_CHALLENGE_METHOD", "S256"
+            )
+            code_challenge = generate_code_challenge(
+                code_verifier, code_challenge_method
+            )
+
+            # Append code_challenge to authentication request parameters
+            params.update(
+                {
+                    "code_challenge": code_challenge,
+                    "code_challenge_method": code_challenge_method,
+                }
+            )
+
+        else:
+            code_verifier = None
+
+        add_state_and_verifier_and_nonce_to_session(
+            request, state, params, code_verifier
+        )
 
         request.session["oidc_login_next"] = get_next_url(request, redirect_field_name)