Skip to content

Commit

Permalink
policy change
Browse files Browse the repository at this point in the history
  • Loading branch information
cecemei committed Dec 18, 2024
1 parent dd251c2 commit b19728b
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 22 deletions.
26 changes: 26 additions & 0 deletions server/src/main/java/org/apache/druid/server/Policy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.apache.druid.server;

import org.apache.druid.query.filter.DimFilter;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Optional;

public class Policy
{
public static final Policy NO_RESTRICTION = new Policy(null);

private final Optional<DimFilter> rowFilter;

public Policy(@Nullable DimFilter rowFilter) {
this.rowFilter = Optional.ofNullable(rowFilter);
}

public static Policy fromRowFilter(@Nonnull DimFilter rowFilter) {
return new Policy(rowFilter);
}

public Optional<DimFilter> getRowFilter() {
return rowFilter;
}
}
21 changes: 11 additions & 10 deletions server/src/main/java/org/apache/druid/server/security/Access.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.base.Strings;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.query.filter.DimFilter;
import org.apache.druid.server.Policy;

import javax.annotation.Nullable;
import java.util.Objects;
Expand All @@ -39,18 +40,18 @@ public class Access
private final String message;
// A row-level policy filter on top of table-level read access. It should be empty if there are no policy restrictions
// or if access is requested for an action other than reading the table.
private final Optional<DimFilter> rowFilter;
private final Policy rowFilter;

/**
* @deprecated use {@link #allow()} or {@link #deny(String)} instead
*/
@Deprecated
public Access(boolean allowed)
{
this(allowed, "", Optional.empty());
this(allowed, "", Policy.NO_RESTRICTION);
}

Access(boolean allowed, String message, Optional<DimFilter> rowFilter)
Access(boolean allowed, String message, Policy rowFilter)
{
this.allowed = allowed;
this.message = message;
Expand All @@ -59,25 +60,25 @@ public Access(boolean allowed)

public static Access allow()
{
return new Access(true, "", Optional.empty());
return new Access(true, "", Policy.NO_RESTRICTION);
}

public static Access deny(@Nullable String message)
{
return new Access(false, Objects.isNull(message) ? "" : message, Optional.empty());
return new Access(false, Objects.isNull(message) ? "" : message, null);
}

public static Access allowWithRestriction(Optional<DimFilter> rowFilter)
public static Access allowWithRestriction(Policy policy)
{
return new Access(true, "", rowFilter);
return new Access(true, "", policy);
}

public boolean isAllowed()
{
return allowed;
}

public Optional<DimFilter> getRowFilter()
public Policy getPolicy()
{
return rowFilter;
}
Expand All @@ -90,9 +91,9 @@ public String getMessage()
stringBuilder.append(", ");
stringBuilder.append(message);
}
if (allowed && rowFilter.isPresent()) {
if (allowed && rowFilter.getRowFilter().isPresent()) {
stringBuilder.append(", with restriction ");
stringBuilder.append(rowFilter.get());
stringBuilder.append(rowFilter.getRowFilter().get());
}
return stringBuilder.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import com.google.common.collect.ImmutableMap;
import org.apache.druid.query.filter.DimFilter;
import org.apache.druid.query.filter.TrueDimFilter;
import org.apache.druid.server.Policy;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -61,7 +63,7 @@ public class AuthorizationResult
@Nullable
private final String failureMessage;

private final Map<String, Optional<DimFilter>> policyFilters;
private final Map<String, Policy> policyFilters;

@Nullable
private final Set<ResourceAction> sqlResourceActions;
Expand All @@ -72,7 +74,7 @@ public class AuthorizationResult
AuthorizationResult(
boolean isAllowed,
@Nullable String failureMessage,
Map<String, Optional<DimFilter>> policyFilters,
Map<String, Policy> policyFilters,
@Nullable Set<ResourceAction> sqlResourceActions,
@Nullable Set<ResourceAction> allResourceActions
)
Expand All @@ -89,7 +91,7 @@ public static AuthorizationResult deny(@Nonnull String failureMessage)
return new AuthorizationResult(false, failureMessage, Collections.emptyMap(), null, null);
}

public static AuthorizationResult allowWithRestriction(Map<String, Optional<DimFilter>> policyFilters)
public static AuthorizationResult allowWithRestriction(Map<String, Policy> policyFilters)
{
return new AuthorizationResult(true, null, policyFilters, null, null);
}
Expand All @@ -102,7 +104,7 @@ public AuthorizationResult withResourceActions(
return new AuthorizationResult(
isAllowed,
failureMessage,
ImmutableMap.copyOf(getPolicyFilters()),
ImmutableMap.copyOf(getPolicy()),
sqlResourceActions,
allResourceActions
);
Expand All @@ -115,7 +117,7 @@ public Optional<String> getPermissionErrorMessage(boolean policyFilterNotPermitt
}

if (policyFilterNotPermitted && policyFilters.values()
.stream()
.stream().map(Policy::getRowFilter)
.flatMap(Optional::stream)
.anyMatch(filter -> !(filter instanceof TrueDimFilter))) {
return Optional.of(Access.DEFAULT_ERROR_MESSAGE);
Expand All @@ -124,11 +126,20 @@ public Optional<String> getPermissionErrorMessage(boolean policyFilterNotPermitt
return Optional.empty();
}

public Map<String, Optional<DimFilter>> getPolicyFilters()
public Map<String, Policy> getPolicy()
{
return policyFilters;
}

public Map<String, Optional<DimFilter>> getPolicyFilters()
{
Map<String, Optional<DimFilter>> filters = new HashMap<>();
for (Map.Entry<String, Policy> entry : policyFilters.entrySet()) {
filters.put(entry.getKey(), entry.getValue().getRowFilter());
}
return filters;
}

@Nullable
public Set<ResourceAction> getSqlResourceActions()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.query.filter.DimFilter;
import org.apache.druid.server.Policy;

import javax.servlet.http.HttpServletRequest;
import java.util.ArrayList;
Expand Down Expand Up @@ -168,7 +169,7 @@ public static AuthorizationResult authorizeAllResourceActions(

// this method returns on first failure, so only successful Access results are kept in the cache
final Set<ResourceAction> resultCache = new HashSet<>();
final Map<String, Optional<DimFilter>> policyFilters = new HashMap<>();
final Map<String, Policy> policyFilters = new HashMap<>();

for (ResourceAction resourceAction : resourceActions) {
if (resultCache.contains(resourceAction)) {
Expand All @@ -185,8 +186,8 @@ public static AuthorizationResult authorizeAllResourceActions(
resultCache.add(resourceAction);
if (resourceAction.getAction().equals(Action.READ)
&& RESTRICTION_APPLICABLE_RESOURCE_TYPES.contains(resourceAction.getResource().getType())) {
policyFilters.put(resourceAction.getResource().getName(), access.getRowFilter());
} else if (access.getRowFilter().isPresent()) {
policyFilters.put(resourceAction.getResource().getName(), access.getPolicy());
} else if (access.getPolicy().getRowFilter().isPresent()) {
throw DruidException.defensive(
"Row policy should only present when reading a table, but was present for %s",
resourceAction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public void testAuthorizedWithNoPolicyRestriction()
new Resource(DATASOURCE, ResourceType.DATASOURCE),
Action.READ
))
.andReturn(Access.allowWithRestriction(Optional.empty())).once();
.andReturn(Access.allowWithRestriction(Policy.NO_RESTRICTION)).once();
EasyMock.expect(conglomerate.getToolChest(EasyMock.anyObject()))
.andReturn(toolChest).times(2);
EasyMock.expect(texasRanger.getQueryRunnerForIntervals(
Expand Down Expand Up @@ -243,7 +243,7 @@ public void testAuthorizedWithAlwaysTruePolicyRestriction()
new Resource(DATASOURCE, ResourceType.DATASOURCE),
Action.READ
))
.andReturn(Access.allowWithRestriction(alwaysTrueFilter)).once();
.andReturn(Access.allowWithRestriction(Policy.fromRowFilter(alwaysTrueFilter.get()))).once();
EasyMock.expect(conglomerate.getToolChest(EasyMock.anyObject()))
.andReturn(toolChest).times(2);
EasyMock.expect(texasRanger.getQueryRunnerForIntervals(queryMatchDataSource(RestrictedDataSource.create(
Expand Down Expand Up @@ -293,7 +293,7 @@ public void testAuthorizedWithOnePolicyRestriction()
new Resource(DATASOURCE, ResourceType.DATASOURCE),
Action.READ
))
.andReturn(Access.allowWithRestriction(rowFilter)).times(1);
.andReturn(Access.allowWithRestriction(Policy.fromRowFilter(rowFilter.get()))).times(1);
EasyMock.expect(conglomerate.getToolChest(EasyMock.anyObject()))
.andReturn(toolChest).times(2);
EasyMock.expect(texasRanger.getQueryRunnerForIntervals(queryMatchDataSource(RestrictedDataSource.create(
Expand Down

0 comments on commit b19728b

Please sign in to comment.