Merge branch 'master' into release/6.1

OpenConext · Jan 22, 2020 · 140f110 · 140f110
2 parents 6afbb50 + cd0a3b1
commit 140f110
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 25 deletions.
diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/ProxiedIdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/ProxiedIdentityProvider.php
@@ -98,4 +98,35 @@ public function getSingleSignOnServices(): array
         $ssoLocation = $this->urlProvider->getUrl('authentication_idp_sso', false, null, $this->getEntityId());
         return [new Service($ssoLocation, Constants::BINDING_HTTP_REDIRECT)];
     }
+
+    /**
+     * Best effort to show a Dutch display name.
+     *
+     * Buisiness rule:
+     * displayname:nl, name:nl, name:en
+     */
+    public function getDisplayNameNl(): string
+    {
+        if (empty($this->entity->getDisplayNameNl())) {
+            if (!empty($this->entity->getNameNl())) {
+                return $this->entity->getNameNl();
+            }
+            return $this->entity->getNameEn();
+        }
+        return $this->entity->getDisplayNameNl();
+    }
+
+    /**
+     * Best effort to show an English display name.
+     *
+     * Buisiness rule:
+     * displayname:en, name:en
+     */
+    public function getDisplayNameEn(): string
+    {
+        if (empty($this->entity->getDisplayNameEn())) {
+            return $this->entity->getNameEn();
+        }
+        return $this->entity->getDisplayNameEn();
+    }
 }
diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Decorator/ProxiedIdentityProviderTest.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Decorator/ProxiedIdentityProviderTest.php
@@ -64,32 +64,9 @@ public function test_methods()
     {
         $this->adapter = $this->createIdentityProviderAdapter();
 
-        $this->urlProvider->expects($this->exactly(1))
-            ->method('getUrl')
-            ->withConsecutive(
-                // SSO: EngineBlockIdentityProvider::getSingleSignOnServices
-                ['authentication_idp_sso', false, null, 'entity-id']
-            ) ->willReturnOnConsecutiveCalls(
-                // SSO
-                'proxiedSsoLocation'
-            );
+        $this->configureUrlProvider();
 
-        $translator = $this->createMock(TranslatorInterface::class);
-        $translator->expects($this->exactly(1))
-            ->method('trans')
-            ->with('suite_name')
-            ->willReturn('test-suite');
-
-        $configuration = new EngineBlockConfiguration(
-            $translator,
-            'configuredSupportUrl',
-            'configuredSupportMail',
-            'configuredDescription',
-            'configuredLogoUrl',
-            'logopath',
-            1209,
-            1009
-        );
+        $configuration = $this->createConfiguration();
 
         $decorator = new ProxiedIdentityProvider(
             $this->adapter,
@@ -119,4 +96,85 @@ public function test_methods()
 
         $this->runIdentityProviderAssertions($this->adapter, $decorator, $overrides);
     }
+
+    /**
+     * Verification test for display name related requirements stated in:
+     * https://www.pivotaltracker.com/story/show/170401452
+     */
+    public function test_name_fallback()
+    {
+        // If Display Name EN is not set, the decorator will fall back on the Name EN
+        $this->adapter = $this->createIdentityProviderAdapter(['displayNameEn' => '']);
+        $configuration = $this->createConfiguration();
+        $decorator = new ProxiedIdentityProvider(
+            $this->adapter,
+            $configuration,
+            $this->keyPairMock,
+            $this->urlProvider
+        );
+        // Falls back on the name EN when display name is not set (empty)
+        $this->assertEquals($decorator->getNameEn(), $decorator->getDisplayNameEn());
+
+        // If Display Name NL is not set, the decorator will fall back on the Name NL
+        $this->adapter = $this->createIdentityProviderAdapter(['displayNameNl' => '']);
+        $configuration = $this->createConfiguration();
+        $decorator = new ProxiedIdentityProvider(
+            $this->adapter,
+            $configuration,
+            $this->keyPairMock,
+            $this->urlProvider
+        );
+        // Falls back on the name NL when display name is not set (empty)
+        $this->assertEquals($decorator->getNameNl(), $decorator->getDisplayNameNl());
+
+        // If Display Name NL is not set, the decorator will fall back on the Name NL
+        $this->adapter = $this->createIdentityProviderAdapter([
+            'displayNameNl' => '',
+            'nameNl' => '',
+        ]);
+        $configuration = $this->createConfiguration();
+        $decorator = new ProxiedIdentityProvider(
+            $this->adapter,
+            $configuration,
+            $this->keyPairMock,
+            $this->urlProvider
+        );
+        // Falls back on the name EN when display name and name NL are not set (empty)
+        $this->assertEquals($decorator->getNameEn(), $decorator->getDisplayNameNl());
+    }
+
+    private function createConfiguration(): EngineBlockConfiguration
+    {
+        $translator = $this->createMock(TranslatorInterface::class);
+        $translator->expects($this->exactly(1))
+            ->method('trans')
+            ->with('suite_name')
+            ->willReturn('test-suite');
+
+        $configuration = new EngineBlockConfiguration(
+            $translator,
+            'configuredSupportUrl',
+            'configuredSupportMail',
+            'configuredDescription',
+            'configuredLogoUrl',
+            'logopath',
+            1209,
+            1009
+        );
+
+        return $configuration;
+    }
+
+    private function configureUrlProvider(): void
+    {
+        $this->urlProvider->expects($this->exactly(1))
+            ->method('getUrl')
+            ->withConsecutive(
+            // SSO: EngineBlockIdentityProvider::getSingleSignOnServices
+                ['authentication_idp_sso', false, null, 'entity-id']
+            )->willReturnOnConsecutiveCalls(
+            // SSO
+                'proxiedSsoLocation'
+            );
+    }
 }
diff --git a/theme/material/templates/modules/Authentication/View/Metadata/partial/ui_info.xml.twig b/theme/material/templates/modules/Authentication/View/Metadata/partial/ui_info.xml.twig
@@ -1,8 +1,12 @@
 <mdui:UIInfo xmlns:mdui="urn:oasis:names:tc:SAML:metadata:ui">
     <mdui:DisplayName xml:lang="nl">{{ metadata.displayNameNl }}</mdui:DisplayName>
     <mdui:DisplayName xml:lang="en">{{ metadata.displayNameEn }}</mdui:DisplayName>
+    {% if metadata.descriptionNl is not empty %}
     <mdui:Description xml:lang="nl">{{ metadata.descriptionNl }}</mdui:Description>
+    {% endif %}
+    {% if metadata.descriptionEn is not empty %}
     <mdui:Description xml:lang="en">{{ metadata.descriptionEn }}</mdui:Description>
+    {% endif %}
     {% if metadata.logo is not null %}
     <mdui:Logo height="{{ metadata.logo.height }}" width="{{ metadata.logo.width }}">{{ metadata.logo.url }}</mdui:Logo>
     {% endif %}