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 59351cc0ae1 Revert "Fix editing connection with sensitive extra field"
(#53888)
59351cc0ae1 is described below
commit 59351cc0ae1ce24daec40fdbc70175f42d8bc7ec
Author: Pierre Jeambrun <[email protected]>
AuthorDate: Tue Jul 29 18:19:55 2025 +0200
Revert "Fix editing connection with sensitive extra field" (#53888)
* Revert "Fix editing connection with sensitive extra field (#52403)"
This reverts commit 8ef792dafef6e64faf5daa376bfa5238b1c14c87.
* Fix CI
---
.pre-commit-config.yaml | 1 +
.../api_fastapi/core_api/datamodels/connections.py | 25 ++++++-
.../ui/src/pages/Connections/ConnectionForm.tsx | 12 +---
.../airflow/ui/src/queries/useEditConnection.tsx | 4 +-
.../core_api/routes/public/test_connections.py | 81 ++++++++++++++++++----
5 files changed, 97 insertions(+), 26 deletions(-)
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 8bb7dd96119..2f144e403de 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1635,6 +1635,7 @@ repos:
^airflow-core/src/airflow/models/__init__\.py$|
^airflow-core/src/airflow/api/common/mark_tasks\.py$|
^airflow-core/src/airflow/api_fastapi/core_api/datamodels/assets\.py$|
+
^airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections\.py$|
^airflow-core/src/airflow/api_fastapi/core_api/datamodels/hitl\.py$|
^airflow-core/src/airflow/api_fastapi/core_api/datamodels/variables\.py$|
^airflow-core/src/airflow/api_fastapi/core_api/routes/ui/grid\.py$|
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
b/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
index f2ac1f7a940..2a6bd6bec8a 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py
@@ -17,12 +17,15 @@
from __future__ import annotations
+import json
from collections import abc
from typing import Annotated
-from pydantic import Field
+from pydantic import Field, field_validator
+from pydantic_core.core_schema import ValidationInfo
from airflow.api_fastapi.core_api.base import BaseModel, StrictBaseModel
+from airflow.sdk.execution_time.secrets_masker import redact
# Response Models
@@ -39,6 +42,26 @@ class ConnectionResponse(BaseModel):
password: str | None
extra: str | None
+ @field_validator("password", mode="after")
+ @classmethod
+ def redact_password(cls, v: str | None, field_info: ValidationInfo) -> str
| None:
+ if v is None:
+ return None
+ return str(redact(v, field_info.field_name))
+
+ @field_validator("extra", mode="before")
+ @classmethod
+ def redact_extra(cls, v: str | None) -> str | None:
+ if v is None:
+ return None
+ try:
+ extra_dict = json.loads(v)
+ redacted_dict = redact(extra_dict)
+ return json.dumps(redacted_dict)
+ except json.JSONDecodeError:
+ # we can't redact fields in an unstructured `extra`
+ return v
+
class ConnectionCollectionResponse(BaseModel):
"""Connection Collection serializer for responses."""
diff --git
a/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
b/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
index 9e4e38647ea..bf921a1de3c 100644
--- a/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Connections/ConnectionForm.tsx
@@ -57,7 +57,7 @@ const ConnectionForm = ({
const { conf: extra, setConf } = useParamStore();
const {
control,
- formState: { isDirty, isValid },
+ formState: { isValid },
handleSubmit,
reset,
watch,
@@ -94,14 +94,6 @@ const ConnectionForm = ({
mutateConnection(data);
};
- const hasChanges = () => {
- if (isDirty) {
- return true;
- }
-
- return JSON.stringify(JSON.parse(extra)) !==
JSON.stringify(JSON.parse(initialConnection.extra));
- };
-
const validateAndPrettifyJson = (value: string) => {
try {
const parsedJson = JSON.parse(value) as JSON;
@@ -242,7 +234,7 @@ const ConnectionForm = ({
<Spacer />
<Button
colorPalette="blue"
- disabled={Boolean(errors.conf) || formErrors || isPending ||
!isValid || !hasChanges()}
+ disabled={Boolean(errors.conf) || formErrors || isPending ||
!isValid}
onClick={() => void handleSubmit(onSubmit)()}
>
<FiSave /> {translate("formActions.save")}
diff --git a/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
b/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
index 2392eda11d6..60c813aa644 100644
--- a/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
+++ b/airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx
@@ -65,9 +65,7 @@ export const useEditConnection = (
const editConnection = (requestBody: ConnectionBody) => {
const updateMask: Array<string> = [];
- if (
- JSON.stringify(JSON.parse(requestBody.extra)) !==
JSON.stringify(JSON.parse(initialConnection.extra))
- ) {
+ if (requestBody.extra !== initialConnection.extra) {
updateMask.push("extra");
}
if (requestBody.conn_type !== initialConnection.conn_type) {
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 eba9f6ddaf8..6c56232a68a 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
@@ -159,6 +159,35 @@ class TestGetConnection(TestConnectionEndpoint):
assert body["conn_type"] == TEST_CONN_TYPE
assert body["extra"] == '{"extra_key": "extra_value"}'
+ @pytest.mark.enable_redact
+ def test_get_should_respond_200_with_extra_redacted(self, test_client,
session):
+ self.create_connection()
+ connection = session.query(Connection).first()
+ connection.extra = '{"password": "test-password"}'
+ session.commit()
+ response = test_client.get(f"/connections/{TEST_CONN_ID}")
+ assert response.status_code == 200
+ body = response.json()
+ assert body["connection_id"] == TEST_CONN_ID
+ assert body["conn_type"] == TEST_CONN_TYPE
+ assert body["extra"] == '{"password": "***"}'
+
+ @pytest.mark.enable_redact
+ def test_get_should_not_overmask_short_password_value_in_extra(self,
test_client, session):
+ connection = Connection(
+ conn_id=TEST_CONN_ID, conn_type="generic", login="a",
password="a", extra='{"key": "value"}'
+ )
+ session.add(connection)
+ session.commit()
+
+ response = test_client.get(f"/connections/{TEST_CONN_ID}")
+ assert response.status_code == 200
+ body = response.json()
+ assert body["connection_id"] == TEST_CONN_ID
+ assert body["conn_type"] == "generic"
+ assert body["login"] == "a"
+ assert body["extra"] == '{"key": "value"}'
+
class TestGetConnections(TestConnectionEndpoint):
@pytest.mark.parametrize(
@@ -280,6 +309,7 @@ class TestPostConnection(TestConnectionEndpoint):
assert "detail" in response_json
assert list(response_json["detail"].keys()) == ["reason", "statement",
"orig_error", "message"]
+ @pytest.mark.enable_redact
@pytest.mark.parametrize(
"body, expected_response",
[
@@ -292,7 +322,21 @@ class TestPostConnection(TestConnectionEndpoint):
"extra": None,
"host": None,
"login": None,
- "password": "test-password",
+ "password": "***",
+ "port": None,
+ "schema": None,
+ },
+ ),
+ (
+ {"connection_id": TEST_CONN_ID, "conn_type": TEST_CONN_TYPE,
"password": "?>@#+!_%()#"},
+ {
+ "connection_id": TEST_CONN_ID,
+ "conn_type": TEST_CONN_TYPE,
+ "description": None,
+ "extra": None,
+ "host": None,
+ "login": None,
+ "password": "***",
"port": None,
"schema": None,
},
@@ -308,23 +352,21 @@ class TestPostConnection(TestConnectionEndpoint):
"connection_id": TEST_CONN_ID,
"conn_type": TEST_CONN_TYPE,
"description": None,
- "extra": '{"password": "test-password"}',
+ "extra": '{"password": "***"}',
"host": None,
"login": None,
- "password": "A!rF|0wi$aw3s0m3",
+ "password": "***",
"port": None,
"schema": None,
},
),
],
)
- def test_post_should_response_201_password_not_masked(
- self, test_client, body, expected_response, session
- ):
+ def test_post_should_response_201_redacted_password(self, test_client,
body, expected_response, session):
response = test_client.post("/connections", json=body)
assert response.status_code == 201
assert response.json() == expected_response
- _check_last_log(session, dag_id=None, event="post_connection",
logical_date=None)
+ _check_last_log(session, dag_id=None, event="post_connection",
logical_date=None, check_masked=True)
class TestPatchConnection(TestConnectionEndpoint):
@@ -734,7 +776,22 @@ class TestPatchConnection(TestConnectionEndpoint):
"extra": None,
"host": "some_host_a",
"login": "some_login",
- "password": "test-password",
+ "password": "***",
+ "port": 8080,
+ "schema": None,
+ },
+ {"update_mask": ["password"]},
+ ),
+ (
+ {"connection_id": TEST_CONN_ID, "conn_type": TEST_CONN_TYPE,
"password": "?>@#+!_%()#"},
+ {
+ "connection_id": TEST_CONN_ID,
+ "conn_type": TEST_CONN_TYPE,
+ "description": "some_description_a",
+ "extra": None,
+ "host": "some_host_a",
+ "login": "some_login",
+ "password": "***",
"port": 8080,
"schema": None,
},
@@ -751,10 +808,10 @@ class TestPatchConnection(TestConnectionEndpoint):
"connection_id": TEST_CONN_ID,
"conn_type": TEST_CONN_TYPE,
"description": "some_description_a",
- "extra": '{"password": "test-password"}',
+ "extra": '{"password": "***"}',
"host": "some_host_a",
"login": "some_login",
- "password": "A!rF|0wi$aw3s0m3",
+ "password": "***",
"port": 8080,
"schema": None,
},
@@ -762,14 +819,14 @@ class TestPatchConnection(TestConnectionEndpoint):
),
],
)
- def test_patch_should_response_200_password_not_masked(
+ def test_patch_should_response_200_redacted_password(
self, test_client, session, body, expected_response, update_mask
):
self.create_connections()
response = test_client.patch(f"/connections/{TEST_CONN_ID}",
json=body, params=update_mask)
assert response.status_code == 200
assert response.json() == expected_response
- _check_last_log(session, dag_id=None, event="patch_connection",
logical_date=None)
+ _check_last_log(session, dag_id=None, event="patch_connection",
logical_date=None, check_masked=True)
class TestConnection(TestConnectionEndpoint):
