From bc9e227fe6e734094f7dcdca3b492fedbf62c8a1 Mon Sep 17 00:00:00 2001
From: Auke van Slooten <auke@muze.nl>
Date: Thu, 29 Sep 2022 14:39:36 +0200
Subject: [PATCH] improved isset() calls fixed @returns at validateDpop add
 $this->markTestSkipped for 3 tests that are probably incorrect

---
 src/Utils/DPop.php            | 13 +++++++------
 tests/unit/Utils/DPOPTest.php | 15 +++++----------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/Utils/DPop.php b/src/Utils/DPop.php
index 59b7071..ffcc95e 100644
--- a/src/Utils/DPop.php
+++ b/src/Utils/DPop.php
@@ -114,11 +114,13 @@ public function makeJwkThumbprint($jwk) {
 		if (!$jwk || !isset($jwk['kty'])) {
 			throw new InvalidTokenException('JWK has no "kty" key type');
 		}
+		// https://www.rfc-editor.org/rfc/rfc7517.html#section-4.1
+		// and https://www.rfc-editor.org/rfc/rfc7518.html#section-6.1
 		if (!in_array($jwk['kty'], ['RSA','EC'])) {
 			throw new InvalidTokenException('JWK "kty" key type value must be one of "RSA" or "EC", got "'.$jwk['kty'].'" instead.');
 		}
-		if ($jwk['kty']=='RSA') {
-			if (!isset($jwk['e']) || !isset($jwk['n'])) {
+		if ($jwk['kty']=='RSA') { // used with RS256 alg
+			if (!isset($jwk['e'], $jwk['n'])) {
 				throw new InvalidTokenException('JWK values do not match "RSA" key type');
 			}
 			$json = vsprintf('{"e":"%s","kty":"%s","n":"%s"}', [
@@ -126,9 +128,8 @@ public function makeJwkThumbprint($jwk) {
 				$jwk['kty'],
 				$jwk['n'],
 			]);
-
-		} else { //EC
-			if (!isset($jwk['crv']) || !isset($jwk['x']) || !isset($jwk['y'])) {
+		} else { // EC used with ES256 alg 
+			if (!isset($jwk['crv'], $jwk['x'], $jwk['y'])) {
 				throw new InvalidTokenException('JWK values doe not match "EC" key type');
 			}
 			//crv, kty, x, y
@@ -219,7 +220,7 @@ public function validateIdTokenDpop($token, $dpop, $request) {
 	 * @param string $dpop The DPOP token
 	 * @param ServerRequestInterface $request Server Request
 	 *
-	 * @return bool True if the DPOP token is valid, false otherwise
+	 * @return bool True if the DPOP token is valid
 	 *
 	 * @throws RequiredConstraintsViolated
 	 * @throws InvalidTokenException
diff --git a/tests/unit/Utils/DPOPTest.php b/tests/unit/Utils/DPOPTest.php
index 34fd27f..132a7c0 100644
--- a/tests/unit/Utils/DPOPTest.php
+++ b/tests/unit/Utils/DPOPTest.php
@@ -349,7 +349,6 @@ final public function testGetWebIdWithDpopWithoutKeyId(): void
         $this->dpop['payload']['jti'] = 'mock jti';
         $this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
 
-
         $dpopToken = $this->sign($this->dpop);
 
         $mockJtiValidator = $this->createMockJtiValidator();
@@ -374,16 +373,15 @@ final public function testGetWebIdWithDpopWithoutKeyId(): void
 
     /**
      * @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without Confirmation Claim
-     * why? which spec?
      * @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 testGetWebIdWithDpopWithoutConfirmationClaim(): void
     {
+        $this->markTestSkipped('Skipped untill we find a spec that requires this');
         $this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
         $this->dpop['payload']['jti'] = 'mock jti';
         $this->dpop['payload']['sub'] = self::MOCK_SUBJECT;
@@ -409,19 +407,18 @@ final public function testGetWebIdWithDpopWithoutConfirmationClaim(): void
 
         $dpop->getWebId($request);
     }
-*/
+
     /**
      * @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without JWT Key Thumbprint
-     * why? which spec?
      * @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 testGetWebIdWithDpopWithoutThumbprint(): void
     {
+        $this->markTestSkipped('Skipped untill we find a spec that requires this');
         $this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT;
         $this->dpop['payload']['cnf'] = [];
         $this->dpop['payload']['jti'] = 'mock jti';
@@ -446,19 +443,18 @@ final public function testGetWebIdWithDpopWithoutThumbprint(): void
 
         $dpop->getWebId($request);
     }
-*/
+
     /**
      * @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP with Thumbprint not matching Key Id
-     * why? which spec?
      * @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 testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): void
     {
+        $this->markTestSkipped('Skipped untill we find a spec that requires this');
         $this->dpop['header']['jwk'][JwkParameter::KEY_ID] = self::MOCK_THUMBPRINT . 'Mismatch';
         $this->dpop['payload']['cnf'] = ['jkt' => self::MOCK_THUMBPRINT];
         $this->dpop['payload']['jti'] = 'mock jti';
@@ -483,7 +479,6 @@ final public function testGetWebIdWithDpopWithMismatchingThumbprintAndKeyId(): v
 
         $dpop->getWebId($request);
     }
-*/
 
     /**
      * @testdox Dpop SHOULD complain WHEN asked to get WebId from Request with valid DPOP without "sub"