Skip to content

Commit

Permalink
Merge pull request #35 from pdsinterop/fix/dpop
Browse files Browse the repository at this point in the history 
Fix dpop
  • Loading branch information
@poef
poef authored Sep 30, 2022
2 parents ea830b6 + 7787c5e commit e5453e0
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 41 deletions.
42 changes: 26 additions & 16 deletions src/TokenGenerator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

namespace Pdsinterop\Solid\Auth;

use Pdsinterop\Solid\Auth\Utils\Jwks;
use Pdsinterop\Solid\Auth\Exception\InvalidTokenException;
use Pdsinterop\Solid\Auth\Utils\DPop;
use Pdsinterop\Solid\Auth\Enum\OpenId\OpenIdConnectMetadata as OidcMeta;
use Laminas\Diactoros\Response\JsonResponse as JsonResponse;
use Laminas\Diactoros\Response\JsonResponse;
use League\OAuth2\Server\CryptTrait;

use DateTimeImmutable;
Expand All @@ -21,14 +22,17 @@ class TokenGenerator
public Config $config;

private \DateInterval $validFor;
private DPop $dpopUtil;

//////////////////////////////// PUBLIC API \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\

final public function __construct(
Config $config,
\DateInterval $validFor
\DateInterval $validFor,
DPop $dpopUtil,
) {
$this->config = $config;
$this->dpopUtil = $dpopUtil;
$this->validFor = $validFor;

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

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

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

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

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

$jkt = $this->makeJwkThumbprint($dpop);

$token = $jwtConfig->builder()
->issuedBy($issuer)
->permittedFor($clientId)
Expand All @@ -74,10 +79,8 @@ public function generateIdToken($accessToken, $clientId, $subject, $nonce, $priv
->withClaim("at_hash", $tokenHash) //FIXME: at_hash should only be added if the response_type is a token
->withClaim("c_hash", $tokenHash) // FIXME: c_hash should only be added if the response_type is a code
->withClaim("cnf", array(
"jkt" => $dpopKey,
// "jwk" => $jwks['keys'][0]
"jkt" => $jkt,
))
->withHeader('kid', $jwks['keys'][0]['kid'])
->getToken($jwtConfig->signer(), $jwtConfig->signingKey());
return $token->toString();
}
Expand All @@ -104,7 +107,7 @@ public function respondToRegistration($registration, $privateKey) {
return array_merge($registrationBase, $registration);
}

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

Expand All @@ -115,7 +118,7 @@ public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $pr
$subject,
$nonce,
$privateKey,
$dpopKey
$dpop
);
$value = preg_replace("/#access_token=(.*?)&/", "#access_token=\$1&id_token=$idToken&", $value);
$response = $response->withHeader("Location", $value);
Expand All @@ -126,7 +129,7 @@ public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $pr
$subject,
$nonce,
$privateKey,
$dpopKey
$dpop
);
$value = preg_replace("/code=(.*?)&/", "code=\$1&id_token=$idToken&", $value);
$response = $response->withHeader("Location", $value);
Expand All @@ -143,7 +146,7 @@ public function addIdTokenToResponse($response, $clientId, $subject, $nonce, $pr
$subject,
$nonce,
$privateKey,
$dpopKey
$dpop
);

$body['access_token'] = $body['id_token'];
Expand Down Expand Up @@ -178,9 +181,16 @@ private function generateTokenHash($accessToken) {
return $atHash;
}

private function getJwks() {
$key = $this->config->getKeys()->getPublicKey();
$jwks = new Jwks($key);
return json_decode($jwks->__toString(), true);
private function makeJwkThumbprint($dpop): string
{
$dpopConfig = Configuration::forUnsecuredSigner();
$parsedDpop = $dpopConfig->parser()->parse($dpop);
$jwk = $parsedDpop->headers()->get("jwk");

if (empty($jwk)) {
throw new InvalidTokenException('Required JWK header missing in DPOP');
}

return $this->dpopUtil->makeJwkThumbprint($jwk);
}
}
22 changes: 12 additions & 10 deletions src/Utils/DPop.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ public function makeJwkThumbprint($jwk) {
* See also: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop#section-7
*
* Validates the above part of the oauth dpop specification
*
* @param string $jwt JWT access token, raw
* @param string $dpop DPoP token, raw
* @param ServerRequestInterface $request Server Request
Expand All @@ -165,16 +166,17 @@ public function makeJwkThumbprint($jwk) {
public function validateJwtDpop($jwt, $dpop, $request) {
$this->validateDpop($dpop, $request);
$jwtConfig = Configuration::forUnsecuredSigner();
$dpopJWT = $jwtConfig->parser()->parse($dpop);

$ath = $dpopJWT->claims()->get('ath');
if ($ath === null) {
throw new InvalidTokenException('DPoP "ath" claim is missing');
}

$hash = hash('sha256', $jwt);
$encoded = Base64Url::encode($hash);
return ($ath === $encoded);
$jwtConfig->parser()->parse($dpop);

/**
* @FIXME: ATH claim is not yet supported/required by the Solid OIDC specification.
* Once the Solid spec catches up to the DPOP spec, not having an ATH is incorrect.
* At that point, instead of returning "true", throw an exception:
*
* @see https://github.com/pdsinterop/php-solid-auth/issues/34
*/
// throw new InvalidTokenException('DPoP "ath" claim is missing');
return true;
}

/**
Expand Down
63 changes: 49 additions & 14 deletions tests/unit/TokenGeneratorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Pdsinterop\Solid\Auth\Config\ServerInterface;
use Pdsinterop\Solid\Auth\Enum\OpenId\OpenIdConnectMetadata as OidcMeta;
use Pdsinterop\Solid\Auth\Utils\Base64Url;
use Pdsinterop\Solid\Auth\Utils\DPop;
use PHPUnit\Framework\MockObject\MockObject;

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

const MOCK_DPOP = "mock.dpop.value";
const MOCK_JKT = 'mock jkt';

private MockObject|Config $mockConfig;
private MockObject|KeysInterface $mockKeys;

private function createTokenGenerator($interval = null): TokenGenerator
private function createTokenGenerator($interval = null, $jkt = null): TokenGenerator
{
$this->mockConfig = $this->getMockBuilder(Config::class)
->disableOriginalConstructor()
Expand All @@ -51,7 +55,19 @@ private function createTokenGenerator($interval = null): TokenGenerator
->willReturn('mock encryption key')
;

return new TokenGenerator($this->mockConfig, $interval??$mockInterval);
$mockDpopUtil = $this->getMockBuilder(DPop::class)
->disableOriginalConstructor()
->getMock()
;

if ($jkt) {
$mockDpopUtil->expects($this->once())
->method('makeJwkThumbprint')
->willReturn($jkt)
;
}

return new TokenGenerator($this->mockConfig, $interval??$mockInterval, $mockDpopUtil);
}

/////////////////////////////////// TESTS \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
Expand Down Expand Up @@ -84,6 +100,27 @@ final public function testInstantiateWithoutValidFor(): void
new TokenGenerator($mockConfig);
}

/**
* @testdox Token Generator SHOULD complain WHEN instantiated without Dpop Utility
*
* @coversNothing
*/
final public function testInstantiateWithoutDpopUtility(): void
{
$this->expectArgumentCountError(3);

$mockConfig = $this->getMockBuilder(Config::class)
->disableOriginalConstructor()
->getMock();

$mockInterval = $this->getMockBuilder(\DateInterval::class)
->disableOriginalConstructor()
->getMock()
;

new TokenGenerator($mockConfig, $mockInterval);
}

/**
* @testdox Token Generator SHOULD be created WHEN instantiated with Config and validity period
*
Expand Down Expand Up @@ -271,14 +308,7 @@ final public function testIdTokenGeneration(): void
{
$validFor = new \DateInterval('PT1S');

$tokenGenerator = $this->createTokenGenerator($validFor);

$mockKey = \Lcobucci\JWT\Signer\Key\InMemory::file(__DIR__.'/../fixtures/keys/public.key');

$this->mockKeys->expects($this->once())
->method('getPublicKey')
->willReturn($mockKey)
;
$tokenGenerator = $this->createTokenGenerator($validFor, self::MOCK_JKT);

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

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

$encodedDpop = vsprintf("%s.%s.%s", [
'header' => Base64Url::encode('{"jwk":"mock jwk"}'),
'body' => Base64Url::encode('{}'),
'signature' => Base64Url::encode('mock signature')
]);

$actual = $tokenGenerator->generateIdToken(
'mock access token',
'mock clientId',
'mock subject',
'mock nonce',
$privateKey,
'mock dpop',
$encodedDpop,
$now
);

$this->assertJwtEquals([[
"alg"=>"RS256",
"kid"=>"0c3932ca20f3a00ad2eb72035f6cc9cb",
"typ"=>"JWT",
],[
'at_hash' => '1EZBnvsFWlK8ESkgHQsrIQ',
'aud' => 'mock clientId',
'azp' => 'mock clientId',
'c_hash' => '1EZBnvsFWlK8ESkgHQsrIQ',
'at_hash' => '1EZBnvsFWlK8ESkgHQsrIQ',
'cnf' => ["jkt" => "mock dpop"],
'cnf' => ["jkt" => self::MOCK_JKT],
'exp' => -23225829903.789,
'iat' => -23225829904.789,
'iss' => 'mock issuer',
Expand Down
35 changes: 34 additions & 1 deletion tests/unit/Utils/DPOPTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,36 @@ final public function testGetWebIdWithDpopWithoutSub(): void
$dpop->getWebId($request);
}

/**
* @testdox Dpop SHOULD not complain WHEN asked to get WebId from Request with valid DPOP without "ath"
*
* @covers ::getWebId
*
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::getDpopKey
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
*/
final public function testGetWebIdWithDpopWithoutOptionalAth(): void
{
unset($this->dpop['payload']['ath']);
$token = $this->sign($this->dpop);

$mockJtiValidator = $this->createMockJtiValidator();
$mockJtiValidator->expects($this->once())
->method('validate')
->willReturn(true)
;
$dpop = new DPop($mockJtiValidator);

$request = new ServerRequest(array(
'HTTP_AUTHORIZATION' => "dpop {$this->accessToken['token']}",
'HTTP_DPOP' => $token['token'],
),array(), $this->url);

$webId = $dpop->getWebId($request);

$this->assertEquals(self::MOCK_SUBJECT, $webId);
}
/**
* @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without "ath"
*
Expand All @@ -526,8 +556,11 @@ final public function testGetWebIdWithDpopWithoutSub(): void
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateDpop
* @uses \Pdsinterop\Solid\Auth\Utils\DPop::validateJwtDpop
*/
final public function testGetWebIdWithDpopWithoutAth(): void
final public function testGetWebIdWithDpopWithoutRequiredAth(): void
{
/*/ @see https://github.com/pdsinterop/php-solid-auth/issues/34 /*/
$this->markTestSkipped('ATH claim is not yet supported/required by the Solid OIDC specification.');

unset($this->dpop['payload']['ath']);
$token = $this->sign($this->dpop);

Expand Down

0 comments on commit e5453e0

Please sign in to comment.