Skip to content

refactor(auth): build PKCE verifier from a Uint8Array and drop deprecated substr - #2514

Open
PedroHenrique0713 wants to merge 1 commit into
supabase:masterfrom
PedroHenrique0713:fix/pkce-verifier-uint8array
Open

refactor(auth): build PKCE verifier from a Uint8Array and drop deprecated substr#2514
PedroHenrique0713 wants to merge 1 commit into
supabase:masterfrom
PedroHenrique0713:fix/pkce-verifier-uint8array

Conversation

@PedroHenrique0713

Copy link
Copy Markdown
Contributor

🔍 Description

What changed?

generatePKCEVerifier() built the PKCE code verifier from a Uint32Array(56) but kept only the low byte of each element (via dec2hex). This switches it to a Uint8Array(56) and replaces the deprecated String.prototype.substr with slice. The typed-array allocation was also moved inside the crypto branch so the no-WebCrypto fallback no longer allocates an unused buffer.

Why was this change needed?

  • Over-allocation of CSPRNG output: new Uint32Array(56) draws 224 bytes from crypto.getRandomValues, but dec2hex (('0' + dec.toString(16)).substr(-2)) keeps only the last hex pair — the low byte — discarding 3 of every 4 bytes. A Uint8Array(56) requests exactly the 56 bytes actually consumed.
  • Deprecated API: String.prototype.substr() is a legacy/deprecated feature; slice(-2) is the standard equivalent.

No behavior change. The output is byte-for-byte equivalent: 56 bytes → two hex chars each → the same 112-character, hex-only verifier (well within RFC 7636's 43–128 range). The low byte of a uniform Uint32 is itself uniform, so the entropy distribution is identical — this only stops requesting (and discarding) 4× the randomness.

🔄 Breaking changes

  • This PR contains no breaking changes

📋 Checklist

  • I have read the Contributing Guidelines
  • My PR title follows the conventional commit format: <type>(<scope>): <description>
  • I have run pnpm nx format to ensure consistent code formatting
  • I have added tests for new functionality (if applicable)
  • I have updated documentation (if applicable)

📝 Additional notes

Added a generatePKCEVerifier test block (length within the RFC 7636 range, unreserved charset, uniqueness). Verified locally: jest test/helpers.test.ts → 27 passed, tsc -p tsconfig.json → clean, prettier --check → clean.

@PedroHenrique0713
PedroHenrique0713 force-pushed the fix/pkce-verifier-uint8array branch from 4b94e20 to a237bd4 Compare August 4, 2026 23:08
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 438fb3bb-c982-4223-932e-b210da42c210

📥 Commits

Reviewing files that changed from the base of the PR and between 21e410f and a237bd4.

📒 Files selected for processing (2)
  • packages/core/auth-js/src/lib/helpers.ts
  • packages/core/auth-js/test/helpers.test.ts

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved PKCE verifier generation to align with RFC 7636 requirements.
    • Ensured verifiers consistently use the expected length and character set.
  • Tests

    • Added coverage for verifier length, valid characters, and uniqueness.

Walkthrough

The PKCE verifier generator now uses Uint8Array with 56 random bytes. It converts the bytes into a 112-character hexadecimal verifier and uses slice for truncation. Tests now cover RFC 7636 length bounds, unreserved characters, and uniqueness across calls.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant