Skip to content

Use secrets.choice() instead of random.sample() for PKCE, nonce, and state generation#889

Open
Copilot wants to merge 2 commits intodevfrom
copilot/improve-random-generator
Open

Use secrets.choice() instead of random.sample() for PKCE, nonce, and state generation#889
Copilot wants to merge 2 commits intodevfrom
copilot/improve-random-generator

Conversation

Copy link

Copilot AI commented Mar 19, 2026

random.sample() uses a deterministic PRNG unsuitable for security-sensitive values. PKCE code verifiers, OAuth state parameters, and OIDC nonces require cryptographically strong randomness.

Changes

  • msal/oauth2cli/oauth2.py: Replace import random with import secrets; update PKCE code verifier and OAuth state generation
  • msal/oauth2cli/oidc.py: Replace import random with import secrets; update nonce generation
# Before
verifier = "".join(random.sample(string.ascii_letters + string.digits + "-._~", length))
"state": state or "".join(random.sample(string.ascii_letters, 16))
nonce = "".join(random.sample(string.ascii_letters, 16))

# After
verifier = "".join(secrets.choice(string.ascii_letters + string.digits + "-._~") for _ in range(length))
"state": state or "".join(secrets.choice(string.ascii_letters) for _ in range(16))
nonce = "".join(secrets.choice(string.ascii_letters) for _ in range(16))

Note: random.sample() also samples without replacement, artificially constraining the output space. secrets.choice() allows repeated characters, increasing entropy for the same output length.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • login.microsoftonline.com
    • Triggering command: /usr/bin/python python -m pytest tests/ -x -q --ignore=tests/test_e2e.py --ignore=tests/test_benchmark.py --ignore=tests/test_e2e_manual.py (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[Feature] Improve Random generator</issue_title>
<issue_description>Improve the random generator - random.sample() function to secrets.choice() to provide more randomness

</issue_description>

<agent_instructions>Look at PKCE, nonce and state generation.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

…ate generation

Co-authored-by: bgavrilMS <12273384+bgavrilMS@users.noreply.github.com>
Copilot AI changed the title [WIP] Improve random generator using secrets.choice Use secrets.choice() instead of random.sample() for PKCE, nonce, and state generation Mar 19, 2026
Copilot AI requested a review from bgavrilMS March 19, 2026 22:17
@bgavrilMS bgavrilMS marked this pull request as ready for review March 20, 2026 12:59
@bgavrilMS bgavrilMS requested a review from a team as a code owner March 20, 2026 12:59
Copilot AI review requested due to automatic review settings March 20, 2026 12:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the security of OAuth/OIDC ephemeral value generation by switching from random.sample() (deterministic PRNG and no-replacement sampling) to secrets.choice() for PKCE code verifiers, OAuth state, and OIDC nonces.

Changes:

  • Update PKCE code verifier generation to use secrets.choice() per character (supports full RFC7636 length range without no-replacement constraints).
  • Update OAuth state generation to use secrets.choice().
  • Update OIDC nonce generation to use secrets.choice().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
msal/oauth2cli/oauth2.py Uses secrets.choice() for PKCE verifier and OAuth state generation, removing reliance on random.sample().
msal/oauth2cli/oidc.py Uses secrets.choice() for OIDC nonce generation, removing reliance on random.sample().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Improve Random generator

3 participants