From acfeba1eca06d6fe96767d8ad99029bf17890a33 Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sun, 9 Aug 2026 20:22:25 -0400 Subject: [PATCH 1/7] Add asynchronous aiohttp Redfish client Add a caller-owned aiohttp transport with Basic authentication, timeout handling, same-origin target validation, and cached responses. Include standard ComputerSystem discovery and advertised reset support for asynchronous consumers. Signed-off-by: Jacob Yundt --- README.rst | 50 +++++ examples/async_client.py | 38 ++++ requirements.txt | 1 + setup.py | 3 + src/redfish/aio/__init__.py | 45 +++++ src/redfish/aio/client.py | 348 ++++++++++++++++++++++++++++++++++ src/redfish/aio/exceptions.py | 44 +++++ src/redfish/aio/models.py | 120 ++++++++++++ src/redfish/aio/response.py | 75 ++++++++ tests/aio/__init__.py | 4 + tests/aio/test_client.py | 339 +++++++++++++++++++++++++++++++++ tests/aio/test_models.py | 56 ++++++ tests/aio/test_systems.py | 304 +++++++++++++++++++++++++++++ tox.ini | 2 +- 14 files changed, 1428 insertions(+), 1 deletion(-) create mode 100644 examples/async_client.py create mode 100644 src/redfish/aio/__init__.py create mode 100644 src/redfish/aio/client.py create mode 100644 src/redfish/aio/exceptions.py create mode 100644 src/redfish/aio/models.py create mode 100644 src/redfish/aio/response.py create mode 100644 tests/aio/__init__.py create mode 100644 tests/aio/test_client.py create mode 100644 tests/aio/test_models.py create mode 100644 tests/aio/test_systems.py diff --git a/README.rst b/README.rst index c9f0143..436a4d1 100644 --- a/README.rst +++ b/README.rst @@ -26,6 +26,12 @@ Installing pip install redfish +The asynchronous client has an optional ``aiohttp`` dependency: + +.. code-block:: console + + pip install redfish[aiohttp] + Building from zip file source ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -52,6 +58,8 @@ Required external packages: requests-toolbelt requests-unixsocket +The optional asynchronous client requires ``aiohttp>=3.9.0``. + If installing from GitHub, you may install the external packages by running: .. code-block:: console @@ -183,6 +191,48 @@ Each of the previous methods allows for the following arguments: - This can be useful when a particular URI is known to take multiple retries. - The default value is ``None``, which indicates the object-defined max retry count is used. +Asynchronous client +~~~~~~~~~~~~~~~~~~~ + +The additive asynchronous API uses ``aiohttp`` and does not change the existing synchronous client. The caller must provide an ``aiohttp.ClientSession`` and remains responsible for closing it. This allows an application to control connection pooling, TLS trust, proxy behavior, and session lifetime in one place. + +The asynchronous client currently supports HTTP Basic authentication. It does not create a Redfish session or require a separate login call. Requests do not follow redirects, and advertised resource or action targets are accepted only when they resolve to the configured Redfish origin. These rules prevent credentials from being sent to another origin. + +.. code-block:: python + + import aiohttp + + from redfish.aio import AsyncRedfishClient + + + async def get_systems(): + async with aiohttp.ClientSession() as session: + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + timeout=10, + discovery_timeout=60, + ) + + service_root = await client.get_service_root() + systems = await client.get_systems() + return service_root, systems + +``get``, ``head``, ``post``, ``put``, ``patch``, and ``delete`` are coroutines with the same ``path``, ``args``, ``body``, ``headers``, and ``timeout`` concepts as the synchronous methods. The returned response is fully read and cached before the coroutine returns, so it can be inspected after the underlying aiohttp response closes. + +``get_systems`` follows the standard ServiceRoot ``Systems`` link, collection pagination, ComputerSystem member links, and reset ActionInfo resources. It returns ``ComputerSystem`` objects containing standard identity, metadata, power state, reset target, and advertised standard reset types. ``reset_system`` sends an advertised reset type to that system's advertised reset target: + +.. code-block:: python + + systems = await client.get_systems() + system = systems["1"] + if "On" in system.reset_types: + await client.reset_system(system, "On") + +The optional request ``timeout`` bounds each HTTP request. ``discovery_timeout`` defaults to 60 seconds and bounds the complete ServiceRoot, collection, member, and ActionInfo discovery operation. TLS verification is controlled entirely by the injected ``ClientSession``. Configure that session with an appropriate CA certificate or SSL context for a Redfish service using a private or self-signed certificate. + Working with tasks ~~~~~~~~~~~~~~~~~~ diff --git a/examples/async_client.py b/examples/async_client.py new file mode 100644 index 0000000..f33078a --- /dev/null +++ b/examples/async_client.py @@ -0,0 +1,38 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +"""Discover ComputerSystem resources with the asynchronous Redfish client.""" + +import asyncio +import os + +import aiohttp + +from redfish.aio import AsyncRedfishClient + + +async def main(): + """Discover and display Redfish ComputerSystem resources.""" + async with aiohttp.ClientSession() as session: + client = AsyncRedfishClient( + base_url=os.environ["REDFISH_BASE_URL"], + username=os.environ["REDFISH_USERNAME"], + password=os.environ["REDFISH_PASSWORD"], + session=session, + timeout=10, + ) + systems = await client.get_systems() + for system in systems.values(): + print( + "{}: power={}, reset_types={}".format( + system.name or system.system_id, + system.power_state, + sorted(system.reset_types), + ) + ) + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/requirements.txt b/requirements.txt index 9840f9a..d78db70 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,4 @@ +aiohttp>=3.9.0 jsonpatch<=1.24 ; python_version == '3.4' jsonpatch ; python_version >= '3.5' jsonpath_ng diff --git a/setup.py b/setup.py index 4e79ea8..c8face9 100644 --- a/setup.py +++ b/setup.py @@ -36,6 +36,9 @@ 'requests-unixsocket' ], extras_require={ + 'aiohttp': [ + 'aiohttp>=3.9.0' + ], ':python_version == "3.4"': [ 'jsonpatch<=1.24' ], diff --git a/src/redfish/aio/__init__.py b/src/redfish/aio/__init__.py new file mode 100644 index 0000000..a33f489 --- /dev/null +++ b/src/redfish/aio/__init__.py @@ -0,0 +1,45 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +"""Asynchronous Redfish client API.""" + +from .client import AsyncRedfishClient +from .exceptions import ( + RedfishAuthenticationError, + RedfishConnectionError, + RedfishError, + RedfishHTTPError, + RedfishInvalidTargetError, + RedfishProtocolError, + RedfishTimeoutError, + RedfishUnsupportedResetError, +) +from .models import ( + STANDARD_RESET_TYPES, + ComputerSystem, + get_reset_action_info_target, + parse_computer_system, + parse_reset_action_info, +) +from .response import AsyncRestRequest, AsyncRestResponse + +__all__ = [ + "AsyncRedfishClient", + "AsyncRestRequest", + "AsyncRestResponse", + "ComputerSystem", + "RedfishAuthenticationError", + "RedfishConnectionError", + "RedfishError", + "RedfishHTTPError", + "RedfishInvalidTargetError", + "RedfishProtocolError", + "RedfishTimeoutError", + "RedfishUnsupportedResetError", + "STANDARD_RESET_TYPES", + "get_reset_action_info_target", + "parse_computer_system", + "parse_reset_action_info", +] diff --git a/src/redfish/aio/client.py b/src/redfish/aio/client.py new file mode 100644 index 0000000..81b2931 --- /dev/null +++ b/src/redfish/aio/client.py @@ -0,0 +1,348 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +"""Asynchronous Redfish client implemented with aiohttp.""" + +import asyncio +import base64 +from dataclasses import replace + +import aiohttp +from multidict import CIMultiDict +from yarl import URL + +from .exceptions import ( + RedfishAuthenticationError, + RedfishConnectionError, + RedfishHTTPError, + RedfishInvalidTargetError, + RedfishProtocolError, + RedfishTimeoutError, + RedfishUnsupportedResetError, +) +from .models import ( + get_reset_action_info_target, + parse_computer_system, + parse_reset_action_info, +) +from .response import AsyncRestRequest, AsyncRestResponse + + +class AsyncRedfishClient: + """Perform asynchronous Redfish requests with an injected session.""" + + def __init__( + self, + base_url, + username=None, + password=None, + session=None, + timeout=None, + default_prefix="/redfish/v1/", + discovery_timeout=60, + ): + if session is None: + raise ValueError( + "A caller-owned aiohttp.ClientSession is required" + ) + if (username is None) != (password is None): + raise ValueError("Username and password must be provided together") + + try: + url = URL(base_url) + except (TypeError, ValueError) as exc: + raise ValueError("Invalid Redfish base URL") from exc + if ( + url.scheme not in ("http", "https") + or url.host is None + or url.user is not None + or url.password is not None + or url.path not in ("", "/") + or url.query_string + or url.fragment + ): + raise ValueError("Invalid Redfish base URL") + + self._base_url = ( + url.with_path("/").with_query(None).with_fragment(None) + ) + self._session = session + self._timeout = self._make_timeout(timeout) + self._default_prefix = default_prefix + self._discovery_timeout = self._make_discovery_timeout( + discovery_timeout + ) + self._authorization = None + if username is not None: + if ":" in username: + raise ValueError("Username cannot contain ':'") + encoded = base64.b64encode( + "{}:{}".format(username, password).encode("utf-8") + ).decode("ascii") + self._authorization = "Basic {}".format(encoded) + + @staticmethod + def _make_timeout(timeout): + if timeout is None or isinstance(timeout, aiohttp.ClientTimeout): + return timeout + if not isinstance(timeout, (int, float)) or timeout < 0: + raise ValueError("Timeout must be a non-negative number") + return aiohttp.ClientTimeout(total=timeout) + + @staticmethod + def _make_discovery_timeout(timeout): + if timeout is not None and ( + not isinstance(timeout, (int, float)) or timeout < 0 + ): + raise ValueError( + "Discovery timeout must be a non-negative number" + ) + return timeout + + def _resolve_url(self, target): + try: + target_url = self._base_url.join(URL(target)) + if target_url.user is not None or target_url.password is not None: + raise RedfishInvalidTargetError( + "Target cannot contain credentials" + ) + if ( + target_url.scheme, + target_url.host, + target_url.port, + ) != ( + self._base_url.scheme, + self._base_url.host, + self._base_url.port, + ): + raise RedfishInvalidTargetError( + "Target must use the configured Redfish origin" + ) + except (TypeError, ValueError) as exc: + raise RedfishInvalidTargetError("Invalid Redfish target") from exc + return target_url + + def _request_headers(self, headers): + request_headers = CIMultiDict( + {"Accept": "*/*", "OData-Version": "4.0"} + ) + if headers is not None: + request_headers.update(headers) + if self._authorization is not None: + request_headers["Authorization"] = self._authorization + return request_headers + + async def _request( + self, + path, + method="GET", + args=None, + body=None, + headers=None, + timeout=None, + ): + request = AsyncRestRequest(path=path, method=method.upper(), body=body) + request_timeout = ( + self._timeout if timeout is None else self._make_timeout(timeout) + ) + kwargs = { + "allow_redirects": False, + "headers": self._request_headers(headers), + "params": args, + } + if request_timeout is not None: + kwargs["timeout"] = request_timeout + if isinstance(body, (dict, list)): + kwargs["json"] = body + elif body is not None: + kwargs["data"] = body + + try: + async with self._session.request( + method.upper(), self._resolve_url(path), **kwargs + ) as response: + content = await response.read() + encoding = response.get_encoding() + return AsyncRestResponse( + request=request, + status=response.status, + headers=response.headers, + read=content, + encoding=encoding, + ) + except asyncio.TimeoutError as exc: + raise RedfishTimeoutError("Redfish request timed out") from exc + except aiohttp.ClientError as exc: + raise RedfishConnectionError("Redfish request failed") from exc + + async def get(self, path, args=None, headers=None, timeout=None): + """Perform a GET request.""" + return await self._request( + path, method="GET", args=args, headers=headers, timeout=timeout + ) + + async def head(self, path, args=None, headers=None, timeout=None): + """Perform a HEAD request.""" + return await self._request( + path, method="HEAD", args=args, headers=headers, timeout=timeout + ) + + async def post( + self, path, args=None, body=None, headers=None, timeout=None + ): + """Perform a POST request.""" + return await self._request( + path, + method="POST", + args=args, + body=body, + headers=headers, + timeout=timeout, + ) + + async def put( + self, path, args=None, body=None, headers=None, timeout=None + ): + """Perform a PUT request.""" + return await self._request( + path, + method="PUT", + args=args, + body=body, + headers=headers, + timeout=timeout, + ) + + async def patch( + self, path, args=None, body=None, headers=None, timeout=None + ): + """Perform a PATCH request.""" + return await self._request( + path, + method="PATCH", + args=args, + body=body, + headers=headers, + timeout=timeout, + ) + + async def delete( + self, path, args=None, headers=None, timeout=None, body=None + ): + """Perform a DELETE request.""" + return await self._request( + path, + method="DELETE", + args=args, + body=body, + headers=headers, + timeout=timeout, + ) + + @staticmethod + def _ensure_success(response): + if response.status in (401, 403): + raise RedfishAuthenticationError( + "Redfish service rejected authentication" + ) + if not 200 <= response.status < 300: + raise RedfishHTTPError(response) + + async def _get_json(self, path): + response = await self.get(path) + self._ensure_success(response) + payload = response.dict + if not isinstance(payload, dict): + raise RedfishProtocolError( + "Redfish resource at {} is not a JSON object".format(path) + ) + return payload + + async def get_service_root(self): + """Return the standard Redfish service root.""" + return await self._get_json(self._default_prefix) + + async def _get_collection_members(self, link): + if ( + not isinstance(link, dict) + or not isinstance(path := link.get("@odata.id"), str) + or not path.strip() + ): + return [] + + payloads = [] + seen_paths = set() + while True: + if path in seen_paths: + raise RedfishProtocolError( + "Redfish collection pagination contains a cycle" + ) + seen_paths.add(path) + collection = await self._get_json(path) + members = collection.get("Members") + if not isinstance(members, list): + return [] + for member in members: + if ( + isinstance(member, dict) + and isinstance(member_path := member.get("@odata.id"), str) + and member_path.strip() + ): + payloads.append(await self._get_json(member_path)) + next_path = collection.get("Members@odata.nextLink") + if not isinstance(next_path, str) or not next_path.strip(): + return payloads + path = next_path + + async def _discover_systems(self): + root = await self.get_service_root() + systems = {} + for payload in await self._get_collection_members(root.get("Systems")): + system = parse_computer_system(payload) + if system is None: + continue + action_info_target = get_reset_action_info_target(payload) + if ( + system.reset_target is not None + and action_info_target is not None + ): + action_info = await self._get_json(action_info_target) + system = replace( + system, + reset_types=system.reset_types + | parse_reset_action_info(action_info), + ) + systems[system.system_id] = system + return systems + + async def get_systems(self): + """Discover ComputerSystem resources from the Redfish service root.""" + try: + return await asyncio.wait_for( + self._discover_systems(), timeout=self._discovery_timeout + ) + except asyncio.TimeoutError as exc: + raise RedfishTimeoutError( + "Redfish system discovery timed out" + ) from exc + + async def reset_system(self, system, reset_type, timeout=None): + """Perform a reset type advertised by a ComputerSystem.""" + if ( + system.reset_target is None + or reset_type not in system.reset_types + ): + raise RedfishUnsupportedResetError( + "ComputerSystem does not advertise ResetType {}".format( + reset_type + ) + ) + response = await self.post( + system.reset_target, + body={"ResetType": reset_type}, + timeout=timeout, + ) + self._ensure_success(response) + return response diff --git a/src/redfish/aio/exceptions.py b/src/redfish/aio/exceptions.py new file mode 100644 index 0000000..7cf28f2 --- /dev/null +++ b/src/redfish/aio/exceptions.py @@ -0,0 +1,44 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +"""Exceptions raised by the asynchronous Redfish client.""" + + +class RedfishError(Exception): + """Base exception for asynchronous Redfish operations.""" + + +class RedfishConnectionError(RedfishError): + """Raised when the Redfish service cannot be reached.""" + + +class RedfishTimeoutError(RedfishConnectionError): + """Raised when a Redfish operation times out.""" + + +class RedfishInvalidTargetError(RedfishError): + """Raised when a target is invalid or outside the configured origin.""" + + +class RedfishAuthenticationError(RedfishError): + """Raised when the Redfish service rejects authentication.""" + + +class RedfishHTTPError(RedfishError): + """Raised when a Redfish service returns an unsuccessful HTTP status.""" + + def __init__(self, response): + super().__init__( + "Redfish request returned HTTP {}".format(response.status) + ) + self.response = response + + +class RedfishProtocolError(RedfishError): + """Raised when a Redfish resource is malformed.""" + + +class RedfishUnsupportedResetError(RedfishError): + """Raised when a reset type is not advertised by a ComputerSystem.""" diff --git a/src/redfish/aio/models.py b/src/redfish/aio/models.py new file mode 100644 index 0000000..7d9c7e4 --- /dev/null +++ b/src/redfish/aio/models.py @@ -0,0 +1,120 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +"""Standard Redfish resource models used by the asynchronous client.""" + +from dataclasses import dataclass +from typing import FrozenSet, Optional + + +STANDARD_RESET_TYPES = frozenset( + { + "ForceOff", + "ForceOn", + "ForceRestart", + "FullPowerCycle", + "GracefulRestart", + "GracefulShutdown", + "Nmi", + "On", + "Pause", + "PowerCycle", + "PushPowerButton", + "Resume", + "Suspend", + } +) + + +@dataclass(frozen=True) +class ComputerSystem: + """Standard properties and reset capabilities of a ComputerSystem.""" + + odata_id: str + system_id: str + name: Optional[str] + uuid: Optional[str] + manufacturer: Optional[str] + model: Optional[str] + serial_number: Optional[str] + power_state: Optional[str] + reset_target: Optional[str] + reset_types: FrozenSet[str] + + +def _non_empty_string(value): + return value if isinstance(value, str) and value.strip() else None + + +def _reset_action(payload): + actions = payload.get("Actions") + if not isinstance(actions, dict): + return None + reset = actions.get("#ComputerSystem.Reset") + return reset if isinstance(reset, dict) else None + + +def get_reset_action_info_target(payload): + """Return the ActionInfo target advertised for ComputerSystem.Reset.""" + reset = _reset_action(payload) + if reset is None: + return None + return _non_empty_string(reset.get("@Redfish.ActionInfo")) + + +def parse_reset_action_info(payload): + """Return standard ResetType values from an ActionInfo resource.""" + parameters = payload.get("Parameters") + if not isinstance(parameters, list): + return frozenset() + for parameter in parameters: + if ( + not isinstance(parameter, dict) + or parameter.get("Name") != "ResetType" + ): + continue + allowable_values = parameter.get("AllowableValues") + if not isinstance(allowable_values, list): + return frozenset() + return frozenset( + value + for value in allowable_values + if isinstance(value, str) and value in STANDARD_RESET_TYPES + ) + return frozenset() + + +def parse_computer_system(payload): + """Parse a ComputerSystem resource or return None when unusable.""" + odata_id = _non_empty_string(payload.get("@odata.id")) + system_id = _non_empty_string(payload.get("Id")) + if odata_id is None or system_id is None: + return None + + reset_target = None + reset_types = frozenset() + reset = _reset_action(payload) + if reset is not None: + reset_target = _non_empty_string(reset.get("target")) + allowable_values = reset.get("ResetType@Redfish.AllowableValues") + if reset_target is not None and isinstance(allowable_values, list): + reset_types = frozenset( + value + for value in allowable_values + if isinstance(value, str) and value in STANDARD_RESET_TYPES + ) + + return ComputerSystem( + odata_id=odata_id, + system_id=system_id, + name=_non_empty_string(payload.get("Name")), + uuid=_non_empty_string(payload.get("UUID")), + manufacturer=_non_empty_string(payload.get("Manufacturer")), + model=_non_empty_string(payload.get("Model")), + serial_number=_non_empty_string(payload.get("SerialNumber")), + power_state=_non_empty_string(payload.get("PowerState")), + reset_target=reset_target, + reset_types=reset_types, + ) diff --git a/src/redfish/aio/response.py b/src/redfish/aio/response.py new file mode 100644 index 0000000..e3935e0 --- /dev/null +++ b/src/redfish/aio/response.py @@ -0,0 +1,75 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +"""Cached request and response objects for asynchronous Redfish operations.""" + +from dataclasses import dataclass +import json + +from multidict import CIMultiDict + +from .exceptions import RedfishProtocolError + + +@dataclass(frozen=True) +class AsyncRestRequest: + """Description of an asynchronous Redfish request.""" + + path: str + method: str = "GET" + body: object = None + + +class AsyncRestResponse: + """Cached response returned by an asynchronous Redfish request.""" + + def __init__(self, request, status, headers, read, encoding="utf-8"): + self._request = request + self._status = status + self._headers = CIMultiDict(headers) + self._read = read + self._encoding = encoding + + @property + def read(self): + """Return the raw response body.""" + return self._read + + @property + def status(self): + """Return the HTTP status code.""" + return self._status + + @property + def text(self): + """Return the decoded response body.""" + return self._read.decode(self._encoding, "replace") + + @property + def dict(self): + """Return the response body decoded as JSON.""" + if not self._read: + return {} + try: + return json.loads(self.text) + except (TypeError, ValueError) as exc: + raise RedfishProtocolError( + "Service responded with invalid JSON at URI {}".format( + self._request.path + ) + ) from exc + + @property + def request(self): + """Return the request that produced this response.""" + return self._request + + def getheaders(self): + """Return all response headers.""" + return list(self._headers.items()) + + def getheader(self, name): + """Return one response header case-insensitively.""" + return self._headers.get(name) diff --git a/tests/aio/__init__.py b/tests/aio/__init__.py new file mode 100644 index 0000000..9c84fd5 --- /dev/null +++ b/tests/aio/__init__.py @@ -0,0 +1,4 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md diff --git a/tests/aio/test_client.py b/tests/aio/test_client.py new file mode 100644 index 0000000..2afd436 --- /dev/null +++ b/tests/aio/test_client.py @@ -0,0 +1,339 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +import asyncio +import unittest + +import aiohttp +from aiohttp import web +from aiohttp.test_utils import TestServer + +from redfish.aio import ( + AsyncRedfishClient, + RedfishConnectionError, + RedfishInvalidTargetError, + RedfishTimeoutError, +) + + +class TestAsyncRedfishClient(unittest.IsolatedAsyncioTestCase): + """Test the asynchronous Redfish HTTP client.""" + + async def asyncSetUp(self): + self.requests = [] + app = web.Application() + + async def response(request): + if request.path == "/slow": + await asyncio.sleep(0.1) + if not request.can_read_body: + body = None + elif request.content_type == "application/json": + body = await request.json() + else: + body = await request.text() + self.requests.append( + { + "method": request.method, + "path_qs": request.path_qs, + "body": body, + "authorization": request.headers.get("Authorization"), + "authorization_all": request.headers.getall( + "Authorization", [] + ), + "accept": request.headers.get("Accept"), + "odata_version": request.headers.get("OData-Version"), + "custom": request.headers.get("X-Custom"), + } + ) + if request.path == "/empty": + return web.Response(status=204) + status = 401 if request.path == "/unauthorized" else 200 + return web.json_response( + {"method": request.method, "path": request.path}, + status=status, + headers={"X-Response": "present"}, + ) + + app.router.add_route("*", "/{path:.*}", response) + self.server = TestServer(app) + await self.server.start_server() + self.session = aiohttp.ClientSession() + self.client = AsyncRedfishClient( + base_url=str(self.server.make_url("/")), + username="user", + password="password", + session=self.session, + ) + + async def asyncTearDown(self): + await self.session.close() + await self.server.close() + + async def test_get_returns_cached_response(self): + """Test GET query parameters and response accessors.""" + response = await self.client.get( + "/resource", + args={"query": "value"}, + headers={"X-Custom": "header"}, + ) + + self.assertEqual(response.status, 200) + self.assertIsInstance(response.read, bytes) + self.assertEqual(response.dict, {"method": "GET", "path": "/resource"}) + self.assertIn(("X-Response", "present"), response.getheaders()) + self.assertEqual(response.getheader("x-response"), "present") + self.assertEqual(response.request.method, "GET") + self.assertEqual(response.request.path, "/resource") + self.assertEqual( + self.requests, + [ + { + "method": "GET", + "path_qs": "/resource?query=value", + "body": None, + "authorization": "Basic dXNlcjpwYXNzd29yZA==", + "authorization_all": [ + "Basic dXNlcjpwYXNzd29yZA==" + ], + "accept": "*/*", + "odata_version": "4.0", + "custom": "header", + } + ], + ) + + async def test_headers_are_case_insensitive_and_auth_is_protected(self): + """Test custom headers cannot duplicate or replace Basic auth.""" + await self.client.get( + "/resource", + headers={ + "accept": "application/json", + "odata-version": "4.01", + "authorization": "Bearer untrusted", + }, + ) + + self.assertEqual( + self.requests[0], + { + "method": "GET", + "path_qs": "/resource", + "body": None, + "authorization": "Basic dXNlcjpwYXNzd29yZA==", + "authorization_all": [ + "Basic dXNlcjpwYXNzd29yZA==" + ], + "accept": "application/json", + "odata_version": "4.01", + "custom": None, + }, + ) + + async def test_write_methods_send_json_body(self): + """Test POST, PUT, PATCH, and DELETE requests.""" + for method_name in ("post", "put", "patch", "delete"): + with self.subTest(method=method_name): + response = await getattr(self.client, method_name)( + "/resource", + args={"query": method_name}, + body={"method": method_name}, + ) + self.assertEqual(response.status, 200) + + self.assertEqual( + [ + (request["method"], request["body"]) + for request in self.requests + ], + [ + ("POST", {"method": "post"}), + ("PUT", {"method": "put"}), + ("PATCH", {"method": "patch"}), + ("DELETE", {"method": "delete"}), + ], + ) + + async def test_head_and_unstructured_body(self): + """Test HEAD requests and unstructured request bodies.""" + response = await self.client.head("/resource") + self.assertEqual(response.status, 200) + + await self.client.post("/resource", body="raw body") + self.assertEqual( + (self.requests[-1]["method"], self.requests[-1]["body"]), + ("POST", "raw body"), + ) + + async def test_caller_owns_session(self): + """Test the client never closes the injected session.""" + await self.client.get("/resource") + + self.assertFalse(self.session.closed) + + async def test_empty_response_has_empty_dictionary(self): + """Test a valid empty response has an empty dictionary body.""" + response = await self.client.post("/empty") + + self.assertEqual(response.status, 204) + self.assertEqual(response.dict, {}) + + async def test_same_origin_absolute_and_scheme_relative_targets(self): + """Test advertised same-origin target forms are accepted.""" + absolute_target = str(self.server.make_url("/absolute")) + scheme_relative_target = str( + self.server.make_url("/scheme-relative").with_scheme("") + ) + + await self.client.post(absolute_target, body={"ResetType": "On"}) + await self.client.post( + scheme_relative_target, body={"ResetType": "GracefulShutdown"} + ) + + self.assertEqual( + [request["path_qs"] for request in self.requests], + ["/absolute", "/scheme-relative"], + ) + + def test_default_ports_share_origin(self): + """Test explicit default ports match their implicit origins.""" + for base_url, target in ( + ("https://bmc.example", "https://bmc.example:443/reset"), + ("http://bmc.example", "http://bmc.example:80/reset"), + ): + with self.subTest(base_url=base_url, target=target): + client = AsyncRedfishClient( + base_url=base_url, session=self.session + ) + self.assertEqual(client._resolve_url(target).path, "/reset") + + async def test_cross_origin_target_is_rejected_before_request(self): + """Test credentials cannot be sent to another origin.""" + malicious_requests = [] + malicious_app = web.Application() + + async def capture_request(request): + malicious_requests.append(request.headers.get("Authorization")) + return web.Response(status=204) + + malicious_app.router.add_route("*", "/{path:.*}", capture_request) + malicious_server = TestServer(malicious_app) + await malicious_server.start_server() + self.addAsyncCleanup(malicious_server.close) + + for target in ( + str(malicious_server.make_url("/reset")), + str(malicious_server.make_url("/reset").with_scheme("")), + ): + with self.subTest(target=target), self.assertRaises( + RedfishInvalidTargetError + ): + await self.client.post(target, body={"ResetType": "On"}) + + self.assertEqual(malicious_requests, []) + + async def test_redirect_is_not_followed(self): + """Test redirects cannot forward credentials to another origin.""" + malicious_requests = [] + malicious_app = web.Application() + + async def capture_request(request): + malicious_requests.append(request.headers.get("Authorization")) + return web.Response(status=204) + + malicious_app.router.add_get("/{path:.*}", capture_request) + malicious_server = TestServer(malicious_app) + await malicious_server.start_server() + self.addAsyncCleanup(malicious_server.close) + + redirect_app = web.Application() + + async def redirect(_request): + raise web.HTTPFound(str(malicious_server.make_url("/target"))) + + redirect_app.router.add_get("/{path:.*}", redirect) + redirect_server = TestServer(redirect_app) + await redirect_server.start_server() + self.addAsyncCleanup(redirect_server.close) + redirect_client = AsyncRedfishClient( + base_url=str(redirect_server.make_url("/")), + username="user", + password="password", + session=self.session, + ) + + response = await redirect_client.get("/redirect") + + self.assertEqual(response.status, 302) + self.assertEqual(malicious_requests, []) + + async def test_default_and_request_timeout(self): + """Test default timeouts and per-request overrides.""" + client = AsyncRedfishClient( + base_url=str(self.server.make_url("/")), + username="user", + password="password", + session=self.session, + timeout=0.01, + ) + + with self.assertRaises(RedfishTimeoutError): + await client.get("/slow") + + response = await client.get("/slow", timeout=0.2) + self.assertEqual(response.status, 200) + + async def test_connection_error_is_translated(self): + """Test aiohttp connection failures use a Redfish exception.""" + server = TestServer(web.Application()) + await server.start_server() + base_url = str(server.make_url("/")) + await server.close() + client = AsyncRedfishClient(base_url=base_url, session=self.session) + + with self.assertRaises(RedfishConnectionError): + await client.get("/resource") + + async def test_client_validation(self): + """Test invalid constructor arguments are rejected.""" + valid = { + "base_url": str(self.server.make_url("/")), + "session": self.session, + } + invalid_arguments = ( + {"base_url": "https://bmc.example"}, + {**valid, "username": "user"}, + {**valid, "password": "password"}, + {**valid, "username": "invalid:user", "password": "password"}, + {**valid, "timeout": -1}, + {**valid, "timeout": "invalid"}, + {**valid, "discovery_timeout": -1}, + {**valid, "discovery_timeout": "invalid"}, + {**valid, "base_url": "https://["}, + {**valid, "base_url": "bmc.example"}, + {**valid, "base_url": "https://user@bmc.example"}, + {**valid, "base_url": "https://bmc.example/redfish"}, + ) + + for arguments in invalid_arguments: + with self.subTest(arguments=arguments), self.assertRaises( + ValueError + ): + AsyncRedfishClient(**arguments) + + async def test_invalid_targets_are_rejected(self): + """Test malformed and credential-bearing targets are rejected.""" + for target in ( + "https://[", + str(self.server.make_url("/resource").with_user("other")), + ): + with self.subTest(target=target), self.assertRaises( + RedfishInvalidTargetError + ): + await self.client.get(target) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/aio/test_models.py b/tests/aio/test_models.py new file mode 100644 index 0000000..825a75d --- /dev/null +++ b/tests/aio/test_models.py @@ -0,0 +1,56 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +import unittest + +from redfish.aio import ( + get_reset_action_info_target, + parse_computer_system, + parse_reset_action_info, +) + + +class TestAsyncResourceModels(unittest.TestCase): + """Test standard resource parsing used by the asynchronous client.""" + + def test_action_info_requires_reset_action(self): + """Test ActionInfo lookup without a reset action.""" + self.assertIsNone(get_reset_action_info_target({})) + + def test_action_info_without_usable_reset_values(self): + """Test malformed ActionInfo parameters produce no reset types.""" + for payload in ( + {}, + {"Parameters": [None, {"Name": "OtherParameter"}]}, + {"Parameters": [{"Name": "ResetType"}]}, + ): + with self.subTest(payload=payload): + self.assertEqual(parse_reset_action_info(payload), frozenset()) + + def test_computer_system_requires_standard_identifiers(self): + """Test systems without usable identifiers are rejected.""" + for payload in ( + {}, + {"Id": "1"}, + {"@odata.id": "/redfish/v1/Systems/1"}, + {"@odata.id": " ", "Id": "1"}, + {"@odata.id": "/redfish/v1/Systems/1", "Id": 1}, + ): + with self.subTest(payload=payload): + self.assertIsNone(parse_computer_system(payload)) + + def test_computer_system_without_reset_action(self): + """Test a usable system need not advertise reset support.""" + system = parse_computer_system( + {"@odata.id": "/redfish/v1/Systems/1", "Id": "1"} + ) + + self.assertIsNotNone(system) + self.assertIsNone(system.reset_target) + self.assertEqual(system.reset_types, frozenset()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/aio/test_systems.py b/tests/aio/test_systems.py new file mode 100644 index 0000000..5f99c32 --- /dev/null +++ b/tests/aio/test_systems.py @@ -0,0 +1,304 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +import asyncio +import unittest + +import aiohttp +from aiohttp import web +from aiohttp.test_utils import TestServer + +from redfish.aio import ( + AsyncRedfishClient, + ComputerSystem, + RedfishAuthenticationError, + RedfishHTTPError, + RedfishProtocolError, + RedfishTimeoutError, + RedfishUnsupportedResetError, +) + + +class TestAsyncComputerSystemDiscovery(unittest.IsolatedAsyncioTestCase): + """Test standard asynchronous ComputerSystem operations.""" + + async def asyncSetUp(self): + self.requests = [] + self.statuses = {} + self.delays = {} + self.raw_responses = {} + self.resources = { + "/redfish/v1/": { + "Systems": {"@odata.id": "/redfish/v1/Systems"}, + }, + "/redfish/v1/Systems": { + "Members": [{"@odata.id": "/redfish/v1/Systems/1"}], + "Members@odata.nextLink": "/redfish/v1/Systems?page=2", + }, + "/redfish/v1/Systems?page=2": { + "Members": [{"@odata.id": "/redfish/v1/Systems/2"}], + }, + "/redfish/v1/Systems/1": { + "@odata.id": "/redfish/v1/Systems/1", + "Id": "1", + "Name": "Server One", + "UUID": "uuid-1", + "Manufacturer": "Acme", + "Model": "Model 1", + "SerialNumber": "serial-1", + "PowerState": "On", + "Actions": { + "#ComputerSystem.Reset": { + "target": ( + "/redfish/v1/Systems/1/Actions/" + "ComputerSystem.Reset" + ), + "ResetType@Redfish.AllowableValues": [ + "On", + "GracefulShutdown", + "VendorReset", + 1, + ], + } + }, + }, + "/redfish/v1/Systems/2": { + "@odata.id": "/redfish/v1/Systems/2", + "Id": "2", + "Name": "Server Two", + "PowerState": "Off", + "Actions": { + "#ComputerSystem.Reset": { + "target": ( + "/redfish/v1/Systems/2/Actions/" + "ComputerSystem.Reset" + ), + "@Redfish.ActionInfo": ( + "/redfish/v1/Systems/2/ResetActionInfo" + ), + } + }, + }, + "/redfish/v1/Systems/2/ResetActionInfo": { + "Parameters": [ + { + "Name": "ResetType", + "AllowableValues": [ + "ForceOff", + "GracefulRestart", + "VendorReset", + ], + } + ] + }, + } + app = web.Application() + + async def response(request): + path = request.path_qs + if delay := self.delays.get(path): + await asyncio.sleep(delay) + body = await request.json() if request.can_read_body else None + self.requests.append((request.method, path, body)) + status = self.statuses.get(path, 200) + if path in self.raw_responses: + return web.Response( + text=self.raw_responses[path], + status=status, + content_type="application/json", + ) + if request.method != "GET": + return web.Response(status=status) + return web.json_response( + self.resources.get(path, {}), status=status + ) + + app.router.add_route("*", "/{path:.*}", response) + self.server = TestServer(app) + await self.server.start_server() + self.session = aiohttp.ClientSession() + self.client = AsyncRedfishClient( + base_url=str(self.server.make_url("/")), + username="user", + password="password", + session=self.session, + ) + + async def asyncTearDown(self): + await self.session.close() + await self.server.close() + + async def test_discovers_paginated_systems_and_metadata(self): + """Test service-root traversal and ComputerSystem parsing.""" + systems = await self.client.get_systems() + + self.assertEqual( + systems, + { + "1": ComputerSystem( + odata_id="/redfish/v1/Systems/1", + system_id="1", + name="Server One", + uuid="uuid-1", + manufacturer="Acme", + model="Model 1", + serial_number="serial-1", + power_state="On", + reset_target=( + "/redfish/v1/Systems/1/Actions/" + "ComputerSystem.Reset" + ), + reset_types=frozenset({"On", "GracefulShutdown"}), + ), + "2": ComputerSystem( + odata_id="/redfish/v1/Systems/2", + system_id="2", + name="Server Two", + uuid=None, + manufacturer=None, + model=None, + serial_number=None, + power_state="Off", + reset_target=( + "/redfish/v1/Systems/2/Actions/" + "ComputerSystem.Reset" + ), + reset_types=frozenset({"ForceOff", "GracefulRestart"}), + ), + }, + ) + self.assertEqual( + [request[1] for request in self.requests], + [ + "/redfish/v1/", + "/redfish/v1/Systems", + "/redfish/v1/Systems/1", + "/redfish/v1/Systems?page=2", + "/redfish/v1/Systems/2", + "/redfish/v1/Systems/2/ResetActionInfo", + ], + ) + + async def test_reset_system_uses_advertised_target_and_type(self): + """Test reset uses the target and ResetType advertised by the BMC.""" + system = (await self.client.get_systems())["2"] + self.requests.clear() + + response = await self.client.reset_system(system, "ForceOff") + + self.assertEqual(response.status, 200) + self.assertEqual( + self.requests, + [ + ( + "POST", + "/redfish/v1/Systems/2/Actions/ComputerSystem.Reset", + {"ResetType": "ForceOff"}, + ) + ], + ) + + async def test_reset_system_rejects_unadvertised_type(self): + """Test an unadvertised reset type never reaches the BMC.""" + system = (await self.client.get_systems())["2"] + self.requests.clear() + + with self.assertRaises(RedfishUnsupportedResetError): + await self.client.reset_system(system, "On") + + self.assertEqual(self.requests, []) + + async def test_missing_and_malformed_systems_are_skipped(self): + """Test unusable collection and ComputerSystem data is skipped.""" + self.resources["/redfish/v1/Systems"] = { + "Members": [ + {}, + {"@odata.id": 1}, + {"@odata.id": " "}, + {"@odata.id": "/redfish/v1/Systems/malformed"}, + ] + } + self.resources["/redfish/v1/Systems/malformed"] = { + "@odata.id": "/redfish/v1/Systems/malformed" + } + + self.assertEqual(await self.client.get_systems(), {}) + + self.resources["/redfish/v1/"] = {} + self.assertEqual(await self.client.get_systems(), {}) + + async def test_repeated_pagination_link_is_rejected(self): + """Test cyclic collection pagination is rejected.""" + self.resources["/redfish/v1/Systems"] = { + "Members": [], + "Members@odata.nextLink": "/redfish/v1/Systems", + } + + with self.assertRaises(RedfishProtocolError): + await self.client.get_systems() + + async def test_discovery_timeout_includes_action_info(self): + """Test the overall discovery deadline includes ActionInfo requests.""" + self.delays["/redfish/v1/Systems/2/ResetActionInfo"] = 0.1 + client = AsyncRedfishClient( + base_url=str(self.server.make_url("/")), + username="user", + password="password", + session=self.session, + discovery_timeout=0.01, + ) + + with self.assertRaises(RedfishTimeoutError): + await client.get_systems() + + async def test_discovery_classifies_http_errors(self): + """Test unsuccessful responses are classified.""" + for status, expected_error in ( + (401, RedfishAuthenticationError), + (403, RedfishAuthenticationError), + (500, RedfishHTTPError), + ): + with self.subTest(status=status): + self.statuses["/redfish/v1/"] = status + with self.assertRaises(expected_error): + await self.client.get_systems() + + async def test_discovery_rejects_malformed_json(self): + """Test malformed JSON is reported as a protocol error.""" + self.raw_responses["/redfish/v1/"] = "{" + + with self.assertRaises(RedfishProtocolError): + await self.client.get_systems() + + async def test_discovery_rejects_non_object_json(self): + """Test a Redfish resource must contain a JSON object.""" + self.raw_responses["/redfish/v1/"] = "[]" + + with self.assertRaises(RedfishProtocolError): + await self.client.get_systems() + + async def test_malformed_collection_members_are_ignored(self): + """Test a collection without a Members array is treated as empty.""" + self.resources["/redfish/v1/Systems"] = {"Members": {}} + + self.assertEqual(await self.client.get_systems(), {}) + + async def test_reset_system_classifies_http_errors(self): + """Test reset HTTP failures are classified.""" + system = (await self.client.get_systems())["1"] + target = "/redfish/v1/Systems/1/Actions/ComputerSystem.Reset" + + for status, expected_error in ( + (401, RedfishAuthenticationError), + (500, RedfishHTTPError), + ): + with self.subTest(status=status): + self.statuses[target] = status + with self.assertRaises(expected_error): + await self.client.reset_system(system, "On") + + +if __name__ == "__main__": + unittest.main() diff --git a/tox.ini b/tox.ini index 751d0d9..20c9123 100644 --- a/tox.ini +++ b/tox.ini @@ -15,7 +15,7 @@ commands = [testenv:pep8] basepython = python3 deps = flake8 -commands = flake8 tests/ src/redfish/discovery +commands = flake8 tests/ src/redfish/discovery src/redfish/aio examples/async_client.py [travis] python = 3.14: py314 From eb2acc3dc38e14ee94c8d32414fe7ea8fa7fdb69 Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sat, 15 Aug 2026 11:11:55 -0400 Subject: [PATCH 2/7] Add session authentication to async client Support standard Redfish login sessions, explicit Basic authentication, safe session cleanup, and read-only session recovery. Protect credentials and avoid retrying state-changing requests automatically. Signed-off-by: Jacob Yundt --- src/redfish/aio/__init__.py | 2 + src/redfish/aio/client.py | 224 +++++++- src/redfish/aio/exceptions.py | 8 + tests/aio/test_auth.py | 978 ++++++++++++++++++++++++++++++++++ tests/aio/test_client.py | 24 +- tests/aio/test_systems.py | 4 - 6 files changed, 1207 insertions(+), 33 deletions(-) create mode 100644 tests/aio/test_auth.py diff --git a/src/redfish/aio/__init__.py b/src/redfish/aio/__init__.py index a33f489..cb57087 100644 --- a/src/redfish/aio/__init__.py +++ b/src/redfish/aio/__init__.py @@ -12,6 +12,7 @@ RedfishError, RedfishHTTPError, RedfishInvalidTargetError, + RedfishPasswordChangeRequiredError, RedfishProtocolError, RedfishTimeoutError, RedfishUnsupportedResetError, @@ -35,6 +36,7 @@ "RedfishError", "RedfishHTTPError", "RedfishInvalidTargetError", + "RedfishPasswordChangeRequiredError", "RedfishProtocolError", "RedfishTimeoutError", "RedfishUnsupportedResetError", diff --git a/src/redfish/aio/client.py b/src/redfish/aio/client.py index 81b2931..169384c 100644 --- a/src/redfish/aio/client.py +++ b/src/redfish/aio/client.py @@ -18,6 +18,7 @@ RedfishConnectionError, RedfishHTTPError, RedfishInvalidTargetError, + RedfishPasswordChangeRequiredError, RedfishProtocolError, RedfishTimeoutError, RedfishUnsupportedResetError, @@ -42,6 +43,8 @@ def __init__( timeout=None, default_prefix="/redfish/v1/", discovery_timeout=60, + session_key=None, + session_location=None, ): if session is None: raise ValueError( @@ -49,6 +52,8 @@ def __init__( ) if (username is None) != (password is None): raise ValueError("Username and password must be provided together") + if session_location is not None and session_key is None: + raise ValueError("Session location requires a session key") try: url = URL(base_url) @@ -74,14 +79,129 @@ def __init__( self._discovery_timeout = self._make_discovery_timeout( discovery_timeout ) + self._auth_lock = asyncio.Lock() + self._username = username + self._password = password + if session_key is not None and ( + not isinstance(session_key, str) or not session_key.strip() + ): + raise ValueError("Session key must be a non-empty string") + if session_key is not None and self._base_url.scheme != "https": + raise ValueError("Redfish authentication requires HTTPS") + if session_location is not None and ( + not isinstance(session_location, str) + or not session_location.strip() + ): + raise ValueError("Session location must be a non-empty string") + if session_location is not None: + self._resolve_url(session_location) + self._session_key = session_key + self._session_location = session_location self._authorization = None - if username is not None: - if ":" in username: - raise ValueError("Username cannot contain ':'") + + async def __aenter__(self): + await self.login() + return self + + async def __aexit__(self, exc_type, exc_value, traceback): + await self.logout() + + async def login(self, auth="session"): + """Authenticate with the Redfish service.""" + if auth not in ("basic", "session"): + raise ValueError("Unsupported Redfish authentication method") + if self._username is None: + raise ValueError("Username and password are required") + if self._base_url.scheme != "https": + raise ValueError("Redfish authentication requires HTTPS") + if self._session_key is not None or self._authorization is not None: + await self.logout() + if auth == "basic": + if ":" in self._username: + raise ValueError( + "Basic authentication username cannot contain ':'" + ) encoded = base64.b64encode( - "{}:{}".format(username, password).encode("utf-8") + "{}:{}".format( + self._username, self._password + ).encode("utf-8") ).decode("ascii") + self._session_key = None + self._session_location = None self._authorization = "Basic {}".format(encoded) + return + + async with self._auth_lock: + await self._login_session() + + async def _login_session(self): + root = await self._get_json( + self._default_prefix, authenticated=False + ) + links = root.get("Links") + sessions = links.get("Sessions") if isinstance(links, dict) else None + target = ( + sessions.get("@odata.id") + if isinstance(sessions, dict) + else None + ) + if not isinstance(target, str) or not target.strip(): + target = "/redfish/v1/SessionService/Sessions" + response = await self._request( + target, + method="POST", + body={"UserName": self._username, "Password": self._password}, + authenticated=False, + sensitive_body=True, + ) + self._ensure_success(response) + session_key = response.getheader("X-Auth-Token") + session_location = response.getheader("Location") + if ( + not isinstance(session_location, str) + or not session_location.strip() + ): + response_body = response.dict + if isinstance(response_body, dict): + session_location = response_body.get("@odata.id") + if ( + not isinstance(session_key, str) + or not session_key.strip() + or not isinstance(session_location, str) + or not session_location.strip() + ): + raise RedfishProtocolError( + "Redfish session response is missing authentication data" + ) + self._resolve_url(session_location) + self._authorization = None + self._session_key = session_key + self._session_location = session_location + + async def _refresh_session(self, expired_session_key): + async with self._auth_lock: + if self._session_key != expired_session_key: + return self._session_key is not None + self._session_key = None + self._session_location = None + await self._login_session() + return True + + async def logout(self): + """Terminate Redfish login without closing the transport.""" + response = None + try: + if ( + self._session_key is not None + and self._session_location is not None + ): + response = await self.delete(self._session_location) + finally: + self._session_key = None + self._session_location = None + self._authorization = None + if response is not None and response.status not in (401, 404): + self._ensure_success(response) @staticmethod def _make_timeout(timeout): @@ -124,13 +244,20 @@ def _resolve_url(self, target): raise RedfishInvalidTargetError("Invalid Redfish target") from exc return target_url - def _request_headers(self, headers): + def _request_headers(self, headers, authenticated=True): request_headers = CIMultiDict( {"Accept": "*/*", "OData-Version": "4.0"} ) if headers is not None: request_headers.update(headers) - if self._authorization is not None: + if not authenticated: + request_headers.popall("Authorization", None) + request_headers.popall("X-Auth-Token", None) + elif self._session_key is not None: + request_headers.popall("Authorization", None) + request_headers["X-Auth-Token"] = self._session_key + elif self._authorization is not None: + request_headers.popall("X-Auth-Token", None) request_headers["Authorization"] = self._authorization return request_headers @@ -142,14 +269,21 @@ async def _request( body=None, headers=None, timeout=None, + authenticated=True, + allow_session_refresh=True, + sensitive_body=False, ): - request = AsyncRestRequest(path=path, method=method.upper(), body=body) + request = AsyncRestRequest( + path=path, + method=method.upper(), + body=None if sensitive_body else body, + ) request_timeout = ( self._timeout if timeout is None else self._make_timeout(timeout) ) kwargs = { "allow_redirects": False, - "headers": self._request_headers(headers), + "headers": self._request_headers(headers, authenticated), "params": args, } if request_timeout is not None: @@ -159,13 +293,14 @@ async def _request( elif body is not None: kwargs["data"] = body + session_key = self._session_key try: async with self._session.request( method.upper(), self._resolve_url(path), **kwargs ) as response: content = await response.read() encoding = response.get_encoding() - return AsyncRestResponse( + cached_response = AsyncRestResponse( request=request, status=response.status, headers=response.headers, @@ -176,6 +311,27 @@ async def _request( raise RedfishTimeoutError("Redfish request timed out") from exc except aiohttp.ClientError as exc: raise RedfishConnectionError("Redfish request failed") from exc + if ( + cached_response.status == 401 + and method.upper() in ("GET", "HEAD") + and authenticated + and allow_session_refresh + and session_key is not None + and self._username is not None + ): + if await self._refresh_session(session_key): + return await self._request( + path, + method=method, + args=args, + body=body, + headers=headers, + timeout=timeout, + authenticated=authenticated, + allow_session_refresh=False, + sensitive_body=sensitive_body, + ) + return cached_response async def get(self, path, args=None, headers=None, timeout=None): """Perform a GET request.""" @@ -243,6 +399,11 @@ async def delete( @staticmethod def _ensure_success(response): + password_change_uri = AsyncRedfishClient._get_password_change_uri( + response + ) + if password_change_uri is not False: + raise RedfishPasswordChangeRequiredError(password_change_uri) if response.status in (401, 403): raise RedfishAuthenticationError( "Redfish service rejected authentication" @@ -250,8 +411,49 @@ def _ensure_success(response): if not 200 <= response.status < 300: raise RedfishHTTPError(response) - async def _get_json(self, path): - response = await self.get(path) + @staticmethod + def _get_password_change_uri(response): + try: + payload = response.dict + except RedfishProtocolError: + return False + if not isinstance(payload, dict): + return False + error = payload.get("error") + if not isinstance(error, dict): + return False + code = error.get("code") + if ( + isinstance(code, str) + and code.startswith("Base.") + and code.endswith(".PasswordChangeRequired") + ): + return None + extended_info = error.get("@Message.ExtendedInfo") + if not isinstance(extended_info, list): + return False + for message in extended_info: + if ( + not isinstance(message, dict) + or not isinstance(message_id := message.get("MessageId"), str) + or not message_id.startswith("Base.") + or not message_id.endswith(".PasswordChangeRequired") + ): + continue + message_args = message.get("MessageArgs") + if ( + isinstance(message_args, list) + and message_args + and isinstance(message_args[0], str) + ): + return message_args[0] + return None + return False + + async def _get_json(self, path, authenticated=True): + response = await self._request( + path, method="GET", authenticated=authenticated + ) self._ensure_success(response) payload = response.dict if not isinstance(payload, dict): diff --git a/src/redfish/aio/exceptions.py b/src/redfish/aio/exceptions.py index 7cf28f2..04ce90b 100644 --- a/src/redfish/aio/exceptions.py +++ b/src/redfish/aio/exceptions.py @@ -26,6 +26,14 @@ class RedfishAuthenticationError(RedfishError): """Raised when the Redfish service rejects authentication.""" +class RedfishPasswordChangeRequiredError(RedfishAuthenticationError): + """Raised when authentication requires a password change.""" + + def __init__(self, password_change_uri=None): + super().__init__("Redfish service requires a password change") + self.password_change_uri = password_change_uri + + class RedfishHTTPError(RedfishError): """Raised when a Redfish service returns an unsuccessful HTTP status.""" diff --git a/tests/aio/test_auth.py b/tests/aio/test_auth.py new file mode 100644 index 0000000..30c8918 --- /dev/null +++ b/tests/aio/test_auth.py @@ -0,0 +1,978 @@ +# Copyright Notice: +# Copyright 2016-2026 DMTF. All rights reserved. +# License: BSD 3-Clause License. For full text see link: +# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md + +import asyncio +import json +import unittest + +from multidict import CIMultiDict + +from redfish.aio import ( + AsyncRedfishClient, + RedfishAuthenticationError, + RedfishHTTPError, + RedfishInvalidTargetError, + RedfishPasswordChangeRequiredError, + RedfishProtocolError, +) + + +class FakeResponse: + """Minimal aiohttp response context manager.""" + + def __init__(self, status=200, headers=None, body=b"{}"): + self.status = status + self.headers = CIMultiDict(headers or {}) + self._body = body + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc_value, traceback): + return None + + async def read(self): + return self._body + + def get_encoding(self): + return "utf-8" + + +class BarrierResponse(FakeResponse): + """Response that releases after every peer has started reading.""" + + def __init__(self, barrier): + super().__init__(status=401) + self._barrier = barrier + + async def read(self): + self._barrier["count"] += 1 + if self._barrier["count"] == 2: + self._barrier["event"].set() + await self._barrier["event"].wait() + return self._body + + +class ControlledResponse(FakeResponse): + """Response controlled by test events.""" + + def __init__(self, started, release): + super().__init__(status=401) + self._started = started + self._release = release + + async def read(self): + self._started.set() + await self._release.wait() + return self._body + + +class FakeSession: + """Minimal caller-owned session for authentication tests.""" + + closed = False + + def __init__(self, responses=None): + self.requests = [] + self.responses = list(responses or []) + + def request(self, method, url, **kwargs): + self.requests.append( + { + "method": method, + "url": str(url), + "headers": CIMultiDict(kwargs["headers"]), + "body": kwargs.get("json", kwargs.get("data")), + } + ) + return self.responses.pop(0) + + +class TestAsyncRedfishAuthentication(unittest.IsolatedAsyncioTestCase): + """Test asynchronous Redfish authentication.""" + + async def test_credentials_require_https(self): + """Test credentials are never sent over plain HTTP.""" + session = FakeSession() + client = AsyncRedfishClient( + base_url="http://bmc.example", + username="user", + password="password", + session=session, + ) + + for auth in ("basic", "session"): + with self.subTest(auth=auth), self.assertRaises(ValueError): + await client.login(auth=auth) + + self.assertEqual(session.requests, []) + + async def test_colon_in_username_is_valid_only_for_session_auth(self): + """Test the Basic-only username restriction is mode specific.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="domain:user", + password="password", + session=session, + ) + + await client.login(auth="session") + + self.assertEqual( + session.requests[1]["body"]["UserName"], "domain:user" + ) + basic_client = AsyncRedfishClient( + base_url="https://bmc.example", + username="domain:user", + password="password", + session=FakeSession(), + ) + with self.assertRaises(ValueError): + await basic_client.login(auth="basic") + + async def test_credentials_are_inactive_until_login(self): + """Test constructing a client does not begin authentication.""" + session = FakeSession([FakeResponse()]) + client = AsyncRedfishClient( + base_url="http://bmc.example", + username="user", + password="password", + session=session, + ) + + await client.get("/redfish/v1/") + + self.assertIsNone( + session.requests[0]["headers"].get("Authorization") + ) + + async def test_basic_authentication_protects_auth_headers(self): + """Test Basic authentication cannot be replaced or duplicated.""" + session = FakeSession([FakeResponse()]) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login(auth="basic") + + await client.get( + "/redfish/v1/Systems", + headers={ + "authorization": "Bearer untrusted", + "x-auth-token": "untrusted-token", + }, + ) + + headers = session.requests[0]["headers"] + self.assertEqual( + headers.getall("Authorization"), + ["Basic dXNlcjpwYXNzd29yZA=="], + ) + self.assertIsNone(headers.get("X-Auth-Token")) + + async def test_basic_logout_clears_authentication(self): + """Test Basic logout clears credentials without an HTTP request.""" + session = FakeSession([FakeResponse()]) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login(auth="basic") + + await client.logout() + await client.get("/redfish/v1/") + + self.assertEqual(len(session.requests), 1) + self.assertIsNone( + session.requests[0]["headers"].get("Authorization") + ) + + async def test_session_authentication_protects_auth_headers(self): + """Test session authentication cannot be replaced or duplicated.""" + session = FakeSession([FakeResponse()]) + client = AsyncRedfishClient( + base_url="https://bmc.example", + session=session, + session_key="session-token", + ) + + await client.get( + "/redfish/v1/Systems", + headers={ + "authorization": "Bearer untrusted", + "x-auth-token": "untrusted-token", + }, + ) + + headers = session.requests[0]["headers"] + self.assertIsNone(headers.get("Authorization")) + self.assertEqual(headers.getall("X-Auth-Token"), ["session-token"]) + + async def test_session_login_uses_advertised_target_and_token(self): + """Test session authentication follows standard advertised data.""" + session = FakeSession( + [ + FakeResponse( + body=( + b'{"Links":{"Sessions":{"@odata.id":' + b'"/redfish/v1/SessionService/Sessions"}}}' + ) + ), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + body=( + b'{"@odata.id":' + b'"/redfish/v1/SessionService/Sessions/1"}' + ), + ), + FakeResponse(body=b'{"Id":"1"}'), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + await client.login(auth="session") + await client.get("/redfish/v1/Systems/1") + + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "GET"], + ) + self.assertEqual( + [request["url"] for request in session.requests], + [ + "https://bmc.example/redfish/v1/", + ( + "https://bmc.example/redfish/v1/SessionService/" + "Sessions" + ), + "https://bmc.example/redfish/v1/Systems/1", + ], + ) + self.assertIsNone( + session.requests[0]["headers"].get("Authorization") + ) + self.assertIsNone( + session.requests[1]["headers"].get("Authorization") + ) + self.assertEqual( + session.requests[1]["body"], + {"UserName": "user", "Password": "password"}, + ) + self.assertEqual( + session.requests[2]["headers"].get("X-Auth-Token"), + "session-token", + ) + self.assertIsNone( + session.requests[2]["headers"].get("Authorization") + ) + + async def test_logout_deletes_session_without_closing_transport(self): + """Test logout deletes only the Redfish login session.""" + session = FakeSession( + [ + FakeResponse( + body=( + b'{"Links":{"Sessions":{"@odata.id":' + b'"/redfish/v1/SessionService/Sessions"}}}' + ) + ), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + FakeResponse(status=204, body=b""), + FakeResponse(), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + await client.logout() + await client.get("/redfish/v1/") + + self.assertEqual(session.requests[2]["method"], "DELETE") + self.assertEqual( + session.requests[2]["url"], + "https://bmc.example/redfish/v1/SessionService/Sessions/1", + ) + self.assertEqual( + session.requests[2]["headers"].get("X-Auth-Token"), + "session-token", + ) + self.assertIsNone( + session.requests[3]["headers"].get("X-Auth-Token") + ) + self.assertFalse(session.closed) + + async def test_logout_accepts_already_expired_session(self): + """Test logout succeeds when the BMC has already removed a session.""" + for status in (401, 404): + with self.subTest(status=status): + session = FakeSession([FakeResponse(status=status)]) + client = AsyncRedfishClient( + base_url="https://bmc.example", + session=session, + session_key="expired-token", + session_location=( + "/redfish/v1/SessionService/Sessions/1" + ), + ) + + await client.logout() + + self.assertEqual(len(session.requests), 1) + + async def test_context_manager_owns_only_redfish_session(self): + """Test the async context manager logs in and out.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + FakeResponse(status=204, body=b""), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + async with client as entered_client: + self.assertIs(entered_client, client) + + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "DELETE"], + ) + self.assertFalse(session.closed) + + async def test_manual_relogin_terminates_previous_session(self): + """Test an explicit second login does not leak a BMC session.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "old-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + FakeResponse(status=204, body=b""), + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "new-token", + "Location": "/redfish/v1/SessionService/Sessions/2", + }, + ), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + await client.login() + + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "DELETE", "GET", "POST"], + ) + self.assertEqual( + session.requests[2]["headers"].get("X-Auth-Token"), + "old-token", + ) + + async def test_password_change_required_is_classified(self): + """Test session login reports a required password change.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=401, + body=( + b'{"error":{"@Message.ExtendedInfo":[{' + b'"MessageId":"Base.1.18.PasswordChangeRequired",' + b'"MessageArgs":["/redfish/v1/AccountService/' + b'Accounts/1"]}]}}' + ), + ), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises( + RedfishPasswordChangeRequiredError + ) as context: + await client.login() + + self.assertEqual( + context.exception.password_change_uri, + "/redfish/v1/AccountService/Accounts/1", + ) + + async def test_password_change_code_without_uri_is_classified(self): + """Test a password-change error does not require MessageArgs.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=401, + body=( + b'{"error":{"code":' + b'"Base.1.18.PasswordChangeRequired"}}' + ), + ), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises( + RedfishPasswordChangeRequiredError + ) as context: + await client.login() + + self.assertIsNone(context.exception.password_change_uri) + + async def test_password_change_extended_info_without_uri_is_classified( + self, + ): + """Test password-change extended information can omit MessageArgs.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=401, + body=( + b'{"error":{"@Message.ExtendedInfo":[{' + b'"MessageId":' + b'"Base.1.18.PasswordChangeRequired"}]}}' + ), + ), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises( + RedfishPasswordChangeRequiredError + ) as context: + await client.login() + + self.assertIsNone(context.exception.password_change_uri) + + async def test_other_authentication_messages_are_not_reclassified(self): + """Test unrelated Redfish messages remain authentication errors.""" + bodies = ( + b'{"error":{"code":"Base.1.18.GeneralError"}}', + ( + b'{"error":{"@Message.ExtendedInfo":[{' + b'"MessageId":"Base.1.18.GeneralError"}]}}' + ), + ) + for body in bodies: + with self.subTest(body=body): + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse(status=401, body=body), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishAuthenticationError): + await client.login() + + async def test_password_change_is_classified_for_basic_request(self): + """Test Basic-authenticated operations report password changes.""" + session = FakeSession( + [ + FakeResponse( + status=401, + body=( + b'{"error":{"code":' + b'"Base.1.18.PasswordChangeRequired"}}' + ), + ) + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login(auth="basic") + + with self.assertRaises(RedfishPasswordChangeRequiredError): + await client.get_systems() + + async def test_login_error_does_not_retain_credentials(self): + """Test a session-creation error cannot expose its credential body.""" + session = FakeSession( + [FakeResponse(body=b"{}"), FakeResponse(status=500)] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishHTTPError) as context: + await client.login() + + self.assertIsNone(context.exception.response.request.body) + + async def test_invalid_session_credentials_are_classified(self): + """Test a rejected session login raises an authentication error.""" + session = FakeSession( + [FakeResponse(body=b"{}"), FakeResponse(status=401)] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishAuthenticationError): + await client.login() + + async def test_malformed_session_response_is_rejected(self): + """Test session creation requires a token and location.""" + for headers in ( + {"Location": "/redfish/v1/SessionService/Sessions/1"}, + {"X-Auth-Token": "session-token"}, + ): + with self.subTest(headers=headers): + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse(status=201, headers=headers), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishProtocolError): + await client.login() + + async def test_session_targets_must_use_configured_origin(self): + """Test session credentials and tokens stay on their BMC origin.""" + for root, login_headers in ( + ( + { + "Links": { + "Sessions": { + "@odata.id": ( + "https://attacker.example/redfish/v1/Sessions" + ) + } + } + }, + None, + ), + ( + {}, + { + "X-Auth-Token": "session-token", + "Location": ( + "https://attacker.example/redfish/v1/Sessions/1" + ), + }, + ), + ): + with self.subTest(root=root, login_headers=login_headers): + responses = [ + FakeResponse( + body=json.dumps(root).encode("utf-8") + ) + ] + if login_headers is not None: + responses.append( + FakeResponse(status=201, headers=login_headers) + ) + session = FakeSession(responses) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishInvalidTargetError): + await client.login() + + async def test_authentication_arguments_are_validated(self): + """Test invalid authentication configuration is rejected.""" + session = FakeSession() + invalid_constructors = ( + {"session_key": "token", "base_url": "http://bmc.example"}, + {"session_key": ""}, + {"session_key": "token", "session_location": ""}, + {"session_location": "/redfish/v1/Sessions/1"}, + ) + for arguments in invalid_constructors: + with self.subTest(arguments=arguments), self.assertRaises( + ValueError + ): + AsyncRedfishClient( + base_url=arguments.get( + "base_url", "https://bmc.example" + ), + session=session, + **{ + key: value + for key, value in arguments.items() + if key != "base_url" + }, + ) + + client = AsyncRedfishClient( + base_url="https://bmc.example", session=session + ) + for auth in ("session", "invalid"): + with self.subTest(auth=auth), self.assertRaises(ValueError): + await client.login(auth=auth) + + async def test_session_location_can_come_from_response_body(self): + """Test a missing Location header uses the standard response link.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={"X-Auth-Token": "session-token"}, + body=( + b'{"@odata.id":' + b'"/redfish/v1/SessionService/Sessions/1"}' + ), + ), + FakeResponse(status=204, body=b""), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + await client.login() + await client.logout() + + self.assertEqual( + session.requests[2]["url"], + "https://bmc.example/redfish/v1/SessionService/Sessions/1", + ) + + async def test_existing_session_token_can_be_injected(self): + """Test a caller can use and terminate an existing Redfish session.""" + session = FakeSession( + [FakeResponse(), FakeResponse(status=204, body=b"")] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + session=session, + session_key="existing-token", + session_location="/redfish/v1/SessionService/Sessions/42", + ) + + await client.get("/redfish/v1/Systems") + await client.logout() + + self.assertEqual( + [ + request["headers"].get("X-Auth-Token") + for request in session.requests + ], + ["existing-token", "existing-token"], + ) + self.assertEqual(session.requests[1]["method"], "DELETE") + + async def test_expired_session_is_refreshed_for_get(self): + """Test a failed session token is refreshed once for a safe read.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "old-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + FakeResponse(status=401), + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "new-token", + "Location": "/redfish/v1/SessionService/Sessions/2", + }, + ), + FakeResponse(body=b'{"Id":"1"}'), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + response = await client.get("/redfish/v1/Systems/1") + + self.assertEqual(response.status, 200) + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "GET", "GET", "POST", "GET"], + ) + self.assertEqual( + session.requests[2]["headers"].get("X-Auth-Token"), + "old-token", + ) + self.assertEqual( + session.requests[5]["headers"].get("X-Auth-Token"), + "new-token", + ) + + async def test_session_refresh_is_attempted_only_once(self): + """Test a second authentication failure is returned to the caller.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "old-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + FakeResponse(status=401), + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "new-token", + "Location": "/redfish/v1/SessionService/Sessions/2", + }, + ), + FakeResponse(status=401), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + response = await client.get("/redfish/v1/Systems/1") + + self.assertEqual(response.status, 401) + self.assertEqual(len(session.requests), 6) + + async def test_expired_session_does_not_retry_write(self): + """Test a failed authenticated write is never retried.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + FakeResponse(status=401), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + response = await client.post( + "/redfish/v1/Systems/1/Actions/ComputerSystem.Reset", + body={"ResetType": "On"}, + ) + + self.assertEqual(response.status, 401) + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "POST"], + ) + + async def test_concurrent_expiration_creates_one_new_session(self): + """Test concurrent failed reads share one session refresh.""" + barrier = {"count": 0, "event": asyncio.Event()} + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "old-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + BarrierResponse(barrier), + BarrierResponse(barrier), + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "new-token", + "Location": "/redfish/v1/SessionService/Sessions/2", + }, + ), + FakeResponse(), + FakeResponse(), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + responses = await asyncio.gather( + client.get("/redfish/v1/Systems/1"), + client.get("/redfish/v1/Systems/2"), + ) + + self.assertEqual( + [response.status for response in responses], [200, 200] + ) + self.assertEqual( + len( + [ + request + for request in session.requests + if request["method"] == "POST" + and request["url"].endswith("SessionService/Sessions") + ] + ), + 2, + ) + + async def test_logout_during_failed_read_does_not_reauthenticate(self): + """Test an explicit logout wins a race with session recovery.""" + started = asyncio.Event() + release = asyncio.Event() + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": "/redfish/v1/SessionService/Sessions/1", + }, + ), + ControlledResponse(started, release), + FakeResponse(status=204, body=b""), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + await client.login() + + request = asyncio.create_task(client.get("/redfish/v1/Systems/1")) + await started.wait() + await client.logout() + release.set() + response = await request + + self.assertEqual(response.status, 401) + self.assertEqual( + [item["method"] for item in session.requests], + ["GET", "POST", "GET", "DELETE"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/aio/test_client.py b/tests/aio/test_client.py index 2afd436..5030e24 100644 --- a/tests/aio/test_client.py +++ b/tests/aio/test_client.py @@ -63,8 +63,6 @@ async def response(request): self.session = aiohttp.ClientSession() self.client = AsyncRedfishClient( base_url=str(self.server.make_url("/")), - username="user", - password="password", session=self.session, ) @@ -94,10 +92,8 @@ async def test_get_returns_cached_response(self): "method": "GET", "path_qs": "/resource?query=value", "body": None, - "authorization": "Basic dXNlcjpwYXNzd29yZA==", - "authorization_all": [ - "Basic dXNlcjpwYXNzd29yZA==" - ], + "authorization": None, + "authorization_all": [], "accept": "*/*", "odata_version": "4.0", "custom": "header", @@ -105,14 +101,13 @@ async def test_get_returns_cached_response(self): ], ) - async def test_headers_are_case_insensitive_and_auth_is_protected(self): - """Test custom headers cannot duplicate or replace Basic auth.""" + async def test_headers_are_case_insensitive(self): + """Test custom headers replace default headers case-insensitively.""" await self.client.get( "/resource", headers={ "accept": "application/json", "odata-version": "4.01", - "authorization": "Bearer untrusted", }, ) @@ -122,10 +117,8 @@ async def test_headers_are_case_insensitive_and_auth_is_protected(self): "method": "GET", "path_qs": "/resource", "body": None, - "authorization": "Basic dXNlcjpwYXNzd29yZA==", - "authorization_all": [ - "Basic dXNlcjpwYXNzd29yZA==" - ], + "authorization": None, + "authorization_all": [], "accept": "application/json", "odata_version": "4.01", "custom": None, @@ -259,8 +252,6 @@ async def redirect(_request): self.addAsyncCleanup(redirect_server.close) redirect_client = AsyncRedfishClient( base_url=str(redirect_server.make_url("/")), - username="user", - password="password", session=self.session, ) @@ -273,8 +264,6 @@ async def test_default_and_request_timeout(self): """Test default timeouts and per-request overrides.""" client = AsyncRedfishClient( base_url=str(self.server.make_url("/")), - username="user", - password="password", session=self.session, timeout=0.01, ) @@ -306,7 +295,6 @@ async def test_client_validation(self): {"base_url": "https://bmc.example"}, {**valid, "username": "user"}, {**valid, "password": "password"}, - {**valid, "username": "invalid:user", "password": "password"}, {**valid, "timeout": -1}, {**valid, "timeout": "invalid"}, {**valid, "discovery_timeout": -1}, diff --git a/tests/aio/test_systems.py b/tests/aio/test_systems.py index 5f99c32..b13fb4a 100644 --- a/tests/aio/test_systems.py +++ b/tests/aio/test_systems.py @@ -121,8 +121,6 @@ async def response(request): self.session = aiohttp.ClientSession() self.client = AsyncRedfishClient( base_url=str(self.server.make_url("/")), - username="user", - password="password", session=self.session, ) @@ -244,8 +242,6 @@ async def test_discovery_timeout_includes_action_info(self): self.delays["/redfish/v1/Systems/2/ResetActionInfo"] = 0.1 client = AsyncRedfishClient( base_url=str(self.server.make_url("/")), - username="user", - password="password", session=self.session, discovery_timeout=0.01, ) From 8ff07bcef17defc2e023c96bc1b9ead8e86d240e Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sat, 15 Aug 2026 11:11:59 -0400 Subject: [PATCH 3/7] Document async authentication lifecycle Document session authentication, Basic authentication, explicit cleanup, session recovery, and caller-owned transport behavior. Signed-off-by: Jacob Yundt --- README.rst | 19 +++++++++++++------ examples/async_client.py | 20 ++++++++++---------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/README.rst b/README.rst index 436a4d1..64cccab 100644 --- a/README.rst +++ b/README.rst @@ -196,7 +196,9 @@ Asynchronous client The additive asynchronous API uses ``aiohttp`` and does not change the existing synchronous client. The caller must provide an ``aiohttp.ClientSession`` and remains responsible for closing it. This allows an application to control connection pooling, TLS trust, proxy behavior, and session lifetime in one place. -The asynchronous client currently supports HTTP Basic authentication. It does not create a Redfish session or require a separate login call. Requests do not follow redirects, and advertised resource or action targets are accepted only when they resolve to the configured Redfish origin. These rules prevent credentials from being sent to another origin. +The asynchronous client supports Redfish session authentication and HTTP Basic authentication. Authentication is explicit: call ``login`` after creating the client and ``logout`` when finished. ``login`` uses Redfish session authentication by default, matching the synchronous client. Redfish authentication requires HTTPS. + +The asynchronous context manager creates and terminates a Redfish session. It does not close the caller's ``aiohttp.ClientSession``: .. code-block:: python @@ -207,18 +209,23 @@ The asynchronous client currently supports HTTP Basic authentication. It does n async def get_systems(): async with aiohttp.ClientSession() as session: - client = AsyncRedfishClient( + async with AsyncRedfishClient( base_url="https://bmc.example", username="user", password="password", session=session, timeout=10, discovery_timeout=60, - ) + ) as client: + service_root = await client.get_service_root() + systems = await client.get_systems() + return service_root, systems + +To use HTTP Basic authentication, call ``await client.login(auth="basic")`` and ensure ``await client.logout()`` is called when finished. An existing Redfish session can be supplied with the ``session_key`` argument and, when available, its resource URI with ``session_location``. Supplying the location allows ``logout`` to terminate that session. + +If an authenticated ``GET`` or ``HEAD`` receives HTTP 401, the client re-establishes an expired Redfish session once when credentials are available. State-changing requests are never retried automatically. Callers can therefore decide whether it is safe to repeat a failed ``POST``, ``PUT``, ``PATCH``, or ``DELETE``. - service_root = await client.get_service_root() - systems = await client.get_systems() - return service_root, systems +Requests do not follow redirects, and advertised resource, action, and session targets are accepted only when they resolve to the configured Redfish origin. Authentication headers provided by the caller cannot replace the client's configured Basic credentials or session token. These rules prevent credentials from being sent to another origin. ``get``, ``head``, ``post``, ``put``, ``patch``, and ``delete`` are coroutines with the same ``path``, ``args``, ``body``, ``headers``, and ``timeout`` concepts as the synchronous methods. The returned response is fully read and cached before the coroutine returns, so it can be inspected after the underlying aiohttp response closes. diff --git a/examples/async_client.py b/examples/async_client.py index f33078a..3dea218 100644 --- a/examples/async_client.py +++ b/examples/async_client.py @@ -16,22 +16,22 @@ async def main(): """Discover and display Redfish ComputerSystem resources.""" async with aiohttp.ClientSession() as session: - client = AsyncRedfishClient( + async with AsyncRedfishClient( base_url=os.environ["REDFISH_BASE_URL"], username=os.environ["REDFISH_USERNAME"], password=os.environ["REDFISH_PASSWORD"], session=session, timeout=10, - ) - systems = await client.get_systems() - for system in systems.values(): - print( - "{}: power={}, reset_types={}".format( - system.name or system.system_id, - system.power_state, - sorted(system.reset_types), + ) as client: + systems = await client.get_systems() + for system in systems.values(): + print( + "{}: power={}, reset_types={}".format( + system.name or system.system_id, + system.power_state, + sorted(system.reset_types), + ) ) - ) if __name__ == "__main__": From 634584db26902a4bda089afa38692cd1f3fc1b08 Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sat, 15 Aug 2026 11:51:22 -0400 Subject: [PATCH 4/7] Harden async authentication lifecycle Preserve restricted password-change sessions, serialize login and logout operations, and retain the existing service-root authentication workaround. Preserve all advertised reset types for forward compatibility. Signed-off-by: Jacob Yundt --- src/redfish/aio/__init__.py | 2 - src/redfish/aio/client.py | 151 ++++++++++++------- src/redfish/aio/models.py | 23 +-- tests/aio/test_auth.py | 286 +++++++++++++++++++++++++++++++++++- tests/aio/test_models.py | 43 ++++++ tests/aio/test_systems.py | 10 +- 6 files changed, 431 insertions(+), 84 deletions(-) diff --git a/src/redfish/aio/__init__.py b/src/redfish/aio/__init__.py index cb57087..33878d1 100644 --- a/src/redfish/aio/__init__.py +++ b/src/redfish/aio/__init__.py @@ -18,7 +18,6 @@ RedfishUnsupportedResetError, ) from .models import ( - STANDARD_RESET_TYPES, ComputerSystem, get_reset_action_info_target, parse_computer_system, @@ -40,7 +39,6 @@ "RedfishProtocolError", "RedfishTimeoutError", "RedfishUnsupportedResetError", - "STANDARD_RESET_TYPES", "get_reset_action_info_target", "parse_computer_system", "parse_reset_action_info", diff --git a/src/redfish/aio/client.py b/src/redfish/aio/client.py index 169384c..e924153 100644 --- a/src/redfish/aio/client.py +++ b/src/redfish/aio/client.py @@ -8,6 +8,7 @@ import asyncio import base64 from dataclasses import replace +import warnings import aiohttp from multidict import CIMultiDict @@ -31,6 +32,9 @@ from .response import AsyncRestRequest, AsyncRestResponse +SESSION_COLLECTION_PATH = "/redfish/v1/SessionService/Sessions" + + class AsyncRedfishClient: """Perform asynchronous Redfish requests with an injected session.""" @@ -100,7 +104,11 @@ def __init__( self._authorization = None async def __aenter__(self): - await self.login() + try: + await self.login() + except RedfishPasswordChangeRequiredError: + await self.logout() + raise return self async def __aexit__(self, exc_type, exc_value, traceback): @@ -114,39 +122,58 @@ async def login(self, auth="session"): raise ValueError("Username and password are required") if self._base_url.scheme != "https": raise ValueError("Redfish authentication requires HTTPS") - if self._session_key is not None or self._authorization is not None: - await self.logout() - if auth == "basic": - if ":" in self._username: - raise ValueError( - "Basic authentication username cannot contain ':'" - ) - encoded = base64.b64encode( - "{}:{}".format( - self._username, self._password - ).encode("utf-8") - ).decode("ascii") - self._session_key = None - self._session_location = None - self._authorization = "Basic {}".format(encoded) - return + if auth == "basic" and ":" in self._username: + raise ValueError( + "Basic authentication username cannot contain ':'" + ) async with self._auth_lock: + if ( + self._session_key is not None + or self._authorization is not None + ): + await self._logout_locked() + if auth == "basic": + encoded = base64.b64encode( + "{}:{}".format( + self._username, self._password + ).encode("utf-8") + ).decode("ascii") + self._authorization = "Basic {}".format(encoded) + return await self._login_session() async def _login_session(self): - root = await self._get_json( + root_response = await self._request( self._default_prefix, authenticated=False ) - links = root.get("Links") - sessions = links.get("Sessions") if isinstance(links, dict) else None - target = ( - sessions.get("@odata.id") - if isinstance(sessions, dict) - else None - ) - if not isinstance(target, str) or not target.strip(): - target = "/redfish/v1/SessionService/Sessions" + if root_response.status == 401: + warnings.warn( + "Service incorrectly responded with HTTP 401 Unauthorized " + "for the service root; contact the vendor", + stacklevel=2, + ) + target = SESSION_COLLECTION_PATH + else: + self._ensure_success(root_response) + root = root_response.dict + if not isinstance(root, dict): + raise RedfishProtocolError( + "Redfish resource at {} is not a JSON object".format( + self._default_prefix + ) + ) + links = root.get("Links") + sessions = ( + links.get("Sessions") if isinstance(links, dict) else None + ) + target = ( + sessions.get("@odata.id") + if isinstance(sessions, dict) + else None + ) + if not isinstance(target, str) or not target.strip(): + target = SESSION_COLLECTION_PATH response = await self._request( target, method="POST", @@ -154,7 +181,9 @@ async def _login_session(self): authenticated=False, sensitive_body=True, ) - self._ensure_success(response) + password_change_uri = self._get_password_change_uri(response) + if not 200 <= response.status < 300: + self._ensure_success(response) session_key = response.getheader("X-Auth-Token") session_location = response.getheader("Location") if ( @@ -177,6 +206,8 @@ async def _login_session(self): self._authorization = None self._session_key = session_key self._session_location = session_location + if password_change_uri is not False: + raise RedfishPasswordChangeRequiredError(password_change_uri) async def _refresh_session(self, expired_session_key): async with self._auth_lock: @@ -187,8 +218,7 @@ async def _refresh_session(self, expired_session_key): await self._login_session() return True - async def logout(self): - """Terminate Redfish login without closing the transport.""" + async def _logout_locked(self): response = None try: if ( @@ -203,6 +233,11 @@ async def logout(self): if response is not None and response.status not in (401, 404): self._ensure_success(response) + async def logout(self): + """Terminate Redfish login without closing the transport.""" + async with self._auth_lock: + await self._logout_locked() + @staticmethod def _make_timeout(timeout): if timeout is None or isinstance(timeout, aiohttp.ClientTimeout): @@ -419,35 +454,39 @@ def _get_password_change_uri(response): return False if not isinstance(payload, dict): return False - error = payload.get("error") - if not isinstance(error, dict): - return False - code = error.get("code") - if ( - isinstance(code, str) - and code.startswith("Base.") - and code.endswith(".PasswordChangeRequired") - ): - return None - extended_info = error.get("@Message.ExtendedInfo") - if not isinstance(extended_info, list): - return False - for message in extended_info: - if ( - not isinstance(message, dict) - or not isinstance(message_id := message.get("MessageId"), str) - or not message_id.startswith("Base.") - or not message_id.endswith(".PasswordChangeRequired") - ): + containers = [payload] + if isinstance(error := payload.get("error"), dict): + containers.append(error) + for container in containers: + extended_info = container.get("@Message.ExtendedInfo") + if not isinstance(extended_info, list): continue - message_args = message.get("MessageArgs") + for message in extended_info: + if ( + not isinstance(message, dict) + or not isinstance( + message_id := message.get("MessageId"), str + ) + or not message_id.startswith("Base.") + or not message_id.endswith(".PasswordChangeRequired") + ): + continue + message_args = message.get("MessageArgs") + if ( + isinstance(message_args, list) + and message_args + and isinstance(message_args[0], str) + ): + return message_args[0] + return None + for container in containers: + code = container.get("code") if ( - isinstance(message_args, list) - and message_args - and isinstance(message_args[0], str) + isinstance(code, str) + and code.startswith("Base.") + and code.endswith(".PasswordChangeRequired") ): - return message_args[0] - return None + return None return False async def _get_json(self, path, authenticated=True): diff --git a/src/redfish/aio/models.py b/src/redfish/aio/models.py index 7d9c7e4..91b6792 100644 --- a/src/redfish/aio/models.py +++ b/src/redfish/aio/models.py @@ -9,25 +9,6 @@ from typing import FrozenSet, Optional -STANDARD_RESET_TYPES = frozenset( - { - "ForceOff", - "ForceOn", - "ForceRestart", - "FullPowerCycle", - "GracefulRestart", - "GracefulShutdown", - "Nmi", - "On", - "Pause", - "PowerCycle", - "PushPowerButton", - "Resume", - "Suspend", - } -) - - @dataclass(frozen=True) class ComputerSystem: """Standard properties and reset capabilities of a ComputerSystem.""" @@ -81,7 +62,7 @@ def parse_reset_action_info(payload): return frozenset( value for value in allowable_values - if isinstance(value, str) and value in STANDARD_RESET_TYPES + if isinstance(value, str) and value.strip() ) return frozenset() @@ -103,7 +84,7 @@ def parse_computer_system(payload): reset_types = frozenset( value for value in allowable_values - if isinstance(value, str) and value in STANDARD_RESET_TYPES + if isinstance(value, str) and value.strip() ) return ComputerSystem( diff --git a/tests/aio/test_auth.py b/tests/aio/test_auth.py index 30c8918..d67ea4f 100644 --- a/tests/aio/test_auth.py +++ b/tests/aio/test_auth.py @@ -58,8 +58,10 @@ async def read(self): class ControlledResponse(FakeResponse): """Response controlled by test events.""" - def __init__(self, started, release): - super().__init__(status=401) + def __init__( + self, started, release, status=401, headers=None, body=b"{}" + ): + super().__init__(status=status, headers=headers, body=body) self._started = started self._release = release @@ -74,9 +76,10 @@ class FakeSession: closed = False - def __init__(self, responses=None): + def __init__(self, responses=None, response_factory=None): self.requests = [] self.responses = list(responses or []) + self.response_factory = response_factory def request(self, method, url, **kwargs): self.requests.append( @@ -87,6 +90,8 @@ def request(self, method, url, **kwargs): "body": kwargs.get("json", kwargs.get("data")), } ) + if self.response_factory is not None: + return self.response_factory(method, url, kwargs) return self.responses.pop(0) @@ -144,6 +149,30 @@ async def test_colon_in_username_is_valid_only_for_session_auth(self): with self.assertRaises(ValueError): await basic_client.login(auth="basic") + async def test_invalid_basic_login_preserves_existing_session(self): + """Test Basic argument validation does not terminate a session.""" + session = FakeSession([FakeResponse()]) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="domain:user", + password="password", + session=session, + session_key="session-token", + session_location="/redfish/v1/SessionService/Sessions/1", + ) + + with self.assertRaises(ValueError): + await client.login(auth="basic") + await client.get("/redfish/v1/Systems/1") + + self.assertEqual( + [request["method"] for request in session.requests], ["GET"] + ) + self.assertEqual( + session.requests[0]["headers"].get("X-Auth-Token"), + "session-token", + ) + async def test_credentials_are_inactive_until_login(self): """Test constructing a client does not begin authentication.""" session = FakeSession([FakeResponse()]) @@ -293,6 +322,59 @@ async def test_session_login_uses_advertised_target_and_token(self): session.requests[2]["headers"].get("Authorization") ) + async def test_session_login_falls_back_after_root_unauthorized(self): + """Test login tolerates a service root that incorrectly needs auth.""" + session = FakeSession( + [ + FakeResponse(status=401), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "session-token", + "Location": ( + "/redfish/v1/SessionService/Sessions/1" + ), + }, + ), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertWarnsRegex( + UserWarning, "incorrectly responded with HTTP 401" + ): + await client.login() + + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST"], + ) + self.assertTrue( + session.requests[1]["url"].endswith( + "/redfish/v1/SessionService/Sessions" + ) + ) + + async def test_session_login_rejects_non_object_service_root(self): + """Test session discovery requires a service-root object.""" + session = FakeSession([FakeResponse(body=b"[]")]) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishProtocolError): + await client.login() + + self.assertEqual(len(session.requests), 1) + async def test_logout_deletes_session_without_closing_transport(self): """Test logout deletes only the Redfish login session.""" session = FakeSession( @@ -430,6 +512,109 @@ async def test_manual_relogin_terminates_previous_session(self): "old-token", ) + async def test_concurrent_logins_do_not_leak_a_session(self): + """Test concurrent explicit logins serialize session replacement.""" + started = asyncio.Event() + release = asyncio.Event() + login_count = 0 + + def response_factory(method, url, kwargs): + nonlocal login_count + if method == "DELETE": + return FakeResponse(status=204, body=b"") + if method != "POST": + return FakeResponse() + login_count += 1 + headers = { + "X-Auth-Token": ( + "first-token" if login_count == 1 else "second-token" + ), + "Location": ( + "/redfish/v1/SessionService/Sessions/{}".format( + login_count + ) + ), + } + if login_count == 1: + return ControlledResponse( + started, release, status=201, headers=headers + ) + return FakeResponse(status=201, headers=headers) + + session = FakeSession(response_factory=response_factory) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + first_login = asyncio.create_task(client.login()) + await started.wait() + second_login = asyncio.create_task(client.login()) + await asyncio.sleep(0) + release.set() + await asyncio.gather(first_login, second_login) + await client.get("/redfish/v1/Systems/1") + + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "DELETE", "GET", "POST", "GET"], + ) + self.assertEqual( + session.requests[-1]["headers"].get("X-Auth-Token"), + "second-token", + ) + + async def test_logout_waits_for_login_in_progress(self): + """Test logout cannot be undone by an in-progress login.""" + started = asyncio.Event() + release = asyncio.Event() + session = FakeSession( + [ + FakeResponse(status=204, body=b""), + FakeResponse(body=b"{}"), + ControlledResponse( + started, + release, + status=201, + headers={ + "X-Auth-Token": "new-token", + "Location": ( + "/redfish/v1/SessionService/Sessions/2" + ), + }, + ), + FakeResponse(status=204, body=b""), + FakeResponse(), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + session_key="old-token", + session_location="/redfish/v1/SessionService/Sessions/1", + ) + + login = asyncio.create_task(client.login()) + await started.wait() + logout = asyncio.create_task(client.logout()) + await asyncio.sleep(0) + self.assertFalse(logout.done()) + release.set() + await asyncio.gather(login, logout) + await client.get("/redfish/v1/Systems/1") + + self.assertEqual( + [request["method"] for request in session.requests], + ["DELETE", "GET", "POST", "DELETE", "GET"], + ) + self.assertIsNone( + session.requests[-1]["headers"].get("X-Auth-Token") + ) + async def test_password_change_required_is_classified(self): """Test session login reports a required password change.""" session = FakeSession( @@ -463,6 +648,101 @@ async def test_password_change_required_is_classified(self): "/redfish/v1/AccountService/Accounts/1", ) + async def test_successful_login_preserves_password_change_session(self): + """Test a restricted session remains usable to change a password.""" + account_uri = "/redfish/v1/AccountService/Accounts/1" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "restricted-token", + "Location": ( + "/redfish/v1/SessionService/Sessions/1" + ), + }, + body=json.dumps( + { + "@Message.ExtendedInfo": [ + { + "MessageId": ( + "Base.1.18.PasswordChangeRequired" + ), + "MessageArgs": [account_uri], + } + ] + } + ).encode("utf-8"), + ), + FakeResponse(status=204, body=b""), + FakeResponse(status=204, body=b""), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises( + RedfishPasswordChangeRequiredError + ) as context: + await client.login() + await client.patch(account_uri, body={"Password": "new-password"}) + await client.logout() + + self.assertEqual(context.exception.password_change_uri, account_uri) + self.assertEqual( + session.requests[2]["headers"].get("X-Auth-Token"), + "restricted-token", + ) + self.assertEqual( + session.requests[3]["headers"].get("X-Auth-Token"), + "restricted-token", + ) + + async def test_context_manager_cleans_up_password_change_session(self): + """Test a failed context entry does not leak a restricted session.""" + session = FakeSession( + [ + FakeResponse(body=b"{}"), + FakeResponse( + status=201, + headers={ + "X-Auth-Token": "restricted-token", + "Location": ( + "/redfish/v1/SessionService/Sessions/1" + ), + }, + body=( + b'{"@Message.ExtendedInfo":[{' + b'"MessageId":' + b'"Base.1.18.PasswordChangeRequired",' + b'"MessageArgs":[' + b'"/redfish/v1/AccountService/Accounts/1"]}]}' + ), + ), + FakeResponse(status=204, body=b""), + ] + ) + client = AsyncRedfishClient( + base_url="https://bmc.example", + username="user", + password="password", + session=session, + ) + + with self.assertRaises(RedfishPasswordChangeRequiredError): + async with client: + self.fail("Context body must not run") + + self.assertEqual( + [request["method"] for request in session.requests], + ["GET", "POST", "DELETE"], + ) + async def test_password_change_code_without_uri_is_classified(self): """Test a password-change error does not require MessageArgs.""" session = FakeSession( diff --git a/tests/aio/test_models.py b/tests/aio/test_models.py index 825a75d..00093ac 100644 --- a/tests/aio/test_models.py +++ b/tests/aio/test_models.py @@ -29,6 +29,49 @@ def test_action_info_without_usable_reset_values(self): with self.subTest(payload=payload): self.assertEqual(parse_reset_action_info(payload), frozenset()) + def test_advertised_reset_types_are_forward_compatible(self): + """Test unknown string values remain available to callers.""" + self.assertEqual( + parse_reset_action_info( + { + "Parameters": [ + { + "Name": "ResetType", + "AllowableValues": [ + "On", + "FutureStandardReset", + " ", + 1, + ], + } + ] + } + ), + frozenset({"On", "FutureStandardReset"}), + ) + system = parse_computer_system( + { + "@odata.id": "/redfish/v1/Systems/1", + "Id": "1", + "Actions": { + "#ComputerSystem.Reset": { + "target": "/redfish/v1/Systems/1/Actions/Reset", + "ResetType@Redfish.AllowableValues": [ + "On", + "FutureStandardReset", + " ", + 1, + ], + } + }, + } + ) + self.assertIsNotNone(system) + self.assertEqual( + system.reset_types, + frozenset({"On", "FutureStandardReset"}), + ) + def test_computer_system_requires_standard_identifiers(self): """Test systems without usable identifiers are rejected.""" for payload in ( diff --git a/tests/aio/test_systems.py b/tests/aio/test_systems.py index b13fb4a..6949e21 100644 --- a/tests/aio/test_systems.py +++ b/tests/aio/test_systems.py @@ -59,6 +59,7 @@ async def asyncSetUp(self): "On", "GracefulShutdown", "VendorReset", + " ", 1, ], } @@ -89,6 +90,7 @@ async def asyncSetUp(self): "ForceOff", "GracefulRestart", "VendorReset", + " ", ], } ] @@ -148,7 +150,9 @@ async def test_discovers_paginated_systems_and_metadata(self): "/redfish/v1/Systems/1/Actions/" "ComputerSystem.Reset" ), - reset_types=frozenset({"On", "GracefulShutdown"}), + reset_types=frozenset( + {"On", "GracefulShutdown", "VendorReset"} + ), ), "2": ComputerSystem( odata_id="/redfish/v1/Systems/2", @@ -163,7 +167,9 @@ async def test_discovers_paginated_systems_and_metadata(self): "/redfish/v1/Systems/2/Actions/" "ComputerSystem.Reset" ), - reset_types=frozenset({"ForceOff", "GracefulRestart"}), + reset_types=frozenset( + {"ForceOff", "GracefulRestart", "VendorReset"} + ), ), }, ) From 0ed07f4e4cbaf3485023aea698be296b87323c6f Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sat, 15 Aug 2026 11:51:28 -0400 Subject: [PATCH 5/7] Document async authentication compatibility Clarify Basic authentication validation, password-change sessions, service-root fallback behavior, and forward-compatible reset discovery. Declare direct async dependencies explicitly. Signed-off-by: Jacob Yundt --- README.rst | 8 +++++--- requirements.txt | 2 ++ setup.py | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/README.rst b/README.rst index 64cccab..e8f9a94 100644 --- a/README.rst +++ b/README.rst @@ -196,7 +196,7 @@ Asynchronous client The additive asynchronous API uses ``aiohttp`` and does not change the existing synchronous client. The caller must provide an ``aiohttp.ClientSession`` and remains responsible for closing it. This allows an application to control connection pooling, TLS trust, proxy behavior, and session lifetime in one place. -The asynchronous client supports Redfish session authentication and HTTP Basic authentication. Authentication is explicit: call ``login`` after creating the client and ``logout`` when finished. ``login`` uses Redfish session authentication by default, matching the synchronous client. Redfish authentication requires HTTPS. +The asynchronous client supports Redfish session authentication and HTTP Basic authentication. Authentication is explicit: call ``login`` after creating the client and ``logout`` when finished. ``login`` uses Redfish session authentication by default, matching the synchronous client. Redfish authentication requires HTTPS. For compatibility with nonconforming services, session login uses the standard session collection URI and emits a warning if the service root incorrectly responds with HTTP 401. The asynchronous context manager creates and terminates a Redfish session. It does not close the caller's ``aiohttp.ClientSession``: @@ -221,7 +221,9 @@ The asynchronous context manager creates and terminates a Redfish session. It d systems = await client.get_systems() return service_root, systems -To use HTTP Basic authentication, call ``await client.login(auth="basic")`` and ensure ``await client.logout()`` is called when finished. An existing Redfish session can be supplied with the ``session_key`` argument and, when available, its resource URI with ``session_location``. Supplying the location allows ``logout`` to terminate that session. +To use HTTP Basic authentication, call ``await client.login(auth="basic")`` and ensure ``await client.logout()`` is called when finished. Basic ``login`` configures the authentication header; the service validates the credentials when the client performs its next request. An existing Redfish session can be supplied with the ``session_key`` argument and, when available, its resource URI with ``session_location``. Supplying the location allows ``logout`` to terminate that session. + +If session login reports that the account password must change, ``login`` raises ``RedfishPasswordChangeRequiredError`` with the account URI in ``password_change_uri`` while retaining the restricted session. The caller can use that client to change the password and then call ``logout``. The asynchronous context manager instead cleans up a restricted session before propagating this exception because a failed ``__aenter__`` call cannot return the client to the context body. If an authenticated ``GET`` or ``HEAD`` receives HTTP 401, the client re-establishes an expired Redfish session once when credentials are available. State-changing requests are never retried automatically. Callers can therefore decide whether it is safe to repeat a failed ``POST``, ``PUT``, ``PATCH``, or ``DELETE``. @@ -229,7 +231,7 @@ Requests do not follow redirects, and advertised resource, action, and session t ``get``, ``head``, ``post``, ``put``, ``patch``, and ``delete`` are coroutines with the same ``path``, ``args``, ``body``, ``headers``, and ``timeout`` concepts as the synchronous methods. The returned response is fully read and cached before the coroutine returns, so it can be inspected after the underlying aiohttp response closes. -``get_systems`` follows the standard ServiceRoot ``Systems`` link, collection pagination, ComputerSystem member links, and reset ActionInfo resources. It returns ``ComputerSystem`` objects containing standard identity, metadata, power state, reset target, and advertised standard reset types. ``reset_system`` sends an advertised reset type to that system's advertised reset target: +``get_systems`` follows the standard ServiceRoot ``Systems`` link, collection pagination, ComputerSystem member links, and reset ActionInfo resources. It returns ``ComputerSystem`` objects containing standard identity, metadata, power state, reset target, and every usable string-valued reset type advertised by the service. Preserving unknown values allows callers to handle newer Redfish reset types without waiting for a library update. ``reset_system`` sends an advertised reset type to that system's advertised reset target: .. code-block:: python diff --git a/requirements.txt b/requirements.txt index d78db70..a3035c6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,6 @@ aiohttp>=3.9.0 +multidict>=4.5 +yarl>=1.0 jsonpatch<=1.24 ; python_version == '3.4' jsonpatch ; python_version >= '3.5' jsonpath_ng diff --git a/setup.py b/setup.py index c8face9..4197a70 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,9 @@ ], extras_require={ 'aiohttp': [ - 'aiohttp>=3.9.0' + 'aiohttp>=3.9.0', + 'multidict>=4.5', + 'yarl>=1.0' ], ':python_version == "3.4"': [ 'jsonpatch<=1.24' From 2cf70096565f2f4e2e4a41db55ba42088d741da3 Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sat, 15 Aug 2026 12:43:30 -0400 Subject: [PATCH 6/7] Keep async client resource-neutral Remove ComputerSystem discovery, parsing, pagination, and reset helpers so the asynchronous API matches the generic abstraction of the existing REST client. Update documentation, examples, and tests to exercise resource-neutral operations. Signed-off-by: Jacob Yundt --- README.rst | 18 +- examples/async_client.py | 15 +- src/redfish/aio/__init__.py | 12 -- src/redfish/aio/client.py | 104 ------------ src/redfish/aio/exceptions.py | 4 - src/redfish/aio/models.py | 101 ----------- tests/aio/test_auth.py | 6 +- tests/aio/test_client.py | 10 +- tests/aio/test_models.py | 99 ----------- tests/aio/test_systems.py | 306 ---------------------------------- 10 files changed, 13 insertions(+), 662 deletions(-) delete mode 100644 src/redfish/aio/models.py delete mode 100644 tests/aio/test_models.py delete mode 100644 tests/aio/test_systems.py diff --git a/README.rst b/README.rst index e8f9a94..ca4eeef 100644 --- a/README.rst +++ b/README.rst @@ -207,7 +207,7 @@ The asynchronous context manager creates and terminates a Redfish session. It d from redfish.aio import AsyncRedfishClient - async def get_systems(): + async def get_service_root(): async with aiohttp.ClientSession() as session: async with AsyncRedfishClient( base_url="https://bmc.example", @@ -215,11 +215,8 @@ The asynchronous context manager creates and terminates a Redfish session. It d password="password", session=session, timeout=10, - discovery_timeout=60, ) as client: - service_root = await client.get_service_root() - systems = await client.get_systems() - return service_root, systems + return await client.get_service_root() To use HTTP Basic authentication, call ``await client.login(auth="basic")`` and ensure ``await client.logout()`` is called when finished. Basic ``login`` configures the authentication header; the service validates the credentials when the client performs its next request. An existing Redfish session can be supplied with the ``session_key`` argument and, when available, its resource URI with ``session_location``. Supplying the location allows ``logout`` to terminate that session. @@ -231,16 +228,7 @@ Requests do not follow redirects, and advertised resource, action, and session t ``get``, ``head``, ``post``, ``put``, ``patch``, and ``delete`` are coroutines with the same ``path``, ``args``, ``body``, ``headers``, and ``timeout`` concepts as the synchronous methods. The returned response is fully read and cached before the coroutine returns, so it can be inspected after the underlying aiohttp response closes. -``get_systems`` follows the standard ServiceRoot ``Systems`` link, collection pagination, ComputerSystem member links, and reset ActionInfo resources. It returns ``ComputerSystem`` objects containing standard identity, metadata, power state, reset target, and every usable string-valued reset type advertised by the service. Preserving unknown values allows callers to handle newer Redfish reset types without waiting for a library update. ``reset_system`` sends an advertised reset type to that system's advertised reset target: - -.. code-block:: python - - systems = await client.get_systems() - system = systems["1"] - if "On" in system.reset_types: - await client.reset_system(system, "On") - -The optional request ``timeout`` bounds each HTTP request. ``discovery_timeout`` defaults to 60 seconds and bounds the complete ServiceRoot, collection, member, and ActionInfo discovery operation. TLS verification is controlled entirely by the injected ``ClientSession``. Configure that session with an appropriate CA certificate or SSL context for a Redfish service using a private or self-signed certificate. +The optional request ``timeout`` bounds each HTTP request. TLS verification is controlled entirely by the injected ``ClientSession``. Configure that session with an appropriate CA certificate or SSL context for a Redfish service using a private or self-signed certificate. Working with tasks ~~~~~~~~~~~~~~~~~~ diff --git a/examples/async_client.py b/examples/async_client.py index 3dea218..bbcb333 100644 --- a/examples/async_client.py +++ b/examples/async_client.py @@ -3,7 +3,7 @@ # License: BSD 3-Clause License. For full text see link: # https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md -"""Discover ComputerSystem resources with the asynchronous Redfish client.""" +"""Retrieve the service root with the asynchronous Redfish client.""" import asyncio import os @@ -14,7 +14,7 @@ async def main(): - """Discover and display Redfish ComputerSystem resources.""" + """Retrieve and display the Redfish service root.""" async with aiohttp.ClientSession() as session: async with AsyncRedfishClient( base_url=os.environ["REDFISH_BASE_URL"], @@ -23,15 +23,8 @@ async def main(): session=session, timeout=10, ) as client: - systems = await client.get_systems() - for system in systems.values(): - print( - "{}: power={}, reset_types={}".format( - system.name or system.system_id, - system.power_state, - sorted(system.reset_types), - ) - ) + service_root = await client.get_service_root() + print(service_root) if __name__ == "__main__": diff --git a/src/redfish/aio/__init__.py b/src/redfish/aio/__init__.py index 33878d1..98112d6 100644 --- a/src/redfish/aio/__init__.py +++ b/src/redfish/aio/__init__.py @@ -15,13 +15,6 @@ RedfishPasswordChangeRequiredError, RedfishProtocolError, RedfishTimeoutError, - RedfishUnsupportedResetError, -) -from .models import ( - ComputerSystem, - get_reset_action_info_target, - parse_computer_system, - parse_reset_action_info, ) from .response import AsyncRestRequest, AsyncRestResponse @@ -29,7 +22,6 @@ "AsyncRedfishClient", "AsyncRestRequest", "AsyncRestResponse", - "ComputerSystem", "RedfishAuthenticationError", "RedfishConnectionError", "RedfishError", @@ -38,8 +30,4 @@ "RedfishPasswordChangeRequiredError", "RedfishProtocolError", "RedfishTimeoutError", - "RedfishUnsupportedResetError", - "get_reset_action_info_target", - "parse_computer_system", - "parse_reset_action_info", ] diff --git a/src/redfish/aio/client.py b/src/redfish/aio/client.py index e924153..ec7de40 100644 --- a/src/redfish/aio/client.py +++ b/src/redfish/aio/client.py @@ -7,7 +7,6 @@ import asyncio import base64 -from dataclasses import replace import warnings import aiohttp @@ -22,12 +21,6 @@ RedfishPasswordChangeRequiredError, RedfishProtocolError, RedfishTimeoutError, - RedfishUnsupportedResetError, -) -from .models import ( - get_reset_action_info_target, - parse_computer_system, - parse_reset_action_info, ) from .response import AsyncRestRequest, AsyncRestResponse @@ -46,7 +39,6 @@ def __init__( session=None, timeout=None, default_prefix="/redfish/v1/", - discovery_timeout=60, session_key=None, session_location=None, ): @@ -80,9 +72,6 @@ def __init__( self._session = session self._timeout = self._make_timeout(timeout) self._default_prefix = default_prefix - self._discovery_timeout = self._make_discovery_timeout( - discovery_timeout - ) self._auth_lock = asyncio.Lock() self._username = username self._password = password @@ -246,16 +235,6 @@ def _make_timeout(timeout): raise ValueError("Timeout must be a non-negative number") return aiohttp.ClientTimeout(total=timeout) - @staticmethod - def _make_discovery_timeout(timeout): - if timeout is not None and ( - not isinstance(timeout, (int, float)) or timeout < 0 - ): - raise ValueError( - "Discovery timeout must be a non-negative number" - ) - return timeout - def _resolve_url(self, target): try: target_url = self._base_url.join(URL(target)) @@ -504,86 +483,3 @@ async def _get_json(self, path, authenticated=True): async def get_service_root(self): """Return the standard Redfish service root.""" return await self._get_json(self._default_prefix) - - async def _get_collection_members(self, link): - if ( - not isinstance(link, dict) - or not isinstance(path := link.get("@odata.id"), str) - or not path.strip() - ): - return [] - - payloads = [] - seen_paths = set() - while True: - if path in seen_paths: - raise RedfishProtocolError( - "Redfish collection pagination contains a cycle" - ) - seen_paths.add(path) - collection = await self._get_json(path) - members = collection.get("Members") - if not isinstance(members, list): - return [] - for member in members: - if ( - isinstance(member, dict) - and isinstance(member_path := member.get("@odata.id"), str) - and member_path.strip() - ): - payloads.append(await self._get_json(member_path)) - next_path = collection.get("Members@odata.nextLink") - if not isinstance(next_path, str) or not next_path.strip(): - return payloads - path = next_path - - async def _discover_systems(self): - root = await self.get_service_root() - systems = {} - for payload in await self._get_collection_members(root.get("Systems")): - system = parse_computer_system(payload) - if system is None: - continue - action_info_target = get_reset_action_info_target(payload) - if ( - system.reset_target is not None - and action_info_target is not None - ): - action_info = await self._get_json(action_info_target) - system = replace( - system, - reset_types=system.reset_types - | parse_reset_action_info(action_info), - ) - systems[system.system_id] = system - return systems - - async def get_systems(self): - """Discover ComputerSystem resources from the Redfish service root.""" - try: - return await asyncio.wait_for( - self._discover_systems(), timeout=self._discovery_timeout - ) - except asyncio.TimeoutError as exc: - raise RedfishTimeoutError( - "Redfish system discovery timed out" - ) from exc - - async def reset_system(self, system, reset_type, timeout=None): - """Perform a reset type advertised by a ComputerSystem.""" - if ( - system.reset_target is None - or reset_type not in system.reset_types - ): - raise RedfishUnsupportedResetError( - "ComputerSystem does not advertise ResetType {}".format( - reset_type - ) - ) - response = await self.post( - system.reset_target, - body={"ResetType": reset_type}, - timeout=timeout, - ) - self._ensure_success(response) - return response diff --git a/src/redfish/aio/exceptions.py b/src/redfish/aio/exceptions.py index 04ce90b..e5387a7 100644 --- a/src/redfish/aio/exceptions.py +++ b/src/redfish/aio/exceptions.py @@ -46,7 +46,3 @@ def __init__(self, response): class RedfishProtocolError(RedfishError): """Raised when a Redfish resource is malformed.""" - - -class RedfishUnsupportedResetError(RedfishError): - """Raised when a reset type is not advertised by a ComputerSystem.""" diff --git a/src/redfish/aio/models.py b/src/redfish/aio/models.py deleted file mode 100644 index 91b6792..0000000 --- a/src/redfish/aio/models.py +++ /dev/null @@ -1,101 +0,0 @@ -# Copyright Notice: -# Copyright 2016-2026 DMTF. All rights reserved. -# License: BSD 3-Clause License. For full text see link: -# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md - -"""Standard Redfish resource models used by the asynchronous client.""" - -from dataclasses import dataclass -from typing import FrozenSet, Optional - - -@dataclass(frozen=True) -class ComputerSystem: - """Standard properties and reset capabilities of a ComputerSystem.""" - - odata_id: str - system_id: str - name: Optional[str] - uuid: Optional[str] - manufacturer: Optional[str] - model: Optional[str] - serial_number: Optional[str] - power_state: Optional[str] - reset_target: Optional[str] - reset_types: FrozenSet[str] - - -def _non_empty_string(value): - return value if isinstance(value, str) and value.strip() else None - - -def _reset_action(payload): - actions = payload.get("Actions") - if not isinstance(actions, dict): - return None - reset = actions.get("#ComputerSystem.Reset") - return reset if isinstance(reset, dict) else None - - -def get_reset_action_info_target(payload): - """Return the ActionInfo target advertised for ComputerSystem.Reset.""" - reset = _reset_action(payload) - if reset is None: - return None - return _non_empty_string(reset.get("@Redfish.ActionInfo")) - - -def parse_reset_action_info(payload): - """Return standard ResetType values from an ActionInfo resource.""" - parameters = payload.get("Parameters") - if not isinstance(parameters, list): - return frozenset() - for parameter in parameters: - if ( - not isinstance(parameter, dict) - or parameter.get("Name") != "ResetType" - ): - continue - allowable_values = parameter.get("AllowableValues") - if not isinstance(allowable_values, list): - return frozenset() - return frozenset( - value - for value in allowable_values - if isinstance(value, str) and value.strip() - ) - return frozenset() - - -def parse_computer_system(payload): - """Parse a ComputerSystem resource or return None when unusable.""" - odata_id = _non_empty_string(payload.get("@odata.id")) - system_id = _non_empty_string(payload.get("Id")) - if odata_id is None or system_id is None: - return None - - reset_target = None - reset_types = frozenset() - reset = _reset_action(payload) - if reset is not None: - reset_target = _non_empty_string(reset.get("target")) - allowable_values = reset.get("ResetType@Redfish.AllowableValues") - if reset_target is not None and isinstance(allowable_values, list): - reset_types = frozenset( - value - for value in allowable_values - if isinstance(value, str) and value.strip() - ) - - return ComputerSystem( - odata_id=odata_id, - system_id=system_id, - name=_non_empty_string(payload.get("Name")), - uuid=_non_empty_string(payload.get("UUID")), - manufacturer=_non_empty_string(payload.get("Manufacturer")), - model=_non_empty_string(payload.get("Model")), - serial_number=_non_empty_string(payload.get("SerialNumber")), - power_state=_non_empty_string(payload.get("PowerState")), - reset_target=reset_target, - reset_types=reset_types, - ) diff --git a/tests/aio/test_auth.py b/tests/aio/test_auth.py index d67ea4f..226017b 100644 --- a/tests/aio/test_auth.py +++ b/tests/aio/test_auth.py @@ -851,7 +851,7 @@ async def test_password_change_is_classified_for_basic_request(self): await client.login(auth="basic") with self.assertRaises(RedfishPasswordChangeRequiredError): - await client.get_systems() + await client.get_service_root() async def test_login_error_does_not_retain_credentials(self): """Test a session-creation error cannot expose its credential body.""" @@ -1150,8 +1150,8 @@ async def test_expired_session_does_not_retry_write(self): await client.login() response = await client.post( - "/redfish/v1/Systems/1/Actions/ComputerSystem.Reset", - body={"ResetType": "On"}, + "/redfish/v1/Actions/Example", + body={"Value": "example"}, ) self.assertEqual(response.status, 401) diff --git a/tests/aio/test_client.py b/tests/aio/test_client.py index 5030e24..b050823 100644 --- a/tests/aio/test_client.py +++ b/tests/aio/test_client.py @@ -180,10 +180,8 @@ async def test_same_origin_absolute_and_scheme_relative_targets(self): self.server.make_url("/scheme-relative").with_scheme("") ) - await self.client.post(absolute_target, body={"ResetType": "On"}) - await self.client.post( - scheme_relative_target, body={"ResetType": "GracefulShutdown"} - ) + await self.client.post(absolute_target) + await self.client.post(scheme_relative_target) self.assertEqual( [request["path_qs"] for request in self.requests], @@ -223,7 +221,7 @@ async def capture_request(request): with self.subTest(target=target), self.assertRaises( RedfishInvalidTargetError ): - await self.client.post(target, body={"ResetType": "On"}) + await self.client.post(target) self.assertEqual(malicious_requests, []) @@ -297,8 +295,6 @@ async def test_client_validation(self): {**valid, "password": "password"}, {**valid, "timeout": -1}, {**valid, "timeout": "invalid"}, - {**valid, "discovery_timeout": -1}, - {**valid, "discovery_timeout": "invalid"}, {**valid, "base_url": "https://["}, {**valid, "base_url": "bmc.example"}, {**valid, "base_url": "https://user@bmc.example"}, diff --git a/tests/aio/test_models.py b/tests/aio/test_models.py deleted file mode 100644 index 00093ac..0000000 --- a/tests/aio/test_models.py +++ /dev/null @@ -1,99 +0,0 @@ -# Copyright Notice: -# Copyright 2016-2026 DMTF. All rights reserved. -# License: BSD 3-Clause License. For full text see link: -# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md - -import unittest - -from redfish.aio import ( - get_reset_action_info_target, - parse_computer_system, - parse_reset_action_info, -) - - -class TestAsyncResourceModels(unittest.TestCase): - """Test standard resource parsing used by the asynchronous client.""" - - def test_action_info_requires_reset_action(self): - """Test ActionInfo lookup without a reset action.""" - self.assertIsNone(get_reset_action_info_target({})) - - def test_action_info_without_usable_reset_values(self): - """Test malformed ActionInfo parameters produce no reset types.""" - for payload in ( - {}, - {"Parameters": [None, {"Name": "OtherParameter"}]}, - {"Parameters": [{"Name": "ResetType"}]}, - ): - with self.subTest(payload=payload): - self.assertEqual(parse_reset_action_info(payload), frozenset()) - - def test_advertised_reset_types_are_forward_compatible(self): - """Test unknown string values remain available to callers.""" - self.assertEqual( - parse_reset_action_info( - { - "Parameters": [ - { - "Name": "ResetType", - "AllowableValues": [ - "On", - "FutureStandardReset", - " ", - 1, - ], - } - ] - } - ), - frozenset({"On", "FutureStandardReset"}), - ) - system = parse_computer_system( - { - "@odata.id": "/redfish/v1/Systems/1", - "Id": "1", - "Actions": { - "#ComputerSystem.Reset": { - "target": "/redfish/v1/Systems/1/Actions/Reset", - "ResetType@Redfish.AllowableValues": [ - "On", - "FutureStandardReset", - " ", - 1, - ], - } - }, - } - ) - self.assertIsNotNone(system) - self.assertEqual( - system.reset_types, - frozenset({"On", "FutureStandardReset"}), - ) - - def test_computer_system_requires_standard_identifiers(self): - """Test systems without usable identifiers are rejected.""" - for payload in ( - {}, - {"Id": "1"}, - {"@odata.id": "/redfish/v1/Systems/1"}, - {"@odata.id": " ", "Id": "1"}, - {"@odata.id": "/redfish/v1/Systems/1", "Id": 1}, - ): - with self.subTest(payload=payload): - self.assertIsNone(parse_computer_system(payload)) - - def test_computer_system_without_reset_action(self): - """Test a usable system need not advertise reset support.""" - system = parse_computer_system( - {"@odata.id": "/redfish/v1/Systems/1", "Id": "1"} - ) - - self.assertIsNotNone(system) - self.assertIsNone(system.reset_target) - self.assertEqual(system.reset_types, frozenset()) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/aio/test_systems.py b/tests/aio/test_systems.py deleted file mode 100644 index 6949e21..0000000 --- a/tests/aio/test_systems.py +++ /dev/null @@ -1,306 +0,0 @@ -# Copyright Notice: -# Copyright 2016-2026 DMTF. All rights reserved. -# License: BSD 3-Clause License. For full text see link: -# https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md - -import asyncio -import unittest - -import aiohttp -from aiohttp import web -from aiohttp.test_utils import TestServer - -from redfish.aio import ( - AsyncRedfishClient, - ComputerSystem, - RedfishAuthenticationError, - RedfishHTTPError, - RedfishProtocolError, - RedfishTimeoutError, - RedfishUnsupportedResetError, -) - - -class TestAsyncComputerSystemDiscovery(unittest.IsolatedAsyncioTestCase): - """Test standard asynchronous ComputerSystem operations.""" - - async def asyncSetUp(self): - self.requests = [] - self.statuses = {} - self.delays = {} - self.raw_responses = {} - self.resources = { - "/redfish/v1/": { - "Systems": {"@odata.id": "/redfish/v1/Systems"}, - }, - "/redfish/v1/Systems": { - "Members": [{"@odata.id": "/redfish/v1/Systems/1"}], - "Members@odata.nextLink": "/redfish/v1/Systems?page=2", - }, - "/redfish/v1/Systems?page=2": { - "Members": [{"@odata.id": "/redfish/v1/Systems/2"}], - }, - "/redfish/v1/Systems/1": { - "@odata.id": "/redfish/v1/Systems/1", - "Id": "1", - "Name": "Server One", - "UUID": "uuid-1", - "Manufacturer": "Acme", - "Model": "Model 1", - "SerialNumber": "serial-1", - "PowerState": "On", - "Actions": { - "#ComputerSystem.Reset": { - "target": ( - "/redfish/v1/Systems/1/Actions/" - "ComputerSystem.Reset" - ), - "ResetType@Redfish.AllowableValues": [ - "On", - "GracefulShutdown", - "VendorReset", - " ", - 1, - ], - } - }, - }, - "/redfish/v1/Systems/2": { - "@odata.id": "/redfish/v1/Systems/2", - "Id": "2", - "Name": "Server Two", - "PowerState": "Off", - "Actions": { - "#ComputerSystem.Reset": { - "target": ( - "/redfish/v1/Systems/2/Actions/" - "ComputerSystem.Reset" - ), - "@Redfish.ActionInfo": ( - "/redfish/v1/Systems/2/ResetActionInfo" - ), - } - }, - }, - "/redfish/v1/Systems/2/ResetActionInfo": { - "Parameters": [ - { - "Name": "ResetType", - "AllowableValues": [ - "ForceOff", - "GracefulRestart", - "VendorReset", - " ", - ], - } - ] - }, - } - app = web.Application() - - async def response(request): - path = request.path_qs - if delay := self.delays.get(path): - await asyncio.sleep(delay) - body = await request.json() if request.can_read_body else None - self.requests.append((request.method, path, body)) - status = self.statuses.get(path, 200) - if path in self.raw_responses: - return web.Response( - text=self.raw_responses[path], - status=status, - content_type="application/json", - ) - if request.method != "GET": - return web.Response(status=status) - return web.json_response( - self.resources.get(path, {}), status=status - ) - - app.router.add_route("*", "/{path:.*}", response) - self.server = TestServer(app) - await self.server.start_server() - self.session = aiohttp.ClientSession() - self.client = AsyncRedfishClient( - base_url=str(self.server.make_url("/")), - session=self.session, - ) - - async def asyncTearDown(self): - await self.session.close() - await self.server.close() - - async def test_discovers_paginated_systems_and_metadata(self): - """Test service-root traversal and ComputerSystem parsing.""" - systems = await self.client.get_systems() - - self.assertEqual( - systems, - { - "1": ComputerSystem( - odata_id="/redfish/v1/Systems/1", - system_id="1", - name="Server One", - uuid="uuid-1", - manufacturer="Acme", - model="Model 1", - serial_number="serial-1", - power_state="On", - reset_target=( - "/redfish/v1/Systems/1/Actions/" - "ComputerSystem.Reset" - ), - reset_types=frozenset( - {"On", "GracefulShutdown", "VendorReset"} - ), - ), - "2": ComputerSystem( - odata_id="/redfish/v1/Systems/2", - system_id="2", - name="Server Two", - uuid=None, - manufacturer=None, - model=None, - serial_number=None, - power_state="Off", - reset_target=( - "/redfish/v1/Systems/2/Actions/" - "ComputerSystem.Reset" - ), - reset_types=frozenset( - {"ForceOff", "GracefulRestart", "VendorReset"} - ), - ), - }, - ) - self.assertEqual( - [request[1] for request in self.requests], - [ - "/redfish/v1/", - "/redfish/v1/Systems", - "/redfish/v1/Systems/1", - "/redfish/v1/Systems?page=2", - "/redfish/v1/Systems/2", - "/redfish/v1/Systems/2/ResetActionInfo", - ], - ) - - async def test_reset_system_uses_advertised_target_and_type(self): - """Test reset uses the target and ResetType advertised by the BMC.""" - system = (await self.client.get_systems())["2"] - self.requests.clear() - - response = await self.client.reset_system(system, "ForceOff") - - self.assertEqual(response.status, 200) - self.assertEqual( - self.requests, - [ - ( - "POST", - "/redfish/v1/Systems/2/Actions/ComputerSystem.Reset", - {"ResetType": "ForceOff"}, - ) - ], - ) - - async def test_reset_system_rejects_unadvertised_type(self): - """Test an unadvertised reset type never reaches the BMC.""" - system = (await self.client.get_systems())["2"] - self.requests.clear() - - with self.assertRaises(RedfishUnsupportedResetError): - await self.client.reset_system(system, "On") - - self.assertEqual(self.requests, []) - - async def test_missing_and_malformed_systems_are_skipped(self): - """Test unusable collection and ComputerSystem data is skipped.""" - self.resources["/redfish/v1/Systems"] = { - "Members": [ - {}, - {"@odata.id": 1}, - {"@odata.id": " "}, - {"@odata.id": "/redfish/v1/Systems/malformed"}, - ] - } - self.resources["/redfish/v1/Systems/malformed"] = { - "@odata.id": "/redfish/v1/Systems/malformed" - } - - self.assertEqual(await self.client.get_systems(), {}) - - self.resources["/redfish/v1/"] = {} - self.assertEqual(await self.client.get_systems(), {}) - - async def test_repeated_pagination_link_is_rejected(self): - """Test cyclic collection pagination is rejected.""" - self.resources["/redfish/v1/Systems"] = { - "Members": [], - "Members@odata.nextLink": "/redfish/v1/Systems", - } - - with self.assertRaises(RedfishProtocolError): - await self.client.get_systems() - - async def test_discovery_timeout_includes_action_info(self): - """Test the overall discovery deadline includes ActionInfo requests.""" - self.delays["/redfish/v1/Systems/2/ResetActionInfo"] = 0.1 - client = AsyncRedfishClient( - base_url=str(self.server.make_url("/")), - session=self.session, - discovery_timeout=0.01, - ) - - with self.assertRaises(RedfishTimeoutError): - await client.get_systems() - - async def test_discovery_classifies_http_errors(self): - """Test unsuccessful responses are classified.""" - for status, expected_error in ( - (401, RedfishAuthenticationError), - (403, RedfishAuthenticationError), - (500, RedfishHTTPError), - ): - with self.subTest(status=status): - self.statuses["/redfish/v1/"] = status - with self.assertRaises(expected_error): - await self.client.get_systems() - - async def test_discovery_rejects_malformed_json(self): - """Test malformed JSON is reported as a protocol error.""" - self.raw_responses["/redfish/v1/"] = "{" - - with self.assertRaises(RedfishProtocolError): - await self.client.get_systems() - - async def test_discovery_rejects_non_object_json(self): - """Test a Redfish resource must contain a JSON object.""" - self.raw_responses["/redfish/v1/"] = "[]" - - with self.assertRaises(RedfishProtocolError): - await self.client.get_systems() - - async def test_malformed_collection_members_are_ignored(self): - """Test a collection without a Members array is treated as empty.""" - self.resources["/redfish/v1/Systems"] = {"Members": {}} - - self.assertEqual(await self.client.get_systems(), {}) - - async def test_reset_system_classifies_http_errors(self): - """Test reset HTTP failures are classified.""" - system = (await self.client.get_systems())["1"] - target = "/redfish/v1/Systems/1/Actions/ComputerSystem.Reset" - - for status, expected_error in ( - (401, RedfishAuthenticationError), - (500, RedfishHTTPError), - ): - with self.subTest(status=status): - self.statuses[target] = status - with self.assertRaises(expected_error): - await self.client.reset_system(system, "On") - - -if __name__ == "__main__": - unittest.main() From 42844ae04abdf8897539afc38483614a1b9e1450 Mon Sep 17 00:00:00 2001 From: Jacob Yundt Date: Sat, 15 Aug 2026 14:17:27 -0400 Subject: [PATCH 7/7] Cover generic service root parsing Exercise successful service-root decoding and rejection of malformed or non-object JSON after removing the resource-specific discovery tests. Signed-off-by: Jacob Yundt --- tests/aio/test_client.py | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/aio/test_client.py b/tests/aio/test_client.py index b050823..b25a315 100644 --- a/tests/aio/test_client.py +++ b/tests/aio/test_client.py @@ -14,6 +14,7 @@ AsyncRedfishClient, RedfishConnectionError, RedfishInvalidTargetError, + RedfishProtocolError, RedfishTimeoutError, ) @@ -50,6 +51,12 @@ async def response(request): ) if request.path == "/empty": return web.Response(status=204) + if request.path == "/invalid-json": + return web.Response( + text="not JSON", content_type="application/json" + ) + if request.path == "/list": + return web.json_response([]) status = 401 if request.path == "/unauthorized" else 200 return web.json_response( {"method": request.method, "path": request.path}, @@ -173,6 +180,42 @@ async def test_empty_response_has_empty_dictionary(self): self.assertEqual(response.status, 204) self.assertEqual(response.dict, {}) + async def test_get_service_root_returns_json_object(self): + """Test service-root retrieval returns its JSON object.""" + service_root = await self.client.get_service_root() + + self.assertEqual( + service_root, {"method": "GET", "path": "/redfish/v1/"} + ) + + async def test_get_service_root_rejects_non_object_json(self): + """Test service-root retrieval rejects non-object JSON.""" + client = AsyncRedfishClient( + base_url=str(self.server.make_url("/")), + default_prefix="/list", + session=self.session, + ) + + with self.assertRaisesRegex( + RedfishProtocolError, + "Redfish resource at /list is not a JSON object", + ): + await client.get_service_root() + + async def test_get_service_root_rejects_invalid_json(self): + """Test service-root retrieval rejects malformed JSON.""" + client = AsyncRedfishClient( + base_url=str(self.server.make_url("/")), + default_prefix="/invalid-json", + session=self.session, + ) + + with self.assertRaisesRegex( + RedfishProtocolError, + "Service responded with invalid JSON at URI /invalid-json", + ): + await client.get_service_root() + async def test_same_origin_absolute_and_scheme_relative_targets(self): """Test advertised same-origin target forms are accepted.""" absolute_target = str(self.server.make_url("/absolute"))