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

Fix GRANT, DENY, REVOKE on view and materialized view #20812

Merged
merged 2 commits into from
Feb 23, 2024
+92 −20
Merged

Fix GRANT, DENY, REVOKE on view and materialized view #20812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1abd058
Fix GRANT, DENY, REVOKE on view and materialized view
findepi Feb 21, 2024
a255da0
Fix Hive getTableHandle to reject views
findepi Feb 21, 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
15 changes: 9 additions & 6 deletions core/trino-main/src/main/java/io/trino/execution/DenyTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,15 @@ private static void executeDenyOnSchema(Session session, Deny statement, Metadat
private static void executeDenyOnTable(Session session, Deny statement, Metadata metadata, AccessControl accessControl)
{
QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getGrantObject().getName());
RedirectionAwareTableHandle redirection = metadata.getRedirectionAwareTableHandle(session, tableName);
if (redirection.tableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}
if (redirection.redirectedTableName().isPresent()) {
throw semanticException(NOT_SUPPORTED, statement, "Table %s is redirected to %s and DENY is not supported with table redirections", tableName, redirection.redirectedTableName().get());

if (!metadata.isMaterializedView(session, tableName) && !metadata.isView(session, tableName)) {
RedirectionAwareTableHandle redirection = metadata.getRedirectionAwareTableHandle(session, tableName);
if (redirection.tableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}
if (redirection.redirectedTableName().isPresent()) {
throw semanticException(NOT_SUPPORTED, statement, "Table %s is redirected to %s and DENY is not supported with table redirections", tableName, redirection.redirectedTableName().get());
}
}

Set<Privilege> privileges = parseStatementPrivileges(statement, statement.getPrivileges());
Expand Down
15 changes: 9 additions & 6 deletions core/trino-main/src/main/java/io/trino/execution/GrantTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,15 @@ private void executeGrantOnSchema(Session session, Grant statement)
private void executeGrantOnTable(Session session, Grant statement)
{
QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getGrantObject().getName());
RedirectionAwareTableHandle redirection = metadata.getRedirectionAwareTableHandle(session, tableName);
if (redirection.tableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}
if (redirection.redirectedTableName().isPresent()) {
throw semanticException(NOT_SUPPORTED, statement, "Table %s is redirected to %s and GRANT is not supported with table redirections", tableName, redirection.redirectedTableName().get());

if (!metadata.isMaterializedView(session, tableName) && !metadata.isView(session, tableName)) {
RedirectionAwareTableHandle redirection = metadata.getRedirectionAwareTableHandle(session, tableName);
if (redirection.tableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}
if (redirection.redirectedTableName().isPresent()) {
throw semanticException(NOT_SUPPORTED, statement, "Table %s is redirected to %s and GRANT is not supported with table redirections", tableName, redirection.redirectedTableName().get());
}
}

Set<Privilege> privileges = parseStatementPrivileges(statement, statement.getPrivileges());
Expand Down
15 changes: 9 additions & 6 deletions core/trino-main/src/main/java/io/trino/execution/RevokeTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,15 @@ private void executeRevokeOnSchema(Session session, Revoke statement)
private void executeRevokeOnTable(Session session, Revoke statement)
{
QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getGrantObject().getName());
RedirectionAwareTableHandle redirection = metadata.getRedirectionAwareTableHandle(session, tableName);
if (redirection.tableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}
if (redirection.redirectedTableName().isPresent()) {
throw semanticException(NOT_SUPPORTED, statement, "Table %s is redirected to %s and REVOKE is not supported with table redirections", tableName, redirection.redirectedTableName().get());

if (!metadata.isMaterializedView(session, tableName) && !metadata.isView(session, tableName)) {
RedirectionAwareTableHandle redirection = metadata.getRedirectionAwareTableHandle(session, tableName);
if (redirection.tableHandle().isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}
if (redirection.redirectedTableName().isPresent()) {
throw semanticException(NOT_SUPPORTED, statement, "Table %s is redirected to %s and REVOKE is not supported with table redirections", tableName, redirection.redirectedTableName().get());
}
}

Set<Privilege> privileges = parseStatementPrivileges(statement, statement.getPrivileges());
Expand Down
3 changes: 3 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,9 @@ public HiveTableHandle getTableHandle(ConnectorSession session, SchemaTableName
return null;
}

if (isSomeKindOfAView(table)) {
return null;
}
if (isDeltaLakeTable(table)) {
throw new TrinoException(UNSUPPORTED_TABLE_TYPE, format("Cannot query Delta Lake table '%s'", tableName));
}
Expand Down
64 changes: 62 additions & 2 deletions testing/trino-tests/src/test/java/io/trino/execution/TestGrantOnTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@
package io.trino.execution;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import io.trino.Session;
import io.trino.connector.Grants;
import io.trino.connector.MockConnectorFactory;
import io.trino.connector.MockConnectorPlugin;
import io.trino.connector.MockConnectorTableHandle;
import io.trino.connector.MutableGrants;
import io.trino.spi.connector.CatalogSchemaTableName;
import io.trino.spi.connector.ConnectorMaterializedViewDefinition;
import io.trino.spi.connector.ConnectorViewDefinition;
import io.trino.spi.connector.SchemaTableName;
import io.trino.spi.security.Identity;
import io.trino.spi.security.Privilege;
Expand All @@ -33,12 +37,14 @@
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.parallel.Execution;

import java.time.Duration;
import java.util.EnumSet;
import java.util.Optional;
import java.util.OptionalLong;

import static io.trino.common.Randoms.randomUsername;
import static io.trino.spi.security.PrincipalType.USER;
import static io.trino.spi.type.BigintType.BIGINT;
import static io.trino.testing.QueryAssertions.assertUpdate;
import static io.trino.testing.TestingSession.testSessionBuilder;
import static java.lang.String.format;
Expand All @@ -52,6 +58,8 @@
public class TestGrantOnTable
{
private final SchemaTableName table = new SchemaTableName("default", "table_one");
private final SchemaTableName view = new SchemaTableName("default", "test_view");
private final SchemaTableName materializedView = new SchemaTableName("default", "test_materialized_view");
private final Session admin = sessionOf("admin");
private final Grants<SchemaTableName> tableGrants = new MutableGrants<>();
private QueryRunner queryRunner;
Expand All @@ -64,15 +72,42 @@ public void initClass()
queryRunner = DistributedQueryRunner.builder(admin).build();
MockConnectorFactory connectorFactory = MockConnectorFactory.builder()
.withListSchemaNames(session -> ImmutableList.of("default"))
.withListTables((session, schemaName) -> "default".equalsIgnoreCase(schemaName) ? ImmutableList.of(table.getTableName()) : ImmutableList.of())
.withListTables((session, schemaName) -> "default".equalsIgnoreCase(schemaName)
? ImmutableList.of(table.getTableName(), view.getTableName(), materializedView.getTableName())
: ImmutableList.of())
.withGetTableHandle((session, tableName) -> tableName.equals(table) ? new MockConnectorTableHandle(tableName) : null)
.withSchemaGrants(new MutableGrants<>())
.withTableGrants(tableGrants)
.withGetViews((session, schemaTablePrefix) ->
ImmutableMap.of(
view, new ConnectorViewDefinition(
"SELECT nationkey AS test_column FROM tpch.tiny.nation",
Optional.empty(),
Optional.empty(),
ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test_column", BIGINT.getTypeId(), Optional.empty())),
Optional.empty(),
Optional.empty(),
true,
ImmutableList.of())))
.withGetMaterializedViews((connectorSession, prefix) ->
ImmutableMap.of(
materializedView, new ConnectorMaterializedViewDefinition(
"SELECT nationkey AS test_column FROM tpch.tiny.nation",
Optional.of(new CatalogSchemaTableName("mock", "default", "test_materialized_view$materialized_view_storage")),
Optional.of("mock"),
Optional.of("default"),
ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("test_column", BIGINT.getTypeId(), Optional.empty())),
Optional.of(Duration.ZERO),
Optional.empty(),
Optional.of("alice"),
ImmutableList.of())))
.build();
queryRunner.installPlugin(new MockConnectorPlugin(connectorFactory));
queryRunner.createCatalog("local", "mock");
assertions = new QueryAssertions(queryRunner);
tableGrants.grant(new TrinoPrincipal(USER, "admin"), table, EnumSet.allOf(Privilege.class), true);
tableGrants.grant(new TrinoPrincipal(USER, "admin"), view, EnumSet.allOf(Privilege.class), true);
tableGrants.grant(new TrinoPrincipal(USER, "admin"), materializedView, EnumSet.allOf(Privilege.class), true);
}

@AfterAll
Expand All @@ -96,7 +131,8 @@ private void testExistingGrants(boolean grantOption)
Session user = sessionOf(username);
tableGrants.grant(new TrinoPrincipal(USER, username), table, EnumSet.allOf(Privilege.class), grantOption);

assertThat(assertions.query(admin, "SHOW TABLES FROM local.default")).matches("VALUES (VARCHAR 'table_one')");
assertThat(assertions.query(admin, "SHOW TABLES FROM local.default"))
.matches("VALUES VARCHAR 'table_one', 'test_view', 'test_materialized_view'");
assertThat(assertions.query(user, "SHOW TABLES FROM local.default")).matches("VALUES (VARCHAR 'table_one')");
}

Expand All @@ -121,6 +157,30 @@ private void testValidGrant(String privilege)
assertThat(assertions.query(user, "SHOW TABLES FROM default")).matches("VALUES (VARCHAR 'table_one')");
}

@Test
public void testGrantOnView()
{
String username = randomUsername();
Session user = sessionOf(username);
assertThat(assertions.query(user, "SHOW TABLES FROM default"))
.result().isEmpty();
queryRunner.execute(admin, format("GRANT SELECT ON TABLE test_view TO %s", username));
assertThat(assertions.query(user, "SHOW TABLES FROM default"))
.matches("VALUES (VARCHAR 'test_view')");
}

@Test
public void testGrantOnMaterializedView()
{
String username = randomUsername();
Session user = sessionOf(username);
assertThat(assertions.query(user, "SHOW TABLES FROM default"))
.result().isEmpty();
queryRunner.execute(admin, format("GRANT SELECT ON TABLE test_materialized_view TO %s", username));
assertThat(assertions.query(user, "SHOW TABLES FROM default"))
.matches("VALUES (VARCHAR 'test_materialized_view')");
}

@Test
public void testValidGrantWithGrantOption()
{
Expand Down
Loading