From 8d530324320869cf27305c7d1ee5b55638ce98eb Mon Sep 17 00:00:00 2001 From: Thomas Mortagne Date: Thu, 12 Dec 2024 18:30:18 +0100 Subject: [PATCH] XWIKI-22726: Allow customizing the validation of HQL queries through configuration * add unit tests --- ...igurableHQLCompleteStatementValidator.java | 19 +++++--- .../hibernate/query/HqlQueryExecutorTest.java | 44 +++++++++++++++++-- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/query/hql/internal/ConfigurableHQLCompleteStatementValidator.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/query/hql/internal/ConfigurableHQLCompleteStatementValidator.java index 2a3f2212e8d..c07d1036998 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/query/hql/internal/ConfigurableHQLCompleteStatementValidator.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/org/xwiki/query/hql/internal/ConfigurableHQLCompleteStatementValidator.java @@ -74,14 +74,19 @@ private List getPatterns(String key) { List patternStrings = this.configuration.getProperty(key, List.class); - List patterns = new ArrayList<>(patternStrings.size()); - for (String patternString : patternStrings) { - try { - patterns.add(Pattern.compile(patternString)); - } catch (Exception e) { - this.logger.warn("Failed to parse pattern [{}] for configuration [{}]: {}", patternString, key, - ExceptionUtils.getRootCauseMessage(e)); + List patterns; + if (patternStrings != null) { + patterns = new ArrayList<>(patternStrings.size()); + for (String patternString : patternStrings) { + try { + patterns.add(Pattern.compile(patternString)); + } catch (Exception e) { + this.logger.warn("Failed to parse pattern [{}] for configuration [{}]: {}", patternString, key, + ExceptionUtils.getRootCauseMessage(e)); + } } + } else { + patterns = List.of(); } return patterns; diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/store/hibernate/query/HqlQueryExecutorTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/store/hibernate/query/HqlQueryExecutorTest.java index c9964974582..eea286d3689 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/store/hibernate/query/HqlQueryExecutorTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/store/hibernate/query/HqlQueryExecutorTest.java @@ -38,13 +38,16 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.xwiki.component.manager.ComponentManager; +import org.xwiki.configuration.ConfigurationSource; import org.xwiki.context.Execution; import org.xwiki.context.ExecutionContext; import org.xwiki.query.Query; import org.xwiki.query.QueryException; import org.xwiki.query.QueryFilter; import org.xwiki.query.WrappingQuery; +import org.xwiki.query.hql.internal.ConfigurableHQLCompleteStatementValidator; import org.xwiki.query.hql.internal.DefaultHQLStatementValidator; +import org.xwiki.query.hql.internal.HQLCompleteStatementValidator; import org.xwiki.query.hql.internal.StandardHQLCompleteStatementValidator; import org.xwiki.query.internal.DefaultQuery; import org.xwiki.security.authorization.ContextualAuthorizationManager; @@ -77,14 +80,18 @@ * @version $Id$ */ @ComponentTest -@ComponentList({DefaultHQLStatementValidator.class, StandardHQLCompleteStatementValidator.class}) +@ComponentList({DefaultHQLStatementValidator.class, StandardHQLCompleteStatementValidator.class, + ConfigurableHQLCompleteStatementValidator.class}) class HqlQueryExecutorTest { @InjectMockComponents private HqlQueryExecutor executor; @InjectMockComponents - private DefaultHQLStatementValidator defaultQueryValidator; + private DefaultHQLStatementValidator defaultValidator; + + @InjectMockComponents + private ConfigurableHQLCompleteStatementValidator configurableValidator; @MockComponent private ContextualAuthorizationManager authorization; @@ -98,6 +105,10 @@ class HqlQueryExecutorTest @MockComponent private Execution execution; + @MockComponent + @Named("xwikiproperties") + private ConfigurationSource xwikiproperties; + @MockComponent @Named("context") private ComponentManager contextComponentMannager; @@ -109,6 +120,11 @@ public void afterComponent() { when(this.hibernateStore.getConfiguration()).thenReturn(new Configuration()); when(this.hibernateStore.getConfigurationMetadata()).thenReturn(mock(Metadata.class)); + + when(this.xwikiproperties.getProperty("query.hql.safe", List.class)) + .thenReturn(List.of("select normallynotallowed from XWikiDocument as doc where 1=\\d+")); + when(this.xwikiproperties.getProperty("query.hql.unsafe", List.class)) + .thenReturn(List.of("select name from XWikiDocument as doc where 1=\\d+")); } @BeforeEach @@ -125,6 +141,10 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable this.hasProgrammingRight = true; + when(this.contextComponentMannager + .getInstanceList(HQLCompleteStatementValidator.class)) + .thenReturn(List.of(configurableValidator)); + // Actual Hibernate query ExecutionContext executionContext = mock(ExecutionContext.class); @@ -383,7 +403,6 @@ void executeShortFromHQLQueryWithProgrammingRights() throws QueryException void executeCompleteHQLQueryWithProgrammingRights() throws QueryException { execute("select u from XWikiDocument as doc", true); - } @Test @@ -404,6 +423,12 @@ void executeShortFromHQLQueryWithoutProgrammingRights() throws QueryException execute(", BaseObject as obj", false); } + @Test + void executeWhenOverwrittenAllowedSelect() throws Exception + { + execute("select normallynotallowed from XWikiDocument as doc where 1=42", false); + } + // Not allowed @Test @@ -456,4 +481,17 @@ void executeUpdateWithoutProgrammingRight() throws Exception expected.getCause().getMessage()); } } + + @Test + void executeWhenOverwrittenUnsafeSelect() throws Exception + { + try { + execute("select name from XWikiDocument as doc where 1=42", false); + } catch (QueryException expected) { + assertEquals( + "The query requires programming right." + + " Query statement = [select name from XWikiDocument as doc where 1=42]", + expected.getCause().getMessage()); + } + } }