implement basic-auth support for user-image download process
This feature was proposed in https://bugs.launchpad.net/ironic-python-agent/+bug/2021947 Change-Id: I9dbfc1402240beb75b6736214753fd86dccae676
This commit is contained in:
parent
1581f91826
commit
70961789a6
@ -262,6 +262,31 @@ cli_opts = [
|
||||
help='Pre-shared token to use when working with the '
|
||||
'ironic API. This value is typically supplied by '
|
||||
'ironic automatically.'),
|
||||
cfg.StrOpt('image_server_user',
|
||||
default=None,
|
||||
help='Pre-shared username used in Basic Auth process '
|
||||
'against the http(s) server that hosts the disk image.'
|
||||
'This variable can be also configured via image_info.'
|
||||
'Value coming from image_info takes precedence over'
|
||||
'value coming from command line or configuration file.'),
|
||||
cfg.StrOpt('image_server_password',
|
||||
default=None,
|
||||
help='Pre-shared password used in Basic Auth process '
|
||||
'against the http(s) server that hosts the disk image.'
|
||||
'This variable can be also configured via image_info.'
|
||||
'Value coming from image_info takes precedence over'
|
||||
'value coming from command line or configuration file.'),
|
||||
cfg.StrOpt('image_server_auth_strategy',
|
||||
default="noauth",
|
||||
help='Option to select authentication strategy used during'
|
||||
'communication with the server that hosts the disk images'
|
||||
'and related checksums. This option also turns'
|
||||
'image_server_password and image_server_user'
|
||||
'into mandatory variables for those authentication'
|
||||
'strategies that require username + password credentials.'
|
||||
'This variable can be also configured via image_info.'
|
||||
'Value coming from image_info takes precedence over'
|
||||
'value coming from command line or configuration file.'),
|
||||
cfg.BoolOpt('agent_token_required',
|
||||
default=APARAMS.get('ipa-agent-token-required', False),
|
||||
help='Control to enforce if API command requests should '
|
||||
|
@ -47,6 +47,93 @@ def _image_location(image_info):
|
||||
return os.path.join(tempfile.gettempdir(), image_info['id'])
|
||||
|
||||
|
||||
def _verify_basic_auth_creds(user, password, image_id):
|
||||
"""Verify the basic auth credentials used for image download are present.
|
||||
|
||||
:param user: Basic auth username
|
||||
:param password: Basic auth password
|
||||
:param image_id: id of the image that is being acted upon
|
||||
|
||||
:raises ImageDownloadError if the credentials are not present
|
||||
"""
|
||||
expected_creds = {'user': user, 'password': password}
|
||||
missing_creds = []
|
||||
for key, value in expected_creds.items():
|
||||
if not value:
|
||||
missing_creds.append(key)
|
||||
if missing_creds:
|
||||
raise errors.ImageDownloadError(
|
||||
image_id,
|
||||
"Missing {} fields from HTTP(S) "
|
||||
"basic auth config".format(missing_creds)
|
||||
)
|
||||
|
||||
|
||||
def _gen_auth_from_image_info_user_pass(image_info, image_id):
|
||||
"""This function is used to pass the credentials to the chosen
|
||||
|
||||
credential verifier and in case the verification is successful
|
||||
generate the compatible authentication object that will be used
|
||||
with the request(s). This function handles the authentication object
|
||||
generation for authentication strategies that are username+password
|
||||
based. Credentials are collected via image_info.
|
||||
|
||||
:param image_info: Image information dictionary.
|
||||
:param image_id: id of the image that is being acted upon
|
||||
|
||||
:return: Authentication object used directly by the request library
|
||||
:rtype: requests.auth.HTTPBasicAuth
|
||||
"""
|
||||
image_server_user = None
|
||||
image_server_password = None
|
||||
|
||||
if image_info.get('image_server_auth_strategy') == 'http_basic':
|
||||
image_server_user = image_info.get('image_server_user')
|
||||
image_server_password = image_info.get('image_server_password')
|
||||
_verify_basic_auth_creds(
|
||||
image_server_user,
|
||||
image_server_password,
|
||||
image_id
|
||||
)
|
||||
else:
|
||||
return None
|
||||
|
||||
return requests.auth.HTTPBasicAuth(image_server_user,
|
||||
image_server_password)
|
||||
|
||||
|
||||
def _gen_auth_from_oslo_conf_user_pass(image_id):
|
||||
"""This function is used to pass the credentials to the chosen
|
||||
|
||||
credential verifier and in case the verification is successful
|
||||
generate the compatible authentication object that will be used
|
||||
with the request(s). This function handles the authentication object
|
||||
generation for authentication strategies that are username+password
|
||||
based. Credentials are collected from the oslo.config framework.
|
||||
|
||||
:param image_id: id of the image that is being acted upon
|
||||
|
||||
:return: Authentication object used directly by the request library
|
||||
:rtype: requests.auth.HTTPBasicAuth
|
||||
"""
|
||||
|
||||
image_server_user = None
|
||||
image_server_password = None
|
||||
|
||||
if CONF.image_server_auth_strategy == 'http_basic':
|
||||
_verify_basic_auth_creds(
|
||||
CONF.image_server_user,
|
||||
CONF.image_server_password,
|
||||
image_id)
|
||||
image_server_user = CONF.image_server_user
|
||||
image_server_password = CONF.image_server_password
|
||||
else:
|
||||
return None
|
||||
|
||||
return requests.auth.HTTPBasicAuth(image_server_user,
|
||||
image_server_password)
|
||||
|
||||
|
||||
def _download_with_proxy(image_info, url, image_id):
|
||||
"""Opens a download stream for the given URL.
|
||||
|
||||
@ -56,13 +143,31 @@ def _download_with_proxy(image_info, url, image_id):
|
||||
|
||||
:raises: ImageDownloadError if the download stream was not started
|
||||
properly.
|
||||
|
||||
:return: HTTP(s) server response for the image/hash download request
|
||||
:rtype: requests.Response
|
||||
"""
|
||||
|
||||
no_proxy = image_info.get('no_proxy')
|
||||
if no_proxy:
|
||||
os.environ['no_proxy'] = no_proxy
|
||||
proxies = image_info.get('proxies', {})
|
||||
verify, cert = utils.get_ssl_client_options(CONF)
|
||||
resp = None
|
||||
image_download_attributes = {
|
||||
"stream": True,
|
||||
"proxies": proxies,
|
||||
"verify": verify,
|
||||
"cert": cert,
|
||||
"timeout": CONF.image_download_connection_timeout
|
||||
}
|
||||
# NOTE(Adam) `image_info` is prioritized over `oslo.conf` for credential
|
||||
# collection and auth strategy selection
|
||||
auth_object = _gen_auth_from_image_info_user_pass(image_info, image_id)
|
||||
if auth_object is None:
|
||||
auth_object = _gen_auth_from_oslo_conf_user_pass(image_id)
|
||||
if auth_object is not None:
|
||||
image_download_attributes['auth'] = auth_object
|
||||
for attempt in range(CONF.image_download_connection_retries + 1):
|
||||
try:
|
||||
# NOTE(TheJulia) The get request below does the following:
|
||||
@ -78,9 +183,8 @@ def _download_with_proxy(image_info, url, image_id):
|
||||
# exactly just as the timeout value exists. The risk in transitory
|
||||
# failure is more so once we've started the download and we are
|
||||
# processing the incoming data.
|
||||
resp = requests.get(url, stream=True, proxies=proxies,
|
||||
verify=verify, cert=cert,
|
||||
timeout=CONF.image_download_connection_timeout)
|
||||
# B113 issue is covered is the image_download_attributs list
|
||||
resp = requests.get(url, **image_download_attributes) # nosec
|
||||
if resp.status_code != 200:
|
||||
msg = ('Received status code {} from {}, expected 200. '
|
||||
'Response body: {} Response headers: {}').format(
|
||||
|
@ -204,6 +204,86 @@ class TestStandbyExtension(base.IronicAgentTest):
|
||||
expected_loc = os.path.join(tempfile.gettempdir(), 'fake_id')
|
||||
self.assertEqual(expected_loc, location)
|
||||
|
||||
def test_verify_basic_auth_creds(self):
|
||||
image_info = _build_fake_image_info()
|
||||
self.assertIsNone(standby._verify_basic_auth_creds("SpongeBob",
|
||||
"SquarePants",
|
||||
image_info['id']))
|
||||
|
||||
def test_gen_auth_from_image_info_user_pass_success(self):
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['image_server_auth_strategy'] = 'http_basic'
|
||||
image_info['image_server_user'] = 'SpongeBob'
|
||||
image_info['image_server_password'] = 'SquarePants'
|
||||
exp_auth = requests.auth.HTTPBasicAuth('SpongeBob', 'SquarePants')
|
||||
return_auth = \
|
||||
standby._gen_auth_from_image_info_user_pass(image_info,
|
||||
image_info['id'])
|
||||
self.assertEqual(exp_auth, return_auth)
|
||||
|
||||
def test_gen_auth_from_image_info_user_pass_none(self):
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['image_server_auth_strategy'] = ''
|
||||
image_info['image_server_user'] = 'SpongeBob'
|
||||
image_info['image_server_password'] = 'SquarePants'
|
||||
return_auth = \
|
||||
standby._gen_auth_from_image_info_user_pass(image_info,
|
||||
image_info['id'])
|
||||
self.assertIsNone(return_auth)
|
||||
|
||||
def test_gen_auth_from_oslo_conf_user_pass_success(self):
|
||||
image_info = _build_fake_image_info()
|
||||
CONF.set_override('image_server_auth_strategy', 'http_basic')
|
||||
CONF.set_override('image_server_password', 'SpongeBob')
|
||||
CONF.set_override('image_server_user', 'SquarePants')
|
||||
correct_auth = \
|
||||
requests.auth.HTTPBasicAuth(CONF['image_server_user'],
|
||||
CONF['image_server_password'])
|
||||
return_auth = \
|
||||
standby._gen_auth_from_oslo_conf_user_pass(image_info['id'])
|
||||
self.assertEqual(correct_auth, return_auth)
|
||||
|
||||
def test_gen_auth_from_oslo_conf_user_pass_none(self):
|
||||
image_info = _build_fake_image_info()
|
||||
CONF.set_override('image_server_auth_strategy', 'noauth')
|
||||
CONF.set_override('image_server_password', 'SpongeBob')
|
||||
CONF.set_override('image_server_user', 'SquarePants')
|
||||
return_auth = \
|
||||
standby._gen_auth_from_oslo_conf_user_pass(image_info['id'])
|
||||
self.assertIsNone(return_auth)
|
||||
|
||||
def test_verify_basic_auth_creds_empty_user(self):
|
||||
image_info = _build_fake_image_info()
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._verify_basic_auth_creds,
|
||||
"",
|
||||
"SquarePants",
|
||||
image_info['id'])
|
||||
|
||||
def test_verify_basic_auth_creds_empty_password(self):
|
||||
image_info = _build_fake_image_info()
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._verify_basic_auth_creds,
|
||||
"SpongeBob",
|
||||
"",
|
||||
image_info['id'])
|
||||
|
||||
def test_verify_basic_auth_creds_none_user(self):
|
||||
image_info = _build_fake_image_info()
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._verify_basic_auth_creds,
|
||||
None,
|
||||
"SquarePants",
|
||||
image_info['id'])
|
||||
|
||||
def test_verify_basic_auth_creds_none_password(self):
|
||||
image_info = _build_fake_image_info()
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._verify_basic_auth_creds,
|
||||
"SpongeBob",
|
||||
None,
|
||||
image_info['id'])
|
||||
|
||||
@mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True)
|
||||
@mock.patch('ironic_lib.disk_utils.trigger_device_rescan', autospec=True)
|
||||
@mock.patch('ironic_lib.disk_utils.convert_image', autospec=True)
|
||||
@ -504,6 +584,106 @@ class TestStandbyExtension(base.IronicAgentTest):
|
||||
standby._download_image,
|
||||
image_info)
|
||||
|
||||
@mock.patch('hashlib.new', autospec=True)
|
||||
@mock.patch('builtins.open', autospec=True)
|
||||
@mock.patch('requests.get', autospec=True)
|
||||
def test_download_image_basic_auth_conf_success(self, requests_mock,
|
||||
open_mock, hash_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
CONF.set_override('image_server_auth_strategy', 'http_basic')
|
||||
CONF.set_override('image_server_password', 'SpongeBob')
|
||||
CONF.set_override('image_server_user', 'SquarePants')
|
||||
user = CONF.image_server_user
|
||||
password = CONF.image_server_password
|
||||
correct_auth = requests.auth.HTTPBasicAuth(user, password)
|
||||
response = requests_mock.return_value
|
||||
response.status_code = 200
|
||||
response.iter_content.return_value = ['some', 'content']
|
||||
file_mock = mock.Mock()
|
||||
open_mock.return_value.__enter__.return_value = file_mock
|
||||
file_mock.read.return_value = None
|
||||
hexdigest_mock = hash_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = image_info['os_hash_value']
|
||||
|
||||
standby._download_image(image_info)
|
||||
requests_mock.assert_called_once_with(image_info['urls'][0],
|
||||
cert=None, verify=True,
|
||||
stream=True, proxies={},
|
||||
timeout=60, auth=correct_auth)
|
||||
write = file_mock.write
|
||||
write.assert_any_call('some')
|
||||
write.assert_any_call('content')
|
||||
self.assertEqual(2, write.call_count)
|
||||
|
||||
@mock.patch('hashlib.new', autospec=True)
|
||||
@mock.patch('builtins.open', autospec=True)
|
||||
@mock.patch('requests.get', autospec=True)
|
||||
def test_download_image_basic_auth_image_info_success(self,
|
||||
requests_mock,
|
||||
open_mock,
|
||||
hash_mock):
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['image_server_auth_strategy'] = 'http_basic'
|
||||
image_info['image_server_password'] = 'SpongeBob'
|
||||
image_info['image_server_user'] = 'SquarePants'
|
||||
user = image_info['image_server_user']
|
||||
password = image_info['image_server_password']
|
||||
correct_auth = requests.auth.HTTPBasicAuth(user, password)
|
||||
response = requests_mock.return_value
|
||||
response.status_code = 200
|
||||
response.iter_content.return_value = ['some', 'content']
|
||||
file_mock = mock.Mock()
|
||||
open_mock.return_value.__enter__.return_value = file_mock
|
||||
file_mock.read.return_value = None
|
||||
hexdigest_mock = hash_mock.return_value.hexdigest
|
||||
hexdigest_mock.return_value = image_info['os_hash_value']
|
||||
|
||||
standby._download_image(image_info)
|
||||
requests_mock.assert_called_once_with(image_info['urls'][0],
|
||||
cert=None, verify=True,
|
||||
stream=True, proxies={},
|
||||
timeout=60, auth=correct_auth)
|
||||
write = file_mock.write
|
||||
write.assert_any_call('some')
|
||||
write.assert_any_call('content')
|
||||
self.assertEqual(2, write.call_count)
|
||||
|
||||
def test_download_image_bad_basic_auth_conf_credential(self):
|
||||
self.config(image_download_connection_retry_interval=0)
|
||||
image_info = _build_fake_image_info()
|
||||
CONF.set_override('image_server_auth_strategy', 'http_basic')
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._download_image,
|
||||
image_info)
|
||||
|
||||
def test_download_image_bad_basic_auth_image_info_credential(self):
|
||||
self.config(image_download_connection_retry_interval=0)
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['image_server_auth_strategy'] = 'http_basic'
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._download_image,
|
||||
image_info)
|
||||
|
||||
def test_download_image_bad_basic_auth_mixed_credential(self):
|
||||
self.config(image_download_connection_retry_interval=0)
|
||||
image_info = _build_fake_image_info()
|
||||
image_info['image_server_auth_strategy'] = 'http_basic'
|
||||
CONF.set_override('image_server_password', 'SpongeBob')
|
||||
CONF.set_override('image_server_user', 'SquarePants')
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._download_image,
|
||||
image_info)
|
||||
|
||||
def test_download_image_bad_basic_auth_mixed_credential_second(self):
|
||||
self.config(image_download_connection_retry_interval=0)
|
||||
image_info = _build_fake_image_info()
|
||||
CONF.set_override('image_server_auth_strategy', 'http_basic')
|
||||
image_info['image_server_password'] = 'SpongeBob'
|
||||
image_info['image_server_user'] = 'SquarePants'
|
||||
self.assertRaises(errors.ImageDownloadError,
|
||||
standby._download_image,
|
||||
image_info)
|
||||
|
||||
@mock.patch('hashlib.new', autospec=True)
|
||||
@mock.patch('builtins.open', autospec=True)
|
||||
@mock.patch('requests.get', autospec=True)
|
||||
|
@ -0,0 +1,27 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
Introducing basic authentication and configurable authentication strategy
|
||||
support for image and image checksum download processes. This feature
|
||||
introduces 3 new variables that could be set (either via oslo.config or
|
||||
image_info) to select the authentication strategy an provide credentials
|
||||
for HTTP(S) basic authentication. The 3 variables are structured in way
|
||||
that 1 of them 'image_server_auth_strategy' (string) provides the ability
|
||||
to select between authentication strategies by specifying the name of
|
||||
the strategy. Currently the only supported authentication strategy is the
|
||||
'http-basic' which will make IPA use HTTP(S) basic authentication also
|
||||
known as the 'RFC 7617' standard. The other 2 variables
|
||||
'image_server_password' and 'image_server_user' provide username and
|
||||
password credentials for image download processes. The
|
||||
'image_server_password' and 'image_server_user' are not strategy specific
|
||||
and could be reused for any username + password based authentication
|
||||
strategy, but for the moment these 2 variables are only used for the
|
||||
'http-basic' strategy. 'image_server_basic_auth' not just enables the
|
||||
feature but enforces checks on the values of the 2 related credentials.
|
||||
When the 'http-basic' strategy is enabled for image server download
|
||||
workflow the download logic will make sure to raise an exception in case
|
||||
any of the credentials are None or an empty string. Values coming from
|
||||
'image_info' are prioritized over values coming from the 'oslo.config'
|
||||
framework and the 2 different credential source can't be mixed. Passing 1
|
||||
or 2 out of the 3 from and source and the remaining values from an other
|
||||
source will result in a exception.
|
Loading…
Reference in New Issue
Block a user