jiangxt2 opened a new pull request, #12786:
URL: https://github.com/apache/gravitino/pull/12786

   ### What changes were proposed in this pull request?
   
   This PR prevents `LoadTableAuthorizationExecutor` from running the 
table-like existence probe after an explicit `MODIFY_TABLE` authorization 
request is denied. The executor records whether the request selected the 
secondary modify-table expression and returns the denial before evaluating the 
probe expression or calling `tableDispatcher().tableExists()`. Ordinary 
load-table denials retain their existing existence-probe behavior.
   
   The regression coverage uses the load-table authorization annotation shape 
and verifies the denial response, denial event, request failure state, REST 
invocation, dispatcher interaction, and probe authorization interaction. A 
separate `NEVER` condition fixture verifies that raw `MODIFY_TABLE` text does 
not suppress ordinary probing when the secondary expression is not active.
   
   This PR is intentionally prepared to be opened as a Draft PR.
   
   ### Why are the changes needed?
   
   When a write-aware client requests `MODIFY_TABLE`, the current Server path 
can fail that authorization and then call `tableExists()` to distinguish a 
missing table from an existing forbidden table. External JDBC catalogs perform 
metadata I/O during that existence check; `jdbc-doris` opens a MySQL-protocol 
connection before checking the table. A denied write can therefore contact the 
external catalog, incur connection and latency costs, and return an internal 
error if the catalog is unavailable instead of a deterministic authorization 
denial.
   
   This fixes Apache Gravitino Issue #12785. The change is generic Server 
authorization logic and does not add Doris, Spark, JDBC, public API, or 
catalog-specific behavior.
   
   Fix: #12785
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes. An explicitly denied `MODIFY_TABLE` request now returns `403 Forbidden` 
without an existence probe, including when the requested table does not exist. 
Such a request previously could probe the external catalog and return `404 Not 
Found` when the probe was allowed. Ordinary load/read denial behavior remains 
unchanged, and no configuration or public API is added.
   
   ### How was this patch tested?
   
   - `./gradlew :server:test --tests 
'org.apache.gravitino.server.web.filter.authorization.TestLoadTableAuthorizationExecutor'
 --tests 
'org.apache.gravitino.server.web.filter.TestGravitinoInterceptionService' 
-PskipITs -PskipDockerTests=true --console=plain --no-daemon`
   - `./gradlew :server:test -PskipITs -PskipDockerTests=true --console=plain 
--no-daemon`
   - `./gradlew :server:spotlessCheck`
   - `./gradlew rat`
   - `./gradlew :server:build -x test`
   - `git diff --check`
   
   The final Server suite completed with 344 tests across 50 suites, with zero 
skipped tests, failures, or errors. No Docker test is required for this 
Server-only change; the downstream Doris/Spark zero-I/O integration test 
remains tracked in the Doris governed read/write worktree and will run after 
this fix is merged and that worktree is rebased.
   


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

Reply via email to