Afford a twilio SMS Authenticator for keycloak

[bickelj]

This project creates a lib-all.jar that can be placed in keycloak's directory named /providers. When present, the jar affords an SMS authenticator. The Authenticator implementation is based on dasniko's at https://github.com/dasniko/keycloak-2fa-sms-authenticator and also requires his keycloak-requiredaction jar to be in /providers. After the jars are in place, keycloak needs to be configured to use them. See https://www.n-k.de/2020/12/keycloak-2fa-sms-authentication.html and https://github.com/PhilanthropyDataCommons/auth/issues/2#issuecomment-1440797906 for more details.

The twilio settings are loaded via environment variables:

• TWILIO_PHONE_NUMBER: the "from" phone number set up in twilio.
• TWILIO_ACCOUNT_SID: the SID or username for twilio API access.
• TWILIO_AUTH_TOKEN: the token or secret for twilio API access.

[jasonaowen]

In https://github.com/PhilanthropyDataCommons/auth/pull/1#issuecomment-1444373868, @bickelj wrote:

Looking back at my objections to this API key extension approach, namely the "internal SPI" and "extending keycloak" objections in PhilanthropyDataCommons/service#96 (comment) the same "internal SPI" warning occurs with PR #3 and it too is an extension to keycloak.

I think a critical difference is that this is extending Keycloak in a way that is not visible to applications that authenticate with Keycloak! This work is more in line with the operational task of "how do we make Keycloak work in our organization with our security requirements", and not part of the development task "how do we make an app that works with Keycloak".

tl;dr: this seems like good and worthwhile work! I have no concerns with this approach.

[bickelj]

I plan to squash the commits once the changes are up to snuff. Does that sound reasonable?

[jasonaowen]

I plan to squash the commits once the changes are up to snuff. Does that sound reasonable?

Of course! I thought that might be the case, but explicit is better than implicit. :)

[bickelj]

I am still torn on all the squashing but that is a discussion for another day. Squashed and it looks like it's working. Merging.

[bickelj]

@jasonaowen and @kfogel I have a question regarding process. Above I squashed, force pushed, and then presumed to merge before re-review. This was following a pattern established in the service project, but would it be better to have a reviewer actually click merge? Or to at least confirm the exact squashed commit has been re-reviewed prior to merge?

[jasonaowen]

@bickelj and I talked about this a bit offline, but for the record: the way I like to work - and I believe the way @slifty likes to work - is for the author to merge PRs after getting an approving review. I like the way this spreads responsibility and autonomy, and it also gives reviewers the opportunity to leave "yes, and..." reviews with optional suggestions - as I did above - and leave it up to the author to take those suggestions or not as they see fit.

One side note is that we should clean up branches after they're merged; there's a setting to do so automatically for this that I'm following up about with the folks who can make that change. For now, I'll delete this one by hand.