From d3320e6ee4efaac35b243337bbdced89690b9667 Mon Sep 17 00:00:00 2001
From: Thijs Kinkhorst <thijs@kinkhorst.com>
Date: Wed, 8 Sep 2021 12:39:52 +0000
Subject: [PATCH] Move configuration of attribute name translation into a
 separate method.

Now also considers any attributes.po in the configured theme, if any.
Makes the attributes.extradictionary config setting obsolete.
---
 config-templates/config.php                   | 28 -------------
 docs/simplesamlphp-upgrade-notes-2.0.md       |  1 +
 lib/SimpleSAML/Locale/Localization.php        | 19 ++++++++-
 lib/SimpleSAML/Locale/Translate.php           | 33 ----------------
 modules/admin/lib/Controller/Test.php         |  2 +-
 modules/core/lib/Controller/Login.php         |  2 +-
 .../SimpleSAML/Locale/LocalizationTest.php    | 39 ++++++++++++++++++-
 7 files changed, 57 insertions(+), 67 deletions(-)

diff --git a/config-templates/config.php b/config-templates/config.php
index 6153dcd54..8c466caaa 100644
--- a/config-templates/config.php
+++ b/config-templates/config.php
@@ -774,34 +774,6 @@ $config = [
      *   'language.get_language_function' => ['\SimpleSAML\Module\example\Template', 'getLanguage'],
      */
 
-    /*
-     * Extra dictionary for attribute names.
-     * This can be used to define local attributes.
-     *
-     * The format of the parameter is a string with <module>:<dictionary>.
-     *
-     * Specifying this option will cause us to look for modules/<module>/dictionaries/<dictionary>.definition.json
-     * The dictionary should look something like:
-     *
-     * {
-     *     "firstattribute": {
-     *         "en": "English name",
-     *         "no": "Norwegian name"
-     *     },
-     *     "secondattribute": {
-     *         "en": "English name",
-     *         "no": "Norwegian name"
-     *     }
-     * }
-     *
-     * Note that all attribute names in the dictionary must in lowercase.
-     *
-     * Example: 'attributes.extradictionary' => 'ourmodule:ourattributes',
-     */
-    'attributes.extradictionary' => null,
-
-
-
     /**************
      | APPEARANCE |
      **************/
diff --git a/docs/simplesamlphp-upgrade-notes-2.0.md b/docs/simplesamlphp-upgrade-notes-2.0.md
index 05f94f1f7..bef378e50 100644
--- a/docs/simplesamlphp-upgrade-notes-2.0.md
+++ b/docs/simplesamlphp-upgrade-notes-2.0.md
@@ -53,3 +53,4 @@ Upgrade notes for SimpleSAMLphp 2.0
 
 - Configuration options removed:
   - languages[priorities]
+  - attributes.extradictionaries. Add an attributes.po to your configured theme instead.
diff --git a/lib/SimpleSAML/Locale/Localization.php b/lib/SimpleSAML/Locale/Localization.php
index ae621c2cf..d49a57a28 100644
--- a/lib/SimpleSAML/Locale/Localization.php
+++ b/lib/SimpleSAML/Locale/Localization.php
@@ -140,13 +140,14 @@ class Localization
      *
      * @param string $module Module name
      * @param string $localeDir Absolute path if the module is housed elsewhere
+     * @param string $domain Translation domain within module; defaults to module name
      */
-    public function addModuleDomain(string $module, string $localeDir = null): void
+    public function addModuleDomain(string $module, string $localeDir = null, string $domain = null): void
     {
         if (!$localeDir) {
             $localeDir = $this->getDomainLocaleDir($module);
         }
-        $this->addDomain($localeDir, $module);
+        $this->addDomain($localeDir, $domain ?? $module);
     }
 
 
@@ -302,4 +303,18 @@ class Localization
     {
         return $this->localeDomainMap;
     }
+
+    /**
+     * Add translation domains specifically used for translating attributes names:
+     * the default in attributes.po and any attributes.po in the enabled theme.
+     */
+    public function addAttributeDomains(): void
+    {
+        $this->addDomain($this->localeDir, 'attributes');
+
+        list($theme,) = explode(':', $this->configuration->getString('theme.use', 'default'));
+        if($theme !== 'default') {
+            $this->addModuleDomain($theme, null, 'attributes');
+        }
+    }
 }
diff --git a/lib/SimpleSAML/Locale/Translate.php b/lib/SimpleSAML/Locale/Translate.php
index 9d57f6c3a..6fb59cc9a 100644
--- a/lib/SimpleSAML/Locale/Translate.php
+++ b/lib/SimpleSAML/Locale/Translate.php
@@ -143,39 +143,6 @@ class Translate
     }
 
 
-    /**
-     * Translate the name of an attribute.
-     *
-     * @param string $name The attribute name.
-     *
-     * @return string The translated attribute name, or the original attribute name if no translation was found.
-     */
-    public function getAttributeTranslation(string $name): string
-    {
-        // normalize attribute name
-        $normName = strtolower($name);
-        $normName = str_replace([":", "-"], "_", $normName);
-
-        // check for an extra dictionary
-        $extraDict = $this->configuration->getString('attributes.extradictionary', null);
-        if ($extraDict !== null) {
-            $dict = $this->getDictionary($extraDict);
-            if (array_key_exists($normName, $dict)) {
-                return $this->getPreferredTranslation($dict[$normName]);
-            }
-        }
-
-        // search the default attribute dictionary
-        $dict = $this->getDictionary('attributes');
-        if (array_key_exists('attribute_' . $normName, $dict)) {
-            return $this->getPreferredTranslation($dict['attribute_' . $normName]);
-        }
-
-        // no translations found
-        return $name;
-    }
-
-
     /**
      * Mark a string for translation without translating it.
      *
diff --git a/modules/admin/lib/Controller/Test.php b/modules/admin/lib/Controller/Test.php
index bb2f9add7..355e87636 100644
--- a/modules/admin/lib/Controller/Test.php
+++ b/modules/admin/lib/Controller/Test.php
@@ -147,7 +147,7 @@ class Test
             $httpUtils = new Utils\HTTP();
             $t = new Template($this->config, 'admin:status.twig', 'attributes');
             $l = $t->getLocalization();
-            $l->addDomain($l->getLocaleDir(), 'attributes');
+            $l->addAttributeDomains();
             $t->data = [
                 'attributes' => $attributes,
                 'authData' => $authData,
diff --git a/modules/core/lib/Controller/Login.php b/modules/core/lib/Controller/Login.php
index 26ea0131b..50a4f3cb0 100644
--- a/modules/core/lib/Controller/Login.php
+++ b/modules/core/lib/Controller/Login.php
@@ -91,7 +91,7 @@ class Login
 
         $t = new Template($this->config, 'auth_status.twig', 'attributes');
         $l = $t->getLocalization();
-        $l->addDomain($l->getLocaleDir(), 'attributes');
+        $l->addAttributeDomains();
         $t->data['header'] = '{status:header_saml20_sp}';
         $t->data['attributes'] = $attributes;
         $t->data['nameid'] = !is_null($auth->getAuthData('saml:sp:NameID'))
diff --git a/tests/lib/SimpleSAML/Locale/LocalizationTest.php b/tests/lib/SimpleSAML/Locale/LocalizationTest.php
index 7f529a0bd..d2b041044 100644
--- a/tests/lib/SimpleSAML/Locale/LocalizationTest.php
+++ b/tests/lib/SimpleSAML/Locale/LocalizationTest.php
@@ -34,7 +34,7 @@ class LocalizationTest extends TestCase
 
 
     /**
-     * Test SimpleSAML\Locale\Localization::activateDomain().
+     * Test SimpleSAML\Locale\Localization::addDomain().
      */
     public function testAddDomain(): void
     {
@@ -45,6 +45,41 @@ class LocalizationTest extends TestCase
         $l->addDomain($newDomainLocaleDir, $newDomain);
         $registeredDomains = $l->getRegisteredDomains();
         $this->assertArrayHasKey($newDomain, $registeredDomains);
-        $this->assertEquals($registeredDomains[$newDomain], $newDomainLocaleDir);
+        $this->assertEquals($newDomainLocaleDir, $registeredDomains[$newDomain]);
+    }
+
+    /**
+     * Test SimpleSAML\Locale\Localization::addModuleDomains().
+     */
+    public function testAddModuleDomain(): void
+    {
+        $c = Configuration::loadFromArray([]);
+        $l = new Localization($c);
+        $newDomainLocaleDir = $l->getLocaleDir();
+
+        $l->addAttributeDomains();
+        $registeredDomains = $l->getRegisteredDomains();
+        $this->assertArrayHasKey('messages', $registeredDomains);
+        $this->assertArrayHasKey('attributes', $registeredDomains);
+        $this->assertEquals($newDomainLocaleDir, $registeredDomains['messages']);
+        $this->assertEquals($newDomainLocaleDir, $registeredDomains['attributes']);
+    }
+
+    /**
+     * Test SimpleSAML\Locale\Localization::addModuleDomains() with a theme.
+     */
+    public function testAddModuleDomainWithTheme(): void
+    {
+        $c = Configuration::loadFromArray(['theme.use' => 'testtheme:Test']);
+        $l = new Localization($c);
+        $newDomainLocaleDir = $l->getLocaleDir();
+        $newModuleDomainLocaleDir = $l->getDomainLocaleDir('testtheme');
+
+        $l->addAttributeDomains();
+        $registeredDomains = $l->getRegisteredDomains();
+        $this->assertArrayHasKey('messages', $registeredDomains);
+        $this->assertArrayHasKey('attributes', $registeredDomains);
+        $this->assertEquals($newDomainLocaleDir, $registeredDomains['messages']);
+        $this->assertEquals($newModuleDomainLocaleDir, $registeredDomains['attributes']);
     }
 }
-- 
GitLab