Merge "Add logging for xmlsec1 installation"
This commit is contained in:
commit
d4f3160334
@ -382,6 +382,38 @@ class SAMLGenerator(object):
|
||||
return signature
|
||||
|
||||
|
||||
def _verify_assertion_binary_is_installed():
|
||||
"""Make sure the specified xmlsec binary is installed.
|
||||
|
||||
If the binary specified in configuration isn't installed, make sure we
|
||||
leave some sort of useful error message for operators since the absense of
|
||||
it is going to throw an HTTP 500.
|
||||
|
||||
"""
|
||||
try:
|
||||
# `check_output` just returns the output of whatever is passed in
|
||||
# (hence the name). We don't really care about where the location of
|
||||
# the binary exists, though. We just want to make sure it's actually
|
||||
# installed and if an `CalledProcessError` isn't thrown, it is.
|
||||
subprocess.check_output( # nosec : The contents of this command are
|
||||
# coming from either the default
|
||||
# configuration value for
|
||||
# CONF.saml.xmlsec1_binary or an operator
|
||||
# supplied location for that binary. In
|
||||
# either case, it is safe to assume this
|
||||
# input is coming from a trusted source and
|
||||
# not a possible attacker (over the API).
|
||||
['/usr/bin/which', CONF.saml.xmlsec1_binary]
|
||||
)
|
||||
except subprocess.CalledProcessError:
|
||||
msg = (
|
||||
'Unable to locate %(binary)s binary on the system. Check to make '
|
||||
'sure it is installed.' % {'binary': CONF.saml.xmlsec1_binary}
|
||||
)
|
||||
LOG.error(msg)
|
||||
raise exception.SAMLSigningError(reason=msg)
|
||||
|
||||
|
||||
def _sign_assertion(assertion):
|
||||
"""Sign a SAML assertion.
|
||||
|
||||
@ -416,6 +448,13 @@ def _sign_assertion(assertion):
|
||||
'idp_private_key': CONF.saml.keyfile,
|
||||
}
|
||||
|
||||
# Verify that the binary used to create the assertion actually exists on
|
||||
# the system. If it doesn't, log a warning for operators to go and install
|
||||
# it. Requests for assertions will fail with HTTP 500s until the package is
|
||||
# installed, so providing something useful in the logs is about the best we
|
||||
# can do.
|
||||
_verify_assertion_binary_is_installed()
|
||||
|
||||
command_list = [
|
||||
CONF.saml.xmlsec1_binary, '--sign', '--privkey-pem', certificates,
|
||||
'--id-attr:ID', 'Assertion']
|
||||
|
@ -3740,6 +3740,7 @@ class SAMLGenerationTests(test_v3.RestfulTestCase):
|
||||
def test_assertion_using_explicit_namespace_prefixes(self):
|
||||
def mocked_subprocess_check_output(*popenargs, **kwargs):
|
||||
# the last option is the assertion file to be signed
|
||||
if popenargs[0] != ['/usr/bin/which', CONF.saml.xmlsec1_binary]:
|
||||
filename = popenargs[0][-1]
|
||||
with open(filename, 'r') as f:
|
||||
assertion_content = f.read()
|
||||
@ -4026,37 +4027,46 @@ class SAMLGenerationTests(test_v3.RestfulTestCase):
|
||||
create_class_mock.assert_called_with(saml.Assertion, 'fakeoutput')
|
||||
|
||||
@mock.patch('oslo_utils.fileutils.write_to_tempfile')
|
||||
@mock.patch.object(subprocess, 'check_output')
|
||||
def test_sign_assertion_exc(self, check_output_mock,
|
||||
write_to_tempfile_mock):
|
||||
def test_sign_assertion_exc(self, write_to_tempfile_mock):
|
||||
# If the command fails the command output is logged.
|
||||
|
||||
write_to_tempfile_mock.return_value = 'tmp_path'
|
||||
|
||||
sample_returncode = 1
|
||||
sample_output = self.getUniqueString()
|
||||
check_output_mock.side_effect = subprocess.CalledProcessError(
|
||||
returncode=sample_returncode, cmd=CONF.saml.xmlsec1_binary,
|
||||
output=sample_output)
|
||||
write_to_tempfile_mock.return_value = 'tmp_path'
|
||||
|
||||
def side_effect(*args, **kwargs):
|
||||
if args[0] == ['/usr/bin/which', CONF.saml.xmlsec1_binary]:
|
||||
return '/usr/bin/xmlsec1\n'
|
||||
else:
|
||||
raise subprocess.CalledProcessError(
|
||||
returncode=sample_returncode, cmd=CONF.saml.xmlsec1_binary,
|
||||
output=sample_output
|
||||
)
|
||||
|
||||
with mock.patch.object(subprocess, 'check_output',
|
||||
side_effect=side_effect):
|
||||
logger_fixture = self.useFixture(fixtures.LoggerFixture())
|
||||
self.assertRaises(exception.SAMLSigningError,
|
||||
self.assertRaises(
|
||||
exception.SAMLSigningError,
|
||||
keystone_idp._sign_assertion,
|
||||
self.signed_assertion)
|
||||
# The function __str__ in subprocess.CalledProcessError is different
|
||||
# between py3.6 and lower python version.
|
||||
self.signed_assertion
|
||||
)
|
||||
|
||||
# The function __str__ in subprocess.CalledProcessError is
|
||||
# different between py3.6 and lower python version.
|
||||
expected_log = (
|
||||
"Error when signing assertion, reason: Command '%s' returned "
|
||||
"non-zero exit status %s\.? %s\n" %
|
||||
(CONF.saml.xmlsec1_binary, sample_returncode, sample_output))
|
||||
|
||||
self.assertRegex(logger_fixture.output,
|
||||
re.compile(r'%s' % expected_log))
|
||||
|
||||
@mock.patch('oslo_utils.fileutils.write_to_tempfile')
|
||||
def test_sign_assertion_fileutils_exc(self, write_to_tempfile_mock):
|
||||
@mock.patch.object(subprocess, 'check_output')
|
||||
def test_sign_assertion_fileutils_exc(self, check_output_mock,
|
||||
write_to_tempfile_mock):
|
||||
exception_msg = 'fake'
|
||||
write_to_tempfile_mock.side_effect = Exception(exception_msg)
|
||||
check_output_mock.return_value = '/usr/bin/xmlsec1'
|
||||
|
||||
logger_fixture = self.useFixture(fixtures.LoggerFixture())
|
||||
self.assertRaises(exception.SAMLSigningError,
|
||||
@ -4066,6 +4076,22 @@ class SAMLGenerationTests(test_v3.RestfulTestCase):
|
||||
'Error when signing assertion, reason: %s\n' % exception_msg)
|
||||
self.assertEqual(expected_log, logger_fixture.output)
|
||||
|
||||
def test_sign_assertion_logs_message_if_xmlsec1_is_not_installed(self):
|
||||
with mock.patch.object(subprocess, 'check_output') as co_mock:
|
||||
co_mock.side_effect = subprocess.CalledProcessError(
|
||||
returncode=1, cmd=CONF.saml.xmlsec1_binary,
|
||||
)
|
||||
logger_fixture = self.useFixture(fixtures.LoggerFixture())
|
||||
self.assertRaises(
|
||||
exception.SAMLSigningError,
|
||||
keystone_idp._sign_assertion,
|
||||
self.signed_assertion
|
||||
)
|
||||
|
||||
expected_log = ('Unable to locate xmlsec1 binary on the system. '
|
||||
'Check to make sure it is installed.\n')
|
||||
self.assertEqual(expected_log, logger_fixture.output)
|
||||
|
||||
|
||||
class IdPMetadataGenerationTests(test_v3.RestfulTestCase):
|
||||
"""A class for testing Identity Provider Metadata generation."""
|
||||
|
Loading…
Reference in New Issue
Block a user