From f1b98622a8a3879abbd2dcd9062ebd7c2d196dde Mon Sep 17 00:00:00 2001
From: Nilesh Choudhary <nichoudhary@microsoft.com>
Date: Tue, 7 Apr 2026 14:44:42 +0100
Subject: [PATCH 1/2] Use secrets module instead of random for
 security-sensitive token generation

Replace random.sample() with secrets.choice() for generating PKCE code
verifiers, OAuth2 state parameters, and OIDC nonces. The random module
uses Mersenne Twister which is not cryptographically secure. The secrets
module uses os.urandom(), providing a CSPRNG suitable for security tokens.

This also fixes a subtle entropy reduction caused by random.sample()
drawing without replacement, which prevented character repetition.
---
 msal/oauth2cli/oauth2.py | 7 ++++---
 msal/oauth2cli/oidc.py   | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/msal/oauth2cli/oauth2.py b/msal/oauth2cli/oauth2.py
index 680c1bc5..f6b4cdb4 100644
--- a/msal/oauth2cli/oauth2.py
+++ b/msal/oauth2cli/oauth2.py
@@ -13,7 +13,7 @@
 import base64
 import sys
 import functools
-import random
+import secrets
 import string
 import hashlib
 
@@ -277,8 +277,9 @@ def _scope_set(scope):
 
 def _generate_pkce_code_verifier(length=43):
     assert 43 <= length <= 128
+    alphabet = string.ascii_letters + string.digits + "-._~"
     verifier = "".join(  # https://tools.ietf.org/html/rfc7636#section-4.1
-        random.sample(string.ascii_letters + string.digits + "-._~", length))
+        secrets.choice(alphabet) for _ in range(length))
     code_challenge = (
         # https://tools.ietf.org/html/rfc7636#section-4.2
         base64.urlsafe_b64encode(hashlib.sha256(verifier.encode("ascii")).digest())
@@ -488,7 +489,7 @@ def initiate_auth_code_flow(
             raise ValueError('response_type="token ..." is not allowed')
         pkce = _generate_pkce_code_verifier()
         flow = {  # These data are required by obtain_token_by_auth_code_flow()
-            "state": state or "".join(random.sample(string.ascii_letters, 16)),
+            "state": state or "".join(secrets.choice(string.ascii_letters) for _ in range(16)),
             "redirect_uri": redirect_uri,
             "scope": scope,
             }
diff --git a/msal/oauth2cli/oidc.py b/msal/oauth2cli/oidc.py
index 01ee7894..1577c81a 100644
--- a/msal/oauth2cli/oidc.py
+++ b/msal/oauth2cli/oidc.py
@@ -1,7 +1,7 @@
 import json
 import base64
 import time
-import random
+import secrets
 import string
 import warnings
 import hashlib
@@ -238,7 +238,7 @@ def initiate_auth_code_flow(
             # Here we just automatically add it. If the caller do not want id_token,
             # they should simply go with oauth2.Client.
             _scope.append("openid")
-        nonce = "".join(random.sample(string.ascii_letters, 16))
+        nonce = "".join(secrets.choice(string.ascii_letters) for _ in range(16))
         flow = super(Client, self).initiate_auth_code_flow(
             scope=_scope, nonce=_nonce_hash(nonce), **kwargs)
         flow["nonce"] = nonce

From 62d6d7629cb2c7d4120b1eb64c9d7e85aef67eb4 Mon Sep 17 00:00:00 2001
From: Nilesh Choudhary <nichoudhary@microsoft.com>
Date: Tue, 7 Apr 2026 15:15:38 +0100
Subject: [PATCH 2/2] Create test_oauth2_security.py

---
 tests/test_oauth2_security.py | 49 +++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 tests/test_oauth2_security.py

diff --git a/tests/test_oauth2_security.py b/tests/test_oauth2_security.py
new file mode 100644
index 00000000..92749b15
--- /dev/null
+++ b/tests/test_oauth2_security.py
@@ -0,0 +1,49 @@
+import unittest
+from msal.oauth2cli import oauth2
+import string
+
+class TestTokenGenerationSecurity(unittest.TestCase):
+    """Verify secure token generation using secrets module."""
+    
+    def test_pkce_verifier_format(self):
+        """Verify PKCE verifier is valid and uses correct alphabet."""
+        pkce = oauth2._generate_pkce_code_verifier(length=43)
+        verifier = pkce["code_verifier"]
+        
+        # Check length
+        self.assertEqual(len(verifier), 43)
+        
+        # Check alphabet compliance (RFC 7636)
+        allowed_chars = set(string.ascii_letters + string.digits + "-._~")
+        self.assertTrue(all(c in allowed_chars for c in verifier))
+        
+        # Verify code_challenge exists and is base64url encoded (no padding)
+        self.assertIn("code_challenge", pkce)
+        self.assertNotIn(b"=", pkce["code_challenge"])
+    
+    def test_pkce_verifier_custom_length(self):
+        """Test PKCE verifier with valid custom lengths."""
+        for length in [43, 64, 128]:
+            pkce = oauth2._generate_pkce_code_verifier(length=length)
+            self.assertEqual(len(pkce["code_verifier"]), length)
+    
+    def test_token_uniqueness(self):
+        """Verify generated tokens are unique (randomness test)."""
+        # Generate multiple tokens - should all be different
+        verifiers = [oauth2._generate_pkce_code_verifier()["code_verifier"] 
+                     for _ in range(10)]
+        self.assertEqual(len(set(verifiers)), 10, "Generated tokens should be unique")
+        
+        # Also test state generation
+        from msal.oauth2cli.oauth2 import Client
+        from unittest.mock import Mock
+        
+        client = Client(
+            server_configuration={"authorization_endpoint": "https://example.com/auth"},
+            client_id="test-client",
+            http_client=Mock()
+        )
+        
+        states = [client.initiate_auth_code_flow(scope=["test"])["state"] 
+                  for _ in range(10)]
+        self.assertEqual(len(set(states)), 10, "Generated states should be unique")
\ No newline at end of file