onboarding: Change logic for preventing new login emails for a new user.

Issue: When you created a new organization with /new, the "new login"
emails were emailed. We previously had a hack of adding the
.just_registered property to the user Python object to attempt to
prevent the emails, and checking that in zerver/signals.py. This
commit gets rid of the .just_registered check.
Instead of the .just_registered check, this checks if the user has
joined more than a minute before.
A test test_dont_send_login_emails_for_new_user_registration_logins
already exists.

Tweaked by tabbott to introduce the constant JUST_CREATED_THRESHOLD.

Fixes #10179.
This commit is contained in:
Shubham Padia 2018-08-10 04:28:44 +05:30 committed by Tim Abbott
parent 674d3804fb
commit feb2cdf378
4 changed files with 26 additions and 11 deletions

View File

@ -15,6 +15,8 @@ from zerver.lib.send_email import FromAddress
from zerver.models import UserProfile
from zerver.lib.timezone import get_timezone
JUST_CREATED_THRESHOLD = 60
def get_device_browser(user_agent: str) -> Optional[str]:
user_agent = user_agent.lower()
if "zulip" in user_agent:
@ -66,7 +68,7 @@ def email_on_new_login(sender: Any, user: UserProfile, request: Any, **kwargs: A
if request:
# If the user's account was just created, avoid sending an email.
if getattr(user, "just_registered", False):
if (timezone_now() - user.date_joined).total_seconds() <= JUST_CREATED_THRESHOLD:
return
user_agent = request.META.get('HTTP_USER_AGENT', "").lower()

View File

@ -6,6 +6,7 @@ from django.test import override_settings
from django_auth_ldap.backend import _LDAPUser
from django.contrib.auth import authenticate
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 builtins import object
from oauth2client.crypt import AppIdentityError
@ -19,6 +20,7 @@ import jwt
import mock
import re
import time
import datetime
from zerver.forms import HomepageForm
from zerver.lib.actions import (
@ -43,6 +45,7 @@ from zerver.lib.test_helpers import POSTRequestMock, HostRequestMock
from zerver.models import \
get_realm, email_to_username, UserProfile, \
PreregistrationUser, Realm, get_user, MultiuseInvite
from zerver.signals import JUST_CREATED_THRESHOLD
from confirmation.models import Confirmation, confirmation_url, create_confirmation_link
@ -657,6 +660,9 @@ class GitHubAuthBackendTest(ZulipTestCase):
mobile_flow_otp = '1234abcd' * 8
account_data_dict = dict(email=self.email, name='Full Name')
self.assertEqual(len(mail.outbox), 0)
self.user_profile.date_joined = timezone_now() - datetime.timedelta(seconds=JUST_CREATED_THRESHOLD + 1)
self.user_profile.save()
with self.settings(SEND_LOGIN_EMAILS=True):
# Verify that the right thing happens with an invalid-format OTP
result = self.github_oauth2_test(account_data_dict, subdomain='zulip',
@ -905,13 +911,17 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest):
self.assertTrue(uri.startswith('http://zulip.testserver/accounts/login/subdomain/'))
def test_google_oauth2_mobile_success(self) -> None:
self.user_profile = self.example_user('hamlet')
self.user_profile.date_joined = timezone_now() - datetime.timedelta(seconds=JUST_CREATED_THRESHOLD + 1)
self.user_profile.save()
mobile_flow_otp = '1234abcd' * 8
token_response = ResponseMock(200, {'access_token': "unique_token"})
account_data = dict(name=dict(formatted="Full Name"),
emails=[dict(type="account",
value=self.example_email("hamlet"))])
value=self.user_profile.email)])
account_response = ResponseMock(200, account_data)
self.assertEqual(len(mail.outbox), 0)
with self.settings(SEND_LOGIN_EMAILS=True):
# Verify that the right thing happens with an invalid-format OTP
result = self.google_oauth2_test(token_response, account_response, subdomain='zulip',
@ -930,9 +940,9 @@ class GoogleSubdomainLoginTest(GoogleOAuthTest):
query_params = urllib.parse.parse_qs(parsed_url.query)
self.assertEqual(parsed_url.scheme, 'zulip')
self.assertEqual(query_params["realm"], ['http://zulip.testserver'])
self.assertEqual(query_params["email"], [self.example_email("hamlet")])
self.assertEqual(query_params["email"], [self.user_profile.email])
encrypted_api_key = query_params["otp_encrypted_api_key"][0]
hamlet_api_keys = get_all_api_keys(self.example_user('hamlet'))
hamlet_api_keys = get_all_api_keys(self.user_profile)
self.assertIn(otp_decrypt_api_key(encrypted_api_key, mobile_flow_otp), hamlet_api_keys)
self.assertEqual(len(mail.outbox), 1)
self.assertIn('Zulip on Android', mail.outbox[0].body)
@ -1354,6 +1364,8 @@ class FetchAPIKeyTest(ZulipTestCase):
SEND_LOGIN_EMAILS=True)
def test_google_oauth2_token_success(self) -> None:
self.assertEqual(len(mail.outbox), 0)
self.user_profile.date_joined = timezone_now() - datetime.timedelta(seconds=JUST_CREATED_THRESHOLD + 1)
self.user_profile.save()
with mock.patch(
'apiclient.sample_tools.client.verify_id_token',
return_value={
@ -1897,9 +1909,11 @@ class TestZulipRemoteUserBackend(ZulipTestCase):
def test_login_mobile_flow_otp_success(self) -> None:
user_profile = self.example_user('hamlet')
email = user_profile.email
user_profile.date_joined = timezone_now() - datetime.timedelta(seconds=61)
user_profile.save()
mobile_flow_otp = '1234abcd' * 8
# Verify that the right thing happens with an invalid-format OTP
# Verify that the right thing happens with an invalid-format OTP
result = self.client_post('/accounts/login/sso/',
dict(mobile_flow_otp="1234"),
REMOTE_USER=email,

View File

@ -3,7 +3,7 @@ from django.conf import settings
from django.core import mail
from django.contrib.auth.signals import user_logged_in
from zerver.lib.test_classes import ZulipTestCase
from zerver.signals import get_device_browser, get_device_os
from zerver.signals import get_device_browser, get_device_os, JUST_CREATED_THRESHOLD
from zerver.lib.actions import notify_new_user
from zerver.models import Recipient, Stream, Realm
from zerver.lib.initial_password import initial_password
@ -25,14 +25,16 @@ class SendLoginEmailTest(ZulipTestCase):
with self.settings(SEND_LOGIN_EMAILS=True):
self.assertTrue(settings.SEND_LOGIN_EMAILS)
# we don't use the self.login method since we spoof the user-agent
utc = get_timezone('utc')
mock_time = datetime.datetime(year=2018, month=1, day=1, tzinfo=utc)
user = self.example_user('hamlet')
user.timezone = 'US/Pacific'
user.date_joined = mock_time - datetime.timedelta(seconds=JUST_CREATED_THRESHOLD + 1)
user.save()
password = initial_password(user.email)
firefox_windows = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0"
utc = get_timezone('utc')
user_tz = get_timezone(user.timezone)
mock_time = datetime.datetime(year=2018, month=1, day=1, tzinfo=utc)
utc_offset = mock_time.astimezone(user_tz).strftime('%z')
reference_time = mock_time.astimezone(user_tz).strftime('%A, %B %d, %Y at %I:%M%p ') + utc_offset
with mock.patch('zerver.signals.timezone_now', return_value=mock_time):

View File

@ -335,9 +335,6 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
)
def login_and_go_to_home(request: HttpRequest, user_profile: UserProfile) -> HttpResponse:
# Mark the user as having been just created, so no "new login" email is sent
user_profile.just_registered = True
do_login(request, user_profile)
return HttpResponseRedirect(user_profile.realm.uri + reverse('zerver.views.home.home'))