Skip to content
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.

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

[fix](auth)Disable colauth #23295

Merged
merged 8 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Copy link
Contributor

Choose a reason for hiding this comment

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

master_only=true

"是否开启列权限",
"Whether to enable col auth"})
public static boolean enable_col_auth = false;

@ConfField
public static boolean forbid_running_alter_job = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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;
Expand Down Expand Up @@ -149,7 +150,7 @@ public void analyze(Analyzer analyzer) throws UserException {
} 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));
}
}
Expand Down Expand Up @@ -181,7 +182,8 @@ public void analyze(Analyzer analyzer) throws UserException {
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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
} 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));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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;
Expand All @@ -44,6 +45,7 @@
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;
Expand Down Expand Up @@ -392,6 +394,13 @@ public void getTables(Analyzer analyzer, boolean expandView, Map<Long, TableIf>
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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ public void analyze(Analyzer analyzer) throws AnalysisException {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ public String getResource() {
@Nullable
@Override
public ExternalDatabase<? extends ExternalTable> getDbNullable(String dbName) {
if (StringUtils.isEmpty(dbName)) {
return null;
}
try {
makeSureInitialized();
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ public List<Long> getDbIds() {
@Nullable
@Override
public Database getDbNullable(String dbName) {
if (StringUtils.isEmpty(dbName)) {
return null;
}
if (fullNameToDb.containsKey(dbName)) {
return fullNameToDb.get(dbName);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ private void grantInternal(UserIdentity userIdent, List<String> roles, boolean i


// return true if user ident exist
private boolean doesUserExist(UserIdentity userIdent) {
public boolean doesUserExist(UserIdentity userIdent) {
return userManager.userIdentityExist(userIdent, false);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -44,17 +47,31 @@ private Plan checkPermission(CatalogRelation relation, ConnectContext connectCon
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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,15 @@
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;
Expand All @@ -61,10 +53,7 @@
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;

Expand Down Expand Up @@ -179,8 +168,6 @@ public void createPlanFragments(StatementBase statement, Analyzer analyzer, TQue
insertStmt.prepareExpressions();
}

checkColumnPrivileges(singleNodePlan);

// TODO chenhao16 , no used materialization work
// compute referenced slots before calling computeMemLayout()
//analyzer.markRefdSlots(analyzer, singleNodePlan, resultExprs, null);
Expand Down Expand Up @@ -319,66 +306,6 @@ public void createPlanFragments(StatementBase statement, Analyzer analyzer, TQue
}
}

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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,15 @@
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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;
Expand Down Expand Up @@ -153,6 +154,7 @@ public void setUp() throws NoSuchMethodException, SecurityException {
};

resolver = new MockDomainResolver(auth);
Config.enable_col_auth = true;
}

@Test
Expand Down
4 changes: 0 additions & 4 deletions regression-test/data/account_p0/test_account.out

This file was deleted.

8 changes: 7 additions & 1 deletion regression-test/suites/account_p0/test_account.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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'))
}
}

Expand All @@ -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'))
}
}

Expand Down
Loading