CalvinKirs opened a new pull request, #66218:
URL: https://github.com/apache/doris/pull/66218

   ### What problem does this PR solve?
   
   Issue Number: close #xxx
   
   Related PR: #xxx
   
   Problem Summary:
   
   The Nereids path in `StmtExecutor` dispatches straight to `Command.run()`, 
so each command has to enforce its own privileges. A number of commands never 
got that check, and can currently be executed by any authenticated user 
regardless of their grants.
   
   This PR adds the missing checks:
   
   | Command | Required privilege |
   | --- | --- |
   | `AdminSetEncryptionRootKeyCommand`, `AdminRotateTdeRootKeyCommand` | 
`ADMIN`, like the other `ADMIN SET` statements |
   | `DropCatalogRecycleBinCommand` | `ADMIN` — it takes a raw object id and 
erases instantly, so it cannot be authorized at db/table level |
   | `CreateDictionaryCommand` | `CREATE` on the target database **and** 
`SELECT` on the source table, since the load task runs internally |
   | `DropDictionaryCommand` | `DROP` on the database |
   | `AddConstraintCommand`, `DropConstraintCommand` | `ALTER` on the table |
   | `WarmUpClusterCommand`, `CancelWarmUpJobCommand`, `DropStageCommand` | 
`ADMIN`, matching `CreateStageCommand` which already checked it |
   
   Two details worth a look during review:
   
   - `AddConstraintCommand`: for a foreign key the referenced table is checked 
as well, because adding the constraint registers a reverse reference on it.
   - `DropConstraintCommand`: the check is placed after the name-based 
fallback, so neither table-resolution path can skip it.
   
   ### Release note
   
   Fix missing privilege checks on `ADMIN SET ENCRYPTION ROOT KEY`, `ADMIN 
ROTATE TDE ROOT KEY`, `DROP CATALOG RECYCLE BIN`, `CREATE/DROP DICTIONARY`, 
`ALTER TABLE ADD/DROP CONSTRAINT`, `WARM UP CLUSTER`, `CANCEL WARM UP JOB` and 
`DROP STAGE`.
   
   ### Check List (For Author)
   
   - Test <!-- At least one of them must be included. -->
       - [x] Regression test
       - [ ] Unit Test
       - [ ] Manual test (add detailed scripts or steps below)
       - [ ] No need to test or manual test. Explain why:
           - [ ] This is a refactor/code format and no logic has been changed.
           - [ ] Previous test can cover this change.
           - [ ] No code files have been changed.
           - [ ] Other reason <!-- Add your reason?  -->
   
   New `auth_call` cases for constraint, dictionary and recycle bin, each 
covering both the denied and the granted path. The cloud-only commands (warm 
up, stage) are not covered by a regression case.
   
   - Behavior changed:
       - [ ] No.
       - [x] Yes. Users without the privileges listed above can no longer run 
these statements; they now get an access-denied error.
   
   - Does this need documentation?
       - [ ] No.
       - [x] Yes. <!-- Add document PR link here. eg: 
https://github.com/apache/doris-website/pull/1214 -->
   
   The privilege documentation for these statements should list the required 
privileges.
   


-- 
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]

Reply via email to