Allow PATCH role endpoint to remove permissions not included in PATCH call

airflow/api_connexion/endpoints/role_and_permission_endpoint.py

@@ -136,7 +136,7 @@ def patch_role(*, role_name: str, update_mask: UpdateMask = None) -> APIResponse
     if "permissions" in data:
         perms = [(item["action"]["name"], item["resource"]["name"]) for item in data["permissions"] if item]
         _check_action_and_resource(security_manager, perms)
-        security_manager.bulk_sync_roles([{"role": role_name, "perms": perms}])
+        security_manager.patch_role_permissions(role, perms)
     new_name = data.get("name")
     if new_name is not None and new_name != role.name:
         security_manager.update_role(role_id=role.id, name=new_name)

airflow/www/security.py

@@ -237,7 +237,6 @@ def bulk_sync_roles(self, roles):
             role_name = config["role"]
             perms = config["perms"]
             role = existing_roles.get(role_name) or self.add_role(role_name)
-
             for action_name, resource_name in perms:
                 perm = non_dag_perms.get((action_name, resource_name)) or self.create_permission(
                     action_name, resource_name
@@ -246,6 +245,34 @@ def bulk_sync_roles(self, roles):
                 if perm not in role.permissions:
                     self.add_permission_to_role(role, perm)
 
+    def patch_role_permissions(self, role: Role, permissions: list[tuple[str, str]]) -> None:
+        """
+        Compares existing role permissions against provided permissions
+        Removes permissions that don't exist in the provided permissions
+        Adds permissions that don't exist in the role's current permissions
+        :param role:
+        :param permissions:
+        """
+        existing_permissions = [(permission.action, permission.resource) for permission in role.permissions]
+        existing_permissions_set = set(
+            [(str(permission[0]), str(permission[1])) for permission in existing_permissions]
+        )
+
+        permissions_set = set([(str(permission[0]), str(permission[1])) for permission in permissions])
+
+        permissions_to_remove = existing_permissions_set.difference(permissions_set)
+        permissions_to_add = permissions_set.difference(existing_permissions_set)
+
+        for action, resource in permissions_to_remove:
+            permission = self.get_permission(action, resource)
+            if permission:
+                self.remove_permission_from_role(role, permission)
+
+        for action, resource in permissions_to_add:
+            permission = self.get_permission(action, resource)
+            if permission:
+                self.add_permission_to_role(role, permission)
+
     def delete_role(self, role_name):
         """
         Delete the given Role

tests/api_connexion/endpoints/test_role_and_permission_endpoint.py

@@ -379,6 +379,52 @@ def test_patch_should_update_correct_roles_permissions(self):
 
         assert len(self.app.appbuilder.sm.find_role("already_exists").permissions) == 0
 
+    def test_patch_should_remove_permissions_not_in_update(self):
+        create_role(self.app, "testrole")
+
+        response = self.client.patch(
+            "/api/v1/roles/testrole",
+            json={
+                "name": "testrole",
+                "actions": [
+                    {"action": {"name": "can_delete"}, "resource": {"name": "DAGs"}},
+                    {"action": {"name": "can_edit"}, "resource": {"name": "Users"}},
+                ],
+            },
+            environ_overrides={"REMOTE_USER": "test"},
+        )
+        assert response.status_code == 200
+
+        updated_permissions = self.app.appbuilder.sm.find_role("testrole").permissions
+        updated_permissions = sorted(updated_permissions, key=lambda x: x.resource.name)
+        assert len(updated_permissions) == 2
+        assert updated_permissions[0].resource.name == "DAGs"
+        assert updated_permissions[0].action.name == "can_delete"
+        assert updated_permissions[1].resource.name == "Users"
+        assert updated_permissions[1].action.name == "can_edit"
+
+        response = self.client.patch(
+            "/api/v1/roles/testrole",
+            json={
+                "name": "testrole",
+                "actions": [
+                    {"action": {"name": "can_edit"}, "resource": {"name": "DAGs"}},
+                    {"action": {"name": "can_edit"}, "resource": {"name": "Users"}},
+                ],
+            },
+            environ_overrides={"REMOTE_USER": "test"},
+        )
+
+        assert response.status_code == 200
+
+        updated_permissions = self.app.appbuilder.sm.find_role("testrole").permissions
+        updated_permissions = sorted(updated_permissions, key=lambda x: x.resource.name)
+        assert len(updated_permissions) == 2
+        assert updated_permissions[0].resource.name == "DAGs"
+        assert updated_permissions[0].action.name == "can_edit"
+        assert updated_permissions[1].resource.name == "Users"
+        assert updated_permissions[1].action.name == "can_edit"
+
     @pytest.mark.parametrize(
         "update_mask, payload, expected_name, expected_actions",
         [