From 09f4ee188e1276a1fa367d42fb7c68e7db3c1295 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tim.dijen@minbzk.nl>
Date: Tue, 31 Dec 2019 21:32:20 +0100
Subject: [PATCH] Twig Strict vars (#1264)

* Use strict variables in Twig, except for our TemplateTest; throw exception on missing variables

* Fix issues due to Twig strict_variables=true

* Fix for #1229

* Fix metadata-template
---
 lib/SimpleSAML/Locale/Translate.php       |  2 +-
 lib/SimpleSAML/XHTML/Template.php         |  9 +++++++--
 modules/core/templates/base.twig          | 12 ++++++++----
 modules/core/templates/show_metadata.twig |  2 +-
 templates/auth_status.twig                |  4 ++--
 templates/includes/expander.twig          |  2 +-
 templates/metadata.twig                   | 10 +++++-----
 templates/selectidp-links.twig            |  8 ++++----
 8 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/lib/SimpleSAML/Locale/Translate.php b/lib/SimpleSAML/Locale/Translate.php
index e4c84cb05..768e53029 100644
--- a/lib/SimpleSAML/Locale/Translate.php
+++ b/lib/SimpleSAML/Locale/Translate.php
@@ -468,7 +468,7 @@ class Translate
     {
         $text = BaseTranslator::$current->gettext($original);
 
-        if (func_num_args() === 1) {
+        if (func_num_args() === 1 || $original === null) {
             return $text;
         }
 
diff --git a/lib/SimpleSAML/XHTML/Template.php b/lib/SimpleSAML/XHTML/Template.php
index 4dd75b68f..e584d825d 100644
--- a/lib/SimpleSAML/XHTML/Template.php
+++ b/lib/SimpleSAML/XHTML/Template.php
@@ -306,8 +306,9 @@ class Template extends Response
 
         // set up translation
         $options = [
-            'cache' => $cache,
             'auto_reload' => $auto_reload,
+            'cache' => $cache,
+            'strict_variables' => true,
             'translation_function' => [Translate::class, 'translateSingularGettext'],
             'translation_function_plural' => [Translate::class, 'translatePluralGettext'],
         ];
@@ -513,7 +514,11 @@ class Template extends Response
         if ($this->controller) {
             $this->controller->display($this->data);
         }
-        return $this->twig->render($this->twig_template, $this->data);
+        try {
+            return $this->twig->render($this->twig_template, $this->data);
+        } catch (\Twig\Error\RuntimeError $e) {
+            throw new \SimpleSAML\Error\Exception(substr($e->getMessage(), 0, -1) . ' in ' . $this->template, 0, $e);
+        }
     }
 
 
diff --git a/modules/core/templates/base.twig b/modules/core/templates/base.twig
index 96fcc07ba..83b2b9a14 100644
--- a/modules/core/templates/base.twig
+++ b/modules/core/templates/base.twig
@@ -1,8 +1,11 @@
 {% extends "base.twig" %}
 {% block contentwrapper %}
-{% if tabname %}
+
 <div id="portalmenu" class="ui-tabs ui-widget ui-widget-content ui-corner-all">
+
     <ul class="tabset_tabs ui-tabs-nav ui-helper-reset ui-helper-clearfix ui-widget-header ui-corner-all">
+
+    {% if links is defined %}
     {% for name, link in links %}
     {% if name == pageid %}
         <li class="ui-state-default ui-corner-top ui-tabs-selected ui-state-active">
@@ -14,12 +17,13 @@
         </li>
     {% endif %}
     {% endfor %}
+    {% endif %}
+
     </ul>
     <div id="portalcontent" class="ui-tabs-panel ui-widget-content ui-corner-bottom">
-{% endif %}
+
 {{ block('content') }}
-{% if tabname %}
+
     </div>
 </div>
-{% endif %}
 {% endblock %}
diff --git a/modules/core/templates/show_metadata.twig b/modules/core/templates/show_metadata.twig
index f39aee9fe..b74756370 100644
--- a/modules/core/templates/show_metadata.twig
+++ b/modules/core/templates/show_metadata.twig
@@ -4,7 +4,7 @@
     <div class="code-box">
         <div class="code-box-title">
             <h3>{{ 'Metadata'|trans }}</h3>
-            <button data-clipboard-target="#metadata" id="btn{{ loop.index }}" class="pure-button right clipboard-btn copy">
+            <button data-clipboard-target="#metadata" class="pure-button right clipboard-btn copy">
                 <span class="fa fa-copy"></span>
             </button>
         </div>
diff --git a/templates/auth_status.twig b/templates/auth_status.twig
index 9965dd376..8a80f413d 100644
--- a/templates/auth_status.twig
+++ b/templates/auth_status.twig
@@ -102,12 +102,12 @@
     </dl>
     <br>
 
-{% if logout %}
+{% if logout is defined %}
     <h2>{% trans %}Logout{% endtrans %}</h2>
     <p> {{ logout }}</p>
 {% endif %}
 
-{% if logouturl %}
+{% if logouturl is defined %}
     <div class="center">
         <a class="pure-button pure-button-red" href="{{ logouturl }}">{{ 'Logout'|trans }}</a>
     </div>
diff --git a/templates/includes/expander.twig b/templates/includes/expander.twig
index 0817520d3..37deceb6b 100644
--- a/templates/includes/expander.twig
+++ b/templates/includes/expander.twig
@@ -1,4 +1,4 @@
-        <div class="expandable{% if expanded %} expanded{% endif %}">
+        <div class="expandable{% if expanded is defined %} expanded{% endif %}">
           {% if block('general') is defined %}
           <div class="general">
             {{- block("general") }}
diff --git a/templates/metadata.twig b/templates/metadata.twig
index 4d3903533..1857b4cbb 100644
--- a/templates/metadata.twig
+++ b/templates/metadata.twig
@@ -3,7 +3,7 @@
 {% block content %}
   <h2>{% trans %}Metadata{% endtrans %}</h2>
   <dl>
-    <dd>{{ metadata_intro }}</dd>
+    <dd>{{ '{admin:metadata_intro}'|trans }}</dd>
 
   {% if metaurl is defined %}
     <dd>{% trans %}You can get the metadata xml on a dedicated URL:{% endtrans %}</dd>
@@ -49,9 +49,9 @@
 
         <li>
           <a href="{{ cert.url }}"><i class="fa fa-download"></i>{{ cert.name }}
-          {#- #}{% if cert.signing %}-signing{% endif %}
-          {#- #}{% if cert.encryption %}-encryption{% endif %}.pem
-          {#- #}{% if cert.prefix %} ({% trans %}new{% endtrans %}){% endif %}</a> {{ cert.comment }}
+          {#- #}{% if cert.signing is defined %}-signing{% endif %}
+          {#- #}{% if cert.encryption is defined %}-encryption{% endif %}.pem
+          {#- #}{% if cert.prefix is defined %} ({% trans %}new{% endtrans %}){% endif %}</a> {{ cert.comment }}
         </li>
       {% endfor %}
 
@@ -59,4 +59,4 @@
   {% endif %}
 
   </dl>
-{% endblock content %}
\ No newline at end of file
+{% endblock content %}
diff --git a/templates/selectidp-links.twig b/templates/selectidp-links.twig
index a7bf1103f..cebe6962a 100644
--- a/templates/selectidp-links.twig
+++ b/templates/selectidp-links.twig
@@ -17,11 +17,11 @@
         {% for idpentry in idplist %}
         {% if idpentry.entityid == preferredidp %}
                 <div class="preferredidp">
-                {% if idpentry.iconurl %}
+                {% if idpentry.iconurl is defined %}
                     <img class="float-l" src="{{ idpentry.iconurl }}">
                 {% endif %}
                 <h3><i class="fa fa-star"></i> {{ idpentry.name }}</h3>
-                {% if idpentry.description %}
+                {% if idpentry.description is defined %}
                     <p>{{ idpentry.description }}</p>
                 {% endif %}
                 <button type="submit" class="btn" name="idp_{{ idpentry.entityid }}">{{'Select'|trans}}</button>
@@ -31,11 +31,11 @@
 
         {% for idpentry in idplist %}
         {% if idpentry.entityid != preferredidp %}
-                {% if idpentry.iconurl %}
+                {% if idpentry.iconurl is defined %}
                     <img class="float-l" src="{{ idpentry.iconurl }}">
                 {% endif %}
                 <h3>{{ idpentry.name }}</h3>
-                {% if idpentry.description %}
+                {% if idpentry.description is defined %}
                     <p>{{ idpentry.description }}</p>
                 {% endif %}
                 <button type="submit" class="btn" name="idp_{{ idpentry.entityid }}">{{'Select'|trans}}</button>
-- 
GitLab