From dea52b58b572ddb7587b4d19655f77b58e77d3d8 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Wed, 20 Mar 2024 12:34:08 +0100 Subject: [PATCH 1/3] Translatable configurable metadata This is a continuation of #1103 where we made the Organization-data in metadata configurable using the translations-files. However, the result would be like this: My fictional organization My fictional organization Instead of: My fictional organization Mijn fictieve organisatie Everything was being translated using the default locale. This PR attempts to fix that. See: https://github.com/OpenConext/OpenConext-engineblock/pull/1292 See: https://github.com/OpenConext/OpenConext-engineblock/pull/1103 --- .../Adapter/IdentityProviderEntity.php | 4 +- .../Factory/Adapter/ServiceProviderEntity.php | 4 +- .../Decorator/AbstractIdentityProvider.php | 4 +- .../Decorator/AbstractServiceProvider.php | 4 +- ...EngineBlockIdentityProviderInformation.php | 4 +- .../EngineBlockServiceProviderInformation.php | 4 +- .../IdentityProviderEntityInterface.php | 4 +- .../ServiceProviderEntityInterface.php | 4 +- .../ValueObject/EngineBlockConfiguration.php | 26 +++++--- .../Metadata/Factory/AbstractEntity.php | 59 +++++++++++++++++++ .../EngineBlockConfigurationTest.php | 18 ++++-- 11 files changed, 105 insertions(+), 30 deletions(-) diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php b/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php index 01fa458ac8..c1acb6f9c4 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php @@ -126,10 +126,10 @@ public function hasCompleteOrganizationData(string $locale): bool } /** - * @param $locale + * @param string $locale * @return Organization */ - public function getOrganization($locale): ?Organization + public function getOrganization(string $locale): ?Organization { switch (true) { case ($locale == 'nl'): diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/ServiceProviderEntity.php b/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/ServiceProviderEntity.php index 905d0ebc72..f6def40041 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/ServiceProviderEntity.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/ServiceProviderEntity.php @@ -128,10 +128,10 @@ public function hasCompleteOrganizationData(string $locale): bool } /** - * @param $locale + * @param string $locale * @return Organization */ - public function getOrganization($locale): ?Organization + public function getOrganization(string $locale): ?Organization { switch (true) { case ($locale == 'nl'): diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php index d206e91588..ab9cde470e 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php @@ -115,10 +115,10 @@ public function hasCompleteOrganizationData(string $locale): bool } /** - * @param $locale + * @param string $locale * @return Organization|null */ - public function getOrganization($locale): ?Organization + public function getOrganization(string $locale): ?Organization { return $this->entity->getOrganization($locale); } diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractServiceProvider.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractServiceProvider.php index 5cf6e1d26a..ae9d5ef911 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractServiceProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractServiceProvider.php @@ -107,10 +107,10 @@ public function hasCompleteOrganizationData(string $locale): bool } /** - * @param $locale + * @param string $locale * @return Organization|null */ - public function getOrganization($locale): ?Organization + public function getOrganization(string $locale): ?Organization { return $this->entity->getOrganization($locale); } diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockIdentityProviderInformation.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockIdentityProviderInformation.php index d7d7e53b56..e315b00c08 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockIdentityProviderInformation.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockIdentityProviderInformation.php @@ -60,9 +60,9 @@ public function getLogo(): ?Logo return $this->engineBlockConfiguration->getLogo(); } - public function getOrganization($locale): ?Organization + public function getOrganization(string $locale): ?Organization { - return $this->engineBlockConfiguration->getOrganization(); + return $this->engineBlockConfiguration->getOrganization($locale); } /** diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProviderInformation.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProviderInformation.php index e5d15ccd3b..d969ac5711 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProviderInformation.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/EngineBlockServiceProviderInformation.php @@ -60,9 +60,9 @@ public function getLogo(): ?Logo return $this->engineBlockConfiguration->getLogo(); } - public function getOrganization($locale): ?Organization + public function getOrganization(string $locale): ?Organization { - return $this->engineBlockConfiguration->getOrganization(); + return $this->engineBlockConfiguration->getOrganization($locale); } /** diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php b/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php index c0dcf12d9d..ec4865748e 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php @@ -70,10 +70,10 @@ public function getLogo(): ?Logo; public function hasCompleteOrganizationData(string $locale): bool; /** - * @param $locale + * @param string $locale * @return Organization|null */ - public function getOrganization($locale): ?Organization; + public function getOrganization(string $locale): ?Organization; /** * @param $locale diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/ServiceProviderEntityInterface.php b/src/OpenConext/EngineBlock/Metadata/Factory/ServiceProviderEntityInterface.php index 271390bac0..925062a212 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/ServiceProviderEntityInterface.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/ServiceProviderEntityInterface.php @@ -63,10 +63,10 @@ public function getLogo(): ?Logo; public function hasCompleteOrganizationData(string $locale): bool; /** - * @param $locale + * @param string $locale * @return Organization|null */ - public function getOrganization($locale): ?Organization; + public function getOrganization(string $locale): ?Organization; /** * @param $locale diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php b/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php index d0b17fa323..dc1f63cfdf 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php @@ -30,6 +30,11 @@ */ class EngineBlockConfiguration { + /** + * @var \Symfony\Component\Translation\TranslatorInterface + */ + private $translator; + /** * @var string */ @@ -84,11 +89,12 @@ public function __construct( int $logoWidth, int $logoHeight ) { + $this->translator = $translator; $this->suiteName = $translator->trans('suite_name'); $this->engineHostName = $engineHostName; - $this->organizationName = $translator->trans('metadata_organization_name'); - $this->organizationDisplayName = $translator->trans('metadata_organization_displayname'); - $this->organizationUrl = $translator->trans('metadata_organization_url'); + $this->organizationName = 'metadata_organization_name'; + $this->organizationDisplayName = 'metadata_organization_displayname'; + $this->organizationUrl = 'metadata_organization_url'; $this->supportMail = $supportMail; $this->description = $description; @@ -101,9 +107,10 @@ public function __construct( $this->logo->height = $logoHeight; // Create the contact person data for the EB SP entity - $support = ContactPerson::from('support', $this->organizationName, 'Support', $this->supportMail); - $technical = ContactPerson::from('technical', $this->organizationName, 'Support', $this->supportMail); - $administrative = ContactPerson::from('administrative', $this->organizationName, 'Support', $this->supportMail); + $organizationName = $translator->trans('metadata_organization_name'); + $support = ContactPerson::from('support', $organizationName, 'Support', $this->supportMail); + $technical = ContactPerson::from('technical', $organizationName, 'Support', $this->supportMail); + $administrative = ContactPerson::from('administrative', $organizationName, 'Support', $this->supportMail); $this->contactPersons = [$support, $technical, $administrative]; } @@ -118,9 +125,12 @@ public function getHostname(): string return $this->engineHostName; } - public function getOrganization() : Organization + public function getOrganization(string $locale) : Organization { - return new Organization($this->organizationName, $this->organizationDisplayName, $this->organizationUrl); + $organizationName = $this->translator->trans($this->organizationName, [], null, $locale); + $organizationDisplayName = $this->translator->trans($this->organizationDisplayName, [], null, $locale); + $organizationUrl = $this->translator->trans($this->organizationUrl, [], null, $locale); + return new Organization($organizationName, $organizationDisplayName, $organizationUrl); } public function getLogo(): Logo diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntity.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntity.php index 84c81e47a0..0fe4cc3b87 100644 --- a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntity.php +++ b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntity.php @@ -35,6 +35,7 @@ use OpenConext\EngineBlock\Metadata\Service; use OpenConext\EngineBlock\Metadata\ShibMdScope; use OpenConext\EngineBlock\Metadata\X509\X509Certificate; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use ReflectionClass; use ReflectionProperty; @@ -659,6 +660,64 @@ protected function getServiceProviderMockProperties() ]; } + protected function setTranslationExpectancies(MockObject $translator) + { + $translator->expects($this->at(0)) + ->method('trans') + ->with('suite_name') + ->willReturn('test-suite'); + + $translator->expects($this->at(1)) + ->method('trans') + ->with('metadata_organization_name') + ->willReturn('configuredOrganizationName'); + + $translator->expects($this->at(2)) + ->method('trans') + ->with('metadata_organization_name', [], null, 'nl') + ->willReturn('configuredOrganizationName'); + + $translator->expects($this->at(3)) + ->method('trans') + ->with('metadata_organization_displayname', [], null, 'nl') + ->willReturn('configuredOrganizationDisplayName'); + + $translator->expects($this->at(4)) + ->method('trans') + ->with('metadata_organization_url', [], null, 'nl') + ->willReturn('configuredOrganizationUrl'); + + $translator->expects($this->at(5)) + ->method('trans') + ->with('metadata_organization_name', [], null, 'en') + ->willReturn('configuredOrganizationName'); + + $translator->expects($this->at(6)) + ->method('trans') + ->with('metadata_organization_displayname', [], null, 'en') + ->willReturn('configuredOrganizationDisplayName'); + + $translator->expects($this->at(7)) + ->method('trans') + ->with('metadata_organization_url', [], null, 'en') + ->willReturn('configuredOrganizationUrl'); + + $translator->expects($this->at(8)) + ->method('trans') + ->with('metadata_organization_name', [], null, 'pt') + ->willReturn('configuredOrganizationName'); + + $translator->expects($this->at(9)) + ->method('trans') + ->with('metadata_organization_displayname', [], null, 'pt') + ->willReturn('configuredOrganizationDisplayName'); + + $translator->expects($this->at(10)) + ->method('trans') + ->with('metadata_organization_url', [], null, 'pt') + ->willReturn('configuredOrganizationUrl'); + } + private function getParameters($className, $skipParameters = []) { $results = []; diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfigurationTest.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfigurationTest.php index 7613b4f299..1bfc8f964b 100644 --- a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfigurationTest.php +++ b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfigurationTest.php @@ -42,17 +42,22 @@ public function test_configuration_creation() ->shouldReceive('trans') ->with('suite_name')->once() ->andReturn($suitName); + $translator + ->shouldReceive('trans') + ->with('metadata_organization_name', [], null, 'en')->once() + ->andReturn($orgName); $translator ->shouldReceive('trans') ->with('metadata_organization_name')->once() ->andReturn($orgName); $translator ->shouldReceive('trans') - ->with('metadata_organization_displayname')->once() + ->with('metadata_organization_displayname', [], null, 'en')->once() ->andReturn($orgDisplayName); + $translator ->shouldReceive('trans') - ->with('metadata_organization_url')->once() + ->with('metadata_organization_url', [], null, 'en')->once() ->andReturn($orgUrl); $mail = 'mail@example.org'; @@ -98,9 +103,10 @@ public function test_configuration_creation() $this->assertEquals($width, $configuration->getLogo()->width); $this->assertEquals($height, $configuration->getLogo()->height); - $this->assertInstanceOf(Organization::class, $configuration->getOrganization()); - $this->assertEquals($orgUrl, $configuration->getOrganization()->url); - $this->assertEquals($orgName, $configuration->getOrganization()->name); - $this->assertEquals($orgDisplayName, $configuration->getOrganization()->displayName); + $organization = $configuration->getOrganization('en'); + $this->assertInstanceOf(Organization::class, $organization); + $this->assertEquals($orgUrl, $organization->url); + $this->assertEquals($orgName, $organization->name); + $this->assertEquals($orgDisplayName, $organization->displayName); } } From bc261b25b43406310602364069c8fe4f969080d7 Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Mon, 30 Sep 2024 18:48:48 +0200 Subject: [PATCH 2/3] Remove unnecessary function call --- .../Factory/ValueObject/EngineBlockConfiguration.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php b/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php index dc1f63cfdf..953678b3dd 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php @@ -107,10 +107,9 @@ public function __construct( $this->logo->height = $logoHeight; // Create the contact person data for the EB SP entity - $organizationName = $translator->trans('metadata_organization_name'); - $support = ContactPerson::from('support', $organizationName, 'Support', $this->supportMail); - $technical = ContactPerson::from('technical', $organizationName, 'Support', $this->supportMail); - $administrative = ContactPerson::from('administrative', $organizationName, 'Support', $this->supportMail); + $support = ContactPerson::from('support', $this->organizationName, 'Support', $this->supportMail); + $technical = ContactPerson::from('technical', $this->organizationName, 'Support', $this->supportMail); + $administrative = ContactPerson::from('administrative', $this->organizationName, 'Support', $this->supportMail); $this->contactPersons = [$support, $technical, $administrative]; } From 9a56b0117dd897b2d8c92c2920441ccbdba3f9c7 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Wed, 18 Mar 2026 11:03:15 +0100 Subject: [PATCH 3/3] fix: translate organization name for contact persons in metadata After storing organization translation keys instead of translated values in EngineBlockConfiguration, contact persons were inadvertently using the raw key string ('metadata_organization_name') as their GivenName rather than the actual translated value. ContactPerson elements in SAML metadata have no locale support, so the translation is resolved once at construction time using the default locale. --- .../Factory/ValueObject/EngineBlockConfiguration.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php b/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php index 953678b3dd..dc1f63cfdf 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/ValueObject/EngineBlockConfiguration.php @@ -107,9 +107,10 @@ public function __construct( $this->logo->height = $logoHeight; // Create the contact person data for the EB SP entity - $support = ContactPerson::from('support', $this->organizationName, 'Support', $this->supportMail); - $technical = ContactPerson::from('technical', $this->organizationName, 'Support', $this->supportMail); - $administrative = ContactPerson::from('administrative', $this->organizationName, 'Support', $this->supportMail); + $organizationName = $translator->trans('metadata_organization_name'); + $support = ContactPerson::from('support', $organizationName, 'Support', $this->supportMail); + $technical = ContactPerson::from('technical', $organizationName, 'Support', $this->supportMail); + $administrative = ContactPerson::from('administrative', $organizationName, 'Support', $this->supportMail); $this->contactPersons = [$support, $technical, $administrative]; }