Skip to content

Commit

Permalink
[fix](auth)fix show load priv bug (apache#41723)
Browse files Browse the repository at this point in the history 
fix when user have `load_priv` on table,but can not show load about this
table

ps: We will add cases to other PRs as a
whole:apache#41802
  • Loading branch information
@zddr @morningman
zddr authored and morningman committed Oct 21, 2024
1 parent e73b6aa commit fda7ba0
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 71 deletions.
7 changes: 3 additions & 4 deletions fe/fe-core/src/main/java/org/apache/doris/cloud/load/CloudLoadManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,13 @@ public List<List<Comparable>> getLoadJobInfosByDb(long dbId, String labelValue,
}
// check auth
try {
checkJobAuth(loadJob.getDb().getCatalog().getName(), loadJob.getDb().getName(),
loadJob.getTableNames());
} catch (AnalysisException e) {
loadJob.checkAuth("show load");
} catch (DdlException e) {
continue;
}
// add load job info
loadJobInfos.add(loadJob.getShowInfo());
} catch (RuntimeException | DdlException | MetaNotFoundException e) {
} catch (RuntimeException | DdlException e) {
// ignore this load job
LOG.warn("get load job info failed. job id: {}", loadJob.getId(), e);
}
Expand Down
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/load/loadv2/BulkLoadJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ public static BulkLoadJob fromLoadStmt(LoadStmt stmt) throws DdlException {
bulkLoadJob.setComment(stmt.getComment());
bulkLoadJob.setJobProperties(stmt.getProperties());
bulkLoadJob.checkAndSetDataSourceInfo((Database) db, stmt.getDataDescriptions());
// In the construction method, there may not be table information yet
bulkLoadJob.rebuildAuthorizationInfo();
return bulkLoadJob;
} catch (MetaNotFoundException e) {
throw new DdlException(e.getMessage());
Expand Down Expand Up @@ -179,6 +181,10 @@ private AuthorizationInfo gatherAuthInfo() throws MetaNotFoundException {
return new AuthorizationInfo(database.getFullName(), getTableNames());
}

public void rebuildAuthorizationInfo() throws MetaNotFoundException {
this.authorizationInfo = gatherAuthInfo();
}

@Override
public Set<String> getTableNamesForShow() {
Optional<Database> db = Env.getCurrentInternalCatalog().getDb(dbId);
Expand Down
3 changes: 1 addition & 2 deletions fe/fe-core/src/main/java/org/apache/doris/load/loadv2/LoadJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ public void cancelJob(FailMsg failMsg) throws DdlException {
}
}

private void checkAuth(String command) throws DdlException {
public void checkAuth(String command) throws DdlException {
if (authorizationInfo == null) {
// use the old method to check priv
checkAuthWithoutAuthInfo(command);
Expand Down Expand Up @@ -747,7 +747,6 @@ public List<Comparable> getShowInfo() throws DdlException {

protected List<Comparable> getShowInfoUnderLock() throws DdlException {
// check auth
checkAuth("SHOW LOAD");
List<Comparable> jobInfo = Lists.newArrayList();
// jobId
jobInfo.add(id);
Expand Down
30 changes: 3 additions & 27 deletions fe/fe-core/src/main/java/org/apache/doris/load/loadv2/LoadManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
import org.apache.doris.common.Config;
import org.apache.doris.common.DataQualityException;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.LabelAlreadyUsedException;
import org.apache.doris.common.MetaNotFoundException;
import org.apache.doris.common.Pair;
Expand Down Expand Up @@ -635,14 +633,13 @@ public List<List<Comparable>> getLoadJobInfosByDb(long dbId, String labelValue,
}
// check auth
try {
checkJobAuth(loadJob.getDb().getCatalog().getName(), loadJob.getDb().getName(),
loadJob.getTableNames());
} catch (AnalysisException e) {
loadJob.checkAuth("show load");
} catch (DdlException e) {
continue;
}
// add load job info
loadJobInfos.add(loadJob.getShowInfo());
} catch (RuntimeException | DdlException | MetaNotFoundException e) {
} catch (RuntimeException | DdlException e) {
// ignore this load job
LOG.warn("get load job info failed. job id: {}", loadJob.getId(), e);
}
Expand All @@ -653,27 +650,6 @@ public List<List<Comparable>> getLoadJobInfosByDb(long dbId, String labelValue,
}
}

public void checkJobAuth(String ctlName, String dbName, Set<String> tableNames) throws AnalysisException {
if (tableNames.isEmpty()) {
if (!Env.getCurrentEnv().getAccessManager()
.checkDbPriv(ConnectContext.get(), ctlName, dbName,
PrivPredicate.LOAD)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_DB_ACCESS_DENIED_ERROR,
PrivPredicate.LOAD.getPrivs().toString(), dbName);
}
} else {
for (String tblName : tableNames) {
if (!Env.getCurrentEnv().getAccessManager()
.checkTblPriv(ConnectContext.get(), ctlName, dbName,
tblName, PrivPredicate.LOAD)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLE_ACCESS_DENIED_ERROR,
PrivPredicate.LOAD.getPrivs().toString(), tblName);
return;
}
}
}
}

public List<List<Comparable>> getAllLoadJobInfos() {
LinkedList<List<Comparable>> loadJobInfos = new LinkedList<List<Comparable>>();

Expand Down
38 changes: 0 additions & 38 deletions fe/fe-core/src/test/java/org/apache/doris/load/loadv2/LoadManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,12 @@
import org.apache.doris.catalog.Database;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.Table;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.FeMetaVersion;
import org.apache.doris.common.jmockit.Deencapsulation;
import org.apache.doris.datasource.InternalCatalog;
import org.apache.doris.meta.MetaContext;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.utframe.TestWithFeService;

import com.google.common.collect.Sets;
import mockit.Expectations;
import mockit.Injectable;
import mockit.Mocked;
Expand All @@ -44,8 +40,6 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -203,36 +197,4 @@ private LoadManager deserializeFromFile(File file) throws Exception {
loadManager.readFields(dis);
return loadManager;
}

@Test
public void testJobAuth() throws IOException, AnalysisException {
UserIdentity user1 = new UserIdentity("testJobAuthUser", "%");
user1.analyze();
new Expectations() {
{
ConnectContext.get();
minTimes = 0;
result = TestWithFeService.createCtx(user1, "%");
}
};
LoadManager manager = new LoadManager(new LoadJobScheduler());
HashSet<String> tableNames = Sets.newHashSet();
try {
// should check db auth
manager.checkJobAuth("ctl1", "db1", tableNames);
throw new RuntimeException("should exception");
} catch (AnalysisException e) {
Assert.assertTrue(e.getMessage().contains("Admin_priv,Load_priv"));
Assert.assertTrue(e.getMessage().contains("db1"));
}
tableNames.add("table1");
try {
// should check db auth
manager.checkJobAuth("ctl1", "db1", tableNames);
throw new RuntimeException("should exception");
} catch (AnalysisException e) {
Assert.assertTrue(e.getMessage().contains("Admin_priv,Load_priv"));
Assert.assertTrue(e.getMessage().contains("table1"));
}
}
}

0 comments on commit fda7ba0

Please sign in to comment.