diff --git a/docs/src/main/sphinx/security/opa-access-control.md b/docs/src/main/sphinx/security/opa-access-control.md index 5df693dca5b3..6e284227ac87 100644 --- a/docs/src/main/sphinx/security/opa-access-control.md +++ b/docs/src/main/sphinx/security/opa-access-control.md @@ -65,6 +65,10 @@ The following table lists the configuration properties for the OPA access contro - Optional HTTP client configurations for the connection from Trino to OPA, for example `opa.http-client.http-proxy` for configuring the HTTP proxy. Find more details in [](/admin/properties-http-client). +* - `opa.context-file` + - Optional properties file, containing user defined properties + (e.g. tenant namespace, tier or cluster) to be included in + the OPA query context. ::: ### Logging diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java index 046a812c2c8c..b4986ed14916 100644 --- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java +++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java @@ -802,11 +802,11 @@ private static Map> convertProperties(Map opaRowFiltersUri = Optional.empty(); private Optional opaColumnMaskingUri = Optional.empty(); private Optional opaBatchColumnMaskingUri = Optional.empty(); + private Optional additionalContextFile = Optional.empty(); @NotNull public URI getOpaUri() @@ -140,4 +143,17 @@ public OpaConfig setOpaBatchColumnMaskingUri(URI opaBatchColumnMaskingUri) this.opaBatchColumnMaskingUri = Optional.ofNullable(opaBatchColumnMaskingUri); return this; } + + public Optional<@FileExists Path> getAdditionalContextFile() + { + return additionalContextFile; + } + + @Config("opa.context-file") + @ConfigDescription("Optional properties file, containing user defined properties (e.g. tenant namespace, tier or cluster) to be included in the OPA query context") + public OpaConfig setAdditionalContextFile(Path additionalContextFile) + { + this.additionalContextFile = Optional.ofNullable(additionalContextFile); + return this; + } } diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaHighLevelClient.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaHighLevelClient.java index 2d50d65fd331..edca47bc52f2 100644 --- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaHighLevelClient.java +++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaHighLevelClient.java @@ -32,16 +32,22 @@ import io.trino.spi.connector.ColumnSchema; import io.trino.spi.security.AccessDeniedException; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; import java.net.URI; +import java.nio.file.Path; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Properties; import java.util.Set; import java.util.function.Function; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static java.nio.file.Files.newInputStream; import static java.util.Objects.requireNonNull; public class OpaHighLevelClient @@ -55,6 +61,7 @@ public class OpaHighLevelClient private final Optional opaRowFiltersUri; private final Optional opaColumnMaskingUri; private final Optional opaBatchColumnMaskingUri; + private final ImmutableMap opaAdditionalContext; @Inject public OpaHighLevelClient( @@ -74,6 +81,7 @@ public OpaHighLevelClient( this.opaRowFiltersUri = config.getOpaRowFiltersUri(); this.opaColumnMaskingUri = config.getOpaColumnMaskingUri(); this.opaBatchColumnMaskingUri = config.getOpaBatchColumnMaskingUri(); + this.opaAdditionalContext = ImmutableMap.copyOf(loadAdditionalContextFromFile(config.getAdditionalContextFile())); } public boolean queryOpa(OpaQueryInput input) @@ -155,6 +163,11 @@ public Map getColumnMasksFromOpa(OpaQueryContex .orElse(ImmutableMap.of()); } + public Map getAdditionalContext() + { + return opaAdditionalContext; + } + public static OpaQueryInput buildQueryInputForSimpleResource(OpaQueryContext context, String operation, OpaQueryInputResource resource) { return new OpaQueryInput(context, OpaQueryInputAction.builder().operation(operation).resource(resource).build()); @@ -190,4 +203,23 @@ private static OpaQueryInput buildQueryInputForSimpleAction(OpaQueryContext cont { return new OpaQueryInput(context, OpaQueryInputAction.builder().operation(operation).build()); } + + private static Map loadAdditionalContextFromFile(Optional additionalContextFile) + { + if (additionalContextFile.isEmpty()) { + return ImmutableMap.of(); + } + + try (InputStream inputStream = newInputStream(additionalContextFile.get())) { + Properties properties = new Properties(); + properties.load(inputStream); + return properties.entrySet().stream() + .collect(toImmutableMap( + entry -> String.valueOf(entry.getKey()), + entry -> String.valueOf(entry.getValue()))); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } } diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaQueryContext.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaQueryContext.java index 66fa7304af4a..01269f42693a 100644 --- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaQueryContext.java +++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/OpaQueryContext.java @@ -13,18 +13,21 @@ */ package io.trino.plugin.opa.schema; +import com.google.common.collect.ImmutableMap; import io.trino.spi.QueryId; +import java.util.Map; import java.util.Optional; import static java.util.Objects.requireNonNull; -public record OpaQueryContext(TrinoIdentity identity, OpaPluginContext softwareStack, Optional queryId) +public record OpaQueryContext(TrinoIdentity identity, OpaPluginContext softwareStack, Map properties, Optional queryId) { public OpaQueryContext { requireNonNull(identity, "identity is null"); requireNonNull(softwareStack, "softwareStack is null"); + properties = ImmutableMap.copyOf(properties); requireNonNull(queryId, "queryId is null"); } } diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java index b06c04e024f2..8e613e953e34 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java @@ -23,6 +23,8 @@ import io.trino.spi.security.SystemSecurityContext; import java.net.URI; +import java.nio.file.Path; +import java.nio.file.Paths; import java.time.Instant; public final class TestConstants @@ -58,6 +60,7 @@ private TestConstants() {} public static final URI OPA_SERVER_BATCH_URI = URI.create("http://my-batch-uri/"); public static final URI OPA_ROW_FILTERING_URI = URI.create("http://my-row-filtering-uri/"); public static final URI OPA_COLUMN_MASKING_URI = URI.create("http://my-column-masking-uri/"); + public static final Path OPA_ADDITIONAL_CONTEXT_FILE = Paths.get("src/test/resources/additional-context.properties"); public static final Identity TEST_IDENTITY = Identity.forUser("source-user").withGroups(ImmutableSet.of("some-group")).build(); public static final QueryId TEST_QUERY_ID = QueryId.valueOf("abcde"); public static final SystemSecurityContext TEST_SECURITY_CONTEXT = new SystemSecurityContext(TEST_IDENTITY, new QueryIdGenerator().createNextQueryId(), Instant.now()); diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestHelpers.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestHelpers.java index 8b4d36a9ff28..c96cd2076f23 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestHelpers.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestHelpers.java @@ -92,6 +92,7 @@ public static Map opaConfigToDict(OpaConfig config) config.getOpaRowFiltersUri().ifPresent(rowFiltersUri -> configBuilder.put("opa.policy.row-filters-uri", rowFiltersUri.toString())); config.getOpaColumnMaskingUri().ifPresent(columnMaskingUri -> configBuilder.put("opa.policy.column-masking-uri", columnMaskingUri.toString())); config.getOpaBatchColumnMaskingUri().ifPresent(batchColumnMaskingUri -> configBuilder.put("opa.policy.batch-column-masking-uri", batchColumnMaskingUri.toString())); + config.getAdditionalContextFile().ifPresent(additionalContextFile -> configBuilder.put("opa.context-file", additionalContextFile.toString())); return configBuilder.buildOrThrow(); } diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java index ab8111c45ab9..48f27205b780 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java @@ -36,6 +36,7 @@ import io.trino.spi.security.TrinoPrincipal; import io.trino.spi.security.ViewExpression; import io.trino.spi.type.VarcharType; +import org.intellij.lang.annotations.Language; import org.junit.jupiter.api.Test; import java.time.Instant; @@ -57,6 +58,7 @@ import static io.trino.plugin.opa.TestConstants.MALFORMED_RESPONSE; import static io.trino.plugin.opa.TestConstants.NO_ACCESS_RESPONSE; import static io.trino.plugin.opa.TestConstants.OK_RESPONSE; +import static io.trino.plugin.opa.TestConstants.OPA_ADDITIONAL_CONTEXT_FILE; import static io.trino.plugin.opa.TestConstants.OPA_COLUMN_MASKING_URI; import static io.trino.plugin.opa.TestConstants.OPA_ROW_FILTERING_URI; import static io.trino.plugin.opa.TestConstants.OPA_SERVER_URI; @@ -682,6 +684,8 @@ private void testRequestContextContentsForGivenTrinoVersion(Optional OK_RESPONSE); + OpaAccessControl authorizer = (OpaAccessControl) OpaAccessControlFactory.create( + ImmutableMap.of("opa.policy.uri", OPA_SERVER_URI.toString(), "opa.context-file", OPA_ADDITIONAL_CONTEXT_FILE.toString()), + Optional.of(mockClient), + Optional.empty()); + Identity sampleIdentityWithGroups = Identity.forUser("test_user").withGroups(ImmutableSet.of("some_group")).build(); + + authorizer.checkCanExecuteQuery(sampleIdentityWithGroups, TEST_QUERY_ID); + + @Language("JSON") + String expectedRequest = + """ + { + "action": { + "operation": "ExecuteQuery" + }, + "context": { + "identity": { + "user": "test_user", + "groups": ["some_group"] + }, + "softwareStack": { + "trinoVersion": "UNKNOWN" + }, + "properties" : { + "namespace" : "some-namespace", + "cluster" : "some-cluster" + } + } + } + """; + assertStringRequestsEqual(ImmutableSet.of(expectedRequest), mockClient.getRequests(), "/input"); + } + @Test void testGetRowFiltersThrowsForIllegalResponse() { diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlAdditionalContextSystem.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlAdditionalContextSystem.java new file mode 100644 index 000000000000..5e8e56739bb4 --- /dev/null +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlAdditionalContextSystem.java @@ -0,0 +1,189 @@ +/* + * 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; + +import io.trino.plugin.blackhole.BlackHolePlugin; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; + +@Testcontainers +@TestInstance(PER_CLASS) +final class TestOpaAccessControlAdditionalContextSystem +{ + private static final String OPA_ALLOW_POLICY_NAME = "allow"; + private static final String OPA_BATCH_ALLOW_POLICY_NAME = "batchAllow"; + private static final Path OPA_ADDITIONAL_CONTEXT_FILE = Paths.get("src/test/resources/additional-context.properties"); + @Container + private static final OpaContainer OPA_CONTAINER = new OpaContainer(); + + @Test + void testAllowsQueryAndFilters() + throws Exception + { + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig() + .setAdditionalContextFile(OPA_ADDITIONAL_CONTEXT_FILE) + .setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME))); + + OPA_CONTAINER.submitPolicy( + """ + package trino + import future.keywords.if + import future.keywords.in + + default allow = false + allow if is_admin + allow { + is_bob + is_some_namespace + is_some_cluster + can_be_accessed_by_bob + } + + is_admin { + input.context.identity.user == "admin" + } + is_bob { + input.context.identity.user == "bob" + } + is_some_namespace { + input.context.properties.namespace == "some-namespace" + } + is_some_cluster { + input.context.properties.cluster == "some-cluster" + } + can_be_accessed_by_bob { + input.action.operation in ["ImpersonateUser", "ExecuteQuery"] + } + can_be_accessed_by_bob { + input.action.operation in ["FilterCatalogs", "AccessCatalog"] + input.action.resource.catalog.name == "catalog_one" + } + """); + Set catalogsForBob = runner.querySetOfStrings("bob", "SHOW CATALOGS"); + assertThat(catalogsForBob).containsExactlyInAnyOrder("catalog_one"); + Set catalogsForAdmin = runner.querySetOfStrings("admin", "SHOW CATALOGS"); + assertThat(catalogsForAdmin).containsExactlyInAnyOrder("catalog_one", "catalog_two", "system"); + runner.teardown(); + } + + @Test + void testShouldDenyQueryIfDirected() + throws Exception + { + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig() + .setAdditionalContextFile(OPA_ADDITIONAL_CONTEXT_FILE) + .setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME))); + + OPA_CONTAINER.submitPolicy( + """ + package trino + import future.keywords.in + default allow = false + + allow { + input.context.identity.user == "admin" + } + + allow { + input.context.identity.user == "someone" + input.context.properties.namespace == "diff-namespace" + } + """); + assertThatThrownBy(() -> runner.querySetOfStrings("someone", "SHOW CATALOGS")) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Access Denied"); + // smoke test: we can still query if we are the right user + runner.querySetOfStrings("admin", "SHOW CATALOGS"); + runner.teardown(); + } + + @Test + void testFilterOutItemsBatch() + throws Exception + { + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig() + .setAdditionalContextFile(OPA_ADDITIONAL_CONTEXT_FILE) + .setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME)) + .setOpaBatchUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_BATCH_ALLOW_POLICY_NAME))); + + OPA_CONTAINER.submitPolicy( + """ + package trino + import future.keywords.if + import future.keywords.in + default allow = false + + allow if is_admin + + allow { + is_bob + input.action.operation in ["AccessCatalog", "ExecuteQuery", "ImpersonateUser", "ShowSchemas", "SelectFromColumns"] + } + + batchAllow[i] { + some i + input.action.filterResources[i] + is_admin + } + + batchAllow[i] { + some i + is_bob + is_some_namespace + is_some_cluster + input.action.operation == "FilterCatalogs" + input.action.filterResources[i].catalog.name == "catalog_one" + } + + is_bob { + input.context.identity.user == "bob" + } + + is_admin { + input.context.identity.user == "admin" + } + + is_some_namespace { + input.context.properties.namespace == "some-namespace" + } + is_some_cluster { + input.context.properties.cluster == "some-cluster" + } + """); + Set catalogsForBob = runner.querySetOfStrings("bob", "SHOW CATALOGS"); + assertThat(catalogsForBob).containsExactlyInAnyOrder("catalog_one"); + Set catalogsForAdmin = runner.querySetOfStrings("admin", "SHOW CATALOGS"); + assertThat(catalogsForAdmin).containsExactlyInAnyOrder("catalog_one", "catalog_two", "system"); + runner.teardown(); + } + + private static QueryRunnerHelper setupTrinoWithOpa(OpaConfig opaConfig) + { + QueryRunnerHelper runner = QueryRunnerHelper.withOpaConfig(opaConfig); + runner.getBaseQueryRunner().installPlugin(new BlackHolePlugin()); + runner.getBaseQueryRunner().createCatalog("catalog_one", "blackhole"); + runner.getBaseQueryRunner().createCatalog("catalog_two", "blackhole"); + return runner; + } +} diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java index 992b64530294..3f0ea50c3c39 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test; import java.net.URI; +import java.nio.file.Paths; import java.util.Map; import static io.airlift.configuration.testing.ConfigAssertions.assertFullMapping; @@ -36,7 +37,8 @@ void testDefaults() .setOpaBatchColumnMaskingUri(null) .setLogRequests(false) .setLogResponses(false) - .setAllowPermissionManagementOperations(false)); + .setAllowPermissionManagementOperations(false) + .setAdditionalContextFile(null)); } @Test @@ -51,6 +53,7 @@ void testExplicitPropertyMappings() .put("opa.log-requests", "true") .put("opa.log-responses", "true") .put("opa.allow-permission-management-operations", "true") + .put("opa.context-file", "src/test/resources/additional-context.properties") .buildOrThrow(); OpaConfig expected = new OpaConfig() @@ -61,7 +64,8 @@ void testExplicitPropertyMappings() .setOpaBatchColumnMaskingUri(URI.create("https://opa-column-masking.example.com")) .setLogRequests(true) .setLogResponses(true) - .setAllowPermissionManagementOperations(true); + .setAllowPermissionManagementOperations(true) + .setAdditionalContextFile(Paths.get("src/test/resources/additional-context.properties")); assertFullMapping(properties, expected); } diff --git a/plugin/trino-opa/src/test/resources/additional-context.properties b/plugin/trino-opa/src/test/resources/additional-context.properties new file mode 100644 index 000000000000..c536f3a0d35e --- /dev/null +++ b/plugin/trino-opa/src/test/resources/additional-context.properties @@ -0,0 +1,2 @@ +namespace=some-namespace +cluster=some-cluster