social_auth: Clear session fields leftover from previous auth attempts.

Fixes #13560.

zerver/tests/test_auth_backends.py

@@ -786,6 +786,28 @@ class SocialAuthBase(ZulipTestCase):
         self.assertEqual(result.status_code, 302)
         self.assert_logged_in_user_id(self.user_profile.id)
 
+    def test_social_auth_session_fields_cleared_correctly(self) -> None:
+        mobile_flow_otp = '1234abcd' * 8
+
+        def initiate_auth(mobile_flow_otp: Optional[str]=None) -> None:
+            url, headers = self.prepare_login_url_and_headers(subdomain='zulip',
+                                                              mobile_flow_otp=mobile_flow_otp)
+            result = self.client_get(url, **headers)
+            self.assertEqual(result.status_code, 302)
+
+            result = self.client_get(result.url, **headers)
+            self.assertEqual(result.status_code, 302)
+
+        # Start social auth with mobile_flow_otp param. It should get saved into the session
+        # on SOCIAL_AUTH_SUBDOMAIN.
+        initiate_auth(mobile_flow_otp)
+        self.assertEqual(self.client.session['mobile_flow_otp'], mobile_flow_otp)
+
+        # Make a request without mobile_flow_otp param and verify the field doesn't persist
+        # in the session from the previous request.
+        initiate_auth()
+        self.assertNotIn('mobile_flow_otp', self.client.session)
+
     def test_social_auth_mobile_and_desktop_flow_in_one_request_error(self) -> None:
         otp = '1234abcd' * 8
         account_data_dict = self.get_account_data_dict(email=self.email, name='Full Name')

zerver/views/auth.py

@@ -47,6 +47,7 @@ import jwt
 import logging
 
 from social_django.utils import load_backend, load_strategy
+from social_django.views import auth as social_django_auth
 
 from two_factor.forms import BackupTokenForm
 from two_factor.views import LoginView as BaseTwoFactorLoginView
@@ -451,6 +452,24 @@ def start_social_signup(request: HttpRequest, backend: str, extra_arg: Optional[
     return oauth_redirect_to_root(request, backend_url, 'social', is_signup=True,
                                   extra_url_params=extra_url_params)
 
+def social_auth(request: HttpRequest, backend: str) -> HttpResponse:
+    """
+    python-social-auth sets certain fields from the request into the session
+    and doesn't clear them if another request is made with a field that was present
+    in the previous request now missing. We use this function to hook into the beginning
+    of the social auth flow to ensure the session is properly cleared out.
+    This function and the corresponding url entry in urls.py should be removed if this issue
+    gets fixed upstream - https://github.com/python-social-auth/social-core/issues/425
+    """
+
+    for field_name in settings.SOCIAL_AUTH_FIELDS_STORED_IN_SESSION:
+        try:
+            del request.session[field_name]
+        except KeyError:
+            pass
+
+    return social_django_auth(request, backend)
+
 def authenticate_remote_user(realm: Realm,
                              email_address: Optional[str]) -> Optional[UserProfile]:
     if email_address is None:

zproject/settings.py

@@ -224,6 +224,10 @@ SILENCED_SYSTEM_CHECKS = [
     # backends support the username not being unique; and they do.
     # See: https://docs.djangoproject.com/en/1.11/topics/auth/customizing/#django.contrib.auth.models.CustomUser.USERNAME_FIELD
     "auth.W004",
+    # urls.W003 warns against using colons in the name in url(..., name) because colons are used
+    # for namespaces. We need to override a url entry in the social: namespace, so we use
+    # the colon in this way intentionally.
+    "urls.W003",
 ]
 
 ########################################################################

zproject/urls.py

@@ -716,6 +716,12 @@ urls += [
 ]
 
 # Python Social Auth
+
+# This overrides the analogical entry in social_django.urls, because we want run our own code
+# at the beginning of social auth process. If deleting this override in the future,
+# it should be possible to remove urls.W003 from SILENCED_SYSTEM_CHECKS.
+urls += [url(r'^login/(?P<backend>[^/]+)/$', zerver.views.auth.social_auth, name='social:begin')]
+
 urls += [url(r'^', include('social_django.urls', namespace='social'))]
 urls += [url(r'^saml/metadata.xml$', zerver.views.auth.saml_sp_metadata)]