1
0
Fork 0
mirror of https://github.com/ansible-collections/community.general.git synced 2024-09-14 20:13:21 +02:00

keycloak_identity_provider: Fix mappers update (#3538)

* set identityprovideralias by default

* refactor mappers change detection

* fix sanity check

* update tests

* add changelog fragment

* Update changelogs/fragments/3538-fix-keycloak-idp-mappers-change-detection.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
This commit is contained in:
Laurent Paumier 2021-10-11 22:43:50 +02:00 committed by GitHub
parent 9772485d3c
commit d44c85aa90
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 149 additions and 18 deletions

View file

@ -0,0 +1,2 @@
bugfixes:
- keycloak_identity_provider - fix change detection when updating identity provider mappers (https://github.com/ansible-collections/community.general/pull/3538, https://github.com/ansible-collections/community.general/issues/3537).

View file

@ -295,6 +295,20 @@ EXAMPLES = '''
clientAuthMethod: client_secret_post clientAuthMethod: client_secret_post
clientId: my-client clientId: my-client
clientSecret: secret clientSecret: secret
syncMode: FORCE
mappers:
- name: first_name
identityProviderMapper: oidc-user-attribute-idp-mapper
config:
claim: first_name
user.attribute: first_name
syncMode: INHERIT
- name: last_name
identityProviderMapper: oidc-user-attribute-idp-mapper
config:
claim: last_name
user.attribute: last_name
syncMode: INHERIT
- name: Create SAML identity provider, authentication with credentials - name: Create SAML identity provider, authentication with credentials
community.general.keycloak_identity_provider: community.general.keycloak_identity_provider:
@ -313,6 +327,14 @@ EXAMPLES = '''
singleSignOnServiceUrl: https://idp.example.com/login singleSignOnServiceUrl: https://idp.example.com/login
wantAuthnRequestsSigned: true wantAuthnRequestsSigned: true
wantAssertionsSigned: true wantAssertionsSigned: true
mappers:
- name: roles
identityProviderMapper: saml-user-attribute-idp-mapper
config:
user.attribute: roles
attribute.friendly.name: User Roles
attribute.name: roles
syncMode: INHERIT
''' '''
RETURN = ''' RETURN = '''
@ -400,15 +422,15 @@ end_state:
from ansible_collections.community.general.plugins.module_utils.identity.keycloak.keycloak import KeycloakAPI, camel, \ from ansible_collections.community.general.plugins.module_utils.identity.keycloak.keycloak import KeycloakAPI, camel, \
keycloak_argument_spec, get_token, KeycloakError keycloak_argument_spec, get_token, KeycloakError
from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.basic import AnsibleModule
from copy import deepcopy
def sanitize(idp): def sanitize(idp):
result = idp.copy() idpcopy = deepcopy(idp)
if 'config' in result: if 'config' in idpcopy:
result['config'] = sanitize(result['config']) if 'clientSecret' in idpcopy['config']:
if 'clientSecret' in result: idpcopy['clientSecret'] = '**********'
result['clientSecret'] = '**********' return idpcopy
return result
def get_identity_provider_with_mappers(kc, alias, realm): def get_identity_provider_with_mappers(kc, alias, realm):
@ -493,18 +515,29 @@ def main():
changeset[camel(param)] = new_param_value changeset[camel(param)] = new_param_value
# special handling of mappers list to allow change detection # special handling of mappers list to allow change detection
changeset['mappers'] = before_idp.get('mappers', list())
if module.params.get('mappers') is not None: if module.params.get('mappers') is not None:
for new_mapper in module.params.get('mappers'): for change in module.params['mappers']:
old_mapper = next((x for x in changeset['mappers'] if x['name'] == new_mapper['name']), None) change = dict((k, v) for k, v in change.items() if change[k] is not None)
new_mapper = dict((k, v) for k, v in new_mapper.items() if new_mapper[k] is not None) if change.get('id') is None and change.get('name') is None:
if old_mapper is not None: module.fail_json(msg='Either `name` or `id` has to be specified on each mapper.')
old_mapper.update(new_mapper) if before_idp == dict():
old_mapper = dict()
elif change.get('id') is not None:
old_mapper = kc.get_identity_provider_mapper(change['id'], alias, realm)
if old_mapper is None:
old_mapper = dict()
else: else:
found = [x for x in kc.get_identity_provider_mappers(alias, realm) if x['name'] == change['name']]
if len(found) == 1:
old_mapper = found[0]
else:
old_mapper = dict()
new_mapper = old_mapper.copy()
new_mapper.update(change)
if new_mapper != old_mapper:
if changeset.get('mappers') is None:
changeset['mappers'] = list()
changeset['mappers'].append(new_mapper) changeset['mappers'].append(new_mapper)
# remove mappers if not present in module params
changeset['mappers'] = [x for x in changeset['mappers']
if [y for y in module.params.get('mappers', []) if y['name'] == x['name']] != []]
# prepare the new representation # prepare the new representation
updated_idp = before_idp.copy() updated_idp = before_idp.copy()
@ -538,6 +571,8 @@ def main():
mappers = updated_idp.pop('mappers', []) mappers = updated_idp.pop('mappers', [])
kc.create_identity_provider(updated_idp, realm) kc.create_identity_provider(updated_idp, realm)
for mapper in mappers: for mapper in mappers:
if mapper.get('identityProviderAlias') is None:
mapper['identityProviderAlias'] = alias
kc.create_identity_provider_mapper(mapper, alias, realm) kc.create_identity_provider_mapper(mapper, alias, realm)
after_idp = get_identity_provider_with_mappers(kc, alias, realm) after_idp = get_identity_provider_with_mappers(kc, alias, realm)
@ -572,6 +607,8 @@ def main():
if mapper.get('id') is not None: if mapper.get('id') is not None:
kc.update_identity_provider_mapper(mapper, alias, realm) kc.update_identity_provider_mapper(mapper, alias, realm)
else: else:
if mapper.get('identityProviderAlias') is None:
mapper['identityProviderAlias'] = alias
kc.create_identity_provider_mapper(mapper, alias, realm) kc.create_identity_provider_mapper(mapper, alias, realm)
for mapper in [x for x in before_idp['mappers'] for mapper in [x for x in before_idp['mappers']
if [y for y in updated_mappers if y["name"] == x['name']] == []]: if [y for y in updated_mappers if y["name"] == x['name']] == []]:

View file

@ -224,7 +224,7 @@ class TestKeycloakIdentityProvider(ModuleTestCase):
# Verify that the module's changed status matches what is expected # Verify that the module's changed status matches what is expected
self.assertIs(exec_info.exception.args[0]['changed'], changed) self.assertIs(exec_info.exception.args[0]['changed'], changed)
def test_create_when_present(self): def test_update_when_present(self):
"""Update existing identity provider""" """Update existing identity provider"""
module_args = { module_args = {
@ -250,6 +250,15 @@ class TestKeycloakIdentityProvider(ModuleTestCase):
'syncMode': "FORCE" 'syncMode': "FORCE"
}, },
'mappers': [{ 'mappers': [{
'name': "username",
'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper",
'config': {
'claim': "username",
'user.attribute': "username",
'syncMode': "INHERIT",
}
}, {
'name': "first_name", 'name': "first_name",
'identityProviderAlias': "oidc-idp", 'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper", 'identityProviderMapper': "oidc-user-attribute-idp-mapper",
@ -319,10 +328,20 @@ class TestKeycloakIdentityProvider(ModuleTestCase):
] ]
return_value_mappers_get = [ return_value_mappers_get = [
[{ [{
'config': {
'claim': "username",
'syncMode': "INHERIT",
'user.attribute': "username"
},
"id": "616f11ba-b9ae-42ae-bd1b-bc618741c10b",
'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper",
'name': "username"
}, {
"config": { "config": {
"claim": "first_name_changeme", "claim": "first_name_changeme",
"syncMode": "INHERIT", "syncMode": "INHERIT",
"user.attribute": "first_name_changeme" "user.attribute": "first_name"
}, },
"id": "5fde49bb-93bd-4f5d-97d6-c5d0c1d07aef", "id": "5fde49bb-93bd-4f5d-97d6-c5d0c1d07aef",
"identityProviderAlias": "oidc-idp", "identityProviderAlias": "oidc-idp",
@ -330,6 +349,79 @@ class TestKeycloakIdentityProvider(ModuleTestCase):
"name": "first_name" "name": "first_name"
}], }],
[{ [{
'config': {
'claim': "username",
'syncMode': "INHERIT",
'user.attribute': "username"
},
"id": "616f11ba-b9ae-42ae-bd1b-bc618741c10b",
'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper",
'name': "username"
}, {
"config": {
"claim": "first_name_changeme",
"syncMode": "INHERIT",
"user.attribute": "first_name"
},
"id": "5fde49bb-93bd-4f5d-97d6-c5d0c1d07aef",
"identityProviderAlias": "oidc-idp",
"identityProviderMapper": "oidc-user-attribute-idp-mapper",
"name": "first_name"
}],
[{
'config': {
'claim': "username",
'syncMode': "INHERIT",
'user.attribute': "username"
},
"id": "616f11ba-b9ae-42ae-bd1b-bc618741c10b",
'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper",
'name': "username"
}, {
"config": {
"claim": "first_name_changeme",
"syncMode": "INHERIT",
"user.attribute": "first_name"
},
"id": "5fde49bb-93bd-4f5d-97d6-c5d0c1d07aef",
"identityProviderAlias": "oidc-idp",
"identityProviderMapper": "oidc-user-attribute-idp-mapper",
"name": "first_name"
}],
[{
'config': {
'claim': "username",
'syncMode': "INHERIT",
'user.attribute': "username"
},
"id": "616f11ba-b9ae-42ae-bd1b-bc618741c10b",
'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper",
'name': "username"
}, {
"config": {
"claim": "first_name_changeme",
"syncMode": "INHERIT",
"user.attribute": "first_name"
},
"id": "5fde49bb-93bd-4f5d-97d6-c5d0c1d07aef",
"identityProviderAlias": "oidc-idp",
"identityProviderMapper": "oidc-user-attribute-idp-mapper",
"name": "first_name"
}],
[{
'config': {
'claim': "username",
'syncMode': "INHERIT",
'user.attribute': "username"
},
"id": "616f11ba-b9ae-42ae-bd1b-bc618741c10b",
'identityProviderAlias': "oidc-idp",
'identityProviderMapper': "oidc-user-attribute-idp-mapper",
'name': "username"
}, {
"config": { "config": {
"claim": "first_name", "claim": "first_name",
"syncMode": "INHERIT", "syncMode": "INHERIT",
@ -371,7 +463,7 @@ class TestKeycloakIdentityProvider(ModuleTestCase):
self.module.main() self.module.main()
self.assertEqual(len(mock_get_identity_provider.mock_calls), 2) self.assertEqual(len(mock_get_identity_provider.mock_calls), 2)
self.assertEqual(len(mock_get_identity_provider_mappers.mock_calls), 2) self.assertEqual(len(mock_get_identity_provider_mappers.mock_calls), 5)
self.assertEqual(len(mock_update_identity_provider.mock_calls), 1) self.assertEqual(len(mock_update_identity_provider.mock_calls), 1)
self.assertEqual(len(mock_update_identity_provider_mapper.mock_calls), 1) self.assertEqual(len(mock_update_identity_provider_mapper.mock_calls), 1)
self.assertEqual(len(mock_create_identity_provider_mapper.mock_calls), 1) self.assertEqual(len(mock_create_identity_provider_mapper.mock_calls), 1)