From f03f40fa7185d374fe0fa79b53bdcdfe200e3185 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Mon, 26 Aug 2024 16:48:58 +0200 Subject: [PATCH] Remove a set of deprecations in airflow.www module --- airflow/config_templates/config.yml | 4 +- airflow/www/app.py | 15 ++---- airflow/www/auth.py | 24 ---------- airflow/www/security.py | 46 ------------------- airflow/www/utils.py | 29 ------------ newsfragments/41758.significant.rst | 12 +++++ .../endpoints/test_forward_to_fab_endpoint.py | 2 +- tests/www/test_app.py | 5 +- tests/www/test_auth.py | 11 ----- tests/www/test_utils.py | 32 ------------- 10 files changed, 21 insertions(+), 159 deletions(-) delete mode 100644 airflow/www/security.py create mode 100644 newsfragments/41758.significant.rst diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index a458105d28798..77615e6e7d2ef 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1910,7 +1910,9 @@ webserver: default: "False" cookie_samesite: description: | - Set samesite policy on session cookie + Set samesite policy on session cookies. + As `recommended `_ + by Flask, the default is set to ``Lax`` and not a empty string. version_added: 1.10.3 type: string example: ~ diff --git a/airflow/www/app.py b/airflow/www/app.py index 93c4e91d6d2a7..d8d58d11f8ee8 100644 --- a/airflow/www/app.py +++ b/airflow/www/app.py @@ -17,7 +17,6 @@ # under the License. from __future__ import annotations -import warnings from datetime import timedelta from os.path import isabs @@ -30,7 +29,7 @@ from airflow import settings from airflow.api_internal.internal_api_call import InternalApiConfig from airflow.configuration import conf -from airflow.exceptions import AirflowConfigException, RemovedInAirflow3Warning +from airflow.exceptions import AirflowConfigException from airflow.logging_config import configure_logging from airflow.models import import_all_models from airflow.settings import _ENABLE_AIP_44 @@ -111,16 +110,8 @@ def create_app(config=None, testing=False): flask_app.config["SESSION_COOKIE_HTTPONLY"] = True flask_app.config["SESSION_COOKIE_SECURE"] = conf.getboolean("webserver", "COOKIE_SECURE") - 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.", - RemovedInAirflow3Warning, - stacklevel=2, - ) - cookie_samesite_config = "Lax" - flask_app.config["SESSION_COOKIE_SAMESITE"] = cookie_samesite_config + # Note: Ensure "Lax" is the default if config not specified + flask_app.config["SESSION_COOKIE_SAMESITE"] = conf.get("webserver", "COOKIE_SAMESITE") or "Lax" # Above Flask 2.0.x, default value of SEND_FILE_MAX_AGE_DEFAULT changed 12 hours to None. # for static file caching, it needs to set value explicitly. diff --git a/airflow/www/auth.py b/airflow/www/auth.py index f3f36e05e346b..47a06f52e94bc 100644 --- a/airflow/www/auth.py +++ b/airflow/www/auth.py @@ -18,7 +18,6 @@ import functools import logging -import warnings from functools import wraps from typing import TYPE_CHECKING, Callable, Sequence, TypeVar, cast @@ -39,7 +38,6 @@ VariableDetails, ) from airflow.configuration import conf -from airflow.exceptions import RemovedInAirflow3Warning from airflow.utils.net import get_hostname from airflow.www.extensions.init_auth_manager import get_auth_manager @@ -64,28 +62,6 @@ def get_access_denied_message(): return conf.get("webserver", "access_denied_message") -def has_access(permissions: Sequence[tuple[str, str]] | None = None) -> Callable[[T], T]: - """ - Check current user's permissions against required permissions. - - Deprecated. Do not use this decorator, use one of the decorator `has_access_*` defined in - airflow/www/auth.py instead. - This decorator will only work with FAB authentication and not with other auth providers. - - This decorator is widely used in user plugins, do not remove it. See - https://github.com/apache/airflow/pull/33213#discussion_r1346287224 - """ - warnings.warn( - "The 'has_access' decorator is deprecated. Please use one of the decorator `has_access_*`" - "defined in airflow/www/auth.py instead.", - RemovedInAirflow3Warning, - stacklevel=2, - ) - from airflow.providers.fab.auth_manager.decorators.auth import _has_access_fab - - return _has_access_fab(permissions) - - def has_access_with_pk(f): """ Check permissions on views. diff --git a/airflow/www/security.py b/airflow/www/security.py deleted file mode 100644 index c3db6969e1776..0000000000000 --- a/airflow/www/security.py +++ /dev/null @@ -1,46 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -from __future__ import annotations - -from deprecated import deprecated - -from airflow.exceptions import RemovedInAirflow3Warning -from airflow.providers.fab.auth_manager.security_manager.override import FabAirflowSecurityManagerOverride - -EXISTING_ROLES = { - "Admin", - "Viewer", - "User", - "Op", - "Public", -} - - -@deprecated( - reason="If you want to override the security manager, you should inherit from " - "`airflow.providers.fab.auth_manager.security_manager.override.FabAirflowSecurityManagerOverride` " - "instead", - category=RemovedInAirflow3Warning, -) -class AirflowSecurityManager(FabAirflowSecurityManagerOverride): - """ - Placeholder, just here to avoid breaking the code of users who inherit from this. - - Do not use if writing new code. - """ - - ... diff --git a/airflow/www/utils.py b/airflow/www/utils.py index 37c559055886f..f64c2e7fe8ae5 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -39,7 +39,6 @@ from sqlalchemy import delete, func, select, types from sqlalchemy.ext.associationproxy import AssociationProxy -from airflow.exceptions import RemovedInAirflow3Warning from airflow.models.dagrun import DagRun from airflow.models.dagwarning import DagWarning from airflow.models.errors import ParseImportError @@ -217,34 +216,6 @@ def check_dag_warnings(dag_id, session): flash(dag_warning.message, "warning") -def get_sensitive_variables_fields(): - import warnings - - from airflow.utils.log.secrets_masker import get_sensitive_variables_fields - - warnings.warn( - "This function is deprecated. Please use " - "`airflow.utils.log.secrets_masker.get_sensitive_variables_fields`", - RemovedInAirflow3Warning, - stacklevel=2, - ) - return get_sensitive_variables_fields() - - -def should_hide_value_for_key(key_name): - import warnings - - from airflow.utils.log.secrets_masker import should_hide_value_for_key - - warnings.warn( - "This function is deprecated. Please use " - "`airflow.utils.log.secrets_masker.should_hide_value_for_key`", - RemovedInAirflow3Warning, - stacklevel=2, - ) - return should_hide_value_for_key(key_name) - - def get_params(**kwargs): """Return URL-encoded params.""" return urlencode({d: v for d, v in kwargs.items() if v is not None}, True) diff --git a/newsfragments/41758.significant.rst b/newsfragments/41758.significant.rst new file mode 100644 index 0000000000000..f8b86d88564a5 --- /dev/null +++ b/newsfragments/41758.significant.rst @@ -0,0 +1,12 @@ +Removed deprecated functions and modules from ``airflow.www`` module. + +- Config flag default warning ``cookie_samesite`` option in section ``[webserver]`` removed. +- Legacy decorator ``@has_access`` in ``airflow.www.auth``: Please use one of the decorator ``has_access_*`` + defined in airflow/www/auth.py instead. +- Removed legacy modules ``airflow.www.security``: Should be inherited from + ``airflow.providers.fab.auth_manager.security_manager.override.FabAirflowSecurityManagerOverride`` instead. + The constant value ``EXISTING_ROLES`` should be used from ``airflow.www.security_manager`` module. +- Removed the method ``get_sensitive_variables_fields()`` from ``airflow.www.utils``: Please use + ``airflow.utils.log.secrets_masker.get_sensitive_variables_fields`` instead. +- Removed the method ``should_hide_value_for_key()`` from ``airflow.www.utils``: Please use + ``airflow.utils.log.secrets_masker.should_hide_value_for_key`` instead. diff --git a/tests/api_connexion/endpoints/test_forward_to_fab_endpoint.py b/tests/api_connexion/endpoints/test_forward_to_fab_endpoint.py index 292f026cfa05c..037e35914f74e 100644 --- a/tests/api_connexion/endpoints/test_forward_to_fab_endpoint.py +++ b/tests/api_connexion/endpoints/test_forward_to_fab_endpoint.py @@ -25,7 +25,7 @@ from airflow.security import permissions from airflow.utils import timezone from airflow.utils.session import create_session -from airflow.www.security import EXISTING_ROLES +from airflow.www.security_manager import EXISTING_ROLES from tests.test_utils.api_connexion_utils import create_role, create_user, delete_role, delete_user pytestmark = [pytest.mark.db_test, pytest.mark.skip_if_database_isolation_mode] diff --git a/tests/www/test_app.py b/tests/www/test_app.py index 7fd648675e7bf..053fd1174dcae 100644 --- a/tests/www/test_app.py +++ b/tests/www/test_app.py @@ -230,9 +230,8 @@ def test_should_set_permanent_session_timeout(self): @conf_vars({("webserver", "cookie_samesite"): ""}) @dont_initialize_flask_app_submodules def test_correct_default_is_set_for_cookie_samesite(self): - """An empty 'cookie_samesite' should be corrected to 'Lax' with a deprecation warning.""" - with pytest.deprecated_call(): - app = application.cached_app(testing=True) + """An empty 'cookie_samesite' should be corrected to 'Lax'.""" + app = application.cached_app(testing=True) assert app.config["SESSION_COOKIE_SAMESITE"] == "Lax" @pytest.mark.parametrize( diff --git a/tests/www/test_auth.py b/tests/www/test_auth.py index 513c8ac2cb067..613812968d27f 100644 --- a/tests/www/test_auth.py +++ b/tests/www/test_auth.py @@ -23,24 +23,13 @@ import airflow.www.auth as auth from airflow.auth.managers.models.resource_details import DagAccessEntity -from airflow.exceptions import RemovedInAirflow3Warning from airflow.models import Connection, Pool, Variable -from airflow.www.auth import has_access mock_call = Mock() pytestmark = pytest.mark.skip_if_database_isolation_mode -class TestHasAccessDecorator: - def test_has_access_decorator_raises_deprecation_warning(self): - with pytest.warns(RemovedInAirflow3Warning): - - @has_access - def test_function(): - pass - - @pytest.mark.parametrize( "decorator_name, is_authorized_method_name", [ diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py index f7470c8c68832..ea7aa6b96faef 100644 --- a/tests/www/test_utils.py +++ b/tests/www/test_utils.py @@ -358,38 +358,6 @@ def test_json_f_webencoder(self): assert formatter(dagrun) == expected_markup -@pytest.mark.filterwarnings("ignore::DeprecationWarning") -def test_get_sensitive_variables_fields(): - with pytest.warns(DeprecationWarning) as warning: - result = utils.get_sensitive_variables_fields() - - # assert deprecation warning - assert len(warning) == 1 - assert "This function is deprecated." in str(warning[-1].message) - - from airflow.utils.log.secrets_masker import get_sensitive_variables_fields - - expected_result = get_sensitive_variables_fields() - assert result == expected_result - - -@pytest.mark.filterwarnings("ignore::DeprecationWarning") -def test_should_hide_value_for_key(): - key_name = "key" - - with pytest.warns(DeprecationWarning) as warning: - result = utils.should_hide_value_for_key(key_name) - - # assert deprecation warning - assert len(warning) == 1 - assert "This function is deprecated." in str(warning[-1].message) - - from airflow.utils.log.secrets_masker import should_hide_value_for_key - - expected_result = should_hide_value_for_key(key_name) - assert result == expected_result - - class TestWrappedMarkdown: def test_wrapped_markdown_with_docstring_curly_braces(self): rendered = wrapped_markdown("{braces}", css_class="a_class")