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
(cherry picked from commit 10578574e8)
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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'}
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user