From acdf034eb189773494849c4b7798f4bf59ea8517 Mon Sep 17 00:00:00 2001
From: guang-yee <guang.yee@hp.com>
Date: Mon, 5 Jan 2015 23:50:48 -0800
Subject: [PATCH] explicit namespace prefixes for SAML2 assertion

Make sure the namespace prefixes for the signed SAML2 assertion are explicitly
defined so they don't get reassigned when we wrap it inside the response.

This patch also fix Python2.6 support by replacing subprocess.check_output()
with subprocess.Popen().

Change-Id: I8751dfbec502e0c71d075f31e6067b2ed65e3851
Closes-Bug: #1402916
(cherry picked from commit abb5f0022f117468016f2736206139830e782d0a)
---
 keystone/contrib/federation/idp.py   | 25 +++++++++++++++-------
 keystone/tests/test_v3_federation.py | 31 ++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/keystone/contrib/federation/idp.py b/keystone/contrib/federation/idp.py
index c9c20c9102..49d6b67ed6 100644
--- a/keystone/contrib/federation/idp.py
+++ b/keystone/contrib/federation/idp.py
@@ -398,14 +398,25 @@ def _sign_assertion(assertion):
                     '--id-attr:ID', 'Assertion']
 
     try:
-        file_path = fileutils.write_to_tempfile(assertion.to_string())
+        # NOTE(gyee): need to make the namespace prefixes explicit so
+        # they won't get reassigned when we wrap the assertion into
+        # SAML2 response
+        file_path = fileutils.write_to_tempfile(assertion.to_string(
+            nspair={'saml': saml2.NAMESPACE,
+                    'xmldsig': xmldsig.NAMESPACE}))
         command_list.append(file_path)
-        stdout = subprocess.check_output(command_list)
-    except Exception as e:
-        msg = _LE('Error when signing assertion, reason: %(reason)s')
-        msg = msg % {'reason': e}
-        LOG.error(msg)
-        raise exception.SAMLSigningError(reason=e)
+        process = subprocess.Popen(command_list,
+                                   stdin=subprocess.PIPE,
+                                   stdout=subprocess.PIPE,
+                                   stderr=subprocess.PIPE,
+                                   close_fds=True)
+        stdout, stderr = process.communicate()
+        retcode = process.poll()
+        if retcode:
+            msg = _LE('Error when signing assertion, reason: %(reason)s')
+            msg = msg % {'reason': stderr}
+            LOG.error(msg)
+            raise exception.SAMLSigningError(reason=stderr)
     finally:
         try:
             os.remove(file_path)
diff --git a/keystone/tests/test_v3_federation.py b/keystone/tests/test_v3_federation.py
index ae3f48dea8..7f593518be 100644
--- a/keystone/tests/test_v3_federation.py
+++ b/keystone/tests/test_v3_federation.py
@@ -1754,6 +1754,37 @@ class SAMLGenerationTests(FederationTests):
         project_attribute = assertion[4][2]
         self.assertEqual(self.PROJECT, project_attribute[0].text)
 
+    def test_assertion_using_explicit_namespace_prefixes(self):
+        class MockedPopen(object):
+            def __init__(self, *popenargs, **kwargs):
+                # the last option is the assertion file to be signed
+                filename = popenargs[0][-1]
+                with open(filename, 'r') as f:
+                    self.stdout = f.read()
+
+            def communicate(self, *args, **kwargs):
+                # since we are not testing the signature itself, we can return
+                # the assertion as is without signing it
+                return (self.stdout, None)
+
+            def poll(self):
+                return 0
+
+        with mock.patch('subprocess.Popen',
+                        side_effect=MockedPopen):
+            generator = keystone_idp.SAMLGenerator()
+            response = generator.samlize_token(self.ISSUER, self.RECIPIENT,
+                                               self.SUBJECT, self.ROLES,
+                                               self.PROJECT)
+            assertion_xml = response.assertion.to_string()
+            # make sure we have the proper tag and prefix for the assertion
+            # namespace
+            self.assertIn('<saml:Assertion', assertion_xml)
+            self.assertIn('xmlns:saml="' + saml2.NAMESPACE + '"',
+                          assertion_xml)
+            self.assertIn('xmlns:xmldsig="' + xmldsig.NAMESPACE + '"',
+                          assertion_xml)
+
     def test_saml_signing(self):
         """Test that the SAML generator produces a SAML object.