From b4fad32161066ef3a3367614c4efb6cd156d4edc Mon Sep 17 00:00:00 2001 From: Stephan Schnabel Date: Mon, 2 Dec 2024 17:22:27 +0100 Subject: [PATCH] Collect all unknown client ids as UNKNOWN. Fix #100 --- README.md | 3 ++- pom.xml | 2 +- .../metrics/event/MetricsEventListener.java | 15 ++++++++++++- .../kokuwa/keycloak/metrics/KeycloakIT.java | 16 ++++++++++++-- .../event/MetricsEventListenerTest.java | 21 +++++++++++++------ 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 3530a0e..2eb12ed 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ User events are added with key `keycloak_event_user_total` and tags: * `type`: [EventType](https://github.com/keycloak/keycloak/blob/main/server-spi-private/src/main/java/org/keycloak/events/EventType.java#L27) from [Event#type](https://github.com/keycloak/keycloak/blob/main/server-spi-private/src/main/java/org/keycloak/events/Event.java#L44) * `realm`: realm id from [Event#realmId](https://github.com/keycloak/keycloak/blob/main/server-spi-private/src/main/java/org/keycloak/events/Event.java#L46) -* `client`: client id from [Event#clientId](https://github.com/keycloak/keycloak/blob/main/server-spi-private/src/main/java/org/keycloak/events/Event.java#L48) +* `client`: client id from [Event#clientId](https://github.com/keycloak/keycloak/blob/main/server-spi-private/src/main/java/org/keycloak/events/Event.java#L48), unknown client_ids are grouped into UNKOWN * `error`: error from [Event#error](https://github.com/keycloak/keycloak/blob/main/server-spi-private/src/main/java/org/keycloak/events/Event.java#L56), only present for error types Examples: @@ -36,6 +36,7 @@ Examples: keycloak_event_user_total{client="test",realm="9039a0b5-e8c9-437a-a02e-9d91b04548a4",type="LOGIN",error="",} 2.0 keycloak_event_user_total{client="test",realm="1fdb3465-1675-49e8-88ad-292e2f42ee72",type="LOGIN",error="",} 1.0 keycloak_event_user_total{client="test",realm="1fdb3465-1675-49e8-88ad-292e2f42ee72",type="LOGIN_ERROR",error="invalid_user_credentials",} 1.0 +keycloak_event_user_total{client="UNKNOWN",realm="1fdb3465-1675-49e8-88ad-292e2f42ee72",type="LOGIN_ERROR",error="invalid_user_credentials",} 1.0 ``` ### Admin Events diff --git a/pom.xml b/pom.xml index 69362fd..3dfa01b 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ io.kokuwa.keycloak keycloak-event-metrics - 1.0.1-SNAPSHOT + 1.1.0-SNAPSHOT Keycloak Metrics Provides metrics for Keycloak user/admin events diff --git a/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java b/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java index 4ba5144..0bed727 100644 --- a/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java +++ b/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java @@ -1,11 +1,13 @@ package io.kokuwa.keycloak.metrics.event; +import java.util.Objects; import java.util.Optional; import org.jboss.logging.Logger; import org.keycloak.events.Event; import org.keycloak.events.EventListenerProvider; import org.keycloak.events.admin.AdminEvent; +import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -33,7 +35,7 @@ public class MetricsEventListener implements EventListenerProvider, AutoCloseabl Metrics.counter("keycloak_event_user", "realm", toBlank(replaceIds ? getRealmName(event.getRealmId()) : event.getRealmId()), "type", toBlank(event.getType()), - "client", toBlank(event.getClientId()), + "client", getClientId(event.getClientId()), "error", toBlank(event.getError())) .increment(); } @@ -65,6 +67,17 @@ public class MetricsEventListener implements EventListenerProvider, AutoCloseabl }); } + private String getClientId(String clientId) { + return Optional.ofNullable(session.getContext()) + .map(KeycloakContext::getClient) + .filter(model -> Objects.equals(model.getClientId(), clientId)) + .map(ClientModel::getClientId) + .orElseGet(() -> { + log.tracev("Client for id {0} is unknown", clientId); + return "UNKNOWN"; + }); + } + private String toBlank(Object value) { return value == null ? "" : value.toString(); } diff --git a/src/test/java/io/kokuwa/keycloak/metrics/KeycloakIT.java b/src/test/java/io/kokuwa/keycloak/metrics/KeycloakIT.java index 1eaa0b1..cd4ebf4 100644 --- a/src/test/java/io/kokuwa/keycloak/metrics/KeycloakIT.java +++ b/src/test/java/io/kokuwa/keycloak/metrics/KeycloakIT.java @@ -48,6 +48,9 @@ public class KeycloakIT { keycloak.createClient(realmName2, clientId2); keycloak.createUser(realmName2, username2, password2); + var clientId3 = realmName2 + "_" + UUID.randomUUID(); + var clientId4 = realmName2 + "_" + UUID.randomUUID(); + prometheus.scrap(); var loginBefore = prometheus.userEvent(EventType.LOGIN); var loginBefore1 = prometheus.userEvent(EventType.LOGIN, realmName1, clientId1); @@ -55,10 +58,13 @@ public class KeycloakIT { var loginErrorBefore = prometheus.userEvent(EventType.LOGIN_ERROR); var loginErrorBefore1 = prometheus.userEvent(EventType.LOGIN_ERROR, realmName1, clientId1); var loginErrorBefore2 = prometheus.userEvent(EventType.LOGIN_ERROR, realmName2, clientId2); + var loginErrorBeforeUNKNOWN = prometheus.userEvent(EventType.LOGIN_ERROR, realmName2, "UNKNOWN"); assertDoesNotThrow(() -> keycloak.login(clientId1, realmName1, username1, password1)); assertDoesNotThrow(() -> keycloak.login(clientId1, realmName1, username1, password1)); assertDoesNotThrow(() -> keycloak.login(clientId2, realmName2, username2, password2)); + assertThrows(NotAuthorizedException.class, () -> keycloak.login(clientId3, realmName2, "nope", "nö")); + assertThrows(NotAuthorizedException.class, () -> keycloak.login(clientId4, realmName2, "foo", "bar")); assertThrows(NotAuthorizedException.class, () -> keycloak.login(clientId2, realmName2, username2, "nope")); prometheus.scrap(); @@ -68,14 +74,20 @@ public class KeycloakIT { var loginErrorAfter = prometheus.userEvent(EventType.LOGIN_ERROR); var loginErrorAfter1 = prometheus.userEvent(EventType.LOGIN_ERROR, realmName1, clientId1); var loginErrorAfter2 = prometheus.userEvent(EventType.LOGIN_ERROR, realmName2, clientId2); + var loginErrorAfter3 = prometheus.userEvent(EventType.LOGIN_ERROR, realmName2, clientId3); + var loginErrorAfter4 = prometheus.userEvent(EventType.LOGIN_ERROR, realmName2, clientId4); + var loginErrorAfterUNKNOWN = prometheus.userEvent(EventType.LOGIN_ERROR, realmName2, "UNKNOWN"); assertAll("prometheus", () -> assertEquals(loginBefore + 3, loginAfter, "login success total"), () -> assertEquals(loginBefore1 + 2, loginAfter1, "login success #1"), () -> assertEquals(loginBefore2 + 1, loginAfter2, "login success #2"), - () -> assertEquals(loginErrorBefore + 1, loginErrorAfter, "login failure total"), + () -> assertEquals(loginErrorBefore + 3, loginErrorAfter, "login failure total"), () -> assertEquals(loginErrorBefore1 + 0, loginErrorAfter1, "login failure #1"), - () -> assertEquals(loginErrorBefore2 + 1, loginErrorAfter2, "login failure #2")); + () -> assertEquals(loginErrorBefore2 + 1, loginErrorAfter2, "login failure #2"), + () -> assertEquals(0, loginErrorAfter3, "login failure #3"), + () -> assertEquals(0, loginErrorAfter4, "login failure #4"), + () -> assertEquals(loginErrorBeforeUNKNOWN + 2 , loginErrorAfterUNKNOWN, "login failure UNKNOWN")); } @DisplayName("user count") diff --git a/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java b/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java index 16684c5..d54fd36 100644 --- a/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java +++ b/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java @@ -13,6 +13,7 @@ import org.keycloak.events.EventType; import org.keycloak.events.admin.AdminEvent; import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.ResourceType; +import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -37,6 +38,8 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { @Mock RealmProvider realmProvider; @Mock + ClientModel clientModel; + @Mock KeycloakContext context; @DisplayName("onEvent(true)") @@ -54,8 +57,10 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { when(session.getContext()).thenReturn(context); when(context.getRealm()).thenReturn(realmModel); + when(context.getClient()).thenReturn(clientModel); when(realmModel.getId()).thenReturn(realmId); when(realmModel.getName()).thenReturn(realmName); + when(clientModel.getClientId()).thenReturn(clientId); listener(true).onEvent(toEvent(realmId, clientId, type, null)); assertEvent(realmName, clientId, type.toString(), ""); @@ -73,8 +78,10 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { when(session.getContext()).thenReturn(context); when(context.getRealm()).thenReturn(realmModel); + when(context.getClient()).thenReturn(clientModel); when(realmModel.getId()).thenReturn(realmId); when(realmModel.getName()).thenReturn(realmName); + when(clientModel.getClientId()).thenReturn(clientId); listener(true).onEvent(toEvent(realmId, clientId, type, error)); assertEvent(realmName, clientId, type.toString(), error); @@ -91,7 +98,7 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { when(realmModel.getName()).thenReturn(realmName); listener(true).onEvent(toEvent(null, null, null, null)); - assertEvent(realmName, "", "", ""); + assertEvent(realmName, "UNKNOWN", "", ""); } @DisplayName("replace(true) - context is null") @@ -108,7 +115,7 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { when(realmModel.getName()).thenReturn(realmName); listener(true).onEvent(toEvent(realmId, clientId, type, null)); - assertEvent(realmName, clientId, type.toString(), ""); + assertEvent(realmName, "UNKNOWN", type.toString(), ""); } @DisplayName("replace(true) - context is empty") @@ -126,7 +133,7 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { when(realmModel.getName()).thenReturn(realmName); listener(true).onEvent(toEvent(realmId, clientId, type, null)); - assertEvent(realmName, clientId, type.toString(), ""); + assertEvent(realmName, "UNKNOWN", type.toString(), ""); } @DisplayName("replace(true) - realmId is unknown") @@ -140,7 +147,9 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { when(session.getContext()).thenReturn(context); when(session.realms()).thenReturn(realmProvider); when(context.getRealm()).thenReturn(realmModel); + when(context.getClient()).thenReturn(clientModel); when(realmModel.getId()).thenReturn(UUID.randomUUID().toString()); + when(clientModel.getClientId()).thenReturn(clientId); listener(true).onEvent(toEvent(realmId, clientId, type, null)); assertEvent(realmId, clientId, type.toString(), ""); @@ -155,7 +164,7 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { var type = EventType.LOGIN; listener(false).onEvent(toEvent(realmId, clientId, type, null)); - assertEvent(realmId, clientId, type.toString(), ""); + assertEvent(realmId, "UNKNOWN", type.toString(), ""); } @DisplayName("replace(false) - with error") @@ -168,14 +177,14 @@ public class MetricsEventListenerTest extends AbstractMockitoTest { var error = UUID.randomUUID().toString(); listener(false).onEvent(toEvent(realmId, clientId, type, error)); - assertEvent(realmId, clientId, type.toString(), error); + assertEvent(realmId, "UNKNOWN", type.toString(), error); } @DisplayName("replace(false) - all fields empty") @Test void notReplaceFieldsEmpty() { listener(false).onEvent(toEvent(null, null, null, null)); - assertEvent("", "", "", ""); + assertEvent("", "UNKNOWN", "", ""); } private Event toEvent(String realmId, String clientId, EventType type, String error) { -- 2.47.2