auth: Change SAML login url scheme, enabling multiple IdP support.

The url scheme is now /accounts/login/social/saml/{idp_name} to initiate
login using the IdP configured under "idp_name" name.

display_name and display_logo (the name and icon to show on the "Log in
with" button) can be customized by adding the apprioprate settings in
the configured IdP dictionaries.

zerver/tests/test_auth_backends.py

@@ -70,6 +70,7 @@ from social_django.strategy import DjangoStrategy
 from social_django.storage import BaseDjangoStorage
 
 import base64
+import copy
 import json
 import urllib
 import ujson
@@ -958,8 +959,8 @@ class SAMLAuthBackendTest(SocialAuthBase):
     __unittest_skip__ = False
 
     BACKEND_CLASS = SAMLAuthBackend
-    LOGIN_URL = "/accounts/login/social/saml"
+    LOGIN_URL = "/accounts/login/social/saml/test_idp"
-    SIGNUP_URL = "/accounts/register/social/saml"
+    SIGNUP_URL = "/accounts/register/social/saml/test_idp"
     AUTHORIZATION_URL = "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO"
     AUTH_FINISH_URL = "/complete/saml/"
     CONFIG_ERROR_URL = "/config-error/saml"
@@ -1153,16 +1154,61 @@ class SAMLAuthBackendTest(SocialAuthBase):
         should lead to misconfiguration page.
         """
 
-        with self.settings(SOCIAL_AUTH_SAML_ENABLED_IDPS={"test_idp1": {}, "test_idp2": {}}):
+        # Setup a new SOCIAL_AUTH_SAML_ENABLED_IDPS dict with two idps.
-            # We don't need to put full idp configurations in the mock settings above
+        # We deepcopy() dictionaries around for the sake of brevity,
-            # to trigger the error.
+        # to avoid having to spell them out explicitly here.
-            with mock.patch("zerver.views.auth.logging.error") as mock_error:
+        # The second idp's configuration is a copy of the first one,
-                result = self.client_get("/accounts/login/social/saml")
+        # with name test_idp2 and altered url.
-                self.assertEqual(result.status_code, 302)
+        idps_dict = copy.deepcopy(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS)
-                self.assertEqual(result.url, '/config-error/saml')
+        idps_dict['test_idp2'] = copy.deepcopy(idps_dict['test_idp'])
-                mock_error.assert_called_once_with(
+        idps_dict['test_idp2']['url'] = 'https://idp2.example.com/idp/profile/SAML2/Redirect/SSO'
-                    "SAML misconfigured - you have specified multiple IdPs. Only one IdP is supported."
+        idps_dict['test_idp2']['display_name'] = 'Second Test IdP'
-                )
+
+        # Run tests with multiple idps configured:
+        with self.settings(SOCIAL_AUTH_SAML_ENABLED_IDPS=idps_dict):
+            # Go to the login page and check that buttons to log in show up for both IdPs:
+            result = self.client_get('/accounts/login/')
+            self.assert_in_success_response(["Log in with Test IdP"], result)
+            self.assert_in_success_response(["/accounts/login/social/saml/test_idp"], result)
+            self.assert_in_success_response(["Log in with Second Test IdP"], result)
+            self.assert_in_success_response(["/accounts/login/social/saml/test_idp2"], result)
+
+            # Try succesful authentication with the regular idp from all previous tests:
+            self.test_social_auth_success()
+
+            # Now test with the second idp:
+            original_LOGIN_URL = self.LOGIN_URL
+            original_SIGNUP_URL = self.SIGNUP_URL
+            original_AUTHORIZATION_URL = self.AUTHORIZATION_URL
+            self.LOGIN_URL = "/accounts/login/social/saml/test_idp2"
+            self.SIGNUP_URL = "/accounts/register/social/saml/test_idp2"
+            self.AUTHORIZATION_URL = idps_dict['test_idp2']['url']
+
+            try:
+                self.test_social_auth_success()
+            finally:
+                # Restore original values at the end, regardless of what happens
+                # in the block above, to avoid affecting other tests in unpredictable
+                # ways.
+                self.LOGIN_URL = original_LOGIN_URL
+                self.SIGNUP_URL = original_SIGNUP_URL
+                self.AUTHORIZATION_URL = original_AUTHORIZATION_URL
+
+    def test_social_auth_saml_login_bad_idp_arg(self) -> None:
+        for action in ['login', 'register']:
+            result = self.client_get('/accounts/{}/social/saml'.format(action))
+            # Missing idp argument.
+            self.assertEqual(result.status_code, 302)
+            self.assertEqual(result.url, '/config-error/saml')
+
+            result = self.client_get('/accounts/{}/social/saml/nonexistent_idp'.format(action))
+            # No such IdP is configured.
+            self.assertEqual(result.status_code, 302)
+            self.assertEqual(result.url, '/config-error/saml')
+
+            result = self.client_get('/accounts/{}/social/saml/'.format(action))
+            # No matching url pattern.
+            self.assertEqual(result.status_code, 404)
 
 class GitHubAuthBackendTest(SocialAuthBase):
     __unittest_skip__ = False

zerver/views/auth.py

@@ -350,7 +350,8 @@ def oauth_redirect_to_root(request: HttpRequest, url: str,
 
     return redirect(main_site_uri + '?' + urllib.parse.urlencode(params))
 
-def start_social_login(request: HttpRequest, backend: str) -> HttpResponse:
+def start_social_login(request: HttpRequest, backend: str, extra_arg: Optional[str]=None
+                       ) -> HttpResponse:
     backend_url = reverse('social:begin', args=[backend])
     extra_url_params = {}  # type: Dict[str, str]
     if backend == "saml":
@@ -366,16 +367,11 @@ def start_social_login(request: HttpRequest, backend: str) -> HttpResponse:
 
         # This backend requires the name of the IdP (from the list of configured ones)
         # to be passed as the parameter.
-        # Currently we support configuring only one IdP.
+        if not extra_arg or extra_arg not in settings.SOCIAL_AUTH_SAML_ENABLED_IDPS:
-        # TODO: Support multiple IdPs. python-social-auth SAML (which we use here)
+            logging.info("Attempted to initiate SAML authentication with wrong idp argument: {}"
-        # already supports that, so essentially only the UI for it on the login pages
+                         .format(extra_arg))
-        # needs to be figured out.
-        if len(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS) != 1:
-            logging.error(
-                "SAML misconfigured - you have specified multiple IdPs. Only one IdP is supported."
-            )
             return redirect_to_config_error("saml")
-        extra_url_params = {'idp': list(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS.keys())[0]}
+        extra_url_params = {'idp': extra_arg}
     if (backend == "github") and not (settings.SOCIAL_AUTH_GITHUB_KEY and
                                       settings.SOCIAL_AUTH_GITHUB_SECRET):
         return redirect_to_config_error("github")
@@ -386,12 +382,16 @@ def start_social_login(request: HttpRequest, backend: str) -> HttpResponse:
 
     return oauth_redirect_to_root(request, backend_url, 'social', extra_url_params=extra_url_params)
 
-def start_social_signup(request: HttpRequest, backend: str) -> HttpResponse:
+def start_social_signup(request: HttpRequest, backend: str, extra_arg: Optional[str]=None
+                        ) -> HttpResponse:
     backend_url = reverse('social:begin', args=[backend])
     extra_url_params = {}  # type: Dict[str, str]
     if backend == "saml":
-        assert len(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS) == 1
+        if not extra_arg or extra_arg not in settings.SOCIAL_AUTH_SAML_ENABLED_IDPS:
-        extra_url_params = {'idp': list(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS.keys())[0]}
+            logging.info("Attempted to initiate SAML authentication with wrong idp argument: {}"
+                         .format(extra_arg))
+            return redirect_to_config_error("saml")
+        extra_url_params = {'idp': extra_arg}
     return oauth_redirect_to_root(request, backend_url, 'social', is_signup=True,
                                   extra_url_params=extra_url_params)
 

zproject/backends.py

@@ -1253,11 +1253,29 @@ def create_standard_social_backend_dict(social_backend: SocialAuthMixin) -> Soci
         sort_order=social_backend.sort_order
     )
 
+def list_saml_backend_dicts(realm: Optional[Realm]=None) -> List[SocialBackendDictT]:
+    result = []  # type: List[SocialBackendDictT]
+    for idp_name, idp_dict in settings.SOCIAL_AUTH_SAML_ENABLED_IDPS.items():
+        saml_dict = dict(
+            name='saml:{}'.format(idp_name),
+            display_name=idp_dict.get('display_name', SAMLAuthBackend.auth_backend_name),
+            display_logo=idp_dict.get('display_logo', SAMLAuthBackend.display_logo),
+            login_url=reverse('login-social-extra-arg', args=('saml', idp_name)),
+            signup_url=reverse('signup-social-extra-arg', args=('saml', idp_name)),
+            sort_order=SAMLAuthBackend.sort_order - len(result)
+        )  # type: SocialBackendDictT
+        result.append(saml_dict)
+
+    return result
+
 def get_social_backend_dicts(realm: Optional[Realm]=None) -> List[SocialBackendDictT]:
     result = []
     for backend in SOCIAL_AUTH_BACKENDS:
         if auth_enabled_helper([backend.auth_backend_name], realm):
-            result.append(create_standard_social_backend_dict(backend))
+            if backend != SAMLAuthBackend:
+                result.append(create_standard_social_backend_dict(backend))
+            else:
+                result += list_saml_backend_dicts(realm)
 
     return sorted(result, key=lambda x: x['sort_order'], reverse=True)
 

zproject/prod_settings_template.py

@@ -223,6 +223,11 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS = {
         "attr_email": "email",
         # The "x509cert" attribute is automatically read from
         # /etc/zulip/saml/idps/{idp_name}.crt; don't specify it here.
+
+        # Optionally, you can edit display_name and display_logo settings below
+        # to change the name and icon that will show on the login button.
+        "display_name": "SAML",
+        "display_logo": "/static/images/landing-page/logos/saml-icon.png"
     }
 }
 

zproject/test_settings.py

@@ -211,5 +211,7 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS = {
         "attr_last_name": "last_name",
         "attr_username": "email",
         "attr_email": "email",
+        "display_name": "Test IdP",
+        "display_logo": "/static/images/landing-page/logos/saml-icon.png",
     }
 }

zproject/urls.py

@@ -423,6 +423,8 @@ i18n_urls = [
     url(r'^accounts/login/jwt/$', zerver.views.auth.remote_user_jwt, name='login-jwt'),
     url(r'^accounts/login/social/([\w,-]+)$', zerver.views.auth.start_social_login,
         name='login-social'),
+    url(r'^accounts/login/social/([\w,-]+)/([\w,-]+)$', zerver.views.auth.start_social_login,
+        name='login-social-extra-arg'),
     # Backwards-compatibility (legacy) Google auth URL for the mobile
     # apps; see https://github.com/zulip/zulip/issues/13081 for
     # background.  We can remove this once older versions of the
@@ -433,6 +435,9 @@ i18n_urls = [
     url(r'^accounts/register/social/([\w,-]+)$',
         zerver.views.auth.start_social_signup,
         name='signup-social'),
+    url(r'^accounts/register/social/([\w,-]+)/([\w,-]+)$',
+        zerver.views.auth.start_social_signup,
+        name='signup-social-extra-arg'),
     url(r'^accounts/login/subdomain/([^/]+)$', zerver.views.auth.log_into_subdomain,
         name='zerver.views.auth.log_into_subdomain'),
     url(r'^accounts/login/local/$', zerver.views.auth.dev_direct_login,