Fix unbound error in federation _sign_assertion

The _sign_assertion() function from keystone.contrib.federation.idp
assigns file_path variable in the try block and then tries to clean
up the file in the finally block.

However, if fileutils.write_to_tempfile call raises then file_path
is not assigned that results in UnboundLocalError.

Fix it by explicitly set file_path to None before calling
fileutils.write_to_tempfile.

Closes-Bug: #1481274
Change-Id: Ib835c0980c20ceb791f1bed314a94e63c9cba910
This commit is contained in:
Roman Bogorodskiy 2015-08-01 18:55:29 +03:00
parent 7b0a401ca5
commit defa9eeb51
2 changed files with 23 additions and 1 deletions

View File

@ -414,6 +414,7 @@ def _sign_assertion(assertion):
command_list = [xmlsec_binary, '--sign', '--privkey-pem', certificates,
'--id-attr:ID', 'Assertion']
file_path = None
try:
# NOTE(gyee): need to make the namespace prefixes explicit so
# they won't get reassigned when we wrap the assertion into
@ -433,7 +434,8 @@ def _sign_assertion(assertion):
raise exception.SAMLSigningError(reason=e)
finally:
try:
os.remove(file_path)
if file_path:
os.remove(file_path)
except OSError:
pass

View File

@ -3424,6 +3424,26 @@ class SAMLGenerationTests(FederationTests):
project_domain_attribute = assertion[4][4]
self.assertIsInstance(project_domain_attribute[0].text, str)
@mock.patch('saml2.create_class_from_xml_string')
@mock.patch('oslo_utils.fileutils.write_to_tempfile')
@mock.patch('subprocess.check_output')
def test__sign_assertion(self, check_output_mock,
write_to_tempfile_mock, create_class_mock):
write_to_tempfile_mock.return_value = 'tmp_path'
check_output_mock.return_value = 'fakeoutput'
keystone_idp._sign_assertion(self.signed_assertion)
create_class_mock.assert_called_with(saml.Assertion, 'fakeoutput')
@mock.patch('oslo_utils.fileutils.write_to_tempfile')
def test__sign_assertion_fileutils_exc(self, write_to_tempfile_mock):
write_to_tempfile_mock.side_effect = Exception('fake')
self.assertRaises(exception.SAMLSigningError,
keystone_idp._sign_assertion,
self.signed_assertion)
class IdPMetadataGenerationTests(FederationTests):
"""A class for testing Identity Provider Metadata generation."""