ipaidp: Fix validation and reset of parameters

The uri parameters auth_uri, dev_auth_uri, token_uri, userinfo_uri and
keys_uri have not been validated before. Also the base_url was not
normalized. The auth_uri, dev_auth_uri, token_uri and userinfo_uri need
to be set for new entries, but might be empty or empty string for reset
or updates.

The ipaidpclientsecret needs to be decoded from binary string in
find_idp result to not trigger no change ipd_mod calls.

The code for validate_uri and base_url normalization has been copied
from the ipaserver idp plugin.

ansible_freeipa_module:
urlparse from urllib.parse with a fallback to six.moves.urllib.parse is
imported and also exported. urlparse is needed for validate_uri in ipaidp
module.

Resolves: RHEL-17954, RHEL-17955, RHEL-17957 and RHEL-17958

plugins/module_utils/ansible_freeipa_module.py

@@ -30,7 +30,8 @@ __all__ = ["gssapi", "netaddr", "api", "ipalib_errors", "Env",
            "kinit_password", "kinit_keytab", "run", "DN", "VERSION",
            "paths", "tasks", "get_credentials_if_valid", "Encoding",
            "DNSName", "getargspec", "certificate_loader",
-           "write_certificate_list", "boolean", "template_str"]
+           "write_certificate_list", "boolean", "template_str",
+           "urlparse"]
 
 import os
 # ansible-freeipa requires locale to be C, IPA requires utf-8.
@@ -147,6 +148,11 @@ try:
     except ImportError:
         _dcerpc_bindings_installed = False  # pylint: disable=invalid-name
 
+    try:
+        from urllib.parse import urlparse
+    except ImportError:
+        from ansible.module_utils.six.moves.urllib.parse import urlparse
+
 except ImportError as _err:
     ANSIBLE_FREEIPA_MODULE_IMPORT_ERROR = str(_err)
 

plugins/modules/ipaidp.py

@@ -185,7 +185,7 @@ RETURN = """
 
 
 from ansible.module_utils.ansible_freeipa_module import \
-    IPAAnsibleModule, compare_args_ipa, template_str
+    IPAAnsibleModule, compare_args_ipa, template_str, urlparse
 from ansible.module_utils import six
 from copy import deepcopy
 import string
@@ -274,7 +274,14 @@ def find_idp(module, name):
         # An exception is raised if idp name is not found.
         return None
 
-    return _result["result"]
+    res = _result["result"]
+
+    # Decode binary string secret
+    if "ipaidpclientsecret" in res and len(res["ipaidpclientsecret"]) > 0:
+        res["ipaidpclientsecret"][0] = \
+            res["ipaidpclientsecret"][0].decode("ascii")
+
+    return res
 
 
 def gen_args(auth_uri, dev_auth_uri, token_uri, userinfo_uri, keys_uri,
@@ -333,6 +340,16 @@ def convert_provider_to_endpoints(module, _args, provider):
     _args.update(points)
 
 
+def validate_uri(module, uri):
+    try:
+        parsed = urlparse(uri, 'https')
+    except Exception:
+        module.fail_json(msg="Invalid URI '%s': not an https scheme" % uri)
+
+    if not parsed.netloc:
+        module.fail_json(msg="Invalid URI '%s': missing netloc" % uri)
+
+
 def main():
     ansible_module = IPAAnsibleModule(
         argument_spec=dict(
@@ -426,19 +443,6 @@ def main():
             if provider not in idp_providers:
                 ansible_module.fail_json(
                     msg="Provider '%s' is unknown" % provider)
-        else:
-            if not auth_uri:
-                ansible_module.fail_json(
-                    msg="Parameter '%s' is missing" % "auth_uri")
-            if not dev_auth_uri:
-                ansible_module.fail_json(
-                    msg="Parameter '%s' is missing" % "dev_auth_uri")
-            if not token_uri:
-                ansible_module.fail_json(
-                    msg="Parameter '%s' is missing" % "token_uri")
-            if not userinfo_uri:
-                ansible_module.fail_json(
-                    msg="Parameter '%s' is missing" % "userinfo_uri")
         invalid = ["rename", "delete_continue"]
     else:
         # state renamed and absent
@@ -459,6 +463,19 @@ def main():
 
     ansible_module.params_fail_used_invalid(invalid, state)
 
+    # Empty client_id test
+    if client_id is not None and client_id == "":
+        ansible_module.fail_json(msg="'client_id' is required")
+
+    # Normalize base_url
+    if base_url is not None and base_url.startswith('https://'):
+        base_url = base_url[len('https://'):]
+
+    # Validate uris
+    for uri in [auth_uri, dev_auth_uri, token_uri, userinfo_uri, keys_uri]:
+        if uri is not None and uri != "":
+            validate_uri(ansible_module, uri)
+
     # Init
 
     changed = False
@@ -507,6 +524,19 @@ def main():
                                             res_find):
                         commands.append([name, "idp_mod", args])
                 else:
+                    if "ipaidpauthendpoint" not in args:
+                        ansible_module.fail_json(
+                            msg="Parameter '%s' is missing" % "auth_uri")
+                    if "ipaidpdevauthendpoint" not in args:
+                        ansible_module.fail_json(
+                            msg="Parameter '%s' is missing" % "dev_auth_uri")
+                    if "ipaidptokenendpoint" not in args:
+                        ansible_module.fail_json(
+                            msg="Parameter '%s' is missing" % "token_uri")
+                    if "ipaidpuserinfoendpoint" not in args:
+                        ansible_module.fail_json(
+                            msg="Parameter '%s' is missing" % "userinfo_uri")
+
                     commands.append([name, "idp_add", args])
 
             elif state == "absent":

tests/idp/test_idp.yml

@@ -62,6 +62,162 @@
       register: result
       failed_when: result.changed or result.failed
 
+    - name: Ensure keycloak idp my-keycloak-idp is present with all params
+      ipaidp:
+        name: my-keycloak-idp
+        organization: main
+        base_url: https://keycloak.idm.example.com:8443/auth
+        keys_uri: https://keycloak.idm.example.com:8443/certs
+        issuer_url: https://keycloak.idm.example.com:8443/issuer
+        secret: secret
+        scope: https://keycloak.idm.example.com:8443/scope
+        idp_user_id: testuser
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp is present with all params, again
+      ipaidp:
+        name: my-keycloak-idp
+        organization: main
+        base_url: https://keycloak.idm.example.com:8443/auth
+        keys_uri: https://keycloak.idm.example.com:8443/certs
+        issuer_url: https://keycloak.idm.example.com:8443/issuer
+        secret: secret
+        scope: https://keycloak.idm.example.com:8443/scope
+        idp_user_id: testuser
+        client_id: my-client-id
+      register: result
+      # failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp auth_uri is empty
+      ipaidp:
+        name: my-keycloak-idp
+        auth_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp auth_uri is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        auth_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp dev_auth_uri is empty
+      ipaidp:
+        name: my-keycloak-idp
+        dev_auth_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp dev_auth_uri is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        dev_auth_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp token_uri is empty
+      ipaidp:
+        name: my-keycloak-idp
+        token_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp token_uri is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        token_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp userinfo_uri is empty
+      ipaidp:
+        name: my-keycloak-idp
+        userinfo_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp userinfo_uri is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        userinfo_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp keys_uri is empty
+      ipaidp:
+        name: my-keycloak-idp
+        keys_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp keys_uri is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        keys_uri: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp issuer_url is empty
+      ipaidp:
+        name: my-keycloak-idp
+        issuer_url: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp issuer_url is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        issuer_url: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp scope is empty
+      ipaidp:
+        name: my-keycloak-idp
+        scope: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp scope is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        scope: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp idp_user_id is empty
+      ipaidp:
+        name: my-keycloak-idp
+        idp_user_id: ""
+        client_id: my-client-id
+      register: result
+      failed_when: not result.changed or result.failed
+
+    - name: Ensure keycloak idp my-keycloak-idp idp_user_id is empty, again
+      ipaidp:
+        name: my-keycloak-idp
+        idp_user_id: ""
+        client_id: my-client-id
+      register: result
+      failed_when: result.changed or result.failed
+
     - name: Ensure idp my-keycloak-idp is absent
       ipaidp:
         name: my-keycloak-idp