ipauser: Fix certmapdata, add missing certmapdata data option

certmapdata was not processed properly. The certificate was not loaded and
therefore the `issuer` and `subject` could not be compared to the
certmapdata entries in the user record. The function `load_cert_from_str`
from ansible_freeipa_moduleis used for this.

Additionally there was no way to use the certmapdata data format. This
is now possible with the `data` option in the certmapdata dict.

Example: "data: X509:<I>dc=com,dc=example,CN=ca<S>dc=com,dc=example,CN=test"

`data` may not be used together with `certificate`, `issuer` and `subject`
in the same record.

Given certmapdata for the ipauser module is now converted to the internal
data representation using also the new function `DN_x500_text` from
`ansible_freeipa_module`.

New functions `convert_certmapdata` and `check_certmapdata` have been added
to ipauser.

tests/user/certmapdata/test_user_certmapdata.yml has been extended with
additional tasks to verify more complex issuer and subjects and also using
the data format.

README-user.md

@@ -417,10 +417,11 @@ Variable | Description | Required
 `employeetype` | Employee Type | no
 `preferredlanguage` | Preferred Language | no
 `certificate` | List of base-64 encoded user certificates. | no
-`certmapdata` | List of certificate mappings. Either `certificate` or `issuer` together with `subject` need to be specified. <br>Options: | no
-&nbsp; | `certificate` - Base-64 encoded user certificate | no
-&nbsp; | `issuer` - Issuer of the certificate | no
-&nbsp; | `subject` - Subject of the certificate | no
+`certmapdata` | List of certificate mappings. Either `data` or `certificate` or `issuer` together with `subject` need to be specified. Only usable with IPA versions 4.5 and up. <br>Options: | no
+&nbsp; | `certificate` - Base-64 encoded user certificate, not usable with other certmapdata options. | no
+&nbsp; | `issuer` - Issuer of the certificate, only usable together with `usbject` option. | no
+&nbsp; | `subject` - Subject of the certificate, only usable together with `issuer` option. | no
+&nbsp; | `data` - Certmap data, not usable with other certmapdata options. | no
 `noprivate` | Do not create user private group. (bool) | no
 `nomembers` | Suppress processing of membership attributes. (bool) | no
 

plugins/modules/ipauser.py

@@ -186,7 +186,9 @@ options:
         description: List of base-64 encoded user certificates
         required: false
       certmapdata:
-        description: List of certificate mappings
+        description:
+        - List of certificate mappings
+        - Only usable with IPA versions 4.5 and up.
         options:
           certificate:
             description: Base-64 encoded user certificate
@@ -197,6 +199,9 @@ options:
           subject:
             description: Subject of the certificate
             required: false
+          data:
+            description: Certmap data
+            required: false
         required: false
       noprivate:
         description: Don't create user private group
@@ -346,7 +351,9 @@ options:
     description: List of base-64 encoded user certificates
     required: false
   certmapdata:
-    description: List of certificate mappings
+    description:
+    - List of certificate mappings
+    - Only usable with IPA versions 4.5 and up.
     options:
       certificate:
         description: Base-64 encoded user certificate
@@ -357,6 +364,9 @@ options:
       subject:
         description: Subject of the certificate
         required: false
+      data:
+        description: Certmap data
+        required: false
     required: false
   noprivate:
     description: Don't create user private group
@@ -467,7 +477,8 @@ from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
     temp_kdestroy, valid_creds, api_connect, api_command, date_format, \
     compare_args_ipa, module_params_get, api_check_param, api_get_realm, \
-    api_command_no_name, gen_add_del_lists, encode_certificate
+    api_command_no_name, gen_add_del_lists, encode_certificate, \
+    load_cert_from_str, DN_x500_text, api_check_command
 import six
 
 
@@ -645,13 +656,21 @@ def check_parameters(module, state, action,
             certificate = x.get("certificate")
             issuer = x.get("issuer")
             subject = x.get("subject")
+            data = x.get("data")
 
+            if data is not None:
+                if certificate is not None or issuer is not None or \
+                   subject is not None:
+                    module.fail_json(
+                        msg="certmapdata: data can not be used with "
+                        "certificate, issuer or subject")
+                check_certmapdata(data)
             if certificate is not None \
                and (issuer is not None or subject is not None):
                 module.fail_json(
                     msg="certmapdata: certificate can not be used with "
                     "issuer or subject")
-            if certificate is None:
+            if data is None and certificate is None:
                 if issuer is None:
                     module.fail_json(msg="certmapdata: issuer is missing")
                 if subject is None:
@@ -666,19 +685,48 @@ def extend_emails(email, default_email_domain):
     return email
 
 
-def gen_certmapdata_args(certmapdata):
-    certificate = certmapdata.get("certificate")
-    issuer = certmapdata.get("issuer")
-    subject = certmapdata.get("subject")
+def convert_certmapdata(certmapdata):
+    if certmapdata is None:
+        return None
 
-    _args = {}
-    if certificate is not None:
-        _args["certificate"] = certificate
-    if issuer is not None:
-        _args["issuer"] = issuer
-    if subject is not None:
-        _args["subject"] = subject
-    return _args
+    _result = []
+    for x in certmapdata:
+        certificate = x.get("certificate")
+        issuer = x.get("issuer")
+        subject = x.get("subject")
+        data = x.get("data")
+
+        if data is None:
+            if issuer is None and subject is None:
+                cert = load_cert_from_str(certificate)
+                issuer = cert.issuer
+                subject = cert.subject
+
+            _result.append("X509:<I>%s<S>%s" % (DN_x500_text(issuer),
+                                                DN_x500_text(subject)))
+        else:
+            _result.append(data)
+
+    return _result
+
+
+def check_certmapdata(data):
+    if not data.startswith("X509:"):
+        return False
+
+    i = data.find("<I>", 4)
+    s = data.find("<S>", i)
+    issuer = data[i+3:s]
+    subject = data[s+3:]
+
+    if i < 0 or s < 0 or "CN" not in issuer or "CN" not in subject:
+        return False
+
+    return True
+
+
+def gen_certmapdata_args(certmapdata):
+    return {"ipacertmapdata": to_text(certmapdata)}
 
 
 def main():
@@ -740,7 +788,8 @@ def main():
                              # Here certificate is a simple string
                              certificate=dict(type="str", default=None),
                              issuer=dict(type="str", default=None),
-                             subject=dict(type="str", default=None)
+                             subject=dict(type="str", default=None),
+                             data=dict(type="str", default=None)
                          ),
                          elements='dict', required=False),
         noprivate=dict(type='bool', default=None),
@@ -875,6 +924,7 @@ def main():
         departmentnumber, employeenumber, employeetype, preferredlanguage,
         certificate, certmapdata, noprivate, nomembers, preserve,
         update_password)
+    certmapdata = convert_certmapdata(certmapdata)
 
     # Use users if names is None
     if users is not None:
@@ -972,6 +1022,7 @@ def main():
                     employeetype, preferredlanguage, certificate,
                     certmapdata, noprivate, nomembers, preserve,
                     update_password)
+                certmapdata = convert_certmapdata(certmapdata)
 
                 # Extend email addresses
 
@@ -1001,6 +1052,16 @@ def main():
                     msg="The use of passwordexpiration is not supported by "
                     "your IPA version")
 
+            # Check certmapdata availability.
+            # We need the connected API for this test, therefore it can not
+            # be part of check_parameters as this is used also before the
+            # connection to the API has been established.
+            if certmapdata is not None and \
+               not api_check_command("user_add_certmapdata"):
+                ansible_module.fail_json(
+                    msg="The use of certmapdata is not supported by "
+                    "your IPA version")
+
             # Make sure user exists
             res_find = find_user(ansible_module, name)
             # Also search for preserved user if the user could not be found
@@ -1082,7 +1143,7 @@ def main():
                             certificate, res_find.get("usercertificate"))
 
                         certmapdata_add, certmapdata_del = gen_add_del_lists(
-                            certmapdata, res_find.get("ipaCertMapData"))
+                            certmapdata, res_find.get("ipacertmapdata"))
 
                     else:
                         # Use given managers and principals
@@ -1169,7 +1230,7 @@ def main():
                     # Remove certmapdata
                     if len(certmapdata_del) > 0:
                         for _data in certmapdata_del:
-                            commands.append([name, "user_add_certmapdata",
+                            commands.append([name, "user_remove_certmapdata",
                                              gen_certmapdata_args(_data)])
 
                 elif action == "member":

tests/user/certmapdata/test_user_certmapdata.yml

@@ -126,8 +126,6 @@
       certmapdata:
       - issuer: CN=issuer1
         subject: CN=subject1
-      - issuer: CN=issuer2
-        subject: CN=subject2
       - issuer: CN=issuer3
         subject: CN=subject3
       action: member
@@ -142,8 +140,6 @@
       certmapdata:
       - issuer: CN=issuer1
         subject: CN=subject1
-      - issuer: CN=issuer2
-        subject: CN=subject2
       - issuer: CN=issuer3
         subject: CN=subject3
       action: member
@@ -151,6 +147,85 @@
     register: result
     failed_when: result.changed
 
+  - name: User test certmapdata members absent
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - issuer: CN=issuer2
+        subject: CN=subject2
+      action: member
+      state: absent
+    register: result
+    failed_when: not result.changed
+
+  - name: User test certmapdata members absent again
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - issuer: CN=issuer2
+        subject: CN=subject2
+      action: member
+      state: absent
+    register: result
+    failed_when: result.changed
+
+  - name: User test certmapdata member present
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - issuer: CN=ca,dc=example,dc=com
+        subject: CN=test,dc=example,dc=com
+      action: member
+    register: result
+    failed_when: not result.changed
+
+  - name: User test certmapdata member present again
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - issuer: CN=ca,dc=example,dc=com
+        subject: CN=test,dc=example,dc=com
+      action: member
+    register: result
+    failed_when: result.changed
+
+  - name: User test certmapdata member (data) present again
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - data: X509:<I>dc=com,dc=example,CN=ca<S>dc=com,dc=example,CN=test
+      action: member
+    register: result
+    failed_when: result.changed
+
+  - name: User test certmapdata member absent
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - issuer: CN=ca,dc=example,dc=com
+        subject: CN=test,dc=example,dc=com
+      action: member
+      state: absent
+    register: result
+    failed_when: not result.changed
+
+  - name: User test certmapdata member (data) absent again
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      name: test
+      certmapdata:
+      - data: X509:<I>dc=com,dc=example,CN=ca<S>dc=com,dc=example,CN=test
+      action: member
+      state: absent
+    register: result
+    failed_when: result.changed
+
   - name: User test absent
     ipauser:
       ipaadmin_password: SomeADMINpassword