codeant-ai-for-open-source[bot] commented on code in PR #42590:
URL: https://github.com/apache/superset/pull/42590#discussion_r3706547539


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

Review Comment:
   **Suggestion:** The schema-only regression is not isolated from the new 
authorship bypass because the test sets `query.user_id` to `gamma_user.id`, the 
same user used by `override_user`. The command can therefore succeed solely 
through query authorship even if schema access handling is broken. Use a 
different author or leave `user_id` unset, while retaining the schema grant, so 
this test actually verifies the schema-access path. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Schema-access regression test can pass through authorship.
   - ⚠️ Coverage does not isolate the intended permission path.
   - ⚠️ A future schema-check regression may go undetected.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=289fd9009d254ed3aed47be971b5ae5f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=289fd9009d254ed3aed47be971b5ae5f&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:** 418:418
   **Comment:**
        *Logic Error: The schema-only regression is not isolated from the new 
authorship bypass because the test sets `query.user_id` to `gamma_user.id`, the 
same user used by `override_user`. The command can therefore succeed solely 
through query authorship even if schema access handling is broken. Use a 
different author or leave `user_id` unset, while retaining the schema grant, so 
this test actually verifies the schema-access path.
   
   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=7ff007502df25aef3ee228bf13bcdeb3326cbfed61e56e85b6a0942a10808d2f&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42590&comment_hash=7ff007502df25aef3ee228bf13bcdeb3326cbfed61e56e85b6a0942a10808d2f&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]

Reply via email to