Skip to content

Commit 8d2c826

Browse files
PothercaPotherca
committed
Fix various bugs in DPop class + unit-tests
- Add various checks for unset keys - Change Exception message to state what makes a token invalid -
1 parent f01a65c commit 8d2c826

2 files changed

Lines changed: 234 additions & 28 deletions

File tree

‎src/Utils/DPop.php‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,15 @@
22

33
namespace Pdsinterop\Solid\Auth\Utils;
44

5-
use Lcobucci\JWT\Configuration;
5+
use Jose\Component\Core\JWK;
6+
use Jose\Component\Core\Util\ECKey;
7+
use Jose\Component\Core\Util\RSAKey;
68
use Lcobucci\Clock\SystemClock;
7-
use DateTimeImmutable;
8-
use DateInterval;
9+
use Lcobucci\JWT\Configuration;
910
use Lcobucci\JWT\Signer\Key\InMemory;
10-
use Lcobucci\JWT\Signer\Rsa\Sha256;
1111
use Lcobucci\JWT\Validation\Constraint\LooseValidAt;
1212
use Lcobucci\JWT\Validation\Constraint\SignedWith;
1313

14-
use Jose\Component\Core\JWK;
15-
use Jose\Component\Core\Util\ECKey;
16-
use Jose\Component\Core\Util\RSAKey;
17-
1814
/**
1915
* This class contains code to fetch the WebId from a request
2016
* It also verifies that the request has a valid DPoP token
@@ -44,12 +40,13 @@ public function getWebId($request) {
4440
$dpop = $request->getServerParams()['HTTP_DPOP'];
4541
//@FIXME: check that there is just one DPoP token in the request
4642
if ($dpop) {
47-
$dpopKey = $this->getDpopKey($dpop, $request);
4843
try {
49-
// @FIXME: What happens when DPOP is not valid?
44+
$dpopKey = $this->getDpopKey($dpop, $request);
5045
$this->validateJwtDpop($jwt, $dpopKey);
51-
} catch (Lcobucci\JWT\Validation\RequiredConstraintsViolated $e) {
52-
throw new \Exception("Invalid token", $e);
46+
} catch (\Lcobucci\JWT\Validation\RequiredConstraintsViolated $e) {
47+
throw new \Exception("Invalid token: {$e->getMessage()}", 0, $e);
48+
} catch (\Exception $e) {
49+
throw new \Exception("Invalid token: {$e->getMessage()}", 0, $e);
5350
}
5451
} else {
5552
throw new \Exception("Missing DPoP token");
@@ -84,19 +81,28 @@ public function getDpopKey($dpop, $request) {
8481
$dpop = $jwtConfig->parser()->parse($dpop);
8582
$jwk = $dpop->headers()->get("jwk");
8683

87-
// @FIXME: What happens when 'kid' is not set? 'Undefined array key "kid"'
84+
if (isset($jwk['kid']) === false) {
85+
throw new \Exception('Key ID is missing from JWK header');
86+
}
87+
8888
return $jwk['kid'];
8989
}
9090

9191
private function validateJwtDpop($jwt, $dpopKey) {
9292
$jwtConfig = $configuration = Configuration::forUnsecuredSigner();
9393
$jwt = $jwtConfig->parser()->parse($jwt);
94-
// @FIXME: What happens if CNF is not set?
9594
$cnf = $jwt->claims()->get("cnf");
9695

97-
// @FIXME: What happens if JKT is not set?
98-
if ($cnf['jkt'] == $dpopKey) {
99-
return true;
96+
if ($cnf === null) {
97+
throw new \Exception('JWT Confirmation claim (cnf) is missing');
98+
}
99+
100+
if (isset($cnf['jkt']) === false) {
101+
throw new \Exception('JWT Confirmation claim (cnf) is missing Thumbprint (jkt)');
102+
}
103+
104+
if ($cnf['jkt'] !== $dpopKey) {
105+
throw new \Exception('JWT Confirmation claim (cnf) provided Thumbprint (jkt) does not match Key ID from JWK header');
100106
}
101107

102108
//@FIXME: add check for "ath" claim in DPoP token, per https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-7
@@ -241,6 +247,9 @@ private function getSubjectFromJwt($jwt) {
241247

242248
// @FIXME: What happens when "sub" is not provided?
243249
$sub = $jwt->claims()->get("sub");
250+
if ($sub === null) {
251+
throw new \Exception('Invalid token: Missing "SUB');
252+
}
244253
return $sub;
245254
}
246255
}

‎tests/unit/Utils/DPOPTest.php‎

Lines changed: 208 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
namespace Pdsinterop\Solid\Auth\Utils;
44

55
use Laminas\Diactoros\ServerRequest;
6-
use Pdsinterop\Solid\Auth\AbstractTestCase;
76
use Lcobucci\JWT\Validation\RequiredConstraintsViolated;
7+
use Pdsinterop\Solid\Auth\AbstractTestCase;
8+
use Pdsinterop\Solid\Auth\Enum\Jwk\Parameter as JwkParameter;
89

910
/**
1011
* @coversDefaultClass \Pdsinterop\Solid\Auth\Utils\DPop
@@ -16,6 +17,9 @@ class DPOPTest extends AbstractTestCase
1617
{
1718
////////////////////////////////// FIXTURES \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
1819

20+
const MOCK_SUBJECT = 'mock sub';
21+
const MOCK_THUMBPRINT = 'Mock Thumbprint';
22+
1923
private $dpop;
2024
private $url;
2125
private $serverRequest;
@@ -81,7 +85,7 @@ final public function testInstantiation(): void
8185
}
8286

8387
/**
84-
* @testdox Dpop SHOULD complain WHEN ask to validate DPOP without JWT given
88+
* @testdox Dpop SHOULD complain WHEN asked to validate DPOP without JWT given
8589
*
8690
* @covers ::validateDpop
8791
*/
@@ -95,7 +99,7 @@ final public function testValidateDpopWithoutJwt(): void
9599
}
96100

97101
/**
98-
* @testdox Dpop SHOULD complain WHEN ask to validate DPOP without Request given
102+
* @testdox Dpop SHOULD complain WHEN asked to validate DPOP without Request given
99103
*
100104
* @covers ::validateDpop
101105
*/
@@ -180,6 +184,11 @@ public function testValidateDpopWithCorrectToken(): void
180184
$this->assertTrue($result);
181185
}
182186

187+
/**
188+
* @testdox Dpop SHOULD complain WHEN asked to get WebId without Request given
189+
*
190+
* @covers ::getWebId
191+
*/
183192
final public function testGetWebIdWithoutRequest(): void
184193
{
185194
$dpop = new DPop();
@@ -190,6 +199,8 @@ final public function testGetWebIdWithoutRequest(): void
190199
}
191200

192201
/**
202+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request without Authorization Header
203+
*
193204
* @covers ::getWebId
194205
*/
195206
final public function testGetWebIdWithoutHttpAuthorizationHeader(): void
@@ -205,6 +216,25 @@ final public function testGetWebIdWithoutHttpAuthorizationHeader(): void
205216
}
206217

207218
/**
219+
* @testdox Dpop SHOULD return "public" WHEN asked to get WebId from Request with incorrect Authorization Header format
220+
*
221+
* @covers ::getWebId
222+
*/
223+
final public function testGetWebIdWithIncorrectAuthHeaderFormat(): void
224+
{
225+
$dpop = new DPop();
226+
227+
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => 'IncorrectAuthorizationFormat'),array(), $this->url);
228+
229+
$actual = $dpop->getWebId($request);
230+
$expected = 'public';
231+
232+
$this->assertEquals($expected, $actual);
233+
}
234+
235+
/**
236+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with invalid JWT
237+
*
208238
* @covers ::getWebId
209239
*/
210240
final public function testGetWebIdWithInvalidJwt(): void
@@ -219,27 +249,194 @@ final public function testGetWebIdWithInvalidJwt(): void
219249
$dpop->getWebId($request);
220250
}
221251

222-
223252
/**
253+
* @testdox Dpop SHOULD return "public" WHEN asked to get WebId from Request with "Basic" authorization
254+
*
224255
* @covers ::getWebId
225256
*/
226-
final public function testGetWebId(): void
257+
final public function testGetWebIdWithoutDpop(): void
227258
{
228259
$dpop = new DPop();
229260

230-
$token = $this->sign($this->dpop)['token'];
261+
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => "Basic YWxhZGRpbjpvcGVuc2VzYW1l"),array(), $this->url);
262+
263+
$this->expectException(\Exception::class);
264+
$this->expectExceptionMessage('Missing DPoP token');
231265

232-
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => $token),array(), $this->url);
266+
$this->markTestIncomplete('The current result is not testable (Undefined array key "HTTP_DPOP")');
233267

234268
$actual = $dpop->getWebId($request);
235-
$expected = 'public';
269+
}
236270

237-
$this->assertEquals($expected, $actual);
271+
/**
272+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without JWT Key Id
273+
*
274+
* @covers ::getWebId
275+
*
276+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
277+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
278+
*/
279+
final public function testGetWebIdWithDpopWithoutKeyId(): void
280+
{
281+
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
282+
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
283+
284+
$token = $this->sign($this->dpop);
285+
286+
$dpop = new DPop();
287+
288+
$request = new ServerRequest(array(
289+
'HTTP_AUTHORIZATION' => "dpop {$token['token']}",
290+
'HTTP_DPOP' => $token['token'],
291+
),array(), $this->url);
292+
293+
$this->expectException(\Exception::class);
294+
$this->expectExceptionMessage('Invalid token');
295+
296+
$dpop->getWebId($request);
238297
}
239298

240-
/////////////////////////////// DATAPROVIDERS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
299+
/**
300+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without Confirmation Claim
301+
*
302+
* @covers ::getWebId
303+
*
304+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
305+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
306+
*/
307+
final public function testGetWebIdWithDpopWithoutConfirmationClaim(): void
308+
{
309+
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
310+
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
311+
312+
$token = $this->sign($this->dpop);
313+
314+
$dpop = new DPop();
315+
316+
$request = new ServerRequest(array(
317+
'HTTP_AUTHORIZATION' => "dpop {$token['token']}",
318+
'HTTP_DPOP' => $token['token'],
319+
),array(), $this->url);
241320

242-
////////////////////////////// MOCKS AND STUBS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\
321+
$this->expectException(\Exception::class);
322+
$this->expectExceptionMessage('Invalid token');
323+
324+
$dpop->getWebId($request);
325+
}
326+
327+
/**
328+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without JWT Key Thumbprint
329+
*
330+
* @covers ::getWebId
331+
*
332+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
333+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
334+
*/
335+
final public function testGetWebIdWithDpopWithoutThumbprint(): void
336+
{
337+
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
338+
$this->dpop['payload']['cnf'] = [];
339+
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
340+
341+
$token = $this->sign($this->dpop);
342+
343+
$dpop = new DPop();
344+
345+
$request = new ServerRequest(array(
346+
'HTTP_AUTHORIZATION' => "dpop {$token['token']}",
347+
'HTTP_DPOP' => $token['token'],
348+
),array(), $this->url);
349+
350+
$this->expectException(\Exception::class);
351+
$this->expectExceptionMessage('Invalid token');
352+
353+
$dpop->getWebId($request);
354+
}
355+
356+
/**
357+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP with Thumbprint not matching Key Id
358+
*
359+
* @covers ::getWebId
360+
*
361+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
362+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
363+
*/
364+
final public function testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): void
365+
{
366+
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT . 'Mismatch';
367+
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
368+
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
369+
370+
$token = $this->sign($this->dpop);
371+
372+
$dpop = new DPop();
373+
374+
$request = new ServerRequest(array(
375+
'HTTP_AUTHORIZATION' => "dpop {$token['token']}",
376+
'HTTP_DPOP' => $token['token'],
377+
),array(), $this->url);
378+
379+
$this->expectException(\Exception::class);
380+
$this->expectExceptionMessage('Invalid token');
381+
382+
$dpop->getWebId($request);
383+
}
384+
385+
/**
386+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without "sub"
387+
*
388+
* @covers ::getWebId
389+
*
390+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
391+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
392+
*/
393+
final public function testGetWebIdWithDpopWithoutSub(): void
394+
{
395+
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
396+
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
397+
398+
$token = $this->sign($this->dpop);
399+
400+
$dpop = new DPop();
401+
402+
$request = new ServerRequest(array(
403+
'HTTP_AUTHORIZATION' => "dpop {$token['token']}",
404+
'HTTP_DPOP' => $token['token'],
405+
),array(), $this->url);
406+
407+
$this->expectException(\Exception::class);
408+
$this->expectExceptionMessage('Invalid token');
409+
410+
$dpop->getWebId($request);
411+
}
412+
413+
/**
414+
* @testdox Dpop SHOULD return given "sub" WHEN asked to get WebId from Request with complete DPOP
415+
*
416+
* @covers ::getWebId
417+
*
418+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
419+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
420+
*/
421+
final public function testGetWebIdWithDpop(): void
422+
{
423+
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
424+
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
425+
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
426+
427+
$token = $this->sign($this->dpop);
428+
429+
$dpop = new DPop();
430+
431+
$request = new ServerRequest(array(
432+
'HTTP_AUTHORIZATION' => "dpop {$token['token']}",
433+
'HTTP_DPOP' => $token['token'],
434+
),array(), $this->url);
435+
436+
$actual = $dpop->getWebId($request);
437+
438+
$this->assertEquals(self::MOCK_SUBJECT, $actual);
439+
}
243440

244441
///////////////////////////// HELPER FUNCTIONS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\
245442

0 commit comments

Comments
 (0)