Skip to content

Commit b982d13

Browse files
RinZ27RinZ27
committed
fix(ingestion): enhance SSL safety and log sanitization
1 parent cdd0b3a commit b982d13

8 files changed

Lines changed: 257 additions & 24 deletions

File tree

‎ingestion/src/metadata/ingestion/source/database/sas/client.py‎

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,17 @@
1717
from metadata.generated.schema.entity.services.connections.database.sasConnection import (
1818
SASConnection,
1919
)
20+
from metadata.generated.schema.security.ssl.verifySSLConfig import VerifySSL
2021
from metadata.ingestion.connections.source_api_client import TrackedREST
2122
from metadata.ingestion.ometa.client import APIError, ClientConfig
2223
from metadata.utils.helpers import clean_uri
2324
from metadata.utils.logger import ingestion_logger
25+
from metadata.utils.ssl_registry import get_verify_ssl_fn
2426

2527
logger = ingestion_logger()
2628

29+
SAS_CLI_AUTH_HEADER = "Basic c2FzLmNsaTo="
30+
2731

2832
class SASClient:
2933
"""
@@ -33,13 +37,14 @@ class SASClient:
3337
def __init__(self, config: SASConnection):
3438
self.config: SASConnection = config
3539
self.auth_token = self.get_token(config.serverHost, config.username, config.password.get_secret_value())
40+
3641
client_config: ClientConfig = ClientConfig(
3742
base_url=clean_uri(config.serverHost),
3843
auth_header="Authorization",
3944
auth_token=self.get_auth_token,
4045
api_version="",
4146
allow_redirects=True,
42-
verify=False,
47+
verify=self._get_verify(),
4348
)
4449
self.client = TrackedREST(client_config, source_name="sas")
4550
# custom setting
@@ -50,6 +55,22 @@ def __init__(self, config: SASConnection):
5055
self.enable_dataflows = config.dataflows
5156
self.custom_filter_dataflows = config.dataflowsCustomFilter
5257

58+
def _get_verify(self):
59+
"""
60+
Helper to determine the SSL verification strategy
61+
"""
62+
verify = True
63+
if self.config.verifySSL == VerifySSL.ignore:
64+
verify = False
65+
elif self.config.verifySSL == VerifySSL.no_ssl:
66+
verify = False
67+
elif self.config.verifySSL == VerifySSL.validate and self.config.sslConfig:
68+
try:
69+
verify = get_verify_ssl_fn(self.config.verifySSL)(self.config.sslConfig)
70+
except Exception: # pylint: disable=broad-except
71+
verify = True
72+
return verify
73+
5374
def check_connection(self):
5475
"""
5576
Check metadata connection to SAS
@@ -72,7 +93,7 @@ def get_instance(self, instance_id):
7293
}
7394
response = self.client.get(path=endpoint, headers=headers)
7495
if "error" in response.keys(): # noqa: SIM118
75-
raise APIError(response["error"])
96+
raise APIError({"message": response["error"]})
7697
return response
7798

7899
def get_information_catalog_link(self, instance_id):
@@ -99,7 +120,7 @@ def list_assets(self, assets):
99120
headers = {"Accept-Item": "application/vnd.sas.metadata.instance.entity+json"}
100121
response = self.client.get(path=endpoint, headers=headers)
101122
if "error" in response.keys(): # noqa: SIM118
102-
raise APIError(response["error"])
123+
raise APIError({"message": response["error"]})
103124
return response["items"]
104125

105126
def get_views(self, query):
@@ -111,7 +132,7 @@ def get_views(self, query):
111132
logger.info(f"{query}")
112133
response = self.client.post(path=endpoint, data=query, headers=headers)
113134
if "error" in response.keys(): # noqa: SIM118
114-
raise APIError(f"{response}")
135+
raise APIError({"message": "Error fetching views from SAS"})
115136
return response
116137

117138
def get_data_source(self, endpoint):
@@ -121,7 +142,7 @@ def get_data_source(self, endpoint):
121142
response = self.client.get(path=endpoint, headers=headers)
122143
logger.info(f"{response}")
123144
if "error" in response.keys(): # noqa: SIM118
124-
raise APIError(response["error"])
145+
raise APIError({"message": response["error"]})
125146
return response
126147

127148
def get_report_link(self, resource, uri):
@@ -136,7 +157,7 @@ def get_report_relationship(self, report_id):
136157
endpoint = f"reports/commons/relationships/reports/{report_id}"
137158
response = self.client.get(endpoint)
138159
if "error" in response.keys(): # noqa: SIM118
139-
raise APIError(response["error"])
160+
raise APIError({"message": response["error"]})
140161
dependencies = []
141162
for item in response["items"]:
142163
if item["type"] == "Dependent":
@@ -146,14 +167,14 @@ def get_report_relationship(self, report_id):
146167
def get_resource(self, endpoint):
147168
response = self.client.get(endpoint)
148169
if "error" in response.keys(): # noqa: SIM118
149-
raise APIError(response["error"])
170+
raise APIError({"message": response["error"]})
150171
return response
151172

152173
def get_instances_with_param(self, data):
153174
endpoint = f"catalog/instances?{data}"
154175
response = self.client.get(endpoint)
155176
if "error" in response.keys(): # noqa: SIM118
156-
raise APIError(response["error"])
177+
raise APIError({"message": response["error"]})
157178
return response["items"]
158179

159180
def get_auth_token(self):
@@ -164,8 +185,30 @@ def get_token(self, base_url, user, password):
164185
payload = {"grant_type": "password", "username": user, "password": password}
165186
headers = {
166187
"Content-type": "application/x-www-form-urlencoded",
167-
"Authorization": "Basic c2FzLmNsaTo=",
188+
"Authorization": SAS_CLI_AUTH_HEADER,
168189
}
169190
url = base_url + endpoint
170-
response = requests.request("POST", url, headers=headers, data=payload, verify=False, timeout=10)
171-
return response.json()["access_token"]
191+
192+
response = requests.request(
193+
"POST",
194+
url,
195+
headers=headers,
196+
data=payload,
197+
verify=self._get_verify(),
198+
timeout=10,
199+
)
200+
logger.debug(
201+
"Token request for user: %s completed with status: %s",
202+
user,
203+
response.status_code,
204+
)
205+
response.raise_for_status()
206+
try:
207+
body = response.json()
208+
except ValueError as exc:
209+
raise RuntimeError(f"SAS token endpoint returned non-JSON response (HTTP {response.status_code})") from exc
210+
211+
token = body.get("access_token")
212+
if not token:
213+
raise RuntimeError(f"Failed to retrieve access_token from SAS (HTTP {response.status_code})")
214+
return token

‎ingestion/src/metadata/ingestion/source/search/elasticsearch/connection.py‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
from metadata.generated.schema.entity.services.connections.testConnectionResult import (
4545
TestConnectionResult,
4646
)
47+
from metadata.generated.schema.security.ssl.verifySSLConfig import VerifySSL
4748
from metadata.ingestion.connections.builders import init_empty_connection_arguments
4849
from metadata.ingestion.connections.test_connections import test_connection_steps
4950
from metadata.ingestion.ometa.ometa_api import OpenMetadata
@@ -102,22 +103,26 @@ def _handle_ssl_context_by_path(ssl_config: SslConfig):
102103
return ca_cert, client_cert, private_key
103104

104105

105-
def get_ssl_context(ssl_config: SslConfig) -> ssl.SSLContext:
106+
def get_ssl_context(ssl_config: SslConfig, verify_ssl: VerifySSL = VerifySSL.validate) -> ssl.SSLContext:
106107
"""
107108
Method to get SSL Context
108109
"""
110+
if verify_ssl == VerifySSL.ignore:
111+
return ssl._create_unverified_context() # pylint: disable=protected-access
112+
113+
if verify_ssl == VerifySSL.no_ssl:
114+
return None
115+
109116
ca_cert = False
110117
client_cert = None
111118
private_key = None
112119
cert_chain = None
113120

114-
if not ssl_config.certificates:
115-
return None
116-
117-
if isinstance(ssl_config.certificates, SslCertificatesByValues):
118-
ca_cert, client_cert, private_key = _handle_ssl_context_by_value(ssl_config=ssl_config)
119-
elif isinstance(ssl_config.certificates, SslCertificatesByPath):
120-
ca_cert, client_cert, private_key = _handle_ssl_context_by_path(ssl_config=ssl_config)
121+
if ssl_config and ssl_config.certificates:
122+
if isinstance(ssl_config.certificates, SslCertificatesByValues):
123+
ca_cert, client_cert, private_key = _handle_ssl_context_by_value(ssl_config=ssl_config)
124+
elif isinstance(ssl_config.certificates, SslCertificatesByPath):
125+
ca_cert, client_cert, private_key = _handle_ssl_context_by_path(ssl_config=ssl_config)
121126

122127
if client_cert and private_key:
123128
cert_chain = (client_cert, private_key)
@@ -133,7 +138,7 @@ def get_ssl_context(ssl_config: SslConfig) -> ssl.SSLContext:
133138
)
134139
return ssl_context # noqa: RET504
135140

136-
return ssl._create_unverified_context() # pylint: disable=protected-access
141+
return ssl.create_default_context()
137142

138143

139144
def get_connection(connection: ElasticsearchConnection) -> Elasticsearch:
@@ -146,7 +151,7 @@ def get_connection(connection: ElasticsearchConnection) -> Elasticsearch:
146151
if isinstance(connection.authType, BasicAuthentication) and connection.authType.username:
147152
basic_auth = (
148153
connection.authType.username,
149-
connection.authType.password.get_secret_value() if connection.authType.password else None,
154+
(connection.authType.password.get_secret_value() if connection.authType.password else None),
150155
)
151156

152157
if isinstance(connection.authType, ApiKeyAuthentication):
@@ -161,8 +166,7 @@ def get_connection(connection: ElasticsearchConnection) -> Elasticsearch:
161166
if not connection.connectionArguments:
162167
connection.connectionArguments = init_empty_connection_arguments()
163168

164-
if connection.sslConfig:
165-
ssl_context = get_ssl_context(connection.sslConfig)
169+
ssl_context = get_ssl_context(connection.sslConfig, connection.verifySSL)
166170

167171
return Elasticsearch(
168172
str(connection.hostPort),

‎ingestion/src/metadata/utils/secrets/aws_secrets_manager.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def get_string_value(self, secret_id: str) -> Optional[str]: # noqa: UP045
5252
try:
5353
kwargs = {"SecretId": secret_id}
5454
response = self.client.get_secret_value(**kwargs)
55-
logger.debug("Got value for secret %s.", secret_id)
55+
logger.debug("Successfully retrieved value from secrets manager.")
5656
except ClientError as err:
5757
logger.debug(traceback.format_exc())
5858
logger.error(f"Couldn't get value for secret [{secret_id}]: {err}")

‎ingestion/tests/unit/topology/database/test_sas_client.py‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
# Copyright 2025 Collate
2+
# Licensed under the Collate Community License, Version 1.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/LICENSE
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
"""
12+
Unit tests for SASClient
13+
"""
14+
from unittest.mock import MagicMock, patch
15+
16+
import pytest
17+
import requests
18+
from pydantic import SecretStr
19+
20+
from metadata.generated.schema.entity.services.connections.database.sasConnection import (
21+
SASConnection,
22+
)
23+
from metadata.generated.schema.security.ssl.verifySSLConfig import VerifySSL
24+
from metadata.ingestion.source.database.sas.client import SASClient
25+
from metadata.ingestion.ometa.client import APIError
26+
27+
MOCK_CONFIG = SASConnection(
28+
username="user",
29+
password=SecretStr("pass"),
30+
serverHost="http://sas.com",
31+
verifySSL=VerifySSL.validate
32+
)
33+
34+
class TestSASClient:
35+
@patch("metadata.ingestion.source.database.sas.client.requests.request")
36+
@patch("metadata.ingestion.source.database.sas.client.TrackedREST")
37+
def test_init_success(self, mock_rest, mock_request):
38+
mock_response = MagicMock()
39+
mock_response.status_code = 200
40+
mock_response.json.return_value = {"access_token": "token123"}
41+
mock_request.return_value = mock_response
42+
43+
client = SASClient(MOCK_CONFIG)
44+
45+
assert client.auth_token == "token123"
46+
mock_rest.assert_called_once()
47+
# Verify default verify is True (VerifySSL.validate)
48+
args, kwargs = mock_rest.call_args
49+
assert kwargs["config"].verify is True
50+
51+
@patch("metadata.ingestion.source.database.sas.client.requests.request")
52+
@patch("metadata.ingestion.source.database.sas.client.TrackedREST")
53+
def test_init_verify_ignore(self, mock_rest, mock_request):
54+
mock_response = MagicMock()
55+
mock_response.status_code = 200
56+
mock_response.json.return_value = {"access_token": "token123"}
57+
mock_request.return_value = mock_response
58+
59+
config = MOCK_CONFIG.model_copy()
60+
config.verifySSL = VerifySSL.ignore
61+
62+
client = SASClient(config)
63+
64+
args, kwargs = mock_rest.call_args
65+
assert kwargs["config"].verify is False
66+
67+
@patch("metadata.ingestion.source.database.sas.client.requests.request")
68+
@patch("metadata.ingestion.source.database.sas.client.TrackedREST")
69+
def test_get_token_error_handling(self, mock_rest, mock_request):
70+
# Case 1: Non-JSON response
71+
mock_response = MagicMock()
72+
mock_response.status_code = 500
73+
mock_response.json.side_effect = ValueError("Not JSON")
74+
mock_request.return_value = mock_response
75+
76+
with pytest.raises(RuntimeError) as exc:
77+
SASClient(MOCK_CONFIG)
78+
assert "non-JSON response" in str(exc.value)
79+
80+
# Case 2: Missing access_token
81+
mock_response.status_code = 200
82+
mock_response.json.side_effect = None
83+
mock_response.json.return_value = {"something": "else"}
84+
85+
with pytest.raises(RuntimeError) as exc:
86+
SASClient(MOCK_CONFIG)
87+
assert "Failed to retrieve access_token" in str(exc.value)
88+
89+
@patch("metadata.ingestion.source.database.sas.client.requests.request")
90+
@patch("metadata.ingestion.source.database.sas.client.TrackedREST")
91+
def test_api_error_wrapping(self, mock_rest, mock_request):
92+
mock_response = MagicMock()
93+
mock_response.status_code = 200
94+
mock_response.json.return_value = {"access_token": "token123"}
95+
mock_request.return_value = mock_response
96+
97+
client = SASClient(MOCK_CONFIG)
98+
99+
# Mocking client.get to return an error key
100+
client.client.get.return_value = {"error": "Unauthorized access"}
101+
102+
with pytest.raises(APIError) as exc:
103+
client.get_instance("123")
104+
assert exc.value.args[0] == "Unauthorized access"
105+
106+
@patch("metadata.ingestion.source.database.sas.client.requests.request")
107+
@patch("metadata.ingestion.source.database.sas.client.TrackedREST")
108+
def test_get_views_error_sanitization(self, mock_rest, mock_request):
109+
mock_response = MagicMock()
110+
mock_response.status_code = 200
111+
mock_response.json.return_value = {"access_token": "token123"}
112+
mock_request.return_value = mock_response
113+
114+
client = SASClient(MOCK_CONFIG)
115+
116+
# Mocking client.post to return an error key
117+
client.client.post.return_value = {"error": "Sensitive internal details"}
118+
119+
with pytest.raises(APIError) as exc:
120+
client.get_views("query")
121+
# Should show the sanitized message instead of the raw error
122+
assert exc.value.args[0] == "Error fetching views from SAS"

‎openmetadata-spec/src/main/resources/json/schema/entity/services/connections/database/sasConnection.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@
114114
"supportsMetadataExtraction": {
115115
"title": "Supports Metadata Extraction",
116116
"$ref": "../connectionBasicType.json#/definitions/supportsMetadataExtraction"
117+
},
118+
"sslConfig": {
119+
"title": "SSL Config",
120+
"$ref": "../../../../security/ssl/verifySSLConfig.json#/definitions/sslConfig"
121+
},
122+
"verifySSL": {
123+
"title": "Verify SSL",
124+
"description": "Client SSL verification. Make sure to configure the SSLConfig if enabled.",
125+
"$ref": "../../../../security/ssl/verifySSLConfig.json#/definitions/verifySSL",
126+
"default": "validate"
117127
}
118128
},
119129
"required": ["username", "password", "serverHost"],

‎openmetadata-spec/src/main/resources/json/schema/entity/services/connections/search/elasticSearchConnection.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@
6060
"supportsMetadataExtraction": {
6161
"title": "Supports Metadata Extraction",
6262
"$ref": "../connectionBasicType.json#/definitions/supportsMetadataExtraction"
63+
},
64+
"verifySSL": {
65+
"title": "Verify SSL",
66+
"description": "Client SSL verification. Make sure to configure the SSLConfig if enabled.",
67+
"$ref": "../../../../security/ssl/verifySSLConfig.json#/definitions/verifySSL",
68+
"default": "validate"
6369
}
6470
},
6571
"additionalProperties": false

0 commit comments

Comments
 (0)