Skip to content

Commit

Permalink
XWIKI-22726: Allow customizing the validation of HQL queries through …
Browse files Browse the repository at this point in the history
…configuration

* refactor the internal validator system to make it easier to reusable for other use cases
* make sure the configurable validator is executed before the standard one
* improve safe named queries initialization
* tried to remove stuff from HqlQueryExecutor, but was not enough to get rid of the ClassFanOutComplexity unfortunately
* various documentation improvements
  • Loading branch information
tmortagne committed Dec 13, 2024
1 parent cc24beb commit b4c568b
Show file tree
Hide file tree
Showing 11 changed files with 257 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xwiki.query.Query;
import org.xwiki.query.WrappingQuery;

import net.sf.jsqlparser.JSQLParserException;
import net.sf.jsqlparser.expression.Expression;
Expand Down Expand Up @@ -343,4 +345,50 @@ public static String replaceLegacyQueryParameters(String queryString)
convertedString = builder.toString();
}
}

/**
* @param statement a potentially short form statement to complete
* @return a complete version of the input {@link Query} (or the {@link Query} as is if it's already complete)
* @since 17.0.0RC1
* @since 16.10.2
* @since 15.10.16
* @since 16.4.6
*/
public static String toCompleteStatement(String statement)
{
String completeStatement = statement;

if (StringUtils.isEmpty(statement) || isShortFormStatement(statement)) {
completeStatement = "select doc.fullName from XWikiDocument doc " + statement.trim();
}

return completeStatement;
}

/**
* @param query a potentially short form query to complete
* @return a complete version of the input {@link Query} (or the {@link Query} as is if it's already complete)
* @since 17.0.0RC1
* @since 16.10.2
* @since 15.10.16
* @since 16.4.6
*/
public static Query toCompleteQuery(Query query)
{
Query completeQuery = query;

String completeStatement = toCompleteStatement(query.getStatement());
if (completeStatement != query.getStatement()) {
completeQuery = new WrappingQuery(query)
{
@Override
public String getStatement()
{
return completeStatement;
}
};
}

return completeQuery;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;

import javax.inject.Inject;
Expand Down Expand Up @@ -53,8 +52,7 @@
import org.xwiki.query.QueryParameter;
import org.xwiki.query.SecureQuery;
import org.xwiki.query.WrappingQuery;
import org.xwiki.query.hql.internal.HQLQueryValidator;
import org.xwiki.query.internal.DefaultQuery;
import org.xwiki.query.hql.internal.HQLStatementValidator;
import org.xwiki.security.authorization.ContextualAuthorizationManager;
import org.xwiki.security.authorization.Right;

Expand All @@ -63,6 +61,7 @@
import com.xpn.xwiki.internal.store.hibernate.query.HqlQueryUtils;
import com.xpn.xwiki.store.XWikiHibernateStore;
import com.xpn.xwiki.util.Util;
import com.xpn.xwiki.web.Utils;

/**
* {@link QueryExecutor} implementation for Hibernate Store.
Expand Down Expand Up @@ -99,6 +98,9 @@ public class HqlQueryExecutor implements QueryExecutor, Initializable
@Named("context")
private Provider<ComponentManager> componentManagerProvider;

@Inject
private HQLStatementValidator queryValidator;

@Inject
private Logger logger;

Expand All @@ -117,21 +119,23 @@ private Set<String> getSafeNamedQueries()
if (this.safeNamedQueries == null) {
synchronized (this) {
if (this.safeNamedQueries == null) {
this.safeNamedQueries = new HashSet<>();
Set<String> safeQueries = new HashSet<>();

// Gather the list of allowed named queries
Collection<NamedQueryDefinition> namedQueries =
this.hibernate.getConfigurationMetadata().getNamedQueryDefinitions();
for (NamedQueryDefinition definition : namedQueries) {
try {
if (isSafe(definition.getQuery())) {
this.safeNamedQueries.add(definition.getName());
if (this.queryValidator.isSafe(definition.getQuery())) {
safeQueries.add(definition.getName());
}
} catch (QueryException e) {
this.logger.warn("Failed to validate named query [{}] with statement [{}]: {}",
definition.getName(), definition.getQuery(), ExceptionUtils.getRootCauseMessage(e));
}
}

this.safeNamedQueries = safeQueries;
}
}
}
Expand All @@ -140,52 +144,20 @@ private Set<String> getSafeNamedQueries()
}

/**
* @param statementString the statement to evaluate
* @param statement the statement to evaluate
* @return true if the select is allowed for user without PR
* @deprecated
*/
@Deprecated(since = "17.0.0RC1, 16.10.2, 15.10.16, 16.4.6")
protected static boolean isSafeSelect(String statementString)
{
// An empty statement is safe
if (statementString.isEmpty()) {
return true;
}

// HqlQueryUtils#isSafe only works with complete statements
String completeStatement = toCompleteShortForm(statementString);

// Parse and validate the statement
return HqlQueryUtils.isSafe(completeStatement);
}

private boolean isSafe(String statement) throws QueryException
protected static boolean isSafeSelect(String statement)
{
// An empty statement is safe
if (statement.isEmpty()) {
return true;
}

// HqlQueryUtils#isSafe only works with complete statements
String completeStatement = toCompleteShortForm(statement);
HQLStatementValidator validator = Utils.getComponent(HQLStatementValidator.class);

// Parse and validate the statement
ComponentManager componentManager = this.componentManagerProvider.get();
try {
for (HQLQueryValidator validator : componentManager
.<HQLQueryValidator>getInstanceList(HQLQueryValidator.class)) {
Optional<Boolean> result = validator.isSafe(completeStatement);
if (result.isPresent()) {
return result.get();
}
}
} catch (ComponentLookupException e) {
throw new QueryException("Failed to get query statement validators",
new DefaultQuery(statement, Query.HQL, this), e);
return validator.isSafe(statement);
} catch (QueryException e) {
return false;
}

// Assume the statement is safe if no validator says otherwise
return true;
}

protected void checkAllowed(final Query query) throws QueryException
Expand All @@ -198,7 +170,7 @@ protected void checkAllowed(final Query query) throws QueryException
throw new QueryException("Named queries requires programming right", query, null);
}

if (!isSafe(query.getStatement())) {
if (!this.queryValidator.isSafe(query.getStatement())) {
throw new QueryException("The query requires programming right", query, null);
}
}
Expand Down Expand Up @@ -245,17 +217,13 @@ public <T> List<T> execute(final Query query) throws QueryException
protected Query filterQuery(Query query)
{
Query filteredQuery = query;

// Named queries are not filtered
if (!filteredQuery.isNamed()) {
// For non-named queries, convert the short form into long form before we apply the filters.
filteredQuery = new WrappingQuery(filteredQuery)
{
@Override
public String getStatement()
{
// handle short queries
return completeShortFormStatement(getWrappedQuery().getStatement());
}
};
// Make sure to work with the complete form of the query
filteredQuery = HqlQueryUtils.toCompleteQuery(filteredQuery);

// Execute filters
filteredQuery = filterQuery(filteredQuery, Query.HQL);
}

Expand Down Expand Up @@ -389,18 +357,7 @@ private boolean hasQueryParametersType(Query query)
*/
protected String completeShortFormStatement(String statement)
{
return toCompleteShortForm(statement);
}

private static String toCompleteShortForm(String statement)
{
String filteredStatement = statement;

if (statement.isEmpty() || HqlQueryUtils.isShortFormStatement(statement)) {
filteredStatement = "select doc.fullName from XWikiDocument doc " + statement.trim();
}

return filteredStatement;
return HqlQueryUtils.toCompleteStatement(statement);
}

private <T> org.hibernate.query.Query<T> createNamedHibernateQuery(Session session, Query query)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,21 @@
import java.util.Optional;
import java.util.regex.Pattern;

import javax.annotation.Priority;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;

import org.apache.commons.lang3.exception.ExceptionUtils;
import org.slf4j.Logger;
import org.xwiki.component.annotation.Component;
import org.xwiki.component.descriptor.ComponentDescriptor;
import org.xwiki.component.phase.Initializable;
import org.xwiki.component.phase.InitializationException;
import org.xwiki.configuration.ConfigurationSource;

/**
* A validator which can be configured.
* A validator which can be configured. This validator is executer before the default one using component priorities
*
* @version $Id$
* @since 17.0.0RC1
Expand All @@ -47,7 +49,8 @@
@Component
@Singleton
@Named("configuration")
public class ConfigurableHQLQueryValidator implements HQLQueryValidator, Initializable
@Priority(ComponentDescriptor.DEFAULT_PRIORITY - 100)
public class ConfigurableHQLCompleteStatementValidator implements HQLCompleteStatementValidator, Initializable
{
@Inject
@Named("xwikiproperties")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.query.hql.internal;

import java.util.Optional;

import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;

import org.apache.commons.lang3.StringUtils;
import org.xwiki.component.annotation.Component;
import org.xwiki.component.manager.ComponentLookupException;
import org.xwiki.component.manager.ComponentManager;
import org.xwiki.query.Query;
import org.xwiki.query.QueryException;
import org.xwiki.query.internal.DefaultQuery;

import com.xpn.xwiki.internal.store.hibernate.query.HqlQueryUtils;

/**
* The main entry point to validate a query.
*
* @version $Id$
* @since 17.0.0RC1
* @since 16.10.2
* @since 15.10.16
* @since 16.4.6
*/
@Component
@Singleton
public class DefaultHQLStatementValidator implements HQLStatementValidator
{
@Inject
@Named("context")
private Provider<ComponentManager> componentManagerProvider;

@Inject
@Named("standard")
private HQLCompleteStatementValidator standardValidator;

@Override
public boolean isSafe(String statement) throws QueryException
{
// An empty statement is safe
if (StringUtils.isEmpty(statement)) {
return true;
}

// Validators are expected to work with complete statements statements
String completeStatement = HqlQueryUtils.toCompleteStatement(statement);

// Parse and validate the statement
ComponentManager componentManager = this.componentManagerProvider.get();
try {
for (HQLCompleteStatementValidator validator : componentManager
.<HQLCompleteStatementValidator>getInstanceList(HQLCompleteStatementValidator.class)) {
Optional<Boolean> result = validator.isSafe(completeStatement);
if (result.isPresent()) {
return result.get();
}
}
} catch (ComponentLookupException e) {
throw new QueryException("Failed to get query statement validators",
new DefaultQuery(completeStatement, Query.HQL, null), e);
}

// Fallback on the standard validator (it's already supposed to be part of the found validators above, but
// something might cause it to be unregistered)
Optional<Boolean> result = this.standardValidator.isSafe(completeStatement);
if (result.isPresent()) {
return result.get();
}

// If we really could not find any way to validate the statement, consider it unsafe
return false;
}
}
Loading

0 comments on commit b4c568b

Please sign in to comment.