mrhhsg commented on code in PR #67343:
URL: https://github.com/apache/doris/pull/67343#discussion_r3895245683
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowDictionariesCommand.java:
##########
@@ -73,14 +76,23 @@ public ShowResultSet doRun(ConnectContext ctx, StmtExecutor
executor) throws Ana
DictionaryManager dictionaryManager =
ctx.getEnv().getDictionaryManager();
List<Dictionary> queryDicts = Lists.newArrayList();
+ String dbName = ctx.getDatabase();
// getDictionaries() already have read lock
- Map<String, Dictionary> dbDictionaries =
dictionaryManager.getDictionaries(ctx.getDatabase());
+ Map<String, Dictionary> dbDictionaries =
dictionaryManager.getDictionaries(dbName);
for (Map.Entry<String, Dictionary> entry : dbDictionaries.entrySet()) {
String dictionaryName = entry.getKey();
// Apply wild condition filtering if wild pattern is provided
- if (wild == null || matcher.match(dictionaryName)) {
- queryDicts.add(entry.getValue());
+ if (wild != null && !matcher.match(dictionaryName)) {
+ continue;
}
+ // Dictionaries are authorized like tables of the internal
catalog. Hide the ones the user
+ // may not show, the same way SHOW TABLES hides tables, so the
source table name, status
+ // and data distribution are not exposed to users without
privileges on the dictionary.
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
InternalCatalog.INTERNAL_CATALOG_NAME,
Review Comment:
Fixed in 00993be: `SHOW DICTIONARIES` now returns before status collection
when the visible set is empty, so an all-hidden (or empty / non-matching
`LIKE`) result no longer triggers the empty-id-list "ALL" semantics of
`get_dictionary_status`, the per-BE fan-out, the misleading missing-dictionary
warnings, or a command failure on a bad BE response.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/refresh/RefreshDictionaryCommand.java:
##########
@@ -45,7 +50,24 @@ public RefreshDictionaryCommand(String dbName, String
dicName) {
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
DictionaryManager dictionaryManager =
ctx.getEnv().getDictionaryManager();
String db = dbName == null ? ctx.getDatabase() : dbName;
+ // The reload is an INSERT INTO the dictionary executed as the current
user, which requires
+ // LOAD on the dictionary (and SELECT on the source table). Check LOAD
up front so a user
+ // without it can neither probe whether the dictionary exists nor flip
it to LOADING.
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
InternalCatalog.INTERNAL_CATALOG_NAME,
+ db, dictionaryName, PrivPredicate.LOAD)) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"LOAD",
+ ctx.getQualifiedUser(), ctx.getRemoteIP(), db + ": " +
dictionaryName);
+ }
Dictionary dictionary = dictionaryManager.getDictionary(db,
dictionaryName);
+ // The reload also reads the source table as the current user.
dataLoad() publishes the
+ // LOADING status before that INSERT is analyzed, so reject a missing
SELECT here instead of
+ // letting an unauthorized request block concurrent refreshes until
planning fails.
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
dictionary.getSourceCtlName(),
Review Comment:
Fixed in 00993be: the source preflight now uses the column-aware contract —
it collects the dictionary's source column names and calls
`AccessControllerManager.checkColumnsPriv(..., SELECT)`, the same authorization
the generated INSERT applies after `BindSink` projects the source to the
dictionary schema. A caller with dictionary LOAD plus column-level `SELECT` on
exactly the dictionary's source columns is authorized again (native and Ranger
column policies alike), and a missing privilege is still rejected before
`LOADING` is published. Added a column-only regression case (`grant
SELECT_PRIV(id)/(username)` then `REFRESH` succeeds), and the negative cases
now assert status `NORMAL` and an unchanged `LastUpdateResult` to prove the
preflight, not the INSERT, rejected the request.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExplainDictionaryCommand.java:
##########
@@ -62,11 +68,17 @@ public ShowResultSetMetaData getMetaData() {
}
@Override
- public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor)
throws DdlException {
+ public ShowResultSet doRun(ConnectContext ctx, StmtExecutor executor)
throws DdlException, AnalysisException {
List<List<String>> rows = Lists.newArrayList();
DictionaryManager dictionaryManager =
ctx.getEnv().getDictionaryManager();
String db = dbName == null ? ctx.getDatabase() : dbName;
+ // Describing a dictionary exposes its schema, so require SHOW on it
like DESCRIBE on a table.
+ if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx,
InternalCatalog.INTERNAL_CATALOG_NAME,
Review Comment:
Fixed in 00993be: both `DictGet.customSignatureDict` and
`DictGetMany.customSignatureDict` now check `SELECT` on the dictionary
(internal catalog key, same as the commands) before
`DictionaryManager.getDictionary`, so an unauthorized caller gets the same
`SELECT command denied` whether or not the name exists — no existence or schema
probing, and nothing reaches `ExpressionTranslator`. Internal paths without a
`ConnectContext` are unaffected. Regression now covers: hidden user's
`dict_get` denied; still denied with db-level SHOW_VIEW/LOAD plus source column
grants; allowed and returning the value after `SELECT` on the database is
granted (a table-level grant on the dictionary name itself is impossible today
because GRANT validates table existence — the namespace issue tracked in
#67345).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
