keycloak / keycloak

Open Source Identity and Access Management For Modern Applications and Services
https://www.keycloak.org
Apache License 2.0
22.45k stars 6.63k forks source link

Cannot force SAML NameID format to persistent when ldap is in read-only mode #11838

Closed sventorben closed 3 months ago

sventorben commented 2 years ago

Describe the bug

We have a realm configured with a LDAP user federation. Edit mode is configured as read-only and user import is enabled. Additionally, we have a SAML client where we need to force the NameID format to persistent.

When we try to login via that client, the login fails an Keycloak logs a warning (see below).

Version

16

Expected behavior

If user does not have a persistent id for that client, Keycloak should generate an id and store it as a user attribute saml.persistent.name.id.for.<clientId>. If that attribute exists, Keycloak should use it as the persistent id for that client and user.

Login should succeed.

Actual behavior

Login fails and Keycloak logs warning LOGIN_ERROR with invalid_user_credentials like this:

{"timestamp":"2022-04-13T08:14:19.448Z","sequence":3870,"loggerClassName":"org.jboss.logging.Logger","loggerName":"org.keycloak.events","level":"WARN","message":"type=LOGIN_ERROR, realmId=******, clientId=https://******/auth/saml, userId=******, ipAddress=******, error=invalid_user_credentials, x509_cert_subject_distinguished_name=******, auth_method=saml, x509_cert_issuer_distinguished_name=******, redirect_uri=https://******/auth/saml, consent=no_consent_required, code_id=******, x509_cert_serial_number=******, username=******...

How to Reproduce?

Anything else?

Everything works as expected when we either switch edit mode to write/unsynced or use a name id mapper. It looks like Keycloak cannot store the persistent name id attribute to the users, because it is read-only.

rmartinc commented 1 year ago

Hi @sventorben!

Yes, this is a known issue when ldap is configured as READ_ONLY and very similar to other issues like KEYCLOAK-3365. But, in this case, the solution is more complicated because the persistent attribute is just a user attribute. But there are two workarounds I can think of:

Name: saml.persistent.name.id.for Mapper type: user-attribute-ldap-mapper User Model Attribute: saml.persistent.name.id.for.* (the generic name, works for all SAML clients in the realm) LDAP Attribute: entryUUID Read Only: true Always Read Value From LDAP: true

This way when the SAML tries to find the persistent attribute it gets this attribute filled with the entryUUID (you can use any other LDAP attribute like uid or email if it's valid for you). And that attribute will be used as the persistent value used by the SAML.

I'm setting this in back-log for the moment but maybe we can just close it as i think it is working as expected.

sventorben commented 1 year ago

Hello @rmartinc,

thanks for the reply.

I am afraid that I need to disagree here. From my point of view, it does not work as expected:

Persistent name identifiers generated by identity providers MUST be constructed using pseudo-random values that have no discernible correspondence with the subject's actual identifier (for example, username). The intent is to create a non-public, pair-wise pseudonym to prevent the discovery of the subject's identity or activities

and further

Persistent identifiers are intended as a privacy protection mechanism;

I consider the entryUUID to be a subject's actual identifier. With this in mind, I am very reluctant to share the entryUUID with other clients.

rmartinc commented 1 year ago

@sventorben I didn't say this is not an issue, I indeed think that it is a bug. But it was moved to backlog which means that we probably don't have the time to fix it in the next releases. As I commented both proposals are just workarounds. Probably one easy solution is just extending the hardcoded-attribute-mapper to also understand ${RANDOM} as the hardcoded LDAP mapper (it would create an attribute with a random value at import time). But I have not checked this in detail. As always, contributions are welcomed!

I didn't know the real intention of the persistent type. The previous times I faced this it was just the SAML client only accepted that type. But this doesn't change the second workaround, you can select any attribute in the ldap entry that works for you as a persistent identifier (entryUUID is just a internal generated id that some ldap servers use to identify the entry). In the end the persistent ID should be stored somewhere.

sventorben commented 1 year ago

If I would choose an attribute from ldap, the value used for persitent name id would be the same for all clients. But I would need a pair-wise pseudonym so that it is different for each client/IdP pair.

Using a hardcoded-attribute-mapper could work, but this would assume that import is enabled, right?

keycloak-github-bot[bot] commented 6 months ago

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a :thumbsup: to the description. We would also welcome a contribution to fix the issue.

keycloak-github-bot[bot] commented 3 months ago

Due to lack of updates in the last 180 days this issue will be automatically closed.