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.

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

Allow Viewing DagRuns and TIs if a user has DAG "read" perms #20663

Merged
merged 2 commits into from Jan 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 2 additions & 10 deletions airflow/www/views.py
Expand Up @@ -3213,14 +3213,6 @@ def apply(self, query, func):
return query.filter(self.model.dag_id.in_(filter_dag_ids))


class DagEditFilter(BaseFilter):
"""Filter using DagIDs"""

def apply(self, query, func):
filter_dag_ids = current_app.appbuilder.sm.get_editable_dag_ids(g.user)
return query.filter(self.model.dag_id.in_(filter_dag_ids))


class AirflowModelView(ModelView):
"""Airflow Mode View."""

Expand Down Expand Up @@ -4078,7 +4070,7 @@ class DagRunModelView(AirflowPrivilegeVerifierModelView):

base_order = ('execution_date', 'desc')

base_filters = [['dag_id', DagEditFilter, lambda: []]]
base_filters = [['dag_id', DagFilter, lambda: []]]

edit_form = DagRunEditForm

Expand Down Expand Up @@ -4439,7 +4431,7 @@ class TaskInstanceModelView(AirflowPrivilegeVerifierModelView):

base_order = ('job_id', 'asc')

base_filters = [['dag_id', DagEditFilter, lambda: []]]
base_filters = [['dag_id', DagFilter, lambda: []]]

def log_url_formatter(self):
"""Formats log URL."""
Expand Down
23 changes: 20 additions & 3 deletions tests/www/views/test_views_dagrun.py
Expand Up @@ -15,6 +15,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import flask
import markupsafe
import pytest
import werkzeug

Expand Down Expand Up @@ -73,6 +75,21 @@ def reset_dagrun():
session.query(TaskInstance).delete()


def test_get_dagrun_can_view_dags_without_edit_perms(session, running_dag_run, client_dr_without_dag_edit):
"""Test that a user without dag_edit but with dag_read permission can view the records"""
assert session.query(DagRun).filter(DagRun.dag_id == running_dag_run.dag_id).count() == 1
resp = client_dr_without_dag_edit.get('/dagrun/list/', follow_redirects=True)

with client_dr_without_dag_edit.application.test_request_context():
url = flask.url_for(
'Airflow.graph', dag_id=running_dag_run.dag_id, execution_date=running_dag_run.execution_date
)
dag_url_link = markupsafe.Markup('<a href="{url}">{dag_id}</a>').format(
url=url, dag_id=running_dag_run.dag_id
)
check_content_in_response(dag_url_link, resp)


def test_create_dagrun_permission_denied(session, client_dr_without_dag_edit):
data = {
"state": "running",
Expand Down Expand Up @@ -103,7 +120,7 @@ def running_dag_run(session):
TaskInstance(dag.get_task("runme_1"), run_id=dr.run_id, state="failed"),
]
session.bulk_save_objects(tis)
session.flush()
session.commit()
return dr


Expand All @@ -114,12 +131,12 @@ def test_delete_dagrun(session, admin_client, running_dag_run):
assert session.query(DagRun).filter(DagRun.dag_id == running_dag_run.dag_id).count() == 0


def test_delete_dagrun_permission_denied(session, client_dr_without_dag_edit, running_dag_run):
def test_delete_dagrun_permission_denied(session, running_dag_run, client_dr_without_dag_edit):
composite_key = _get_appbuilder_pk_string(DagRunModelView, running_dag_run)

assert session.query(DagRun).filter(DagRun.dag_id == running_dag_run.dag_id).count() == 1
resp = client_dr_without_dag_edit.post(f"/dagrun/delete/{composite_key}", follow_redirects=True)
assert resp.status_code == 404 # If it doesn't fully succeed it gives a 404.
check_content_in_response(f"Access denied for dag_id {running_dag_run.dag_id}", resp)
assert session.query(DagRun).filter(DagRun.dag_id == running_dag_run.dag_id).count() == 1


Expand Down
4 changes: 3 additions & 1 deletion tests/www/views/test_views_tasks.py
Expand Up @@ -617,13 +617,15 @@ def test_task_instance_delete_permission_denied(session, client_ti_without_dag_e
task_id="test_task_instance_delete_permission_denied",
execution_date=timezone.utcnow(),
state=State.DEFERRED,
session=session,
)
session.commit()
composite_key = _get_appbuilder_pk_string(TaskInstanceModelView, task_instance_to_delete)
task_id = task_instance_to_delete.task_id

assert session.query(TaskInstance).filter(TaskInstance.task_id == task_id).count() == 1
resp = client_ti_without_dag_edit.post(f"/taskinstance/delete/{composite_key}", follow_redirects=True)
assert resp.status_code == 404 # If it doesn't fully succeed it gives a 404.
check_content_in_response(f"Access denied for dag_id {task_instance_to_delete.dag_id}", resp)
assert session.query(TaskInstance).filter(TaskInstance.task_id == task_id).count() == 1


Expand Down