Kunal8954 opened a new pull request, #43994:
URL: https://github.com/apache/superset/pull/43994
### SUMMARY
Fix three interrelated bugs that caused a 403 error when a non-admin user
holding only `schema_access` (or `catalog_access`) on a query's schema tried to
create a chart from a SQL Lab query authored by a different user.
**Root cause:** The query branch of `raise_for_access()` lacked a terminal
`return`, so a cleared Query fell through to the generic `datasource=` branch
whose synthetic permission string could never match. Additionally, a 6.1.0
refactor added an `isinstance` gate in `can_access_schema()` that excluded
`Query` objects, and `Query.schema_perm` produced a non-canonical format that
never matched granted permissions.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
### TESTING INSTRUCTIONS
1. Register a database connection with a schema.
2. Create a Gamma + sql_lab role with `schema_access` on that schema only
(no `database_access`). Assign it to a test user.
3. Have a **different** user run a query in SQL Lab against that schema.
4. As the test user, open that query and click **Create chart**.
5. Before this fix: 403 error. After this fix: Explore opens.
Also run the existing security test suite:
```bash
pytest tests/unit_tests/security/ -x -v
pytest tests/unit_tests/explore/ -x -v
pytest tests/unit_tests/subjects/test_raise_for_access.py -x -v
```
### ADDITIONAL INFORMATION
- [x] Has associated issue: #43987
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### Changes
**`superset/security/manager.py`**
1. **`raise_for_access()` (line 4751):** Added `return` after the per-table
loop succeeds with no denied tables. Without this, the method fell through to
the generic `datasource=` branch which checks `Query.perm` — a synthetic string
no role is ever granted — and always raises 403.
2. **`can_access_schema()` (line 2255):** Widened the isinstance check from
`BaseDatasource` only to also accept any object with `database` and
`schema_perm` attributes. This undoes the 6.1.0 regression where the Explorable
refactor added a type gate that `Query` (which inherits from `ExploreMixin`,
not `BaseDatasource`) fails, causing `can_access_schema()` to always return
False for queries.
**`superset/models/sql_lab.py`**
3. **`Query.schema_perm` (line 373):** Changed from
`f"{database_name}.{schema}"` to delegate to
`security_manager.get_schema_perm()`, which produces the canonical bracketed
format `[database].[catalog].[schema]` that matches the permissions created by
`sync_permissions`.
--
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]
