csrgen: Use data_sources option to define which fields are rendered

This removes the ipa.syntaxrule and ipa.datarule macros in favor of
simple 'if' statements based on the data referenced in the rules. The
'if' statement for a syntax rule is generated based on the data rules it
contains.

The Subject DN should not be generated unless all data rules are in
place, so the ability to override the logical operator that combines
data_sources (from 'or' to 'and') is added.

https://fedorahosted.org/freeipa/ticket/4899

Reviewed-By: Jan Cholasta <jcholast@redhat.com>

13 files changed, 75 insertions, 77 deletions

diff --git a/install/share/csr/templates/ipa_macros.tmpl b/install/share/csr/templates/ipa_macros.tmpl
deleted file mode 100644
index e790d4eb5..000000000
--- a/install/share/csr/templates/ipa_macros.tmpl
+++ /dev/null
@@ -1,42 +0,0 @@
-{% set rendersyntax = {} %}
-
-{% set renderdata = {} %}
-
-{# Wrapper for syntax rules. We render the contents of the rule into a
-variable, so that if we find that none of the contained data rules rendered we
-can suppress the whole syntax rule. That is, a syntax rule is rendered either
-if no data rules are specified (unusual) or if at least one of the data rules
-rendered successfully. #}
-{% macro syntaxrule() -%}
-{% do rendersyntax.update(none=true, any=false) -%}
-{% set contents -%}
-{{ caller() -}}
-{% endset -%}
-{% if rendersyntax['none'] or rendersyntax['any'] -%}
-{{ contents -}}
-{% endif -%}
-{% endmacro %}
-
-{# Wrapper for data rules. A data rule is rendered only when all of the data
-fields it contains have data available. #}
-{% macro datarule() -%}
-{% do rendersyntax.update(none=false) -%}
-{% do renderdata.update(all=true) -%}
-{% set contents -%}
-{{ caller() -}}
-{% endset -%}
-{% if renderdata['all'] -%}
-{% do rendersyntax.update(any=true) -%}
-{{ contents -}}
-{% endif -%}
-{% endmacro %}
-
-{# Wrapper for fields in data rules. If any value wrapped by this macro
-produces an empty string, the entire data rule will be suppressed. #}
-{% macro datafield(value) -%}
-{% if value -%}
-{{ value -}}
-{% else -%}
-{% do renderdata.update(all=false) -%}
-{% endif -%}
-{% endmacro %}
diff --git a/install/share/csrgen/Makefile.am b/install/share/csrgen/Makefile.am
index 2cd6ce2d1..12c62c4d9 100644
--- a/install/share/csrgen/Makefile.am
+++ b/install/share/csrgen/Makefile.am
@@ -12,6 +12,7 @@ rule_DATA =				\
 	rules/dataEmail.json		\
 	rules/dataHostCN.json		\
 	rules/dataUsernameCN.json	\
+	rules/dataSubjectBase.json	\
 	rules/syntaxSAN.json		\
 	rules/syntaxSubject.json	\
 	$(NULL)
@@ -21,7 +22,6 @@ template_DATA =			\
 	templates/certutil_base.tmpl	\
 	templates/openssl_base.tmpl	\
 	templates/openssl_macros.tmpl	\
-	templates/ipa_macros.tmpl	\
 	$(NULL)
 
 EXTRA_DIST =				\
diff --git a/install/share/csrgen/profiles/caIPAserviceCert.json b/install/share/csrgen/profiles/caIPAserviceCert.json
index 0d1be5e9b..114d2ffd4 100644
--- a/install/share/csrgen/profiles/caIPAserviceCert.json
+++ b/install/share/csrgen/profiles/caIPAserviceCert.json
@@ -2,7 +2,8 @@
     {
         "syntax": "syntaxSubject",
         "data": [
-            "dataHostCN"
+            "dataHostCN",
+            "dataSubjectBase"
         ]
     },
     {
diff --git a/install/share/csrgen/profiles/userCert.json b/install/share/csrgen/profiles/userCert.json
index d5f822e46..d6cf5cfff 100644
--- a/install/share/csrgen/profiles/userCert.json
+++ b/install/share/csrgen/profiles/userCert.json
@@ -2,7 +2,8 @@
     {
         "syntax": "syntaxSubject",
         "data": [
-            "dataUsernameCN"
+            "dataUsernameCN",
+            "dataSubjectBase"
         ]
     },
     {
diff --git a/install/share/csrgen/rules/dataDNS.json b/install/share/csrgen/rules/dataDNS.json
index f0aadca3a..2663f1141 100644
--- a/install/share/csrgen/rules/dataDNS.json
+++ b/install/share/csrgen/rules/dataDNS.json
@@ -2,11 +2,14 @@
   "rules": [
     {
       "helper": "openssl",
-      "template": "DNS = {{ipa.datafield(subject.krbprincipalname.0.partition('/')[2].partition('@')[0])}}"
+      "template": "DNS = {{subject.krbprincipalname.0.partition('/')[2].partition('@')[0]}}"
     },
     {
       "helper": "certutil",
-      "template": "dns:{{ipa.datafield(subject.krbprincipalname.0.partition('/')[2].partition('@')[0])|quote}}"
+      "template": "dns:{{subject.krbprincipalname.0.partition('/')[2].partition('@')[0]|quote}}"
     }
-  ]
+  ],
+  "options": {
+    "data_source": "subject.krbprincipalname.0.partition('/')[2].partition('@')[0]"
+  }
 }
diff --git a/install/share/csrgen/rules/dataEmail.json b/install/share/csrgen/rules/dataEmail.json
index cfc1f6014..2eae9fb25 100644
--- a/install/share/csrgen/rules/dataEmail.json
+++ b/install/share/csrgen/rules/dataEmail.json
@@ -2,11 +2,14 @@
   "rules": [
     {
       "helper": "openssl",
-      "template": "email = {{ipa.datafield(subject.mail.0)}}"
+      "template": "email = {{subject.mail.0}}"
     },
     {
       "helper": "certutil",
-      "template": "email:{{ipa.datafield(subject.mail.0)|quote}}"
+      "template": "email:{{subject.mail.0|quote}}"
     }
-  ]
+  ],
+  "options": {
+    "data_source": "subject.mail.0"
+  }
 }
diff --git a/install/share/csrgen/rules/dataHostCN.json b/install/share/csrgen/rules/dataHostCN.json
index 172c7ec51..5c415bb8c 100644
--- a/install/share/csrgen/rules/dataHostCN.json
+++ b/install/share/csrgen/rules/dataHostCN.json
@@ -2,11 +2,14 @@
   "rules": [
     {
       "helper": "openssl",
-      "template": "{{ipa.datafield(config.ipacertificatesubjectbase.0)}}\nCN={{ipa.datafield(subject.krbprincipalname.0.partition('/')[2].partition('@')[0])}}"
+      "template": "CN={{subject.krbprincipalname.0.partition('/')[2].partition('@')[0]}}"
     },
     {
       "helper": "certutil",
-      "template": "CN={{ipa.datafield(subject.krbprincipalname.0.partition('/')[2].partition('@')[0])|quote}},{{ipa.datafield(config.ipacertificatesubjectbase.0)|quote}}"
+      "template": "CN={{subject.krbprincipalname.0.partition('/')[2].partition('@')[0]|quote}}"
     }
-  ]
+  ],
+  "options": {
+    "data_source": "subject.krbprincipalname.0.partition('/')[2].partition('@')[0]"
+  }
 }
diff --git a/install/share/csrgen/rules/dataSubjectBase.json b/install/share/csrgen/rules/dataSubjectBase.json
new file mode 100644
index 000000000..309dfb1ed
--- /dev/null
+++ b/install/share/csrgen/rules/dataSubjectBase.json
@@ -0,0 +1,15 @@
+{
+  "rules": [
+    {
+      "helper": "openssl",
+      "template": "{{config.ipacertificatesubjectbase.0}}"
+    },
+    {
+      "helper": "certutil",
+      "template": "{{config.ipacertificatesubjectbase.0|quote}}"
+    }
+  ],
+  "options": {
+    "data_source": "config.ipacertificatesubjectbase.0"
+  }
+}
diff --git a/install/share/csrgen/rules/dataUsernameCN.json b/install/share/csrgen/rules/dataUsernameCN.json
index c3e240917..37e7e0113 100644
--- a/install/share/csrgen/rules/dataUsernameCN.json
+++ b/install/share/csrgen/rules/dataUsernameCN.json
@@ -2,11 +2,14 @@
   "rules": [
     {
       "helper": "openssl",
-      "template": "{{ipa.datafield(config.ipacertificatesubjectbase.0)}}\nCN={{ipa.datafield(subject.uid.0)}}"
+      "template": "CN={{subject.uid.0}}"
     },
     {
       "helper": "certutil",
-      "template": "CN={{ipa.datafield(subject.uid.0)|quote}},{{ipa.datafield(config.ipacertificatesubjectbase.0)|quote}}"
+      "template": "CN={{subject.uid.0|quote}}"
     }
-  ]
+  ],
+  "options": {
+    "data_source": "subject.uid.0"
+  }
 }
diff --git a/install/share/csrgen/rules/syntaxSubject.json b/install/share/csrgen/rules/syntaxSubject.json
index 7dfa9325d..af6ec03d3 100644
--- a/install/share/csrgen/rules/syntaxSubject.json
+++ b/install/share/csrgen/rules/syntaxSubject.json
@@ -2,14 +2,15 @@
   "rules": [
     {
       "helper": "openssl",
-      "template": "distinguished_name = {% call openssl.section() %}{{ datarules|first }}{% endcall %}"
+      "template": "distinguished_name = {% call openssl.section() %}{{ datarules|reverse|join('\n') }}{% endcall %}"
     },
     {
       "helper": "certutil",
-      "template": "-s {{ datarules|first }}"
+      "template": "-s {{ datarules|join(',') }}"
     }
   ],
   "options": {
-    "required": true
+    "required": true,
+    "data_source_combinator": "and"
   }
 }
diff --git a/install/share/csrgen/templates/certutil_base.tmpl b/install/share/csrgen/templates/certutil_base.tmpl
index 6c6425fc0..a5556fda0 100644
--- a/install/share/csrgen/templates/certutil_base.tmpl
+++ b/install/share/csrgen/templates/certutil_base.tmpl
@@ -1,6 +1,3 @@
-{% raw -%}
-{% import "ipa_macros.tmpl" as ipa -%}
-{%- endraw %}
 #!/bin/bash -e
 
 if [[ $# -lt 1 ]]; then
diff --git a/install/share/csrgen/templates/openssl_base.tmpl b/install/share/csrgen/templates/openssl_base.tmpl
index 597577bcd..2d6c0709d 100644
--- a/install/share/csrgen/templates/openssl_base.tmpl
+++ b/install/share/csrgen/templates/openssl_base.tmpl
@@ -1,6 +1,5 @@
 {% raw -%}
 {% import "openssl_macros.tmpl" as openssl -%}
-{% import "ipa_macros.tmpl" as ipa -%}
 {%- endraw %}
 #!/bin/bash -e
 
diff --git a/ipaclient/csrgen.py b/ipaclient/csrgen.py
index 0ffad7b28..0b9472f21 100644
--- a/ipaclient/csrgen.py
+++ b/ipaclient/csrgen.py
@@ -81,8 +81,6 @@ class Formatter(object):
             keep_trailing_newline=True, undefined=IndexableUndefined)
 
         self.passthrough_globals = {}
-        self._define_passthrough('ipa.syntaxrule')
-        self._define_passthrough('ipa.datarule')
 
     def _define_passthrough(self, call):
 
@@ -109,8 +107,15 @@ class Formatter(object):
         for description, syntax_rule, data_rules in rules:
             data_rules_prepared = [
                 self._prepare_data_rule(rule) for rule in data_rules]
+
+            data_sources = []
+            for rule in data_rules:
+                data_source = rule.options.get('data_source')
+                if data_source:
+                    data_sources.append(data_source)
+
             syntax_rules.append(self._prepare_syntax_rule(
-                syntax_rule, data_rules_prepared, description))
+                syntax_rule, data_rules_prepared, description, data_sources))
 
         template_params = self._get_template_params(syntax_rules)
         base_template = self.jinja2.get_template(
@@ -129,11 +134,9 @@ class Formatter(object):
 
         return combined_template
 
-    def _wrap_rule(self, rule, rule_type):
-        template = '{%% call ipa.%srule() %%}%s{%% endcall %%}' % (
-            rule_type, rule)
-
-        return template
+    def _wrap_conditional(self, rule, condition):
+        rule = '{%% if %s %%}%s{%% endif %%}' % (condition, rule)
+        return rule
 
     def _wrap_required(self, rule, description):
         template = '{%% filter required("%s") %%}%s{%% endfilter %%}' % (
@@ -142,9 +145,16 @@ class Formatter(object):
         return template
 
     def _prepare_data_rule(self, data_rule):
-        return self._wrap_rule(data_rule.template, 'data')
+        template = data_rule.template
+
+        data_source = data_rule.options.get('data_source')
+        if data_source:
+            template = self._wrap_conditional(template, data_source)
+
+        return template
 
-    def _prepare_syntax_rule(self, syntax_rule, data_rules, description):
+    def _prepare_syntax_rule(
+            self, syntax_rule, data_rules, description, data_sources):
         logger.debug('Syntax rule template: %s' % syntax_rule.template)
         template = self.jinja2.from_string(
             syntax_rule.template, globals=self.passthrough_globals)
@@ -156,7 +166,10 @@ class Formatter(object):
             raise errors.CSRTemplateError(reason=_(
                 'Template error when formatting certificate data'))
 
-        prepared_template = self._wrap_rule(rendered, 'syntax')
+        combinator = ' %s ' % syntax_rule.options.get(
+            'data_source_combinator', 'or')
+        condition = combinator.join(data_sources)
+        prepared_template = self._wrap_conditional(rendered, condition)
         if is_required:
             prepared_template = self._wrap_required(
                 prepared_template, description)
@@ -197,10 +210,11 @@ class OpenSSLFormatter(Formatter):
 
         return {'parameters': parameters, 'extensions': extensions}
 
-    def _prepare_syntax_rule(self, syntax_rule, data_rules, description):
+    def _prepare_syntax_rule(
+            self, syntax_rule, data_rules, description, data_sources):
         """Overrides method to pull out whether rule is an extension or not."""
         prepared_template = super(OpenSSLFormatter, self)._prepare_syntax_rule(
-            syntax_rule, data_rules, description)
+            syntax_rule, data_rules, description, data_sources)
         is_extension = syntax_rule.options.get('extension', False)
         return self.SyntaxRule(prepared_template, is_extension)