Handle AccessError with Basic Auth instead of "reauth"

An operator reported issues when changing the password on a physical
machine and in Ironic, because connections are cached. However when
they supplied logs, they were actually not raising an error back to
Ironic that signaled an authentication failure, which would have
triggered the creation of a new session.

This is now handled appropriately.

Story: 2010162
Task: 45823

Change-Id: Ic52c4353cb1e1674511417223de10fa625b99af1

releasenotes/notes/handle-basic-auth-access-errors-393b368b31f5cad2.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes an issue in the sushy connector object handling where a
+    Sushy client utilizing ``basic`` authentication would not raise
+    an AccessError exception once the credentials stopped working.
+    We now explicitly check to see if ``basic`` authentication
+    is in use, and raise the exception if an AccessError is
+    encountered.

sushy/auth.py

@@ -101,6 +101,7 @@ class BasicAuth(AuthBase):
         """Attempts to establish a Basic Authentication Session.
 
         """
+        LOG.debug('Setting basic authentication connector information.')
         self._connector.set_http_basic_auth(self._username, self._password)
 
     def can_refresh_session(self):

sushy/connector.py

@@ -136,6 +136,15 @@ class Connector(object):
                           'Please check credentials and try again.')
                 raise
             if self._auth is not None:
+                # self._session.auth value is only set when basic auth is used
+                if self._session.auth is not None:
+                    LOG.warning('We have encountered an AccessError when '
+                                'using \'basic\' authentication. %(err)s',
+                                {'err': str(e)})
+                    # NOTE(TheJulia): There is no way to recover Basic auth,
+                    # as we need the client to be re-launched with new
+                    # credentials.
+                    raise
                 try:
                     if self._auth.can_refresh_session():
                         self._auth.refresh_session()

sushy/main.py

@@ -422,7 +422,6 @@ class Sushy(base.ResourceBase):
         :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

sushy/tests/unit/test_connector.py

@@ -259,6 +259,7 @@ class ConnectorOpTestCase(base.TestCase):
 
     def test_timed_out_session_unable_to_create_session(self):
         self.conn._auth.can_refresh_session.return_value = False
+        self.session.auth = None
         self.conn._session = self.session
         self.request = self.session.request
         self.request.return_value.status_code = http_client.FORBIDDEN
@@ -280,6 +281,7 @@ class ConnectorOpTestCase(base.TestCase):
         self.auth.get_session_key.return_value = 'asdf1234'
         self.conn._auth = self.auth
         self.session = mock.Mock(spec=requests.Session)
+        self.session.auth = None
         self.conn._session = self.session
         self.request = self.session.request
         first_response = mock.MagicMock()
@@ -299,6 +301,7 @@ class ConnectorOpTestCase(base.TestCase):
         self.auth.get_session_key.return_value = 'asdf1234'
         self.conn._auth = self.auth
         self.session = mock.Mock(spec=requests.Session)
+        self.session.auth = None
         self.conn._session = self.session
         self.request = self.session.request
         first_response = mock.MagicMock()
@@ -434,6 +437,8 @@ class ConnectorOpTestCase(base.TestCase):
         mock_response = mock.Mock()
         mock_response.json.side_effect = value_error
         mock_response.status_code = http_client.FORBIDDEN
+        self.session.auth = None
+        self.conn._session = self.session
         # 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.
@@ -469,6 +474,8 @@ class ConnectorOpTestCase(base.TestCase):
         mock_response = mock.Mock()
         mock_response.json.side_effect = ValueError('no json')
         mock_response.status_code = http_client.FORBIDDEN
+        self.session.auth = None
+        self.conn._session = self.session
         self.conn._auth.authenticate.side_effect = \
             exceptions.AccessError('POST', 'fake/path', mock_response)
         self.request.return_value.json.side_effect = ValueError('no json')
@@ -527,3 +534,27 @@ class ConnectorOpTestCase(base.TestCase):
         self.request.side_effect = [response1, response1, response2]
         with self.assertRaisesRegex(exceptions.BadRequestError, message):
             self.conn._op('POST', 'http://foo.bar', blocking=True)
+
+    @mock.patch.object(connector.LOG, 'warning', autospec=True)
+    def test_access_error_basic_auth(self, mock_log):
+        self.conn._auth.can_refresh_session.return_value = False
+        self.conn._session.auth = ('foo', 'bar')
+        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.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_not_called()
+        mock_log.assert_called_once_with(
+            "We have encountered an AccessError when using 'basic' "
+            "authentication. %(err)s",
+            {'err': 'HTTP GET http://redfish/v1/SessionService '
+                    'returned code HTTPStatus.FORBIDDEN. unknown error '
+                    'Extended information: none'}
+        )