Skip to content

Notifies systemSecurityMetadata on column add, rename, and drop events if using system security manager #16833

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

Merged
losipiuk merged 1 commit into from
Apr 13, 2023
Merged

Notifies systemSecurityMetadata on column add, rename, and drop events if using system security manager #16833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions core/trino-main/src/main/java/io/trino/execution/AddColumnTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ public ListenableFuture<Void> execute(
TableHandle tableHandle = redirectionAwareTableHandle.getTableHandle().get();
CatalogHandle catalogHandle = tableHandle.getCatalogHandle();

accessControl.checkCanAddColumns(session.toSecurityContext(), redirectionAwareTableHandle.getRedirectedTableName().orElse(originalTableName));
QualifiedObjectName qualifiedTableName = redirectionAwareTableHandle.getRedirectedTableName().orElse(originalTableName);
accessControl.checkCanAddColumns(session.toSecurityContext(), qualifiedTableName);

Map<String, ColumnHandle> columnHandles = plannerContext.getMetadata().getColumnHandles(session, tableHandle);

Expand Down Expand Up @@ -133,7 +134,7 @@ public ListenableFuture<Void> execute(
.setProperties(columnProperties)
.build();

plannerContext.getMetadata().addColumn(session, tableHandle, column);
plannerContext.getMetadata().addColumn(session, tableHandle, qualifiedTableName.asCatalogSchemaTableName(), column);

return immediateVoidFuture();
}
Expand Down
5 changes: 3 additions & 2 deletions core/trino-main/src/main/java/io/trino/execution/DropColumnTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ public ListenableFuture<Void> execute(
// Use getParts method because the column name should be lowercase
String column = statement.getField().getParts().get(0);

accessControl.checkCanDropColumn(session.toSecurityContext(), redirectionAwareTableHandle.getRedirectedTableName().orElse(tableName));
QualifiedObjectName qualifiedTableName = redirectionAwareTableHandle.getRedirectedTableName().orElse(tableName);
accessControl.checkCanDropColumn(session.toSecurityContext(), qualifiedTableName);

ColumnHandle columnHandle = metadata.getColumnHandles(session, tableHandle).get(column);
if (columnHandle == null) {
Expand All @@ -107,7 +108,7 @@ public ListenableFuture<Void> execute(
.filter(info -> !info.isHidden()).count() <= 1) {
throw semanticException(NOT_SUPPORTED, statement, "Cannot drop the only column in a table");
}
metadata.dropColumn(session, tableHandle, columnHandle);
metadata.dropColumn(session, tableHandle, qualifiedTableName.asCatalogSchemaTableName(), columnHandle);
}
else {
RowType containingType = null;
Expand Down
5 changes: 3 additions & 2 deletions core/trino-main/src/main/java/io/trino/execution/RenameColumnTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public ListenableFuture<Void> execute(
String source = statement.getSource().getValue().toLowerCase(ENGLISH);
String target = statement.getTarget().getValue().toLowerCase(ENGLISH);

accessControl.checkCanRenameColumn(session.toSecurityContext(), redirectionAwareTableHandle.getRedirectedTableName().orElse(originalTableName));
QualifiedObjectName qualifiedTableName = redirectionAwareTableHandle.getRedirectedTableName().orElse(originalTableName);
accessControl.checkCanRenameColumn(session.toSecurityContext(), qualifiedTableName);

Map<String, ColumnHandle> columnHandles = metadata.getColumnHandles(session, tableHandle);
ColumnHandle columnHandle = columnHandles.get(source);
Expand All @@ -99,7 +100,7 @@ public ListenableFuture<Void> execute(
throw semanticException(NOT_SUPPORTED, statement, "Cannot rename hidden column");
}

metadata.renameColumn(session, tableHandle, columnHandle, target);
metadata.renameColumn(session, tableHandle, qualifiedTableName.asCatalogSchemaTableName(), columnHandle, target);

return immediateVoidFuture();
}
Expand Down
9 changes: 9 additions & 0 deletions core/trino-main/src/main/java/io/trino/metadata/DisabledSystemSecurityMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,15 @@ public void tableRenamed(Session session, CatalogSchemaTableName sourceTable, Ca
@Override
public void tableDropped(Session session, CatalogSchemaTableName table) {}

@Override
public void columnCreated(Session session, CatalogSchemaTableName table, String column) {}

@Override
public void columnRenamed(Session session, CatalogSchemaTableName table, String oldName, String newName) {}

@Override
public void columnDropped(Session session, CatalogSchemaTableName table, String column) {}

Comment thread
codyzwief marked this conversation as resolved.
Outdated
private static TrinoException notSupportedException(String catalogName)
{
return new TrinoException(NOT_SUPPORTED, "Catalog does not support permission management: " + catalogName);
Expand Down
6 changes: 3 additions & 3 deletions core/trino-main/src/main/java/io/trino/metadata/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,12 +233,12 @@ Optional<TableExecuteHandle> getTableHandleForExecute(
/**
* Rename the specified column.
*/
void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target);
void renameColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle source, String target);

/**
* Add the specified column to the table.
*/
void addColumn(Session session, TableHandle tableHandle, ColumnMetadata column);
void addColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnMetadata column);

/**
* Set the specified type to the column.
Expand All @@ -253,7 +253,7 @@ Optional<TableExecuteHandle> getTableHandleForExecute(
/**
* Drop the specified column.
*/
void dropColumn(Session session, TableHandle tableHandle, ColumnHandle column);
void dropColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle column);

/**
* Drop the specified field from the column.
Expand Down
20 changes: 17 additions & 3 deletions core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,27 +735,41 @@ public void setColumnComment(Session session, TableHandle tableHandle, ColumnHan
}

@Override
public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target)
public void renameColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle source, String target)
{
CatalogHandle catalogHandle = tableHandle.getCatalogHandle();
CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, catalogHandle.getCatalogName());
Copy link
Copy Markdown
Member

@djsagain djsagain Apr 5, 2023

Choose a reason for hiding this comment

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

Hyper-nit - - since it's only used only in the if stm, I would have been tempted to move the catalogMetadata value into the if stm here and in the other two methods.

Copy link
Copy Markdown
Contributor Author

@codyzwief codyzwief Apr 6, 2023

Choose a reason for hiding this comment

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

I thought about that too. I'd rather keep it as-is though, personally; it matches the style of other methods that notify system security metadata.

ConnectorMetadata metadata = getMetadataForWrite(session, catalogHandle);
metadata.renameColumn(session.toConnectorSession(catalogHandle), tableHandle.getConnectorHandle(), source, target.toLowerCase(ENGLISH));
if (catalogMetadata.getSecurityManagement() == SYSTEM) {
ColumnMetadata columnMetadata = getColumnMetadata(session, tableHandle, source);
systemSecurityMetadata.columnRenamed(session, table, columnMetadata.getName(), target);
}
}

@Override
public void addColumn(Session session, TableHandle tableHandle, ColumnMetadata column)
public void addColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnMetadata column)
{
CatalogHandle catalogHandle = tableHandle.getCatalogHandle();
CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, catalogHandle.getCatalogName());
ConnectorMetadata metadata = getMetadataForWrite(session, catalogHandle);
metadata.addColumn(session.toConnectorSession(catalogHandle), tableHandle.getConnectorHandle(), column);
if (catalogMetadata.getSecurityManagement() == SYSTEM) {
systemSecurityMetadata.columnCreated(session, table, column.getName());
}
}

@Override
public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle column)
public void dropColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle column)
{
CatalogHandle catalogHandle = tableHandle.getCatalogHandle();
CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, catalogHandle.getCatalogName());
ConnectorMetadata metadata = getMetadataForWrite(session, catalogHandle);
metadata.dropColumn(session.toConnectorSession(catalogHandle), tableHandle.getConnectorHandle(), column);
if (catalogMetadata.getSecurityManagement() == SYSTEM) {
ColumnMetadata columnMetadata = getColumnMetadata(session, tableHandle, column);
systemSecurityMetadata.columnDropped(session, table, columnMetadata.getName());
}
}

@Override
Expand Down
15 changes: 15 additions & 0 deletions core/trino-main/src/main/java/io/trino/metadata/SystemSecurityMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,19 @@ public interface SystemSecurityMetadata
* A table or view was dropped
*/
void tableDropped(Session session, CatalogSchemaTableName table);

/**
* A column was created
*/
void columnCreated(Session session, CatalogSchemaTableName table, String column);

/**
* A column was renamed
*/
void columnRenamed(Session session, CatalogSchemaTableName table, String oldName, String newName);

/**
* A column was dropped
*/
void columnDropped(Session session, CatalogSchemaTableName table, String column);
}
12 changes: 6 additions & 6 deletions core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,9 @@ public void renameTable(Session session, TableHandle tableHandle, CatalogSchemaT
}

@Override
public void addColumn(Session session, TableHandle tableHandle, ColumnMetadata column)
public void addColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnMetadata column)
{
SchemaTableName tableName = getTableName(tableHandle);
SchemaTableName tableName = table.getSchemaTableName();
ConnectorTableMetadata metadata = tables.get(tableName);

ImmutableList.Builder<ColumnMetadata> columns = ImmutableList.builderWithExpectedSize(metadata.getColumns().size() + 1);
Expand All @@ -341,9 +341,9 @@ public void addColumn(Session session, TableHandle tableHandle, ColumnMetadata c
}

@Override
public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle columnHandle)
public void dropColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle columnHandle)
{
SchemaTableName tableName = getTableName(tableHandle);
SchemaTableName tableName = table.getSchemaTableName();
ConnectorTableMetadata metadata = tables.get(tableName);
String columnName = ((TestingColumnHandle) columnHandle).getName();

Expand All @@ -354,9 +354,9 @@ public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle co
}

@Override
public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target)
public void renameColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle source, String target)
{
SchemaTableName tableName = getTableName(tableHandle);
SchemaTableName tableName = table.getSchemaTableName();
ConnectorTableMetadata metadata = tables.get(tableName);
String columnName = ((TestingColumnHandle) source).getName();

Expand Down
6 changes: 3 additions & 3 deletions core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,19 +295,19 @@ public void setColumnComment(Session session, TableHandle tableHandle, ColumnHan
}

@Override
public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target)
public void renameColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle source, String target)
{
throw new UnsupportedOperationException();
}

@Override
public void addColumn(Session session, TableHandle tableHandle, ColumnMetadata column)
public void addColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnMetadata column)
{
throw new UnsupportedOperationException();
}

@Override
public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle column)
public void dropColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle column)
{
throw new UnsupportedOperationException();
}
Expand Down
12 changes: 6 additions & 6 deletions core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,15 @@ public void setColumnType(Session session, TableHandle tableHandle, ColumnHandle
}

@Override
public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target)
public void renameColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle source, String target)
{
delegate.renameColumn(session, tableHandle, source, target);
delegate.renameColumn(session, tableHandle, table, source, target);
}

@Override
public void addColumn(Session session, TableHandle tableHandle, ColumnMetadata column)
public void addColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnMetadata column)
{
delegate.addColumn(session, tableHandle, column);
delegate.addColumn(session, tableHandle, table, column);
}

@Override
Expand All @@ -323,9 +323,9 @@ public void setTableAuthorization(Session session, CatalogSchemaTableName table,
}

@Override
public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle column)
public void dropColumn(Session session, TableHandle tableHandle, CatalogSchemaTableName table, ColumnHandle column)
{
delegate.dropColumn(session, tableHandle, column);
delegate.dropColumn(session, tableHandle, table, column);
}

@Override
Expand Down
18 changes: 18 additions & 0 deletions testing/trino-tests/src/test/java/io/trino/security/TestingSystemSecurityMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,4 +259,22 @@ public void tableRenamed(Session session, CatalogSchemaTableName sourceTable, Ca

@Override
public void tableDropped(Session session, CatalogSchemaTableName table) {}

@Override
public void columnCreated(Session session, CatalogSchemaTableName table, String column)
{
throw new UnsupportedOperationException();
}

@Override
public void columnRenamed(Session session, CatalogSchemaTableName table, String oldName, String newName)
{
throw new UnsupportedOperationException();
}

@Override
public void columnDropped(Session session, CatalogSchemaTableName table, String column)
{
throw new UnsupportedOperationException();
}
}