Hide sensitive values from extra in connection edit form

airflow/providers_manager.py

@@ -223,6 +223,7 @@ class ConnectionFormWidgetInfo(NamedTuple):
     package_name: str
     field: Any
     field_name: str
+    is_sensitive: bool
 
 
 T = TypeVar("T", bound=Callable)
@@ -882,7 +883,12 @@ def _add_widgets(self, package_name: str, hook_class: type, widgets: dict[str, A
                 # In case of inherited hooks this might be happening several times
                 continue
             self._connection_form_widgets[prefixed_field_name] = ConnectionFormWidgetInfo(
-                hook_class.__name__, package_name, field, field_identifier
+                hook_class.__name__,
+                package_name,
+                field,
+                field_identifier,
+                hasattr(field.field_class.widget, "input_type")
+                and field.field_class.widget.input_type == "password",
             )
 
     def _add_customized_fields(self, package_name: str, hook_class: type, customized_fields: dict):

airflow/www/views.py

@@ -88,10 +88,12 @@
 from airflow.exceptions import (
     AirflowConfigException,
     AirflowException,
+    AirflowNotFoundException,
     ParamValidationError,
     RemovedInAirflow3Warning,
 )
 from airflow.executors.executor_loader import ExecutorLoader
+from airflow.hooks.base import BaseHook
 from airflow.jobs.job import Job
 from airflow.jobs.scheduler_job_runner import SchedulerJobRunner
 from airflow.jobs.triggerer_job_runner import TriggererJobRunner
@@ -147,6 +149,8 @@
     "} else {xLabel = d3.time.format('%H:%M, %d %b')(new Date(parseInt(d)));} return xLabel;}"
 )
 
+SENSITIVE_FIELD_PLACEHOLDER = "RATHER_LONG_SENSITIVE_FIELD_PLACEHOLDER"
+
 
 def sanitize_args(args: dict[str, str]) -> dict[str, str]:
     """
@@ -4643,14 +4647,22 @@ class ConnectionModelView(AirflowModelView):
 
     base_order = ("conn_id", "asc")
 
-    def _iter_extra_field_names(self) -> Iterator[tuple[str, str]]:
+    def _iter_extra_field_names_and_sensitivity(self) -> Iterator[tuple[str, str, bool]]:
         """Iterate through provider-backed connection fields.
 
         Note that this cannot be a property (including a cached property)
         because Flask-Appbuilder attempts to access all members on startup, and
         using a property would initialize the providers manager too eagerly.
+
+        Returns tuple of:
+
+        * key
+        * field_name
+        * whether the field is sensitive
         """
-        return ((k, v.field_name) for k, v in ProvidersManager().connection_form_widgets.items())
+        return (
+            (k, v.field_name, v.is_sensitive) for k, v in ProvidersManager().connection_form_widgets.items()
+        )
 
     @property
     def add_columns(self) -> list[str]:
@@ -4663,7 +4675,10 @@ def add_columns(self) -> list[str]:
         superfuluous checks done by Flask-Appbuilder on startup).
         """
         if self._add_columns is type(self)._add_columns and has_request_context():
-            self._add_columns = [*self._add_columns, *(k for k, _ in self._iter_extra_field_names())]
+            self._add_columns = [
+                *self._add_columns,
+                *(k for k, _, _ in self._iter_extra_field_names_and_sensitivity()),
+            ]
         return self._add_columns
 
     @property
@@ -4677,7 +4692,10 @@ def edit_columns(self) -> list[str]:
         superfuluous checks done by Flask-Appbuilder on startup).
         """
         if self._edit_columns is type(self)._edit_columns and has_request_context():
-            self._edit_columns = [*self._edit_columns, *(k for k, _ in self._iter_extra_field_names())]
+            self._edit_columns = [
+                *self._edit_columns,
+                *(k for k, _, _ in self._iter_extra_field_names_and_sensitivity()),
+            ]
         return self._edit_columns
 
     @action("muldelete", "Delete", "Are you sure you want to delete selected records?", single=False)
@@ -4766,7 +4784,6 @@ def process_form(self, form, is_created):
         """Process form data."""
         conn_id = form.data["conn_id"]
         conn_type = form.data["conn_type"]
-
         # The extra value is the combination of custom fields for this conn_type and the Extra field.
         # The extra form field with all extra values (including custom fields) is in the form being processed
         # so we start with those values, and override them with anything in the custom fields.
@@ -4791,8 +4808,7 @@ def process_form(self, form, is_created):
                 )
                 del form.extra
         del extra_json
-
-        for key, field_name in self._iter_extra_field_names():
+        for key, field_name, is_sensitive in self._iter_extra_field_names_and_sensitivity():
             if key in form.data and key.startswith("extra__"):
                 conn_type_from_extra_field = key.split("__")[1]
                 if conn_type_from_extra_field == conn_type:
@@ -4801,8 +4817,21 @@ def process_form(self, form, is_created):
                     # value isn't an empty string.
                     if value != "":
                         extra[field_name] = value
-
         if extra.keys():
+            sensitive_unchanged_keys = set()
+            for key, value in extra.items():
+                if value == SENSITIVE_FIELD_PLACEHOLDER:
+                    sensitive_unchanged_keys.add(key)
+            if sensitive_unchanged_keys:
+                try:
+                    conn = BaseHook.get_connection(conn_id)
+                except AirflowNotFoundException:
+                    conn = None
+                for key in sensitive_unchanged_keys:
+                    if conn and conn.extra_dejson.get(key):
+                        extra[key] = conn.extra_dejson.get(key)
+                    else:
+                        del extra[key]
             form.extra.data = json.dumps(extra)
 
     def prefill_form(self, form, pk):
@@ -4820,7 +4849,7 @@ def prefill_form(self, form, pk):
             logging.warning("extra field for %s is not a dictionary", form.data.get("conn_id", "<unknown>"))
             return
 
-        for field_key, field_name in self._iter_extra_field_names():
+        for field_key, field_name, is_sensitive in self._iter_extra_field_names_and_sensitivity():
             value = extra_dictionary.get(field_name, "")
 
             if not value:
@@ -4830,6 +4859,10 @@ def prefill_form(self, form, pk):
             if value:
                 field = getattr(form, field_key)
                 field.data = value
+            if is_sensitive and field_name in extra_dictionary:
+                extra_dictionary[field_name] = SENSITIVE_FIELD_PLACEHOLDER
+        # form.data is a property that builds the dictionary from fields so we have to modify the fields
+        form.extra.data = json.dumps(extra_dictionary)
 
 
 class PluginView(AirflowBaseView):

tests/www/views/test_views_connection.py

@@ -79,10 +79,29 @@ def test_prefill_form_null_extra():
     mock_form.data = {"conn_id": "test", "extra": None, "conn_type": "test"}
 
     cmv = ConnectionModelView()
-    cmv._iter_extra_field_names = mock.Mock(return_value=())
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(return_value=())
     cmv.prefill_form(form=mock_form, pk=1)
 
 
+def test_prefill_form_sensitive_fields_extra():
+    mock_form = mock.Mock()
+    mock_form.data = {
+        "conn_id": "test",
+        "extra": json.dumps({"sensitive_extra": "TEST1", "non_sensitive_extra": "TEST2"}),
+        "conn_type": "test",
+    }
+
+    cmv = ConnectionModelView()
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(
+        return_value=[("sensitive_extra_key", "sensitive_extra", True)]
+    )
+    cmv.prefill_form(form=mock_form, pk=1)
+    assert json.loads(mock_form.extra.data) == {
+        "sensitive_extra": "RATHER_LONG_SENSITIVE_FIELD_PLACEHOLDER",
+        "non_sensitive_extra": "TEST2",
+    }
+
+
 @pytest.mark.parametrize(
     "extras, expected",
     [
@@ -106,7 +125,9 @@ def test_prefill_form_backcompat(extras, expected):
     mock_form.data = {"conn_id": "test", "extra": json.dumps(extras), "conn_type": "test"}
 
     cmv = ConnectionModelView()
-    cmv._iter_extra_field_names = mock.Mock(return_value=[("extra__test__my_param", "my_param")])
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(
+        return_value=[("extra__test__my_param", "my_param", False)]
+    )
     cmv.prefill_form(form=mock_form, pk=1)
     assert mock_form.extra__test__my_param.data == expected
 
@@ -134,7 +155,9 @@ def test_process_form_extras_both(mock_pm_hooks, mock_import_str, field_name):
     }
 
     cmv = ConnectionModelView()
-    cmv._iter_extra_field_names = mock.Mock(return_value=[("extra__test__custom_field", field_name)])
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(
+        return_value=[("extra__test__custom_field", field_name, False)]
+    )
     cmv.process_form(form=mock_form, is_created=True)
     assert json.loads(mock_form.extra.data) == {
         field_name: "custom_field_val",
@@ -160,7 +183,7 @@ def test_process_form_extras_extra_only(mock_pm_hooks, mock_import_str):
     }
 
     cmv = ConnectionModelView()
-    cmv._iter_extra_field_names = mock.Mock(return_value=())
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(return_value=())
     cmv.process_form(form=mock_form, is_created=True)
     assert json.loads(mock_form.extra.data) == {"param2": "param2_val"}
 
@@ -186,10 +209,10 @@ def test_process_form_extras_custom_only(mock_pm_hooks, mock_import_str, field_n
     }
 
     cmv = ConnectionModelView()
-    cmv._iter_extra_field_names = mock.Mock(
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(
         return_value=[
-            ("extra__test3__custom_field", field_name),
-            ("extra__test3__custom_bool_field", False),
+            ("extra__test3__custom_field", field_name, False),
+            ("extra__test3__custom_bool_field", False, False),
         ],
     )
     cmv.process_form(form=mock_form, is_created=True)
@@ -217,7 +240,9 @@ def test_process_form_extras_updates(mock_pm_hooks, mock_import_str, field_name)
     }
 
     cmv = ConnectionModelView()
-    cmv._iter_extra_field_names = mock.Mock(return_value=[("extra__test4__custom_field", field_name)])
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(
+        return_value=[("extra__test4__custom_field", field_name, False)]
+    )
     cmv.process_form(form=mock_form, is_created=True)
 
     if field_name == "custom_field":
@@ -229,6 +254,36 @@ def test_process_form_extras_updates(mock_pm_hooks, mock_import_str, field_name)
         assert json.loads(mock_form.extra.data) == {"extra__test4__custom_field": "custom_field_val4"}
 
 
+@mock.patch("airflow.utils.module_loading.import_string")
+@mock.patch("airflow.providers_manager.ProvidersManager.hooks", new_callable=PropertyMock)
+@mock.patch("airflow.www.views.BaseHook")
+def test_process_form_extras_updates_sensitive_placeholder_unchanged(
+    mock_base_hook, mock_pm_hooks, mock_import_str
+):
+    """
+    Test the handling of sensitive unchanged field (where placeholder has not been modified).
+    """
+
+    # Testing parameters set in both extra and custom fields (connection updates).
+    mock_form = mock.Mock()
+    mock_form.data = {
+        "conn_type": "test4",
+        "conn_id": "extras_test4",
+        "extra": '{"sensitive_extra": "RATHER_LONG_SENSITIVE_FIELD_PLACEHOLDER", "extra__custom": "value"}',
+    }
+    mock_base_hook.get_connection.return_value = Connection(extra='{"sensitive_extra": "old_value"}')
+    cmv = ConnectionModelView()
+    cmv._iter_extra_field_names_and_sensitivity = mock.Mock(
+        return_value=[("sensitive_extra_key", "sensitive_extra", True)]
+    )
+    cmv.process_form(form=mock_form, is_created=True)
+
+    assert json.loads(mock_form.extra.data) == {
+        "extra__custom": "value",
+        "sensitive_extra": "old_value",
+    }
+
+
 def test_duplicate_connection(admin_client):
     """Test Duplicate multiple connection with suffix"""
     conn1 = Connection(