netzbegruenung / keycloak-mfa-plugins

Keycloak plugins for MFA (enforce MFA, SMS authentication step, native app integration)
Apache License 2.0
61 stars 15 forks source link

feature/phone number format e164 #94

Closed dan-netizen closed 1 month ago

dan-netizen commented 1 month ago

Add the option to format the phone number to E164 standard using the google libphonenumber library, which should be packaged as a dependency in the final jar for the authenticator to work. Add the option to add the phone number to the 'mobile_number' user attribute on successful phone number validation action. Based on authenticator configuration, if the phone does not pass the required formatting checks, an error is shown and the user is asked to re-enter a valid phone number.

dan-netizen commented 1 month ago

I could not find a way around making a jar with dependencies, which increases its size quite a bit. Is there a way to not include this functionality (and not include the dependency in the jar) if one chooses to? Some users might not want this added functionality if it comes with such a fat jar. Maybe in the build process? Keycloak throws an exception if you try to use the jar without the library included, so it would have to also remove the code altogether. Just something I was wandering about.

svenseeberg commented 1 month ago

Thanks for the PR :tada:

I could not find a way around making a jar with dependencies, which increases its size quite a bit.

The biggest issue here is that io.quarkus would be included, right? We only need this for development. libphonenumber in contrast should probably be included in the jar. If I understand https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#dependency-scope correctly, choosing the correct scope should mitigate the problem.

alexanderhofstaetter commented 1 month ago

This PR trys to implement https://github.com/netzbegruenung/keycloak-mfa-plugins/issues/84 and https://github.com/netzbegruenung/keycloak-mfa-plugins/issues/85

dan-netizen commented 1 month ago

I think I applied most of the suggestions, and replaced maven-assembly-plugin with maven-shade-plugin, that seemed to work better and the jar is now ~397KB down from ~13MB+. Also added some other misc fixes and improvements.

alexanderhofstaetter commented 1 month ago

There seems to be some problems regarding encoding on your side:

image

alexanderhofstaetter commented 1 month ago

@melegiul can you provide a new release with all the changes?

melegiul commented 1 month ago

Yes, sure. I will create a new release not later than tomorrow. I just want to add another feature, but unrelated to SMS.

melegiul commented 1 month ago

And there it is :smiley: https://github.com/netzbegruenung/keycloak-mfa-plugins/releases/tag/v25.0.2