codeant-ai-for-open-source[bot] commented on code in PR #42590:
URL: https://github.com/apache/superset/pull/42590#discussion_r3701476874
##########
tests/integration_tests/explore/form_data/commands_tests.py:
##########
@@ -345,3 +371,140 @@ def test_delete_form_data_command_key_expired(self,
mock_g):
response = delete_command.run()
assert response is False # noqa: E712
+
+ def
test_create_form_data_command_schema_access_no_all_datasource_access(self):
+ """
+ Regression for #39296: a user who has schema_access on the schema a
+ SQL Lab query ran in (but neither all_datasource_access nor
+ datasource_access on a specific registered dataset, since an ad-hoc
+ query result is never registered as one) should still be able to
+ jump straight from SQL Lab to "Create Chart" -- i.e.
+ CreateFormDataCommand.run() with datasource_type=QUERY should not be
+ blocked purely for lacking all_datasource_access, as long as the
+ query's schema is one they're granted schema_access on.
+
+ This exercises the same non-strict (force_dataset_match=False)
+ fallthrough in SupersetSecurityManager.raise_for_access that
+ test_raise_for_access_force_dataset_match_denies_schema_only (in
+ security_tests.py) exercises for the strict SQL Lab path -- here we
+ confirm the *non*-strict Explore/"Create Chart" path grants access on
+ schema_access alone, without needing a registered dataset at all.
+ """
+ schema = get_example_default_schema()
+ database = get_example_database()
+ # raise_for_access qualifies the query's tables against the
+ # database's default catalog (e.g. the Postgres database name),
+ # so the granted schema_access permission must be built the same
+ # way -- get_schema_perm() falls back to the plain [db].[schema]
+ # form when the backend (e.g. sqlite, mysql) doesn't support
+ # catalogs at all.
+ view_menu_name = security_manager.get_schema_perm(
+ database.database_name, database.get_default_catalog(), schema
+ )
+
+ security_manager.add_role(FORM_DATA_SCHEMA_ACCESS_ROLE)
+ db.session.commit()
+ _grant_schema_access(view_menu_name)
+ gamma_user = security_manager.find_user(username="gamma")
+ gamma_user.roles.append(
+ security_manager.find_role(FORM_DATA_SCHEMA_ACCESS_ROLE)
+ )
+ db.session.commit()
+
+ query = Query(
+ sql="SELECT * FROM wb_health_population",
+ client_id="fd_sch_acc1",
+ database=database,
+ schema=schema,
+ user_id=gamma_user.id,
Review Comment:
**Suggestion:** The schema-access test makes the query author and the
exploring user the same person. `raise_for_access` returns through the
query-author bypass before evaluating catalog, schema, or datasource
permissions, so this test can pass even if the schema-access fallthrough is
removed or broken. Use a different query author, or explicitly use a non-author
current user, so the test actually verifies schema-only authorization.
[incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Schema-access regression test can produce false positives.
- ⚠️ Permission fallthrough coverage is not independently verified.
- ⚠️ Future authorization regressions may pass CI unnoticed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cb0c86a41fbb480a827cbcf772b10911&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=cb0c86a41fbb480a827cbcf772b10911&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/explore/form_data/commands_tests.py
**Line:** 419:419
**Comment:**
*Incorrect Condition Logic: The schema-access test makes the query
author and the exploring user the same person. `raise_for_access` returns
through the query-author bypass before evaluating catalog, schema, or
datasource permissions, so this test can pass even if the schema-access
fallthrough is removed or broken. Use a different query author, or explicitly
use a non-author current user, so the test actually verifies schema-only
authorization.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42590&comment_hash=2a6a996d5deb670e27abe5ac5176c0a604947e4651ce1b44af77c74c71cf9834&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42590&comment_hash=2a6a996d5deb670e27abe5ac5176c0a604947e4651ce1b44af77c74c71cf9834&reaction=dislike'>👎</a>
--
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]
