Skip to content

Commit bc9e227

Browse files
committed
improved isset() calls
fixed @returns at validateDpop add $this->markTestSkipped for 3 tests that are probably incorrect
1 parent 961a2b9 commit bc9e227

File tree

2 files changed

+12
-16
lines changed

2 files changed

+12
-16
lines changed

src/Utils/DPop.php

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,21 +114,22 @@ public function makeJwkThumbprint($jwk) {
114114
if (!$jwk || !isset($jwk['kty'])) {
115115
throw new InvalidTokenException('JWK has no "kty" key type');
116116
}
117+
// https://www.rfc-editor.org/rfc/rfc7517.html#section-4.1
118+
// and https://www.rfc-editor.org/rfc/rfc7518.html#section-6.1
117119
if (!in_array($jwk['kty'], ['RSA','EC'])) {
118120
throw new InvalidTokenException('JWK "kty" key type value must be one of "RSA" or "EC", got "'.$jwk['kty'].'" instead.');
119121
}
120-
if ($jwk['kty']=='RSA') {
121-
if (!isset($jwk['e']) || !isset($jwk['n'])) {
122+
if ($jwk['kty']=='RSA') { // used with RS256 alg
123+
if (!isset($jwk['e'], $jwk['n'])) {
122124
throw new InvalidTokenException('JWK values do not match "RSA" key type');
123125
}
124126
$json = vsprintf('{"e":"%s","kty":"%s","n":"%s"}', [
125127
$jwk['e'],
126128
$jwk['kty'],
127129
$jwk['n'],
128130
]);
129-
130-
} else { //EC
131-
if (!isset($jwk['crv']) || !isset($jwk['x']) || !isset($jwk['y'])) {
131+
} else { // EC used with ES256 alg
132+
if (!isset($jwk['crv'], $jwk['x'], $jwk['y'])) {
132133
throw new InvalidTokenException('JWK values doe not match "EC" key type');
133134
}
134135
//crv, kty, x, y
@@ -219,7 +220,7 @@ public function validateIdTokenDpop($token, $dpop, $request) {
219220
* @param string $dpop The DPOP token
220221
* @param ServerRequestInterface $request Server Request
221222
*
222-
* @return bool True if the DPOP token is valid, false otherwise
223+
* @return bool True if the DPOP token is valid
223224
*
224225
* @throws RequiredConstraintsViolated
225226
* @throws InvalidTokenException

tests/unit/Utils/DPOPTest.php

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,6 @@ final public function testGetWebIdWithDpopWithoutKeyId(): void
349349
$this->dpop['payload']['jti'] = 'mock jti';
350350
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
351351

352-
353352
$dpopToken = $this->sign($this->dpop);
354353

355354
$mockJtiValidator = $this->createMockJtiValidator();
@@ -374,16 +373,15 @@ final public function testGetWebIdWithDpopWithoutKeyId(): void
374373

375374
/**
376375
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without Confirmation Claim
377-
* why? which spec?
378376
* @covers ::getWebId
379377
*
380378
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
381379
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
382380
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
383381
*/
384-
/*
385382
final public function testGetWebIdWithDpopWithoutConfirmationClaim(): void
386383
{
384+
$this->markTestSkipped('Skipped untill we find a spec that requires this');
387385
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
388386
$this->dpop['payload']['jti'] = 'mock jti';
389387
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
@@ -409,19 +407,18 @@ final public function testGetWebIdWithDpopWithoutConfirmationClaim(): void
409407

410408
$dpop->getWebId($request);
411409
}
412-
*/
410+
413411
/**
414412
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without JWT Key Thumbprint
415-
* why? which spec?
416413
* @covers ::getWebId
417414
*
418415
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
419416
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
420417
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
421418
*/
422-
/*
423419
final public function testGetWebIdWithDpopWithoutThumbprint(): void
424420
{
421+
$this->markTestSkipped('Skipped untill we find a spec that requires this');
425422
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
426423
$this->dpop['payload']['cnf'] = [];
427424
$this->dpop['payload']['jti'] = 'mock jti';
@@ -446,19 +443,18 @@ final public function testGetWebIdWithDpopWithoutThumbprint(): void
446443

447444
$dpop->getWebId($request);
448445
}
449-
*/
446+
450447
/**
451448
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP with Thumbprint not matching Key Id
452-
* why? which spec?
453449
* @covers ::getWebId
454450
*
455451
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
456452
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
457453
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
458454
*/
459-
/*
460455
final public function testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): void
461456
{
457+
$this->markTestSkipped('Skipped untill we find a spec that requires this');
462458
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT . 'Mismatch';
463459
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
464460
$this->dpop['payload']['jti'] = 'mock jti';
@@ -483,7 +479,6 @@ final public function testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): v
483479

484480
$dpop->getWebId($request);
485481
}
486-
*/
487482

488483
/**
489484
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without "sub"

0 commit comments

Comments
 (0)