From e4525f2e1b182be970cf85beaed84c8ccdc46cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Hal=C3=A1sz?= Date: Tue, 24 May 2022 11:53:44 +0200 Subject: [PATCH 1/2] feat: added aud claim validation --- CHANGELOG.md | 5 ++ src/Service/Client/LtiServiceClient.php | 9 ++++ tests/Traits/DomainTestingTrait.php | 54 +++++++++++++++++++ .../Service/Client/LtiServiceClientTest.php | 20 +++++++ 4 files changed, 88 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c74d963..bcc0780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +Unreleased +----- + +* Added AUD claim validation in `LtiServiceClient` + 6.7.1 ----- diff --git a/src/Service/Client/LtiServiceClient.php b/src/Service/Client/LtiServiceClient.php index ec3e3ae..18f07dc 100644 --- a/src/Service/Client/LtiServiceClient.php +++ b/src/Service/Client/LtiServiceClient.php @@ -25,6 +25,7 @@ use GuzzleHttp\Client; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\ClientException; +use InvalidArgumentException; use OAT\Library\Lti1p3Core\Exception\LtiException; use OAT\Library\Lti1p3Core\Exception\LtiExceptionInterface; use OAT\Library\Lti1p3Core\Message\Payload\MessagePayloadInterface; @@ -197,6 +198,14 @@ private function generateCredentials(RegistrationInterface $registration): strin throw new LtiException('Tool key chain is not configured'); } + if ($registration->getPlatform()->getAudience() === '') { + throw new InvalidArgumentException('Platform audience cannot be null'); + } + + if ($registration->getPlatform()->getOAuth2AccessTokenUrl() === null) { + throw new InvalidArgumentException('Platform OAuth2 access token url cannot be null'); + } + $token = $this->builder->build( [ MessagePayloadInterface::HEADER_KID => $toolKeyChain->getIdentifier() diff --git a/tests/Traits/DomainTestingTrait.php b/tests/Traits/DomainTestingTrait.php index 8b52cb7..56aab4c 100644 --- a/tests/Traits/DomainTestingTrait.php +++ b/tests/Traits/DomainTestingTrait.php @@ -197,6 +197,60 @@ private function createTestRegistrationWithoutToolLaunchUrl( ); } + private function createTestRegistrationWithoutPlatformAudience( + string $identifier = 'registrationIdentifier', + string $clientId = 'registrationClientId', + string $platformJwksUrl = 'http://platform.com/jwks', + string $toolJwksUrl = 'http://tool.com/jwks' + ): Registration { + $tool = $this->createTestTool(); + + return new Registration( + $identifier, + $clientId, + new Platform( + 'platformIdentifier', + 'platformName', + '', + 'http://platform.com/oidc-auth', + 'http://platform.com/access-token' + ), + $this->createTestTool(), + ['deploymentIdentifier'], + $this->createTestKeyChain('platformKeyChain'), + $this->createTestKeyChain('toolKeyChain'), + $platformJwksUrl, + $toolJwksUrl + ); + } + + private function createTestRegistrationWithoutPlatformOAuth2AccessTokenUrl( + string $identifier = 'registrationIdentifier', + string $clientId = 'registrationClientId', + string $platformJwksUrl = 'http://platform.com/jwks', + string $toolJwksUrl = 'http://tool.com/jwks' + ): Registration { + $tool = $this->createTestTool(); + + return new Registration( + $identifier, + $clientId, + new Platform( + 'platformIdentifier', + 'platformName', + 'platformAudience', + 'http://platform.com/oidc-auth', + null + ), + $this->createTestTool(), + ['deploymentIdentifier'], + $this->createTestKeyChain('platformKeyChain'), + $this->createTestKeyChain('toolKeyChain'), + $platformJwksUrl, + $toolJwksUrl + ); + } + private function createTestRegistrationRepository(array $registrations = []): RegistrationRepositoryInterface { $registrations = !empty($registrations) diff --git a/tests/Unit/Service/Client/LtiServiceClientTest.php b/tests/Unit/Service/Client/LtiServiceClientTest.php index 17069da..1759618 100644 --- a/tests/Unit/Service/Client/LtiServiceClientTest.php +++ b/tests/Unit/Service/Client/LtiServiceClientTest.php @@ -519,6 +519,26 @@ public function testItThrowAnLtiExceptionOnPlatformEndpointFailureAfterAutoRetry $this->subject->request($this->registration, 'GET', 'http://example.com', [], $scopes); } + public function testItThrowsAnLtiExceptionOnNullPlatformAudience(): void + { + $this->expectException(LtiException::class); + $this->expectExceptionMessage('Cannot generate credentials: Platform audience cannot be null'); + + $this->registration = $this->createTestRegistrationWithoutPlatformAudience(); + + $this->subject->request($this->registration, 'GET', 'http://example.com'); + } + + public function testItThrowsAnLtiExceptionOnNullPlatformOAuth2AccessTokenUrl(): void + { + $this->expectException(LtiException::class); + $this->expectExceptionMessage('Cannot generate credentials: Platform OAuth2 access token url cannot be null'); + + $this->registration = $this->createTestRegistrationWithoutPlatformOAuth2AccessTokenUrl(); + + $this->subject->request($this->registration, 'GET', 'http://example.com'); + } + private function generateAccessTokenCacheKey(RegistrationInterface $registration, array $scopes = []): string { return sprintf( From 99646a8a31e9dee3bf4f12e6c0d7e32a73d8e764 Mon Sep 17 00:00:00 2001 From: Sergei Mikhailov Date: Tue, 13 Sep 2022 16:34:25 +0200 Subject: [PATCH 2/2] fix: allow empty validation keys to be set into security configuration --- CHANGELOG.md | 5 +++-- src/Security/Jwt/Configuration/ConfigurationFactory.php | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bcc0780..53082e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,11 @@ CHANGELOG ========= -Unreleased +6.7.2 ----- -* Added AUD claim validation in `LtiServiceClient` +* Fixed AUD claim validation in `LtiServiceClient` +* Fixed empty validation key assignment to the security configuration 6.7.1 ----- diff --git a/src/Security/Jwt/Configuration/ConfigurationFactory.php b/src/Security/Jwt/Configuration/ConfigurationFactory.php index 4f2a0b9..1e936cf 100644 --- a/src/Security/Jwt/Configuration/ConfigurationFactory.php +++ b/src/Security/Jwt/Configuration/ConfigurationFactory.php @@ -85,7 +85,9 @@ private function findAlgorithm(?KeyInterface $signingKey = null, ?KeyInterface $ private function convertKey(?KeyInterface $key = null): Key { if (null === $key) { - return InMemory::plainText(''); + return method_exists(InMemory::class, 'empty') + ? InMemory::empty() + : InMemory::plainText(''); } return $this->converter->convert($key);