[GitHub] [incubator-superset] villebro commented on a change in pull request #10034: chore(security): Updating assert logic

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #10034:
URL: https://github.com/apache/incubator-superset/pull/10034#discussion_r443320839



##########
File path: superset/security/manager.py
##########
@@ -858,38 +852,65 @@ def set_perm(
                     )
                 )
 
-    def assert_datasource_permission(self, datasource: "BaseDatasource") -> None:
+    def raise_for_access(
+        self,
+        database: Optional["Database"] = None,
+        datasource: Optional["BaseDatasource"] = None,
+        query_context: Optional["QueryContext"] = None,
+        table: Optional["Table"] = None,
+        viz: Optional["BaseViz"] = None,
+    ) -> None:
         """
-        Assert the the user has permission to access the Superset datasource.
+        Raise an exception if the user cannot access the resource.
 
+        :param database: The Superset database (see table)
         :param datasource: The Superset datasource
-        :raises SupersetSecurityException: If the user does not have permission
+        :param query_context: The query context
+        :param table: The Superset table (see database)
+        :param viz: The visualization
+        :raises SupersetSecurityException: If the user cannot access the resource
         """
 
-        if not self.datasource_access(datasource):
-            raise SupersetSecurityException(
-                self.get_datasource_access_error_object(datasource),
+        from superset import db

Review comment:
       An optional approach here would be to define `DatasourceMixin` which provides a datasource, and add it to `BaseViz`, `QueryContext` and `Datasource`. in this case we could simplify the signature of this method by combining `viz`, `datasource` and `query_context` into one single `datasource: DatasourceMixin`. The same could be done to `query` and `database` (`DatabaseMixin`). This could help clarify what the diffefent arguments mean.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org