From ab688e63eae0338f21c8aa1147eecd3cd36b46ff Mon Sep 17 00:00:00 2001
From: Omar Khan <omar@opencraft.com>
Date: Thu, 14 Jan 2016 23:28:52 +0700
Subject: [PATCH] Redirect to login when SAML accessed without idp param

The python social auth SAML page returns a 500 response when accessed
without the 'idp' query param. It should redirect to the login page if
the param is missing.

SOL-1550
---
 common/djangoapps/third_party_auth/saml.py       | 16 +++++++++++++++-
 .../third_party_auth/tests/test_views.py         | 14 ++++++++++++++
 requirements/edx/base.txt                        |  1 +
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/common/djangoapps/third_party_auth/saml.py b/common/djangoapps/third_party_auth/saml.py
index db40104b14..61f7e0d8f5 100644
--- a/common/djangoapps/third_party_auth/saml.py
+++ b/common/djangoapps/third_party_auth/saml.py
@@ -3,7 +3,7 @@ Slightly customized python-social-auth backend for SAML 2.0 support
 """
 import logging
 from social.backends.saml import SAMLAuth, OID_EDU_PERSON_ENTITLEMENT
-from social.exceptions import AuthForbidden
+from social.exceptions import AuthForbidden, AuthMissingParameter
 
 log = logging.getLogger(__name__)
 
@@ -33,6 +33,20 @@ class SAMLAuthBackend(SAMLAuth):  # pylint: disable=abstract-method
         except KeyError:
             return self.strategy.setting(name, default)
 
+    def auth_url(self):
+        """
+        Check that the request includes an 'idp' parameter before getting the
+        URL to which we must redirect in order to authenticate the user.
+
+        raise AuthMissingParameter if the 'idp' parameter is missing.
+
+        TODO: remove this method once the fix is merged upstream:
+        https://github.com/omab/python-social-auth/pull/821
+        """
+        if 'idp' not in self.strategy.request_data():
+            raise AuthMissingParameter(self, 'idp')
+        return super(SAMLAuthBackend, self).auth_url()
+
     def _check_entitlements(self, idp, attributes):
         """
         Check if we require the presence of any specific eduPersonEntitlement.
diff --git a/common/djangoapps/third_party_auth/tests/test_views.py b/common/djangoapps/third_party_auth/tests/test_views.py
index da57c49fc8..538c82eb46 100644
--- a/common/djangoapps/third_party_auth/tests/test_views.py
+++ b/common/djangoapps/third_party_auth/tests/test_views.py
@@ -129,3 +129,17 @@ class SAMLMetadataTest(SAMLTestCase):
         self.assertEqual(support_name_node.text, support_name)
         support_email_node = support_node.find(etree.QName(SAML_XML_NS, 'EmailAddress'))
         self.assertEqual(support_email_node.text, support_email)
+
+
+@unittest.skipUnless(AUTH_FEATURE_ENABLED, 'third_party_auth not enabled')
+class SAMLAuthTest(SAMLTestCase):
+    """
+    Test the SAML auth views
+    """
+    LOGIN_URL = '/auth/login/tpa-saml/'
+
+    def test_login_without_idp(self):
+        """ Accessing the login endpoint without an idp query param should return 302 """
+        self.enable_saml()
+        response = self.client.get(self.LOGIN_URL)
+        self.assertEqual(response.status_code, 302)
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index 72c7b84e78..ff6bb9e8b0 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -68,6 +68,7 @@ python-dateutil==2.1
 
 # This module gets monkey-patched in third_party_auth.py to fix a Django 1.8 incompatibility.
 # When this dependency gets upgraded, the monkey patch should be removed, if possible.
+# We can also remove the fix to auth_url in third_party_auth/saml.py when that fix is included upstream.
 python-social-auth==0.2.12
 
 pytz==2015.2