From a37e7ec809db7842bcac4bb466395ff698fd4fea Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Thu, 26 Nov 2020 10:29:27 +0100 Subject: [PATCH 1/3] Remove RegisteredClaimGiven exception in favour of deprecation message for withClaim This PR is proposed as per the discussion in #559. --- src/Builder.php | 6 ++++- test/unit/BuilderTest.php | 52 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/Builder.php b/src/Builder.php index 379ac308..abd7c092 100644 --- a/src/Builder.php +++ b/src/Builder.php @@ -412,7 +412,11 @@ private function configureClaim($name, $value) public function withClaim($name, $value) { if (in_array($name, RegisteredClaims::ALL, true)) { - throw RegisteredClaimGiven::forClaim($name); + trigger_error('The use of the method "withClaim" is deprecated for registered claims. Please use dedicated method instead.', E_USER_DEPRECATED); + } + + if ($name === RegisteredClaims::AUDIENCE) { + return $this->permittedFor($value); } return $this->configureClaim($name, $value); diff --git a/test/unit/BuilderTest.php b/test/unit/BuilderTest.php index 8c501fa2..cdd305ab 100644 --- a/test/unit/BuilderTest.php +++ b/test/unit/BuilderTest.php @@ -564,15 +564,61 @@ public function withClaimMustKeepAFluentInterface() /** * @test * + * @param string $name + * @param mixed $value + * @param mixed $expected + * * @covers ::__construct * @covers ::withClaim * @covers \Lcobucci\JWT\Token\RegisteredClaimGiven + * + * @dataProvider dataWithClaimDeprecationNotice */ - public function withClaimShouldThrowExceptionWhenTryingToConfigureARegisteredClaim() + public function withClaimShouldSendDeprecationNoticeWhenTryingToConfigureARegisteredClaim($name, $value, $expected) + { + $key = $this->createMock(Key::class); + $signature = $this->createMock(Signature::class); + $signature + ->expects(static::once()) + ->method('hash') + ->willReturn('--hash--') + ; + + $signer = $this->createMock(Signer::class); + $signer + ->expects(static::once()) + ->method('sign') + ->willReturn($signature) + ; + + + + $this->expectDeprecation('The use of the method "withClaim" is deprecated for registered claims. Please use dedicated method instead.'); + + $token = $this + ->createBuilder() + ->withClaim($name, $value) + ->getToken($signer, $key) + ; + + self::assertEquals($expected, $token->claims()->get($name)); + } + + public function dataWithClaimDeprecationNotice() { - $this->expectException(RegisteredClaimGiven::class); + $now = time(); + $nowAsDate = new DateTimeImmutable('@' . $now); + $nowPlus1HourAsDate = $nowAsDate->modify('+1 hour'); - $this->createBuilder()->withClaim('sub', 'me'); + return [ + ['sub', 'me', 'me'], + ['aud', 'him', ['him']], + ['jti', '0123456789ABCDEF', '0123456789ABCDEF'], + ['iss', 'you', 'you'], + ['exp', $nowPlus1HourAsDate->getTimestamp(), $nowPlus1HourAsDate->getTimestamp()], + ['iat', $now, $nowAsDate->getTimestamp()], + ['nbf', $now, $nowAsDate->getTimestamp()], + ]; } /** From 317e4c9b484dfb582912cf3772f6ee2511825bc3 Mon Sep 17 00:00:00 2001 From: Spomky Date: Thu, 26 Nov 2020 14:06:57 +0100 Subject: [PATCH 2/3] Use of the new method for all registered claims --- src/Builder.php | 24 +++++++++++++++--- test/unit/BuilderTest.php | 53 ++++++++++++++++++++------------------- 2 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/Builder.php b/src/Builder.php index abd7c092..6a5e9dc0 100644 --- a/src/Builder.php +++ b/src/Builder.php @@ -415,11 +415,27 @@ public function withClaim($name, $value) trigger_error('The use of the method "withClaim" is deprecated for registered claims. Please use dedicated method instead.', E_USER_DEPRECATED); } - if ($name === RegisteredClaims::AUDIENCE) { - return $this->permittedFor($value); - } + return $this->forwardCallToCorrectClaimMethod($name, $value); + } - return $this->configureClaim($name, $value); + private function forwardCallToCorrectClaimMethod($name, $value) + { + switch ($name) { + case RegisteredClaims::ID: + return $this->identifiedBy($value); + case RegisteredClaims::EXPIRATION_TIME: + return $this->expiresAt($value); + case RegisteredClaims::NOT_BEFORE: + return $this->canOnlyBeUsedAfter($value); + case RegisteredClaims::ISSUED_AT: + return $this->issuedAt($value); + case RegisteredClaims::ISSUER: + return $this->issuedBy($value); + case RegisteredClaims::AUDIENCE: + return $this->permittedFor($value); + default: + return $this->configureClaim($name, $value); + } } /** diff --git a/test/unit/BuilderTest.php b/test/unit/BuilderTest.php index cdd305ab..e6d98a86 100644 --- a/test/unit/BuilderTest.php +++ b/test/unit/BuilderTest.php @@ -533,6 +533,7 @@ public function relatedToMustKeepAFluentInterface() * @covers ::configureClaim * @covers ::createSignature * @covers ::convertItems + * @covers ::forwardCallToCorrectClaimMethod * * @uses \Lcobucci\JWT\Builder::getToken */ @@ -553,6 +554,7 @@ public function withClaimMustConfigureTheGivenClaim() * @covers ::__construct * @covers ::withClaim * @covers ::configureClaim + * @covers ::forwardCallToCorrectClaimMethod */ public function withClaimMustKeepAFluentInterface() { @@ -567,39 +569,37 @@ public function withClaimMustKeepAFluentInterface() * @param string $name * @param mixed $value * @param mixed $expected + * @param null|string $otherMessage * * @covers ::__construct * @covers ::withClaim - * @covers \Lcobucci\JWT\Token\RegisteredClaimGiven + * @covers ::canOnlyBeUsedAfter + * @covers ::configureClaim + * @covers ::convertItems + * @covers ::convertToDate + * @covers ::getToken + * @covers ::setRegisteredClaim + * @covers ::createSignature + * @covers ::expiresAt + * @covers ::issuedBy + * @covers ::identifiedBy + * @covers ::permittedFor + * @covers ::forwardCallToCorrectClaimMethod + * @covers ::issuedAt * * @dataProvider dataWithClaimDeprecationNotice */ - public function withClaimShouldSendDeprecationNoticeWhenTryingToConfigureARegisteredClaim($name, $value, $expected) + public function withClaimShouldSendDeprecationNoticeWhenTryingToConfigureARegisteredClaim($name, $value, $expected, $otherMessage = null) { - $key = $this->createMock(Key::class); - $signature = $this->createMock(Signature::class); - $signature - ->expects(static::once()) - ->method('hash') - ->willReturn('--hash--') - ; - - $signer = $this->createMock(Signer::class); - $signer - ->expects(static::once()) - ->method('sign') - ->willReturn($signature) - ; - - - $this->expectDeprecation('The use of the method "withClaim" is deprecated for registered claims. Please use dedicated method instead.'); - $token = $this - ->createBuilder() + if ($otherMessage) { + $this->expectDeprecation($otherMessage); + } + + $token = $this->createBuilder() ->withClaim($name, $value) - ->getToken($signer, $key) - ; + ->getToken(new None(), Key\InMemory::plainText('')); self::assertEquals($expected, $token->claims()->get($name)); } @@ -615,9 +615,9 @@ public function dataWithClaimDeprecationNotice() ['aud', 'him', ['him']], ['jti', '0123456789ABCDEF', '0123456789ABCDEF'], ['iss', 'you', 'you'], - ['exp', $nowPlus1HourAsDate->getTimestamp(), $nowPlus1HourAsDate->getTimestamp()], - ['iat', $now, $nowAsDate->getTimestamp()], - ['nbf', $now, $nowAsDate->getTimestamp()], + ['exp', $nowPlus1HourAsDate->getTimestamp(), $nowPlus1HourAsDate, 'Using integers for registered date claims is deprecated, please use DateTimeImmutable objects instead.'], + ['iat', $now, $nowAsDate, 'Using integers for registered date claims is deprecated, please use DateTimeImmutable objects instead.'], + ['nbf', $now, $nowAsDate, 'Using integers for registered date claims is deprecated, please use DateTimeImmutable objects instead.'], ]; } @@ -733,6 +733,7 @@ public function unsignMustKeepAFluentInterface(Builder $builder) * @uses \Lcobucci\JWT\Builder::__construct * @uses \Lcobucci\JWT\Builder::configureClaim * @uses \Lcobucci\JWT\Builder::withClaim + * @uses \Lcobucci\JWT\Builder::forwardCallToCorrectClaimMethod */ public function getTokenMustReturnANewTokenWithCurrentConfiguration() { From 0542295ec687a4cf867a3c96ddaa69216fb085f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Fri, 27 Nov 2020 02:12:06 +0100 Subject: [PATCH 3/3] Apply type guards also on old `Builder#set()` --- src/Builder.php | 2 +- test/unit/BuilderTest.php | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Builder.php b/src/Builder.php index 6a5e9dc0..9f0a5bd9 100644 --- a/src/Builder.php +++ b/src/Builder.php @@ -451,7 +451,7 @@ private function forwardCallToCorrectClaimMethod($name, $value) */ public function set($name, $value) { - return $this->configureClaim($name, $value); + return $this->forwardCallToCorrectClaimMethod($name, $value); } /** diff --git a/test/unit/BuilderTest.php b/test/unit/BuilderTest.php index e6d98a86..54789634 100644 --- a/test/unit/BuilderTest.php +++ b/test/unit/BuilderTest.php @@ -604,6 +604,46 @@ public function withClaimShouldSendDeprecationNoticeWhenTryingToConfigureARegist self::assertEquals($expected, $token->claims()->get($name)); } + + /** + * @test + * + * @param string $name + * @param mixed $value + * @param mixed $expected + * @param null|string $otherMessage + * + * @covers ::__construct + * @covers ::set + * @covers ::canOnlyBeUsedAfter + * @covers ::configureClaim + * @covers ::convertItems + * @covers ::convertToDate + * @covers ::getToken + * @covers ::setRegisteredClaim + * @covers ::createSignature + * @covers ::expiresAt + * @covers ::issuedBy + * @covers ::identifiedBy + * @covers ::permittedFor + * @covers ::forwardCallToCorrectClaimMethod + * @covers ::issuedAt + * + * @dataProvider dataWithClaimDeprecationNotice + */ + public function setShouldSendDeprecationNoticeWhenTryingToConfigureARegisteredClaim($name, $value, $expected, $otherMessage = null) + { + if ($otherMessage) { + $this->expectDeprecation($otherMessage); + } + + $token = $this->createBuilder() + ->set($name, $value) + ->getToken(new None(), Key\InMemory::plainText('')); + + self::assertEquals($expected, $token->claims()->get($name)); + } + public function dataWithClaimDeprecationNotice() { $now = time();