community.general icon indicating copy to clipboard operation
community.general copied to clipboard

community.general.keycloak_user_federation is not idempotent

Open ykuksenko opened this issue 2 years ago • 4 comments

Summary

When running the 'community.general.keycloak_user_federation' module it keeps adding more entries of ldap user federations every time it is run instead of recognizing it is already there.

At the same time if a mapper is defined and happens to be the same as one that already exists it just gets added as a second mapper instead of updating the one that is auto created.

If mappers are defined when creating a user federation it would be good to delete the mappers that are auto created as there is no good way to clean those up using this module.

Issue Type

Bug Report

Component Name

community.general.keycloak_user_federation

Ansible Version

ansible [core 2.12.5]
  config file = /home/user/repo/ansible/control_example-com/ansible.cfg
  configured module search path = ['/home/user/repo/ansible/control_example-org/plugins/modules']
  ansible python module location = /usr/lib/python3.10/site-packages/ansible
  ansible collection location = /home/user/repo/ansible/control_example-com/plugins/collections
  executable location = /usr/bin/ansible
  python version = 3.10.4 (main, Mar 23 2022, 23:05:40) [GCC 11.2.0]
  jinja version = 3.0.3
  libyaml = True

Community.general Version

# /usr/lib/python3.10/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 4.8.0  

Configuration

ANSIBLE_NOCOWS(/home/repo/ansible/control_example-com/ansible.cfg) = True
CACHE_PLUGIN(/home/repo/ansible/control_example-com/ansible.cfg) = jsonfile
CACHE_PLUGIN_CONNECTION(/home/repo/ansible/control_example-com/ansible.cfg) = ~/.ansible/dev_cache
CACHE_PLUGIN_TIMEOUT(/home/repo/ansible/control_example-com/ansible.cfg) = 6000
COLLECTIONS_PATHS(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/plugins/collections']
DEFAULT_CALLBACK_PLUGIN_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/user/.local/lib/python3.7/site-packages/ara/plugins/callback']
DEFAULT_FILTER_PLUGIN_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/plugins/filter']
DEFAULT_GATHERING(/home/repo/ansible/control_example-com/ansible.cfg) = smart
DEFAULT_HOST_LIST(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/inventories/development']
DEFAULT_LOOKUP_PLUGIN_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/plugins/lookup']
DEFAULT_MANAGED_STR(/home/repo/ansible/control_example-com/ansible.cfg) = File is managed by Ansible. Do not edit.
DEFAULT_MODULE_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/plugins/modules']
DEFAULT_PRIVATE_KEY_FILE(/home/repo/ansible/control_example-com/ansible.cfg) = /home/repo/ansible/control_example-com/.private/key.rsa
DEFAULT_PRIVATE_ROLE_VARS(/home/repo/ansible/control_example-com/ansible.cfg) = True
DEFAULT_REMOTE_USER(/home/repo/ansible/control_example-com/ansible.cfg) = ansibler
DEFAULT_ROLES_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/.containers', '/home/repo/ansible/control_example-com/.containers_old', '/home/repo/ansible/control_example-com/roles', '/home/repo/ansible/control_example-com/.roles']
DEFAULT_STDOUT_CALLBACK(/home/repo/ansible/control_example-com/ansible.cfg) = yaml
DEFAULT_UNDEFINED_VAR_BEHAVIOR(/home/repo/ansible/control_example-com/ansible.cfg) = True
DEFAULT_VARS_PLUGIN_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = ['/home/repo/ansible/control_example-com/plugins/vars']
DIFF_ALWAYS(/home/repo/ansible/control_example-com/ansible.cfg) = True
DIFF_CONTEXT(/home/repo/ansible/control_example-com/ansible.cfg) = 5
DISPLAY_ARGS_TO_STDOUT(/home/repo/ansible/control_example-com/ansible.cfg) = True
DISPLAY_SKIPPED_HOSTS(/home/repo/ansible/control_example-com/ansible.cfg) = True
HOST_KEY_CHECKING(/home/repo/ansible/control_example-com/ansible.cfg) = False
INVENTORY_IGNORE_PATTERNS(/home/repo/ansible/control_example-com/ansible.cfg) = ['ca', 'configs', 'files']
RETRY_FILES_SAVE_PATH(/home/repo/ansible/control_example-com/ansible.cfg) = /home/user/.ansible/.ansible-retry

the home paths are inconsistent due to redactions...

OS / Environment

Archlinux

Steps to Reproduce

  - name: setup ldap link to freeipa
    community.general.keycloak_user_federation:
      auth_keycloak_url: https://keycloak.example.com/auth
      auth_realm: master
      auth_username: "user"
      auth_password: "Password@3"
      realm: "example.com"
      name: freeipa3
      state: present
      provider_id: ldap
      provider_type: org.keycloak.storage.UserStorageProvider
      config:
        priority: 0
        enabled: true
        cachePolicy: NO_CACHE
        batchSizeForSync: 1000
        editMode: WRITABLE #READ_ONLY
        importEnabled: true
        syncRegistrations: false
        vendor: other
        usernameLDAPAttribute: uid
        rdnLDAPAttribute: uid
        uuidLDAPAttribute: ipaUniqueID
        userObjectClasses: inetOrgPerson, organizationalPerson
        connectionUrl: ldaps://freeipa-01.ipa.example.com:636
        usersDn: cn=users,cn=accounts,dc=ipa,dc=example,dc=org
        authType: simple
        bindDn: "cn=Directory Manager"
        bindCredential: "blahblahpassword"
        searchScope: 2
        validatePasswordPolicy: true
        trustEmail: false
        useTruststoreSpi: always
        connectionPooling: true
        pagination: true
        allowKerberosAuthentication: false
        debug: false
        useKerberosForPasswordAuthentication: false
      mappers:
        - name: "full name"
          providerId: "full-name-ldap-mapper"
          providerType: "org.keycloak.storage.ldap.mappers.LDAPStorageMapper"
          config:
            ldap.full.name.attribute: cn
            read.only: true
            write.only: false
        - name: first name
          providerId: "user-attribute-ldap-mapper"
          providerType: "org.keycloak.storage.ldap.mappers.LDAPStorageMapper"
          config:
            always.read.value.from.ldap: true
            is.mandatory.in.ldap: true
            ldap.attribute: cn
            read.only: false
            user.model.attribute: givenName

Run this twice then check the keycloak ui and you will find two freeipa3 entries and ansible will say changed both times You will also find that each entry contains two first name mappers when the first one should have been overwritten. Ideally you would only find the full name and first name mappers but instead you will find multiple additional mappers for both freeipa3 user federation entries.

Expected Results

One freeipa3 user federation entry should exist, the default first name mapper would be updated and mappers not listed in the above config would not be present.

Actual Results

--- before
+++ after
@@ -0,0 +1,51 @@
+config:
+  allowKerberosAuthentication: 'false'
+  authType: simple
+  batchSizeForSync: '1000'
+  bindCredential: '**********'
+  bindDn: cn=Directory Manager
+  cachePolicy: NO_CACHE
+  changedSyncPeriod: '-1'
+  connectionPooling: 'true'
+  connectionUrl: ldaps://freeipa-01.ipa.example.come:636
+  debug: 'false'
+  editMode: WRITABLE
+  enabled: 'true'
+  fullSyncPeriod: '-1'
+  importEnabled: 'true'
+  pagination: 'true'
+  priority: '0'
+  rdnLDAPAttribute: uid
+  searchScope: '2'
+  startTls: 'false'
+  syncRegistrations: 'false'
+  trustEmail: 'false'
+  useKerberosForPasswordAuthentication: 'false'
+  usePasswordModifyExtendedOp: 'false'
+  useTruststoreSpi: always
+  userObjectClasses: inetOrgPerson, organizationalPerson
+  usernameLDAPAttribute: uid
+  usersDn: cn=users,cn=accounts,dc=ipa,dc=example,dc=com
+  uuidLDAPAttribute: ipaUniqueID
+  validatePasswordPolicy: 'true'
+  vendor: other
+mappers:
+- config:
+    ldap.full.name.attribute: cn
+    read.only: 'true'
+    write.only: 'false'
+  name: full name
+  providerId: full-name-ldap-mapper
+  providerType: org.keycloak.storage.ldap.mappers.LDAPStorageMapper
+- config:
+    always.read.value.from.ldap: 'true'
+    is.mandatory.in.ldap: 'true'
+    ldap.attribute: cn
+    read.only: 'false'
+    user.model.attribute: givenName
+  name: first name
+  providerId: user-attribute-ldap-mapper
+  providerType: org.keycloak.storage.ldap.mappers.LDAPStorageMapper
+name: freeipa3
+providerId: ldap
+providerType: org.keycloak.storage.UserStorageProvider

every time the module is run.

Code of Conduct

  • [X] I agree to follow the Ansible Code of Conduct

ykuksenko avatar May 12 '22 04:05 ykuksenko

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot avatar May 12 '22 04:05 ansibullbot

cc @eikef @laurpaum @ndclt click here for bot help

ansibullbot avatar May 12 '22 04:05 ansibullbot

I just came across this problem as well. If it helps: the issue can be avoided if you provide the id parameter. The documentation for id states that if left empty, the user federation will be searched by its name - but apparently this doesn't work.

zitnik avatar Aug 09 '22 13:08 zitnik

Facing the same problem here. If you run the default example on the documentation it will fail on the second run complaining about a duplicated mapper that it was created previously instead of modifying the default "full name" mapper:

- name: Create LDAP user federation
  community.general.keycloak_user_federation:
    auth_keycloak_url: https://keycloak.example.com/auth
    auth_realm: master
    auth_username: admin
    auth_password: password
    realm: my-realm
    name: my-ldap
    state: present
    provider_id: ldap
    provider_type: org.keycloak.storage.UserStorageProvider
    config:
      priority: 0
      enabled: true
      cachePolicy: DEFAULT
      batchSizeForSync: 1000
      editMode: READ_ONLY
      importEnabled: true
      syncRegistrations: false
      vendor: other
      usernameLDAPAttribute: uid
      rdnLDAPAttribute: uid
      uuidLDAPAttribute: entryUUID
      userObjectClasses: inetOrgPerson, organizationalPerson
      connectionUrl: ldaps://ldap.example.com:636
      usersDn: ou=Users,dc=example,dc=com
      authType: simple
      bindDn: cn=directory reader
      bindCredential: password
      searchScope: 1
      validatePasswordPolicy: false
      trustEmail: false
      useTruststoreSpi: ldapsOnly
      connectionPooling: true
      pagination: true
      allowKerberosAuthentication: false
      debug: false
      useKerberosForPasswordAuthentication: false
    mappers:
      - name: "full name"
        providerId: "full-name-ldap-mapper"
        providerType: "org.keycloak.storage.ldap.mappers.LDAPStorageMapper"
        config:
          ldap.full.name.attribute: cn
          read.only: true
          write.only: false

jsalatiel avatar Aug 30 '22 21:08 jsalatiel

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot avatar Nov 10 '22 22:11 ansibullbot

I stumbled about this problem today and found the cause, or at least a cause. This is the request the module assembles under the hood to probe for existing user federations:

https://<server-url>/admin/realms/<realm-name>/components?type=org.keycloak.storage.UserStorageProvider&parent=<realm-name>&name=<federation-name>

This calls always returns an empty list and will also do so when trying this manually by using curl or similar. The problem is the second query parameter: parent=<realm-name>. Not that it is not supported or so but what the rest api documentation of keycloak fails to make clear is that it expects the parent-id here, not the name. So this works: parent=<parent-id>.

But quite frankly in my humble opinion the even better solution would be to completly remove the query param which also works fine. The parent is already part of the url path so repeating it as query param seems superfluous. But maybe api versions play a role here? Documentation for this module references keycloak api version 8.0 which is quite old while I personally work with keycloak 20.0.2. Maybe this worked differently for older keycloak versions?

However I will try to put a PR together which should fix this at least for recent versions of keycloak.

Note: If someone wants to test if my proposed fix helps them one can test it by temporarly replacing community.general with the following snippet inside the collections requirements file:

 - name: https://github.com/sma-de/community.general.git
   type: git
   version: bugfix/keycloak-userfed-idempotency

morco avatar Dec 22 '22 20:12 morco

Okay, I played around a bit more and it seems the situation is unfortunately a bit more complicated than anticipated above. It seems that the critical query parameter parent=<realm-name> seems to work sometimes as expected and sometimes not depending on what the parent is. To say it more precise the one "special" realm I can confirm so far that it does not work because of this query parameter is master, while other's seem fine for my probing.

This would make this technically an upstream issue for keycloak rest api but I would still argue to remove the seemingly unnecessary dependency on this here in ansible to make it more robust against such kind of errors.

morco avatar Dec 23 '22 13:12 morco