Skip to content

Commit e5453e0

Browse files
poefpoef
authored
Merge pull request #35 from pdsinterop/fix/dpop
Fix dpop
2 parents ea830b6 + 7787c5e commit e5453e0

File tree

4 files changed

+121
-41
lines changed

4 files changed

+121
-41
lines changed

‎src/TokenGenerator.php‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22

33
namespace Pdsinterop\Solid\Auth;
44

5-
use Pdsinterop\Solid\Auth\Utils\Jwks;
5+
use Pdsinterop\Solid\Auth\Exception\InvalidTokenException;
6+
use Pdsinterop\Solid\Auth\Utils\DPop;
67
use Pdsinterop\Solid\Auth\Enum\OpenId\OpenIdConnectMetadata as OidcMeta;
7-
use Laminas\Diactoros\Response\JsonResponse as JsonResponse;
8+
use Laminas\Diactoros\Response\JsonResponse;
89
use League\OAuth2\Server\CryptTrait;
910

1011
use DateTimeImmutable;
@@ -21,14 +22,17 @@ class TokenGenerator
2122
public Config $config;
2223

2324
private \DateInterval $validFor;
25+
private DPop $dpopUtil;
2426

2527
//////////////////////////////// PUBLIC API \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
2628

2729
final public function __construct(
2830
Config $config,
29-
\DateInterval $validFor
31+
\DateInterval $validFor,
32+
DPop $dpopUtil,
3033
) {
3134
$this->config = $config;
35+
$this->dpopUtil = $dpopUtil;
3236
$this->validFor = $validFor;
3337

3438
$this->setEncryptionKey($this->config->getKeys()->getEncryptionKey());
@@ -48,10 +52,9 @@ public function generateRegistrationAccessToken($clientId, $privateKey) {
4852
return $token->toString();
4953
}
5054

51-
public function generateIdToken($accessToken, $clientId, $subject, $nonce, $privateKey, $dpopKey, $now=null) {
55+
public function generateIdToken($accessToken, $clientId, $subject, $nonce, $privateKey, $dpop, $now=null) {
5256
$issuer = $this->config->getServer()->get(OidcMeta::ISSUER);
5357

54-
$jwks = $this->getJwks();
5558
$tokenHash = $this->generateTokenHash($accessToken);
5659

5760
// Create JWT
@@ -61,6 +64,8 @@ public function generateIdToken($accessToken, $clientId, $subject, $nonce, $priv
6164

6265
$expire = $now->add($this->validFor);
6366

67+
$jkt = $this->makeJwkThumbprint($dpop);
68+
6469
$token = $jwtConfig->builder()
6570
->issuedBy($issuer)
6671
->permittedFor($clientId)
@@ -74,10 +79,8 @@ public function generateIdToken($accessToken, $clientId, $subject, $nonce, $priv
7479
->withClaim("at_hash", $tokenHash) //FIXME: at_hash should only be added if the response_type is a token
7580
->withClaim("c_hash", $tokenHash) // FIXME: c_hash should only be added if the response_type is a code
7681
->withClaim("cnf", array(
77-
"jkt" => $dpopKey,
78-
// "jwk" => $jwks['keys'][0]
82+
"jkt" => $jkt,
7983
))
80-
->withHeader('kid', $jwks['keys'][0]['kid'])
8184
->getToken($jwtConfig->signer(), $jwtConfig->signingKey());
8285
return $token->toString();
8386
}
@@ -104,7 +107,7 @@ public function respondToRegistration($registration, $privateKey) {
104107
return array_merge($registrationBase, $registration);
105108
}
106109

107-
public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $privateKey, $dpopKey=null) {
110+
public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $privateKey, $dpop=null) {
108111
if ($response->hasHeader("Location")) {
109112
$value = $response->getHeaderLine("Location");
110113

@@ -115,7 +118,7 @@ public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $pr
115118
$subject,
116119
$nonce,
117120
$privateKey,
118-
$dpopKey
121+
$dpop
119122
);
120123
$value = preg_replace("/#access_token=(.*?)&/", "#access_token=\$1&id_token=$idToken&", $value);
121124
$response = $response->withHeader("Location", $value);
@@ -126,7 +129,7 @@ public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $pr
126129
$subject,
127130
$nonce,
128131
$privateKey,
129-
$dpopKey
132+
$dpop
130133
);
131134
$value = preg_replace("/code=(.*?)&/", "code=\$1&id_token=$idToken&", $value);
132135
$response = $response->withHeader("Location", $value);
@@ -143,7 +146,7 @@ public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $pr
143146
$subject,
144147
$nonce,
145148
$privateKey,
146-
$dpopKey
149+
$dpop
147150
);
148151

149152
$body['access_token'] = $body['id_token'];
@@ -178,9 +181,16 @@ private function generateTokenHash($accessToken) {
178181
return $atHash;
179182
}
180183

181-
private function getJwks() {
182-
$key = $this->config->getKeys()->getPublicKey();
183-
$jwks = new Jwks($key);
184-
return json_decode($jwks->__toString(), true);
184+
private function makeJwkThumbprint($dpop): string
185+
{
186+
$dpopConfig = Configuration::forUnsecuredSigner();
187+
$parsedDpop = $dpopConfig->parser()->parse($dpop);
188+
$jwk = $parsedDpop->headers()->get("jwk");
189+
190+
if (empty($jwk)) {
191+
throw new InvalidTokenException('Required JWK header missing in DPOP');
192+
}
193+
194+
return $this->dpopUtil->makeJwkThumbprint($jwk);
185195
}
186196
}

‎src/Utils/DPop.php‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ public function makeJwkThumbprint($jwk) {
157157
* See also: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-7
158158
*
159159
* Validates the above part of the oauth dpop specification
160+
*
160161
* @param string $jwt JWT access token, raw
161162
* @param string $dpop DPoP token, raw
162163
* @param ServerRequestInterface $request Server Request
@@ -165,16 +166,17 @@ public function makeJwkThumbprint($jwk) {
165166
public function validateJwtDpop($jwt, $dpop, $request) {
166167
$this->validateDpop($dpop, $request);
167168
$jwtConfig = Configuration::forUnsecuredSigner();
168-
$dpopJWT = $jwtConfig->parser()->parse($dpop);
169-
170-
$ath = $dpopJWT->claims()->get('ath');
171-
if ($ath === null) {
172-
throw new InvalidTokenException('DPoP "ath" claim is missing');
173-
}
174-
175-
$hash = hash('sha256', $jwt);
176-
$encoded = Base64Url::encode($hash);
177-
return ($ath === $encoded);
169+
$jwtConfig->parser()->parse($dpop);
170+
171+
/**
172+
* @FIXME: ATH claim is not yet supported/required by the Solid OIDC specification.
173+
* Once the Solid spec catches up to the DPOP spec, not having an ATH is incorrect.
174+
* At that point, instead of returning "true", throw an exception:
175+
*
176+
* @see https://github.com/pdsinterop/php-solid-auth/issues/34
177+
*/
178+
// throw new InvalidTokenException('DPoP "ath" claim is missing');
179+
return true;
178180
}
179181

180182
/**

‎tests/unit/TokenGeneratorTest.php‎

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Pdsinterop\Solid\Auth\Config\ServerInterface;
77
use Pdsinterop\Solid\Auth\Enum\OpenId\OpenIdConnectMetadata as OidcMeta;
88
use Pdsinterop\Solid\Auth\Utils\Base64Url;
9+
use Pdsinterop\Solid\Auth\Utils\DPop;
910
use PHPUnit\Framework\MockObject\MockObject;
1011

1112
function time() { return 1234;}
@@ -21,10 +22,13 @@ class TokenGeneratorTest extends AbstractTestCase
2122
{
2223
////////////////////////////////// FIXTURES \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
2324

25+
const MOCK_DPOP = "mock.dpop.value";
26+
const MOCK_JKT = 'mock jkt';
27+
2428
private MockObject|Config $mockConfig;
2529
private MockObject|KeysInterface $mockKeys;
2630

27-
private function createTokenGenerator($interval = null): TokenGenerator
31+
private function createTokenGenerator($interval = null, $jkt = null): TokenGenerator
2832
{
2933
$this->mockConfig = $this->getMockBuilder(Config::class)
3034
->disableOriginalConstructor()
@@ -51,7 +55,19 @@ private function createTokenGenerator($interval = null): TokenGenerator
5155
->willReturn('mock encryption key')
5256
;
5357

54-
return new TokenGenerator($this->mockConfig, $interval??$mockInterval);
58+
$mockDpopUtil = $this->getMockBuilder(DPop::class)
59+
->disableOriginalConstructor()
60+
->getMock()
61+
;
62+
63+
if ($jkt) {
64+
$mockDpopUtil->expects($this->once())
65+
->method('makeJwkThumbprint')
66+
->willReturn($jkt)
67+
;
68+
}
69+
70+
return new TokenGenerator($this->mockConfig, $interval??$mockInterval, $mockDpopUtil);
5571
}
5672

5773
/////////////////////////////////// TESTS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
@@ -84,6 +100,27 @@ final public function testInstantiateWithoutValidFor(): void
84100
new TokenGenerator($mockConfig);
85101
}
86102

103+
/**
104+
* @testdox Token Generator SHOULD complain WHEN instantiated without Dpop Utility
105+
*
106+
* @coversNothing
107+
*/
108+
final public function testInstantiateWithoutDpopUtility(): void
109+
{
110+
$this->expectArgumentCountError(3);
111+
112+
$mockConfig = $this->getMockBuilder(Config::class)
113+
->disableOriginalConstructor()
114+
->getMock();
115+
116+
$mockInterval = $this->getMockBuilder(\DateInterval::class)
117+
->disableOriginalConstructor()
118+
->getMock()
119+
;
120+
121+
new TokenGenerator($mockConfig, $mockInterval);
122+
}
123+
87124
/**
88125
* @testdox Token Generator SHOULD be created WHEN instantiated with Config and validity period
89126
*
@@ -271,14 +308,7 @@ final public function testIdTokenGeneration(): void
271308
{
272309
$validFor = new \DateInterval('PT1S');
273310

274-
$tokenGenerator = $this->createTokenGenerator($validFor);
275-
276-
$mockKey = \Lcobucci\JWT\Signer\Key\InMemory::file(__DIR__.'/../fixtures/keys/public.key');
277-
278-
$this->mockKeys->expects($this->once())
279-
->method('getPublicKey')
280-
->willReturn($mockKey)
281-
;
311+
$tokenGenerator = $this->createTokenGenerator($validFor, self::MOCK_JKT);
282312

283313
$mockServer = $this->getMockBuilder(ServerInterface::class)
284314
->disableOriginalConstructor()
@@ -300,26 +330,31 @@ final public function testIdTokenGeneration(): void
300330

301331
$now = new \DateTimeImmutable('1234-01-01 12:34:56.789');
302332

333+
$encodedDpop = vsprintf("%s.%s.%s", [
334+
'header' => Base64Url::encode('{"jwk":"mock jwk"}'),
335+
'body' => Base64Url::encode('{}'),
336+
'signature' => Base64Url::encode('mock signature')
337+
]);
338+
303339
$actual = $tokenGenerator->generateIdToken(
304340
'mock access token',
305341
'mock clientId',
306342
'mock subject',
307343
'mock nonce',
308344
$privateKey,
309-
'mock dpop',
345+
$encodedDpop,
310346
$now
311347
);
312348

313349
$this->assertJwtEquals([[
314350
"alg"=>"RS256",
315-
"kid"=>"0c3932ca20f3a00ad2eb72035f6cc9cb",
316351
"typ"=>"JWT",
317352
],[
353+
'at_hash' => '1EZBnvsFWlK8ESkgHQsrIQ',
318354
'aud' => 'mock clientId',
319355
'azp' => 'mock clientId',
320356
'c_hash' => '1EZBnvsFWlK8ESkgHQsrIQ',
321-
'at_hash' => '1EZBnvsFWlK8ESkgHQsrIQ',
322-
'cnf' => ["jkt" => "mock dpop"],
357+
'cnf' => ["jkt" => self::MOCK_JKT],
323358
'exp' => -23225829903.789,
324359
'iat' => -23225829904.789,
325360
'iss' => 'mock issuer',

‎tests/unit/Utils/DPOPTest.php‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,36 @@ final public function testGetWebIdWithDpopWithoutSub(): void
517517
$dpop->getWebId($request);
518518
}
519519

520+
/**
521+
* @testdox Dpop SHOULD not complain WHEN asked to get WebId from Request with valid DPOP without "ath"
522+
*
523+
* @covers ::getWebId
524+
*
525+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
526+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
527+
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
528+
*/
529+
final public function testGetWebIdWithDpopWithoutOptionalAth(): void
530+
{
531+
unset($this->dpop['payload']['ath']);
532+
$token = $this->sign($this->dpop);
533+
534+
$mockJtiValidator = $this->createMockJtiValidator();
535+
$mockJtiValidator->expects($this->once())
536+
->method('validate')
537+
->willReturn(true)
538+
;
539+
$dpop = new DPop($mockJtiValidator);
540+
541+
$request = new ServerRequest(array(
542+
'HTTP_AUTHORIZATION' => "dpop {$this->accessToken['token']}",
543+
'HTTP_DPOP' => $token['token'],
544+
),array(), $this->url);
545+
546+
$webId = $dpop->getWebId($request);
547+
548+
$this->assertEquals(self::MOCK_SUBJECT, $webId);
549+
}
520550
/**
521551
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without "ath"
522552
*
@@ -526,8 +556,11 @@ final public function testGetWebIdWithDpopWithoutSub(): void
526556
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
527557
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
528558
*/
529-
final public function testGetWebIdWithDpopWithoutAth(): void
559+
final public function testGetWebIdWithDpopWithoutRequiredAth(): void
530560
{
561+
/*/ @see https://github.com/pdsinterop/php-solid-auth/issues/34 /*/
562+
$this->markTestSkipped('ATH claim is not yet supported/required by the Solid OIDC specification.');
563+
531564
unset($this->dpop['payload']['ath']);
532565
$token = $this->sign($this->dpop);
533566

0 commit comments

Comments
 (0)