Repository: incubator-airflow Updated Branches: refs/heads/master 6613676d7 -> 6aca2c2d3
[AIRFLOW-836] Use POST and CSRF for state changing endpoints Closes #2054 from saguziel/aguziel-use-post Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/6aca2c2d Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/6aca2c2d Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/6aca2c2d Branch: refs/heads/master Commit: 6aca2c2d395952341ab1b201c59011920b5a5c77 Parents: 6613676 Author: Alex Guziel <[email protected]> Authored: Sun Feb 19 09:33:57 2017 +0100 Committer: Bolke de Bruin <[email protected]> Committed: Sun Feb 19 09:33:57 2017 +0100 ---------------------------------------------------------------------- airflow/www/templates/admin/master.html | 8 +++++ airflow/www/templates/airflow/dag.html | 4 +-- airflow/www/templates/airflow/dags.html | 4 +-- airflow/www/templates/airflow/query.html | 18 ++++++---- airflow/www/views.py | 10 +++--- tests/core.py | 50 ++++++++++++++++++++++++--- 6 files changed, 73 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/6aca2c2d/airflow/www/templates/admin/master.html ---------------------------------------------------------------------- diff --git a/airflow/www/templates/admin/master.html b/airflow/www/templates/admin/master.html index 9c6f61d..49660a6 100644 --- a/airflow/www/templates/admin/master.html +++ b/airflow/www/templates/admin/master.html @@ -38,6 +38,14 @@ alert('{{ hostname }}'); }); $('span').tooltip(); + + $.ajaxSetup({ + beforeSend: function(xhr, settings) { + if (!/^(GET|HEAD|OPTIONS|TRACE)$/i.test(settings.type) && !this.crossDomain) { + xhr.setRequestHeader("X-CSRFToken", "{{ csrf_token() }}"); + } + } + }); </script> {% endblock %} http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/6aca2c2d/airflow/www/templates/airflow/dag.html ---------------------------------------------------------------------- diff --git a/airflow/www/templates/airflow/dag.html b/airflow/www/templates/airflow/dag.html index 8a4793d..43fe8c5 100644 --- a/airflow/www/templates/airflow/dag.html +++ b/airflow/www/templates/airflow/dag.html @@ -32,7 +32,7 @@ {% if dag.parent_dag %} <span style='color:#AAA;'>SUBDAG: </span> <span> {{ dag.dag_id }}</span> {% else %} - <input id="pause_resume" dag_id="{{ dag.dag_id }}" type="checkbox" {{ "checked" if not dag.is_paused else "" }} data-toggle="toggle" data-size="mini"> + <input id="pause_resume" dag_id="{{ dag.dag_id }}" type="checkbox" {{ "checked" if not dag.is_paused else "" }} data-toggle="toggle" data-size="mini" method="post"> <span style='color:#AAA;'>DAG: </span> <span> {{ dag.dag_id }}</span> <small class="text-muted"> {{ dag.description }} </small> {% endif %} {% if root %} @@ -364,7 +364,7 @@ function updateQueryStringParameter(uri, key, value) { is_paused = 'false' } url = "{{ url_for('airflow.paused') }}" + '?is_paused=' + is_paused + '&dag_id=' + dag_id; - $.ajax(url); + $.post(url); }); </script> http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/6aca2c2d/airflow/www/templates/airflow/dags.html ---------------------------------------------------------------------- diff --git a/airflow/www/templates/airflow/dags.html b/airflow/www/templates/airflow/dags.html index 2cbd12e..59a8bd0 100644 --- a/airflow/www/templates/airflow/dags.html +++ b/airflow/www/templates/airflow/dags.html @@ -66,7 +66,7 @@ <!-- Column 2: Turn dag on/off --> <td> {% if dag_id in orm_dags %} - <input id="toggle-{{ dag_id }}" dag_id="{{ dag_id }}" type="checkbox" {{ "checked" if not orm_dags[dag_id].is_paused else "" }} data-toggle="toggle" data-size="mini"> + <input id="toggle-{{ dag_id }}" dag_id="{{ dag_id }}" type="checkbox" {{ "checked" if not orm_dags[dag_id].is_paused else "" }} data-toggle="toggle" data-size="mini" method="post"> {% endif %} </td> @@ -214,7 +214,7 @@ is_paused = 'false' } url = 'airflow/paused?is_paused=' + is_paused + '&dag_id=' + dag_id; - $.ajax(url); + $.post(url); }); }); $('#dags').dataTable({ http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/6aca2c2d/airflow/www/templates/airflow/query.html ---------------------------------------------------------------------- diff --git a/airflow/www/templates/airflow/query.html b/airflow/www/templates/airflow/query.html index abcc1af..0dbb070 100644 --- a/airflow/www/templates/airflow/query.html +++ b/airflow/www/templates/airflow/query.html @@ -29,12 +29,12 @@ {% block body %} <h2>Ad Hoc Query</h2> - <form method="get" id="query_form"> + <form method="post" id="query_form"> <div class="form-inline"> <input name="_csrf_token" type="hidden" value="{{ csrf_token() }}"> {{ form.conn_id }} - <input type="submit" class="btn btn-default" value="Run!"> - <input type="button" class="btn btn-default" value=".csv" id="csv"> + <input type="submit" class="btn btn-default" value="Run!" id="submit_without_csv"> + <input type="submit" class="btn btn-default" value=".csv" id="submit_with_csv"> <span id="results"></span><br> <div id='ace_container'> {{ form.sql(rows=10) }} @@ -71,12 +71,16 @@ }); $('select').addClass("form-control"); sync(); - $("#query_form").submit(function(event){ + $("#submit_without_csv").submit(function(event){ $("#results").html("<img width='25'src='{{ url_for('static', filename='loading.gif') }}'>"); }); - $("#csv").on("click", function(){ - window.location += '&csv=true'; - }) + $("#submit_with_csv").click(function(){ + $("#csv_value").remove(); + $("#query_form").append('<input name="csv" type="hidden" value="true" id="csv_value">'); + }); + $("#submit_without_csv").click(function(){ + $("#csv_value").remove(); + }); }); </script> {% endblock %} http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/6aca2c2d/airflow/www/views.py ---------------------------------------------------------------------- diff --git a/airflow/www/views.py b/airflow/www/views.py index bda4921..8c9b2df 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1597,7 +1597,7 @@ class Airflow(BaseView): form=form, ) - @expose('/paused') + @expose('/paused', methods=['POST']) @login_required @wwwutils.action_logging def paused(self): @@ -1865,7 +1865,7 @@ class HomeView(AdminIndexView): class QueryView(wwwutils.DataProfilingMixin, BaseView): - @expose('/') + @expose('/', methods=['POST', 'GET']) @wwwutils.gzipped def query(self): session = settings.Session() @@ -1874,9 +1874,9 @@ class QueryView(wwwutils.DataProfilingMixin, BaseView): session.expunge_all() db_choices = list( ((db.conn_id, db.conn_id) for db in dbs if db.get_hook())) - conn_id_str = request.args.get('conn_id') - csv = request.args.get('csv') == "true" - sql = request.args.get('sql') + conn_id_str = request.form.get('conn_id') + csv = request.form.get('csv') == "true" + sql = request.form.get('sql') class QueryForm(Form): conn_id = SelectField("Layout", choices=db_choices) http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/6aca2c2d/tests/core.py ---------------------------------------------------------------------- diff --git a/tests/core.py b/tests/core.py index 4dbd7cc..c7160cb 100644 --- a/tests/core.py +++ b/tests/core.py @@ -1407,6 +1407,44 @@ class CliTests(unittest.TestCase): os.remove('variables1.json') os.remove('variables2.json') +class CSRFTests(unittest.TestCase): + def setUp(self): + configuration.load_test_config() + configuration.conf.set("webserver", "authenticate", "False") + configuration.conf.set("webserver", "expose_config", "True") + app = application.create_app() + app.config['TESTING'] = True + self.app = app.test_client() + + self.dagbag = models.DagBag( + dag_folder=DEV_NULL, include_examples=True) + self.dag_bash = self.dagbag.dags['example_bash_operator'] + self.runme_0 = self.dag_bash.get_task('runme_0') + + def get_csrf(self, response): + tree = html.fromstring(response.data) + form = tree.find('.//form') + + return form.find('.//input[@name="_csrf_token"]').value + + def test_csrf_rejection(self): + endpoints = ([ + "/admin/queryview/", + "/admin/airflow/paused?dag_id=example_python_operator&is_paused=false", + ]) + for endpoint in endpoints: + response = self.app.post(endpoint) + self.assertIn('CSRF token is missing', response.data.decode('utf-8')) + + def test_csrf_acceptance(self): + response = self.app.get("/admin/queryview/") + csrf = self.get_csrf(response) + response = self.app.post("/admin/queryview/", data=dict(csrf_token=csrf)) + self.assertEqual(200, response.status_code) + + def tearDown(self): + configuration.conf.set("webserver", "expose_config", "False") + self.dag_bash.clear(start_date=DEFAULT_DATE, end_date=datetime.now()) class WebUiTests(unittest.TestCase): def setUp(self): @@ -1415,6 +1453,7 @@ class WebUiTests(unittest.TestCase): configuration.conf.set("webserver", "expose_config", "True") app = application.create_app() app.config['TESTING'] = True + app.config['WTF_CSRF_METHODS'] = [] self.app = app.test_client() self.dagbag = models.DagBag(include_examples=True) @@ -1445,10 +1484,10 @@ class WebUiTests(unittest.TestCase): def test_query(self): response = self.app.get('/admin/queryview/') self.assertIn("Ad Hoc Query", response.data.decode('utf-8')) - response = self.app.get( - "/admin/queryview/?" - "conn_id=airflow_db&" - "sql=SELECT+COUNT%281%29+as+TEST+FROM+task_instance") + response = self.app.post( + "/admin/queryview/", data=dict( + conn_id="airflow_db", + sql="SELECT+COUNT%281%29+as+TEST+FROM+task_instance")) self.assertIn("TEST", response.data.decode('utf-8')) def test_health(self): @@ -1563,9 +1602,10 @@ class WebUiTests(unittest.TestCase): response = self.app.get( "/admin/airflow/refresh?dag_id=example_bash_operator") response = self.app.get("/admin/airflow/refresh_all") - response = self.app.get( + response = self.app.post( "/admin/airflow/paused?" "dag_id=example_python_operator&is_paused=false") + self.assertIn("OK", response.data.decode('utf-8')) response = self.app.get("/admin/xcom", follow_redirects=True) self.assertIn("Xcoms", response.data.decode('utf-8'))
