Skip to content

feat: add DPoP sender-constrained token support (RFC 9449)#109

Open
davidbemer wants to merge 3 commits into
mainfrom
dpop
Open

feat: add DPoP sender-constrained token support (RFC 9449)#109
davidbemer wants to merge 3 commits into
mainfrom
dpop

Conversation

@davidbemer

Copy link
Copy Markdown

Summary

  • Adds src/SDK/Token/DPoP.php implementing RFC 9449 DPoP proof validation (signature verification for RSA, EC P-256/384/521, RSA-PSS, and EdDSA; JWK thumbprint check; htm/htu/iat/ath claim validation)
  • Adds DescopeSDK::validateDPoP() public method — a thin wrapper for server-side use after verify(); no-op when the session token has no cnf.jkt claim
  • Adds src/tests/DPoPTest.php with PHPUnit tests covering valid RSA and EC proofs, all structural error paths, and URL normalisation edge cases

Mirrors the Go SDK implementation: descope/go-sdk#737

🤖 Generated with Claude Code

Implements DPoP proof JWT validation so that DPoP-bound Descope session
tokens (those with a cnf.jkt claim) are enforced on every request.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 20, 2026 15:41
@shuni-bot-dev

shuni-bot-dev Bot commented May 20, 2026

Copy link
Copy Markdown

🐕 Review complete — View session on Shuni Portal 🐾

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds server-side validation for DPoP sender-constrained session tokens (RFC 9449) to the PHP SDK, plus docs and unit tests, so applications can enforce cnf.jkt-bound tokens after verify().

Changes:

  • Introduces Descope\SDK\Token\DPoP to validate DPoP proof JWT structure, signature, and core claims (htm/htu/iat/ath) and cnf.jkt thumbprint binding.
  • Exposes DescopeSDK::validateDPoP() as a public wrapper for application use.
  • Documents the new flow in the README and adds PHPUnit tests for validation behavior and URL normalization.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
src/SDK/Token/DPoP.php Implements DPoP proof validation, JWK import, signature verification, and claim checks.
src/SDK/DescopeSDK.php Adds public validateDPoP() wrapper calling the DPoP validator.
src/tests/DPoPTest.php Adds unit tests for DPoP validation and URL normalization cases.
README.md Documents how to use validateDPoP() after verifying the session token.

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

Comment thread src/SDK/Token/DPoP.php
$result = json_decode($decoded, true);
if (json_last_error() !== JSON_ERROR_NONE) {
throw new \Exception('Invalid JSON in JWT part: ' . json_last_error_msg());
}
Comment thread src/SDK/Token/DPoP.php Outdated

// Encode as ASN.1 INTEGER (prepend 0x00 if high bit set)
$r = ltrim($r, "\x00");
if (ord($r[0]) > 0x7f) {
Comment thread src/SDK/Token/DPoP.php
Comment on lines +104 to +108
// Step 14-15: Import JWK and verify signature
$publicKey = self::importJwkAsPublicKey($jwk, $alg);
$signingInput = $parts[0] . '.' . $parts[1];
$signature = self::base64urlDecode($parts[2]);
self::verifyDpopSignature($signingInput, $signature, $publicKey, $jwk, $alg);
Comment thread src/SDK/Token/DPoP.php
Comment on lines +113 to +116
// Steps 17-19: Validate required claims
if (empty($payload['jti'])) {
throw new \Exception('missing jti');
}
Comment thread src/SDK/Token/DPoP.php
Comment on lines +460 to +495
* Uses openssl_public_decrypt with PKCS1_PSS padding via raw OpenSSL.
* PHP 7.x doesn't expose RSA-PSS directly in openssl_verify, so we
* use openssl_public_decrypt with OPENSSL_PKCS1_OAEP_PADDING as a
* workaround is NOT correct. Instead, PHP 8.x added support via EVP.
* For broad compatibility we attempt openssl_verify with the PSS digest;
* if not available, throw an informative error.
*
* @throws \Exception on verification failure or unsupported environment.
*/
private static function verifyPss(string $signingInput, string $signature, $publicKey, string $alg): void
{
// Map alg to hash algorithm name
$hashMap = ['PS256' => 'sha256', 'PS384' => 'sha384', 'PS512' => 'sha512'];
$hashAlg = $hashMap[$alg];

// PHP 8.x: openssl_verify supports RSA-PSS via algorithm string
// We try using the OpenSSL algorithm identifier directly
$opensslAlgMap = [
'PS256' => defined('OPENSSL_ALGO_SHA256') ? 'SHA256' : null,
'PS384' => defined('OPENSSL_ALGO_SHA384') ? 'SHA384' : null,
'PS512' => defined('OPENSSL_ALGO_SHA512') ? 'SHA512' : null,
];

// Use openssl_public_decrypt with manual PSS verification
// Compute the digest of the signing input
$digest = hash($hashAlg, $signingInput, true);
$digestLen = strlen($digest);

// Decrypt the signature using the RSA public key (raw RSA operation)
$decrypted = '';
$decryptResult = openssl_public_decrypt($signature, $decrypted, $publicKey, OPENSSL_NO_PADDING);
if (!$decryptResult) {
throw new \Exception('RSA-PSS signature verification failed (decrypt step)');
}

// Verify PSS encoding manually
Comment thread src/SDK/Token/DPoP.php
Comment on lines +61 to +70
// Step 1-2: Check proof length
$dpopProof = trim($dpopProof);
if (strlen($dpopProof) > self::MAX_PROOF_LEN) {
throw new \Exception('DPoP proof exceeds maximum length');
}

// Step 3: Require proof when token is DPoP-bound
if ($dpopProof === '') {
throw new \Exception('DPoP proof required: access token is DPoP-bound (cnf.jkt present)');
}
Comment thread src/SDK/DescopeSDK.php
Comment on lines +230 to +241
/**
* Validate a DPoP proof for a DPoP-bound session token (RFC 9449).
*
* If the session token does not contain a cnf.jkt claim, this method
* returns immediately (the token is not DPoP-bound).
*
* @param string $sessionToken The session JWT (already verified).
* @param string $dpopProof The value of the DPoP HTTP header sent by the client.
* @param string $method The HTTP method of the incoming request (e.g. "GET", "POST").
* @param string $requestUrl The full URL of the incoming request.
* @throws \Exception If the DPoP proof is missing or invalid.
*/
Comment thread src/tests/DPoPTest.php
Comment on lines +8 to +14
/**
* Unit tests for DPoP (RFC 9449) proof validation.
*
* These tests exercise the static helper methods directly without
* requiring a network connection or live Descope credentials.
*/
final class DPoPTest extends TestCase

@shuni-bot-dev shuni-bot-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐕 Shuni's Review

Adds RFC 9449 DPoP proof validation with a new DPoP class and a validateDPoP() wrapper on DescopeSDK. Cryptographic primitives (RSA/EC import, EMSA-PSS, thumbprint, htu normalization) look correct.

Sniffed out 4 issues:

  • 2 🟡 MEDIUM: generic \Exception instead of typed SDK exceptions, no alg/kty consistency check
  • 2 🟢 LOW: asymmetric handling of zero-R in EC signature decode, README example doesn't guard $_SERVER['HTTP_DPOP']

See inline comments for details. Good bones! Woof!

Comment thread src/SDK/DescopeSDK.php
string $method,
string $requestUrl
): void {
DPoP::validateProof($dpopProof, $method, $requestUrl, $sessionToken);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM: validateDPoP (and the underlying DPoP::validateProof) throws raw \Exception on every failure path.

The rest of the SDK uses typed exceptions — verify() throws AuthException / ValidationException. Callers can't catch (AuthException $e) to handle DPoP failures alongside other auth errors; they must catch the entire \Exception hierarchy, which swallows unrelated bugs.

Fix: wrap the throws in DPoP::validateProof with AuthException (signature/claim/format failures) and ValidationException (missing proof / arguments), matching the convention in Verifier.

Comment thread src/SDK/Token/DPoP.php
$result = openssl_verify($signingInput, $signature, $publicKey, $opensslAlg);
if ($result !== 1) {
throw new \Exception('DPoP proof signature verification failed');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM: No cross-check that alg matches jwk.kty.

Key import dispatches on kty (line 226), signature verification dispatches on alg (line 383). Nothing prevents e.g. alg: RS256 paired with jwk.kty: EC — the EC key gets imported, the kty === 'EC' branch on line 414 converts the signature, and openssl_verify happily verifies it as ECDSA-SHA256 because OpenSSL uses the key type, not the OPENSSL_ALGO_* constant, to pick the algorithm.

The thumbprint check at the end (line 154) binds the JWK to the session's cnf.jkt, so this isn't directly exploitable as an impersonation vector. But it violates RFC 9449 §4.3 ("the value of alg [...] indicates the algorithm of the asymmetric signature" — i.e. consistent with the key type) and mirrors the classic JWT alg-confusion vulnerability class. Add an explicit check, e.g.:

$expectedKty = match (true) {
    str_starts_with($alg, 'RS'), str_starts_with($alg, 'PS') => 'RSA',
    str_starts_with($alg, 'ES') => 'EC',
    $alg === 'EdDSA' => 'OKP',
    default => null,
};
if ($expectedKty !== null && ($jwk['kty'] ?? '') !== $expectedKty) {
    throw new \Exception('alg ' . $alg . ' inconsistent with jwk.kty ' . ($jwk['kty'] ?? ''));
}

Comment thread src/SDK/Token/DPoP.php
$r = ltrim($r, "\x00");
if (ord($r[0]) > 0x7f) {
$r = "\x00" . $r;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 LOW: Asymmetric handling of zero-value coordinates between R and S.

Lines 445-448 guard strlen($s) === 0 before ord($s[0]), but lines 441-443 don't do the same for $r. If $r is all-null after ltrim (degenerate/malformed signature), ord($r[0]) triggers a PHP 8 warning on uninitialized offset and the code emits 02 00 — an invalid ASN.1 INTEGER with zero-length content. OpenSSL rejects this with a less-than-helpful error.

Suggested change
}
$r = ltrim($r, "\x00");
if (strlen($r) === 0 || ord($r[0]) > 0x7f) {
$r = "\x00" . $r;
}
$s = ltrim($s, "\x00");
if (strlen($s) === 0 || ord($s[0]) > 0x7f) {
$s = "\x00" . $s;
}

Comment thread README.md
$descopeSDK->validateDPoP(
$sessionToken, // the verified session JWT
$_SERVER['HTTP_DPOP'], // DPoP header sent by the client
$_SERVER['REQUEST_METHOD'], // e.g. "GET" or "POST"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 LOW: $_SERVER['HTTP_DPOP'] is unset when the client doesn't send a DPoP header. Passing null into validateDPoP(string $dpopProof, ...) raises TypeError rather than the intended \Exception('DPoP proof required...').

Suggest showing the safe form so users don't copy a snippet that crashes on the very case the function is designed to detect:

Suggested change
$_SERVER['REQUEST_METHOD'], // e.g. "GET" or "POST"
$_SERVER['HTTP_DPOP'] ?? '', // DPoP header sent by the client (empty string triggers a clear error)

- Use TokenException (not \Exception) throughout DPoP.php for SDK consistency
- Add is_array guard in base64urlDecodeJson to satisfy : array return type
- Fix rawEcSigToDer to guard empty \$r after ltrim (same guard as \$s)
- Add alg/kty cross-check: RS*/PS* require RSA, ES* require EC, EdDSA requires OKP
- Remove dead \$opensslAlgMap array and stale comment in verifyPss
- Update @throws docblock in DescopeSDK::validateDPoP to TokenException
- Add DPoPTest.php to phpunit.xml test suite
- Fix README example to use \$_SERVER['HTTP_DPOP'] ?? '' (null-safe)
- Add class docblock note that jti replay protection is out of scope for stateless SDK

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@davidbemer

Copy link
Copy Markdown
Author

All review comments addressed in commit a8e70c2. Summary of changes:

Fixed:

  • base64urlDecodeJson — added is_array($result) guard; throws TokenException if JSON does not decode to an array
  • rawEcSigToDer — added strlen($r) === 0 guard (same pattern as $s) to prevent ord($r[0]) on empty string when all-zero input is ltrimmed
  • alg/kty cross-check — added explicit compatibility check before key import: RS*/PS* require kty=RSA, ES* require kty=EC, EdDSA requires kty=OKP; throws TokenException on mismatch
  • Exception type — replaced all throw new \Exception( with throw new TokenException( throughout DPoP.php, consistent with Extractor.php and Verifier.php
  • validateDPoP docblock — updated @throws to \Descope\SDK\Exception\TokenException
  • phpunit.xml — added src/tests/DPoPTest.php to the test suite
  • verifyPss dead code — removed unused $opensslAlgMap array and the stale comment describing an unimplemented openssl_verify fallback
  • README — changed $_SERVER['HTTP_DPOP'] to $_SERVER['HTTP_DPOP'] ?? '' to handle absent header; updated throws description to TokenException

Deferred:

  • jti replay protection (RFC 9449 §11.1) — intentionally out of scope for a stateless SDK, consistent with the go-sdk reference implementation. Added a note in the class docblock explaining this and directing callers to implement their own jti store if needed.
  • RSA CRT parameters (p/q/dp/dq/qi) — the existing isset($jwk['d']) check is sufficient per RFC 7517 (d is the primary private-key indicator). CRT param checking deferred as d covers the common case.

@davidbemer davidbemer left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Inline replies to all review comments (details in the PR-level comment above).

Comment thread src/SDK/Token/DPoP.php
}
if (!is_array($result)) {
throw new TokenException('JWT part JSON did not decode to an array');
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Added if (!is_array($result)) { throw new TokenException('JWT part JSON did not decode to an array'); } after the json_last_error() check. The function now guarantees it only returns a true PHP array.

Comment thread src/SDK/Token/DPoP.php
$publicKey = self::importJwkAsPublicKey($jwk, $alg);
$signingInput = $parts[0] . '.' . $parts[1];
$signature = self::base64urlDecode($parts[2]);
self::verifyDpopSignature($signingInput, $signature, $publicKey, $jwk, $alg);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Added an explicit alg/kty cross-check block before importJwkAsPublicKey: RS*/PS* algs require kty=RSA, ES* require kty=EC, and EdDSA requires kty=OKP. A TokenException is thrown on mismatch.

Comment thread src/SDK/Token/DPoP.php
// Steps 17-19: Validate required claims
if (empty($payload['jti'])) {
throw new TokenException('missing jti');
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deferred — jti replay protection (RFC 9449 §11.1) is intentionally out of scope for a stateless SDK. This matches the go-sdk reference implementation. Added a note in the class docblock explaining the omission and directing callers to maintain their own jti store (e.g., a short-lived cache keyed on jti with TTL matching the iat window) if replay protection is required.

Comment thread src/SDK/Token/DPoP.php
// Step 3: Require proof when token is DPoP-bound
if ($dpopProof === '') {
throw new TokenException('DPoP proof required: access token is DPoP-bound (cnf.jkt present)');
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. All throw new \Exception( calls in DPoP.php have been replaced with throw new TokenException(, consistent with Extractor.php and Verifier.php. Added use Descope\SDK\Exception\TokenException; at the top of the file.

Comment thread src/SDK/DescopeSDK.php
* @param string $method The HTTP method of the incoming request (e.g. "GET", "POST").
* @param string $requestUrl The full URL of the incoming request.
* @throws \Descope\SDK\Exception\TokenException If the DPoP proof is missing or invalid.
*/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Updated @throws docblock to \Descope\SDK\Exception\TokenException to match what DPoP::validateProof() now throws.

Comment thread src/tests/DPoPTest.php
* These tests exercise the static helper methods directly without
* requiring a network connection or live Descope credentials.
*/
final class DPoPTest extends TestCase

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Added <file>src/tests/DPoPTest.php</file> to the test suite in phpunit.xml.

Comment thread README.md
$descopeSDK->validateDPoP(
$sessionToken, // the verified session JWT
$_SERVER['HTTP_DPOP'] ?? '', // DPoP header sent by the client (empty string if absent)
$_SERVER['REQUEST_METHOD'], // e.g. "GET" or "POST"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Changed $_SERVER['HTTP_DPOP'] to $_SERVER['HTTP_DPOP'] ?? '' so the example handles the case where the DPoP header is absent without a PHP notice.

@davidbemer davidbemer left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Follow-up inline replies for rawEcSigToDer and verifyPss.

Comment thread src/SDK/Token/DPoP.php
throw new TokenException('RSA-PSS signature verification failed (decrypt step)');
}

// Verify PSS encoding manually

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Removed the unused $opensslAlgMap array and the stale comment describing an openssl_verify fallback. The function now only contains the actual working implementation: raw RSA public decrypt + manual EMSA-PSS verification.

Comment thread src/SDK/Token/DPoP.php
$result = openssl_verify($signingInput, $signature, $publicKey, $opensslAlg);
if ($result !== 1) {
throw new TokenException('DPoP proof signature verification failed');
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Changed the $r guard from if (ord($r[0]) > 0x7f) to if (strlen($r) === 0 || ord($r[0]) > 0x7f), matching the identical pattern already applied to $s. All-zero input for $r will now safely prepend \x00 rather than triggering a warning on $r[0].

Comment thread src/SDK/DescopeSDK.php
string $method,
string $requestUrl
): void {
DPoP::validateProof($dpopProof, $method, $requestUrl, $sessionToken);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. DPoP::validateProof() and all other throw sites in DPoP.php now throw TokenException (imported via use Descope\SDK\Exception\TokenException). The @throws docblock on validateDPoP() has been updated accordingly.

Comment thread README.md
$descopeSDK->validateDPoP(
$sessionToken, // the verified session JWT
$_SERVER['HTTP_DPOP'] ?? '', // DPoP header sent by the client (empty string if absent)
$_SERVER['REQUEST_METHOD'], // e.g. "GET" or "POST"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. Changed to $_SERVER['HTTP_DPOP'] ?? '' so the example handles the absent-header case without a PHP notice. Also updated the validateDPoP throws description to \Descope\SDK\Exception\TokenException.

- Updated composer.lock with phpunit/phpunit 12.5.25 (was 9.6.33)
- Updated config.platform.php from 7.3.0 to 8.3.0 to match phpunit 12 requirements
- Updated require.php from ^7.3 || ^8.0 to ^8.3 to align with phpunit 12
- Updated CI workflows to use PHP 8.3+ since phpunit 12 requires PHP >=8.3

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@davidbemer

Copy link
Copy Markdown
Author

Fixed: updated composer.lock to resolve phpunit version mismatch (lock had 9.6.33, composer.json requires ^12.0.0).

Changes made:

  • composer.lock: updated phpunit/phpunit to 12.5.25 and all dependent packages
  • composer.json: updated config.platform.php from 7.3.0 to 8.3.0 (phpunit 12 requires PHP >=8.3) and require.php to ^8.3
  • .github/workflows/ci.yml: bumped PHP version from 8.1 to 8.3
  • .github/workflows/phpunit.yml: updated matrix from [8.1, 7.4] to [8.4, 8.3]

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.

2 participants