ramitkataria commented on code in PR #67293:
URL: https://github.com/apache/airflow/pull/67293#discussion_r3285161340


##########
airflow-core/src/airflow/api_fastapi/core_api/security.py:
##########
@@ -884,3 +902,34 @@ def _get_resource_methods_from_bulk_request(
     if action.action == BulkAction.CREATE and action.action_on_existence == 
BulkActionOnExistence.OVERWRITE:
         resource_methods.append("PUT")
     return resource_methods
+
+
+def _collect_teams_for_bulk_entity(
+    action: BulkCreateAction | BulkUpdateAction | BulkDeleteAction,
+    entity_team_name: str | None,
+    existing_team_name: str | None,
+) -> set[str | None]:
+    """
+    Collect the set of team names to authorize for a single entity in a bulk 
request.
+
+    For CREATE/UPDATE actions the caller must be authorized for the team_name 
specified in the
+    request body (if any). For UPDATE actions (or CREATE with overwrite) on 
existing resources,
+    the existing team is also checked so that a user cannot move a resource 
out of a team they
+    don't have access to.
+    """
+    if not conf.getboolean("core", "multi_team"):
+        return {None}
+
+    teams: set[str | None] = set()
+
+    if action.action == BulkAction.DELETE:
+        teams.add(existing_team_name)
+    elif action.action == BulkAction.UPDATE:
+        teams.add(existing_team_name)
+        teams.add(entity_team_name)
+    elif action.action == BulkAction.CREATE:
+        teams.add(entity_team_name)
+        if action.action_on_existence == BulkActionOnExistence.OVERWRITE:
+            teams.add(existing_team_name)

Review Comment:
   I think `existing_team_name` will always be `None` here. The callers build 
the team mapping dict from a list that's filtered with `if action.action != 
BulkAction.CREATE` (e.g., line 449 for pools), which excludes all CREATE 
actions, including CREATE-with-overwrite. So the .get() call that populates 
existing_team_name never finds the key.
   
   I believe the filters for pools, connections and variables need to also 
include CREATE-with-overwrite entities, something like:
   
   `if action.action != BulkAction.CREATE or action.action_on_existence == 
BulkActionOnExistence.OVERWRITE`



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

Reply via email to