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 8c12081e815c..4740ebdf94e2 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 @@ -76,10 +76,10 @@ import static io.trino.plugin.opa.TestHelpers.createResponseHandlerForParallelColumnMasking; import static org.assertj.core.api.Assertions.assertThat; -public class TestOpaAccessControl +final class TestOpaAccessControl { @Test - public void testResponseHasExtraFields() + void testResponseHasExtraFields() { InstrumentedHttpClient mockClient = createMockHttpClient( OPA_SERVER_URI, @@ -98,7 +98,7 @@ public void testResponseHasExtraFields() } @Test - public void testNoResourceAction() + void testNoResourceAction() { testNoResourceAction("ExecuteQuery", (opaAccessControl, identity) -> opaAccessControl.checkCanExecuteQuery(identity, TEST_QUERY_ID)); testNoResourceAction("ReadSystemInformation", OpaAccessControl::checkCanReadSystemInformation); @@ -118,7 +118,7 @@ private void testNoResourceAction(String actionName, BiConsumer accessControl.checkCanSetSystemSessionProperty(systemSecurityContext.getIdentity(), TEST_QUERY_ID, argument)); testStringResourceAction("CreateCatalog", "catalog", OpaAccessControl::checkCanCreateCatalog); @@ -271,7 +271,7 @@ private void testStringResourceAction( } @Test - public void testCanImpersonateUser() + void testCanImpersonateUser() { String expectedRequest = """ @@ -290,7 +290,7 @@ public void testCanImpersonateUser() } @Test - public void testCanAccessCatalog() + void testCanAccessCatalog() { ReturningMethodWrapper wrappedMethod = new ReturningMethodWrapper( accessControl -> accessControl.canAccessCatalog(TEST_SECURITY_CONTEXT, "test_catalog")); @@ -309,7 +309,7 @@ public void testCanAccessCatalog() } @Test - public void testSchemaResourceActions() + void testSchemaResourceActions() { testSchemaResourceActions("DropSchema", OpaAccessControl::checkCanDropSchema); testSchemaResourceActions("ShowCreateSchema", OpaAccessControl::checkCanShowCreateSchema); @@ -340,7 +340,7 @@ private void testSchemaResourceActions( } @Test - public void testCreateSchema() + void testCreateSchema() { CatalogSchemaName schema = new CatalogSchemaName("my_catalog", "my_schema"); ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper( @@ -362,7 +362,7 @@ public void testCreateSchema() } @Test - public void testCreateSchemaWithProperties() + void testCreateSchemaWithProperties() { CatalogSchemaName schema = new CatalogSchemaName("my_catalog", "my_schema"); ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper( @@ -386,7 +386,7 @@ public void testCreateSchemaWithProperties() } @Test - public void testRenameSchema() + void testRenameSchema() { ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper(accessControl -> accessControl.checkCanRenameSchema( TEST_SECURITY_CONTEXT, @@ -413,7 +413,7 @@ public void testRenameSchema() } @Test - public void testRenameTableLikeObjects() + void testRenameTableLikeObjects() { testRenameTableLikeObject("RenameTable", OpaAccessControl::checkCanRenameTable); testRenameTableLikeObject("RenameView", OpaAccessControl::checkCanRenameView); @@ -453,7 +453,7 @@ private void testRenameTableLikeObject( } @Test - public void testSetSchemaAuthorization() + void testSetSchemaAuthorization() { CatalogSchemaName schema = new CatalogSchemaName("my_catalog", "my_schema"); TrinoPrincipal principal = new TrinoPrincipal(PrincipalType.USER, "my_user"); @@ -481,7 +481,7 @@ public void testSetSchemaAuthorization() } @Test - public void testSetAuthorizationOnTableLikeObjects() + void testSetAuthorizationOnTableLikeObjects() { testSetAuthorizationOnTableLikeObject("SetTableAuthorization", OpaAccessControl::checkCanSetTableAuthorization); testSetAuthorizationOnTableLikeObject("SetViewAuthorization", OpaAccessControl::checkCanSetViewAuthorization); @@ -523,7 +523,7 @@ private void testSetAuthorizationOnTableLikeObject( } @Test - public void testColumnOperationsOnTableLikeObjects() + void testColumnOperationsOnTableLikeObjects() { testColumnOperationOnTableLikeObject("SelectFromColumns", OpaAccessControl::checkCanSelectFromColumns); testColumnOperationOnTableLikeObject("UpdateTableColumns", OpaAccessControl::checkCanUpdateTableColumns); @@ -561,7 +561,7 @@ private void testColumnOperationOnTableLikeObject( } @Test - public void testCanSetCatalogSessionProperty() + void testCanSetCatalogSessionProperty() { ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper( accessControl -> accessControl.checkCanSetCatalogSessionProperty(TEST_SECURITY_CONTEXT, "my_catalog", "my_property")); @@ -581,7 +581,7 @@ public void testCanSetCatalogSessionProperty() } @Test - public void testFunctionResourceActions() + void testFunctionResourceActions() { CatalogSchemaRoutineName routine = new CatalogSchemaRoutineName("my_catalog", "my_schema", "my_routine_name"); String baseRequest = @@ -618,7 +618,7 @@ public void testFunctionResourceActions() } @Test - public void testCanExecuteTableProcedure() + void testCanExecuteTableProcedure() { CatalogSchemaTableName table = new CatalogSchemaTableName("my_catalog", "my_schema", "my_table"); String expectedRequest = @@ -643,7 +643,7 @@ public void testCanExecuteTableProcedure() } @Test - public void testRequestContextContentsWithKnownTrinoVersion() + void testRequestContextContentsWithKnownTrinoVersion() { testRequestContextContentsForGivenTrinoVersion( Optional.of(new TestingSystemAccessControlContext("12345.67890")), @@ -651,7 +651,7 @@ public void testRequestContextContentsWithKnownTrinoVersion() } @Test - public void testRequestContextContentsWithUnknownTrinoVersion() + void testRequestContextContentsWithUnknownTrinoVersion() { testRequestContextContentsForGivenTrinoVersion(Optional.empty(), "UNKNOWN"); } @@ -688,7 +688,7 @@ private void testRequestContextContentsForGivenTrinoVersion(Optional methodUnderTest = authorizer -> authorizer.getRowFilters(TEST_SECURITY_CONTEXT, TEST_COLUMN_MASKING_TABLE_NAME); assertAccessControlMethodThrowsForIllegalResponses(methodUnderTest, rowFilteringOpaConfig(), OPA_ROW_FILTERING_URI); @@ -712,7 +712,7 @@ public void testGetRowFiltersThrowsForIllegalResponse() } @Test - public void testGetRowFilters() + void testGetRowFilters() { // This example is a bit strange - an undefined policy would in most cases // result in an access denied situation. However, since this is row-level-filtering @@ -794,7 +794,7 @@ private void testGetRowFilters(String responseContent, List e } @Test - public void testGetRowFiltersDoesNothingIfNotConfigured() + void testGetRowFiltersDoesNothingIfNotConfigured() { InstrumentedHttpClient httpClient = createMockHttpClient( OPA_SERVER_URI, @@ -814,7 +814,7 @@ public void testGetRowFiltersDoesNothingIfNotConfigured() * We test that it is a no-op if called. */ @Test - public void testGetColumnMaskDoesNothing() + void testGetColumnMaskDoesNothing() { InstrumentedHttpClient httpClient = createMockHttpClient( OPA_SERVER_URI, @@ -829,7 +829,7 @@ public void testGetColumnMaskDoesNothing() } @Test - public void testGetColumnMasks() + void testGetColumnMasks() { testGetColumnMasks(ImmutableMap.of(createColumnSchema("some-column"), "{}"), ImmutableMap.of()); @@ -898,7 +898,7 @@ public void testGetColumnMasks() } @Test - public void testGetColumnMasksDoesNothingIfNotConfigured() + void testGetColumnMasksDoesNothingIfNotConfigured() { InstrumentedHttpClient httpClient = createMockHttpClient( OPA_SERVER_URI, @@ -915,7 +915,7 @@ public void testGetColumnMasksDoesNothingIfNotConfigured() } @Test - public void testGetColumnMasksThrowsForIllegalResponse() + void testGetColumnMasksThrowsForIllegalResponse() { OpaConfig opaConfig = columnMaskingOpaConfig(); diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlDataFilteringSystem.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlDataFilteringSystem.java index 51cdf0b129dd..442c53fe2c45 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlDataFilteringSystem.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlDataFilteringSystem.java @@ -38,7 +38,7 @@ @Testcontainers @TestInstance(PER_CLASS) -public class TestOpaAccessControlDataFilteringSystem +final class TestOpaAccessControlDataFilteringSystem { @Container private static final OpaContainer OPA_CONTAINER = new OpaContainer(); @@ -122,7 +122,7 @@ public void teardown() } @Test - public void testRowFilteringEnabled() + void testRowFilteringEnabled() throws Exception { setupTrinoWithOpa( @@ -140,7 +140,7 @@ public void testRowFilteringEnabled() } @Test - public void testRowFilteringDisabledDoesNothing() + void testRowFilteringDisabledDoesNothing() throws Exception { setupTrinoWithOpa( @@ -157,7 +157,7 @@ public void testRowFilteringDisabledDoesNothing() } @Test - public void testColumnMasking() + void testColumnMasking() throws Exception { testColumnMasking( @@ -167,7 +167,7 @@ public void testColumnMasking() } @Test - public void testBatchColumnMasking() + void testBatchColumnMasking() throws Exception { testColumnMasking( @@ -239,7 +239,7 @@ private void testColumnMasking(OpaConfig opaConfig) } @Test - public void testColumnMaskingDisabledDoesNothing() + void testColumnMaskingDisabledDoesNothing() throws Exception { setupTrinoWithOpa(new OpaConfig().setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME))); @@ -254,7 +254,7 @@ public void testColumnMaskingDisabledDoesNothing() } @Test - public void testColumnMaskingAndRowFiltering() + void testColumnMaskingAndRowFiltering() throws Exception { setupTrinoWithOpa( diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFactory.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFactory.java index 37108b340b3d..fd06d8124d0f 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFactory.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFactory.java @@ -22,10 +22,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -public class TestOpaAccessControlFactory +final class TestOpaAccessControlFactory { @Test - public void testCreatesSimpleAuthorizerIfNoBatchUriProvided() + void testCreatesSimpleAuthorizerIfNoBatchUriProvided() { OpaAccessControlFactory factory = new OpaAccessControlFactory(); SystemAccessControl opaAuthorizer = factory.create(ImmutableMap.of("opa.policy.uri", "foo"), new TestingSystemAccessControlContext()); @@ -35,7 +35,7 @@ public void testCreatesSimpleAuthorizerIfNoBatchUriProvided() } @Test - public void testCreatesBatchAuthorizerIfBatchUriProvided() + void testCreatesBatchAuthorizerIfBatchUriProvided() { OpaAccessControlFactory factory = new OpaAccessControlFactory(); SystemAccessControl opaAuthorizer = factory.create( @@ -50,7 +50,7 @@ public void testCreatesBatchAuthorizerIfBatchUriProvided() } @Test - public void testBasePolicyUriCannotBeUnset() + void testBasePolicyUriCannotBeUnset() { OpaAccessControlFactory factory = new OpaAccessControlFactory(); @@ -58,7 +58,7 @@ public void testBasePolicyUriCannotBeUnset() } @Test - public void testConfigMayNotBeNull() + void testConfigMayNotBeNull() { OpaAccessControlFactory factory = new OpaAccessControlFactory(); @@ -66,7 +66,7 @@ public void testConfigMayNotBeNull() } @Test - public void testSupportsAirliftHttpConfigs() + void testSupportsAirliftHttpConfigs() { OpaAccessControlFactory factory = new OpaAccessControlFactory(); SystemAccessControl opaAuthorizer = factory.create( diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFiltering.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFiltering.java index 8decf8808bce..3cf6c3bfbacf 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFiltering.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlFiltering.java @@ -44,10 +44,10 @@ import static io.trino.plugin.opa.TestHelpers.createOpaAuthorizer; import static org.assertj.core.api.Assertions.assertThat; -public class TestOpaAccessControlFiltering +final class TestOpaAccessControlFiltering { @Test - public void testFilterViewQueryOwnedBy() + void testFilterViewQueryOwnedBy() { Identity userOne = Identity.ofUser("user-one"); Identity userTwo = Identity.ofUser("user-two"); @@ -97,7 +97,7 @@ public void testFilterViewQueryOwnedBy() } @Test - public void testFilterCatalogs() + void testFilterCatalogs() { Set requestedCatalogs = ImmutableSet.of("catalog_one", "catalog_two"); assertAccessControlMethodThrowsForIllegalResponses( @@ -142,7 +142,7 @@ public void testFilterCatalogs() } @Test - public void testFilterSchemas() + void testFilterSchemas() { Set requestedSchemas = ImmutableSet.of("schema_one", "schema_two"); assertAccessControlMethodThrowsForIllegalResponses( @@ -178,7 +178,7 @@ public void testFilterSchemas() } @Test - public void testFilterTables() + void testFilterTables() { Set tables = ImmutableSet.builder() .add(new SchemaTableName("schema_one", "table_one")) @@ -217,7 +217,7 @@ public void testFilterTables() } @Test - public void testFilterColumns() + void testFilterColumns() { SchemaTableName tableOne = SchemaTableName.schemaTableName("my_schema", "table_one"); SchemaTableName tableTwo = SchemaTableName.schemaTableName("my_schema", "table_two"); @@ -282,7 +282,7 @@ public void testFilterColumns() } @Test - public void testFilterFunctions() + void testFilterFunctions() { SchemaFunctionName functionOne = new SchemaFunctionName("my_schema", "function_one"); SchemaFunctionName functionTwo = new SchemaFunctionName("my_schema", "function_two"); diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPermissionManagementOperations.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPermissionManagementOperations.java index 62f06965b489..d36feacf3321 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPermissionManagementOperations.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPermissionManagementOperations.java @@ -35,12 +35,12 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -public class TestOpaAccessControlPermissionManagementOperations +final class TestOpaAccessControlPermissionManagementOperations { private static final URI OPA_SERVER_URI = URI.create("http://my-uri/"); @Test - public void testTablePrivilegeGrantingOperationsDeniedOrAllowedByConfig() + void testTablePrivilegeGrantingOperationsDeniedOrAllowedByConfig() { CatalogSchemaTableName sampleTableName = new CatalogSchemaTableName("some_catalog", "some_schema", "some_table"); TrinoPrincipal samplePrincipal = new TrinoPrincipal(PrincipalType.USER, "some_user"); @@ -54,7 +54,7 @@ public void testTablePrivilegeGrantingOperationsDeniedOrAllowedByConfig() } @Test - public void testSchemaPrivilegeGrantingOperationsDeniedOrAllowedByConfig() + void testSchemaPrivilegeGrantingOperationsDeniedOrAllowedByConfig() { CatalogSchemaName sampleSchemaName = new CatalogSchemaName("some_catalog", "some_schema"); TrinoPrincipal samplePrincipal = new TrinoPrincipal(PrincipalType.USER, "some_user"); @@ -68,21 +68,21 @@ public void testSchemaPrivilegeGrantingOperationsDeniedOrAllowedByConfig() } @Test - public void testCanCreateRoleAllowedOrDeniedByConfig() + void testCanCreateRoleAllowedOrDeniedByConfig() { testOperationAllowedOrDeniedByConfig( authorizer -> authorizer.checkCanCreateRole(TEST_SECURITY_CONTEXT, "some_role", Optional.empty())); } @Test - public void testCanDropRoleAllowedOrDeniedByConfig() + void testCanDropRoleAllowedOrDeniedByConfig() { testOperationAllowedOrDeniedByConfig( authorizer -> authorizer.checkCanDropRole(TEST_SECURITY_CONTEXT, "some_role")); } @Test - public void testCanGrantRolesAllowedOrDeniedByConfig() + void testCanGrantRolesAllowedOrDeniedByConfig() { Set roles = ImmutableSet.of("role_one", "role_two"); Set grantees = ImmutableSet.of(new TrinoPrincipal(PrincipalType.USER, "some_principal")); @@ -91,19 +91,19 @@ public void testCanGrantRolesAllowedOrDeniedByConfig() } @Test - public void testShowRolesAlwaysAllowedRegardlessOfConfig() + void testShowRolesAlwaysAllowedRegardlessOfConfig() { testOperationAlwaysAllowedRegardlessOfConfig(authorizer -> authorizer.checkCanShowRoles(TEST_SECURITY_CONTEXT)); } @Test - public void testShowCurrentRolesAlwaysAllowedRegardlessOfConfig() + void testShowCurrentRolesAlwaysAllowedRegardlessOfConfig() { testOperationAlwaysAllowedRegardlessOfConfig(authorizer -> authorizer.checkCanShowCurrentRoles(TEST_SECURITY_CONTEXT)); } @Test - public void testShowRoleGrantsAlwaysAllowedRegardlessOfConfig() + void testShowRoleGrantsAlwaysAllowedRegardlessOfConfig() { testOperationAlwaysAllowedRegardlessOfConfig(authorizer -> authorizer.checkCanShowRoleGrants(TEST_SECURITY_CONTEXT)); } diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPlugin.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPlugin.java index 13f5c7a37d73..72a8459655d5 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPlugin.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlPlugin.java @@ -21,10 +21,10 @@ import static com.google.common.collect.Iterables.getOnlyElement; -public class TestOpaAccessControlPlugin +final class TestOpaAccessControlPlugin { @Test - public void testCreatePlugin() + void testCreatePlugin() { Plugin opaPlugin = new OpaAccessControlPlugin(); SystemAccessControlFactory factory = getOnlyElement(opaPlugin.getSystemAccessControlFactories()); diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlSystem.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlSystem.java index 50915c74ee26..871993670961 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlSystem.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControlSystem.java @@ -14,16 +14,11 @@ package io.trino.plugin.opa; import io.trino.plugin.blackhole.BlackHolePlugin; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; 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.io.IOException; import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; @@ -32,195 +27,172 @@ @Testcontainers @TestInstance(PER_CLASS) -public class TestOpaAccessControlSystem +final class TestOpaAccessControlSystem { - private QueryRunnerHelper runner; private static final String OPA_ALLOW_POLICY_NAME = "allow"; private static final String OPA_BATCH_ALLOW_POLICY_NAME = "batchAllow"; @Container private static final OpaContainer OPA_CONTAINER = new OpaContainer(); - @Nested - @TestInstance(PER_CLASS) - @DisplayName("Unbatched Authorizer Tests") - class UnbatchedAuthorizerTests + @Test + void testAllowsQueryAndFilters() + throws Exception { - @BeforeAll - public void setupTrino() - { - setupTrinoWithOpa(new OpaConfig().setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME))); - } - - @AfterAll - public void teardown() - { - runner.teardown(); - } - - @Test - public void testAllowsQueryAndFilters() - throws IOException, InterruptedException - { - OPA_CONTAINER.submitPolicy( - """ - package trino - import future.keywords.in - import future.keywords.if - - default allow = false - allow { - is_bob - can_be_accessed_by_bob - } - allow if is_admin - - is_admin { - input.context.identity.user == "admin" - } - is_bob { - input.context.identity.user == "bob" - } - 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"); - } - - @Test - public void testShouldDenyQueryIfDirected() - throws IOException, InterruptedException - { - OPA_CONTAINER.submitPolicy( - """ - package trino - import future.keywords.in - default allow = false - - allow { - input.context.identity.user in ["someone", "admin"] - } - """); - assertThatThrownBy(() -> runner.querySetOfStrings("bob", "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"); - } + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig().setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME))); + OPA_CONTAINER.submitPolicy( + """ + package trino + import future.keywords.in + import future.keywords.if + + default allow = false + allow { + is_bob + can_be_accessed_by_bob + } + allow if is_admin + + is_admin { + input.context.identity.user == "admin" + } + is_bob { + input.context.identity.user == "bob" + } + 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(); } - @Nested - @TestInstance(PER_CLASS) - @DisplayName("Batched Authorizer Tests") - class BatchedAuthorizerTests + @Test + void testShouldDenyQueryIfDirected() + throws Exception { - @BeforeAll - public void setupTrino() - { - setupTrinoWithOpa(new OpaConfig() - .setOpaUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_ALLOW_POLICY_NAME)) - .setOpaBatchUri(OPA_CONTAINER.getOpaUriForPolicyPath(OPA_BATCH_ALLOW_POLICY_NAME))); - } - - @AfterAll - public void teardown() - { - if (runner != null) { - runner.teardown(); - } - } - - @Test - public void testFilterOutItemsBatch() - throws IOException, InterruptedException - { - OPA_CONTAINER.submitPolicy( - """ - package trino - import future.keywords.in - import future.keywords.if - default allow = false - - allow if is_admin - - allow { - is_bob - input.action.operation in ["AccessCatalog", "ExecuteQuery", "ImpersonateUser", "ShowSchemas", "SelectFromColumns"] - } - - is_bob { - input.context.identity.user == "bob" - } - - is_admin { - input.context.identity.user == "admin" - } - - batchAllow[i] { - some i - is_bob - input.action.operation == "FilterCatalogs" - input.action.filterResources[i].catalog.name == "catalog_one" - } - - batchAllow[i] { - some i - input.action.filterResources[i] - is_admin - } - """); - 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"); - } - - @Test - public void testDenyUnbatchedQuery() - throws IOException, InterruptedException - { - OPA_CONTAINER.submitPolicy( - """ - package trino - import future.keywords.in - default allow = false - """); - assertThatThrownBy(() -> runner.querySetOfStrings("bob", "SELECT version()")) - .isInstanceOf(RuntimeException.class) - .hasMessageContaining("Access Denied"); - } - - @Test - public void testAllowUnbatchedQuery() - throws IOException, InterruptedException - { - OPA_CONTAINER.submitPolicy( - """ - package trino - import future.keywords.in - default allow = false - allow { - input.context.identity.user == "bob" - input.action.operation in ["ImpersonateUser", "ExecuteFunction", "AccessCatalog", "ExecuteQuery"] - } - """); - Set version = runner.querySetOfStrings("bob", "SELECT version()"); - assertThat(version).isNotEmpty(); - } + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig().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 in ["someone", "admin"] + } + """); + assertThatThrownBy(() -> runner.querySetOfStrings("bob", "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(); } - private void setupTrinoWithOpa(OpaConfig opaConfig) + @Test + void testFilterOutItemsBatch() + throws Exception { - this.runner = QueryRunnerHelper.withOpaConfig(opaConfig); + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig() + .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.in + import future.keywords.if + default allow = false + + allow if is_admin + + allow { + is_bob + input.action.operation in ["AccessCatalog", "ExecuteQuery", "ImpersonateUser", "ShowSchemas", "SelectFromColumns"] + } + + is_bob { + input.context.identity.user == "bob" + } + + is_admin { + input.context.identity.user == "admin" + } + + batchAllow[i] { + some i + is_bob + input.action.operation == "FilterCatalogs" + input.action.filterResources[i].catalog.name == "catalog_one" + } + + batchAllow[i] { + some i + input.action.filterResources[i] + is_admin + } + """); + 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 testDenyUnbatchedQuery() + throws Exception + { + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig() + .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.in + default allow = false + """); + assertThatThrownBy(() -> runner.querySetOfStrings("bob", "SELECT version()")) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Access Denied"); + runner.teardown(); + } + + @Test + void testAllowUnbatchedQuery() + throws Exception + { + QueryRunnerHelper runner = setupTrinoWithOpa(new OpaConfig() + .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.in + default allow = false + allow { + input.context.identity.user == "bob" + input.action.operation in ["ImpersonateUser", "ExecuteFunction", "AccessCatalog", "ExecuteQuery"] + } + """); + Set version = runner.querySetOfStrings("bob", "SELECT version()"); + assertThat(version).isNotEmpty(); + 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/TestOpaBatchAccessControlFiltering.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java index c45108a65dc1..07442a3386ab 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java @@ -47,10 +47,10 @@ import static io.trino.plugin.opa.TestHelpers.createOpaAuthorizer; import static org.assertj.core.api.Assertions.assertThat; -public class TestOpaBatchAccessControlFiltering +final class TestOpaBatchAccessControlFiltering { @Test - public void testFilterViewQueryOwnedBy() + void testFilterViewQueryOwnedBy() { Identity identityOne = Identity.ofUser("user-one"); Identity identityTwo = Identity.ofUser("user-two"); @@ -91,7 +91,7 @@ public void testFilterViewQueryOwnedBy() } @Test - public void testFilterCatalogs() + void testFilterCatalogs() { String expectedRequest = """ @@ -121,7 +121,7 @@ public void testFilterCatalogs() } @Test - public void testFilterSchemas() + void testFilterSchemas() { String expectedRequest = """ @@ -158,7 +158,7 @@ public void testFilterSchemas() } @Test - public void testFilterTables() + void testFilterTables() { String expectedRequest = """ @@ -198,7 +198,7 @@ public void testFilterTables() } @Test - public void testFilterColumns() + void testFilterColumns() { SchemaTableName tableOne = SchemaTableName.schemaTableName("my_schema", "table_one"); SchemaTableName tableTwo = SchemaTableName.schemaTableName("my_schema", "table_two"); @@ -256,7 +256,7 @@ public void testFilterColumns() } @Test - public void testEmptyFilterColumns() + void testEmptyFilterColumns() { assertFilteringAccessControlMethodDoesNotSendRequests( accessControl -> accessControl.filterColumns(TEST_SECURITY_CONTEXT, "my_catalog", ImmutableMap.of()).entrySet()); @@ -272,7 +272,7 @@ public void testEmptyFilterColumns() } @Test - public void testFilterColumnErrorCases() + void testFilterColumnErrorCases() { assertAccessControlMethodThrowsForIllegalResponses( accessControl -> accessControl.filterColumns( @@ -284,7 +284,7 @@ public void testFilterColumnErrorCases() } @Test - public void testFilterFunctions() + void testFilterFunctions() { String expectedRequest = """ 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 307bb272a101..992b64530294 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 @@ -23,10 +23,10 @@ import static io.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults; import static io.airlift.configuration.testing.ConfigAssertions.recordDefaults; -public class TestOpaConfig +final class TestOpaConfig { @Test - public void testDefaults() + void testDefaults() { assertRecordedDefaults(recordDefaults(OpaConfig.class) .setOpaUri(null) @@ -40,7 +40,7 @@ public void testDefaults() } @Test - public void testExplicitPropertyMappings() + void testExplicitPropertyMappings() { Map properties = ImmutableMap.builder() .put("opa.policy.uri", "https://opa.example.com") diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaResponseDecoding.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaResponseDecoding.java index 501eb5016ce2..6da1d489cd43 100644 --- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaResponseDecoding.java +++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaResponseDecoding.java @@ -27,7 +27,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -public class TestOpaResponseDecoding +final class TestOpaResponseDecoding { private final JsonCodec responseCodec = new JsonCodecFactory().jsonCodec(OpaQueryResult.class); private final JsonCodec batchResponseCodec = new JsonCodecFactory().jsonCodec(OpaBatchQueryResult.class); @@ -35,7 +35,7 @@ public class TestOpaResponseDecoding private final JsonCodec columnMaskingResponseCodec = new JsonCodecFactory().jsonCodec(OpaColumnMaskQueryResult.class); @Test - public void testCanDeserializeOpaSingleResponse() + void testCanDeserializeOpaSingleResponse() { testCanDeserializeOpaSingleResponse(true); testCanDeserializeOpaSingleResponse(false); @@ -55,7 +55,7 @@ private void testCanDeserializeOpaSingleResponse(boolean response) } @Test - public void testCanDeserializeOpaSingleResponseWithNoDecisionId() + void testCanDeserializeOpaSingleResponseWithNoDecisionId() { testCanDeserializeOpaSingleResponseWithNoDecisionId(true); testCanDeserializeOpaSingleResponseWithNoDecisionId(false); @@ -74,7 +74,7 @@ private void testCanDeserializeOpaSingleResponseWithNoDecisionId(boolean respons } @Test - public void testSingleResponseWithExtraFields() + void testSingleResponseWithExtraFields() { OpaQueryResult result = this.responseCodec.fromJson( """ @@ -88,7 +88,7 @@ public void testSingleResponseWithExtraFields() } @Test - public void testUndefinedDecisionSingleResponseTreatedAsDeny() + void testUndefinedDecisionSingleResponseTreatedAsDeny() { OpaQueryResult result = this.responseCodec.fromJson("{}"); assertThat(result.result()).isFalse(); @@ -96,13 +96,13 @@ public void testUndefinedDecisionSingleResponseTreatedAsDeny() } @Test - public void testIllegalResponseThrows() + void testIllegalResponseThrows() { testIllegalResponseDecodingThrows("{\"result\": \"foo\"}", responseCodec); } @Test - public void testBatchEmptyOrUndefinedResponses() + void testBatchEmptyOrUndefinedResponses() { testBatchEmptyOrUndefinedResponses("{}"); testBatchEmptyOrUndefinedResponses("{\"result\": []}"); @@ -116,7 +116,7 @@ private void testBatchEmptyOrUndefinedResponses(String response) } @Test - public void testBatchResponseWithItemsNoDecisionId() + void testBatchResponseWithItemsNoDecisionId() { OpaBatchQueryResult result = this.batchResponseCodec.fromJson( """ @@ -129,7 +129,7 @@ public void testBatchResponseWithItemsNoDecisionId() } @Test - public void testBatchResponseWithItemsAndDecisionId() + void testBatchResponseWithItemsAndDecisionId() { OpaBatchQueryResult result = this.batchResponseCodec.fromJson( """ @@ -143,7 +143,7 @@ public void testBatchResponseWithItemsAndDecisionId() } @Test - public void testBatchResponseIllegalResponseThrows() + void testBatchResponseIllegalResponseThrows() { testIllegalResponseDecodingThrows( """ @@ -155,7 +155,7 @@ public void testBatchResponseIllegalResponseThrows() } @Test - public void testBatchResponseWithExtraFields() + void testBatchResponseWithExtraFields() { OpaBatchQueryResult result = this.batchResponseCodec.fromJson( """ @@ -171,7 +171,7 @@ public void testBatchResponseWithExtraFields() } @Test - public void testRowFilteringEmptyOrUndefinedResponses() + void testRowFilteringEmptyOrUndefinedResponses() { testRowFilteringEmptyOrUndefinedResponses("{}"); testRowFilteringEmptyOrUndefinedResponses("{\"result\": []}"); @@ -185,7 +185,7 @@ private void testRowFilteringEmptyOrUndefinedResponses(String response) } @Test - public void testRowFilteringResponseWithItemsNoDecisionId() + void testRowFilteringResponseWithItemsNoDecisionId() { OpaRowFiltersQueryResult result = this.rowFilteringResponseCodec.fromJson( """ @@ -203,7 +203,7 @@ public void testRowFilteringResponseWithItemsNoDecisionId() } @Test - public void testRowFilteringResponseWithItemsAndDecisionId() + void testRowFilteringResponseWithItemsAndDecisionId() { OpaRowFiltersQueryResult result = this.rowFilteringResponseCodec.fromJson( """ @@ -217,7 +217,7 @@ public void testRowFilteringResponseWithItemsAndDecisionId() } @Test - public void testRowFilteringResponseWithExtraFields() + void testRowFilteringResponseWithExtraFields() { OpaRowFiltersQueryResult result = this.rowFilteringResponseCodec.fromJson( """ @@ -233,7 +233,7 @@ public void testRowFilteringResponseWithExtraFields() } @Test - public void testRowFilteringResponseIllegalResponseThrows() + void testRowFilteringResponseIllegalResponseThrows() { testIllegalResponseDecodingThrows( """ @@ -244,7 +244,7 @@ public void testRowFilteringResponseIllegalResponseThrows() } @Test - public void testColumnMaskingEmptyOrUndefinedResponse() + void testColumnMaskingEmptyOrUndefinedResponse() { OpaColumnMaskQueryResult emptyResult = columnMaskingResponseCodec.fromJson("{}"); assertThat(emptyResult.result()).isEmpty(); @@ -255,7 +255,7 @@ public void testColumnMaskingEmptyOrUndefinedResponse() } @Test - public void testColumnMaskingResponsesWithNoDecisionId() + void testColumnMaskingResponsesWithNoDecisionId() { OpaColumnMaskQueryResult result = this.columnMaskingResponseCodec.fromJson( """ @@ -268,7 +268,7 @@ public void testColumnMaskingResponsesWithNoDecisionId() } @Test - public void testColumnMaskingResponsesWithDecisionId() + void testColumnMaskingResponsesWithDecisionId() { OpaColumnMaskQueryResult resultWithExpression = this.columnMaskingResponseCodec.fromJson( """ @@ -291,7 +291,7 @@ public void testColumnMaskingResponsesWithDecisionId() } @Test - public void testColumnMaskingResponseWithExtraFields() + void testColumnMaskingResponseWithExtraFields() { OpaColumnMaskQueryResult result = this.columnMaskingResponseCodec.fromJson( """ @@ -307,7 +307,7 @@ public void testColumnMaskingResponseWithExtraFields() } @Test - public void testColumnMaskingResponseIllegalResponseThrows() + void testColumnMaskingResponseIllegalResponseThrows() { testIllegalResponseDecodingThrows( """