[GitHub] [airflow] houqp commented on a change in pull request #10594: WIP: Add permissions for stable API

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

houqp commented on a change in pull request #10594:
URL: https://github.com/apache/airflow/pull/10594#discussion_r479838304



##########
File path: airflow/www/security.py
##########
@@ -521,6 +524,18 @@ def sync_roles(self):
         self.update_admin_perm_view()
         self.clean_perms()
 
+    def sync_resource_permissions(self):
+        """
+        Populates resource-based permissions.
+        """
+        resources = ['Pool', 'Connection', 'Dag', 'DagBag', 'DagRun',
+                     'DagCode', 'Log', 'Task', 'ImportError', 'Variable', 'XCom']
+        actions = ['can_create', 'can_read', 'can_edit', 'can_delete']

Review comment:
       some of the resources here like ImportError doesn't have edit/delete/create endpoints right?

##########
File path: airflow/www/security.py
##########
@@ -505,6 +507,7 @@ def sync_roles(self):
 
         :return: None.
         """
+        # breakpoint()

Review comment:
       is it expected to keep this comment here?

##########
File path: airflow/api_connexion/security.py
##########
@@ -37,3 +37,32 @@ def decorated(*args, **kwargs):
         return function(*args, **kwargs)
 
     return cast(T, decorated)
+
+
+def requires_access(permissions: Sequence[Tuple[str, str]]) -> Callable[[T], T]:

Review comment:
       should we combine @security.requires_access and @security.requires_authentication into one decorator instead? will there be a case where one would only use require_access without requires_authentication? this doesn't make sense semantically right?




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