Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPA: Implement row level filtering and column masking #20921

Merged
merged 5 commits into from
Mar 6, 2024
Merged

OPA: Implement row level filtering and column masking #20921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
610b160
Implement row level filtering and masking and add system tests
vagaerg Dec 18, 2023
7eb96fd
Code review: fix formatting and change config builder
vagaerg Mar 5, 2024
50cc016
Code review: remove unnecessary NotNull annotations
vagaerg Mar 5, 2024
532e4b6
Code review: use StandaloneQueryRunner
vagaerg Mar 5, 2024
be8b8bc
Add Language annotations to tests
vagaerg Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions plugin/trino-opa/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.trino</groupId>
<artifactId>trino-main</artifactId>
<type>test-jar</type>
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.trino</groupId>
<artifactId>trino-testing</artifactId>
Expand All @@ -132,6 +139,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jetbrains</groupId>
<artifactId>annotations</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
Expand Down
22 changes: 22 additions & 0 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.trino.plugin.opa.schema.OpaQueryInput;
import io.trino.plugin.opa.schema.OpaQueryInputAction;
import io.trino.plugin.opa.schema.OpaQueryInputResource;
import io.trino.plugin.opa.schema.OpaViewExpression;
import io.trino.plugin.opa.schema.TrinoCatalogSessionProperty;
import io.trino.plugin.opa.schema.TrinoFunction;
import io.trino.plugin.opa.schema.TrinoGrantPrincipal;
Expand All @@ -40,15 +41,19 @@
import io.trino.spi.security.SystemAccessControl;
import io.trino.spi.security.SystemSecurityContext;
import io.trino.spi.security.TrinoPrincipal;
import io.trino.spi.security.ViewExpression;
import io.trino.spi.type.Type;

import java.security.Principal;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Consumer;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.Iterables.getOnlyElement;
import static io.trino.plugin.opa.OpaHighLevelClient.buildQueryInputForSimpleResource;
Expand Down Expand Up @@ -709,6 +714,23 @@ public void checkCanDropFunction(SystemSecurityContext systemSecurityContext, Ca
OpaQueryInputResource.builder().function(TrinoFunction.fromTrinoFunction(functionName)).build());
}

@Override
public List<ViewExpression> getRowFilters(SystemSecurityContext context, CatalogSchemaTableName tableName)
{
List<OpaViewExpression> rowFilterExpressions = opaHighLevelClient.getRowFilterExpressionsFromOpa(buildQueryContext(context), tableName);
return rowFilterExpressions.stream()
.map(expression -> expression.toTrinoViewExpression(tableName.getCatalogName(), tableName.getSchemaTableName().getSchemaName()))
.collect(toImmutableList());
}

@Override
public Optional<ViewExpression> getColumnMask(SystemSecurityContext context, CatalogSchemaTableName tableName, String columnName, Type type)
{
return opaHighLevelClient
.getColumnMaskFromOpa(buildQueryContext(context), tableName, columnName, type)
.map(expression -> expression.toTrinoViewExpression(tableName.getCatalogName(), tableName.getSchemaTableName().getSchemaName()));
}

private void checkTableOperation(SystemSecurityContext context, String actionName, CatalogSchemaTableName table, Consumer<String> deny)
{
opaHighLevelClient.queryAndEnforce(
Expand Down
4 changes: 4 additions & 0 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControlFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
import io.airlift.concurrent.BoundedExecutor;
import io.airlift.http.client.HttpClient;
import io.airlift.json.JsonModule;
import io.trino.plugin.opa.schema.OpaColumnMaskQueryResult;
import io.trino.plugin.opa.schema.OpaPluginContext;
import io.trino.plugin.opa.schema.OpaQuery;
import io.trino.plugin.opa.schema.OpaQueryResult;
import io.trino.plugin.opa.schema.OpaRowFiltersQueryResult;
import io.trino.spi.security.SystemAccessControl;
import io.trino.spi.security.SystemAccessControlFactory;

Expand Down Expand Up @@ -71,6 +73,8 @@ protected static SystemAccessControl create(Map<String, String> config, Optional
binder -> {
jsonCodecBinder(binder).bindJsonCodec(OpaQuery.class);
jsonCodecBinder(binder).bindJsonCodec(OpaQueryResult.class);
jsonCodecBinder(binder).bindJsonCodec(OpaRowFiltersQueryResult.class);
jsonCodecBinder(binder).bindJsonCodec(OpaColumnMaskQueryResult.class);
httpClient.ifPresentOrElse(
client -> binder.bind(Key.get(HttpClient.class, ForOpa.class)).toInstance(client),
() -> httpClientBinder(binder).bindHttpClient("opa", ForOpa.class));
Expand Down
31 changes: 31 additions & 0 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class OpaConfig
private boolean logRequests;
private boolean logResponses;
private boolean allowPermissionManagementOperations;
private Optional<URI> opaRowFiltersUri = Optional.empty();
private Optional<URI> opaColumnMaskingUri = Optional.empty();
Comment on lines +31 to +32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, it would be nice if the OPA server had a configration endpoint (like Oauth), which told us these URI along with the batch URI. We could then either call this once during setup, or periodically in the background.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great idea, we were playing with a similar concept. The one thing I'm unsure about is allowing a configuration endpoint to determine whether row filtering is applied at all, for instance - as that feels more like a configuration responsibility. Kind of like if the OAuth config endpoint allowed you to disable auth altogether.

Not opposed to it, just some thoughts.

An alternative:

What's interesting is that we don't technically need these to be different URIs, they could just be different keys from the response and we could do everything OPA-side without including another type of request.

Suppose we had:

  • opa.policy.uri = https://opa/v1/data/my_policy/allow
  • opa.policy.batched-uri = https://opa/v1/data/my_policy/batchAllow
  • opa.policy.row-filters-uri = https://opa/v1/data/my_policy/rowFilters
  • opa.policy.column-masking-uri = https://opa/v1/data/my_policy/columnMask

This is equivalent to querying https://opa/v1/data/my_policy for all possible types of request and pulling the relevant field from the response (allow, batchAllow, rowFilters or columnMask).

The OPA policy itself can even directly determine what result to provide so that Trino doesn't need to pop fields from the response manually. This is because the request unequivocally determines what Trino is looking for:

  • If operation is GetRowFilters: the relevant result is the row filters decision
  • If operation is GetColumnMask: the relevant result is the column mask decision
  • If action.filterResources is present: the relevant result is the batch filtering decision
  • Otherwise, the relevant result is the standard boolean allow decision

Some care would need to be exercised for performance (but we can provide a small rego wrapper to deal with this)

The config would then become:

  • opa.policy.unified-uri (or whatever we call this new "catch-all" URI)
  • opa.policy.enable-row-filtering
  • opa.policy.enable-column-masking

The last two would allow us to disable row filtering and column masking from the config in case there's performance concerns

Maybe we could take this into an issue? CC @sbernauer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds interesting. Let's discuss on slack


@NotNull
public URI getOpaUri()
Expand All @@ -43,6 +45,7 @@ public OpaConfig setOpaUri(@NotNull URI opaUri)
return this;
}

@NotNull
public Optional<URI> getOpaBatchUri()
{
return opaBatchUri;
Expand Down Expand Up @@ -94,4 +97,32 @@ public OpaConfig setAllowPermissionManagementOperations(boolean allowPermissionM
this.allowPermissionManagementOperations = allowPermissionManagementOperations;
return this;
}

@NotNull
public Optional<URI> getOpaRowFiltersUri()
{
return opaRowFiltersUri;
}

@Config("opa.policy.row-filters-uri")
@ConfigDescription("URI for fetching row filters - if not set no row filtering will be applied")
public OpaConfig setOpaRowFiltersUri(@NotNull URI opaRowFiltersUri)
{
this.opaRowFiltersUri = Optional.ofNullable(opaRowFiltersUri);
return this;
}

@NotNull
public Optional<URI> getOpaColumnMaskingUri()
{
return opaColumnMaskingUri;
}

@Config("opa.policy.column-masking-uri")
@ConfigDescription("URI for fetching column masks - if not set no masking will be applied")
public OpaConfig setOpaColumnMaskingUri(URI opaColumnMaskingUri)
{
this.opaColumnMaskingUri = Optional.ofNullable(opaColumnMaskingUri);
return this;
}
}
47 changes: 46 additions & 1 deletion plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaHighLevelClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,27 @@
*/
package io.trino.plugin.opa;

import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
import io.airlift.json.JsonCodec;
import io.trino.plugin.opa.schema.OpaColumnMaskQueryResult;
import io.trino.plugin.opa.schema.OpaQueryContext;
import io.trino.plugin.opa.schema.OpaQueryInput;
import io.trino.plugin.opa.schema.OpaQueryInputAction;
import io.trino.plugin.opa.schema.OpaQueryInputResource;
import io.trino.plugin.opa.schema.OpaQueryResult;
import io.trino.plugin.opa.schema.OpaRowFiltersQueryResult;
import io.trino.plugin.opa.schema.OpaViewExpression;
import io.trino.plugin.opa.schema.TrinoColumn;
import io.trino.plugin.opa.schema.TrinoTable;
import io.trino.spi.connector.CatalogSchemaTableName;
import io.trino.spi.security.AccessDeniedException;
import io.trino.spi.type.Type;

import java.net.URI;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;

Expand All @@ -32,18 +42,28 @@
public class OpaHighLevelClient
{
private final JsonCodec<OpaQueryResult> queryResultCodec;
private final URI opaPolicyUri;
private final JsonCodec<OpaRowFiltersQueryResult> rowFiltersQueryResultCodec;
private final JsonCodec<OpaColumnMaskQueryResult> columnMaskQueryResultCodec;
private final OpaHttpClient opaHttpClient;
private final URI opaPolicyUri;
private final Optional<URI> opaRowFiltersUri;
private final Optional<URI> opaColumnMaskingUri;

@Inject
public OpaHighLevelClient(
JsonCodec<OpaQueryResult> queryResultCodec,
JsonCodec<OpaRowFiltersQueryResult> rowFiltersQueryResultCodec,
JsonCodec<OpaColumnMaskQueryResult> columnMaskQueryResultCodec,
OpaHttpClient opaHttpClient,
OpaConfig config)
{
this.queryResultCodec = requireNonNull(queryResultCodec, "queryResultCodec is null");
this.rowFiltersQueryResultCodec = requireNonNull(rowFiltersQueryResultCodec, "rowFiltersQueryResultCodec is null");
this.columnMaskQueryResultCodec = requireNonNull(columnMaskQueryResultCodec, "columnMaskQueryResultCodec is null");
this.opaHttpClient = requireNonNull(opaHttpClient, "opaHttpClient is null");
this.opaPolicyUri = config.getOpaUri();
this.opaRowFiltersUri = config.getOpaRowFiltersUri();
this.opaColumnMaskingUri = config.getOpaColumnMaskingUri();
}

public boolean queryOpa(OpaQueryInput input)
Expand Down Expand Up @@ -105,6 +125,31 @@ public <T> Set<T> parallelFilterFromOpa(
return opaHttpClient.parallelFilterFromOpa(items, requestBuilder, opaPolicyUri, queryResultCodec);
}

public List<OpaViewExpression> getRowFilterExpressionsFromOpa(OpaQueryContext context, CatalogSchemaTableName table)
{
OpaQueryInput queryInput = new OpaQueryInput(
context,
OpaQueryInputAction.builder()
.operation("GetRowFilters")
.resource(OpaQueryInputResource.builder().table(new TrinoTable(table)).build())
.build());
return opaRowFiltersUri
.map(uri -> opaHttpClient.consumeOpaResponse(opaHttpClient.submitOpaRequest(queryInput, uri, rowFiltersQueryResultCodec)).result())
.orElse(ImmutableList.of());
}

public Optional<OpaViewExpression> getColumnMaskFromOpa(OpaQueryContext context, CatalogSchemaTableName table, String columnName, Type type)
{
OpaQueryInput queryInput = new OpaQueryInput(
context,
OpaQueryInputAction.builder()
.operation("GetColumnMask")
.resource(OpaQueryInputResource.builder().column(new TrinoColumn(table, columnName, type)).build())
.build());
return opaColumnMaskingUri
.flatMap(uri -> opaHttpClient.consumeOpaResponse(opaHttpClient.submitOpaRequest(queryInput, uri, columnMaskQueryResultCodec)).result());
}

public static OpaQueryInput buildQueryInputForSimpleResource(OpaQueryContext context, String operation, OpaQueryInputResource resource)
{
return new OpaQueryInput(context, OpaQueryInputAction.builder().operation(operation).resource(resource).build());
Expand Down
3 changes: 1 addition & 2 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaBatchQueryResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableList;
import jakarta.validation.constraints.NotNull;

import java.util.List;

import static java.util.Objects.requireNonNullElse;

public record OpaBatchQueryResult(@JsonProperty("decision_id") String decisionId, @NotNull List<Integer> result)
public record OpaBatchQueryResult(@JsonProperty("decision_id") String decisionId, List<Integer> result)
{
public OpaBatchQueryResult
{
Expand Down
28 changes: 28 additions & 0 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaColumnMaskQueryResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.trino.plugin.opa.schema;

import com.fasterxml.jackson.annotation.JsonProperty;

import java.util.Optional;

import static java.util.Objects.requireNonNull;

public record OpaColumnMaskQueryResult(@JsonProperty("decision_id") String decisionId, Optional<OpaViewExpression> result)
{
public OpaColumnMaskQueryResult
{
requireNonNull(result, "result is null");
}
}
3 changes: 1 addition & 2 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaQueryInputAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import com.fasterxml.jackson.annotation.JsonInclude;
import com.google.common.collect.ImmutableList;
import jakarta.validation.constraints.NotNull;

import java.util.Collection;
import java.util.List;
Expand All @@ -25,7 +24,7 @@

@JsonInclude(NON_NULL)
public record OpaQueryInputAction(
@NotNull String operation,
String operation,
OpaQueryInputResource resource,
List<OpaQueryInputResource> filterResources,
OpaQueryInputResource targetResource,
Expand Down
16 changes: 12 additions & 4 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaQueryInputResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package io.trino.plugin.opa.schema;

import com.fasterxml.jackson.annotation.JsonInclude;
import jakarta.validation.constraints.NotNull;

import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
import static java.util.Objects.requireNonNull;
Expand All @@ -27,9 +26,10 @@ public record OpaQueryInputResource(
TrinoFunction function,
NamedEntity catalog,
TrinoSchema schema,
TrinoTable table)
TrinoTable table,
TrinoColumn column)
{
public record NamedEntity(@NotNull String name)
public record NamedEntity(String name)
{
public NamedEntity
{
Expand All @@ -51,6 +51,7 @@ public static class Builder
private TrinoSchema schema;
private TrinoTable table;
private TrinoFunction function;
private TrinoColumn column;

private Builder() {}

Expand Down Expand Up @@ -102,6 +103,12 @@ public Builder function(String functionName)
return this;
}

public Builder column(TrinoColumn column)
{
this.column = column;
return this;
}

public OpaQueryInputResource build()
{
return new OpaQueryInputResource(
Expand All @@ -111,7 +118,8 @@ public OpaQueryInputResource build()
this.function,
this.catalog,
this.schema,
this.table);
this.table,
this.column);
}
}
}
29 changes: 29 additions & 0 deletions plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaRowFiltersQueryResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.trino.plugin.opa.schema;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableList;

import java.util.List;

import static java.util.Objects.requireNonNullElse;

public record OpaRowFiltersQueryResult(@JsonProperty("decision_id") String decisionId, List<OpaViewExpression> result)
{
public OpaRowFiltersQueryResult
{
result = ImmutableList.copyOf(requireNonNullElse(result, ImmutableList.of()));
}
}
Loading