Fix authentication bug with httpx usage

ipfshttpclient/exceptions.py

@@ -14,10 +14,10 @@
           │    ├── ProtocolError
           │    ├── StatusError
           │    ├── ErrorResponse
-          │    │    └── PartialErrorResponse
           │    ├── ConnectionError
           │    └── TimeoutError
-          └── MatcherSpecInvalidError
+          ├── MatcherSpecInvalidError
+          └── PartialErrorResponse
 
 """
 
@@ -137,8 +137,7 @@ class CommunicationError(Error):
 
 	original: ty.Optional[Exception]
 
-	def __init__(self, original: ty.Optional[Exception],
-	             _message: ty.Optional[str] = None) -> None:
+	def __init__(self, original: ty.Optional[Exception], _message: ty.Optional[str] = None) -> None:
 		self.original = original
 
 		if _message:
@@ -158,25 +157,36 @@ class ProtocolError(CommunicationError):
 
 class StatusError(CommunicationError):
 	"""Raised when the daemon responds with an error to our request."""
-	__slots__ = ()
+	__slots__ = ('status_code',)
+
+	def __init__(self, status_code: int, message: str, original: ty.Optional[Exception]) -> None:
+		super().__init__(
+			_message=message,
+			original=original
+		)
+
+		self.status_code = status_code
 
 
 class ErrorResponse(StatusError):
 	"""Raised when the daemon has responded with an error message because the
 	requested operation could not be carried out."""
 	__slots__ = ()
 
-	def __init__(self, message: str, original: ty.Optional[Exception]) -> None:
-		super().__init__(original, message)
+	def __init__(self, status_code: int, message: str, original: ty.Optional[Exception]) -> None:
+		super().__init__(
+			status_code=status_code,
+			message=message,
+			original=original
+		)
 
 
-class PartialErrorResponse(ErrorResponse):
+class PartialErrorResponse(Error):
 	"""Raised when the daemon has responded with an error message after having
 	already returned some data."""
-	__slots__ = ()
-
-	def __init__(self, message: str, original: ty.Optional[Exception] = None) -> None:
-		super().__init__(message, original)
+
+	def __init__(self, message: str) -> None:
+		super().__init__(message)
 
 
 class ConnectionError(CommunicationError):

ipfshttpclient/http_common.py

@@ -684,3 +684,34 @@ def download(
 		finally:
 			for closable in closables:
 				closable.close()
+
+	@staticmethod
+	def _raise_for_response_status(
+			error: Exception,
+			status_code: int,
+			content: ty.List[object]
+	) -> None:
+
+		"""
+		If we have decoded an error response from the server,
+		use that as the exception message; otherwise, just pass
+		the exception on to the caller.
+		"""
+
+		if len(content) == 1:
+			item = content[0]
+
+			if isinstance(item, dict) and "Message" in item:
+				msg: str = item["Message"]
+
+				raise exceptions.ErrorResponse(
+					status_code=status_code,
+					message=msg,
+					original=error
+				) from error
+
+		raise exceptions.StatusError(
+			status_code=status_code,
+			message=str(error),
+			original=error
+		) from error

ipfshttpclient/http_httpx.py

@@ -129,8 +129,9 @@ def _make_session(self) -> httpx.Client:
 		return httpx.Client(**self._session_kwargs,
 		                    base_url  = self._session_base,
 		                    transport = connection_pool)
-
-	def _do_raise_for_status(self, response: httpx.Response) -> None:
+
+	@classmethod
+	def _do_raise_for_status(cls, response: httpx.Response) -> None:
 		try:
 			response.raise_for_status()
 		except httpx.HTTPError as error:
@@ -142,18 +143,9 @@ def _do_raise_for_status(self, response: httpx.Response) -> None:
 				content += list(decoder.parse_finalize())
 			except exceptions.DecodingError:
 				pass
-
-			# If we have decoded an error response from the server,
-			# use that as the exception message; otherwise, just pass
-			# the exception on to the caller.
-			if len(content) == 1 \
-			   and isinstance(content[0], dict) \
-			   and "Message" in content[0]:
-				msg: str = content[0]["Message"]
-				raise exceptions.ErrorResponse(msg, error) from error
-			else:
-				raise exceptions.StatusError(error) from error
-
+
+			cls._raise_for_response_status(error, response.status_code, content)
+
 	def _request(
 			self, method: str, path: str, params: ty.Sequence[ty.Tuple[str, str]], *,
 			auth: auth_t,
@@ -179,7 +171,7 @@ def _request(
 					url=path,
 					**map_args_to_httpx(
 						params=params,
-						auth=auth,
+						auth=auth or session.auth,  # type: ignore[arg-type]
 						headers=headers,
 						timeout=timeout,
 					),

ipfshttpclient/http_requests.py

@@ -1,6 +1,6 @@
 """HTTP client for API requests based on good old requests library
 
-This exists mainly for Python 3.5 compatibility.
+This exists while httpx remains in beta.
 """
 
 import math
@@ -20,8 +20,11 @@
 	Closable,
 )
 
-PATCH_REQUESTS = (os.environ.get("PY_IPFS_HTTP_CLIENT_PATCH_REQUESTS", "yes").lower()
-                  not in ("false", "no"))
+PATCH_REQUESTS = (
+	os.environ.get("PY_IPFS_HTTP_CLIENT_PATCH_REQUESTS", "yes").lower()
+	not in ("false", "no")
+)
+
 if PATCH_REQUESTS:
 	from . import requests_wrapper as requests
 elif not ty.TYPE_CHECKING:  # pragma: no cover (always enabled in production)
@@ -117,8 +120,9 @@ def _make_session(self) -> requests.Session:  # type: ignore[name-defined]
 		except:  # pragma: no cover
 			session.close()
 			raise
-
-	def _do_raise_for_status(self, response: requests.Request) -> None:  # type: ignore[name-defined]
+
+	@classmethod
+	def _do_raise_for_status(cls, response: requests.Response) -> None:  # type: ignore[name-defined]
 		try:
 			response.raise_for_status()
 		except requests.exceptions.HTTPError as error:  # type: ignore[attr-defined]
@@ -131,17 +135,8 @@ def _do_raise_for_status(self, response: requests.Request) -> None:  # type: ign
 			except exceptions.DecodingError:
 				pass
 
-			# If we have decoded an error response from the server,
-			# use that as the exception message; otherwise, just pass
-			# the exception on to the caller.
-			if len(content) == 1 \
-			   and isinstance(content[0], dict) \
-			   and "Message" in content[0]:
-				msg = content[0]["Message"]
-				raise exceptions.ErrorResponse(msg, error) from error
-			else:
-				raise exceptions.StatusError(error) from error
-
+			cls._raise_for_response_status(error, response.status_code, content)
+
 	def _request(
 			self, method: str, path: str, params: ty.Sequence[ty.Tuple[str, str]], *,
 			auth: auth_t,

test/functional/test_auth.py

@@ -0,0 +1,128 @@
+
+import base64
+import ipfshttpclient
+import json
+import pytest
+
+from ipaddress import ip_address, IPv4Address, IPv6Address
+from ipfshttpclient.exceptions import StatusError
+from multiaddr import Multiaddr
+from _pytest.fixtures import FixtureRequest
+from pytest_localserver.http import ContentServer
+from urllib.parse import urlparse
+from werkzeug import Request, Response
+
+
+BASIC_USERNAME = 'basic_username'
+BASIC_PASSWORD = 'basic_password'
+
+
+def _basic_auth_token(username: str, password: str) -> str:
+	return base64.b64encode(f'{username}:{password}'.encode('utf-8')).decode('utf-8')
+
+
+def _url_to_multiaddr(url: str) -> Multiaddr:
+	parsed = urlparse(url)
+
+	try:
+		ip = ip_address(parsed.hostname)
+	except ValueError:
+		ip = None
+
+	if ip is None:
+		prefix = '/dns'
+	elif isinstance(ip, IPv4Address):
+		prefix = '/ip4'
+	elif isinstance(ip, IPv6Address):
+		prefix = '/ip6'
+	else:
+		raise TypeError(f"Don't know how to convert {ip} to a {Multiaddr.__name__}")
+
+	return Multiaddr(f'{prefix}/{parsed.hostname}/tcp/{parsed.port}/{parsed.scheme}')
+
+
+class AuthenticatingServer(ContentServer):
+	@staticmethod
+	def _is_authorized(expected_credentials: str, request: Request) -> bool:
+		authorizations = request.headers.get_all('Authorization')
+
+		if authorizations and len(authorizations) == 1:
+			authorization = authorizations[0]
+
+			return authorization == f'Basic {expected_credentials}'
+		else:
+			return False
+
+	def __call__(self, environ, start_response) -> Response:
+		request = Request(environ)
+		self.requests.append(request)
+
+		expected_credentials = _basic_auth_token(BASIC_USERNAME, BASIC_PASSWORD)
+
+		if self._is_authorized(expected_credentials, request):
+			response = Response(status=self.code)
+			response.headers.clear()
+			response.headers.extend(self.headers)
+
+			response.data = self.content
+		else:
+			response = Response(status=401)
+			response.headers.clear()
+			response.data = 'Unauthorized'
+
+		return response(environ, start_response)
+
+
+@pytest.fixture(scope='module')
+def authenticating_server(request: FixtureRequest) -> ContentServer:
+	server = AuthenticatingServer()
+	server.start()
+	request.addfinalizer(server.stop)
+
+	return server
+
+
+def test_basic_auth_failure(authenticating_server: ContentServer) -> None:
+	headers = {
+		'Content-Type': 'text/json'
+	}
+
+	version = '0.0.1'
+
+	response = {
+		'Version': version
+	}
+
+	authenticating_server.serve_content(json.dumps(response), headers=headers)
+
+	address = _url_to_multiaddr(authenticating_server.url)
+
+	with pytest.raises(StatusError) as failure:
+		ipfshttpclient.connect(
+			addr=address,
+			auth=('wrong', 'wrong')
+		)
+
+	assert failure.value.status_code == 401
+
+
+def test_basic_auth_success(authenticating_server: ContentServer) -> None:
+	headers = {
+		'Content-Type': 'text/json'
+	}
+
+	version = '0.0.1'
+
+	response = {
+		'Version': version
+	}
+
+	authenticating_server.serve_content(json.dumps(response), headers=headers)
+
+	address = _url_to_multiaddr(authenticating_server.url)
+
+	with ipfshttpclient.connect(
+		addr=address,
+		auth=(BASIC_USERNAME, BASIC_PASSWORD)
+	) as client:
+		assert client.version()['Version'] == version

test/run-tests.py

@@ -113,16 +113,18 @@ def pytest_pyfunc_call(self, pyfuncitem):
 	with tempfile.NamedTemporaryFile("r+") as coveragerc:
 		coverage_args = []
 		if os.name != "nt":
-			PREFER_HTTPX = (os.environ.get("PY_IPFS_HTTP_CLIENT_PREFER_HTTPX", "no").lower()
-			                not in ("0", "f", "false", "n", "no"))
+			PREFER_HTTPX = (
+				os.environ.get("PY_IPFS_HTTP_CLIENT_PREFER_HTTPX", "no").lower()
+				not in ("0", "f", "false", "n", "no")
+			)
 
 			# Assemble list of files to exclude from coverage analysis
 			omitted_files = [
 				"ipfshttpclient/requests_wrapper.py",
 			]
-			if PREFER_HTTPX and sys.version_info >= (3, 6):
+			if PREFER_HTTPX:
 				omitted_files.append("ipfshttpclient/http_requests.py")
-			else:  #PY35: Fallback to old requests-based code instead of HTTPX
+			else:
 				omitted_files.append("ipfshttpclient/http_httpx.py")
 
 			# Assemble list of coverage data exclusion patterns (also escape the
@@ -152,9 +154,9 @@ def pytest_pyfunc_call(self, pyfuncitem):
 					"|".join(map(str, range(sys.version_info.minor + 1, 20)))
 				))
 
-			if PREFER_HTTPX and sys.version_info >= (3, 6):
+			if PREFER_HTTPX:
 				exclusions.append(r"\# pragma: http-backend=requests")
-			else:  #PY35: Fallback to old requests-based code instead of HTTPX
+			else:
 				exclusions.append(r"\# pragma: http-backend=httpx")
 
 			# Create temporary file with extended *coverage.py* configuration data

test/unit/test_http_common.py

@@ -0,0 +1,44 @@
+
+import pytest
+
+from ipfshttpclient.exceptions import ErrorResponse, StatusError
+from ipfshttpclient.http_common import ClientSyncBase
+
+
+def test_client_sync_base_raises_for_standardized_failure():
+	original = Exception('foo')
+
+	with pytest.raises(ErrorResponse) as failure:
+		ClientSyncBase._raise_for_response_status(
+			error=original,
+			status_code=405,
+			content=[
+				{
+					'Message': 'bar'
+				}
+			]
+		)
+
+	assert failure.value.status_code == 405
+	assert str(failure.value) == 'bar'
+	assert failure.value.original is original
+
+
+@pytest.mark.parametrize('content', [
+	[],
+	[{'wrong': 'ignored'}],
+	[{'Message': 'too'}, {'Message': 'many'}]
+])
+def test_client_sync_base_raises_for_non_standard_failure(content):
+	original = Exception('qux')
+
+	with pytest.raises(StatusError) as failure:
+		ClientSyncBase._raise_for_response_status(
+			error=original,
+			status_code=406,
+			content=content
+		)
+
+	assert failure.value.status_code == 406
+	assert str(failure.value) == 'qux'
+	assert failure.value.original is original