Merge lp:~roadmr/canonical-identity-provider/pass-custom-cert-to-django-saml2-idp into lp:canonical-identity-provider/release

[Daniel Manrique]

Use the SAML remote's configured certificate, if present.

This allows setting a custom certificate per RP. RPs for which this
field is empty fall back to the global certificate configured in settings.

All certificates must be generated from the global private key in settings,
which is a single setting for all RPs.

Use the SAML remote's configured certificate, if present.

This allows setting a custom certificate per RP. RPs for which this
field is empty fall back to the global certificate configured in settings.

All certificates must be generated from the global private key in settings,
which is a single setting for all RPs.

Note in addition to the prerequisite branch, this requires django-saml2-idp's changes from here:

https://code.launchpad.net/~roadmr/django-saml2-idp/custom-certificate-support

So once that is landed, this branch will need to be updated to point to the django-saml2-idp revision where those changes land. Tests won't pass without that :)

[Daniel Manrique (roadmr)]

See responses, and push with updates is coming up

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

[Daniel Manrique (roadmr)]

Thanks!

=== modified file 'config-manager.txt'
--- config-manager.txt	2017-11-07 14:38:03 +0000
+++ config-manager.txt	2017-12-11 22:58:20 +0000
@@ -5,7 +5,7 @@
 branches/django-openid-auth    lp:~ubuntuone-pqm-team/django-openid-auth/stable;revno=107
 branches/django-pgtools        lp:django-pgtools;revno=8
 branches/django-piston         lp:~ubuntuone-pqm-team/django-piston/stable;revno=7
-branches/django-saml2-idp      lp:~ubuntuone-pqm-team/django-saml2-idp/stable;revno=69
+branches/django-saml2-idp      lp:~ubuntuone-pqm-team/django-saml2-idp/stable;revno=70
 branches/ols-tests             lp:ols-tests;tag=ols-tests-1.0.0
 branches/ols-tests-django      lp:ols-tests-django;revno=19
 branches/python-openid         lp:~ubuntuone-pqm-team/python-openid/stable;revno=1989

=== added file 'src/identityprovider/tests/cert/custom-test-certificate.pem'
--- src/identityprovider/tests/cert/custom-test-certificate.pem	1970-01-01 00:00:00 +0000
+++ src/identityprovider/tests/cert/custom-test-certificate.pem	2017-12-11 22:58:20 +0000
@@ -0,0 +1,14 @@
+-----BEGIN CERTIFICATE-----
+MIICFzCCAcGgAwIBAgIJAIsuoCD5UgkTMA0GCSqGSIb3DQEBCwUAMGcxCzAJBgNV
+BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX
+aWRnaXRzIFB0eSBMdGQxIDAeBgNVBAMMF2N1c3RvbSB0ZXN0IGNlcnRpZmljYXRl
+MB4XDTE3MTIwNzIzMjYxOFoXDTE4MTIwNzIzMjYxOFowZzELMAkGA1UEBhMCQVUx
+EzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMg
+UHR5IEx0ZDEgMB4GA1UEAwwXY3VzdG9tIHRlc3QgY2VydGlmaWNhdGUwXDANBgkq
+hkiG9w0BAQEFAANLADBIAkEAyISI4ElrIygKGduoi46g6EMjz9kq/u0FRRjHzKqd
+sOUB0CRzC6h9yhEJ3rKcgXjlnTjTrQUoNpinyw3GZmI+hwIDAQABo1AwTjAdBgNV
+HQ4EFgQU0BwTxn4CBWxoCDuj5jUW+M1g4NMwHwYDVR0jBBgwFoAU0BwTxn4CBWxo
+CDuj5jUW+M1g4NMwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAAgS9Hz7
+iPLP/a4AcOwLsz2YHIYjpMLovSXbsgjT61afsFLj2HS2oh1ucZFyfw1L8BGDa4Qx
+myf5qJ6bRVusA4c=
+-----END CERTIFICATE-----

=== modified file 'src/ubuntu_sso_saml/processors.py'
--- src/ubuntu_sso_saml/processors.py	2017-12-04 16:21:42 +0000
+++ src/ubuntu_sso_saml/processors.py	2017-12-11 22:58:20 +0000
@@ -10,7 +10,10 @@
 from saml2idp.exceptions import CannotHandleAssertion
 
 from identityprovider.models import OpenIDRPConfig
-from ubuntu_sso_saml.utils import get_config_for_acs
+from ubuntu_sso_saml.utils import (
+    certificate_from_sp_config,
+    get_config_for_acs
+)
 
 
 __all__ = [
@@ -321,11 +324,12 @@
                 attrs = json.loads(sp_config.attributes)
             else:
                 attrs = {}
+            certificate = certificate_from_sp_config(sp_config)
             self._assertion_params['ATTRIBUTES'] = self._eval_attributes(attrs)
             if sp_config.audience:
-                self._format_assertion_restricted()
+                self._format_assertion_restricted(certificate=certificate)
             else:
-                self._format_assertion_unrestricted()
+                self._format_assertion_unrestricted(certificate=certificate)
 
     def _validate_request(self):
         acs_url = self._request_params['ACS_URL']
@@ -366,13 +370,20 @@
     def _decode_request_compressed(self):
         return codex.decode_base64_and_inflate(self._saml_request)
 
-    def _format_assertion_unrestricted(self):
+    def _format_assertion_unrestricted(self, certificate=None):
         self._assertion_xml = xml_render.get_assertion_googleapps_xml(
-            self._assertion_params, signed=True)
+            self._assertion_params, signed=True, certificate=certificate)
 
-    def _format_assertion_restricted(self):
+    def _format_assertion_restricted(self, certificate=None):
         self._assertion_xml = xml_render.get_assertion_salesforce_xml(
-            self._assertion_params, signed=True)
+            self._assertion_params, signed=True, certificate=certificate)
+
+    def _format_response(self):
+        sp_config = self.get_config()
+        # Helper properly handles sp_config being None.
+        certificate = certificate_from_sp_config(sp_config)
+        super(CanonicalProcessor, self)._format_response(
+            certificate=certificate)
 
     def _eval_attributes(self, attributes):
         for key, value in attributes.items():

=== modified file 'src/ubuntu_sso_saml/tests/helpers.py'
--- src/ubuntu_sso_saml/tests/helpers.py	2017-05-11 15:22:36 +0000
+++ src/ubuntu_sso_saml/tests/helpers.py	2017-12-11 22:58:20 +0000
@@ -1,8 +1,28 @@
 from identityprovider import tests
 
 
-def find_cert_path():
+def find_cert_path(filename=None):
     """
     Find the right path to certificates
     """
-    return tests.__path__[0] + '/cert/'
+    if filename:
+        return tests.__path__[0] + '/cert/' + filename
+    else:
+        return tests.__path__[0] + '/cert/'
+
+
+def cert_string_from_file(cert_filename):
+    """
+    Returns a string with contents of a PEM-format certificate read from file.
+
+    Returns only the "meat" of the certificate, just the base64-encoded content
+    without the BEGIN/END delimiter lines, all in a single-line string.
+    """
+    with open(cert_filename) as cert_file:
+        cert_lines = cert_file.readlines()
+
+    delimiters = ["-----BEGIN CERTIFICATE-----\n",
+                  "-----END CERTIFICATE-----\n"]
+    cert_lines = [l.strip() for l in cert_lines if l not in delimiters]
+
+    return "".join(cert_lines)

=== added file 'src/ubuntu_sso_saml/tests/test_helpers.py'
--- src/ubuntu_sso_saml/tests/test_helpers.py	1970-01-01 00:00:00 +0000
+++ src/ubuntu_sso_saml/tests/test_helpers.py	2017-12-11 22:58:20 +0000
@@ -0,0 +1,27 @@
+"""Tests for helper methods."""
+from unittest import TestCase
+from ubuntu_sso_saml.tests.helpers import (
+    cert_string_from_file,
+    find_cert_path,
+)
+
+
+class CertStringHelpersTestCase(TestCase):
+
+    def setUp(self, *args, **kwargs):
+        super(CertStringHelpersTestCase, self).setUp(*args, **kwargs)
+        self.test_cert_file = find_cert_path("certificate.pem")
+
+    def test_from_file(self):
+        # Build the "meat" of the certificate for verification with
+        # a slightly different algorithm than cert_string_from_file
+        # (otherwise this test would be a tautology)
+        expected = ""
+        with open(self.test_cert_file) as cert_file:
+            for line in cert_file:
+                if "CERTIFICATE-----" not in line:
+                    expected = expected + line.strip()
+
+        cs = cert_string_from_file(self.test_cert_file)
+
+        self.assertEqual(expected, cs)

=== modified file 'src/ubuntu_sso_saml/tests/test_processors.py'
--- src/ubuntu_sso_saml/tests/test_processors.py	2017-12-04 16:21:42 +0000
+++ src/ubuntu_sso_saml/tests/test_processors.py	2017-12-11 22:58:20 +0000
@@ -35,11 +35,13 @@
 from identityprovider.tests import DEFAULT_USER_PASSWORD
 from identityprovider.tests.utils import AuthenticatedTestCase
 from ubuntu_sso_saml.models import SAMLConfig
-from ubuntu_sso_saml.tests.helpers import find_cert_path
+from ubuntu_sso_saml.tests.helpers import cert_string_from_file, find_cert_path
 
 
 SAML_TEAM = 'saml2team'  # Name of team enabled for SAML 2.0
 
+CUSTOM_TEST_CERTIFICATE = "custom-test-certificate.pem"
+
 
 def get_saml_response(response):
     tree = PyQuery(response.content)
@@ -86,9 +88,8 @@
         new_saml2idp_config = copy.deepcopy(saml2idp_metadata.SAML2IDP_CONFIG)
         self.patch(saml2idp_metadata, 'SAML2IDP_CONFIG',
                    new=new_saml2idp_config)
-        path = find_cert_path()
-        cert_file = path + 'certificate.pem'
-        priv_file = path + 'private-key.pem'
+        cert_file = find_cert_path('certificate.pem')
+        priv_file = find_cert_path('private-key.pem')
         test_config = {
             'certificate_file': cert_file,
             'private_key_file': priv_file,
@@ -1604,3 +1605,31 @@
         # I should be sent to the login page
         query_params = urllib.urlencode({'next': "/saml/process"})
         self.assertRedirects(response, reverse('login') + "?" + query_params)
+
+    def test_default_certificate_in_assertion(self):
+        self.setup_saml_sp()
+
+        data = self.get_request_data()
+        response = self.client.get('/+saml', data=data, follow=True)
+        samlresponse = get_saml_response(response)
+
+        cert_filename = saml2idp_metadata.SAML2IDP_CONFIG['certificate_file']
+
+        self.assertIn(cert_string_from_file(cert_filename), samlresponse)
+
+    def test_custom_certificate_in_assertion(self):
+        cert_filename = saml2idp_metadata.SAML2IDP_CONFIG['certificate_file']
+        custom_cert_filename = find_cert_path(CUSTOM_TEST_CERTIFICATE)
+        with open(custom_cert_filename) as custom_cert_file:
+            custom_cert_string = custom_cert_file.read()
+        # Setup the SP with the delimited certificate
+        self.setup_saml_sp(certificate=custom_cert_string)
+        # Keep the non-delimited certificate for asserting later.
+        custom_cert_string = cert_string_from_file(custom_cert_filename)
+
+        data = self.get_request_data()
+        response = self.client.get('/+saml', data=data, follow=True)
+        samlresponse = get_saml_response(response)
+        # Ensure the assertion contains the custom cert, not the default one.
+        self.assertIn(custom_cert_string, samlresponse)
+        self.assertNotIn(cert_string_from_file(cert_filename), samlresponse)

=== modified file 'src/ubuntu_sso_saml/tests/test_utils.py'
--- src/ubuntu_sso_saml/tests/test_utils.py	2017-05-12 15:39:10 +0000
+++ src/ubuntu_sso_saml/tests/test_utils.py	2017-12-11 22:58:20 +0000
@@ -3,7 +3,9 @@
 from django.test import TestCase
 
 from ubuntu_sso_saml.models import SAMLConfig
+from ubuntu_sso_saml.tests.helpers import find_cert_path
 from ubuntu_sso_saml.utils import (
+    certificate_from_sp_config,
     get_config_for_acs,
     get_config_and_link_for_resource,
     get_deeplink_url,
@@ -75,3 +77,23 @@
 
         url = get_deeplink_url(resource_name, (resource_pattern, url_pattern))
         self.assertEqual(url, 'http://example.com/foo/bar-baz')
+
+
+class CertificateFromSpConfigTestCase(TestCase):
+
+    def test_certificate_from_no_sp_config(self):
+        self.assertEqual(None, certificate_from_sp_config(None))
+
+    def test_certificate_from_sp_config_no_cert(self):
+        sp_config = SAMLConfig.objects.create(
+            acs_url='http://localhost')
+        self.assertEqual(None, certificate_from_sp_config(sp_config))
+
+    def test_certificate_from_sp_config_with_cert(self):
+        with open(find_cert_path('certificate.pem')) as cert_file:
+            cert_string = cert_file.read()
+        sp_config = SAMLConfig.objects.create(
+            acs_url='http://localhost', certificate=cert_string)
+        # Ensure it's identical to a what we read from the file, and not
+        # a djangoized unicode.
+        self.assertEqual(cert_string, certificate_from_sp_config(sp_config))

=== modified file 'src/ubuntu_sso_saml/utils.py'
--- src/ubuntu_sso_saml/utils.py	2017-05-17 13:16:46 +0000
+++ src/ubuntu_sso_saml/utils.py	2017-12-11 22:58:20 +0000
@@ -51,3 +51,19 @@
     if match is not None:
         url = url_pattern % match.groupdict()
         return url
+
+
+def certificate_from_sp_config(sp_config):
+    """
+    Return the SP's certificate (bytes) or None if there's no certificate.
+
+    Also returns None if the passed sp_config is None.
+
+    """
+    if sp_config and sp_config.certificate:
+        # Django stores string/unicode but M2Crypto needs bytes for
+        # certificate values.
+        certificate = sp_config.certificate.encode('ascii')
+    else:
+        certificate = None
+    return certificate