From 593c14cd26cfcba9a5921cb0c39149dc32d012d3 Mon Sep 17 00:00:00 2001 From: vramik Date: Mon, 23 Oct 2023 15:35:30 +0200 Subject: [PATCH] Data too long for column 'DETAILS_JSON' Closes #17258 --- .../server_admin/topics/events/login.adoc | 14 ------- .../topics/keycloak/changes-23_0_0.adoc | 4 ++ .../keycloak/events/jpa/AdminEventEntity.java | 2 +- .../org/keycloak/events/jpa/EventEntity.java | 8 +++- .../events/jpa/JpaEventStoreProvider.java | 42 +++---------------- .../jpa/JpaEventStoreProviderFactory.java | 8 +--- .../META-INF/jpa-changelog-23.0.0.xml | 7 ++++ .../testsuite/admin/event/AdminEventTest.java | 19 +++++++++ .../model/events/AdminEventQueryTest.java | 28 +++++++++++++ .../model/events/EventQueryTest.java | 36 ++++++++++++++++ 10 files changed, 107 insertions(+), 61 deletions(-) diff --git a/docs/documentation/server_admin/topics/events/login.adoc b/docs/documentation/server_admin/topics/events/login.adoc index 1df417920f..899bd797e9 100644 --- a/docs/documentation/server_admin/topics/events/login.adoc +++ b/docs/documentation/server_admin/topics/events/login.adoc @@ -171,17 +171,3 @@ You can exclude events by using the `--spi-events-listener-email-exclude-events` kc.[sh|bat] --spi-events-listener-email-exclude-events=UPDATE_TOTP,REMOVE_TOTP ---- -You can set a maximum length of each Event detail in the database by using the `--spi-events-store-jpa-max-detail-length` argument. This setting is useful if a detail (for example, redirect_uri) is long. For example: - -[source,bash] ----- -kc.[sh|bat] --spi-events-store-jpa-max-detail-length=1000 ----- - -Also you can set a maximum length of all Event's details by using the `--spi-events-store-jpa-max-field-length` argument. This setting is useful if you want to adhere to the underlying storage limitation. For example: - -[source,bash] ----- -kc.[sh|bat] --spi-events-store-jpa-max-field-length=2500 ----- - diff --git a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc index c3655f90f2..66410d02b5 100644 --- a/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc +++ b/docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc @@ -141,3 +141,7 @@ Relying on RESTEasy Classic is not longer an option because it is not available = Partial export requires manage-realm permission The endpoint `POST {keycloak server}/realms/{realm}/partial-export` and the corresponding action in the admin console now require `manage-realm` permission for execution instead of `view-realm`. This endpoint exports the realm configuration into a JSON file and the new permission is more appropriate. The parameters `exportGroupsAndRoles` and `exportClients`, which include the realm groups/roles and clients in the export respectively, continue managing the same permissions (`query-groups` and `view-clients`). + += Removal of the options to trim the event's details length + +Since this release, Keycloak supports long value for `EventEntity` details column. Therefore, it no longer supports options for trimming event detail length `--spi-events-store-jpa-max-detail-length` and `--spi-events-store-jpa-max-field-length`. diff --git a/model/jpa/src/main/java/org/keycloak/events/jpa/AdminEventEntity.java b/model/jpa/src/main/java/org/keycloak/events/jpa/AdminEventEntity.java index f5be1d5d02..76655a6246 100644 --- a/model/jpa/src/main/java/org/keycloak/events/jpa/AdminEventEntity.java +++ b/model/jpa/src/main/java/org/keycloak/events/jpa/AdminEventEntity.java @@ -60,7 +60,7 @@ public class AdminEventEntity { @Column(name="RESOURCE_PATH") private String resourcePath; - @Column(name="REPRESENTATION", length = 25500) + @Column(name="REPRESENTATION") private String representation; @Column(name="ERROR") diff --git a/model/jpa/src/main/java/org/keycloak/events/jpa/EventEntity.java b/model/jpa/src/main/java/org/keycloak/events/jpa/EventEntity.java index 1e26a486da..38cab319a0 100644 --- a/model/jpa/src/main/java/org/keycloak/events/jpa/EventEntity.java +++ b/model/jpa/src/main/java/org/keycloak/events/jpa/EventEntity.java @@ -57,9 +57,13 @@ public class EventEntity { @Column(name="ERROR") private String error; + // This is the legacy field which is kept here to be able to read old events without the need to migrate them @Column(name="DETAILS_JSON", length = 2550) private String detailsJson; + @Column(name="DETAILS_JSON_LONG_VALUE") + private String detailsJsonLongValue; + public String getId() { return id; } @@ -133,11 +137,11 @@ public class EventEntity { } public String getDetailsJson() { - return detailsJson; + return detailsJsonLongValue != null ? detailsJsonLongValue : detailsJson; } public void setDetailsJson(String detailsJson) { - this.detailsJson = detailsJson; + this.detailsJsonLongValue = detailsJson; } } diff --git a/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java b/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java index e1a1690f7d..28e3ff6af9 100755 --- a/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java +++ b/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProvider.java @@ -39,7 +39,6 @@ import org.keycloak.models.utils.KeycloakModelUtils; import jakarta.persistence.EntityManager; import jakarta.persistence.TypedQuery; import java.io.IOException; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -57,14 +56,10 @@ public class JpaEventStoreProvider implements EventStoreProvider { private final KeycloakSession session; private final EntityManager em; - private final int maxDetailLength; - private final int maxFieldLength; - public JpaEventStoreProvider(KeycloakSession session, EntityManager em, int maxDetailLength, int maxFieldLength) { + public JpaEventStoreProvider(KeycloakSession session, EntityManager em) { this.session = session; this.em = em; - this.maxDetailLength = maxDetailLength; - this.maxFieldLength = maxFieldLength; } @Override @@ -165,40 +160,13 @@ public class JpaEventStoreProvider implements EventStoreProvider { eventEntity.setIpAddress(event.getIpAddress()); eventEntity.setError(event.getError()); try { - if (maxDetailLength > 0 && event.getDetails() != null) { - Map result = new HashMap<>(event.getDetails()); - result.entrySet().forEach(t -> t.setValue(trimToMaxDetailLength(t.getValue()))); - - eventEntity.setDetailsJson(trimToMaxFieldLength(mapper.writeValueAsString(result))); - } else { - eventEntity.setDetailsJson(mapper.writeValueAsString(event.getDetails())); - } + eventEntity.setDetailsJson(mapper.writeValueAsString(event.getDetails())); } catch (IOException ex) { logger.error("Failed to write log details", ex); } return eventEntity; } - private String trimToMaxDetailLength(String detail) { - if (detail != null && detail.length() > maxDetailLength) { - logger.warnf("Detail '%s' will be truncated.", detail); - // (maxDetailLength - 3) takes "..." into account - return detail.substring(0, maxDetailLength - 3).concat("..."); - } else { - return detail; - } - } - - private String trimToMaxFieldLength(String field) { - if (maxFieldLength > 0 && field != null && field.length() > maxFieldLength) { - logger.warnf("Field '%s' will be truncated.", field); - // (maxFieldLength - 3) takes "..." into account - return field.substring(0, maxFieldLength - 3).concat("..."); - } else { - return field; - } - } - static Event convertEvent(EventEntity eventEntity) { Event event = new Event(); event.setId(eventEntity.getId() == null ? UUID.randomUUID().toString() : eventEntity.getId()); @@ -234,8 +202,8 @@ public class JpaEventStoreProvider implements EventStoreProvider { adminEventEntity.setResourcePath(adminEvent.getResourcePath()); adminEventEntity.setError(adminEvent.getError()); - if(includeRepresentation) { - adminEventEntity.setRepresentation(trimToMaxFieldLength(adminEvent.getRepresentation())); + if (includeRepresentation) { + adminEventEntity.setRepresentation(adminEvent.getRepresentation()); } return adminEventEntity; } @@ -255,7 +223,7 @@ public class JpaEventStoreProvider implements EventStoreProvider { adminEvent.setResourcePath(adminEventEntity.getResourcePath()); adminEvent.setError(adminEventEntity.getError()); - if(adminEventEntity.getRepresentation() != null) { + if (adminEventEntity.getRepresentation() != null) { adminEvent.setRepresentation(adminEventEntity.getRepresentation()); } return adminEvent; diff --git a/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProviderFactory.java b/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProviderFactory.java index 49506c7b00..24868b3037 100755 --- a/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/events/jpa/JpaEventStoreProviderFactory.java @@ -32,21 +32,15 @@ import org.keycloak.storage.datastore.PeriodicEventInvalidation; public class JpaEventStoreProviderFactory implements EventStoreProviderFactory, InvalidationHandler { public static final String ID = "jpa"; - private int maxDetailLength; - private int maxFieldLength; @Override public EventStoreProvider create(KeycloakSession session) { JpaConnectionProvider connection = session.getProvider(JpaConnectionProvider.class); - return new JpaEventStoreProvider(session, connection.getEntityManager(), maxDetailLength, maxFieldLength); + return new JpaEventStoreProvider(session, connection.getEntityManager()); } @Override public void init(Config.Scope config) { - maxDetailLength = config.getInt("max-detail-length", -1); - maxFieldLength = config.getInt("max-field-length", -1); - if (maxDetailLength != -1 && maxDetailLength < 3) throw new IllegalArgumentException("max-detail-length cannot be less that 3."); - if (maxFieldLength != -1 && maxFieldLength < 3) throw new IllegalArgumentException("max-field-length cannot be less that 3."); } @Override diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-23.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-23.0.0.xml index 4f35cc440d..4c499231a4 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-23.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-23.0.0.xml @@ -30,4 +30,11 @@ + + + + + + + diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/event/AdminEventTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/event/AdminEventTest.java index b99518cca8..23d08e05d8 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/event/AdminEventTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/event/AdminEventTest.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.admin.event; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.Before; import org.junit.Test; import org.keycloak.admin.client.resource.RealmResource; @@ -45,6 +46,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertNull; import static org.keycloak.testsuite.auth.page.AuthRealm.MASTER; @@ -167,6 +169,23 @@ public class AdminEventTest extends AbstractEventTest { assertThat(realm.getAdminEvents(null, null, null, null, null, null, null, null, 0, 1000).size(), is(greaterThanOrEqualTo(110))); } + @Test + public void adminEventRepresentationLenght() { + RealmResource realm = adminClient.realms().realm("test"); + AdminEventRepresentation event = new AdminEventRepresentation(); + event.setOperationType(OperationType.CREATE.toString()); + event.setAuthDetails(new AuthDetailsRepresentation()); + event.setRealmId(realm.toRepresentation().getId()); + String longValue = RandomStringUtils.random(30000, true, true); + event.setRepresentation(longValue); + + testingClient.testing("test").onAdminEvent(event, true); + List adminEvents = realm.getAdminEvents(null, null, null, null, null, null, null, null, null, null); + + assertThat(adminEvents, hasSize(1)); + assertThat(adminEvents.get(0).getRepresentation(), equalTo(longValue)); + } + @Test public void orderResultsTest() { RealmResource realm = adminClient.realms().realm("test"); diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/AdminEventQueryTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/AdminEventQueryTest.java index aa411e1210..bc961c1e14 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/AdminEventQueryTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/AdminEventQueryTest.java @@ -17,8 +17,11 @@ package org.keycloak.testsuite.model.events; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.Test; import org.keycloak.common.ClientConnection; import org.keycloak.events.EventStoreProvider; @@ -38,6 +41,7 @@ import org.keycloak.testsuite.model.KeycloakModelTest; import org.keycloak.testsuite.model.RequireProvider; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; @RequireProvider(EventStoreProvider.class) @@ -113,6 +117,30 @@ public class AdminEventQueryTest extends KeycloakModelTest { }); } + @Test + public void testAdminEventRepresentationLongValue() { + String longValue = RandomStringUtils.random(30000, true, true); + + withRealm(realmId, (session, realm) -> { + + AdminEvent event = createClientEvent(realm, OperationType.CREATE); + event.setRepresentation(longValue); + + session.getProvider(EventStoreProvider.class).onEvent(event, true); + + return null; + }); + + withRealm(realmId, (session, realm) -> { + List events = session.getProvider(EventStoreProvider.class).createAdminQuery().realm(realmId).getResultStream().collect(Collectors.toList()); + assertThat(events, hasSize(1)); + + assertThat(events.get(0).getRepresentation(), equalTo(longValue)); + + return null; + }); + } + private AdminEvent createClientEvent(RealmModel realm, OperationType operation) { return new AdminEventBuilder(realm, new DummyAuth(realm), null, DummyClientConnection.DUMMY_CONNECTION) .resource(ResourceType.CLIENT).operation(operation).getEvent(); diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/EventQueryTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/EventQueryTest.java index 6724403086..ffadf60ce9 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/EventQueryTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/events/EventQueryTest.java @@ -27,9 +27,13 @@ import org.keycloak.models.RealmModel; import org.keycloak.testsuite.model.KeycloakModelTest; import org.keycloak.testsuite.model.RequireProvider; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; /** @@ -134,6 +138,38 @@ public class EventQueryTest extends KeycloakModelTest { }); } + @Test + public void testEventDetailsLongValue() { + String v1 = RandomStringUtils.random(1000, true, true); + String v2 = RandomStringUtils.random(1000, true, true); + String v3 = RandomStringUtils.random(1000, true, true); + String v4 = RandomStringUtils.random(1000, true, true); + + withRealm(realmId, (session, realm) -> { + + Map details = Map.of("k1", v1, "k2", v2, "k3", v3, "k4", v4); + + Event event = createAuthEventForUser(session, realm, "u1"); + event.setDetails(details); + + session.getProvider(EventStoreProvider.class).onEvent(event); + + return null; + }); + + withRealm(realmId, (session, realm) -> { + List events = session.getProvider(EventStoreProvider.class).createQuery().realm(realmId).getResultStream().collect(Collectors.toList()); + assertThat(events, hasSize(1)); + Map details = events.get(0).getDetails(); + + assertThat(details.get("k1"), equalTo(v1)); + assertThat(details.get("k2"), equalTo(v2)); + assertThat(details.get("k3"), equalTo(v3)); + assertThat(details.get("k4"), equalTo(v4)); + return null; + }); + } + private static class DummyClientConnection implements ClientConnection { private static DummyClientConnection DUMMY_CONNECTION = new DummyClientConnection();