From 9016075ac1b94b5e1709a42f44fde1e32b574a3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Thu, 9 Jan 2025 17:19:27 +0100 Subject: [PATCH] Add coverage --- src/Federation/EntityStatement.php | 19 ++ src/Federation/EntityStatementFetcher.php | 15 +- src/Jws/AbstractJwsFetcher.php | 2 +- src/Jws/JwsFetcher.php | 2 +- src/Utils/ArtifactFetcher.php | 2 +- .../Federation/EntityStatementFetcherTest.php | 71 +++++++- tests/src/Jws/JwsFetcherTest.php | 169 ++++++++++++++++++ tests/src/Utils/ArtifactFetcherTest.php | 159 ++++++++++++++++ 8 files changed, 424 insertions(+), 15 deletions(-) create mode 100644 tests/src/Jws/JwsFetcherTest.php diff --git a/src/Federation/EntityStatement.php b/src/Federation/EntityStatement.php index b3d885c..a8b7e36 100644 --- a/src/Federation/EntityStatement.php +++ b/src/Federation/EntityStatement.php @@ -5,6 +5,7 @@ namespace SimpleSAML\OpenID\Federation; use SimpleSAML\OpenID\Codebooks\ClaimsEnum; +use SimpleSAML\OpenID\Codebooks\EntityTypesEnum; use SimpleSAML\OpenID\Codebooks\JwtTypesEnum; use SimpleSAML\OpenID\Decorators\DateIntervalDecorator; use SimpleSAML\OpenID\Exceptions\EntityStatementException; @@ -192,6 +193,24 @@ public function getKeyId(): string return parent::getKeyId() ?? throw new EntityStatementException('No KeyId header claim found.'); } + /** + * @throws \SimpleSAML\OpenID\Exceptions\JwsException + */ + public function getFederationFetchEndpoint(): ?string + { + /** @psalm-suppress MixedAssignment */ + $federationFetchEndpoint = $this->getPayload() + [ClaimsEnum::Metadata->value] + [EntityTypesEnum::FederationEntity->value] + [ClaimsEnum::FederationFetchEndpoint->value] ?? null; + + if (is_null($federationFetchEndpoint)) { + return null; + } + + return (string)$federationFetchEndpoint; + } + /** * @throws \SimpleSAML\OpenID\Exceptions\EntityStatementException * @throws \SimpleSAML\OpenID\Exceptions\JwsException diff --git a/src/Federation/EntityStatementFetcher.php b/src/Federation/EntityStatementFetcher.php index e298ab1..650e0e2 100644 --- a/src/Federation/EntityStatementFetcher.php +++ b/src/Federation/EntityStatementFetcher.php @@ -10,6 +10,7 @@ use SimpleSAML\OpenID\Codebooks\EntityTypesEnum; use SimpleSAML\OpenID\Codebooks\WellKnownEnum; use SimpleSAML\OpenID\Decorators\DateIntervalDecorator; +use SimpleSAML\OpenID\Exceptions\EntityStatementException; use SimpleSAML\OpenID\Exceptions\FetchException; use SimpleSAML\OpenID\Exceptions\JwsException; use SimpleSAML\OpenID\Federation\Factories\EntityStatementFactory; @@ -34,7 +35,7 @@ protected function buildJwsInstance(string $token): EntityStatement return $this->parsedJwsFactory->fromToken($token); } - protected function getExpectedContentTypeHttpHeader(): string + public function getExpectedContentTypeHttpHeader(): string { return ContentTypesEnum::ApplicationEntityStatementJwt->value; } @@ -74,12 +75,8 @@ public function fromCacheOrFetchEndpoint( string $subjectId, EntityStatement $entityConfiguration, ): EntityStatement { - $entityConfigurationPayload = $entityConfiguration->getPayload(); - - $fetchEndpointUri = (string)($entityConfigurationPayload[ClaimsEnum::Metadata->value] - [EntityTypesEnum::FederationEntity->value] - [ClaimsEnum::FederationFetchEndpoint->value] ?? - throw new JwsException('No fetch endpoint found in entity configuration.')); + $fetchEndpointUri = $entityConfiguration->getFederationFetchEndpoint() ?? + throw new EntityStatementException('No fetch endpoint found in entity configuration.'); $this->logger?->debug( 'Entity statement fetch from cache or fetch endpoint.', @@ -125,6 +122,7 @@ public function fromCache(string $uri): ?EntityStatement return $entityStatement; } + // @codeCoverageIgnoreStart $message = 'Unexpected entity statement instance encountered for cache fetch.'; $this->logger?->error( $message, @@ -132,6 +130,7 @@ public function fromCache(string $uri): ?EntityStatement ); throw new FetchException($message); + // @codeCoverageIgnoreEnd } /** @@ -148,6 +147,7 @@ public function fromNetwork(string $uri): EntityStatement return $entityStatement; } + // @codeCoverageIgnoreStart $message = 'Unexpected entity statement instance encountered for network fetch.'; $this->logger?->error( $message, @@ -155,5 +155,6 @@ public function fromNetwork(string $uri): EntityStatement ); throw new FetchException($message); + // @codeCoverageIgnoreEnd } } diff --git a/src/Jws/AbstractJwsFetcher.php b/src/Jws/AbstractJwsFetcher.php index 8df727a..8464983 100644 --- a/src/Jws/AbstractJwsFetcher.php +++ b/src/Jws/AbstractJwsFetcher.php @@ -24,5 +24,5 @@ public function __construct( * @throws \SimpleSAML\OpenID\Exceptions\JwsException */ abstract protected function buildJwsInstance(string $token): ParsedJws; - abstract protected function getExpectedContentTypeHttpHeader(): ?string; + abstract public function getExpectedContentTypeHttpHeader(): ?string; } diff --git a/src/Jws/JwsFetcher.php b/src/Jws/JwsFetcher.php index 423b657..1ca2cbc 100644 --- a/src/Jws/JwsFetcher.php +++ b/src/Jws/JwsFetcher.php @@ -32,7 +32,7 @@ protected function buildJwsInstance(string $token): ParsedJws return $this->parsedJwsFactory->fromToken($token); } - protected function getExpectedContentTypeHttpHeader(): ?string + public function getExpectedContentTypeHttpHeader(): ?string { return null; } diff --git a/src/Utils/ArtifactFetcher.php b/src/Utils/ArtifactFetcher.php index 6f57118..96eb059 100644 --- a/src/Utils/ArtifactFetcher.php +++ b/src/Utils/ArtifactFetcher.php @@ -11,7 +11,6 @@ use SimpleSAML\OpenID\Decorators\CacheDecorator; use SimpleSAML\OpenID\Decorators\HttpClientDecorator; use SimpleSAML\OpenID\Exceptions\FetchException; -use SimpleSAML\OpenID\Exceptions\HttpException; use Throwable; class ArtifactFetcher @@ -49,6 +48,7 @@ public function fromCacheAsString(string $keyElement, string ...$keyElements): ? 'Artifact not found in cache.', compact('keyElement', 'keyElements'), ); + return null; } if (is_string($artifact)) { diff --git a/tests/src/Federation/EntityStatementFetcherTest.php b/tests/src/Federation/EntityStatementFetcherTest.php index c155b89..86a5862 100644 --- a/tests/src/Federation/EntityStatementFetcherTest.php +++ b/tests/src/Federation/EntityStatementFetcherTest.php @@ -8,8 +8,13 @@ use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; +use SimpleSAML\OpenID\Codebooks\ContentTypesEnum; +use SimpleSAML\OpenID\Codebooks\WellKnownEnum; use SimpleSAML\OpenID\Decorators\DateIntervalDecorator; +use SimpleSAML\OpenID\Exceptions\EntityStatementException; +use SimpleSAML\OpenID\Exceptions\FetchException; use SimpleSAML\OpenID\Federation\EntityStatement; use SimpleSAML\OpenID\Federation\EntityStatementFetcher; use SimpleSAML\OpenID\Federation\Factories\EntityStatementFactory; @@ -21,6 +26,7 @@ #[CoversClass(EntityStatementFetcher::class)] #[UsesClass(AbstractJwsFetcher::class)] #[UsesClass(JwsFetcher::class)] +#[UsesClass(WellKnownEnum::class)] class EntityStatementFetcherTest extends TestCase { protected MockObject $entityStatementFactoryMock; @@ -28,6 +34,8 @@ class EntityStatementFetcherTest extends TestCase protected MockObject $maxCacheDurationMock; protected MockObject $helpersMock; protected MockObject $loggerMock; + protected MockObject $responseMock; + protected MockObject $entityStatementMock; protected function setUp(): void { @@ -36,6 +44,11 @@ protected function setUp(): void $this->maxCacheDurationMock = $this->createMock(DateIntervalDecorator::class); $this->helpersMock = $this->createMock(Helpers::class); $this->loggerMock = $this->createMock(LoggerInterface::class); + + $this->responseMock = $this->createMock(ResponseInterface::class); + $this->artifactFetcherMock->method('fromNetwork')->willReturn($this->responseMock); + + $this->entityStatementMock = $this->createMock(EntityStatement::class); } protected function sut( @@ -65,20 +78,68 @@ public function testCanCreateInstance(): void $this->assertInstanceOf(EntityStatementFetcher::class, $this->sut()); } - public function testCanFetchFromCache(): void + public function testHasRightExpectedContentTypeHttpHeader(): void + { + $this->assertSame( + ContentTypesEnum::ApplicationEntityStatementJwt->value, + $this->sut()->getExpectedContentTypeHttpHeader(), + ); + } + + public function testCanFetchFromCacheOrWellKnownEndpoint(): void { $this->artifactFetcherMock->expects($this->once())->method('fromCacheAsString') - ->with('uri') + ->willReturn(null); + + $this->responseMock->method('getStatusCode')->willReturn(200); + $this->responseMock->method('getHeaderLine') + ->willReturn(ContentTypesEnum::ApplicationEntityStatementJwt->value); + + $this->artifactFetcherMock->expects($this->once())->method('fromNetwork') + ->willReturn($this->responseMock); + + $this->assertInstanceOf( + EntityStatement::class, + $this->sut()->fromCacheOrWellKnownEndpoint('entityId'), + ); + } + + public function testCanFetchFromCacheOrFetchEndpoint(): void + { + $this->entityStatementMock->expects($this->once()) + ->method('getFederationFetchEndpoint') + ->willReturn('fetch-uri'); + + $this->artifactFetcherMock->expects($this->once())->method('fromCacheAsString') ->willReturn('token'); $this->entityStatementFactoryMock->expects($this->once())->method('fromToken') ->with('token'); - $this->assertInstanceOf(EntityStatement::class, $this->sut()->fromCache('uri')); + $this->sut()->fromCacheOrFetchEndpoint('entityId', $this->entityStatementMock); + } + + public function testFetchFromCacheOrFetchEndpointThrowsIfNoFetchEndpoint(): void + { + $this->entityStatementMock->expects($this->once()) + ->method('getFederationFetchEndpoint') + ->willReturn(null); + + $this->expectException(EntityStatementException::class); + $this->expectExceptionMessage('fetch'); + + $this->sut()->fromCacheOrFetchEndpoint('entityId', $this->entityStatementMock); } - public function testFetchFromCacheReturnsNull(): void + public function testCanFetchFromCache(): void { - $this->markTestIncomplete('TODO mivanci'); + $this->artifactFetcherMock->expects($this->once())->method('fromCacheAsString') + ->with('uri') + ->willReturn('token'); + + $this->entityStatementFactoryMock->expects($this->once())->method('fromToken') + ->with('token'); + + $this->assertInstanceOf(EntityStatement::class, $this->sut()->fromCache('uri')); } } diff --git a/tests/src/Jws/JwsFetcherTest.php b/tests/src/Jws/JwsFetcherTest.php new file mode 100644 index 0000000..b716d80 --- /dev/null +++ b/tests/src/Jws/JwsFetcherTest.php @@ -0,0 +1,169 @@ +parsedJwsFactoryMock = $this->createMock(ParsedJwsFactory::class); + $this->artifactFetcherMock = $this->createMock(ArtifactFetcher::class); + $this->maxCacheDurationMock = $this->createMock(DateIntervalDecorator::class); + $this->helpersMock = $this->createMock(Helpers::class); + $this->loggerMock = $this->createMock(LoggerInterface::class); + + $this->responseMock = $this->createMock(ResponseInterface::class); + $this->responseBodyMock = $this->createMock(StreamInterface::class); + $this->responseMock->method('getBody')->willReturn($this->responseBodyMock); + $this->artifactFetcherMock->method('fromNetwork')->willReturn($this->responseMock); + + $this->parsedJwsMock = $this->createMock(ParsedJws::class); + } + + protected function sut( + ?ParsedJwsFactory $parsedJwsFactory = null, + ?ArtifactFetcher $artifactFetcher = null, + ?DateIntervalDecorator $maxCacheDuration = null, + ?Helpers $helpers = null, + ?LoggerInterface $logger = null, + ): JwsFetcher { + $parsedJwsFactory ??= $this->parsedJwsFactoryMock; + $artifactFetcher ??= $this->artifactFetcherMock; + $maxCacheDuration ??= $this->maxCacheDurationMock; + $helpers ??= $this->helpersMock; + $logger ??= $this->loggerMock; + + return new JwsFetcher( + $parsedJwsFactory, + $artifactFetcher, + $maxCacheDuration, + $helpers, + $logger, + ); + } + + public function testCanCreateInstance(): void + { + $this->assertInstanceOf(JwsFetcher::class, $this->sut()); + } + + public function testCanFetchFromCache(): void + { + $this->artifactFetcherMock->expects($this->once())->method('fromCacheAsString') + ->with('uri') + ->willReturn('token'); + + $this->parsedJwsFactoryMock->expects($this->once())->method('fromToken') + ->with('token'); + + $this->assertInstanceOf(ParsedJws::class, $this->sut()->fromCache('uri')); + } + + public function testCanFetchFromCacheOrNetwork(): void + { + $this->artifactFetcherMock->expects($this->once())->method('fromCacheAsString') + ->with('uri') + ->willReturn(null); + + $this->responseMock->method('getStatusCode')->willReturn(200); + + $this->artifactFetcherMock->expects($this->once())->method('fromNetwork') + ->with('uri') + ->willReturn($this->responseMock); + + $this->assertInstanceOf( + ParsedJws::class, + $this->sut()->fromCacheOrNetwork('uri'), + ); + } + + public function testFetchFromNetworkThrowsForInvalidResponseStatusCode(): void + { + $this->artifactFetcherMock->expects($this->once())->method('fromNetwork') + ->with('uri'); + + $this->responseMock->method('getStatusCode')->willReturn(500); + + $this->expectException(FetchException::class); + $this->expectExceptionMessage('500'); + + $this->loggerMock->expects($this->once())->method('error') + ->with($this->stringContains('500')); + + $this->sut()->fromNetwork('uri'); + } + + public function testChecksForExpectedContentTypeHttpHeader(): void + { + $sut = new class ( + $this->parsedJwsFactoryMock, + $this->artifactFetcherMock, + $this->maxCacheDurationMock, + $this->helpersMock, + $this->loggerMock, + ) extends JwsFetcher { + public function getExpectedContentTypeHttpHeader(): ?string + { + return 'application/jwt'; + } + }; + + $this->responseMock->method('getStatusCode')->willReturn(200); + + $this->expectException(FetchException::class); + $this->expectExceptionMessage('application/jwt'); + + $sut->fromNetwork('uri'); + } + + public function testWillUseJwsExpirationTimeWhenConsideringTtlForCaching(): void + { + $expirationTime = time() + 60; + $this->responseMock->method('getStatusCode')->willReturn(200); + + $this->parsedJwsMock->expects($this->once())->method('getExpirationTime') + ->willReturn($expirationTime); + + $this->parsedJwsFactoryMock->expects($this->once())->method('fromToken') + ->willReturn($this->parsedJwsMock); + + $this->maxCacheDurationMock->expects($this->once())->method('lowestInSecondsComparedToExpirationTime') + ->with($expirationTime) + ->willReturn(60); + + $this->artifactFetcherMock->expects($this->once())->method('cacheIt') + ->with($this->isType('string'), 60, 'uri'); + + + $this->sut()->fromNetwork('uri'); + } +} diff --git a/tests/src/Utils/ArtifactFetcherTest.php b/tests/src/Utils/ArtifactFetcherTest.php index 5a89d53..843b467 100644 --- a/tests/src/Utils/ArtifactFetcherTest.php +++ b/tests/src/Utils/ArtifactFetcherTest.php @@ -5,11 +5,170 @@ namespace SimpleSAML\Test\OpenID\Utils; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\MockObject\MockObject; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; +use Psr\Log\LoggerInterface; +use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; +use SimpleSAML\OpenID\Decorators\CacheDecorator; +use SimpleSAML\OpenID\Decorators\HttpClientDecorator; +use SimpleSAML\OpenID\Exceptions\FetchException; use SimpleSAML\OpenID\Utils\ArtifactFetcher; use PHPUnit\Framework\TestCase; #[CoversClass(ArtifactFetcher::class)] class ArtifactFetcherTest extends TestCase { + protected MockObject $httpClientDecoratorMock; + protected MockObject $cacheDecoratorMock; + protected MockObject $loggerMock; + protected MockObject $responseMock; + protected MockObject $responseBodyMock; + protected function setUp(): void + { + $this->httpClientDecoratorMock = $this->createMock(HttpClientDecorator::class); + $this->cacheDecoratorMock = $this->createMock(CacheDecorator::class); + $this->loggerMock = $this->createMock(LoggerInterface::class); + + $this->responseMock = $this->createMock(ResponseInterface::class); + $this->responseBodyMock = $this->createMock(StreamInterface::class); + $this->responseMock->method('getBody')->willReturn($this->responseBodyMock); + } + + protected function sut( + ?HttpClientDecorator $httpClientDecorator = null, + ?CacheDecorator $cacheDecorator = null, + ?LoggerInterface $logger = null, + ): ArtifactFetcher { + $httpClientDecorator ??= $this->httpClientDecoratorMock; + $cacheDecorator ??= $this->cacheDecoratorMock; + $logger ??= $this->loggerMock; + + return new ArtifactFetcher( + $httpClientDecorator, + $cacheDecorator, + $logger, + ); + } + + public function testCanCreateInstance(): void + { + $this->assertInstanceOf(ArtifactFetcher::class, $this->sut()); + } + + public function testReturnsNullIfCacheNotAvailable(): void + { + $sut = new ArtifactFetcher($this->httpClientDecoratorMock, null, $this->loggerMock); + + $this->loggerMock->expects($this->once())->method('debug') + ->with($this->stringContains('skipping')); + + $this->assertNull($sut->fromCacheAsString('key')); + } + + public function testReturnsNullIfNotInCache(): void + { + $this->cacheDecoratorMock->expects($this->once())->method('get') + ->with(null, 'key') + ->willReturn(null); + + $this->assertNull($this->sut()->fromCacheAsString('key')); + } + + public function testReturnsArtifactIfString(): void + { + $this->cacheDecoratorMock->expects($this->once())->method('get') + ->with(null, 'key') + ->willReturn('artifact'); + + $this->assertSame('artifact', $this->sut()->fromCacheAsString('key')); + } + + public function testReturnsNullIfNotString(): void + { + $this->cacheDecoratorMock->expects($this->once())->method('get') + ->with(null, 'key') + ->willReturn(['artifact-in-array']); + + $this->loggerMock->expects($this->once())->method('warning') + ->with($this->stringContains('unexpected')); + + $this->assertNull($this->sut()->fromCacheAsString('key')); + } + + public function testReturnsNullOnCacheFailure(): void + { + $this->cacheDecoratorMock->expects($this->once())->method('get') + ->with(null, 'key') + ->willThrowException(new \Exception('Error')); + + $this->loggerMock->expects($this->once())->method('error') + ->with($this->stringContains('error')); + + $this->assertNull($this->sut()->fromCacheAsString('key')); + } + + public function testCanFetchFromNetwork(): void + { + $this->httpClientDecoratorMock->expects($this->once())->method('request') + ->with(HttpMethodsEnum::GET, 'uri'); + + $this->assertInstanceOf(ResponseInterface::class, $this->sut()->fromNetwork('uri')); + } + + public function testFromNetworkThrowsOnNetworkError(): void + { + $this->httpClientDecoratorMock->expects($this->once())->method('request') + ->willThrowException(new \Exception('Error')); + + $this->expectException(FetchException::class); + $this->expectExceptionMessage('HTTP'); + + $this->loggerMock->expects($this->once())->method('error') + ->with($this->stringContains('error')); + + $this->sut()->fromNetwork('uri'); + } + + public function testCanFetchFromNetworkAsString(): void + { + $this->httpClientDecoratorMock->expects($this->once())->method('request') + ->willReturn($this->responseMock); + $this->responseBodyMock->method('getContents')->willReturn('artifact'); + + $this->assertSame('artifact', $this->sut()->fromNetworkAsString('uri')); + } + + public function testCanCacheArtifact(): void + { + $this->cacheDecoratorMock->expects($this->once())->method('set') + ->with('artifact', 60, 'key'); + + $this->loggerMock->expects($this->once())->method('debug') + ->with($this->stringContains('saved')); + + $this->sut()->cacheIt('artifact', 60, 'key'); + } + + public function testSkipsCachingIfCacheNotAvailable(): void + { + $this->loggerMock->expects($this->once())->method('debug') + ->with($this->stringContains('skipping')); + + $sut = new ArtifactFetcher($this->httpClientDecoratorMock, null, $this->loggerMock); + + $sut->cacheIt('artifact', 60, 'key'); + } + + public function testCanLogCacheError(): void + { + $this->loggerMock->expects($this->once())->method('error') + ->with($this->stringContains('error')); + + $this->cacheDecoratorMock->expects($this->once())->method('set') + ->willThrowException(new \Exception('Error')); + + $this->sut()->cacheIt('artifact', 60, 'key'); + } }