sha174n opened a new pull request, #38648: URL: https://github.com/apache/superset/pull/38648
`QueryEstimationCommand.validate()` loaded a `Database` by integer ID without verifying the requesting user has access to it, allowing any authenticated `sql_lab` user to run EXPLAIN queries against any database by enumerating `database_id`. Add `security_manager.raise_for_access(database=self._database)` after the database-not-found guard in `validate()`. The resulting `SupersetSecurityException` propagates to Flask-AppBuilder's error handler which returns HTTP 403. <!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY <!--- Describe the change below, including rationale and design decisions --> Summary This PR adds a mandatory database authorization check to the SQL Lab query cost estimation command. Previously, the command loaded a database by its integer ID but did not verify if the requesting user had permission to access that database. This update ensures that estimation requests are subject to the same security policies as query execution. **Changes**: - Authorization Enforcement: Added security_manager.raise_for_access(database=self._database) to the validate() method of QueryEstimationCommand. - Validation Logic: The access check is performed immediately after the database is successfully retrieved, ensuring no metadata (such as row count estimates or query plans) is returned to unauthorized users. - Unit Testing: Added comprehensive unit tests in tests/unit_tests/commands/sql_lab/test_estimate.py covering database-not-found, access-denied, and successful-authorization scenarios. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> 1. Identify a Database ID that your current authenticated user cannot access in SQL Lab. 2. Submit an estimation request via the API using that ID: * Endpoint: POST /api/v1/sqllab/estimate/ * Payload: {"database_id": <unauthorized_id>, "sql": "SELECT 1"} 3. Verify the Response: - Confirm the request is blocked. The user should receive an error response (403 Forbidden) rather than the query plan or cost details. - Test again with a valid Database ID to ensure authorized users still receive correct estimates. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] 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 -- 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]
