sha174n opened a new pull request, #38647: URL: https://github.com/apache/superset/pull/38647
<!--- 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 --> This PR addresses a security vulnerability (IDOR) in the legacy /datasource endpoints where metadata could be disclosed to unauthorized users. It also closes a logic gap in the save endpoint that allowed users to bypass ownership checks by omitting specific fields from the request payload. **Changes**: - Access Control: Added security_manager.raise_for_access to the get, external_metadata, and external_metadata_by_name endpoints in Datasource view. - Ownership Enforcement: Refactored the save method to ensure security_manager.raise_for_ownership is always called before updates, regardless of whether the owners field is present in the request. - Legacy Hardening: While these endpoints are deprecated in favor of the V1 API, they remain active and are now properly secured against unauthorized metadata harvesting. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> To verify the fix for the reported IDOR: 1. Identify a Datasource ID that your current user does not have access to (e.g., owned by another user or in a private database). 2. Attempt to access the metadata via the legacy endpoint: `GET /datasource/get/table/<unauthorized_id>/` 3. Verify the response: - Before Fix: Returns the datasource metadata JSON. - After Fix: Returns a 403 Forbidden (or redirects to login if unauthenticated). 4. Test the Save Bypass: - Attempt a POST to /datasource/save/ with a payload containing the ID of a datasource you don't own, but omit the owners key from the JSON data. - Verify: The request should fail with a 403 Forbidden because ownership is now checked regardless of the payload structure. ### 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]
