This is an automated email from the ASF dual-hosted git repository.
pierrejeambrun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new a0b0c4dd6f8 Reuse stored credentials only when testing an unchanged
connection (#76453)
a0b0c4dd6f8 is described below
commit a0b0c4dd6f8cf1493a0214adbfd07f3997981433
Author: Pierre Jeambrun <[email protected]>
AuthorDate: Mon Nov 9 18:59:48 2026 +0100
Reuse stored credentials only when testing an unchanged connection (#76453)
* Reuse stored credentials only when testing an unchanged connection
Testing an existing connection reuses its stored password and extra so that
it can be tested without typing the secrets again. Only a different host or
port made the test require the caller's own credentials, but the connection
type, schema, login and extra also decide how and where a hook connects: an
HTTP schema can carry a full URL, and extra can set endpoints, proxies or TLS
options.
* Add a newsfragment for the connection test credential reuse change
* Compare connection fields through one normalised view when testing
Each field compared to decide whether stored credentials are reused now
goes through one place, so adding a field is a single entry rather than another
check.
---
airflow-core/newsfragments/76453.significant.rst | 16 ++++++++
.../core_api/routes/public/connections.py | 43 +++++++++++++---------
.../core_api/routes/public/test_connections.py | 35 +++++++++++++-----
3 files changed, 66 insertions(+), 28 deletions(-)
diff --git a/airflow-core/newsfragments/76453.significant.rst
b/airflow-core/newsfragments/76453.significant.rst
new file mode 100644
index 00000000000..f088f1b592e
--- /dev/null
+++ b/airflow-core/newsfragments/76453.significant.rst
@@ -0,0 +1,16 @@
+Testing an existing connection reuses its stored secrets only when the
connection is unchanged
+
+``POST /api/v2/connections/test`` with the ``connection_id`` of an existing
connection fills in the stored password and ``extra`` for the fields the
request leaves out or sends back masked. It now does so only when the request
keeps the stored connection type, host, port, schema, login and ``extra``. A
request that changes any of them, including a partial request that only sets
the changed fields, is rejected with a 400 unless it includes the password to
test, and is then tested with exa [...]
+
+Testing a saved connection from the UI is not affected.
+
+* Types of change
+
+ * [ ] Dag changes
+ * [ ] Config changes
+ * [x] API changes
+ * [ ] CLI changes
+ * [x] Behaviour changes
+ * [ ] Plugin changes
+ * [ ] Dependency changes
+ * [ ] Code interface changes
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
index 7fa0151bae8..92687bfdd26 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py
@@ -16,8 +16,9 @@
# under the License.
from __future__ import annotations
+import json
import os
-from typing import TYPE_CHECKING, Annotated
+from typing import TYPE_CHECKING, Annotated, Any
from fastapi import Depends, Header, HTTPException, Query, status
from fastapi.exceptions import RequestValidationError
@@ -107,23 +108,21 @@ def _ensure_executor_is_configured(executor: str | None)
-> None:
_MASKED_CREDENTIAL_SENTINEL = "***"
-def _same_endpoint(requested: str | int | None, stored: str | int | None) ->
bool:
+def _get_connection_target(conn: ConnectionBody | ConnectionResponse, fields:
set[str]) -> dict[str, Any]:
"""
- Return True when request and stored host/port refer to the same
destination.
+ Return the fields that decide where and how a hook connects, with blank
values as None.
- The UI sends empty string for hidden unused host/port fields; the ORM
stores
- those as NULL. Treat blank as unset so connection types that do not use
- host/port still reuse stored credentials.
+ The UI sends empty string for hidden unused fields; the ORM stores those
as NULL. Treat
+ blank as unset so connection types that do not use host/port still reuse
stored credentials.
Port arrives as ``0`` rather than blank, because the UI builds its body
with
``Number(connection.port)`` and ``Number(null)`` is ``0``. No connection
addresses
port 0, so it means the same thing as blank here and is normalised with it.
"""
-
- def _norm(value: str | int | None) -> str | int | None:
- return None if value in (None, "", 0) else value
-
- return _norm(requested) == _norm(stored)
+ target = {name: None if (value := getattr(conn, name)) in (None, "", 0)
else value for name in fields}
+ if "extra" in target:
+ target["extra"] = json.loads(conn.extra or "{}")
+ return target
def _supplies_own_credentials(test_body: ConnectionBody) -> bool:
@@ -389,16 +388,24 @@ def test_connection(
existing_conn = None
if existing_conn is not None:
- # Stored credentials are only reused to test the connection's own
- # host/port; testing a different destination must supply its own.
- fields_set = test_body.model_fields_set
- host_changed = "host" in fields_set and not
_same_endpoint(test_body.host, existing_conn.host)
- port_changed = "port" in fields_set and not
_same_endpoint(test_body.port, existing_conn.port)
- if host_changed or port_changed:
+ # Stored credentials are only reused to test the stored connection
itself;
+ # testing a different one must supply its own. Besides host and
port, the
+ # connection type, schema, login and extra also decide where and
how a hook
+ # connects (e.g. a URL in schema, or proxies and endpoints in
extra).
+ compared = {"host", "port", "schema_", "login", "extra"} &
test_body.model_fields_set
+ # conn_type goes first: a stored connection without one cannot be
read as a
+ # ConnectionResponse, and it differs from the request anyway.
+ if test_body.conn_type != existing_conn.conn_type or
_get_connection_target(
+ test_body, compared
+ ) != _get_connection_target(
+ # Compare with the stored connection as the API shows it, with
its secrets masked.
+ ConnectionResponse.model_validate(existing_conn),
+ compared,
+ ):
if not _supplies_own_credentials(test_body):
raise HTTPException(
status.HTTP_400_BAD_REQUEST,
- "The host or port to test differs from the stored
connection. "
+ "The connection to test differs from the stored
connection. "
"Include the credentials to test in the request body.",
)
existing_conn = None
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
index f53932d3a00..fde94cbb5b0 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
@@ -1268,7 +1268,7 @@ class TestConnection(TestConnectionEndpoint):
) as spy_secrets:
response = test_client.post(
"/connections/test",
- json={"connection_id": TEST_CONN_ID, "conn_type": "sqlite"},
+ json={"connection_id": TEST_CONN_ID, "conn_type":
TEST_CONN_TYPE},
)
assert response.status_code == 200
@@ -1353,7 +1353,7 @@ class TestConnection(TestConnectionEndpoint):
body = {
"connection_id": TEST_CONN_ID,
- "conn_type": "new_sqlite",
+ "conn_type": "sqlite",
"password": "***",
}
@@ -1364,8 +1364,7 @@ class TestConnection(TestConnectionEndpoint):
assert response.json()["status"] is True
# Verify that the existing password was used, not "***"
assert captured_value["password"] == "existing_password"
- # Verify that payload info were used for other fields
- assert captured_value["conn_type"] == "new_sqlite"
+ assert captured_value["conn_type"] == "sqlite"
# Verify DB was not mutated
session.expire_all()
@@ -1393,6 +1392,7 @@ class TestConnection(TestConnectionEndpoint):
body = {
"connection_id": TEST_CONN_ID,
"conn_type": "fs",
+ "password": "caller_password",
"extra": '{"path": "/", "new_key": "new_value"}',
}
@@ -1451,7 +1451,12 @@ class TestConnection(TestConnectionEndpoint):
captured_extra["value"] = self.extra
return True, "mocked"
- body = {"connection_id": TEST_CONN_ID, "conn_type": "sqlite", "extra":
'{"path": "***"}'}
+ body = {
+ "connection_id": TEST_CONN_ID,
+ "conn_type": "sqlite",
+ "password": "caller_password",
+ "extra": '{"path": "***"}',
+ }
with mock.patch.object(Connection, "test_connection",
mock_test_connection):
response = test_client.post("/connections/test", json=body)
@@ -1481,7 +1486,7 @@ class TestConnection(TestConnectionEndpoint):
"connection_id": TEST_CONN_ID,
"conn_type": "fs",
"password": "***",
- "extra": '{"path": "/", "new_key": "new_value"}',
+ "extra": '{"path": "/", "existing_key": "existing_value"}',
}
with mock.patch.object(Connection, "test_connection",
mock_test_connection):
@@ -1491,10 +1496,7 @@ class TestConnection(TestConnectionEndpoint):
assert response.json()["status"] is True
# Verify that the existing password was used, not "***"
assert captured_values["password"] == "existing_password"
- # Verify that new_key is reflected in the merged extra
- merged_extra = json.loads(captured_values["extra"])
- assert merged_extra["new_key"] == "new_value"
- assert merged_extra["path"] == "/"
+ assert json.loads(captured_values["extra"]) == {"path": "/",
"existing_key": "existing_value"}
# Verify DB was not mutated
session.expire_all()
@@ -1509,6 +1511,16 @@ class TestConnection(TestConnectionEndpoint):
[
pytest.param({"host": "other_host"}, id="host-changed"),
pytest.param({"host": "stored_host", "port": 9999},
id="port-changed"),
+ pytest.param({"conn_type": "http"}, id="conn-type-changed"),
+ pytest.param({"schema": "http://other_host:8080/path"},
id="schema-changed"),
+ pytest.param({"login": "other_login"}, id="login-changed"),
+ pytest.param(
+ {
+ "password": "***",
+ "extra": json.dumps({"api_key": "***", "proxies": {"http":
"http://other"}}),
+ },
+ id="extra-changed",
+ ),
],
)
def test_should_reject_test_when_target_overridden_without_credentials(
@@ -1520,7 +1532,9 @@ class TestConnection(TestConnectionEndpoint):
conn_type="sqlite",
host="stored_host",
port=1234,
+ login="stored_login",
password="existing_password",
+ extra=json.dumps({"api_key": "existing_key"}),
)
)
session.commit()
@@ -1568,6 +1582,7 @@ class TestConnection(TestConnectionEndpoint):
tested_connection = mock_test.call_args.args[0]
assert tested_connection.password == expected_password
+ @pytest.mark.enable_redact
@mock.patch.dict(os.environ, {"AIRFLOW__CORE__TEST_CONNECTION": "Enabled"})
def test_should_reuse_stored_extra_when_host_and_port_are_blank(self,
test_client, session):
"""Hidden unused host/port (None vs "") must not skip restoring masked
extra."""
