search

netzbegruenung / keycloak-mfa-plugins

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

SMS-Auth: APi Gateway Returns Error - User does not know #89

Open alexanderhofstaetter opened 4 months ago

alexanderhofstaetter commented 4 months ago

When the API Gateway returns an error code it outputs the log that there was an error, however, the user still gets the SMS code input gets shown?

Just as a note, dont know how to handle this currently

svenseeberg commented 4 months ago

Currently the user does not receive an error notification, no. We have not tackled this as we get the return codes asynchronously and therefore do not know if the SMS was sent when the user reloads the page. However, we have not yet received many complaints. This obviously depends on the stability of the SMS API.

melegiul commented 4 months ago

Actually we send the request synchronously, blocking for the response if necessary. https://github.com/netzbegruenung/keycloak-mfa-plugins/blob/209a40e806feb5d8d81b58873af89403006ae7b2/sms-authenticator/src/main/java/netzbegruenung/keycloak/authenticator/gateway/ApiSmsService.java#L91 https://docs.oracle.com/en%2Fjava%2Fjavase%2F11%2Fdocs%2Fapi%2F%2F/java.net.http/java/net/http/HttpClient.html#send(java.net.http.HttpRequest,java.net.http.HttpResponse.BodyHandler)

We should indeed improve here the UX by for example showing a error message to the user. Also we could add a retry button, which either sends the SMS Code again to the same number or let the user enter the phone number again to correct possible typos. This retry button could be present independent of the SMS Sent Status, because we had some cases where a custom provider reported success while actually no SMS was sent.

svenseeberg commented 4 months ago

Agreed, the HTTP response code should be parsed somehow. In our case a 200 OK will not guarantee that the SMS has actually been sent though. There is a second asynchronous step in our SMS gateway implementation. This might not be the case for other use cases.

After we implemented this feature, we can even change the behavior of our SMS gateway to make it wait until the SMS has actually been sent.