Skip to content
This repository has been archived by the owner on Nov 16, 2022. It is now read-only.

KEYCLOAK-13582 user federation provider and mapper #240

Conversation

chlunde
Copy link
Contributor

@chlunde chlunde commented Aug 5, 2020

JIRA ID

https://issues.redhat.com/browse/KEYCLOAK-13582

Additional Information

Support for LDAP and customizing it using mappers.

This PR builds on other PRs, only the latest commit is relevant. I will rebase when the other commits are merged.

Verification Steps

The configuration has been e2e tested using an export (this ensures all field names are correct).

For a manual test:

  1. Create a realm CRD point to an existing LDAP
apiVersion: keycloak.org/v1alpha1
kind: KeycloakRealm
metadata:
  labels:
    app: sso
  name: ldap-realm
spec:
  instanceSelector:
    matchLabels:
      app: sso
  realm:
    displayName: LDAPRealm
    enabled: true
    id: ldap-realm
    realm: ldap-realm
    userFederationProviders:
      - displayName: "ldap"
        providerName: "ldap"
        config:
          vendor: "ad"
          connectionUrl: "ldap://localhost"
          bindDn: "USERNAME"
          bindCredential: "PASSWORD"
          usersDn: DC=example,DC=com"
          usernameLDAPAttribute: "mail"
          uuidLDAPAttribute: "objectGUID"
          searchScope: "2" # sub
          useTruststoreSpi: "ldapsOnly"
          trustEmail: "true"
          userObjectClasses: "person, organizationalPerson, user"
          rdnLDAPAttribute: "cn"
          editMode: "READ_ONLY"
          # debug: "false"
    userFederationMappers:
      - name: username
        federationProviderDisplayName: ldap
        federationMapperType: user-attribute-ldap-mapper
        config:
          always.read.value.from.ldap: 'true'
          is.binary.attribute: 'false'
          is.mandatory.in.ldap: 'true'
          ldap.attribute: mail
          read.only: 'true'
          user.model.attribute: username
      - name: MSAD account controls
        federationProviderDisplayName: ldap
        federationMapperType:  msad-user-account-control-mapper
        config:
          ldap.password.policy.hints.enabled: 'false'
      - name: last name
        federationProviderDisplayName: ldap
        federationMapperType: user-attribute-ldap-mapper
        config:
          always.read.value.from.ldap: 'true'
          is.binary.attribute: 'false'
          is.mandatory.in.ldap: 'true'
          ldap.attribute: sn
          read.only: 'true'
          user.model.attribute: lastName
      - name: email
        federationProviderDisplayName: ldap
        federationMapperType: user-attribute-ldap-mapper
        config:
          always.read.value.from.ldap: 'true'
          is.binary.attribute: 'false'
          is.mandatory.in.ldap: 'true'
          ldap.attribute: mail
          read.only: 'true'
          user.model.attribute: email
      - name: full name
        federationProviderDisplayName: ldap
        federationMapperType: full-name-ldap-mapper
        config:
          ldap.full.name.attribute: cn
          read.only: 'true'
          write.only: 'false'
  1. Log in using usernamer and password

Checklist:

@kfox1111
Copy link

kfox1111 commented Aug 7, 2020

This looks really good. But bind credential should probably be a secret ref.

@chlunde
Copy link
Contributor Author

chlunde commented Aug 10, 2020

But bind credential should probably be a secret ref.

@kfox1111 Yes, I think so too, but there should be a unified way to do it. The samme problem already exists for example in clientSecret for an IdP. I briefly asked about it here:

But I think it should be done in another PR. I never got a response in KEYCLOAK-12677 - do you think we should create a new Jira issue for this (clientSecret, bindCredential etc) and discuss possible designs there?

@abstractj abstractj self-assigned this Aug 24, 2020
@abstractj abstractj assigned slaskawi and unassigned abstractj Aug 24, 2020
@slaskawi
Copy link
Contributor

But I think it should be done in another PR. I never got a response in KEYCLOAK-12677 - do you think we should create a new Jira issue for this (clientSecret, bindCredential etc) and discuss possible designs there?

@chlunde Yes, that's a good point. All Secret refs should be somehow consistent across all our CRs. Could you please spin up a discussion on Keycloak DEV: https://groups.google.com/g/keycloak-dev ?

Copy link
Contributor

@slaskawi slaskawi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chlunde I like the proposed change but the situation is exactly the same as in #230 (review)

@slaskawi
Copy link
Contributor

slaskawi commented Sep 9, 2020

@chlunde Since your previous PR has been merged, maybe we can get this one rolling?

@chlunde chlunde force-pushed the KEYCLOAK-13582-user-federation-provider-and-mapper branch from 9a4d1db to 232d1ca Compare September 9, 2020 18:57
@chlunde chlunde force-pushed the KEYCLOAK-13582-user-federation-provider-and-mapper branch from 232d1ca to 0aed1e3 Compare September 9, 2020 19:17
@chlunde
Copy link
Contributor Author

chlunde commented Sep 9, 2020

@slaskawi done

@slaskawi slaskawi merged commit 3730784 into keycloak:master Sep 22, 2020
@slaskawi
Copy link
Contributor

Integrated, thanks @chlunde !

@Chive
Copy link

Chive commented Sep 29, 2020

@slaskawi / @chlunde sorry to hijack this issue, I was unsure on where to best post this.
Is there any plan on implementing the mappers (identityProviderMappers) for the already existing identity providers (added in #49)?

@slaskawi
Copy link
Contributor

@Chive We have a ticket for this - https://issues.redhat.com/browse/KEYCLOAK-13098. I think it's best to subscribe to it.

Perhaps you'd be interested in contributing this feature?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants