Skip to content

Commit

Permalink
Add Unimplemented methods and add distro dependency to trino-module
Browse files Browse the repository at this point in the history 
checkCanUpdateTableColumns
checkCanAlterColumn
  • Loading branch information
@aakashnand
aakashnand committed Jan 21, 2024
1 parent 1727d95 commit a19fe10
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 0 deletions.
13 changes: 13 additions & 0 deletions ...main/java/org/apache/ranger/authorization/trino/authorizer/RangerSystemAccessControl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,14 @@ public void checkCanRenameColumn(SystemSecurityContext context, CatalogSchemaTab
}
}

@Override
public void checkCanAlterColumn(SystemSecurityContext context, CatalogSchemaTableName table) {
RangerTrinoResource res = createResource(table);
if (!hasPermission(res, context, TrinoAccessType.ALTER)) {
LOG.debug("RangerSystemAccessControl.checkCanAlterColumn(" + table.getSchemaTableName().getTableName() + ") denied");
AccessDeniedException.denyAlterColumn(table.getSchemaTableName().getTableName());
}
}
/**
* This is evaluated on table level
*/
Expand Down Expand Up @@ -668,6 +676,11 @@ public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner) {
}
}

@Override
public void checkCanUpdateTableColumns(SystemSecurityContext securityContext, CatalogSchemaTableName table, Set<String> updatedColumnNames) {
SystemAccessControl.super.checkCanUpdateTableColumns(securityContext, table, updatedColumnNames);
}

/** FUNCTIONS **/

This comment has been minimized.

Sign in to view

Copy link
@okayhooni

okayhooni Apr 5, 2024 •
edited
Loading

I noticed this method is unimplemented today due not to perform MERGE INTO query on Iceberg format table with our Trino.

As you know, checkCanUpdateTableColumns() method of SPI base class ALWAYS deny the request.

How about using INSERT & DELETE permission check instead like below?

  @Override
  public void checkCanUpdateTableColumns(SystemSecurityContext securityContext, CatalogSchemaTableName table, Set<String> updatedColumnNames) {
    // SystemAccessControl.super.checkCanUpdateTableColumns(securityContext, table, updatedColumnNames); // ALWAYS DENY
    try {
      // USE INSERT AND DELETE PERMISSIONS INSTEAD
      checkCanInsertIntoTable(securityContext, table);
      checkCanDeleteFromTable(securityContext, table);
    } catch (AccessDeniedException ade) {
      AccessDeniedException.denyUpdateTableColumns(table.toString(), updatedColumnNames);
    }
  }

plus, I guess the stub of those missing methods have to be implemented also on the shim codes!

I made a PR for it :)
@Override
public boolean canExecuteFunction(SystemSecurityContext systemSecurityContext, CatalogSchemaRoutineName functionName) {
Expand Down
1 change: 1 addition & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@
<module>ranger-util</module>
<module>plugin-trino</module>
<module>ranger-trino-plugin-shim</module>
<module>distro</module>
</modules>
</profile>
<profile>
Expand Down

0 comments on commit a19fe10

Please sign in to comment.