From 1acff0608019ad6c6a363b2f600c03400fd80670 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Thu, 9 Dec 2021 13:04:42 -0800 Subject: [PATCH] Fix session authentication issues Session authentication issues can occur when the client has been initialized and some intermediate failure has occurred such as a transport failure to the remote BMC or even with setting changes on the remote BMC. Previously, these sorts of issues could result in cases where the client does not properly respond. Either by falling back to "basic" authentication, or getting stuck in a loop where the client was unable to re-negotiate a new session with the BMC. In the event of the BMC no longer being able to support Session authentication, after a session initialization, the client will raise an AccessError exception to signify to the consumer that something has occurred with credentials preventing further client use without potentially re-initializing the client. Should the re-authentication fail, the exception will be directly raised such that the client can no longer fall into a recursive loop. Should the state in the client be unable to be determined, and an AccessError is raised, the client will now attempt to re-authenticate. Ultimately moves from using the SessionService to using a new base class method ``create_session``. The SessionService class ``create_session`` method is still usable as well, however new authentication attempts will not utilize it in order to bypass potential failure cases. Functionally, this change requires change I9eb238d1bbaf522d03ee3fce12fc0ab80c1b69b4 to be present as it removes X-Auth-Token headers which can lead to spurious authentication failures when sessions expire. Prior to any authentication action, this patch does *also* do the needful to prevent a poisoned client which would have potentially resulted in an AccessError previously. In backporting, also minor conflict with https://review.opendev.org/c/openstack/sushy/+/799504 resulted in a minor change, specifically around the argument being passed which was added in that changeset. Additional minor changes around ConnectionError exceptions which previously did not include the ability to pass a message back as part of the error. Change-Id: Id9a32ebcc04f4f3fecb4e10cf157a97322707c94 Story: 2009719 Task: 44106 (cherry picked from commit 8d55d51284288ef7cc3dad4dce00fe0155b1e6f1) (cherry picked from commit 38e62dd16a971d75e85fcea53612da900f19f606) (cherry picked from commit 078707439a67f60d6626c36faa096713a88097ac) (cherry picked from commit 8427417e208d7d7b48348b7eb9c36ff78d87613f) --- doc/source/reference/usage.rst | 42 ++--- ...llback-failure-fixes-4f0dcfdad1afd2d7.yaml | 26 +++ sushy/auth.py | 79 ++++++--- sushy/connector.py | 21 ++- sushy/main.py | 67 +++++++- .../sessionservice/sessionservice.py | 19 ++- .../session_creation_headers_no_location.json | 17 ++ .../sessionservice/test_sessionservice.py | 12 +- sushy/tests/unit/test_auth.py | 160 ++++++++++++++---- sushy/tests/unit/test_connector.py | 100 ++++++++++- sushy/tests/unit/test_main.py | 106 ++++++++++++ 11 files changed, 556 insertions(+), 93 deletions(-) create mode 100644 releasenotes/notes/reauthentication-session-fallback-failure-fixes-4f0dcfdad1afd2d7.yaml create mode 100644 sushy/tests/unit/json_samples/session_creation_headers_no_location.json diff --git a/doc/source/reference/usage.rst b/doc/source/reference/usage.rst index 64e50be9..5a24acd5 100644 --- a/doc/source/reference/usage.rst +++ b/doc/source/reference/usage.rst @@ -248,9 +248,9 @@ Creating and using a sushy manager object virtmedia_inst.eject_media() -------------------------------------------------- -Creating and using a sushy session service object -------------------------------------------------- +----------------------------------------------- +Creating and using a sushy client with Sessions +----------------------------------------------- .. code-block:: python @@ -266,33 +266,27 @@ Creating and using a sushy session service object s = sushy.Sushy('http://localhost:8000/redfish/v1', username='foo', password='bar') - # Instantiate a SessionService object - sess_serv = s.get_session_service() + # Get the ComputerSystem object (if there is only one), otherwise + # the identity must be provided as a path to the system. + system = s.get_system() - # Get SessionCollection - sess_col = sess_serv.sessions + # A session is created automatically for you. + # Print the boot field in the ComputerSystem. + print(system.boot) - # Print the ID of the sessions available in the collection - print(sess_col.members_identities) + # Upon session timeout, Sushy recreates the session based upon + # provided credentials. If this fails, an exception is raised. - # Get a list of systems objects available in the collection - sess_col_insts = sess_col.get_members() + # Explicitly request a session_key and session_uri. + # This is not stored, but may be useful. + session_key, session_uri = s.create_session(username='foo', + password='bar') - # Instantiate a session object, same as getting it directly - sess_inst = sess_col.get_member(sess_col.members_identities[0]) - # Getting it directly - sess_inst = s.get_session(sess_col.members_identities[0]) + # Retrieve the session + session = s.get_session(session_uri) # Delete the session - sess_inst.delete() - - # Create a new session - session_key, session_id = sess_serv.create_session( - username='foo', password='bar') - - # Delete a session - sess_serv.close_session(sess_col.members_identities[0]) - + session.delete() -------------------- Using OEM extensions diff --git a/releasenotes/notes/reauthentication-session-fallback-failure-fixes-4f0dcfdad1afd2d7.yaml b/releasenotes/notes/reauthentication-session-fallback-failure-fixes-4f0dcfdad1afd2d7.yaml new file mode 100644 index 00000000..6d2c773c --- /dev/null +++ b/releasenotes/notes/reauthentication-session-fallback-failure-fixes-4f0dcfdad1afd2d7.yaml @@ -0,0 +1,26 @@ +--- +fixes: + - | + Fixes issues with the refresh of ``Session`` based authentication where + a previous refresh attempt failing could result in a fallback + to ``Basic`` authentication and would silently fail. The client library + now attempts to re-authenticate. + - | + Fixes silent failures when a refresh of an authentication ``Session`` + fails and was unable to be re-established due to an ``AccessError``. + Should this occur, now the ``AccessError`` exception is explicitly raised + as opposed to attempting to fall back to ``Basic`` authentication. + - | + Fixes issues where the ``Session`` and ``Basic`` auth interface would + fallback to ``Basic`` authentication should a ``ConnectionError`` + exception occur while attempting to perform an authentication action. + ``ConnectionError`` exceptions are signs of networking transport issues, + and should be investigated. A ``ConnectionError`` exception is now raised. + - | + Prevents the combined ``Session`` and ``Basic`` authentication support + from falling back to ``Basic`` authentication once ``Session`` based + authentication has been established. This should be considered a potential + security issue or an environmental change requiring potential client + re-initialization. This is exposed as an ``AccessError`` exception. + Continued operations against the Sushy library will attempt to + reauthenticate, if possible. diff --git a/sushy/auth.py b/sushy/auth.py index 479aa262..5c3ded9b 100644 --- a/sushy/auth.py +++ b/sushy/auth.py @@ -40,6 +40,8 @@ class AuthBase(object, metaclass=abc.ABCMeta): :param root_resource: Root sushy object :param connector: Connector for http connections """ + # Set the root resource, and connector to use + # for normal opreations. self._root_resource = root_resource self._connector = connector self._connector.set_auth(self) @@ -123,6 +125,8 @@ class SessionAuth(AuthBase): """Our Sessions Key""" self._session_resource_id = None """Our Sessions Unique Resource ID or URL""" + self._session_auth_previously_successful = False + """Our reminder for tracking if session auth has previously worked.""" super(SessionAuth, self).__init__(username, password) @@ -149,21 +153,18 @@ class SessionAuth(AuthBase): :raises: AccessError :raises: HTTPError """ - target_uri = None - try: - target_uri = self._root_resource.get_sessions_path() - except exceptions.MissingAttributeError: - LOG.debug('Missing Sessions attribute under Links in Root ' - 'Service, we\'ll try to determine it from Session ' - 'Service') - session_service = self._root_resource.get_session_service() - session_auth_token, session_uri = ( - session_service.create_session(self._username, - self._password, - target_uri=target_uri)) - self._session_key = session_auth_token + auth_token = None + + auth_token, session_uri = self._root_resource.create_session( + self._username, self._password) + # Record the session authentication data. + self._session_key = auth_token self._session_resource_id = session_uri - self._connector.set_http_session_auth(session_auth_token) + # Set flag so we know we've previously successfully achieved + # session authentication in order to lockout possible fallback during + # session refresh/renegotiation. + self._session_auth_previously_successful = True + self._connector.set_http_session_auth(auth_token) def can_refresh_session(self): """Method to assert if session based refresh can be done.""" @@ -225,6 +226,12 @@ class SessionOrBasicAuth(SessionAuth): super(SessionOrBasicAuth, self).__init__(username, password) self.basic_auth = BasicAuth(username=username, password=password) + def _fallback_to_basic_authentication(self): + """Fallback to basic authentication.""" + self.reset_session_attrs() + self.basic_auth.set_context(self._root_resource, self._connector) + self.basic_auth.authenticate() + def _do_authenticate(self): """Establish a RedfishSession. @@ -234,16 +241,50 @@ class SessionOrBasicAuth(SessionAuth): try: # Attempt session based authentication super(SessionOrBasicAuth, self)._do_authenticate() + except exceptions.AccessError as e: + if (not self.can_refresh_session() + and not self._session_auth_previously_successful): + # We should only try and fallback if we've not been able + # to establish a session. If we had a session previously + # and we're trying to fallback, something fishy is afoot. + LOG.warning('Falling back to "Basic" authentication as ' + 'we have been unable to authenticate to the ' + 'BMC. Exception: %(exception)s.', + {'exception': e}) + self._fallback_to_basic_authentication() + else: + # We previously had session authentication, something + # has changed which is far out of the ordinary. + LOG.debug('Received exception "%(exception)s" while ' + 'attempting to re-establish a session. ' + 'Raising AccessError, as something fishy ' + 'has occurred and the BMC has suddenly ' + 'ceased to support Session based ' + 'authentication.', + {'exception': e}) + raise + except exceptions.ConnectionError as e: + # The reason to explicitly catch a connectivity failure is the + # case where transitory connectivity failures can occur while + # working to authenticate. + LOG.debug('Encountered a connectivity failure while attempting ' + 'to authenticate. We will not explicitly fallback. ' + 'Error: %(exception)s', + {'exception': e}) + # Previously we would silently eat the failure as SushyError + # and fallback as it is a general fault. Callers on direct + # invocations through a connector _op method call can still + # receieve these exceptions, and applicaitons like Ironic do + # consider a client re-use disqualifier if there has been + # a connection failure, so it is okay for us to fix the behavior + # here. + raise except exceptions.SushyError as e: LOG.debug('Received exception "%(exception)s" while ' 'attempting to establish a session. ' 'Falling back to basic authentication.', {'exception': e}) - - # Fall back to basic authentication - self.reset_session_attrs() - self.basic_auth.set_context(self._root_resource, self._connector) - self.basic_auth.authenticate() + self._fallback_to_basic_authentication() def refresh_session(self): """Method to refresh a session to a Redfish controller. diff --git a/sushy/connector.py b/sushy/connector.py index 1f5ab5fd..c1c49b32 100644 --- a/sushy/connector.py +++ b/sushy/connector.py @@ -42,6 +42,9 @@ class Connector(object): self._response_callback = response_callback self._auth = None + # NOTE(TheJulia): In order to help prevent recursive post operations + # by allowing us to understand that we should stop authentication. + self._sessions_uri = None # NOTE(etingof): field studies reveal that some BMCs choke at # long-running persistent HTTP connections (or TCP connections). # By default, we ask HTTP server to shut down HTTP connection we've @@ -126,9 +129,23 @@ class Connector(object): try: exceptions.raise_for_response(method, url, response) except exceptions.AccessError as e: - if self._auth is not None and self._auth.can_refresh_session(): + if (method == 'POST' + and self._sessions_uri is not None + and self._sessions_uri in url): + LOG.error('Authentication to the session service failed. ' + 'Please check credentials and try again.') + raise + if self._auth is not None: try: - self._auth.refresh_session() + if self._auth.can_refresh_session(): + self._auth.refresh_session() + else: + LOG.warning('Session authentication appears to have ' + 'been lost at some point in time. ' + 'Connectivity may have been lost during ' + 'a prior session refresh. Attempting to ' + 're-authenticate.') + self._auth.authenticate() except exceptions.AccessError as refresh_exc: LOG.error("A failure occured while attempting to refresh " "the session. Error: %s", refresh_exc.message) diff --git a/sushy/main.py b/sushy/main.py index b483670e..6d62019a 100644 --- a/sushy/main.py +++ b/sushy/main.py @@ -14,6 +14,7 @@ # under the License. import collections import logging +import os import pkg_resources import requests @@ -367,10 +368,15 @@ class Sushy(base.ResourceBase): def get_sessions_path(self): """Returns the Sessions url""" - + # NOTE(TheJulia): This method is the standard discovery method + # advised by the DMTF DSP0266 standard to find the session service. try: links_url = self.json.get('Links') - return links_url['Sessions']['@odata.id'] + sessions_uri = links_url['Sessions']['@odata.id'] + # Save the session URL for post detection and prevention + # of recursive autentication attempts. + self._conn._sessions_uri = sessions_uri + return sessions_uri except (TypeError, KeyError): raise exceptions.MissingAttributeError( attribute='Links/Sessions/@data.id', resource=self.path) @@ -386,6 +392,63 @@ class Sushy(base.ResourceBase): redfish_version=self.redfish_version, registries=self.lazy_registries) + def create_session(self, username=None, password=None): + """Creates a session without invoking SessionService. + + For use when a new connection is to be established. Removes prior + Session and authentication data before making the request. + + :param username: The username to utilize to create a session with the + remote endpoint. + :param password: The password to utilize to create a session with the + remote endpoint. + :returns: A session key and uri in the form of a tuple + :raises: MissingXAuthToken + :raises: ConnectionError + :raises: AccessError + :raises: HTTPError + :raises: MissingAttributeError + """ + + # Explicitly removes in-client session data to proceed. This prevents + # AccessErrors as prior authentication shouldn't be submitted with a + # new authentication attempt, and doing so with old/invalid session + # data can result in an AccessError being raised. + self._conn._session.auth = None + if 'X-Auth-Token' in self._conn._session.headers: + # Delete the token value that was saved to the session + # as otherwise we would end up with a dictionary containing + # a {'X-Auth-Token': null} being sent across to the remote + # bmc. + del self._conn._session.headers['X-Auth-Token'] + try: + session_service_path = self.get_sessions_path() + except (exceptions.MissingAttributeError, exceptions.AccessError): + # Guesses path based upon DMTF standard, in the event + # the links resource on the service root is incorrect. + session_service_path = os.path.join(self._path, + 'SessionService/Sessions') + LOG.warning('Could not discover the Session service path, ' + 'falling back to %s.', + session_service_path) + session_url = self._root_prefix + 'SessionService/Sessions' + self._conn._sessions_uri = session_url + + data = {'UserName': username, 'Password': password} + LOG.debug("Requesting new session from %s.", + session_service_path) + rsp = self._conn.post(session_service_path, data=data) + session_key = rsp.headers.get('X-Auth-Token') + if session_key is None: + raise exceptions.MissingXAuthToken( + method='POST', url=session_service_path, response=rsp) + + session_uri = rsp.headers.get('Location') + if session_uri is None: + LOG.warning("Received X-Auth-Token but NO session uri.") + + return session_key, session_uri + def get_update_service(self): """Get the UpdateService object diff --git a/sushy/resources/sessionservice/sessionservice.py b/sushy/resources/sessionservice/sessionservice.py index 24f52764..f5468f71 100644 --- a/sushy/resources/sessionservice/sessionservice.py +++ b/sushy/resources/sessionservice/sessionservice.py @@ -47,6 +47,9 @@ class SessionService(base.ResourceBase): registries=None): """A class representing a SessionService + Warning: This class should only be invoked with a connector which + has already established authentication. + :param connector: A Connector instance :param identity: The identity of the SessionService resource :param redfish_version: The version of RedFish. Used to construct @@ -54,15 +57,11 @@ class SessionService(base.ResourceBase): :param registries: Dict of Redfish Message Registry objects to be used in any resource that needs registries to parse messages """ - try: - super(SessionService, self).__init__( - connector, identity, redfish_version, registries) - - except exceptions.AccessError as ae: - LOG.debug('Received access error "%s" when trying to refresh the ' - 'SessionService. If this happens before ' - 'authentication, we\'ll have to guess the Sessions URL.', - ae) + # Populating the base resource so session interactions can + # occur based on the contents of it. + super(SessionService, self).__init__( + connector, identity, redfish_version=redfish_version, + registries=registries) def _get_sessions_collection_path(self): """Helper function to find the SessionCollections path""" @@ -94,6 +93,8 @@ class SessionService(base.ResourceBase): def create_session(self, username, password, target_uri=None): """This function will try to create a session. + Create a session and return the associated key and URI. + :param username: the username of the user requesting a new session :param password: the password associated to the user requesting a new session diff --git a/sushy/tests/unit/json_samples/session_creation_headers_no_location.json b/sushy/tests/unit/json_samples/session_creation_headers_no_location.json new file mode 100644 index 00000000..f847d50c --- /dev/null +++ b/sushy/tests/unit/json_samples/session_creation_headers_no_location.json @@ -0,0 +1,17 @@ +{ + "Content-Security-Policy": "default-src 'none'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; connect-src 'self'; img-src 'self'; frame-src 'self'; font-src 'self'; object-src 'self'; style-src 'self' 'unsafe-inline'", + "ETag": "'W/\"7dc5e2b9\"'", + "Cache-Control": "max-age=0, no-cache, no-store, must-revalidate", + "Connection": "Keep-Alive", + "X-XSS-Protection": "1; mode=block", + "X-Auth-Token": "adc530e2016a0ea98c76c087f0e4b76f", + "Expires": "0", + "X-Frame-Options": "SAMEORIGIN", + "Content-Length": "392", + "X-Content-Type-Options": "nosniff", + "Content-Type": "application/json;charset=utf-8", + "OData-Version": "4.0", + "Keep-Alive": "timeout=1, max=32", + "Strict-Transport-Security": "max-age=31536000; includeSubDomains", + "Date": "Tue, 06 Jun 2017 17:07:48 GMT" +} diff --git a/sushy/tests/unit/resources/sessionservice/test_sessionservice.py b/sushy/tests/unit/resources/sessionservice/test_sessionservice.py index 61a102f8..a46b848c 100644 --- a/sushy/tests/unit/resources/sessionservice/test_sessionservice.py +++ b/sushy/tests/unit/resources/sessionservice/test_sessionservice.py @@ -37,15 +37,17 @@ class SessionServiceTestCase(base.TestCase): self.conn, '/redfish/v1/SessionService', redfish_version='1.0.2') - @mock.patch.object(sessionservice, 'LOG', autospec=True) - def test__init_throws_exception(self, mock_LOG): + def test__init_throws_exception(self): self.conn.get.return_value.json.reset_mock() self.conn.get.return_value.json.side_effect = ( exceptions.AccessError( 'GET', 'any_url', mock.MagicMock())) - sessionservice.SessionService( - self.conn, '/redfish/v1/SessionService', redfish_version='1.0.2') - self.assertTrue(mock_LOG.debug.called) + # Previously sushy would just mask these, but now we raise + # the access error to the user. + self.assertRaises(exceptions.AccessError, + sessionservice.SessionService, + self.conn, '/redfish/v1/SessionService', + redfish_version='1.0.2') def test__parse_attributes(self): self.sess_serv_inst._parse_attributes(self.json_doc) diff --git a/sushy/tests/unit/test_auth.py b/sushy/tests/unit/test_auth.py index e477a72e..b42d212a 100644 --- a/sushy/tests/unit/test_auth.py +++ b/sushy/tests/unit/test_auth.py @@ -140,24 +140,21 @@ class SessionAuthTestCase(base.TestCase): def test__do_authenticate(self): self.assertIsNone(self.sess_auth.get_session_resource_id()) self.assertIsNone(self.sess_auth.get_session_key()) - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.return_value = (self.sess_key, - self.sess_uri) - self.root.get_session_service.return_value = mock_sess_serv + self.assertFalse(self.sess_auth._session_auth_previously_successful) + self.root.create_session.return_value = (self.sess_key, + self.sess_uri) self.sess_auth.set_context(self.root, self.conn) self.sess_auth.authenticate() self.assertEqual(self.sess_uri, self.sess_auth.get_session_resource_id()) self.assertEqual(self.sess_key, self.sess_auth.get_session_key()) + self.assertTrue(self.sess_auth._session_auth_previously_successful) self.conn.set_http_session_auth.assert_called_once_with(self.sess_key) def test_can_refresh_session(self): - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.return_value = (self.sess_key, - self.sess_uri) - self.root.get_session_service.return_value = mock_sess_serv - + self.root.create_session.return_value = (self.sess_key, + self.sess_uri) self.sess_auth.set_context(self.root, self.conn) self.sess_auth.authenticate() @@ -166,10 +163,8 @@ class SessionAuthTestCase(base.TestCase): def test_refresh(self): self.assertIsNone(self.sess_auth.get_session_resource_id()) self.assertIsNone(self.sess_auth.get_session_key()) - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.return_value = (self.sess_key, - self.sess_uri) - self.root.get_session_service.return_value = mock_sess_serv + self.root.create_session.return_value = (self.sess_key, + self.sess_uri) self._session = mock.Mock(spec=requests.Session) self.sess_auth.set_context(self.root, self.conn) self.sess_auth.refresh_session() @@ -287,10 +282,8 @@ class SessionOrBasicAuthTestCase(base.TestCase): def test__do_authenticate(self): self.assertIsNone(self.sess_basic_auth.get_session_resource_id()) self.assertIsNone(self.sess_basic_auth.get_session_key()) - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.return_value = (self.sess_key, - self.sess_uri) - self.root.get_session_service.return_value = mock_sess_serv + self.root.create_session.return_value = (self.sess_key, + self.sess_uri) self.sess_basic_auth.set_context(self.root, self.conn) self.sess_basic_auth.authenticate() self.assertEqual(self.sess_uri, @@ -302,10 +295,7 @@ class SessionOrBasicAuthTestCase(base.TestCase): def test__do_authenticate_for_basic_auth(self): self.assertIsNone(self.sess_basic_auth.get_session_resource_id()) self.assertIsNone(self.sess_basic_auth.get_session_key()) - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.side_effect = exceptions.SushyError - self.root.get_session_service.return_value = mock_sess_serv - + self.root.create_session.side_effect = exceptions.SushyError self.sess_basic_auth.set_context(self.root, self.conn) self.sess_basic_auth.authenticate() @@ -315,11 +305,8 @@ class SessionOrBasicAuthTestCase(base.TestCase): self.username, self.password) def test_can_refresh_session(self): - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.return_value = (self.sess_key, - self.sess_uri) - self.root.get_session_service.return_value = mock_sess_serv - + self.root.create_session.return_value = (self.sess_key, + self.sess_uri) self.sess_basic_auth.set_context(self.root, self.conn) self.sess_basic_auth.authenticate() @@ -340,10 +327,8 @@ class SessionOrBasicAuthTestCase(base.TestCase): test_url = ('https://testing:8000/redfish/v1/SessionService' '/Sessions/testingfirst') self.sess_basic_auth._session_resource_id = test_url - mock_sess_serv = mock.Mock() - mock_sess_serv.create_session.return_value = (self.sess_key, - self.sess_uri) - self.root.get_session_service.return_value = mock_sess_serv + self.root.create_session.return_value = (self.sess_key, + self.sess_uri) self.sess_basic_auth.set_context(self.root, self.conn) self.sess_basic_auth.refresh_session() self.assertEqual(self.sess_uri, @@ -352,6 +337,121 @@ class SessionOrBasicAuthTestCase(base.TestCase): self.sess_basic_auth.get_session_key()) self.conn.set_http_session_auth.assert_called_once_with(self.sess_key) + @mock.patch.object(auth.SessionOrBasicAuth, + '_fallback_to_basic_authentication', + autospec=True) + def test_refresh_refresh_connection_error(self, mock_activate_basic_auth): + self.sess_basic_auth._session_key = 'ThisisFirstKey' + test_url = ('https://testing:8000/redfish/v1/SessionService' + '/Sessions/testingfirst') + self.sess_basic_auth._session_auth_previously_successful = True + self.sess_basic_auth._session_resource_id = test_url + self.root.create_session.side_effect = \ + exceptions.ConnectionError() + self.sess_basic_auth.set_context(self.root, self.conn) + self.assertRaises(exceptions.ConnectionError, + self.sess_basic_auth.refresh_session) + self.assertIsNone(self.sess_basic_auth.get_session_resource_id()) + self.assertIsNone(self.sess_basic_auth.get_session_key()) + self.conn.set_http_session_auth.assert_not_called() + self.assertIsNone(self.sess_basic_auth.basic_auth._root_resource) + self.assertIsNone(self.sess_basic_auth.basic_auth._connector) + self.assertFalse(mock_activate_basic_auth.called) + + @mock.patch.object(auth.SessionOrBasicAuth, + '_fallback_to_basic_authentication', + autospec=True) + def test_refresh_refresh_connection_error_clears( + self, mock_activate_basic_auth): + self.sess_basic_auth._session_key = 'ThisisFirstKey' + test_url = ('https://testing:8000/redfish/v1/SessionService' + '/Sessions/testingfirst') + self.sess_basic_auth._session_auth_previously_successful = True + self.sess_basic_auth._session_resource_id = test_url + self.root.create_session.side_effect = [ + exceptions.ConnectionError(), + (self.sess_key, self.sess_uri) + ] + self.sess_basic_auth.set_context(self.root, self.conn) + self.assertRaises(exceptions.ConnectionError, + self.sess_basic_auth.refresh_session) + self.assertIsNone(self.sess_basic_auth.get_session_resource_id()) + self.assertIsNone(self.sess_basic_auth.get_session_key()) + self.conn.set_http_session_auth.assert_not_called() + self.assertIsNone(self.sess_basic_auth.basic_auth._root_resource) + self.assertIsNone(self.sess_basic_auth.basic_auth._connector) + self.assertFalse(mock_activate_basic_auth.called) + # Refresh no longer works, explicit authentication is now required. + self.sess_basic_auth.refresh_session() + self.conn.set_http_session_auth.assert_not_called() + + self.sess_basic_auth.authenticate() + self.conn.set_http_session_auth.assert_called_once_with(self.sess_key) + + @mock.patch.object(auth.SessionOrBasicAuth, + '_fallback_to_basic_authentication', + autospec=True) + def test_authenticate_session_fails(self, mock_activate_basic_auth): + self.sess_basic_auth._session_key = None + self.sess_basic_auth._session_auth_previously_successful = False + test_url = ('https://testing:8000/redfish/v1/SessionService' + '/Sessions/testingfirst') + self.sess_basic_auth._session_resource_id = test_url + ae_exc = exceptions.AccessError( + 'GET', 'any_url', mock.MagicMock()) + + self.root.create_session.side_effect = ae_exc + self.sess_basic_auth.set_context(self.root, self.conn) + self.sess_basic_auth.authenticate() + # We fall back to basic auth as we failed to authenticate. + self.assertTrue(mock_activate_basic_auth.called) + + @mock.patch.object(auth, 'LOG', autospec=True) + @mock.patch.object(auth.SessionOrBasicAuth, + '_fallback_to_basic_authentication', + autospec=True) + def test_authenticate_session_fails_reauth_raises_exception( + self, mock_activate_basic_auth, mock_log): + self.sess_basic_auth._session_key = None + self.sess_basic_auth._session_auth_previously_successful = True + test_url = ('https://testing:8000/redfish/v1/SessionService' + '/Sessions/testingfirst') + self.sess_basic_auth._session_resource_id = test_url + ae_exc = exceptions.AccessError( + 'GET', 'any_url', mock.MagicMock()) + + self.root.create_session.side_effect = ae_exc + self.sess_basic_auth.set_context(self.root, self.conn) + self.assertRaises(exceptions.AccessError, + self.sess_basic_auth.authenticate) + self.assertTrue(mock_log.debug.called) + # We do not fallback to basic auth if we have already been + # authenticated. + self.assertFalse(mock_activate_basic_auth.called) + + @mock.patch.object(auth.SessionOrBasicAuth, + '_fallback_to_basic_authentication', + autospec=True) + def test_authenticate_session_fails_connection_error( + self, mock_activate_basic_auth): + self.sess_basic_auth._session_key = None + self.sess_basic_auth._session_auth_previously_successful = False + test_url = ('https://testing:8000/redfish/v1/SessionService' + '/Sessions/testingfirst') + self.sess_basic_auth._session_resource_id = test_url + mock_sess_serv = mock.Mock() + + ae_exc = exceptions.ConnectionError() + + self.root.create_session.side_effect = ae_exc + self.root.get_session_service.return_value = mock_sess_serv + self.sess_basic_auth.set_context(self.root, self.conn) + self.assertRaises(exceptions.ConnectionError, + self.sess_basic_auth.authenticate) + # We don't fall back to basic auth if we've never connected + # before + self.assertFalse(mock_activate_basic_auth.called) + def test_close_do_nothing(self): self.conn.delete.assert_not_called() diff --git a/sushy/tests/unit/test_connector.py b/sushy/tests/unit/test_connector.py index 603430e7..049fcbad 100644 --- a/sushy/tests/unit/test_connector.py +++ b/sushy/tests/unit/test_connector.py @@ -263,6 +263,11 @@ class ConnectorOpTestCase(base.TestCase): self.request = self.session.request self.request.return_value.status_code = http_client.FORBIDDEN self.request.return_value.json.side_effect = ValueError('no json') + mock_response = mock.Mock() + mock_response.json.side_effect = ValueError('no json') + mock_response.status_code = http_client.FORBIDDEN + self.conn._auth.authenticate.side_effect = \ + exceptions.AccessError('POST', 'fake/path', mock_response) with self.assertRaisesRegex(exceptions.AccessError, 'unknown error') as ae: self.conn._op('POST', path='fake/path', data=self.data, @@ -289,6 +294,42 @@ class ConnectorOpTestCase(base.TestCase): self.auth.can_refresh_session.assert_called_with() self.assertEqual(response.json, second_response.json) + def test_timed_out_session_timed_out_refresh(self): + self.auth._session_key = 'asdf1234' + self.auth.get_session_key.return_value = 'asdf1234' + self.conn._auth = self.auth + self.session = mock.Mock(spec=requests.Session) + self.conn._session = self.session + self.request = self.session.request + first_response = mock.MagicMock() + first_response.status_code = http_client.FORBIDDEN + second_response = first_response + self.auth.refresh_session.side_effect = \ + exceptions.ConnectionError() + third_response = mock.MagicMock() + third_response.status_code = http_client.OK + third_response.json = {'Test': 'Testing'} + self.auth.can_refresh_session.return_value = True + + self.request.side_effect = [first_response, second_response, + third_response] + self.assertRaises(exceptions.ConnectionError, self.conn._op, 'POST', + path='fake/path', data=self.data, + headers=self.headers) + self.auth.refresh_session.assert_called_with() + self.auth.refresh_session.reset_mock() + # Normally, this would be reset by refresh_session, but given + # the heavy mocking, we need to do it for this test. + self.auth._session_key = None + self.auth.get_session_key.return_value = None + self.auth.can_refresh_session.return_value = False + + response = self.conn._op('POST', path='fake/path', data=self.data, + headers=self.headers) + self.auth.refresh_session.assert_not_called() + self.auth.authenticate.assert_called_once() + self.assertEqual(response.json, third_response.json) + def test_connection_error(self): self.request.side_effect = requests.exceptions.ConnectionError self.assertRaises(exceptions.ConnectionError, self.conn._op, 'GET') @@ -361,7 +402,7 @@ class ConnectorOpTestCase(base.TestCase): self.assertEqual(6, self.request.call_count) def test_access_error(self): - self.conn._auth.can_refresh_session.return_value = False + self.conn._auth = None self.request.return_value.status_code = http_client.FORBIDDEN self.request.return_value.json.side_effect = ValueError('no json') @@ -372,6 +413,42 @@ class ConnectorOpTestCase(base.TestCase): exc = cm.exception self.assertEqual(http_client.FORBIDDEN, exc.status_code) + def test_access_error_on_session_post_does_not_retry(self): + self.request.return_value.status_code = http_client.FORBIDDEN + self.request.return_value.json.side_effect = ValueError('no json') + + self.conn._sessions_uri = '/redfish/v1/SessionService/Sessions' + + with self.assertRaisesRegex(exceptions.AccessError, + 'unknown error') as cm: + self.conn._op('POST', 'http://foo.bar/redfish/v1/Session' + 'Service/Sessions', + data={'foo': 'bar'}) + exc = cm.exception + self.assertEqual(http_client.FORBIDDEN, exc.status_code) + self.request.assert_called_once() + + def test_access_error_triggers_auth_attempt(self): + self.conn._auth.can_refresh_session.return_value = False + value_error = ValueError('no json') + mock_response = mock.Mock() + mock_response.json.side_effect = value_error + mock_response.status_code = http_client.FORBIDDEN + # This doesn't test/wire all way back through auth -> main + # and ultimately we want to make sure we get an error all + # the way back. + self.conn._auth.authenticate.side_effect = \ + exceptions.AccessError('GET', '/', mock_response) + self.request.return_value.status_code = http_client.FORBIDDEN + self.request.return_value.json.side_effect = value_error + + with self.assertRaisesRegex(exceptions.AccessError, + 'unknown error') as cm: + self.conn._op('GET', 'http://foo.bar') + exc = cm.exception + self.assertEqual(http_client.FORBIDDEN, exc.status_code) + self.conn._auth.authenticate.assert_called_once() + def test_access_error_without_auth(self): self.conn._auth = None @@ -385,9 +462,28 @@ class ConnectorOpTestCase(base.TestCase): self.assertEqual(http_client.FORBIDDEN, exc.status_code) @mock.patch.object(connector.LOG, 'debug', autospec=True) - def test_access_error_service_session(self, mock_log): + def test_access_error_service_session_reauth(self, mock_log): self.conn._auth.can_refresh_session.return_value = False + self.request.return_value.status_code = http_client.FORBIDDEN + mock_response = mock.Mock() + mock_response.json.side_effect = ValueError('no json') + mock_response.status_code = http_client.FORBIDDEN + self.conn._auth.authenticate.side_effect = \ + exceptions.AccessError('POST', 'fake/path', mock_response) + self.request.return_value.json.side_effect = ValueError('no json') + + with self.assertRaisesRegex(exceptions.AccessError, + 'unknown error') as cm: + self.conn._op('GET', 'http://redfish/v1/SessionService') + exc = cm.exception + self.assertEqual(http_client.FORBIDDEN, exc.status_code) + self.conn._auth.authenticate.assert_called_once() + + @mock.patch.object(connector.LOG, 'debug', autospec=True) + def test_access_error_service_session_no_auth(self, mock_log): + self.conn._auth = None + self.request.return_value.status_code = http_client.FORBIDDEN self.request.return_value.json.side_effect = ValueError('no json') diff --git a/sushy/tests/unit/test_main.py b/sushy/tests/unit/test_main.py index 096f7db2..11c9e216 100644 --- a/sushy/tests/unit/test_main.py +++ b/sushy/tests/unit/test_main.py @@ -280,6 +280,110 @@ class MainTestCase(base.TestCase): self.root._conn, 'asdf', self.root.redfish_version, self.root.lazy_registries) + def test_create_session(self): + self.root._conn._session.headers = [] + self.root._conn._sessions_uri = None + with open('sushy/tests/unit/json_samples/' + 'session_creation_headers.json') as f: + self.conn.post.return_value.headers = json.load(f) + + session_key, session_uri = ( + self.root.create_session('foo', 'secret')) + self.assertEqual('adc530e2016a0ea98c76c087f0e4b76f', session_key) + self.assertEqual( + '/redfish/v1/SessionService/Sessions/151edd65d41c0b89', + session_uri) + self.assertEqual('/redfish/v1/SessionService/Sessions', + self.root._conn._sessions_uri) + + def test_create_session_removes_auth_data(self): + self.root._conn._session.headers = {'X-Auth-Token': 'meow'} + self.root._conn._session.auth = 'meow' + with open('sushy/tests/unit/json_samples/' + 'session_creation_headers.json') as f: + self.conn.post.return_value.headers = json.load(f) + + session_key, session_uri = ( + self.root.create_session('foo', 'secret')) + self.assertEqual('adc530e2016a0ea98c76c087f0e4b76f', session_key) + self.assertEqual( + '/redfish/v1/SessionService/Sessions/151edd65d41c0b89', + session_uri) + self.assertIsNone(self.root._conn._session.auth) + self.assertNotIn('X-Auth-Token', self.root._conn._session.headers) + + def test_create_session_no_session_path(self): + self.root._conn._session.headers = [] + mock_get_session_path = mock.Mock() + mock_get_session_path.side_effect = exceptions.MissingAttributeError() + self.root.get_sessions_path = mock_get_session_path + with open('sushy/tests/unit/json_samples/' + 'session_creation_headers.json') as f: + self.conn.post.return_value.headers = json.load(f) + + session_key, session_uri = ( + self.root.create_session('foo', 'secret')) + self.assertEqual('adc530e2016a0ea98c76c087f0e4b76f', session_key) + self.assertEqual( + '/redfish/v1/SessionService/Sessions/151edd65d41c0b89', + session_uri) + self.conn.post.assert_called_once_with( + '/redfish/v1/SessionService/Sessions', + data={'UserName': 'foo', 'Password': 'secret'}) + + @mock.patch.object(main, 'LOG', autospec=True) + def test_create_session_no_session_path_access_error(self, mock_log): + self.root._conn._session.headers = [] + mock_res = mock.Mock() + mock_res.status_code = 403 + mock_res.json.side_effect = ValueError('no json') + mock_get_session_path = mock.Mock() + mock_get_session_path.side_effect = exceptions.AccessError( + 'GET', 'redfish/v1', mock_res) + self.root.get_sessions_path = mock_get_session_path + with open('sushy/tests/unit/json_samples/' + 'session_creation_headers_no_location.json') as f: + self.conn.post.return_value.headers = json.load(f) + + session_key, session_uri = ( + self.root.create_session('foo', 'secret')) + self.assertEqual('adc530e2016a0ea98c76c087f0e4b76f', session_key) + self.assertIsNone(session_uri) + self.conn.post.assert_called_once_with( + '/redfish/v1/SessionService/Sessions', + data={'UserName': 'foo', 'Password': 'secret'}) + self.assertTrue(mock_log.warning.called) + + def test_create_session_path_discovery(self): + self.root._conn._session.headers = [] + with open('sushy/tests/unit/json_samples/root.json') as f: + self.conn.get.json.return_value = json.load(f) + with open('sushy/tests/unit/json_samples/' + 'session_creation_headers.json') as f: + self.conn.post.return_value.headers = json.load(f) + + session_key, session_uri = ( + self.root.create_session('foo', 'secret')) + self.assertEqual('adc530e2016a0ea98c76c087f0e4b76f', session_key) + self.assertEqual( + '/redfish/v1/SessionService/Sessions/151edd65d41c0b89', + session_uri) + self.conn.get.assert_called_once_with(path='/redfish/v1/') + self.conn.post.assert_called_once_with( + '/redfish/v1/SessionService/Sessions', + data={'UserName': 'foo', 'Password': 'secret'}) + + def test_create_session_missing_x_auth_token(self): + self.root._conn._session.headers = [] + with open('sushy/tests/unit/json_samples/' + 'session_creation_headers.json') as f: + self.conn.post.return_value.headers = json.load(f) + + self.conn.post.return_value.headers.pop('X-Auth-Token') + self.assertRaisesRegex( + exceptions.MissingXAuthToken, 'No X-Auth-Token returned', + self.root.create_session, 'foo', 'bar') + @mock.patch.object(updateservice, 'UpdateService', autospec=True) def test_get_update_service(self, mock_upd_serv): self.root.get_update_service() @@ -395,8 +499,10 @@ class MainTestCase(base.TestCase): self.assertEqual(1, mock_registries.__getitem__.call_count) def test_get_sessions_path(self): + self.root._conn._sessions_uri = None expected = '/redfish/v1/SessionService/Sessions' self.assertEqual(expected, self.root.get_sessions_path()) + self.assertEqual(expected, self.root._conn._sessions_uri) class BareMinimumMainTestCase(base.TestCase):