Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

register: Pre-populate Name in social backend flow. 

By adding some additional plumbing (through PreregistrationUser) of the
full_name and an additional full_name_validated option, we
pre-populate the Full Name field in the registration form when coming
through a social backend (google/github/saml/etc.) and potentially skip
the registration form (if the user would have nothing to do there other
than clicking the Confirm button) and just create the account and log
the user in.
This commit is contained in:
Mateusz Mandera 2019-11-01 00:00:36 +01:00 committed by Tim Abbott
parent 34a540bacb
commit 5aded51b73
7 changed files with 147 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
zerver/migrations/0251_prereg_user_add_full_name.py Normal file
View File

@ -0,0 +1,25 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.25 on 2019-10-31 20:31
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0250_saml_auth'),
]
operations = [
migrations.AddField(
model_name='preregistrationuser',
name='full_name',
field=models.CharField(max_length=100, null=True),
),
migrations.AddField(
model_name='preregistrationuser',
name='full_name_validated',
field=models.BooleanField(default=False),
),
]

5
zerver/models.py
View File

@ -1168,6 +1168,11 @@ class PreregistrationUser(models.Model):
# form.
email = models.EmailField() # type: str
# If the pre-registration process provides a suggested full name for this user,
# store it here to use it to prepopulate the Full Name field in the registration form:
full_name = models.CharField(max_length=UserProfile.MAX_NAME_LENGTH, null=True) # type: Optional[str]
full_name_validated = models.BooleanField(default=False)
referred_by = models.ForeignKey(UserProfile, null=True, on_delete=CASCADE) # type: Optional[UserProfile]
streams = models.ManyToManyField('Stream') # type: Manager
invited_at = models.DateTimeField(auto_now=True) # type: datetime.datetime

52
zerver/tests/test_auth_backends.py
View File

@ -766,6 +766,7 @@ class SocialAuthBase(ZulipTestCase):
# Name wasn't changed at all
self.assertEqual(hamlet.full_name, "King Hamlet")
@override_settings(TERMS_OF_SERVICE=None)
def test_social_auth_registration(self) -> None:
"""If the user doesn't exist yet, social auth can be used to register an account"""
email = "newuser@zulip.com"
@ -795,26 +796,30 @@ class SocialAuthBase(ZulipTestCase):
result = self.client_get(result.url)
self.assert_in_response('action="/accounts/register/"', result)
data = {"from_confirmation": "1",
"full_name": name,
"key": confirmation_key}
result = self.client_post('/accounts/register/', data)
self.assert_in_response("We just need you to do one last thing", result)
if not self.BACKEND_CLASS.full_name_validated:
self.assert_in_response("We just need you to do one last thing", result)
# Verify that the user is asked for name but not password
self.assert_not_in_success_response(['id_password'], result)
self.assert_in_success_response(['id_full_name'], result)
# Verify that the user is asked for name but not password
self.assert_not_in_success_response(['id_password'], result)
self.assert_in_success_response(['id_full_name'], result)
# Verify the name field gets correctly pre-populated:
self.assert_in_success_response([name], result)
# Click confirm registration button.
result = self.client_post(
'/accounts/register/',
{'full_name': name,
'key': confirmation_key,
'terms': True})
# Click confirm registration button.
result = self.client_post(
'/accounts/register/',
{'full_name': name,
'key': confirmation_key,
'terms': True})
self.assertEqual(result.status_code, 302)
user_profile = get_user(email, realm)
self.assert_logged_in_user_id(user_profile.id)
self.assertEqual(user_profile.full_name, name)
@override_settings(TERMS_OF_SERVICE=None)
def test_social_auth_registration_using_multiuse_invite(self) -> None:
"""If the user doesn't exist yet, social auth can be used to register an account"""
email = "newuser@zulip.com"
@ -870,25 +875,28 @@ class SocialAuthBase(ZulipTestCase):
result = self.client_get(result.url)
self.assert_in_response('action="/accounts/register/"', result)
data = {"from_confirmation": "1",
"full_name": name,
"key": confirmation_key}
result = self.client_post('/accounts/register/', data)
self.assert_in_response("We just need you to do one last thing", result)
if not self.BACKEND_CLASS.full_name_validated:
self.assert_in_response("We just need you to do one last thing", result)
# Verify that the user is asked for name but not password
self.assert_not_in_success_response(['id_password'], result)
self.assert_in_success_response(['id_full_name'], result)
# Verify that the user is asked for name but not password
self.assert_not_in_success_response(['id_password'], result)
self.assert_in_success_response(['id_full_name'], result)
# Verify the name field gets correctly pre-populated:
self.assert_in_success_response([name], result)
# Click confirm registration button.
result = self.client_post(
'/accounts/register/',
{'full_name': name,
'key': confirmation_key,
'terms': True})
# Click confirm registration button.
result = self.client_post(
'/accounts/register/',
{'full_name': name,
'key': confirmation_key,
'terms': True})
self.assertEqual(result.status_code, 302)
user_profile = get_user(email, realm)
self.assert_logged_in_user_id(user_profile.id)
self.assertEqual(user_profile.full_name, name)
def test_social_auth_registration_without_is_signup(self) -> None:
"""If `is_signup` is not set then a new account isn't created"""

15
zerver/tests/test_signup.py
View File

@ -79,6 +79,7 @@ class RedirectAndLogIntoSubdomainTestCase(ZulipTestCase):
data = load_subdomain_token(response)
self.assertDictEqual(data, {'name': name, 'next': '',
'email': email,
'full_name_validated': False,
'subdomain': realm.subdomain,
'is_signup': False,
'multiuse_object_key': ''})
@ -89,6 +90,20 @@ class RedirectAndLogIntoSubdomainTestCase(ZulipTestCase):
data = load_subdomain_token(response)
self.assertDictEqual(data, {'name': name, 'next': '',
'email': email,
'full_name_validated': False,
'subdomain': realm.subdomain,
'is_signup': True,
'multiuse_object_key': 'key'
})
response = redirect_and_log_into_subdomain(realm, name, email,
is_signup=True,
full_name_validated=True,
multiuse_object_key='key')
data = load_subdomain_token(response)
self.assertDictEqual(data, {'name': name, 'next': '',
'email': email,
'full_name_validated': True,
'subdomain': realm.subdomain,
'is_signup': True,
'multiuse_object_key': 'key'

42
zerver/views/auth.py
View File

@ -60,21 +60,27 @@ def get_safe_redirect_to(url: str, redirect_host: str) -> str:
return redirect_host
def create_preregistration_user(email: str, request: HttpRequest, realm_creation: bool=False,
password_required: bool=True) -> HttpResponse:
password_required: bool=True, full_name: Optional[str]=None,
full_name_validated: bool=False) -> HttpResponse:
realm = None
if not realm_creation:
try:
realm = get_realm(get_subdomain(request))
except Realm.DoesNotExist:
pass
return PreregistrationUser.objects.create(email=email,
realm_creation=realm_creation,
password_required=password_required,
realm=realm)
return PreregistrationUser.objects.create(
email=email,
realm_creation=realm_creation,
password_required=password_required,
realm=realm,
full_name=full_name,
full_name_validated=full_name_validated
)
def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str='',
is_signup: bool=False, password_required: bool=True,
multiuse_object_key: str='') -> HttpResponse:
multiuse_object_key: str='',
full_name_validated: bool=False) -> HttpResponse:
"""Given a successful authentication for an email address (i.e. we've
confirmed the user controls the email address) that does not
currently have a Zulip account in the target realm, send them to
@ -114,8 +120,12 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str=
prereg_user = create_preregistration_user(email, request,
password_required=password_required)
else:
prereg_user = create_preregistration_user(email, request,
password_required=password_required)
prereg_user = create_preregistration_user(
email, request,
password_required=password_required,
full_name=full_name,
full_name_validated=full_name_validated
)
if multiuse_object_key:
request.session.modified = True
@ -167,7 +177,8 @@ def login_or_register_remote_user(request: HttpRequest, remote_username: str,
user_profile: Optional[UserProfile], full_name: str='',
mobile_flow_otp: Optional[str]=None,
is_signup: bool=False, redirect_to: str='',
multiuse_object_key: str='') -> HttpResponse:
multiuse_object_key: str='',
full_name_validated: bool=False) -> HttpResponse:
"""Given a successful authentication showing the user controls given
email address (remote_username) and potentially a UserProfile
object (if the user already has a Zulip account), redirect the
@ -192,7 +203,8 @@ def login_or_register_remote_user(request: HttpRequest, remote_username: str,
# there's no associated Zulip user account. Consider sending
# the request to registration.
return maybe_send_to_registration(request, email, full_name, password_required=False,
is_signup=is_signup, multiuse_object_key=multiuse_object_key)
is_signup=is_signup, multiuse_object_key=multiuse_object_key,
full_name_validated=full_name_validated)
# Otherwise, the user has successfully authenticated to an
# account, and we need to do the right thing depending whether
@ -435,6 +447,7 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
full_name = data['name']
is_signup = data['is_signup']
redirect_to = data['next']
full_name_validated = data.get('full_name_validated', False)
if 'multiuse_object_key' in data:
multiuse_object_key = data['multiuse_object_key']
@ -473,14 +486,17 @@ def log_into_subdomain(request: HttpRequest, token: str) -> HttpResponse:
return login_or_register_remote_user(request, email_address, user_profile,
full_name,
is_signup=is_signup, redirect_to=redirect_to,
multiuse_object_key=multiuse_object_key)
multiuse_object_key=multiuse_object_key,
full_name_validated=full_name_validated)
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='') -> HttpResponse:
multiuse_object_key: str='',
full_name_validated: bool=False) -> HttpResponse:
data = {'name': full_name, 'email': email_address, 'subdomain': realm.subdomain,
'is_signup': is_signup, 'next': redirect_to,
'multiuse_object_key': multiuse_object_key}
'multiuse_object_key': multiuse_object_key,
'full_name_validated': full_name_validated}
token = signing.dumps(data, salt=_subdomain_token_salt)
subdomain_login_uri = (realm.uri
+ reverse('zerver.views.auth.log_into_subdomain', args=[token]))

9
zerver/views/registration.py
View File

@ -166,6 +166,15 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
except TypeError:
# Let the user fill out a name and/or try another backend
form = RegistrationForm(realm_creation=realm_creation)
elif prereg_user.full_name:
if prereg_user.full_name_validated:
request.session['authenticated_full_name'] = prereg_user.full_name
name_validated = True
form = RegistrationForm({'full_name': prereg_user.full_name},
realm_creation=realm_creation)
else:
form = RegistrationForm(initial={'full_name': prereg_user.full_name},
realm_creation=realm_creation)
elif 'full_name' in request.POST:
form = RegistrationForm(
initial={'full_name': request.POST.get('full_name')},

43
zproject/backends.py
View File

@ -948,6 +948,7 @@ def social_auth_finish(backend: Any,
assert return_data.get('valid_attestation') is True
strategy = backend.strategy
full_name_validated = backend.full_name_validated
email_address = return_data['validated_email']
full_name = return_data['full_name']
is_signup = strategy.session_get('is_signup') == '1'
@ -970,11 +971,14 @@ def social_auth_finish(backend: Any,
# redirect directly from here, saving a round trip over what
# we need to do to create session cookies on the right domain
# in the web login flow (below).
return login_or_register_remote_user(strategy.request, email_address,
user_profile, full_name,
mobile_flow_otp=mobile_flow_otp,
is_signup=is_signup,
redirect_to=redirect_to)
return login_or_register_remote_user(
strategy.request, email_address,
user_profile, full_name,
mobile_flow_otp=mobile_flow_otp,
is_signup=is_signup,
redirect_to=redirect_to,
full_name_validated=full_name_validated
)
# If this authentication code were executing on
# subdomain.zulip.example.com, we would just call
@ -989,10 +993,13 @@ def social_auth_finish(backend: Any,
# cryptographically signed token) to a route on
# subdomain.zulip.example.com that will verify the signature and
# then call login_or_register_remote_user.
return redirect_and_log_into_subdomain(realm, full_name, email_address,
is_signup=is_signup,
redirect_to=redirect_to,
multiuse_object_key=multiuse_object_key)
return redirect_and_log_into_subdomain(
realm, full_name, email_address,
is_signup=is_signup,
redirect_to=redirect_to,
multiuse_object_key=multiuse_object_key,
full_name_validated=full_name_validated
)
class SocialAuthMixin(ZulipAuthMixin):
auth_backend_name = "undeclared"
@ -1003,6 +1010,18 @@ class SocialAuthMixin(ZulipAuthMixin):
# higher sort order are displayed first.
sort_order = 0
# Whether we expect that the full_name value obtained by the
# social backend is definitely how the user should be referred to
# in Zulip, which in turn determines whether we should always show
# a registration form in the event with a default value of the
# user's name when using this social backend so they can change
# it. For social backends like SAML that are expected to be a
# central database, this should be True; for backends like GitHub
# where the user might not have a name set or have it set to
# something other than the name they will prefer to use in Zulip,
# it should be False.
full_name_validated = False
def auth_complete(self, *args: Any, **kwargs: Any) -> Optional[HttpResponse]:
"""This is a small wrapper around the core `auth_complete` method of
python-social-auth, designed primarily to prevent 500s for
@ -1125,6 +1144,12 @@ class SAMLAuthBackend(SocialAuthMixin, SAMLAuth):
# There's no common default logo for SAML authentication.
display_icon = ""
# The full_name provided by the IdP is very likely the standard
# employee directory name for the user, and thus what they and
# their organization want to use in Zulip. So don't unnecessarily
# provide a registration flow prompt for them to set their name.
full_name_validated = True
def auth_url(self) -> str:
"""Get the URL to which we must redirect in order to
authenticate the user. Overriding the original SAMLAuth.auth_url.