Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 43 additions & 13 deletions airflow/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ def default_config_yaml() -> List[Dict[str, Any]]:
return yaml.safe_load(config_file)


SENSITIVE_CONFIG_VALUES = {
('database', 'sql_alchemy_conn'),
('core', 'fernet_key'),
('celery', 'broker_url'),
('celery', 'flower_basic_auth'),
('celery', 'result_backend'),
('atlas', 'password'),
('smtp', 'smtp_password'),
('webserver', 'secret_key'),
# The following options are deprecated
('core', 'sql_alchemy_conn'),
}


class AirflowConfigParser(ConfigParser):
"""Custom Airflow Configparser supporting defaults and deprecated options"""

Expand All @@ -150,18 +164,8 @@ class AirflowConfigParser(ConfigParser):
# is to not store password on boxes in text files.
# These configs can also be fetched from Secrets backend
# following the "{section}__{name}__secret" pattern
sensitive_config_values: Set[Tuple[str, str]] = {
('database', 'sql_alchemy_conn'),
('core', 'fernet_key'),
('celery', 'broker_url'),
('celery', 'flower_basic_auth'),
('celery', 'result_backend'),
('atlas', 'password'),
('smtp', 'smtp_password'),
('webserver', 'secret_key'),
# The following options are deprecated
('core', 'sql_alchemy_conn'),
}

sensitive_config_values: Set[Tuple[str, str]] = SENSITIVE_CONFIG_VALUES

# A mapping of (new section, new option) -> (old section, old option, since_version).
# When reading new option, the old option will be checked to see if it exists. If it does a
Expand Down Expand Up @@ -887,6 +891,15 @@ def as_dict(
:return: Dictionary, where the key is the name of the section and the content is
the dictionary with the name of the parameter and its value.
"""
if not display_sensitive:
# We want to hide the sensitive values at the appropriate methods
# since envs from cmds, secrets can be read at _include_envs method
if not all([include_env, include_cmds, include_secret]):
raise ValueError(
"If display_sensitive is false, then include_env, "
"include_cmds, include_secret must all be set as True"
)

config_sources: ConfigSourcesType = {}
configs = [
('default', self.airflow_defaults),
Expand Down Expand Up @@ -922,6 +935,20 @@ def as_dict(
else:
self._filter_by_source(config_sources, display_source, self._get_secret_option)

if not display_sensitive:
# This ensures the ones from config file is hidden too
# if they are not provided through env, cmd and secret
hidden = '< hidden >'
for (section, key) in self.sensitive_config_values:
if not config_sources.get(section):
continue
if config_sources[section].get(key, None):
if display_source:
source = config_sources[section][key][1]
config_sources[section][key] = (hidden, source)
else:
config_sources[section][key] = hidden

return config_sources

def _include_secrets(
Expand Down Expand Up @@ -987,7 +1014,10 @@ def _include_envs(
log.warning("Ignoring unknown env var '%s'", env_var)
continue
if not display_sensitive and env_var != self._env_var_name('core', 'unit_test_mode'):
opt = '< hidden >'
# Don't hide cmd/secret values here
if not env_var.lower().endswith('cmd') and not env_var.lower().endswith("secret"):
opt = '< hidden >'

elif raw:
opt = opt.replace('%', '%%')
if display_source:
Expand Down
26 changes: 24 additions & 2 deletions airflow/www/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union
from urllib.parse import parse_qsl, unquote, urlencode, urlparse

import configupdater
import lazy_object_proxy
import markupsafe
import nvd3
Expand Down Expand Up @@ -3743,12 +3744,33 @@ def conf(self):
raw = request.args.get('raw') == "true"
title = "Airflow Configuration"
subtitle = AIRFLOW_CONFIG

expose_config = conf.get('webserver', 'expose_config')

# Don't show config when expose_config variable is False in airflow config
if conf.getboolean("webserver", "expose_config"):
# Don't show sensitive config values if expose_config variable is 'non-sensitive-only'
# in airflow config
if expose_config.lower() == 'non-sensitive-only':
from airflow.configuration import SENSITIVE_CONFIG_VALUES

updater = configupdater.ConfigUpdater()
updater.read(AIRFLOW_CONFIG)
for sect, key in SENSITIVE_CONFIG_VALUES:
if updater.has_option(sect, key):
updater[sect][key].value = '< hidden >'
config = str(updater)

table = [
(section, key, str(value), source)
for section, parameters in conf.as_dict(True, False).items()
for key, (value, source) in parameters.items()
]
elif expose_config.lower() in ['true', 't', '1']:

with open(AIRFLOW_CONFIG) as file:
config = file.read()
table = [
(section, key, value, source)
(section, key, str(value), source)
for section, parameters in conf.as_dict(True, True).items()
for key, (value, source) in parameters.items()
]
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ install_requires =
# Colorlog 6.x merges TTYColoredFormatter into ColoredFormatter, breaking backwards compatibility with 4.x
# Update CustomTTYColoredFormatter to remove
colorlog>=4.0.2, <5.0
configupdater>=3.1.1
connexion[swagger-ui,flask]>=2.10.0
cron-descriptor>=1.2.24
croniter>=0.3.17
Expand Down
34 changes: 34 additions & 0 deletions tests/core/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,31 @@ def test_config_from_secret_backend(self, mock_hvac):

assert 'sqlite:////Users/airflow/airflow/airflow.db' == test_conf.get('test', 'sql_alchemy_conn')

def test_hidding_of_sensitive_config_values(self):
test_config = '''[test]
sql_alchemy_conn_secret = sql_alchemy_conn
'''
test_config_default = '''[test]
sql_alchemy_conn = airflow
'''
test_conf = AirflowConfigParser(default_config=parameterized_config(test_config_default))
test_conf.read_string(test_config)
test_conf.sensitive_config_values = test_conf.sensitive_config_values | {
('test', 'sql_alchemy_conn'),
}

assert 'airflow' == test_conf.get('test', 'sql_alchemy_conn')
# Hide sensitive fields
asdict = test_conf.as_dict(display_sensitive=False)
assert '< hidden >' == asdict['test']['sql_alchemy_conn']
# If display_sensitive is false, then include_cmd, include_env,include_secrets must all be True
# This ensures that cmd and secrets env are hidden at the appropriate method and no surprises
with pytest.raises(ValueError):
test_conf.as_dict(display_sensitive=False, include_cmds=False)
# Test that one of include_cmds, include_env, include_secret can be false when display_sensitive
# is True
assert test_conf.as_dict(display_sensitive=True, include_cmds=False)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
@conf_vars(
{
Expand Down Expand Up @@ -559,6 +584,15 @@ def test_command_from_env(self):
# the environment variable's echo command
assert test_cmdenv_conf.get('testcmdenv', 'notacommand') == 'OK'

@pytest.mark.parametrize('display_sensitive, result', [(True, 'OK'), (False, '< hidden >')])
def test_as_dict_display_sensitivewith_command_from_env(self, display_sensitive, result):

test_cmdenv_conf = AirflowConfigParser()
test_cmdenv_conf.sensitive_config_values.add(('testcmdenv', 'itsacommand'))
with unittest.mock.patch.dict('os.environ'):
asdict = test_cmdenv_conf.as_dict(True, display_sensitive)
assert asdict['testcmdenv']['itsacommand'] == (result, 'cmd')

def test_parameterized_config_gen(self):
config = textwrap.dedent(
"""
Expand Down
75 changes: 75 additions & 0 deletions tests/www/views/test_views_configuration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# 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.

import html

from airflow.configuration import SENSITIVE_CONFIG_VALUES, conf
from tests.test_utils.config import conf_vars
from tests.test_utils.www import check_content_in_response, check_content_not_in_response


@conf_vars({("webserver", "expose_config"): 'False'})
def test_user_cant_view_configuration(admin_client):
resp = admin_client.get('configuration', follow_redirects=True)
check_content_in_response(
"Your Airflow administrator chose not to expose the configuration, "
"most likely for security reasons.",
resp,
)


@conf_vars({("webserver", "expose_config"): 'True'})
def test_user_can_view_configuration(admin_client):
resp = admin_client.get('configuration', follow_redirects=True)
for section, key in SENSITIVE_CONFIG_VALUES:
value = conf.get(section, key, fallback='')
if not value:
continue
check_content_in_response(html.escape(value), resp)


@conf_vars({("webserver", "expose_config"): 'non-sensitive-only'})
def test_configuration_redacted(admin_client):
resp = admin_client.get('configuration', follow_redirects=True)
for section, key in SENSITIVE_CONFIG_VALUES:
value = conf.get(section, key, fallback='')
if not value or value == 'airflow':
continue
if value.startswith('db+postgresql'): # this is in configuration comment
continue
Comment on lines 52 to 53

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is needed is it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. It's needed because an example of postgresql connection string was used in the comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then we should make the test better.

Instead of:

        check_content_not_in_response(value, resp)

something like this should test that the value isn't showing in the running config section

        check_content_not_in_response('<td>' + html.escape(value), resp)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added another test instead of removing this one because we also hide those in source files

check_content_not_in_response(value, resp)


@conf_vars({("webserver", "expose_config"): 'non-sensitive-only'})
def test_configuration_redacted_in_running_configuration(admin_client):
resp = admin_client.get('configuration', follow_redirects=True)
for section, key in SENSITIVE_CONFIG_VALUES:
value = conf.get(section, key, fallback='')
if not value or value == 'airflow':
continue
check_content_not_in_response("<td class='code'>" + html.escape(value) + '</td', resp)


@conf_vars({("webserver", "expose_config"): 'non-sensitive-only'})
@conf_vars({("database", "# sql_alchemy_conn"): 'testconn'})
@conf_vars({("core", " # secret_key"): 'core_secret'})
@conf_vars({("core", "fernet_key"): 'secret_fernet_key'})
def test_commented_out_config(admin_client):
resp = admin_client.get('configuration', follow_redirects=True)
check_content_in_response("testconn", resp)
check_content_in_response("core_secret", resp)
check_content_not_in_response("secret_fernet_key", resp)