netzbegruenung / keycloak-mfa-plugins

Keycloak Authentication Provider implementation to get a 2nd-factor authentication with a OTP/code/token send via SMS
Apache License 2.0
42 stars 9 forks source link

SMS: Sync phone Number with user attribute #84

Open alexanderhofstaetter opened 4 days ago

alexanderhofstaetter commented 4 days ago

We need to store the phone number as a custom attribute on the user object to be able to provide it as a token claim.

Is there any chance to also store the number as "mobile_number" user attribute every time the number gets changed?

Maybe someone can give a hint on where to add this and I can try doing a pull request.

svenseeberg commented 2 days ago

I think this should be possible. There is an architectural caveat: we chose to implement the phone number as a credential and not a user attribute. If it was an attribute, a simple LDAP attribute mapper would suffice.

To implement this, we probably need a new LDAP mapper type, similar to user-attribute-ldap-mapper. A good start would be to understand how that works: https://github.com/search?q=repo%3Akeycloak%2Fkeycloak%20user-attribute-ldap-mapper&type=code

In the end the main difference would be to update a credential type (mobile_number) instead of a user attribute.

alexanderhofstaetter commented 1 day ago

Also this should be implemented together with https://github.com/netzbegruenung/keycloak-mfa-plugins/issues/85 then we would have a normalized phone number attribute.

so the chain should be:

with this behaviour we have the confidence, that the number in the attribute is always the same and verified. so we can use the number as trusted claim in our target application.

Warning: logic should be added when the number gets changed in the backend or when the number changes.

maybe also add a flag for number_verified -> like we already have in email_verified

svenseeberg commented 1 day ago

I also found https://lists.jboss.org/pipermail/keycloak-user/2017-January/009234.html, which is a quite similar problem.

svenseeberg commented 1 day ago

Which storage would be the source of truth? LDAP attribute or the credential? What happens if the value in the LDAP changes?

IMHO the LDAP directory should be the primary storage back end, when enabled. That means we store the phone number there, when the confirmation code has been entered & validated. If the phone number in the LDAP directory changes, so would the phone number for the authentication step in Keycloak.

We can then implement it this way:

alexanderhofstaetter commented 1 day ago

I like the approach that the user attribute is the source of truth. We do not have an LDAP backend.

When the user attribute gets changed from the backend (e.g. manual admin input or sync override) the phone number is stored, but its maybe not confirmed.

What about also adding a boolean flag like we have for email (phone_verified) ?

svenseeberg commented 1 day ago

We do not have an LDAP backend.

Ah, quite relevant. I missed this part. Sorry, I was thinking too much in our setups.

melegiul commented 18 hours ago

Just one thought I want to add about this topic: Keycloak aims to provide the ability to order the 2FA Methods by their credentials. If we go on moving the SMS Credentials to a user attribute we kind of locking us out from using such improvements. https://github.com/keycloak/keycloak/issues/12102 https://github.com/keycloak/keycloak/issues/14340

alexanderhofstaetter commented 11 hours ago

I think it should have both -> the real auth/2FA auth part should be handled by the Credential and additionally it should be stored as user attribute for extra use (e.g. claim).

The attribute should already get normalized (E164) by phonenumber lib before it gets stored as a credential.