You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by as...@apache.org on 2021/03/19 15:06:14 UTC

[airflow] 12/42: Use `Lax` for `cookie_samesite` when empty string is passed (#14183)

This is an automated email from the ASF dual-hosted git repository.

ash pushed a commit to branch v2-0-test
in repository https://gitbox.apache.org/repos/asf/airflow.git

commit 7790b2f68831091b036eed9d7f88cbba80c7d425
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Thu Feb 11 00:20:40 2021 +0000

    Use `Lax` for `cookie_samesite` when empty string is passed (#14183)
    
    closes https://github.com/apache/airflow/issues/13971
    
    The value of `[webserver] cookie_samesite` was changed to `Lax` in >=2.0
    from `''` (empty string) in 1.10.x.
    
    This causes the following error for users migrating from 1.10.x to 2.0
    if the old airflow.cfg already exists.
    
    ```
    Traceback (most recent call last):
    File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
    File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1953, in full_dispatch_request
    return self.finalize_request(rv)
    File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 1970, in finalize_request
    response = self.process_response(response)
    File "/usr/local/lib/python3.9/site-packages/flask/app.py", line 2269, in process_response
    self.session_interface.save_session(self, ctx.session, response)
    File "/usr/local/lib/python3.9/site-packages/flask/sessions.py", line 379, in save_session
    response.set_cookie(
    File "/usr/local/lib/python3.9/site-packages/werkzeug/wrappers/base_response.py", line 468, in set_cookie
    dump_cookie(
    File "/usr/local/lib/python3.9/site-packages/werkzeug/http.py", line 1217, in dump_cookie
    raise ValueError("SameSite must be 'Strict', 'Lax', or 'None'.")
    ValueError: SameSite must be 'Strict', 'Lax', or 'None'.**
    ```
    
    This commit takes care of it by using `Lax` when the value is empty string (``)
    
    (cherry picked from commit 4336f4cfdbd843085672b8e49367cf1b9ab4a432)
---
 UPDATING.md           |  2 +-
 airflow/www/app.py    | 12 +++++++++++-
 tests/www/test_app.py |  6 ++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/UPDATING.md b/UPDATING.md
index 60dc211..d024d51 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -268,7 +268,7 @@ def execution_date_fn(execution_date, ds_nodash, dag):
 ### The default value for `[webserver] cookie_samesite` has been changed to `Lax`
 
 As [recommended](https://flask.palletsprojects.com/en/1.1.x/config/#SESSION_COOKIE_SAMESITE) by Flask, the
-`[webserver] cookie_samesite` has been changed to `Lax` from `None`.
+`[webserver] cookie_samesite` has been changed to `Lax` from `''` (empty string) .
 
 #### Changes to import paths
 
diff --git a/airflow/www/app.py b/airflow/www/app.py
index 77b5a51..aa9b5ed 100644
--- a/airflow/www/app.py
+++ b/airflow/www/app.py
@@ -16,6 +16,7 @@
 # specific language governing permissions and limitations
 # under the License.
 #
+import warnings
 from datetime import timedelta
 from typing import Optional
 
@@ -79,7 +80,16 @@ def create_app(config=None, testing=False, app_name="Airflow"):
 
     flask_app.config['SESSION_COOKIE_HTTPONLY'] = True
     flask_app.config['SESSION_COOKIE_SECURE'] = conf.getboolean('webserver', 'COOKIE_SECURE')
-    flask_app.config['SESSION_COOKIE_SAMESITE'] = conf.get('webserver', 'COOKIE_SAMESITE')
+
+    cookie_samesite_config = conf.get('webserver', 'COOKIE_SAMESITE')
+    if cookie_samesite_config == "":
+        warnings.warn(
+            "Old deprecated value found for `cookie_samesite` option in `[webserver]` section. "
+            "Using `Lax` instead. Change the value to `Lax` in airflow.cfg to remove this warning.",
+            DeprecationWarning,
+        )
+        cookie_samesite_config = "Lax"
+    flask_app.config['SESSION_COOKIE_SAMESITE'] = cookie_samesite_config
 
     if config:
         flask_app.config.from_mapping(config)
diff --git a/tests/www/test_app.py b/tests/www/test_app.py
index b731db5..dddfb71 100644
--- a/tests/www/test_app.py
+++ b/tests/www/test_app.py
@@ -233,6 +233,12 @@ class TestApp(unittest.TestCase):
         app = application.cached_app(testing=True)
         assert app.config['PERMANENT_SESSION_LIFETIME'] == timedelta(minutes=3600)
 
+    @conf_vars({('webserver', 'cookie_samesite'): ''})
+    @mock.patch("airflow.www.app.app", None)
+    def test_correct_default_is_set_for_cookie_samesite(self):
+        app = application.cached_app(testing=True)
+        assert app.config['SESSION_COOKIE_SAMESITE'] == 'Lax'
+
 
 class TestFlaskCli(unittest.TestCase):
     def test_flask_cli_should_display_routes(self):