Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Treat dag-defined access_control as authoritative if defined #33632

Merged
merged 6 commits into from
Aug 24, 2023
Merged

Treat dag-defined access_control as authoritative if defined #33632

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b7df390
Treat dag-defined access_control as authoritative only if it exists
SamWheating Aug 22, 2023
bdfe3e0
Add example DAG and documentation
SamWheating Aug 23, 2023
b29d7d0
Adding tests
SamWheating Aug 24, 2023
cce7b21
Doc fixes
SamWheating Aug 24, 2023
08bd3c0
formatting + new lines
SamWheating Aug 24, 2023
c607332
Fix syntax in docs
SamWheating Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions airflow/www/security.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,26 +681,12 @@ def sync_perm_for_dag(
dag_resource_name = permissions.resource_name_for_dag(dag_id)
for dag_action_name in self.DAG_ACTIONS:
self.create_permission(dag_action_name, dag_resource_name)

def _revoke_all_stale_permissions(resource: Resource):
existing_dag_perms = self.get_resource_permissions(resource)
for perm in existing_dag_perms:
non_admin_roles = [role for role in perm.role if role.name != "Admin"]
for role in non_admin_roles:
self.log.info(
"Revoking '%s' on DAG '%s' for role '%s'",
perm.action,
dag_resource_name,
role.name,
)
self.remove_permission_from_role(role, perm)

if access_control:

if access_control is not None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most important change in this PR - if access control will proceed into the following block if and only if access_control is a non-empty dict, whereas if access control is not None will proceed given any dict, including an empty one.

self.log.info("Syncing DAG-level permissions for DAG '%s'", dag_resource_name)
self._sync_dag_view_permissions(dag_resource_name, access_control)
else:
resource = self.get_resource(dag_resource_name)
if resource:
_revoke_all_stale_permissions(resource)
self.log.info("Not syncing DAG-level permissions for DAG '%s' as access control is unset.", dag_resource_name)

def _sync_dag_view_permissions(self, dag_id: str, access_control: dict[str, Collection[str]]) -> None:
"""
Expand Down
Loading