mikebridge opened a new pull request, #44432:
URL: https://github.com/apache/superset/pull/44432

   ### SUMMARY
   
   Follow-up to #44416 and 
[SC-121388](https://app.shortcut.com/preset/story/121388), addressing 
[Richard's immediate follow-up request in his September 18, 18:21 UTC 
review](https://github.com/apache/superset/pull/44416#pullrequestreview-5251162269).
 Denied semantic-view chart saves and Explore form_data requests loaded 
provider metadata just to construct an access-denied payload. When that 
provider failed, the denial became a server error. Read the persisted 
datasource id directly in both denial builders so the request returns 403 
without contacting the provider. Table ids and denial disclosure protections 
remain unchanged.
   
   The new Gamma tests use actual permissions, including chart editorship for 
PUT, and a provider mocked to raise; they verify 403, zero provider calls and 
no saved changes.
   
   Deliberate scope deferral: chart save still accepts `semantic_view` when 
`SEMANTIC_LAYERS` is off, while semantic discovery returns 404. A consistent 
flag-off policy across save, Explore and existing-chart use is deferred; this 
change fixes denial rendering only. Query-context datasource identity is 
checked on create and query-context-only update; the full PUT path does not 
perform that binding check. This follow-up does not expand that contract.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Backend only. Before: all three tested denial paths returned 500 after 
provider metadata failure. After: 403 with no provider access.
   
   ### TESTING INSTRUCTIONS
   
   - 113 tests passed across semantic chart-save and complete chart API suites 
on isolated SQLite.
   - 262 security manager/permission-link, chart-command and Explore unit tests 
passed.
   - Red-first Gamma chart POST/PUT and form_data POST controls failed before 
the fix.
   - Required branch-scoped hooks passed, including MyPy, pylint and Ruff. 
Independent review approved the exact staged tree with zero findings.
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: SC-121388; follows #44416 and Richard's review.
   - [ ] Required feature flags: denial rendering is independent of 
feature-flag state.
   - [ ] Changes UI
   - [ ] Includes DB Migration
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   Codex authored; independent security-path review approved with zero 
findings. No live-provider/browser validation claimed.
   


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