sventorben / keycloak-home-idp-discovery

Keycloak: Home IdP Discovery - discover home identity provider or realm by email domain
MIT License
263 stars 48 forks source link

[Feature] Error Message for unvalidated Email Addresses #131

Closed pawl1234 closed 1 year ago

pawl1234 commented 1 year ago

Is there an existing feature request for this?

Is your feature related to a problem? Please describe.

I had the issue that my authentication flow worked in the first run, but I was not able to login after logout. I received the console error. I guess I spent too much time for debugging as I'm new to keycloak, but the reason for this issue was that the email address was not validated.

keycloak    | 2023-01-10 14:53:08,218 WARN  [de.sventorben.keycloak.authentication.hidpd.HomeIdpDiscoverer] (executor-thread-217) Could not extract domain from email address user1@idpa.com
keycloak    | 2023-01-10 14:53:08,218 WARN  [org.keycloak.services] (executor-thread-217) KC-SERVICES0013: Failed authentication: org.keycloak.authentication.AuthenticationFlowException
keycloak    |   at org.keycloak.authentication.AuthenticationProcessor.authenticationAction(AuthenticationProcessor.java:983)
keycloak    |   at org.keycloak.services.resources.LoginActionsService.processFlow(LoginActionsService.java:311)
keycloak    |   at org.keycloak.services.resources.LoginActionsService.processAuthentication(LoginActionsService.java:282)
keycloak    |   at org.keycloak.services.resources.LoginActionsService.authenticate(LoginActionsService.java:274)
keycloak    |   at org.keycloak.services.resources.LoginActionsService.authenticateForm(LoginActionsService.java:339)
keycloak    |   at jdk.internal.reflect.GeneratedMethodAccessor599.invoke(Unknown Source)
keycloak    |   at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
keycloak    |   at java.base/java.lang.reflect.Method.invoke(Method.java:566)
keycloak    |   at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:170)
keycloak    |   at org.jboss.resteasy.core.MethodInjectorImpl.invoke(MethodInjectorImpl.java:130)
keycloak    |   at org.jboss.resteasy.core.ResourceMethodInvoker.internalInvokeOnTarget(ResourceMethodInvoker.java:660)
keycloak    |   at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTargetAfterFilter(ResourceMethodInvoker.java:524)
keycloak    |   at org.jboss.resteasy.core.ResourceMethodInvoker.lambda$invokeOnTarget$2(ResourceMethodInvoker.java:474)
keycloak    |   at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364)
keycloak    |   at org.jboss.resteasy.core.ResourceMethodInvoker.invokeOnTarget(ResourceMethodInvoker.java:476)
keycloak    |   at org.jboss.resteasy.core.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:434)
keycloak    |   at org.jboss.resteasy.core.ResourceLocatorInvoker.invokeOnTargetObject(ResourceLocatorInvoker.java:192)
keycloak    |   at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:141)
keycloak    |   at org.jboss.resteasy.core.ResourceLocatorInvoker.invoke(ResourceLocatorInvoker.java:32)
keycloak    |   at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:492)
keycloak    |   at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:261)
keycloak    |   at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:161)
keycloak    |   at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:364)
keycloak    |   at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:164)
keycloak    |   at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:247)
keycloak    |   at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:73)
keycloak    |   at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:151)
keycloak    |   at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.handle(VertxRequestHandler.java:82)
keycloak    |   at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.handle(VertxRequestHandler.java:42)
keycloak    |   at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284)
keycloak    |   at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173)
keycloak    |   at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140)
keycloak    |   at io.quarkus.vertx.http.runtime.StaticResourcesRecorder$2.handle(StaticResourcesRecorder.java:84)
keycloak    |   at io.quarkus.vertx.http.runtime.StaticResourcesRecorder$2.handle(StaticResourcesRecorder.java:71)
keycloak    |   at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284)
keycloak    |   at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173)
keycloak    |   at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140)
keycloak    |   at io.quarkus.vertx.http.runtime.VertxHttpRecorder$6.handle(VertxHttpRecorder.java:430)
keycloak    |   at io.quarkus.vertx.http.runtime.VertxHttpRecorder$6.handle(VertxHttpRecorder.java:408)
keycloak    |   at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1284)
keycloak    |   at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:173)
keycloak    |   at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:140)
keycloak    |   at org.keycloak.quarkus.runtime.integration.web.QuarkusRequestFilter.lambda$createBlockingHandler$0(QuarkusRequestFilter.java:82)
keycloak    |   at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:564)
keycloak    |   at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
keycloak    |   at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478)
keycloak    |   at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
keycloak    |   at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
keycloak    |   at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
keycloak    |   at java.base/java.lang.Thread.run(Thread.java:829)

Describe the solution you'd like

I think this code line is not run. Haven't checked how to solve it and wanted to ask first. If wanted I can offer to investigate further.

https://github.com/sventorben/keycloak-home-idp-discovery/blob/9b743b6e815a7fd8c6bc8d7c5ab94009af2f4094/src/main/java/de/sventorben/keycloak/authentication/hidpd/DomainExtractor.java#L29

Describe alternatives you've considered

No response

Anything else?

Thanks for your Keycloak Plugin! :-)

sventorben commented 1 year ago

Hello @pawl1234

I think you are facing the same issue as mentioned in #35, right?

The problem with showing an error message when email has not been validated yet, is that it would not allow for executing alternative authenticators. For example, Keycloak would not be able to fallback to password authentication.

Can you show me what your login flow looks like and elaborate a little more how you would like it to behave, please?

I am currently trying to collect as many use cases as I can, before adding additional features.

Thanks and regards Sven-Torben

pawl1234 commented 1 year ago

Hi @sventorben

Yes, thats the same thing and that "Trust Email" helps me to permanently solve the issue. Thanks.

I opened the ticket because the error message on the console was not very helpful to me. While debugging I looked at the code to understand whats happening. From this line of code I was not directly thinking of an issue with "Trust Email" because there is this part which should print the right error message https://github.com/sventorben/keycloak-home-idp-discovery/blob/9b743b6e815a7fd8c6bc8d7c5ab94009af2f4094/src/main/java/de/sventorben/keycloak/authentication/hidpd/DomainExtractor.java#L29 (at least I tought so)

What I want to achieve is very basic I think. We want to use Keycloak as a central Broker and each of our Tenants for the service we will provide, will have its own Keycloak Instance which then acts as IdP. So we will have 1 Broker and 20 IdPs and need this plugin to allow automatic selection of the right IdP for the User.

The authentication flow is currently in PoC state and looks like this grafik

sventorben commented 1 year ago

@pawl1234 I assume that this works for you now, right? I will check how to improve the error message in that case.