Skip to content

Commit 4337acf

Browse files
PothercaPotherca
committed
Change checks in DPop class to have more explicit messages
1 parent 304d51c commit 4337acf

2 files changed

Lines changed: 84 additions & 37 deletions

File tree

‎src/Utils/DPop.php‎

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,26 +44,40 @@ public function __construct(JtiValidator $jtiValidator)
4444
* @throws Exception "Missing DPoP token" when the DPoP token is missing, but the Authorisation header in the request specifies it
4545
*/
4646
public function getWebId($request) {
47-
// @FIXME: What happens when HTTP_AUTHORIZATION is not set?
48-
$auth = explode(" ", $request->getServerParams()['HTTP_AUTHORIZATION']);
49-
$jwt = $auth[1] ?? false;
50-
51-
if (strtolower($auth[0]) == "dpop") {
52-
// @FIXME: What happens when HTTP_DPOP is not set?
53-
$dpop = $request->getServerParams()['HTTP_DPOP'];
54-
//@FIXME: check that there is just one DPoP token in the request
55-
if ($dpop) {
56-
try {
57-
$dpopKey = $this->getDpopKey($dpop, $request);
58-
$this->validateJwtDpop($jwt, $dpopKey);
59-
} catch (\Lcobucci\JWT\Validation\RequiredConstraintsViolated $e) {
60-
throw new \Exception("Invalid token: {$e->getMessage()}", 0, $e);
61-
} catch (\Exception $e) {
62-
throw new \Exception("Invalid token: {$e->getMessage()}", 0, $e);
63-
}
64-
} else {
65-
throw new \Exception("Missing DPoP token");
66-
}
47+
$serverParams = $request->getServerParams();
48+
49+
if (isset($serverParams['HTTP_AUTHORIZATION']) === false) {
50+
throw new Exception("Authorization Header missing");
51+
}
52+
53+
if (str_contains($serverParams['HTTP_AUTHORIZATION'], ' ') === false) {
54+
throw new Exception("Authorization Header does not contain parameters");
55+
}
56+
57+
[$authScheme, $jwt] = explode(" ", $serverParams['HTTP_AUTHORIZATION'], 2);
58+
$authScheme = strtolower($authScheme);
59+
60+
if ($authScheme !== "dpop") {
61+
throw new Exception('Only "dpop" authorization scheme is supported');
62+
}
63+
64+
if (isset($serverParams['HTTP_DPOP']) === false) {
65+
throw new Exception("Missing DPoP token");
66+
}
67+
68+
$dpop = $serverParams['HTTP_DPOP'];
69+
70+
//@FIXME: check that there is just one DPoP token in the request
71+
try {
72+
$dpopKey = $this->getDpopKey($dpop, $request);
73+
} catch (InvalidTokenStructure $e) {
74+
throw new Exception("Invalid JWT token: {$e->getMessage()}", 0, $e);
75+
}
76+
77+
try {
78+
$this->validateJwtDpop($jwt, $dpopKey);
79+
} catch (RequiredConstraintsViolated $e) {
80+
throw new Exception("Invalid token: {$e->getMessage()}", 0, $e);
6781
}
6882

6983
if ($jwt) {

‎tests/unit/Utils/DPOPTest.php‎

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,14 @@ final public function testGetWebIdWithoutHttpAuthorizationHeader(): void
243243

244244
$request = new ServerRequest(array(),array(), $this->url);
245245

246-
$this->markTestIncomplete('The current result is not testable (Undefined array key "HTTP_AUTHORIZATION")');
246+
$this->expectException(\Exception::class);
247+
$this->expectExceptionMessage('Authorization Header missing');
247248

248249
$dpop->getWebId($request);
249250
}
250251

251252
/**
252-
* @testdox Dpop SHOULD return "public" WHEN asked to get WebId from Request with incorrect Authorization Header format
253+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with incorrect Authorization Header format
253254
*
254255
* @covers ::getWebId
255256
*/
@@ -260,16 +261,18 @@ final public function testGetWebIdWithIncorrectAuthHeaderFormat(): void
260261

261262
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => 'IncorrectAuthorizationFormat'),array(), $this->url);
262263

263-
$actual = $dpop->getWebId($request);
264-
$expected = 'public';
264+
$this->expectException(\Exception::class);
265+
$this->expectExceptionMessage('Authorization Header does not contain parameters');
265266

266-
$this->assertEquals($expected, $actual);
267+
$dpop->getWebId($request);
267268
}
268269

269270
/**
270271
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with invalid JWT
271272
*
272273
* @covers ::getWebId
274+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
275+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
273276
*/
274277
final public function testGetWebIdWithInvalidJwt(): void
275278
{
@@ -279,13 +282,16 @@ final public function testGetWebIdWithInvalidJwt(): void
279282
$this->expectException(\Exception::class);
280283
$this->expectExceptionMessage('Invalid JWT token');
281284

282-
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => 'Invalid JWT'),array(), $this->url);
285+
$request = new ServerRequest(array(
286+
'HTTP_AUTHORIZATION' => "dpop Invalid JWT",
287+
'HTTP_DPOP' => 'Mock dpop',
288+
),array(), $this->url);
283289

284290
$dpop->getWebId($request);
285291
}
286292

287293
/**
288-
* @testdox Dpop SHOULD return "public" WHEN asked to get WebId from Request with "Basic" authorization
294+
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request without DPOP authorization
289295
*
290296
* @covers ::getWebId
291297
*/
@@ -294,14 +300,12 @@ final public function testGetWebIdWithoutDpop(): void
294300
$mockJtiValidator = $this->createMockJtiValidator();
295301
$dpop = new DPop($mockJtiValidator);
296302

297-
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => "Basic YWxhZGRpbjpvcGVuc2VzYW1l"),array(), $this->url);
298-
299303
$this->expectException(\Exception::class);
300-
$this->expectExceptionMessage('Missing DPoP token');
304+
$this->expectExceptionMessage('Only "dpop" authorization scheme is supported');
301305

302-
$this->markTestIncomplete('The current result is not testable (Undefined array key "HTTP_DPOP")');
306+
$request = new ServerRequest(array('HTTP_AUTHORIZATION' => "Basic YWxhZGRpbjpvcGVuc2VzYW1l"),array(), $this->url);
303307

304-
$actual = $dpop->getWebId($request);
308+
$dpop->getWebId($request);
305309
}
306310

307311
/**
@@ -315,11 +319,18 @@ final public function testGetWebIdWithoutDpop(): void
315319
final public function testGetWebIdWithDpopWithoutKeyId(): void
316320
{
317321
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
322+
$this->dpop['payload']['jti'] = 'mock jti';
318323
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
319324

320325
$token = $this->sign($this->dpop);
321326

322327
$mockJtiValidator = $this->createMockJtiValidator();
328+
329+
$mockJtiValidator->expects($this->once())
330+
->method('validate')
331+
->willReturn(true)
332+
;
333+
323334
$dpop = new DPop($mockJtiValidator);
324335

325336
$request = new ServerRequest(array(
@@ -328,7 +339,7 @@ final public function testGetWebIdWithDpopWithoutKeyId(): void
328339
),array(), $this->url);
329340

330341
$this->expectException(\Exception::class);
331-
$this->expectExceptionMessage('Invalid token');
342+
$this->expectExceptionMessage('Key ID is missing from JWK header');
332343

333344
$dpop->getWebId($request);
334345
}
@@ -344,11 +355,18 @@ final public function testGetWebIdWithDpopWithoutKeyId(): void
344355
final public function testGetWebIdWithDpopWithoutConfirmationClaim(): void
345356
{
346357
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
358+
$this->dpop['payload']['jti'] = 'mock jti';
347359
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
348360

349361
$token = $this->sign($this->dpop);
350362

351363
$mockJtiValidator = $this->createMockJtiValidator();
364+
365+
$mockJtiValidator->expects($this->once())
366+
->method('validate')
367+
->willReturn(true)
368+
;
369+
352370
$dpop = new DPop($mockJtiValidator);
353371

354372
$request = new ServerRequest(array(
@@ -357,7 +375,7 @@ final public function testGetWebIdWithDpopWithoutConfirmationClaim(): void
357375
),array(), $this->url);
358376

359377
$this->expectException(\Exception::class);
360-
$this->expectExceptionMessage('Invalid token');
378+
$this->expectExceptionMessage('JWT Confirmation claim (cnf) is missing');
361379

362380
$dpop->getWebId($request);
363381
}
@@ -374,11 +392,16 @@ final public function testGetWebIdWithDpopWithoutThumbprint(): void
374392
{
375393
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
376394
$this->dpop['payload']['cnf'] = [];
395+
$this->dpop['payload']['jti'] = 'mock jti';
377396
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
378397

379398
$token = $this->sign($this->dpop);
380399

381400
$mockJtiValidator = $this->createMockJtiValidator();
401+
$mockJtiValidator->expects($this->once())
402+
->method('validate')
403+
->willReturn(true)
404+
;
382405
$dpop = new DPop($mockJtiValidator);
383406

384407
$request = new ServerRequest(array(
@@ -387,7 +410,7 @@ final public function testGetWebIdWithDpopWithoutThumbprint(): void
387410
),array(), $this->url);
388411

389412
$this->expectException(\Exception::class);
390-
$this->expectExceptionMessage('Invalid token');
413+
$this->expectExceptionMessage('JWT Confirmation claim (cnf) is missing Thumbprint (jkt)');
391414

392415
$dpop->getWebId($request);
393416
}
@@ -404,11 +427,16 @@ final public function testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): v
404427
{
405428
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT . 'Mismatch';
406429
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
430+
$this->dpop['payload']['jti'] = 'mock jti';
407431
$this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
408432

409433
$token = $this->sign($this->dpop);
410434

411435
$mockJtiValidator = $this->createMockJtiValidator();
436+
$mockJtiValidator->expects($this->once())
437+
->method('validate')
438+
->willReturn(true)
439+
;
412440
$dpop = new DPop($mockJtiValidator);
413441

414442
$request = new ServerRequest(array(
@@ -417,7 +445,7 @@ final public function testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): v
417445
),array(), $this->url);
418446

419447
$this->expectException(\Exception::class);
420-
$this->expectExceptionMessage('Invalid token');
448+
$this->expectExceptionMessage('JWT Confirmation claim (cnf) provided Thumbprint (jkt) does not match Key ID from JWK header');
421449

422450
$dpop->getWebId($request);
423451
}
@@ -434,10 +462,15 @@ final public function testGetWebIdWithDpopWithoutSub(): void
434462
{
435463
$this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
436464
$this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
465+
$this->dpop['payload']['jti'] = 'mock jti';
437466

438467
$token = $this->sign($this->dpop);
439468

440469
$mockJtiValidator = $this->createMockJtiValidator();
470+
$mockJtiValidator->expects($this->once())
471+
->method('validate')
472+
->willReturn(true)
473+
;
441474
$dpop = new DPop($mockJtiValidator);
442475

443476
$request = new ServerRequest(array(
@@ -446,7 +479,7 @@ final public function testGetWebIdWithDpopWithoutSub(): void
446479
),array(), $this->url);
447480

448481
$this->expectException(\Exception::class);
449-
$this->expectExceptionMessage('Invalid token');
482+
$this->expectExceptionMessage('Missing "SUB"');
450483

451484
$dpop->getWebId($request);
452485
}

0 commit comments

Comments
 (0)