Repository: sentry Updated Branches: refs/heads/sentry-ha-redesign a79f923dc -> 5849ab0e5
SENTRY-1609: DelegateSentryStore is subject to JDQL injection (Alex Kolbasov, Review by: Vamsee Yarlagadda and kalyan kumar kalvagadda) The fix re-implements getGroupsByRoles() using SentryStore methods instead of directly issuing queries. Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/5849ab0e Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/5849ab0e Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/5849ab0e Branch: refs/heads/sentry-ha-redesign Commit: 5849ab0e530a8e3a250b7e58fde211842960288f Parents: a79f923 Author: Alexander Kolbasov <[email protected]> Authored: Wed Feb 8 12:29:52 2017 -0800 Committer: Alexander Kolbasov <[email protected]> Committed: Wed Feb 8 12:29:52 2017 -0800 ---------------------------------------------------------------------- .../service/persistent/DelegateSentryStore.java | 51 ++++++++------------ .../db/service/persistent/SentryStore.java | 2 +- 2 files changed, 22 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/5849ab0e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java index 23f0891..0c5d019 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java @@ -20,12 +20,10 @@ package org.apache.sentry.provider.db.generic.service.persistent; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Set; import javax.jdo.PersistenceManager; -import javax.jdo.Query; import org.apache.hadoop.conf.Configuration; import org.apache.sentry.core.common.exception.SentryUserException; @@ -45,7 +43,6 @@ import org.apache.sentry.provider.db.service.thrift.TSentryRole; import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; @@ -252,37 +249,31 @@ public class DelegateSentryStore implements SentryStoreLayer { @Override public Set<String> getGroupsByRoles(final String component, final Set<String> roles) throws Exception { + // In all calls roles contain exactly one group if (roles.isEmpty()) { return Collections.emptySet(); } - return delegate.getTransactionManager().executeTransaction( - new TransactionBlock<Set<String>>() { - public Set<String> execute(PersistenceManager pm) throws Exception { - //get groups by roles - final Set<String> trimmedRoles = SentryStore.toTrimedLower(roles); - - Query query = pm.newQuery(MSentryGroup.class); - StringBuilder filters = new StringBuilder(); - query.declareVariables("MSentryRole role"); - List<String> rolesFiler = new LinkedList<>(); - for (String role : trimmedRoles) { - rolesFiler.add("role.roleName == \"" + role + "\" "); - } - filters.append("roles.contains(role) " + "&& (" + Joiner.on(" || ").join(rolesFiler) + ")"); - query.setFilter(filters.toString()); - @SuppressWarnings("unchecked") - List<MSentryGroup> groups = (List<MSentryGroup>)query.execute(); - if (groups.isEmpty()) { - return Collections.emptySet(); - } - Set<String> groupNames = new HashSet<>(groups.size()); - for (MSentryGroup group : groups) { - groupNames.add(group.getGroupName()); - } - return groupNames; - } - }); + // Collect resulting group names in a set + Set<String> groupNames = new HashSet<>(); + for (String role : roles) { + MSentryRole sentryRole = null; + try { + sentryRole = delegate.getMSentryRoleByName(role); + } + catch (SentryNoSuchObjectException e) { + // Role disappeared - not a big deal, just ognore it + continue; + } + // Collect all group names for this role. + // Since we use a set, a group can appear multiple times and will only + // show up once in a set + for (MSentryGroup group : sentryRole.getGroups()) { + groupNames.add(group.getGroupName()); + } + } + + return groupNames; } @Override http://git-wip-us.apache.org/repos/asf/sentry/blob/5849ab0e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index bfc6da5..4cc123f 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -1197,7 +1197,7 @@ public class SentryStore { } @VisibleForTesting - MSentryRole getMSentryRoleByName(final String roleName) throws Exception { + public MSentryRole getMSentryRoleByName(final String roleName) throws Exception { return tm.executeTransaction( new TransactionBlock<MSentryRole>() { public MSentryRole execute(PersistenceManager pm) throws Exception {
