[airflow] branch v1-10-test updated: Mask Password in Log table when using the CLI (#11468)

[ka...@apache.org]

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

kaxilnaik pushed a commit to branch v1-10-test
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/v1-10-test by this push:
     new 3cd9e9f  Mask Password in Log table when using the CLI (#11468)
3cd9e9f is described below

commit 3cd9e9fb868547b6fb82fc84c78622daa22ca00f
Author: Kaxil Naik <ka...@gmail.com>
AuthorDate: Mon Oct 12 19:27:01 2020 +0100

    Mask Password in Log table when using the CLI (#11468)
    
    (cherry picked from commit 4e32546faf227a6497ce8b282fff7450cae6f665)
---
 airflow/utils/cli.py         | 13 +++++++++++-
 tests/utils/test_cli_util.py | 48 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/airflow/utils/cli.py b/airflow/utils/cli.py
index 9018407..31c8c62 100644
--- a/airflow/utils/cli.py
+++ b/airflow/utils/cli.py
@@ -99,9 +99,20 @@ def _build_metrics(func_name, namespace):
     :param namespace: Namespace instance from argparse
     :return: dict with metrics
     """
+    sensitive_fields = {'-p', '--password', '--conn-password'}
+    full_command = list(sys.argv)
+    for idx, command in enumerate(full_command):  # pylint: disable=too-many-nested-blocks
+        if command in sensitive_fields:
+            # For cases when password is passed as "--password xyz" (with space between key and value)
+            full_command[idx + 1] = "*" * 8
+        else:
+            # For cases when password is passed as "--password=xyz" (with '=' between key and value)
+            for sensitive_field in sensitive_fields:
+                if command.startswith('{}='.format(sensitive_field)):
+                    full_command[idx] = '{}={}'.format(sensitive_field, "*" * 8)
 
     metrics = {'sub_command': func_name, 'start_datetime': datetime.utcnow(),
-               'full_command': '{}'.format(list(sys.argv)), 'user': getpass.getuser()}
+               'full_command': '{}'.format(full_command), 'user': getpass.getuser()}
 
     assert isinstance(namespace, Namespace)
     tmp_dic = vars(namespace)
diff --git a/tests/utils/test_cli_util.py b/tests/utils/test_cli_util.py
index 6a411fa..fc7b771 100644
--- a/tests/utils/test_cli_util.py
+++ b/tests/utils/test_cli_util.py
@@ -17,14 +17,18 @@
 # specific language governing permissions and limitations
 # under the License.
 #
-
+import json
 import os
+import sys
 import unittest
 from argparse import Namespace
 from contextlib import contextmanager
 from datetime import datetime
 
+from parameterized import parameterized
+
 from airflow.utils import cli, cli_action_loggers
+from tests.compat import mock
 
 
 class CliUtilTest(unittest.TestCase):
@@ -71,6 +75,48 @@ class CliUtilTest(unittest.TestCase):
         with fail_action_logger_callback():
             success_func(Namespace())
 
+    @parameterized.expand(
+        [
+            (
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin --password test",
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin --password ********"
+            ),
+            (
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin -p test",
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin -p ********"
+            ),
+            (
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin --password=test",
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin --password=********"
+            ),
+            (
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin -p=test",
+                "airflow create_user -u test2 -l doe -f jon -e jdoe@apache.org -r admin -p=********"
+            ),
+            (
+                "airflow connections --add dsfs --conn-login asd --conn-password test --conn-type google",
+                "airflow connections --add dsfs --conn-login asd --conn-password ******** --conn-type google",
+            )
+        ]
+    )
+    def test_cli_create_user_supplied_password_is_masked(self, given_command, expected_masked_command):
+        args = given_command.split()
+
+        expected_command = expected_masked_command.split()
+
+        exec_date = datetime.utcnow()
+        namespace = Namespace(dag_id='foo', task_id='bar', subcommand='test', execution_date=exec_date)
+        with mock.patch.object(sys, "argv", args):
+            metrics = cli._build_metrics(args[1], namespace)
+
+        self.assertTrue(metrics.get('start_datetime') <= datetime.utcnow())
+
+        log = metrics.get('log')
+        command = json.loads(log.extra).get('full_command')  # type: str
+        # Replace single quotes to double quotes to avoid json decode error
+        command = json.loads(command.replace("'", '"'))
+        self.assertEqual(command, expected_command)
+
 
 @contextmanager
 def fail_action_logger_callback():