From 9208bd15b34e9be9da70283125dd79f39918fb1d Mon Sep 17 00:00:00 2001 From: Miguel Grinberg Date: Tue, 1 Apr 2025 16:25:01 +0100 Subject: [PATCH] Revert "Remove deprecated http_auth parameter (#2839)" This reverts commit 74387d0a3f42517e2a5805c9bb0aec57511b6e15. --- elasticsearch/_async/client/__init__.py | 21 +++++ elasticsearch/_async/client/_base.py | 26 ++++++ elasticsearch/_async/client/utils.py | 4 + elasticsearch/_async/helpers.py | 7 +- elasticsearch/_sync/client/__init__.py | 21 +++++ elasticsearch/_sync/client/_base.py | 26 ++++++ elasticsearch/_sync/client/utils.py | 35 ++++++++ elasticsearch/helpers/actions.py | 6 +- .../test_async/test_server/test_helpers.py | 6 +- .../test_client/test_deprecated_options.py | 48 +++++++++++ .../test_client/test_requests_auth.py | 84 +++++++++++++++++++ .../test_client/test_rewrite_parameters.py | 4 +- .../test_server/test_helpers.py | 7 +- 13 files changed, 287 insertions(+), 8 deletions(-) create mode 100644 test_elasticsearch/test_client/test_deprecated_options.py create mode 100644 test_elasticsearch/test_client/test_requests_auth.py diff --git a/elasticsearch/_async/client/__init__.py b/elasticsearch/_async/client/__init__.py index f88bb0190..d09bcd2bf 100644 --- a/elasticsearch/_async/client/__init__.py +++ b/elasticsearch/_async/client/__init__.py @@ -87,6 +87,8 @@ _rewrite_parameters, _stability_warning, client_node_configs, + is_requests_http_auth, + is_requests_node_class, ) from .watcher import WatcherClient from .xpack import XPackClient @@ -178,6 +180,7 @@ def __init__( t.Callable[[t.Dict[str, t.Any], NodeConfig], t.Optional[NodeConfig]] ] = None, meta_header: t.Union[DefaultType, bool] = DEFAULT, + http_auth: t.Union[DefaultType, t.Any] = DEFAULT, # Internal use only _transport: t.Optional[AsyncTransport] = None, ) -> None: @@ -225,9 +228,26 @@ def __init__( sniff_callback = default_sniff_callback if _transport is None: + requests_session_auth = None + if http_auth is not None and http_auth is not DEFAULT: + if is_requests_http_auth(http_auth): + # If we're using custom requests authentication + # then we need to alert the user that they also + # need to use 'node_class=requests'. + if not is_requests_node_class(node_class): + raise ValueError( + "Using a custom 'requests.auth.AuthBase' class for " + "'http_auth' must be used with node_class='requests'" + ) + + # Reset 'http_auth' to DEFAULT so it's not consumed below. + requests_session_auth = http_auth + http_auth = DEFAULT + node_configs = client_node_configs( hosts, cloud_id=cloud_id, + requests_session_auth=requests_session_auth, connections_per_node=connections_per_node, http_compress=http_compress, verify_certs=verify_certs, @@ -314,6 +334,7 @@ def __init__( self._headers["x-opaque-id"] = opaque_id self._headers = resolve_auth_headers( self._headers, + http_auth=http_auth, api_key=api_key, basic_auth=basic_auth, bearer_auth=bearer_auth, diff --git a/elasticsearch/_async/client/_base.py b/elasticsearch/_async/client/_base.py index 3907cfd84..cc090671c 100644 --- a/elasticsearch/_async/client/_base.py +++ b/elasticsearch/_async/client/_base.py @@ -68,6 +68,7 @@ def resolve_auth_headers( headers: Optional[Mapping[str, str]], + http_auth: Union[DefaultType, None, Tuple[str, str], str] = DEFAULT, api_key: Union[DefaultType, None, Tuple[str, str], str] = DEFAULT, basic_auth: Union[DefaultType, None, Tuple[str, str], str] = DEFAULT, bearer_auth: Union[DefaultType, None, str] = DEFAULT, @@ -77,7 +78,32 @@ def resolve_auth_headers( elif not isinstance(headers, HttpHeaders): headers = HttpHeaders(headers) + resolved_http_auth = http_auth if http_auth is not DEFAULT else None resolved_basic_auth = basic_auth if basic_auth is not DEFAULT else None + if resolved_http_auth is not None: + if resolved_basic_auth is not None: + raise ValueError( + "Can't specify both 'http_auth' and 'basic_auth', " + "instead only specify 'basic_auth'" + ) + if isinstance(http_auth, str) or ( + isinstance(resolved_http_auth, (list, tuple)) + and all(isinstance(x, str) for x in resolved_http_auth) + ): + resolved_basic_auth = resolved_http_auth + else: + raise TypeError( + "The deprecated 'http_auth' parameter must be either 'Tuple[str, str]' or 'str'. " + "Use either the 'basic_auth' parameter instead" + ) + + warnings.warn( + "The 'http_auth' parameter is deprecated. " + "Use 'basic_auth' or 'bearer_auth' parameters instead", + category=DeprecationWarning, + stacklevel=warn_stacklevel(), + ) + resolved_api_key = api_key if api_key is not DEFAULT else None resolved_bearer_auth = bearer_auth if bearer_auth is not DEFAULT else None if resolved_api_key or resolved_basic_auth or resolved_bearer_auth: diff --git a/elasticsearch/_async/client/utils.py b/elasticsearch/_async/client/utils.py index ac7a35062..97918d9e4 100644 --- a/elasticsearch/_async/client/utils.py +++ b/elasticsearch/_async/client/utils.py @@ -27,6 +27,8 @@ _rewrite_parameters, _stability_warning, client_node_configs, + is_requests_http_auth, + is_requests_node_class, ) __all__ = [ @@ -41,4 +43,6 @@ "client_node_configs", "_rewrite_parameters", "_stability_warning", + "is_requests_http_auth", + "is_requests_node_class", ] diff --git a/elasticsearch/_async/helpers.py b/elasticsearch/_async/helpers.py index 8b5d3d0d2..e4d5e6bc5 100644 --- a/elasticsearch/_async/helpers.py +++ b/elasticsearch/_async/helpers.py @@ -418,9 +418,12 @@ def pop_transport_kwargs(kw: MutableMapping[str, Any]) -> MutableMapping[str, An # Grab options that should be propagated to every # API call within this helper instead of just 'search()' transport_kwargs = {} - for key in ("headers", "api_key", "basic_auth", "bearer_auth"): + for key in ("headers", "api_key", "http_auth", "basic_auth", "bearer_auth"): try: - transport_kwargs[key] = kw.pop(key) + value = kw.pop(key) + if key == "http_auth": + key = "basic_auth" + transport_kwargs[key] = value except KeyError: pass return transport_kwargs diff --git a/elasticsearch/_sync/client/__init__.py b/elasticsearch/_sync/client/__init__.py index b39cbae26..e8d63b040 100644 --- a/elasticsearch/_sync/client/__init__.py +++ b/elasticsearch/_sync/client/__init__.py @@ -87,6 +87,8 @@ _rewrite_parameters, _stability_warning, client_node_configs, + is_requests_http_auth, + is_requests_node_class, ) from .watcher import WatcherClient from .xpack import XPackClient @@ -178,6 +180,7 @@ def __init__( t.Callable[[t.Dict[str, t.Any], NodeConfig], t.Optional[NodeConfig]] ] = None, meta_header: t.Union[DefaultType, bool] = DEFAULT, + http_auth: t.Union[DefaultType, t.Any] = DEFAULT, # Internal use only _transport: t.Optional[Transport] = None, ) -> None: @@ -225,9 +228,26 @@ def __init__( sniff_callback = default_sniff_callback if _transport is None: + requests_session_auth = None + if http_auth is not None and http_auth is not DEFAULT: + if is_requests_http_auth(http_auth): + # If we're using custom requests authentication + # then we need to alert the user that they also + # need to use 'node_class=requests'. + if not is_requests_node_class(node_class): + raise ValueError( + "Using a custom 'requests.auth.AuthBase' class for " + "'http_auth' must be used with node_class='requests'" + ) + + # Reset 'http_auth' to DEFAULT so it's not consumed below. + requests_session_auth = http_auth + http_auth = DEFAULT + node_configs = client_node_configs( hosts, cloud_id=cloud_id, + requests_session_auth=requests_session_auth, connections_per_node=connections_per_node, http_compress=http_compress, verify_certs=verify_certs, @@ -314,6 +334,7 @@ def __init__( self._headers["x-opaque-id"] = opaque_id self._headers = resolve_auth_headers( self._headers, + http_auth=http_auth, api_key=api_key, basic_auth=basic_auth, bearer_auth=bearer_auth, diff --git a/elasticsearch/_sync/client/_base.py b/elasticsearch/_sync/client/_base.py index 64abdc250..868b71073 100644 --- a/elasticsearch/_sync/client/_base.py +++ b/elasticsearch/_sync/client/_base.py @@ -68,6 +68,7 @@ def resolve_auth_headers( headers: Optional[Mapping[str, str]], + http_auth: Union[DefaultType, None, Tuple[str, str], str] = DEFAULT, api_key: Union[DefaultType, None, Tuple[str, str], str] = DEFAULT, basic_auth: Union[DefaultType, None, Tuple[str, str], str] = DEFAULT, bearer_auth: Union[DefaultType, None, str] = DEFAULT, @@ -77,7 +78,32 @@ def resolve_auth_headers( elif not isinstance(headers, HttpHeaders): headers = HttpHeaders(headers) + resolved_http_auth = http_auth if http_auth is not DEFAULT else None resolved_basic_auth = basic_auth if basic_auth is not DEFAULT else None + if resolved_http_auth is not None: + if resolved_basic_auth is not None: + raise ValueError( + "Can't specify both 'http_auth' and 'basic_auth', " + "instead only specify 'basic_auth'" + ) + if isinstance(http_auth, str) or ( + isinstance(resolved_http_auth, (list, tuple)) + and all(isinstance(x, str) for x in resolved_http_auth) + ): + resolved_basic_auth = resolved_http_auth + else: + raise TypeError( + "The deprecated 'http_auth' parameter must be either 'Tuple[str, str]' or 'str'. " + "Use either the 'basic_auth' parameter instead" + ) + + warnings.warn( + "The 'http_auth' parameter is deprecated. " + "Use 'basic_auth' or 'bearer_auth' parameters instead", + category=DeprecationWarning, + stacklevel=warn_stacklevel(), + ) + resolved_api_key = api_key if api_key is not DEFAULT else None resolved_bearer_auth = bearer_auth if bearer_auth is not DEFAULT else None if resolved_api_key or resolved_basic_auth or resolved_bearer_auth: diff --git a/elasticsearch/_sync/client/utils.py b/elasticsearch/_sync/client/utils.py index 3293e356e..48ebcb217 100644 --- a/elasticsearch/_sync/client/utils.py +++ b/elasticsearch/_sync/client/utils.py @@ -16,6 +16,7 @@ # under the License. import base64 +import inspect import urllib.parse import warnings from datetime import date, datetime @@ -43,6 +44,7 @@ AsyncTransport, HttpHeaders, NodeConfig, + RequestsHttpNode, SniffOptions, Transport, ) @@ -90,6 +92,7 @@ class Stability(Enum): _TRANSPORT_OPTIONS = { "api_key", + "http_auth", "request_timeout", "opaque_id", "headers", @@ -102,6 +105,7 @@ class Stability(Enum): def client_node_configs( hosts: Optional[_TYPE_HOSTS], cloud_id: Optional[str], + requests_session_auth: Optional[Any] = None, **kwargs: Any, ) -> List[NodeConfig]: if cloud_id is not None: @@ -122,6 +126,12 @@ def client_node_configs( headers.setdefault("user-agent", USER_AGENT) node_options["headers"] = headers + # If a custom Requests AuthBase is passed we set that via '_extras'. + if requests_session_auth is not None: + node_options.setdefault("_extras", {})[ + "requests.session.auth" + ] = requests_session_auth + def apply_node_options(node_config: NodeConfig) -> NodeConfig: """Needs special handling of headers since .replace() wipes out existing headers""" headers = node_config.headers.copy() # type: ignore[attr-defined] @@ -438,3 +448,28 @@ def wrapped(*args: Any, **kwargs: Any) -> Any: return wrapped # type: ignore[return-value] return wrapper + + +def is_requests_http_auth(http_auth: Any) -> bool: + """Detect if an http_auth value is a custom Requests auth object""" + try: + from requests.auth import AuthBase + + return isinstance(http_auth, AuthBase) + except ImportError: + pass + return False + + +def is_requests_node_class(node_class: Any) -> bool: + """Detect if 'RequestsHttpNode' would be used given the setting of 'node_class'""" + return ( + node_class is not None + and node_class is not DEFAULT + and ( + node_class == "requests" + or ( + inspect.isclass(node_class) and issubclass(node_class, RequestsHttpNode) + ) + ) + ) diff --git a/elasticsearch/helpers/actions.py b/elasticsearch/helpers/actions.py index 25c21cdd4..d1a43a8dc 100644 --- a/elasticsearch/helpers/actions.py +++ b/elasticsearch/helpers/actions.py @@ -698,12 +698,16 @@ def pop_transport_kwargs(kw: MutableMapping[str, Any]) -> Dict[str, Any]: for key in ( "headers", "api_key", + "http_auth", "basic_auth", "bearer_auth", "opaque_id", ): try: - transport_kwargs[key] = kw.pop(key) + value = kw.pop(key) + if key == "http_auth": + key = "basic_auth" + transport_kwargs[key] = value except KeyError: pass return transport_kwargs diff --git a/test_elasticsearch/test_async/test_server/test_helpers.py b/test_elasticsearch/test_async/test_server/test_helpers.py index 86439b487..0bb781304 100644 --- a/test_elasticsearch/test_async/test_server/test_helpers.py +++ b/test_elasticsearch/test_async/test_server/test_helpers.py @@ -741,8 +741,7 @@ async def test_clear_scroll(self, async_client, scan_teardown): "kwargs", [ {"api_key": ("name", "value")}, - {"basic_auth": ("username", "password")}, - {"bearer_auth": "token"}, + {"http_auth": ("username", "password")}, {"headers": {"custom", "header"}}, ], ) @@ -791,6 +790,9 @@ async def test_scan_auth_kwargs_forwarded( assert data == [{"search_data": 1}] + if "http_auth" in kwargs: + kwargs = {"basic_auth": kwargs.pop("http_auth")} + assert options.call_args_list == [ call(request_timeout=None, **kwargs), call(ignore_status=404), diff --git a/test_elasticsearch/test_client/test_deprecated_options.py b/test_elasticsearch/test_client/test_deprecated_options.py new file mode 100644 index 000000000..bbde2e90b --- /dev/null +++ b/test_elasticsearch/test_client/test_deprecated_options.py @@ -0,0 +1,48 @@ +# Licensed to Elasticsearch B.V. under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Elasticsearch B.V. licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import warnings + +import pytest + +from elasticsearch import Elasticsearch + + +def test_http_auth(): + with warnings.catch_warnings(record=True) as w: + client = Elasticsearch( + "http://localhost:9200", http_auth=("username", "password") + ) + + assert len(w) == 1 + assert w[0].category == DeprecationWarning + assert ( + str(w[0].message) + == "The 'http_auth' parameter is deprecated. Use 'basic_auth' or 'bearer_auth' parameters instead" + ) + assert client._headers["Authorization"] == "Basic dXNlcm5hbWU6cGFzc3dvcmQ=" + + with pytest.raises(ValueError) as e: + Elasticsearch( + "http://localhost:9200", + http_auth=("username", "password"), + basic_auth=("username", "password"), + ) + assert ( + str(e.value) + == "Can't specify both 'http_auth' and 'basic_auth', instead only specify 'basic_auth'" + ) diff --git a/test_elasticsearch/test_client/test_requests_auth.py b/test_elasticsearch/test_client/test_requests_auth.py new file mode 100644 index 000000000..2eb656f5d --- /dev/null +++ b/test_elasticsearch/test_client/test_requests_auth.py @@ -0,0 +1,84 @@ +# Licensed to Elasticsearch B.V. under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Elasticsearch B.V. licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import warnings + +import pytest +import requests +from elastic_transport import RequestsHttpNode, Urllib3HttpNode +from elastic_transport.client_utils import DEFAULT +from requests.auth import HTTPBasicAuth + +from elasticsearch import AsyncElasticsearch, Elasticsearch + + +class CustomRequestHttpNode(RequestsHttpNode): + pass + + +class CustomUrllib3HttpNode(Urllib3HttpNode): + pass + + +@pytest.mark.parametrize( + "node_class", ["requests", RequestsHttpNode, CustomRequestHttpNode] +) +def test_requests_auth(node_class): + http_auth = HTTPBasicAuth("username", "password") + + with warnings.catch_warnings(record=True) as w: + client = Elasticsearch( + "http://localhost:9200", http_auth=http_auth, node_class=node_class + ) + + # http_auth is deprecated for all other cases except this one. + assert len(w) == 0 + + # Instance should be forwarded directly to requests.Session.auth. + node = client.transport.node_pool.get() + assert isinstance(node, RequestsHttpNode) + assert isinstance(node.session, requests.Session) + assert node.session.auth is http_auth + + +@pytest.mark.parametrize("client_class", [Elasticsearch, AsyncElasticsearch]) +@pytest.mark.parametrize( + "node_class", ["urllib3", "aiohttp", None, DEFAULT, CustomUrllib3HttpNode] +) +def test_error_for_requests_auth_node_class(client_class, node_class): + http_auth = HTTPBasicAuth("username", "password") + + with pytest.raises(ValueError) as e: + client_class( + "http://localhost:9200", http_auth=http_auth, node_class=node_class + ) + assert str(e.value) == ( + "Using a custom 'requests.auth.AuthBase' class for " + "'http_auth' must be used with node_class='requests'" + ) + + +def test_error_for_requests_auth_async(): + http_auth = HTTPBasicAuth("username", "password") + + with pytest.raises(ValueError) as e: + AsyncElasticsearch( + "http://localhost:9200", http_auth=http_auth, node_class="requests" + ) + assert str(e.value) == ( + "Specified 'node_class' is not async, should be async instead" + ) diff --git a/test_elasticsearch/test_client/test_rewrite_parameters.py b/test_elasticsearch/test_client/test_rewrite_parameters.py index f933cfd51..50a232563 100644 --- a/test_elasticsearch/test_client/test_rewrite_parameters.py +++ b/test_elasticsearch/test_client/test_rewrite_parameters.py @@ -208,7 +208,7 @@ def test_ignore_deprecated_options(self): body={"query": {"match_all": {}}}, params={"key": "value"}, param=1, - request_timeout=10, + http_auth=("key", "value"), ) assert len(w) == 1 @@ -219,7 +219,7 @@ def test_ignore_deprecated_options(self): ) assert self.calls == [ - ((), {"request_timeout": 10}), + ((), {"http_auth": ("key", "value")}), ( (), { diff --git a/test_elasticsearch/test_server/test_helpers.py b/test_elasticsearch/test_server/test_helpers.py index 361a98ae2..6ed43e2af 100644 --- a/test_elasticsearch/test_server/test_helpers.py +++ b/test_elasticsearch/test_server/test_helpers.py @@ -626,6 +626,7 @@ def test_no_scroll_id_fast_route(sync_client): "kwargs", [ {"api_key": ("name", "value")}, + {"http_auth": ("username", "password")}, {"basic_auth": ("username", "password")}, {"bearer_auth": "token"}, {"headers": {"custom", "header"}}, @@ -633,6 +634,8 @@ def test_no_scroll_id_fast_route(sync_client): ) @pytest.mark.usefixtures("scan_teardown") def test_scan_auth_kwargs_forwarded(sync_client, kwargs): + ((key, val),) = kwargs.items() + with patch.object( sync_client, "options", return_value=sync_client ) as options, patch.object( @@ -665,7 +668,9 @@ def test_scan_auth_kwargs_forwarded(sync_client, kwargs): assert data == [{"search_data": 1}] assert options.call_args_list == [ - call(request_timeout=None, **kwargs), + call( + request_timeout=None, **{key if key != "http_auth" else "basic_auth": val} + ), call(ignore_status=404), ]