diff --git a/plugins/module_utils/_keycloak.py b/plugins/module_utils/_keycloak.py index 5097c1e1e9..677b2fe5d0 100644 --- a/plugins/module_utils/_keycloak.py +++ b/plugins/module_utils/_keycloak.py @@ -1428,20 +1428,17 @@ class KeycloakAPI: self.fail_request(e, msg=f"Unable to delete clientscope {cid}: {e}") def get_clientscope_protocolmappers(self, cid, realm: str = "master"): - """Fetch the name and ID of all clientscopes on the Keycloak server. - - To fetch the full data of the group, make a subsequent call to - get_clientscope_by_clientscopeid, passing in the ID of the group you wish to return. + """Fetch all protocolmappers in the clientscope. :param cid: id of clientscope (not name). :param realm: Realm in which the clientscope resides; default 'master'. - :return The protocolmappers of this realm (default "master") + :return The protocolmappers of this clientscope """ protocolmappers_url = URL_CLIENTSCOPE_PROTOCOLMAPPERS.format(id=cid, url=self.baseurl, realm=realm) try: return self._request_and_deserialize(protocolmappers_url, method="GET") except Exception as e: - self.fail_request(e, msg=f"Could not fetch list of protocolmappers in realm {realm}: {e}") + self.fail_request(e, msg=f"Could not fetch list of protocolmappers for client {cid} in realm {realm}: {e}") def get_clientscope_protocolmapper_by_protocolmapperid(self, pid, cid, realm: str = "master"): """Fetch a keycloak clientscope from the provided realm using the clientscope's unique ID. @@ -1450,7 +1447,7 @@ class KeycloakAPI: gid is a UUID provided by the Keycloak API - :param cid: UUID of the protocolmapper to be returned + :param pid: UUID of the protocolmapper to be returned :param cid: UUID of the clientscope to be returned :param realm: Realm in which the clientscope resides; default 'master'. """ @@ -1505,8 +1502,8 @@ class KeycloakAPI: except Exception as e: self.fail_request(e, msg=f"Could not create protocolmapper {mapper_rep['name']} in realm {realm}: {e}") - def update_clientscope_protocolmappers(self, cid, mapper_rep, realm: str = "master"): - """Update an existing clientscope. + def update_clientscope_protocolmapper(self, cid, mapper_rep, realm: str = "master"): + """Update an existing protocolmapper. :param cid: Id of the clientscope. :param mapper_rep: A ProtocolMapperRepresentation of the updated protocolmapper. @@ -1524,6 +1521,21 @@ class KeycloakAPI: e, msg=f"Could not update protocolmappers for clientscope {mapper_rep} in realm {realm}: {e}" ) + def delete_clientscope_protocolmapper(self, cid, pid, realm: str = "master"): + """Delete an existing protocolmapper. + + :param cid: UUID of the clientscope + :param pid: UUID of the protocolmapper to be deleted + :return HTTPResponse object on success + """ + protocolmapper_url = URL_CLIENTSCOPE_PROTOCOLMAPPER.format(url=self.baseurl, realm=realm, id=cid, mapper_id=pid) + try: + return self._request(protocolmapper_url, method="DELETE") + except Exception as e: + self.fail_request( + e, msg=f"Could not delete protocolmappers {pid} for clientscope {cid} in realm {realm}: {e}" + ) + def get_default_clientscopes(self, realm, client_id=None): """Fetch the name and ID of all clientscopes on the Keycloak server. diff --git a/plugins/modules/keycloak_clientscope.py b/plugins/modules/keycloak_clientscope.py index 317ba0ed38..64347e6f9d 100644 --- a/plugins/modules/keycloak_clientscope.py +++ b/plugins/modules/keycloak_clientscope.py @@ -143,6 +143,19 @@ options: description: - A dict of key/value pairs to set as custom attributes for the client_scope. - Values may be single values (for example a string) or a list of strings. + protocol_mappers_behavior: + description: + - Determine how O(protocol_mappers) behave when updating an existing client_scope. + choices: + subset: + - Add missing protocol mappers, do not remove any missing mappers. + exact: + - Make the protocol mappers exactly as specified, adding and removing mappers as needed. + aliases: + - protocolMappersBehavior + default: 'subset' + type: str + version_added: "13.1.0" extends_documentation_fragment: - community.general._keycloak - community.general._keycloak.actiongroup_keycloak @@ -288,6 +301,8 @@ end_state: } """ +import copy + from ansible.module_utils.basic import AnsibleModule from ansible_collections.community.general.plugins.module_utils._keycloak import ( @@ -310,7 +325,7 @@ def normalise_cr(clientscoperep, remove_ids=False): :return: normalised clientscoperep dict """ # Avoid the dict passed in to be modified - clientscoperep = clientscoperep.copy() + clientscoperep = copy.deepcopy(clientscoperep) if "protocolMappers" in clientscoperep: clientscoperep["protocolMappers"] = sorted( @@ -332,7 +347,7 @@ def sanitize_cr(clientscoperep): :param clientscoperep: the clientscoperep dict to be sanitized :return: sanitized clientrep dict """ - result = clientscoperep.copy() + result = copy.deepcopy(clientscoperep) if "secret" in result: result["secret"] = "no_log" if "attributes" in result: @@ -366,6 +381,9 @@ def main(): protocol=dict(type="str", choices=["openid-connect", "saml", "wsfed", "docker-v2"]), attributes=dict(type="dict"), protocol_mappers=dict(type="list", elements="dict", options=protmapper_spec, aliases=["protocolMappers"]), + protocol_mappers_behavior=dict( + type="str", aliases=["protocolMappersBehavior"], choices=["subset", "exact"], default="subset" + ), ) argument_spec.update(meta_args) @@ -393,17 +411,19 @@ def main(): kc = KeycloakAPI(module, connection_header) - realm = module.params.get("realm") - state = module.params.get("state") - cid = module.params.get("id") - name = module.params.get("name") - protocol_mappers = module.params.get("protocol_mappers") + realm = module.params["realm"] + state = module.params["state"] + cid = module.params["id"] + name = module.params["name"] + protocol_mappers = module.params["protocol_mappers"] + protocol_mappers_behavior = module.params["protocol_mappers_behavior"] # Filter and map the parameters names that apply to the client scope clientscope_params = [ x for x in module.params - if x not in list(keycloak_argument_spec().keys()) + ["state", "realm"] and module.params.get(x) is not None + if x not in list(keycloak_argument_spec().keys()) + ["state", "realm", "protocol_mappers_behavior"] + and module.params[x] is not None ] # See if it already exists in Keycloak @@ -415,11 +435,13 @@ def main(): if before_clientscope is None: before_clientscope = {} + before_mappers = before_clientscope["protocolMappers"] if "protocolMappers" in before_clientscope else [] + # Build a proposed changeset from parameters given to this module changeset = {} for clientscope_param in clientscope_params: - new_param_value = module.params.get(clientscope_param) + new_param_value = module.params[clientscope_param] # Unfortunately, the ansible argument spec checker introduces variables with null values when # they are not specified @@ -428,8 +450,17 @@ def main(): changeset[camel(clientscope_param)] = new_param_value # Prepare the desired values using the existing values (non-existence results in a dict that is save to use as a basis) - desired_clientscope = before_clientscope.copy() + desired_clientscope = copy.deepcopy(before_clientscope) desired_clientscope.update(changeset) + desired_mappers_names = [x["name"] for x in protocol_mappers] if protocol_mappers else [] + + if protocol_mappers_behavior == "subset": + # add exsisting mappers to desired object + for mapper in before_mappers: + if mapper["name"] not in desired_mappers_names: + if "protocolMappers" not in desired_clientscope: + desired_clientscope["protocolMappers"] = [] + desired_clientscope["protocolMappers"].append(mapper) # Cater for when it doesn't exist (an empty dict) if not before_clientscope: @@ -502,11 +533,18 @@ def main(): ) if current_protocolmapper is not None: protocol_mapper["id"] = current_protocolmapper["id"] - kc.update_clientscope_protocolmappers(desired_clientscope["id"], protocol_mapper, realm=realm) + kc.update_clientscope_protocolmapper(desired_clientscope["id"], protocol_mapper, realm=realm) # create otherwise else: kc.create_clientscope_protocolmapper(desired_clientscope["id"], protocol_mapper, realm=realm) + if protocol_mappers_behavior == "exact": + # check if we need to delete protocol mappers on the server + for mapper in before_mappers: + if mapper["name"] not in desired_mappers_names: + # raise Exception(mapper) + kc.delete_clientscope_protocolmapper(desired_clientscope["id"], mapper["id"], realm=realm) + after_clientscope = kc.get_clientscope_by_clientscopeid(desired_clientscope["id"], realm=realm) result["end_state"] = after_clientscope diff --git a/tests/unit/plugins/modules/test_keycloak_clientscope.py b/tests/unit/plugins/modules/test_keycloak_clientscope.py index 6ea4a1620f..6104ff879b 100644 --- a/tests/unit/plugins/modules/test_keycloak_clientscope.py +++ b/tests/unit/plugins/modules/test_keycloak_clientscope.py @@ -24,9 +24,11 @@ def patch_keycloak_api( get_clientscope_by_clientscopeid=None, create_clientscope=None, update_clientscope=None, + get_clientscope_protocolmappers=None, get_clientscope_protocolmapper_by_name=None, - update_clientscope_protocolmappers=None, + update_clientscope_protocolmapper=None, create_clientscope_protocolmapper=None, + delete_clientscope_protocolmapper=None, delete_clientscope=None, ): """Mock context manager for patching the methods in PwPolicyIPAClient that contact the IPA server @@ -65,24 +67,36 @@ def patch_keycloak_api( side_effect=get_clientscope_protocolmapper_by_name, ) as mock_get_clientscope_protocolmapper_by_name: with patch.object( - obj, "update_clientscope_protocolmappers", side_effect=update_clientscope_protocolmappers - ) as mock_update_clientscope_protocolmappers: + obj, "update_clientscope_protocolmapper", side_effect=update_clientscope_protocolmapper + ) as mock_update_clientscope_protocolmapper: with patch.object( obj, "create_clientscope_protocolmapper", side_effect=create_clientscope_protocolmapper ) as mock_create_clientscope_protocolmapper: with patch.object( obj, "delete_clientscope", side_effect=delete_clientscope ) as mock_delete_clientscope: - yield ( - mock_get_clientscope_by_name, - mock_get_clientscope_by_clientscopeid, - mock_create_clientscope, - mock_update_clientscope, - mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, - mock_create_clientscope_protocolmapper, - mock_delete_clientscope, - ) + with patch.object( + obj, + "delete_clientscope_protocolmapper", + side_effect=delete_clientscope_protocolmapper, + ) as mock_delete_clientscope_protocolmapper: + with patch.object( + obj, + "get_clientscope_protocolmappers", + side_effect=get_clientscope_protocolmappers, + ) as mock_get_clientscope_protocolmappers: + yield ( + mock_get_clientscope_by_name, + mock_get_clientscope_by_clientscopeid, + mock_create_clientscope, + mock_update_clientscope, + mock_get_clientscope_protocolmappers, + mock_get_clientscope_protocolmapper_by_name, + mock_update_clientscope_protocolmapper, + mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, + mock_delete_clientscope, + ) def get_response(object_with_future_response, method, get_id_call_count): @@ -163,9 +177,11 @@ class TestKeycloakAuthentication(ModuleTestCase): mock_get_clientscope_by_clientscopeid, mock_create_clientscope, mock_update_clientscope, + mock_get_clientscope_protocolmappers, mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, + mock_update_clientscope_protocolmapper, mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, mock_delete_clientscope, ): with self.assertRaises(AnsibleExitJson) as exec_info: @@ -177,8 +193,9 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 0) self.assertEqual(mock_update_clientscope.call_count, 0) self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 0) - self.assertEqual(mock_update_clientscope_protocolmappers.call_count, 0) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_delete_clientscope.call_count, 0) # Verify that the module's changed status matches what is expected @@ -211,9 +228,11 @@ class TestKeycloakAuthentication(ModuleTestCase): mock_get_clientscope_by_clientscopeid, mock_create_clientscope, mock_update_clientscope, + mock_get_clientscope_protocolmappers, mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, + mock_update_clientscope_protocolmapper, mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, mock_delete_clientscope, ): with self.assertRaises(AnsibleExitJson) as exec_info: @@ -225,8 +244,9 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 0) self.assertEqual(mock_update_clientscope.call_count, 0) self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 0) - self.assertEqual(mock_update_clientscope_protocolmappers.call_count, 0) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_delete_clientscope.call_count, 0) # Verify that the module's changed status matches what is expected @@ -259,9 +279,11 @@ class TestKeycloakAuthentication(ModuleTestCase): mock_get_clientscope_by_clientscopeid, mock_create_clientscope, mock_update_clientscope, + mock_get_clientscope_protocolmappers, mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, + mock_update_clientscope_protocolmapper, mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, mock_delete_clientscope, ): with self.assertRaises(AnsibleExitJson) as exec_info: @@ -273,8 +295,9 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 0) self.assertEqual(mock_update_clientscope.call_count, 0) self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 0) - self.assertEqual(mock_update_clientscope_protocolmappers.call_count, 0) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_delete_clientscope.call_count, 1) # Verify that the module's changed status matches what is expected @@ -305,9 +328,11 @@ class TestKeycloakAuthentication(ModuleTestCase): mock_get_clientscope_by_clientscopeid, mock_create_clientscope, mock_update_clientscope, + mock_get_clientscope_protocolmappers, mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, + mock_update_clientscope_protocolmapper, mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, mock_delete_clientscope, ): with self.assertRaises(AnsibleExitJson) as exec_info: @@ -319,8 +344,9 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 0) self.assertEqual(mock_update_clientscope.call_count, 0) self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 0) - self.assertEqual(mock_update_clientscope_protocolmappers.call_count, 0) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_delete_clientscope.call_count, 0) # Verify that the module's changed status matches what is expected @@ -440,9 +466,11 @@ class TestKeycloakAuthentication(ModuleTestCase): mock_get_clientscope_by_clientscopeid, mock_create_clientscope, mock_update_clientscope, + mock_get_clientscope_protocolmappers, mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, + mock_update_clientscope_protocolmapper, mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, mock_delete_clientscope, ): with self.assertRaises(AnsibleExitJson) as exec_info: @@ -454,8 +482,9 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 0) self.assertEqual(mock_update_clientscope.call_count, 0) self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 0) - self.assertEqual(mock_update_clientscope_protocolmappers.call_count, 0) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_delete_clientscope.call_count, 0) # Verify that the module's changed status matches what is expected @@ -508,6 +537,7 @@ class TestKeycloakAuthentication(ModuleTestCase): }, "name": "protocol3", "protocolMapper": "oidc-group-membership-mapper", + "id": "a7f19adb-cc58-41b1-94ce-782dc255139b", }, ], } @@ -628,9 +658,11 @@ class TestKeycloakAuthentication(ModuleTestCase): mock_get_clientscope_by_clientscopeid, mock_create_clientscope, mock_update_clientscope, + mock_get_clientscope_protocolmappers, mock_get_clientscope_protocolmapper_by_name, - mock_update_clientscope_protocolmappers, + mock_update_clientscope_protocolmapper, mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, mock_delete_clientscope, ): with self.assertRaises(AnsibleExitJson) as exec_info: @@ -641,10 +673,184 @@ class TestKeycloakAuthentication(ModuleTestCase): self.assertEqual(mock_create_clientscope.call_count, 0) self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 1) self.assertEqual(mock_update_clientscope.call_count, 1) + self.assertEqual(mock_get_clientscope_protocolmappers.call_count, 0) self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 3) - self.assertEqual(mock_update_clientscope_protocolmappers.call_count, 3) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 3) self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 0) self.assertEqual(mock_delete_clientscope.call_count, 0) # Verify that the module's changed status matches what is expected self.assertIs(exec_info.exception.args[0]["changed"], changed) + + def test_update_clientscope_with_deleted_protocolmappers(self): + """Add a new authentication flow from copy of an other flow""" + + module_args = { + "auth_keycloak_url": "http://keycloak.url/auth", + "auth_username": "admin", + "auth_password": "admin", + "auth_realm": "master", + "realm": "realm-name", + "state": "present", + "name": "my-new-kc-clientscope", + "protocol_mappers_behavior": "exact", + "protocolMappers": [ + { + "protocol": "openid-connect", + "config": { + "full.path": "false", + "id.token.claim": "false", + "access.token.claim": "false", + "userinfo.token.claim": "false", + "claim.name": "protocol1_updated", + }, + "name": "protocol1", + "protocolMapper": "oidc-group-membership-mapper", + }, + { + "protocol": "openid-connect", + "config": { + "full.path": "true", + "id.token.claim": "false", + "access.token.claim": "false", + "userinfo.token.claim": "false", + "claim.name": "protocol2_updated", + }, + "name": "protocol2", + "protocolMapper": "oidc-group-membership-mapper", + }, + ], + } + # before + return_value_get_clientscope_by_name = [ + { + "attributes": {}, + "id": "890ec72e-fe1d-4308-9f27-485ef7eaa182", + "name": "my-new-kc-clientscope", + "protocolMappers": [ + { + "protocol": "openid-connect", + "config": { + "full.path": "false", + "id.token.claim": "false", + "access.token.claim": "false", + "userinfo.token.claim": "false", + "claim.name": "protocol1_updated", + }, + "name": "protocol1", + "protocolMapper": "oidc-group-membership-mapper", + "id": "p1", + }, + { + "protocol": "openid-connect", + "config": { + "full.path": "true", + "id.token.claim": "false", + "access.token.claim": "false", + "userinfo.token.claim": "false", + "claim.name": "protocol2_updated", + }, + "name": "protocol2", + "protocolMapper": "oidc-group-membership-mapper", + "id": "p2", + }, + { + "protocol": "openid-connect", + "config": { + "full.path": "true", + "id.token.claim": "true", + "access.token.claim": "true", + "userinfo.token.claim": "true", + "claim.name": "protocol3_updated", + }, + "name": "protocol3", + "protocolMapper": "oidc-group-membership-mapper", + "id": "p3", + }, + ], + } + ] + # after + return_value_get_clientscope_by_clientscopeid = [ + { + "attributes": {}, + "id": "2286032f-451e-44d5-8be6-e45aac7983a1", + "name": "my-new-kc-clientscope", + "protocolMappers": [ + { + "config": { + "access.token.claim": "true", + "claim.name": "protocol2_updated", + "full.path": "true", + "id.token.claim": "false", + "userinfo.token.claim": "false", + }, + "consentRequired": "false", + "id": "p2", + "name": "protocol2", + "protocol": "openid-connect", + "protocolMapper": "oidc-group-membership-mapper", + }, + { + "config": { + "access.token.claim": "false", + "claim.name": "protocol1_updated", + "full.path": "false", + "id.token.claim": "false", + "userinfo.token.claim": "false", + }, + "consentRequired": "false", + "id": "p1", + "name": "protocol1", + "protocol": "openid-connect", + "protocolMapper": "oidc-group-membership-mapper", + }, + ], + }, + ] + + changed = True + + # Run the module + + with set_module_args(module_args): + with mock_good_connection(): + with patch_keycloak_api( + get_clientscope_by_name=return_value_get_clientscope_by_name, + get_clientscope_by_clientscopeid=return_value_get_clientscope_by_clientscopeid, + ) as ( + mock_get_clientscope_by_name, + mock_get_clientscope_by_clientscopeid, + mock_create_clientscope, + mock_update_clientscope, + mock_get_clientscope_protocolmappers, + mock_get_clientscope_protocolmapper_by_name, + mock_update_clientscope_protocolmapper, + mock_create_clientscope_protocolmapper, + mock_delete_clientscope_protocolmapper, + mock_delete_clientscope, + ): + with self.assertRaises(AnsibleExitJson) as exec_info: + self.module.main() + + # Verify number of call on each mock + self.assertEqual(mock_get_clientscope_by_name.call_count, 1) + self.assertEqual(mock_create_clientscope.call_count, 0) + self.assertEqual(mock_get_clientscope_by_clientscopeid.call_count, 1) + self.assertEqual(mock_update_clientscope.call_count, 1) + # will not be called, because mock_get_clientscope_protocolmapper_by_name is patched + self.assertEqual(mock_get_clientscope_protocolmappers.call_count, 0) + self.assertEqual(mock_get_clientscope_protocolmapper_by_name.call_count, 2) + self.assertEqual(mock_update_clientscope_protocolmapper.call_count, 2) + self.assertEqual(mock_create_clientscope_protocolmapper.call_count, 0) + self.assertEqual(mock_delete_clientscope_protocolmapper.call_count, 1) + self.assertEqual(mock_delete_clientscope.call_count, 0) + + # expect "p3" to be deleted + mock_delete_clientscope_protocolmapper.assert_called_with( + return_value_get_clientscope_by_name[0]["id"], "p3", realm=module_args["realm"] + ) + + # Verify that the module's changed status matches what is expected + self.assertIs(exec_info.exception.args[0]["changed"], changed)