Skip to content

Commit

Permalink
Fix case-sensitivity issue with views and column masks
Browse files Browse the repository at this point in the history
The table column reference was registered incorectly with the original
case taken from the view definition. It then failed to match the column
schema returned from `SystemAccessControl` and the mask was not applied.

Additionally, the registered table column references are also used to
check access to the columns; those were also using the original case
from the view definition and access control was not recognizing them.

Fixes trinodb#24054.
  • Loading branch information
ksobolew committed Nov 7, 2024
1 parent ec2a985 commit 0ede278
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ private Type handleResolvedField(Expression node, ResolvedField resolvedField, C
}

if (field.getOriginTable().isPresent() && field.getOriginColumnName().isPresent()) {
tableColumnReferences.put(field.getOriginTable().get(), field.getOriginColumnName().get());
tableColumnReferences.put(field.getOriginTable().get(), field.getOriginColumnName().get().toLowerCase(ENGLISH));
}

sourceFields.add(field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@
import static io.trino.sql.tree.PatternSearchMode.Mode.INITIAL;
import static io.trino.sql.tree.SkipTo.Position.PAST_LAST;
import static io.trino.type.Json2016Type.JSON_2016;
import static java.util.Locale.ENGLISH;
import static java.util.Objects.requireNonNull;

class RelationPlanner
Expand Down Expand Up @@ -440,7 +441,7 @@ private RelationPlan addColumnMasks(Table table, RelationPlan plan)
for (int i = 0; i < plan.getDescriptor().getAllFieldCount(); i++) {
Field field = plan.getDescriptor().getFieldByIndex(i);

io.trino.sql.tree.Expression mask = columnMasks.get(field.getName().orElseThrow());
io.trino.sql.tree.Expression mask = columnMasks.get(field.getName().orElseThrow().toLowerCase(ENGLISH));
Symbol symbol = plan.getFieldMappings().get(i);
Expression projection = symbol.toSymbolReference();
if (mask != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,17 @@ public TestColumnMask()
Optional.of(VIEW_OWNER),
false,
ImmutableList.of());
ConnectorViewDefinition viewWithDifferentCase = new ConnectorViewDefinition(
"SELECT NATIONKEY, NAME FROM local.tiny.nation",
Optional.empty(),
Optional.empty(),
ImmutableList.of(
new ConnectorViewDefinition.ViewColumn("NATIONKEY", BigintType.BIGINT.getTypeId(), Optional.empty()),
new ConnectorViewDefinition.ViewColumn("NAME", VarcharType.createVarcharType(25).getTypeId(), Optional.empty())),
Optional.empty(),
Optional.of(VIEW_OWNER),
false,
ImmutableList.of());

ConnectorViewDefinition viewWithNested = new ConnectorViewDefinition(
"""
Expand Down Expand Up @@ -171,6 +182,7 @@ public TestColumnMask()
})
.withGetViews((s, prefix) -> ImmutableMap.of(
new SchemaTableName("default", "nation_view"), view,
new SchemaTableName("default", "nation_view_uppercase"), viewWithDifferentCase,
new SchemaTableName("default", "view_with_nested"), viewWithNested))
.withGetMaterializedViews((s, prefix) -> ImmutableMap.of(
new SchemaTableName("default", "nation_materialized_view"), materializedView,
Expand Down Expand Up @@ -456,6 +468,23 @@ public void testView()
assertThat(assertions.query("SELECT name FROM mock.default.nation_view WHERE nationkey = 1")).matches("VALUES CAST('ANITNEGRA' AS VARCHAR(25))");
}

@Test
public void testViewWithUppercaseColumnName()
{
accessControl.reset();
accessControl.columnMask(
new QualifiedObjectName(MOCK_CATALOG, "default", "nation_view_uppercase"),
"name",
USER,
ViewExpression.builder()
.identity(USER)
.catalog(LOCAL_CATALOG)
.schema("tiny")
.expression("reverse(name)")
.build());
assertThat(assertions.query("SELECT name FROM mock.default.nation_view_uppercase WHERE nationkey = 1")).matches("VALUES CAST('ANITNEGRA' AS VARCHAR(25))");
}

@Test
public void testTableReferenceInWithClause()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,16 @@ public void testViewColumnAccessControl()
"SELECT * FROM " + columnAccessViewName,
privilege(getSession().getUser(), "orders", SELECT_COLUMN));

// a view which exposes underlying columns with different case
assertAccessAllowed(
viewOwnerSession,
"CREATE VIEW " + columnAccessViewName + "_uppercase AS SELECT orderkey AS ORDERKEY FROM orders");
// access control should "see" lowercase name
assertAccessDenied(
"SELECT * FROM " + columnAccessViewName + "_uppercase",
"Cannot select from columns \\[orderkey] in table or view .*_uppercase",
privilege(getSession().getUser(), columnAccessViewName + "_uppercase.orderkey", SELECT_COLUMN));

Session nestedViewOwnerSession = TestingSession.testSessionBuilder()
.setIdentity(Identity.ofUser("test_nested_view_access_owner"))
.setCatalog(getSession().getCatalog())
Expand Down

0 comments on commit 0ede278

Please sign in to comment.