Use secrets module for secure random token generation

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

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

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