Skip to content

Commit

Permalink
[BugFix] Fix show table status not check view privilege (backport #53811
Browse files Browse the repository at this point in the history
) (#53899)

Co-authored-by: Harbor Liu <[email protected]>
  • Loading branch information
mergify[bot] and HangyuanLiu authored Dec 13, 2024
1 parent 0372389 commit ca834d0
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 10 deletions.
10 changes: 5 additions & 5 deletions fe/fe-core/src/main/java/com/starrocks/qe/ShowExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ public ShowResultSet visitShowTemporaryTablesStatement(ShowTemporaryTableStmt st
@Override
public ShowResultSet visitShowTableStatusStatement(ShowTableStatusStmt statement, ConnectContext context) {
List<List<String>> rows = Lists.newArrayList();
Database db = context.getGlobalStateMgr().getLocalMetastore().getDb(statement.getDb());
Database db = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb(statement.getDb());
ZoneId currentTimeZoneId = TimeUtils.getTimeZone().toZoneId();
if (db != null) {
Locker locker = new Locker();
Expand All @@ -593,8 +593,8 @@ public ShowResultSet visitShowTableStatusStatement(ShowTableStatusStmt statement
}

try {
Authorizer.checkAnyActionOnTable(context.getCurrentUserIdentity(),
context.getCurrentRoleIds(), new TableName(db.getFullName(), table.getName()));
Authorizer.checkAnyActionOnTableLikeObject(context.getCurrentUserIdentity(),
context.getCurrentRoleIds(), db.getFullName(), table);
} catch (AccessDeniedException e) {
continue;
}
Expand Down Expand Up @@ -1853,10 +1853,10 @@ public ShowResultSet visitShowRestoreStatement(ShowRestoreStmt statement, Connec

// restore info for external catalog
AbstractJob jobI = GlobalStateMgr.getCurrentState().getBackupHandler().getJob(-1L);
if (jobI != null && jobI instanceof RestoreJob) {
if (jobI != null && jobI instanceof RestoreJob) {
RestoreJob restoreJob = (RestoreJob) jobI;
List<String> info = restoreJob.getInfo();
infos.add(info);
infos.add(info);
}

return new ShowResultSet(statement.getMetaData(), infos);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ public static void checkAnyActionOnTableLikeObject(UserIdentity currentUser, Set

private static void doCheckTableLikeObject(UserIdentity currentUser, Set<Long> roleIds, String dbName,
BasicTable tbl, PrivilegeType privilegeType) throws AccessDeniedException {
if (tbl == null) {
return;
}

Table.TableType type = tbl.getType();
switch (type) {
case OLAP:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.starrocks.analysis.TableRef;
import com.starrocks.backup.AbstractJob;
import com.starrocks.backup.BackupJob;
import com.starrocks.catalog.BasicTable;
import com.starrocks.catalog.Database;
import com.starrocks.catalog.Function;
import com.starrocks.catalog.FunctionSearchDesc;
Expand Down Expand Up @@ -1630,7 +1631,7 @@ public Void visitCancelAlterTableStatement(CancelAlterTableStmt statement, Conne
Database db = GlobalStateMgr.getCurrentState().getLocalMetastore().getDb(statement.getDbName());
if (db != null) {
Table table = GlobalStateMgr.getCurrentState().getLocalMetastore()
.getTable(db.getFullName(), statement.getTableName());
.getTable(db.getFullName(), statement.getTableName());
if (table == null || !table.isMaterializedView()) {
// ignore privilege check for old mv
return null;
Expand Down Expand Up @@ -1681,7 +1682,10 @@ public Void visitDescTableStmt(DescribeStmt statement, ConnectContext context) {
@Override
public Void visitShowCreateTableStatement(ShowCreateTableStmt statement, ConnectContext context) {
try {
Authorizer.checkAnyActionOnTable(context.getCurrentUserIdentity(), context.getCurrentRoleIds(), statement.getTbl());
BasicTable basicTable = GlobalStateMgr.getCurrentState().getMetadataMgr().getBasicTable(
statement.getTbl().getCatalog(), statement.getTbl().getDb(), statement.getTbl().getTbl());
Authorizer.checkAnyActionOnTableLikeObject(context.getCurrentUserIdentity(), context.getCurrentRoleIds(),
statement.getDb(), basicTable);
} catch (AccessDeniedException e) {
AccessDeniedException.reportAccessDenied(
statement.getTbl().getCatalog(),
Expand Down Expand Up @@ -2153,7 +2157,7 @@ public Void visitBackupStatement(BackupStmt statement, ConnectContext context) {
externalCatalogs.forEach(externalCatalog -> {
try {
Authorizer.checkCatalogAction(context.getCurrentUserIdentity(), context.getCurrentRoleIds(),
externalCatalog.getCatalogName(), PrivilegeType.USAGE);
externalCatalog.getCatalogName(), PrivilegeType.USAGE);
} catch (AccessDeniedException e) {
AccessDeniedException.reportAccessDenied(
externalCatalog.getCatalogName(),
Expand Down Expand Up @@ -2279,7 +2283,7 @@ public Void visitRestoreStatement(RestoreStmt statement, ConnectContext context)
// check insert on specified table
for (TableRef tableRef : tableRefs) {
Table table = GlobalStateMgr.getCurrentState().getLocalMetastore()
.getTable(db.getFullName(), tableRef.getName().getTbl());
.getTable(db.getFullName(), tableRef.getName().getTbl());
if (table != null) {
try {
Authorizer.checkTableAction(context.getCurrentUserIdentity(), context.getCurrentRoleIds(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ public Void visitShowTemporaryTablesStatement(ShowTemporaryTableStmt node, Conne
return visitShowTableStatement(node, context);
}


@Override
public Void visitShowTabletStatement(ShowTabletStmt node, ConnectContext context) {
ShowTabletStmtAnalyzer.analyze(node, context);
Expand Down
1 change: 1 addition & 0 deletions fe/starrocks_intellij_style.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
</option>
</JavaCodeStyleSettings>
<codeStyleSettings language="JAVA">
<option name="RIGHT_MARGIN" value="130" />
<option name="KEEP_FIRST_COLUMN_COMMENT" value="false" />
<option name="KEEP_CONTROL_STATEMENT_IN_ONE_LINE" value="false" />
<option name="KEEP_BLANK_LINES_IN_DECLARATIONS" value="1" />
Expand Down
32 changes: 32 additions & 0 deletions test/sql/test_rbac/R/test_view_privilege
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
-- name: test_view_privilege
drop user if exists u1;
-- result:
-- !result
create user u1;
-- result:
-- !result
grant impersonate on user root to u1;
-- result:
-- !result
CREATE TABLE tbl1 (col1 INT, col2 INT, col3 INT);
-- result:
-- !result
CREATE VIEW view1 AS SELECT * FROM tbl1;
-- result:
-- !result
execute as u1 with no revert;
-- result:
-- !result
SHOW TABLE STATUS;
-- result:
-- !result
show create view view1;
-- result:
E: (5203, 'Access denied; you need (at least one of) the ANY privilege(s) on TABLE view1 for this operation. Please ask the admin to grant permission(s) or try activating existing roles using <set [default] role>. Current role(s): NONE. Inactivated role(s): NONE.')
-- !result
execute as root with no revert;
-- result:
-- !result
drop user u1;
-- result:
-- !result
14 changes: 14 additions & 0 deletions test/sql/test_rbac/T/test_view_privilege
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- name: test_view_privilege
drop user if exists u1;
create user u1;
grant impersonate on user root to u1;

CREATE TABLE tbl1 (col1 INT, col2 INT, col3 INT);
CREATE VIEW view1 AS SELECT * FROM tbl1;

execute as u1 with no revert;
SHOW TABLE STATUS;
show create view view1;

execute as root with no revert;
drop user u1;

0 comments on commit ca834d0

Please sign in to comment.