p2-inc / keycloak-events

Useful Keycloak event listener implementations and utilities.
https://phasetwo.io
Other
194 stars 37 forks source link

java.lang.NullPointerException when brute force deteciton is enabled on Keycloak 24.0.1 #52

Open jacquemard opened 8 months ago

jacquemard commented 8 months ago

With Keycloak 24.0.1, under some conditions, the following error occurs. Seems to be when we activate brute force detection with ext-event-http. On Keycloak 23.0.7, everything seems to work fine. I have also tested to build from the branch xgp/24-testcontainers, and the same error occurs :

deepsso-1         | 2024-03-08 09:56:48,494 WARN  [io.phasetwo.keycloak.events.MultiEventListenerProviderFactory] (Brute Force Protector) Error configuring provider: java.lang.NullPointerException: Cannot invoke "org.keycloak.models.RealmModel.getName()" because the return value of "io.phasetwo.keycloak.config.ConfigurationAware.getRealm(org.keycloak.models.KeycloakSession)" is null
deepsso-1         |     at io.phasetwo.keycloak.config.ConfigurationAware.getConfigurations(ConfigurationAware.java:17)
deepsso-1         |     at io.phasetwo.keycloak.events.MultiEventListenerProviderFactory.create(MultiEventListenerProviderFactory.java:23)
deepsso-1         |     at io.phasetwo.keycloak.events.MultiEventListenerProviderFactory.create(MultiEventListenerProviderFactory.java:13)
deepsso-1         |     at org.keycloak.services.DefaultKeycloakSession.getProvider(DefaultKeycloakSession.java:195)
deepsso-1         |     at org.keycloak.events.EventBuilder.lambda$getEventListeners$0(EventBuilder.java:84)
deepsso-1         |     at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
deepsso-1         |     at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1707)
deepsso-1         |     at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
deepsso-1         |     at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
deepsso-1         |     at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
deepsso-1         |     at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
deepsso-1         |     at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
deepsso-1         |     at org.keycloak.events.EventBuilder.getEventListeners(EventBuilder.java:93)
deepsso-1         |     at org.keycloak.events.EventBuilder.<init>(EventBuilder.java:68)
deepsso-1         |     at org.keycloak.services.managers.DefaultBruteForceProtector.sendEvent(DefaultBruteForceProtector.java:240)
deepsso-1         |     at org.keycloak.services.managers.DefaultBruteForceProtector.failure(DefaultBruteForceProtector.java:203)
deepsso-1         |     at org.keycloak.services.managers.DefaultBruteForceProtector.run(DefaultBruteForceProtector.java:291)
deepsso-1         |     at java.base/java.lang.Thread.run(Thread.java:840)

I am not sure what the current status regarding the support of KC 24 is, but I thought I might as well report this issue.

Steps to reproduce

version: '3.3'
services:
  deepsso:
    environment:
      - KEYCLOAK_ADMIN=admin
      - KEYCLOAK_ADMIN_PASSWORD=admin
    image: quay.io/keycloak/keycloak:24.0.1
    command:
      - "start-dev"
    volumes:
      - ./providers:/opt/keycloak/providers
    ports:
      - 8080:8080

  webhook-tester:
    image: coveros/webhook-tester
    hostname: webhook.internal
{
  "id": "test",
  "realm": "test",
  "attributes": {
    "_providerConfig.ext-event-http": "{\"targetUri\": \"http://webhook.internal:8080/json-hook\",\"sharedSecret\":\"abcd123\"}"
  }
}

In the created realm:

Let me know if you need additional information.

xgp commented 8 months ago

Thanks for the report. We are currently testing 24, and we will incorporate this before a release that supports that version.

xgp commented 8 months ago

@jacquemard It looks like there is a regression in Keycloak where KeycloakContext is not set in session when executing DefaultBruteForceProtector.sendEvent. I filed this in the Keycloak issues https://github.com/keycloak/keycloak/issues/27740

We'll probably stop the NPE from being thrown, but since there is no way to get the realm, the listener will just fail silently until Keycloak fixes the upstream bug.

jacquemard commented 8 months ago

Oh ok I see, thanks for the details and clarifying the issue, in that case we'll probably wait for the fix from keycloak side to be pushed before upgrading to v24.

xgp commented 8 months ago

51 has a fix to stop this throwing an exception, but we will not fix the core regression from Keycloak. However, it is now on their radar, and they actually looked at the bug.

xgp commented 7 months ago

@jacquemard this isn't going to be changed by the Keycloak team, so this event type won't be received by this event listener type for the time being. I'll leave this open until we have the time to figure out a workaround or someone wants to suggest and PR a fix.

jacquemard commented 7 months ago

Thanks a lot @xgp for the update and your involvement on this. I'll see what we will do concerning this issue.

spinanicky commented 3 months ago

@xgp Has this been resolved? Still seeing this error on webhook. (Using V.30)

2024-07-24 20:53:50,561 WARN  [io.phasetwo.keycloak.events.WebhookSenderEventListenerProvider] (executor-thread-39) Error converting and scheduling event: org.keycloak.events.admin.AdminEvent@529cfc54: java.lang.NullPointerException: Cannot invoke "org.keycloak.models.UserModel.getUsername()" because "user" is null

    at io.phasetwo.keycloak.events.WebhookSenderEventListenerProvider.completeAdminEventAttributes(WebhookSenderEventListenerProvider.java:166)

    at io.phasetwo.keycloak.events.WebhookSenderEventListenerProvider.onEvent(WebhookSenderEventListenerProvider.java:69)

    at org.keycloak.services.resources.admin.AdminEventBuilder.send(AdminEventBuilder.java:290)

    at org.keycloak.services.resources.admin.AdminEventBuilder.success(AdminEventBuilder.java:272)

    at org.keycloak.services.resources.admin.GroupResource.deleteGroup(GroupResource.java:159)
xgp commented 3 months ago

@spinanicky That's a different error. It looks like there are cases where there is a userId in the event, but the lookup of that user returns null. Can you file a separate issue for this?

Where it's failing: https://github.com/p2-inc/keycloak-events/blob/main/src/main/java/io/phasetwo/keycloak/events/WebhookSenderEventListenerProvider.java#L166