[fix](auth)Disable column auth temporarily (#23295)

- add config `enable_col_auth` to temporarily disable column permissions(because old/new planner has bug when select from view)
- Restore the old optimizer to the previous authentication method
- Support for new optimizer authentication(Legacy issue: When querying the view, the permissions of the base table will be authenticated. The view's own permissions should be authenticated and processed after the new optimizer is improved)
- fix: show grants for non-existent users
- fix: role:`admin` can not grant/revoke to/from user
This commit is contained in:
zhangdong
2023-08-24 23:37:06 +08:00
committed by GitHub
parent 966561a6ed
commit 6a4976921d
15 changed files with 67 additions and 90 deletions

View File

@ -2112,6 +2112,11 @@ public class Config extends ConfigBase {
"Whether to use mysql's bigint type to return Doris's largeint type"})
public static boolean use_mysql_bigint_for_largeint = false;
@ConfField(description = {
"是否开启列权限",
"Whether to enable col auth"})
public static boolean enable_col_auth = false;
@ConfField
public static boolean forbid_running_alter_job = false;

View File

@ -21,6 +21,7 @@ import org.apache.doris.catalog.AccessPrivilegeWithCols;
import org.apache.doris.catalog.Env;
import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.FeNameFormat;
@ -149,7 +150,7 @@ public class GrantStmt extends DdlStmt {
} else if (roles != null) {
for (int i = 0; i < roles.size(); i++) {
String originalRoleName = roles.get(i);
FeNameFormat.checkRoleName(originalRoleName, false /* can not be admin */, "Can not grant role");
FeNameFormat.checkRoleName(originalRoleName, true /* can be admin */, "Can not grant role");
roles.set(i, ClusterNamespace.getFullName(analyzer.getClusterName(), originalRoleName));
}
}
@ -181,7 +182,8 @@ public class GrantStmt extends DdlStmt {
public static void checkAccessPrivileges(
List<AccessPrivilegeWithCols> accessPrivileges) throws AnalysisException {
for (AccessPrivilegeWithCols access : accessPrivileges) {
if (!access.getAccessPrivilege().canHasColPriv() && !CollectionUtils.isEmpty(access.getCols())) {
if ((!access.getAccessPrivilege().canHasColPriv() || !Config.enable_col_auth) && !CollectionUtils
.isEmpty(access.getCols())) {
throw new AnalysisException(
String.format("%s do not support col auth.", access.getAccessPrivilege().name()));
}

View File

@ -133,7 +133,7 @@ public class RevokeStmt extends DdlStmt {
} else if (roles != null) {
for (int i = 0; i < roles.size(); i++) {
String originalRoleName = roles.get(i);
FeNameFormat.checkRoleName(originalRoleName, false /* can not be admin */, "Can not revoke role");
FeNameFormat.checkRoleName(originalRoleName, true /* can be admin */, "Can not revoke role");
roles.set(i, ClusterNamespace.getFullName(analyzer.getClusterName(), originalRoleName));
}
}

View File

@ -24,6 +24,7 @@ import org.apache.doris.analysis.CompoundPredicate.Operator;
import org.apache.doris.catalog.AggregateFunction;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.DatabaseIf;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.FunctionSet;
import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.PrimitiveType;
@ -44,6 +45,7 @@ import org.apache.doris.common.TreeNode;
import org.apache.doris.common.UserException;
import org.apache.doris.common.util.SqlUtils;
import org.apache.doris.common.util.ToSqlContext;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.rewrite.ExprRewriter;
import org.apache.doris.rewrite.mvrewrite.MVSelectFailedException;
@ -392,6 +394,13 @@ public class SelectStmt extends QueryStmt {
View view = (View) table;
view.getQueryStmt().getTables(analyzer, expandView, tableMap, parentViewNameSet);
} else {
// check auth
if (!Env.getCurrentEnv().getAccessManager()
.checkTblPriv(ConnectContext.get(), tblRef.getName(), PrivPredicate.SELECT)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR, "SELECT",
ConnectContext.get().getQualifiedUser(), ConnectContext.get().getRemoteIP(),
dbName + "." + tableName);
}
tableMap.put(table.getId(), table);
}
}

View File

@ -89,6 +89,9 @@ public class ShowGrantsStmt extends ShowStmt {
ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "GRANT");
}
}
if (userIdent != null && !Env.getCurrentEnv().getAccessManager().getAuth().doesUserExist(userIdent)) {
throw new AnalysisException(String.format("User: %s does not exist", userIdent));
}
}
@Override

View File

@ -350,6 +350,9 @@ public abstract class ExternalCatalog
@Nullable
@Override
public ExternalDatabase<? extends ExternalTable> getDbNullable(String dbName) {
if (StringUtils.isEmpty(dbName)) {
return null;
}
try {
makeSureInitialized();
} catch (Exception e) {

View File

@ -256,6 +256,9 @@ public class InternalCatalog implements CatalogIf<Database> {
@Nullable
@Override
public Database getDbNullable(String dbName) {
if (StringUtils.isEmpty(dbName)) {
return null;
}
if (fullNameToDb.containsKey(dbName)) {
return fullNameToDb.get(dbName);
} else {

View File

@ -663,7 +663,7 @@ public class Auth implements Writable {
// return true if user ident exist
private boolean doesUserExist(UserIdentity userIdent) {
public boolean doesUserExist(UserIdentity userIdent) {
return userManager.userIdentityExist(userIdent, false);
}

View File

@ -17,7 +17,10 @@
package org.apache.doris.nereids.rules.analysis;
import org.apache.doris.catalog.DatabaseIf;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.datasource.CatalogIf;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
@ -44,17 +47,31 @@ public class UserAuthentication extends OneAnalysisRuleFactory {
if (connectContext.getSessionVariable().isPlayNereidsDump()) {
return null;
}
String dbName = relation.getDatabase().getFullName();
String tableName = relation.getTable().getName();
if (!connectContext.getEnv().getAccessManager().checkTblPriv(connectContext, dbName,
TableIf table = relation.getTable();
if (table == null) {
return null;
}
String tableName = table.getName();
DatabaseIf db = table.getDatabase();
// when table inatanceof FunctionGenTable,db will be null
if (db == null) {
return null;
}
String dbName = db.getFullName();
CatalogIf catalog = db.getCatalog();
if (catalog == null) {
return null;
}
String ctlName = catalog.getName();
// TODO: 2023/7/19 checkColumnsPriv
if (!connectContext.getEnv().getAccessManager().checkTblPriv(connectContext, ctlName, dbName,
tableName, PrivPredicate.SELECT)) {
String message = ErrorCode.ERR_TABLEACCESS_DENIED_ERROR.formatErrorMsg("SELECT",
ConnectContext.get().getQualifiedUser(), ConnectContext.get().getRemoteIP(),
dbName + ": " + tableName);
ctlName + ": " + dbName + ": " + tableName);
throw new AnalysisException(message);
}
return null;
}
}

View File

@ -33,23 +33,15 @@ import org.apache.doris.analysis.SlotId;
import org.apache.doris.analysis.SlotRef;
import org.apache.doris.analysis.StatementBase;
import org.apache.doris.analysis.StorageBackend;
import org.apache.doris.analysis.TableName;
import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.ScalarType;
import org.apache.doris.catalog.Table;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.catalog.Type;
import org.apache.doris.catalog.external.ExternalTable;
import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.UserException;
import org.apache.doris.datasource.InternalCatalog;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.qe.CommonResultSet;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.ResultSet;
@ -61,10 +53,7 @@ import org.apache.doris.thrift.TQueryOptions;
import org.apache.doris.thrift.TRuntimeFilterMode;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@ -179,8 +168,6 @@ public class OriginalPlanner extends Planner {
insertStmt.prepareExpressions();
}
checkColumnPrivileges(singleNodePlan);
// TODO chenhao16 , no used materialization work
// compute referenced slots before calling computeMemLayout()
//analyzer.markRefdSlots(analyzer, singleNodePlan, resultExprs, null);
@ -319,66 +306,6 @@ public class OriginalPlanner extends Planner {
}
}
private void checkColumnPrivileges(PlanNode singleNodePlan) throws UserException {
if (ConnectContext.get() == null) {
return;
}
// 1. collect all columns from all scan nodes
List<ScanNode> scanNodes = Lists.newArrayList();
singleNodePlan.collect((PlanNode planNode) -> planNode instanceof ScanNode, scanNodes);
// catalog : <db.table : column>
Map<String, HashMultimap<TableName, String>> ctlToTableColumnMap = Maps.newHashMap();
for (ScanNode scanNode : scanNodes) {
if (!scanNode.needToCheckColumnPriv()) {
continue;
}
TupleDescriptor tupleDesc = scanNode.getTupleDesc();
TableIf table = tupleDesc.getTable();
if (table == null) {
continue;
}
TableName tableName = getFullQualifiedTableNameFromTable(table);
for (SlotDescriptor slotDesc : tupleDesc.getSlots()) {
if (!slotDesc.isMaterialized()) {
continue;
}
Column column = slotDesc.getColumn();
if (column == null) {
continue;
}
HashMultimap<TableName, String> tableColumnMap = ctlToTableColumnMap.get(tableName.getCtl());
if (tableColumnMap == null) {
tableColumnMap = HashMultimap.create();
ctlToTableColumnMap.put(tableName.getCtl(), tableColumnMap);
}
tableColumnMap.put(tableName, column.getName());
LOG.debug("collect column {} in {}", column.getName(), tableName);
}
}
// 2. check privs
// TODO: only support SELECT_PRIV now
PrivPredicate wanted = PrivPredicate.SELECT;
for (Map.Entry<String, HashMultimap<TableName, String>> entry : ctlToTableColumnMap.entrySet()) {
Env.getCurrentEnv().getAccessManager().checkColumnsPriv(ConnectContext.get().getCurrentUserIdentity(),
entry.getKey(), entry.getValue(), wanted);
}
}
private TableName getFullQualifiedTableNameFromTable(TableIf table) throws AnalysisException {
if (table instanceof Table) {
String dbName = ClusterNamespace.getNameFromFullName(((Table) table).getQualifiedDbName());
if (Strings.isNullOrEmpty(dbName)) {
throw new AnalysisException("failed to get db name from table " + table.getName());
}
return new TableName(InternalCatalog.INTERNAL_CATALOG_NAME, dbName, table.getName());
} else if (table instanceof ExternalTable) {
ExternalTable extTable = (ExternalTable) table;
return new TableName(extTable.getCatalog().getName(), extTable.getDbName(), extTable.getName());
} else {
throw new AnalysisException("table " + table.getName() + " is not internal or external table instance");
}
}
/**
* If there are unassigned conjuncts, returns a SelectNode on top of root that evaluate those conjuncts; otherwise
* returns root unchanged.

View File

@ -48,12 +48,15 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.junit.Assert;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import java.util.List;
import java.util.Map;
import java.util.Set;
// when `select` suppport `col auth`,will open ColumnPrivTest
@Disabled
public class ColumnPrivTest extends TestWithFeService {
private static Auth auth;
private static Env env;

View File

@ -34,6 +34,7 @@ import org.apache.doris.catalog.AccessPrivilegeWithCols;
import org.apache.doris.catalog.DomainResolver;
import org.apache.doris.catalog.Env;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
@ -153,6 +154,7 @@ public class AuthTest {
};
resolver = new MockDomainResolver(auth);
Config.enable_col_auth = true;
}
@Test

View File

@ -1,4 +0,0 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !show_grants1 --
'default_cluster:non_existent_user_1'@'%' \N \N \N \N \N \N \N \N \N

View File

@ -18,5 +18,11 @@ suite("test_account") {
// todo: test account management, such as role, user, grant, revoke ...
sql "show roles"
qt_show_grants1 "show grants for 'non_existent_user_1'"
try {
sql "show grants for 'non_existent_user_1'"
fail()
} catch (Exception e) {
log.info(e.getMessage())
assertTrue(e.getMessage().contains('not exist'))
}
}

View File

@ -28,6 +28,7 @@ suite("test_nereids_authentication", "query") {
}
sql "set enable_nereids_planner = true"
sql "set enable_fallback_to_original_planner = false"
def dbName = "nereids_authentication"
sql "DROP DATABASE IF EXISTS ${dbName}"
@ -57,7 +58,7 @@ suite("test_nereids_authentication", "query") {
fail()
} catch (Exception e) {
log.info(e.getMessage())
assertTrue(e.getMessage().contains('Permission denied'))
assertTrue(e.getMessage().contains('denied to user'))
}
}
@ -67,7 +68,7 @@ suite("test_nereids_authentication", "query") {
fail()
} catch (Exception e) {
log.info(e.getMessage())
assertTrue(e.getMessage().contains('Permission denied'))
assertTrue(e.getMessage().contains('denied to user'))
}
}