auth: Use tokens, with data stored in redis, for log_into_subdomain.

The desktop otp flow (to be added in next commits) will want to generate
one-time tokens for the app that will allow it to obtain an
authenticated session. log_into_subdomain will be the endpoint to pass
the one-time token to. Currently it uses signed data as its input
"tokens", which is not compatible with the otp flow, which requires
simpler (and fixed-length) token. Thus the correct scheme to use is to
store the authenticated data in redis and return a token tied to the
data, which should be passed to the log_into_subdomain endpoint.

In this commit, we replace the "pass signed data around" scheme with the
redis scheme, because there's no point having both.

zerver/lib/redis_utils.py

@@ -11,9 +11,10 @@ def get_redis_client() -> redis.StrictRedis:
 
 def put_dict_in_redis(redis_client: redis.StrictRedis, key_format: str,
                       data_to_store: Dict[str, Any],
-                      expiration_seconds: int) -> str:
+                      expiration_seconds: int,
+                      token_length: int=64) -> str:
     with redis_client.pipeline() as pipeline:
-        token = generate_random_token(64)
+        token = generate_random_token(token_length)
         key = key_format.format(token=token)
         pipeline.set(key, ujson.dumps(data_to_store))
         pipeline.expire(key, expiration_seconds)

zerver/lib/test_helpers.py

@@ -4,7 +4,6 @@ from typing import (
     Optional, Tuple, Union, IO, TypeVar, TYPE_CHECKING
 )
 
-from django.core import signing
 from django.urls.resolvers import LocaleRegexURLResolver
 from django.conf import settings
 from django.test import override_settings
@@ -24,6 +23,7 @@ from zerver.tornado import event_queue
 from zerver.tornado.handlers import allocate_handler_id
 from zerver.worker import queue_processors
 from zerver.lib.integrations import WEBHOOK_INTEGRATIONS
+from zerver.views.auth import get_login_data
 
 from zerver.lib.actions import (
     get_stream_recipient,
@@ -468,7 +468,9 @@ def get_all_templates() -> List[str]:
 def load_subdomain_token(response: HttpResponse) -> Dict[str, Any]:
     assert isinstance(response, HttpResponseRedirect)
     token = response.url.rsplit('/', 1)[1]
-    return signing.loads(token, salt='zerver.views.auth.log_into_subdomain')
+    data = get_login_data(token, should_delete=False)
+    assert data is not None
+    return data
 
 FuncT = TypeVar('FuncT', bound=Callable[..., None])
 

zerver/tests/test_auth_backends.py

@@ -8,7 +8,6 @@ from django_auth_ldap.backend import LDAPSearch, _LDAPUser
 from django.test.client import RequestFactory
 from django.utils.timezone import now as timezone_now
 from typing import Any, Callable, Dict, List, Optional, Tuple
-from django.core import signing
 from django.urls import reverse
 
 import responses
@@ -17,7 +16,6 @@ import ldap
 import jwt
 import mock
 import re
-import time
 import datetime
 
 from zerver.lib.actions import (
@@ -42,6 +40,7 @@ from zerver.lib.request import JsonableError
 from zerver.lib.storage import static_path
 from zerver.lib.users import get_all_api_keys
 from zerver.lib.upload import resize_avatar, MEDIUM_AVATAR_SIZE
+from zerver.lib.utils import generate_random_token
 from zerver.lib.initial_password import initial_password
 from zerver.lib.test_classes import (
     ZulipTestCase,
@@ -66,7 +65,7 @@ from zproject.backends import ZulipDummyBackend, EmailAuthBackend, \
     ZulipLDAPUser
 
 from zerver.views.auth import (maybe_send_to_registration,
-                               _subdomain_token_salt)
+                               store_login_data, LOGIN_TOKEN_LENGTH)
 
 from onelogin.saml2.auth import OneLogin_Saml2_Auth
 from onelogin.saml2.response import OneLogin_Saml2_Response
@@ -1567,8 +1566,12 @@ class GoogleAuthBackendTest(SocialAuthBase):
         with self.settings(AUTHENTICATION_BACKENDS=('zproject.backends.GoogleAuthBackend',)):
             self.assertTrue(google_auth_enabled())
 
-    def get_log_into_subdomain(self, data: Dict[str, Any], *, key: Optional[str]=None, subdomain: str='zulip') -> HttpResponse:
+    def get_log_into_subdomain(self, data: Dict[str, Any], *, subdomain: str='zulip',
-        token = signing.dumps(data, salt=_subdomain_token_salt, key=key)
+                               force_token: Optional[str]=None) -> HttpResponse:
+        if force_token is None:
+            token = store_login_data(data)
+        else:
+            token = force_token
         url_path = reverse('zerver.views.auth.log_into_subdomain', args=[token])
         return self.client_get(url_path, subdomain=subdomain)
 
@@ -1598,29 +1601,27 @@ class GoogleAuthBackendTest(SocialAuthBase):
         self.assertEqual(res.status_code, 302)
         self.assertEqual(res.url, 'http://zulip.testserver/#narrow/stream/7-test-here')
 
-    def test_log_into_subdomain_when_signature_is_bad(self) -> None:
+    def test_log_into_subdomain_when_token_is_malformed(self) -> None:
         data = {'name': 'Full Name',
                 'email': self.example_email("hamlet"),
                 'subdomain': 'zulip',
                 'is_signup': False,
                 'next': ''}
-        with mock.patch('logging.warning') as mock_warning:
+        with mock.patch("logging.warning") as mock_warn:
-            result = self.get_log_into_subdomain(data, key='nonsense')
+            result = self.get_log_into_subdomain(data, force_token='nonsense')
-            mock_warning.assert_called_with("Subdomain cookie: Bad signature.")
+            mock_warn.assert_called_once_with("log_into_subdomain: Malformed token given: nonsense")
         self.assertEqual(result.status_code, 400)
 
-    def test_log_into_subdomain_when_signature_is_expired(self) -> None:
+    def test_log_into_subdomain_when_token_not_found(self) -> None:
         data = {'name': 'Full Name',
                 'email': self.example_email("hamlet"),
                 'subdomain': 'zulip',
                 'is_signup': False,
                 'next': ''}
-        with mock.patch('django.core.signing.time.time', return_value=time.time() - 45):
+        with mock.patch("logging.warning") as mock_warn:
-            token = signing.dumps(data, salt=_subdomain_token_salt)
+            token = generate_random_token(LOGIN_TOKEN_LENGTH)
-        url_path = reverse('zerver.views.auth.log_into_subdomain', args=[token])
+            result = self.get_log_into_subdomain(data, force_token=token)
-        with mock.patch('logging.warning') as mock_warning:
+            mock_warn.assert_called_once_with("log_into_subdomain: Invalid token given: %s" % (token,))
-            result = self.client_get(url_path, subdomain='zulip')
-            mock_warning.assert_called_once()
         self.assertEqual(result.status_code, 400)
 
     def test_log_into_subdomain_when_is_signup_is_true(self) -> None:
@@ -1757,10 +1758,8 @@ class GoogleAuthBackendTest(SocialAuthBase):
         data = {'name': 'Full Name',
                 'email': self.example_email("hamlet"),
                 'subdomain': 'zephyr'}
-        with mock.patch('logging.warning') as mock_warning:
+        result = self.get_log_into_subdomain(data)
-            result = self.get_log_into_subdomain(data)
+        self.assert_json_error(result, "Invalid subdomain")
-            mock_warning.assert_called_with("Login attempt on invalid subdomain")
-        self.assertEqual(result.status_code, 400)
 
 class JSONFetchAPIKeyTest(ZulipTestCase):
     def setUp(self) -> None:

zerver/views/auth.py

@@ -15,7 +15,6 @@ from django.views.decorators.csrf import csrf_exempt
 from django.views.decorators.http import require_safe
 from django.utils.translation import ugettext as _
 from django.utils.http import is_safe_url
-from django.core import signing
 import urllib
 from typing import Any, Dict, List, Optional, Mapping
 
@@ -27,11 +26,13 @@ from zerver.forms import HomepageForm, OurAuthenticationForm, \
     AuthenticationTokenForm
 from zerver.lib.mobile_auth_otp import is_valid_otp, otp_encrypt_api_key
 from zerver.lib.push_notifications import push_notifications_enabled
+from zerver.lib.redis_utils import get_redis_client, get_dict_from_redis, put_dict_in_redis
 from zerver.lib.request import REQ, has_request_variables, JsonableError
 from zerver.lib.response import json_success, json_error
 from zerver.lib.subdomains import get_subdomain, is_subdomain_root_or_alias
 from zerver.lib.user_agent import parse_user_agent
 from zerver.lib.users import get_api_key
+from zerver.lib.utils import has_api_key_format
 from zerver.lib.validator import validate_login_email
 from zerver.models import PreregistrationUser, UserProfile, remote_user_to_email, Realm, \
     get_realm
@@ -52,6 +53,8 @@ from two_factor.views import LoginView as BaseTwoFactorLoginView
 
 ExtraContext = Optional[Dict[str, Any]]
 
+redis_client = get_redis_client()
+
 def get_safe_redirect_to(url: str, redirect_host: str) -> str:
     is_url_safe = is_safe_url(url=url, host=redirect_host)
     if is_url_safe:
@@ -437,22 +440,25 @@ def authenticate_remote_user(realm: Realm,
     return user_profile
 
 _subdomain_token_salt = 'zerver.views.auth.log_into_subdomain'
+LOGIN_KEY_PREFIX = "login_key_"
+LOGIN_KEY_FORMAT = LOGIN_KEY_PREFIX + "{token}"
+LOGIN_KEY_EXPIRATION_SECONDS = 15
+LOGIN_TOKEN_LENGTH = UserProfile.API_KEY_LENGTH
 
 @log_view_func
 def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
-    """Given a valid signed authentication token (generated by
+    """Given a valid authentication token (generated by
     redirect_and_log_into_subdomain called on auth.zulip.example.com),
     call login_or_register_remote_user, passing all the authentication
-    result data that had been encoded in the signed token.
+    result data that has been stored in redis, associated with this token.
     """
-
+    if not has_api_key_format(token):  # The tokens are intended to have the same format as API keys.
-    try:
+        logging.warning("log_into_subdomain: Malformed token given: %s" % (token,))
-        data = signing.loads(token, salt=_subdomain_token_salt, max_age=15)
-    except signing.SignatureExpired as e:
-        logging.warning('Subdomain cookie: {}'.format(e))
         return HttpResponse(status=400)
-    except signing.BadSignature:
+
-        logging.warning('Subdomain cookie: Bad signature.')
+    data = get_login_data(token)
+    if data is None:
+        logging.warning("log_into_subdomain: Invalid token given: %s" % (token,))
         return HttpResponse(status=400)
 
     # We extract fields provided by the caller via the data object.
@@ -462,8 +468,7 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
     # narrow after login.
     subdomain = get_subdomain(request)
     if data['subdomain'] != subdomain:
-        logging.warning('Login attempt on invalid subdomain')
+        raise JsonableError(_("Invalid subdomain"))
-        return HttpResponse(status=400)
     email_address = data['email']
 
     full_name = data.get('name', '')
@@ -507,6 +512,20 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
                                          multiuse_object_key=multiuse_object_key,
                                          full_name_validated=full_name_validated)
 
+def store_login_data(data: Dict[str, Any]) -> str:
+    key = put_dict_in_redis(redis_client, LOGIN_KEY_FORMAT, data,
+                            expiration_seconds=LOGIN_KEY_EXPIRATION_SECONDS,
+                            token_length=LOGIN_TOKEN_LENGTH)
+    token = key.split(LOGIN_KEY_PREFIX, 1)[1]  # remove the prefix
+    return token
+
+def get_login_data(token: str, should_delete: bool=True) -> Optional[Dict[str, Any]]:
+    key = LOGIN_KEY_FORMAT.format(token=token)
+    data = get_dict_from_redis(redis_client, key)
+    if data is not None and should_delete:
+        redis_client.delete(key)
+    return data
+
 def redirect_and_log_into_subdomain(realm: Realm, full_name: str, email_address: str,
                                     is_signup: bool=False, redirect_to: str='',
                                     multiuse_object_key: str='',
@@ -515,7 +534,7 @@ def redirect_and_log_into_subdomain(realm: Realm, full_name: str, email_address:
             'is_signup': is_signup, 'next': redirect_to,
             'multiuse_object_key': multiuse_object_key,
             'full_name_validated': full_name_validated}
-    token = signing.dumps(data, salt=_subdomain_token_salt)
+    token = store_login_data(data)
     subdomain_login_uri = (realm.uri
                            + reverse('zerver.views.auth.log_into_subdomain', args=[token]))
     return redirect(subdomain_login_uri)