Commit c0c06d6
committed
[fix](auth) Add privilege checks to SHOW/EXPLAIN/REFRESH DICTIONARY
### What problem does this PR solve?
Issue Number: None
Related PR: #66218
Problem Summary:
`SHOW DICTIONARIES` and `EXPLAIN DICTIONARY` did not check any privilege. Any
user who can `USE` a database (which only needs a privilege on some table of
that database) could list every dictionary of the database together with its
source table name, status and BE data distribution, and describe its columns.
`REFRESH DICTIONARY` only failed inside the internal `INSERT INTO`, after the
dictionary had been looked up and switched to `LOADING`. Reading values through
`dict_get()` / `dict_get_many()` never checked any privilege at all.
This is inconsistent with `SHOW TABLES`, which hides tables the user cannot
show, and with `CREATE/DROP DICTIONARY`, which already require privileges on the
dictionary name (#66218).
Dictionaries are authorized like tables of the internal catalog, so:
- `SHOW DICTIONARIES` now skips dictionaries the user has no `SHOW` privilege
on, the same way `SHOW TABLES` filters tables.
- `EXPLAIN DICTIONARY` requires `SHOW` on the dictionary, like `DESCRIBE` on a
table.
- `REFRESH DICTIONARY` checks `LOAD` on the dictionary and column-aware
`SELECT` on the dictionary's source columns up front (`checkColumnsPriv` on
the source columns' own names, the same contract the generated `INSERT`
enforces through `CheckPrivileges`, so column-level grants keep working even
when the dictionary definition spells the columns differently). These are the
privileges the reload already required; the checks now happen before the
dictionary is resolved and before its status is flipped to `LOADING`, so an
unauthorized request can no longer block concurrent refreshes while its
INSERT is being planned.
- `dict_get()` / `dict_get_many()` require `SELECT` on the dictionary before
resolving it, so a user who cannot see a dictionary cannot read its values or
probe whether it exists.
- Statements that read a dictionary are kept out of the sql cache and are not
short-circuited as reusable point-query plans of a server prepared statement:
neither mechanism records the dictionary's privilege or version, so a cached
result / reused plan could survive a revoke or a refresh. Dictionaries are a
cache already, so the lost sql-cache hit is not a real cost.
- `SHOW DICTIONARIES` returns early when the visible set is empty: an empty id
list means "all dictionaries" to the BE status RPC (`get_dictionary_status`),
so the previous code fanned status RPCs to every alive BE (also reachable
before this change via an empty database or a non-matching `LIKE`).
The checks run before the dictionary is looked up, so a denied user cannot
probe whether a dictionary exists either.
### Release note
None
### Check List (For Author)
- Test
- [x] Regression test: `auth_call/test_ddl_dictionary_auth` now covers a
user with a privilege on another table of the database (must not see,
describe or refresh the dictionary), `SHOW_VIEW` on the database (sees
the dictionary and its source table, may describe it, still cannot
refresh), `LOAD` on the database without `SELECT` on the source table
(rejected before the dictionary starts loading), column-level `SELECT`
on the source columns (may refresh), and `dict_get()` denied until
`SELECT` on the dictionary is granted. New
`auth_call/test_dictionary_read_auth_cache` (nonConcurrent) covers a
dictionary spelling its columns differently from the source table, a
primed sql-cache statement that is denied right after the revoke, and a
server prepared point query whose second EXECUTE is denied after the
revoke.
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. Users without `SHOW` on a dictionary no longer see it in
`SHOW DICTIONARIES` and cannot `EXPLAIN DICTIONARY` it. `REFRESH
DICTIONARY` still needs `LOAD` on the dictionary and (column-aware)
`SELECT` on the source columns, but is now rejected before the dictionary
is touched. `dict_get()` / `dict_get_many()` now require `SELECT` on the
dictionary; internal paths without a user context are unaffected.
Statements reading a dictionary no longer use the sql cache or a reused
short-circuit point-query plan.
- Does this need documentation?
- [ ] No.
- [x] Yes. The privilege requirements of the three statements should be
documented.
Claude-Session: https://claude.ai/code/session_01X9KukfTLYxHmP6iYyEnQtW1 parent 4a9956e commit c0c06d6
9 files changed
Lines changed: 401 additions & 6 deletions
File tree
- fe/fe-core/src/main/java/org/apache/doris/nereids
- rules
- analysis
- rewrite
- trees
- expressions/functions/scalar
- plans/commands
- refresh
- regression-test/suites/auth_call
Lines changed: 7 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
| 81 | + | |
| 82 | + | |
81 | 83 | | |
82 | 84 | | |
83 | 85 | | |
| |||
588 | 590 | | |
589 | 591 | | |
590 | 592 | | |
| 593 | + | |
| 594 | + | |
591 | 595 | | |
592 | | - | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
593 | 599 | | |
594 | 600 | | |
595 | 601 | | |
| |||
Lines changed: 14 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| 29 | + | |
| 30 | + | |
29 | 31 | | |
30 | 32 | | |
31 | 33 | | |
| 34 | + | |
32 | 35 | | |
33 | 36 | | |
34 | 37 | | |
| |||
72 | 75 | | |
73 | 76 | | |
74 | 77 | | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
75 | 86 | | |
76 | 87 | | |
77 | 88 | | |
| |||
98 | 109 | | |
99 | 110 | | |
100 | 111 | | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
101 | 115 | | |
102 | | - | |
103 | 116 | | |
104 | 117 | | |
105 | 118 | | |
| |||
Lines changed: 13 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| 24 | + | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
| 28 | + | |
27 | 29 | | |
28 | 30 | | |
29 | 31 | | |
| |||
32 | 34 | | |
33 | 35 | | |
34 | 36 | | |
| 37 | + | |
35 | 38 | | |
36 | 39 | | |
37 | 40 | | |
| |||
93 | 96 | | |
94 | 97 | | |
95 | 98 | | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
96 | 109 | | |
97 | 110 | | |
98 | 111 | | |
| |||
Lines changed: 13 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
36 | 38 | | |
37 | 39 | | |
38 | 40 | | |
| 41 | + | |
39 | 42 | | |
40 | 43 | | |
41 | 44 | | |
| |||
99 | 102 | | |
100 | 103 | | |
101 | 104 | | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
102 | 115 | | |
103 | 116 | | |
104 | 117 | | |
| |||
Lines changed: 13 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| 21 | + | |
21 | 22 | | |
| 23 | + | |
22 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
23 | 28 | | |
24 | 29 | | |
| 30 | + | |
25 | 31 | | |
26 | 32 | | |
27 | 33 | | |
| |||
62 | 68 | | |
63 | 69 | | |
64 | 70 | | |
65 | | - | |
| 71 | + | |
66 | 72 | | |
67 | 73 | | |
68 | 74 | | |
69 | 75 | | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
70 | 82 | | |
71 | 83 | | |
72 | 84 | | |
| |||
Lines changed: 23 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| 27 | + | |
26 | 28 | | |
27 | 29 | | |
| 30 | + | |
28 | 31 | | |
29 | 32 | | |
30 | 33 | | |
| |||
73 | 76 | | |
74 | 77 | | |
75 | 78 | | |
| 79 | + | |
76 | 80 | | |
77 | | - | |
| 81 | + | |
78 | 82 | | |
79 | 83 | | |
80 | 84 | | |
81 | | - | |
82 | | - | |
| 85 | + | |
| 86 | + | |
83 | 87 | | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
84 | 104 | | |
85 | 105 | | |
86 | 106 | | |
| |||
Lines changed: 30 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
21 | 25 | | |
22 | 26 | | |
| 27 | + | |
23 | 28 | | |
24 | 29 | | |
25 | 30 | | |
26 | 31 | | |
27 | 32 | | |
28 | 33 | | |
29 | 34 | | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
30 | 39 | | |
31 | 40 | | |
32 | 41 | | |
| |||
45 | 54 | | |
46 | 55 | | |
47 | 56 | | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
48 | 65 | | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
49 | 79 | | |
50 | 80 | | |
51 | 81 | | |
| |||
0 commit comments