From ff084fbd96b443f175a0d99b6e5621da9b404cdb Mon Sep 17 00:00:00 2001
From: Thomas Woerner <twoerner@redhat.com>
Date: Mon, 4 Dec 2023 19:38:54 +0100
Subject: [PATCH] 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
---
.../module_utils/ansible_freeipa_module.py | 8 +-
plugins/modules/ipaidp.py | 60 +++++--
tests/idp/test_idp.yml | 156 ++++++++++++++++++
3 files changed, 208 insertions(+), 16 deletions(-)
diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py
index dfec4c58..190f585a 100644
--- a/plugins/module_utils/ansible_freeipa_module.py
+++ b/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)
diff --git a/plugins/modules/ipaidp.py b/plugins/modules/ipaidp.py
index a3c1cea7..8c0aea6b 100644
--- a/plugins/modules/ipaidp.py
+++ b/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":
diff --git a/tests/idp/test_idp.yml b/tests/idp/test_idp.yml
index c2c78e38..d72ba1c5 100644
--- a/tests/idp/test_idp.yml
+++ b/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
--
GitLab