From 5ab7708b033d1701a6415c3353d26ed1a0d58a01 Mon Sep 17 00:00:00 2001 From: Marko Ivancic Date: Mon, 31 Mar 2025 17:15:13 +0200 Subject: [PATCH 1/2] Add initial dynamic Trust Mark fetch capabilities --- UPGRADE.md | 1 + config/module_oidc.php.dist | 11 ++++- src/Controllers/Admin/ConfigController.php | 19 ++++++++- .../Federation/EntityStatementController.php | 42 +++++++++++++++++++ src/ModuleConfig.php | 11 +++++ templates/config/federation.twig | 2 +- .../EntityStatementControllerTest.php | 6 +++ 7 files changed, 89 insertions(+), 3 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 0484b251..e3443186 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -47,6 +47,7 @@ and optionally a port (as in all previous module versions). - federation caching adapter and its arguments - PKI keys - federation keys used for example to sign federation entity statements - federation participation limiting based on Trust Marks for RPs + - (from v6.1) own Trust Marks to dynamically fetch - signer algorithm - entity statement duration - organization name diff --git a/config/module_oidc.php.dist b/config/module_oidc.php.dist index b1ca0118..12a176c3 100644 --- a/config/module_oidc.php.dist +++ b/config/module_oidc.php.dist @@ -369,11 +369,20 @@ $config = [ ], // (optional) Federation Trust Mark tokens. An array of tokens (signed JWTs), each representing a Trust Mark - // issued to this entity. + // issued to this entity. This option is primarily intended for long-lasting or non-expiring tokens, so it + // is not necessary to dynamically fetch / refresh them. ModuleConfig::OPTION_FEDERATION_TRUST_MARK_TOKENS => [ // 'eyJ...GHg', ], + // (optional) Federation Trust Marks for dynamic fetching. An array of key-value pairs, where key is Trust Mark ID + // and value is Trust Mark Issuer ID, each representing a Trust Mark issued to this entity. Each Trust Mark ID + // in this array will be dynamically fetched from noted Trust Mark Issuer as necessary. If federation caching + // is enabled (recommended), fetched Trust Marks will also be cached until their expiry. + ModuleConfig::OPTION_FEDERATION_DYNAMIC_TRUST_MARK_TOKENS => [ +// 'trust-mark-id' => 'trust-mark-issuer-id', + ], + // (optional) Federation participation limit by Trust Marks. This is an array with the following format: // [ // 'trust-anchor-id' => [ diff --git a/src/Controllers/Admin/ConfigController.php b/src/Controllers/Admin/ConfigController.php index b8bf55b6..85e35a6c 100644 --- a/src/Controllers/Admin/ConfigController.php +++ b/src/Controllers/Admin/ConfigController.php @@ -68,7 +68,7 @@ public function protocolSettings(): Response public function federationSettings(): Response { - $trustMarks = null; + $trustMarks = []; if (is_array($trustMarkTokens = $this->moduleConfig->getFederationTrustMarkTokens())) { $trustMarks = array_map( function (string $token): Federation\TrustMark { @@ -78,6 +78,23 @@ function (string $token): Federation\TrustMark { ); } + if (is_array($dynamicTrustMarks = $this->moduleConfig->getFederationDynamicTrustMarks())) { + /** + * @var non-empty-string $trustMarkId + * @var non-empty-string $trustMarkIssuerId + */ + foreach ($dynamicTrustMarks as $trustMarkId => $trustMarkIssuerId) { + $trustMarkIssuerConfigurationStatement = $this->federation->entityStatementFetcher() + ->fromCacheOrWellKnownEndpoint($trustMarkIssuerId); + + $trustMarks[] = $this->federation->trustMarkFetcher()->fromCacheOrFederationTrustMarkEndpoint( + $trustMarkId, + $this->moduleConfig->getIssuer(), + $trustMarkIssuerConfigurationStatement, + ); + } + } + return $this->templateFactory->build( 'oidc:config/federation.twig', [ diff --git a/src/Controllers/Federation/EntityStatementController.php b/src/Controllers/Federation/EntityStatementController.php index d67e413e..105d8100 100644 --- a/src/Controllers/Federation/EntityStatementController.php +++ b/src/Controllers/Federation/EntityStatementController.php @@ -11,6 +11,7 @@ use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Services\JsonWebKeySetService; use SimpleSAML\Module\oidc\Services\JsonWebTokenBuilderService; +use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Services\OpMetadataService; use SimpleSAML\Module\oidc\Utils\FederationCache; use SimpleSAML\Module\oidc\Utils\Routes; @@ -42,6 +43,7 @@ public function __construct( private readonly Helpers $helpers, private readonly Routes $routes, private readonly Federation $federation, + private readonly LoggerService $loggerService, private readonly ?FederationCache $federationCache, ) { if (!$this->moduleConfig->getFederationEnabled()) { @@ -126,6 +128,8 @@ public function configuration(): Response $builder = $builder->withClaim(ClaimsEnum::AuthorityHints->value, $authorityHints); } + $trustMarks = []; + if ( is_array($trustMarkTokens = $this->moduleConfig->getFederationTrustMarkTokens()) && (!empty($trustMarkTokens)) @@ -145,7 +149,45 @@ public function configuration(): Response ClaimsEnum::TrustMark->value => $token, ]; }, $trustMarkTokens); + } + + if ( + is_array($dynamicTrustMarks = $this->moduleConfig->getFederationDynamicTrustMarks()) && + (!empty($dynamicTrustMarks)) + ) { + /** + * @var non-empty-string $trustMarkId + * @var non-empty-string $trustMarkIssuerId + */ + foreach ($dynamicTrustMarks as $trustMarkId => $trustMarkIssuerId) { + try { + $trustMarkIssuerConfigurationStatement = $this->federation->entityStatementFetcher() + ->fromCacheOrWellKnownEndpoint($trustMarkIssuerId); + + $trustMarkEntity = $this->federation->trustMarkFetcher()->fromCacheOrFederationTrustMarkEndpoint( + $trustMarkId, + $this->moduleConfig->getIssuer(), + $trustMarkIssuerConfigurationStatement, + ); + + $trustMarks[] = [ + ClaimsEnum::TrustMarkId->value => $trustMarkId, + ClaimsEnum::TrustMark->value => $trustMarkEntity->getToken(), + ]; + } catch (\Throwable $exception) { + $this->loggerService->error( + 'Error fetching Trust Mark: ' . $exception->getMessage(), + [ + 'trustMarkId' => $trustMarkId, + 'subjectId' => $this->moduleConfig->getIssuer(), + 'trustMarkIssuerId' => $trustMarkIssuerId, + ], + ); + } + } + } + if (!empty($trustMarks)) { $builder = $builder->withClaim(ClaimsEnum::TrustMarks->value, $trustMarks); } diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index 8f89fcaa..96fcab0d 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -76,6 +76,7 @@ class ModuleConfig final public const OPTION_FEDERATION_CACHE_MAX_DURATION_FOR_FETCHED = 'federation_cache_max_duration_for_fetched'; final public const OPTION_FEDERATION_TRUST_ANCHORS = 'federation_trust_anchors'; final public const OPTION_FEDERATION_TRUST_MARK_TOKENS = 'federation_trust_mark_tokens'; + final public const OPTION_FEDERATION_DYNAMIC_TRUST_MARK_TOKENS = 'federation_dynamic_trust_mark_tokens'; final public const OPTION_FEDERATION_CACHE_DURATION_FOR_PRODUCED = 'federation_cache_duration_for_produced'; final public const OPTION_PROTOCOL_CACHE_ADAPTER = 'protocol_cache_adapter'; final public const OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments'; @@ -632,6 +633,16 @@ public function getFederationTrustMarkTokens(): ?array return empty($trustMarks) ? null : $trustMarks; } + public function getFederationDynamicTrustMarks(): ?array + { + $dynamicTrustMarks = $this->config()->getOptionalArray( + self::OPTION_FEDERATION_DYNAMIC_TRUST_MARK_TOKENS, + null, + ); + + return empty($dynamicTrustMarks) ? null : $dynamicTrustMarks; + } + public function getOrganizationName(): ?string { return $this->config()->getOptionalString( diff --git a/templates/config/federation.twig b/templates/config/federation.twig index 8bb739ee..4b05424c 100644 --- a/templates/config/federation.twig +++ b/templates/config/federation.twig @@ -93,7 +93,7 @@ {% if trustMarks|default is not empty %} {% for trustMark in trustMarks %}

- - {{ trustMark.getPayload.id }} + - {{ trustMark.getPayload.trust_mark_id }} {{- trustMark.getPayload|json_encode(constant('JSON_PRETTY_PRINT') b-or constant('JSON_UNESCAPED_SLASHES')) -}} diff --git a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php index e2170067..56a5e589 100644 --- a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php +++ b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php @@ -14,6 +14,7 @@ use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Services\JsonWebKeySetService; use SimpleSAML\Module\oidc\Services\JsonWebTokenBuilderService; +use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Services\OpMetadataService; use SimpleSAML\Module\oidc\Utils\FederationCache; use SimpleSAML\Module\oidc\Utils\Routes; @@ -30,6 +31,7 @@ class EntityStatementControllerTest extends TestCase protected MockObject $helpersMock; protected MockObject $routesMock; protected MockObject $federationMock; + protected MockObject $loggerServiceMock; protected MockObject $federationCacheMock; protected function setUp(): void @@ -42,6 +44,7 @@ protected function setUp(): void $this->helpersMock = $this->createMock(Helpers::class); $this->routesMock = $this->createMock(Routes::class); $this->federationMock = $this->createMock(Federation::class); + $this->loggerServiceMock = $this->createMock(LoggerService::class); $this->federationCacheMock = $this->createMock(FederationCache::class); } @@ -54,6 +57,7 @@ protected function sut( ?Helpers $helpers = null, ?Routes $routes = null, ?Federation $federation = null, + ?LoggerService $loggerService = null, ?FederationCache $federationCache = null, ): EntityStatementController { $moduleConfig ??= $this->moduleConfigMock; @@ -64,6 +68,7 @@ protected function sut( $helpers ??= $this->helpersMock; $routes ??= $this->routesMock; $federation ??= $this->federationMock; + $loggerService ??= $this->loggerServiceMock; $federationCache ??= $this->federationCacheMock; return new EntityStatementController( @@ -75,6 +80,7 @@ protected function sut( $helpers, $routes, $federation, + $loggerService, $federationCache, ); } From 1e1759e153a5630099eec8f1f87bbe3628a62b02 Mon Sep 17 00:00:00 2001 From: Marko Ivancic Date: Tue, 1 Apr 2025 11:24:27 +0200 Subject: [PATCH 2/2] Add coverage --- config/module_oidc.php.dist | 2 +- src/ModuleConfig.php | 4 +- .../Admin/ConfigControllerTest.php | 29 +++++++ tests/unit/src/ModuleConfigTest.php | 81 ++++++++++++++++++- 4 files changed, 111 insertions(+), 5 deletions(-) diff --git a/config/module_oidc.php.dist b/config/module_oidc.php.dist index 12a176c3..8c56930b 100644 --- a/config/module_oidc.php.dist +++ b/config/module_oidc.php.dist @@ -379,7 +379,7 @@ $config = [ // and value is Trust Mark Issuer ID, each representing a Trust Mark issued to this entity. Each Trust Mark ID // in this array will be dynamically fetched from noted Trust Mark Issuer as necessary. If federation caching // is enabled (recommended), fetched Trust Marks will also be cached until their expiry. - ModuleConfig::OPTION_FEDERATION_DYNAMIC_TRUST_MARK_TOKENS => [ + ModuleConfig::OPTION_FEDERATION_DYNAMIC_TRUST_MARKS => [ // 'trust-mark-id' => 'trust-mark-issuer-id', ], diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index 96fcab0d..4a9fe3b8 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -76,7 +76,7 @@ class ModuleConfig final public const OPTION_FEDERATION_CACHE_MAX_DURATION_FOR_FETCHED = 'federation_cache_max_duration_for_fetched'; final public const OPTION_FEDERATION_TRUST_ANCHORS = 'federation_trust_anchors'; final public const OPTION_FEDERATION_TRUST_MARK_TOKENS = 'federation_trust_mark_tokens'; - final public const OPTION_FEDERATION_DYNAMIC_TRUST_MARK_TOKENS = 'federation_dynamic_trust_mark_tokens'; + final public const OPTION_FEDERATION_DYNAMIC_TRUST_MARKS = 'federation_dynamic_trust_mark_tokens'; final public const OPTION_FEDERATION_CACHE_DURATION_FOR_PRODUCED = 'federation_cache_duration_for_produced'; final public const OPTION_PROTOCOL_CACHE_ADAPTER = 'protocol_cache_adapter'; final public const OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS = 'protocol_cache_adapter_arguments'; @@ -636,7 +636,7 @@ public function getFederationTrustMarkTokens(): ?array public function getFederationDynamicTrustMarks(): ?array { $dynamicTrustMarks = $this->config()->getOptionalArray( - self::OPTION_FEDERATION_DYNAMIC_TRUST_MARK_TOKENS, + self::OPTION_FEDERATION_DYNAMIC_TRUST_MARKS, null, ); diff --git a/tests/unit/src/Controllers/Admin/ConfigControllerTest.php b/tests/unit/src/Controllers/Admin/ConfigControllerTest.php index b648fd57..07c4a8d6 100644 --- a/tests/unit/src/Controllers/Admin/ConfigControllerTest.php +++ b/tests/unit/src/Controllers/Admin/ConfigControllerTest.php @@ -28,6 +28,8 @@ class ConfigControllerTest extends TestCase protected MockObject $federationMock; protected MockObject $routesMock; protected MockObject $trustMarkFactoryMock; + protected MockObject $entityStatementFetcherMock; + protected MockObject $trustMarkFetcherMock; protected function setUp(): void { @@ -41,6 +43,12 @@ protected function setUp(): void $this->trustMarkFactoryMock = $this->createMock(TrustMarkFactory::class); $this->federationMock->method('trustMarkFactory')->willReturn($this->trustMarkFactoryMock); + + $this->entityStatementFetcherMock = $this->createMock(Federation\EntityStatementFetcher::class); + $this->federationMock->method('entityStatementFetcher')->willReturn($this->entityStatementFetcherMock); + + $this->trustMarkFetcherMock = $this->createMock(Federation\TrustMarkFetcher::class); + $this->federationMock->method('trustMarkFetcher')->willReturn($this->trustMarkFetcherMock); } public function sut( @@ -129,4 +137,25 @@ public function testCanIncludeTrustMarksInFederationSettings(): void $this->sut()->federationSettings(); } + + public function testCanIncludeDynamicTrustMarksInFederationSettings(): void + { + $this->moduleConfigMock->method('getIssuer')->willReturn('issuer-id'); + $this->moduleConfigMock->method('getFederationDynamicTrustMarks') + ->willReturn(['trust-mark-id' => 'trust-mark-issuer-id']); + + $this->entityStatementFetcherMock->expects($this->once())->method('fromCacheOrWellKnownEndpoint') + ->with('trust-mark-issuer-id'); + + $this->trustMarkFetcherMock->expects($this->once())->method('fromCacheOrFederationTrustMarkEndpoint') + ->with( + 'trust-mark-id', + 'issuer-id', + ); + + $this->templateFactoryMock->expects($this->once())->method('build') + ->with('oidc:config/federation.twig'); + + $this->sut()->federationSettings(); + } } diff --git a/tests/unit/src/ModuleConfigTest.php b/tests/unit/src/ModuleConfigTest.php index 22faea7d..f2991676 100644 --- a/tests/unit/src/ModuleConfigTest.php +++ b/tests/unit/src/ModuleConfigTest.php @@ -378,9 +378,86 @@ public function testCanGetProtocolDiscoveryShowClaimsSupported(): void $this->assertFalse($this->sut()->getProtocolDiscoveryShowClaimsSupported()); $this->assertTrue( $this->sut( - null, - [ModuleConfig::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED => true], + overrides: [ModuleConfig::OPTION_PROTOCOL_DISCOVERY_SHOW_CLAIMS_SUPPORTED => true], )->getProtocolDiscoveryShowClaimsSupported(), ); } + + public function testCanGetProtocolNewCertPath(): void + { + $this->assertNull($this->sut()->getProtocolNewCertPath()); + + $sut = $this->sut( + overrides: [ModuleConfig::OPTION_PKI_NEW_CERTIFICATE_FILENAME => 'new-cert'], + ); + + $this->assertStringContainsString('new-cert', $sut->getProtocolNewCertPath()); + } + + public function testCanGetFederationNewCertPath(): void + { + $this->assertNull($this->sut()->getFederationNewCertPath()); + + $sut = $this->sut( + overrides: [ModuleConfig::OPTION_PKI_FEDERATION_NEW_CERTIFICATE_FILENAME => 'new-cert'], + ); + + $this->assertStringContainsString('new-cert', $sut->getFederationNewCertPath()); + } + + public function testCanGetFederationDynamicTrustMarks(): void + { + $this->assertNull($this->sut()->getFederationDynamicTrustMarks()); + + $sut = $this->sut( + overrides: [ + ModuleConfig::OPTION_FEDERATION_DYNAMIC_TRUST_MARKS => [ + 'trust-mark-id' => 'trust-mark-issuer-id', + ], + ], + ); + + $this->assertArrayHasKey( + 'trust-mark-id', + $sut->getFederationDynamicTrustMarks(), + ); + } + + public function testCanGetFederationParticipationLimitByTrustMarks(): void + { + $this->assertArrayHasKey( + 'https://ta.example.org/', + $this->sut()->getFederationParticipationLimitByTrustMarks(), + ); + } + + public function testCanGetTrustMarksNeededForFederationParticipationFor(): void + { + $neededTrustMarks = $this->sut()->getTrustMarksNeededForFederationParticipationFor('https://ta.example.org/'); + + $this->assertArrayHasKey('one_of', $neededTrustMarks); + $this->assertTrue(in_array('trust-mark-id', $neededTrustMarks['one_of'])); + } + + public function testGetTrustMarksNeededForFederationParticipationForThrowsOnInvalidConfigValue(): void + { + $sut = $this->sut( + overrides: [ + ModuleConfig::OPTION_FEDERATION_PARTICIPATION_LIMIT_BY_TRUST_MARKS => [ + 'https://ta.example.org/' => 'invalid', + ], + ], + ); + + $this->expectException(ConfigurationError::class); + + $sut->getTrustMarksNeededForFederationParticipationFor('https://ta.example.org/'); + } + + public function testCanGetIsFederationParticipationLimitedByTrustMarksFor(): void + { + $this->assertTrue( + $this->sut()->isFederationParticipationLimitedByTrustMarksFor('https://ta.example.org/'), + ); + } }