Skip to content

Commit

Permalink
registration: Require an explicit realm on PreregistrationUser.
Browse files Browse the repository at this point in the history
This completes the last commit's work to fix CVE-2017-0910, applying
to any invite links already created before the fix was deployed.  With
this change, all new-user registrations must match an explicit realm
in the PreregistrationUser row, except when creating a new realm.

[greg: rewrote commit message]
  • Loading branch information
hackerkid authored and gnprice committed Nov 22, 2017
1 parent 28a3dcf commit 960d736
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 8 deletions.
6 changes: 2 additions & 4 deletions templates/confirmation/link_expired.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@

<div class="pitch">
<hr/>
<p class="lead">Whoops. The confirmation link has expired.</p>
<p class="lead">Whoops. The confirmation link has expired or been deactivated.</p>

<p>
If you're not sure how to generate a new one, shoot us a line at
<a href="mailto:{{ support_email }}">{{ support_email }}</a>
and we'll get this resolved shortly.
Please contact your organization administrator for a new one.
</p>

</div>
Expand Down
2 changes: 1 addition & 1 deletion zerver/tests/test_auth_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ def do_auth(*args, **kwargs):
def test_github_backend_new_user(self):
# type: () -> None
rf = RequestFactory()
request = rf.get('/complete')
request = rf.get('/complete', HTTP_HOST=self.user_profile.realm.host)
request.session = {}
request.user = self.user_profile
self.backend.strategy.request = request
Expand Down
2 changes: 1 addition & 1 deletion zerver/tests/test_email_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def test_confirm_email_change_when_time_exceeded(self):
type=Confirmation.EMAIL_CHANGE)
url = confirmation_url(key, user_profile.realm.host, Confirmation.EMAIL_CHANGE)
response = self.client_get(url)
self.assert_in_success_response(["Whoops. The confirmation link has expired."], response)
self.assert_in_success_response(["The confirmation link has expired or been deactivated."], response)

def test_confirm_email_change(self):
# type: () -> None
Expand Down
22 changes: 21 additions & 1 deletion zerver/tests/test_signup.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ def test_expired_multiuse_link(self):
result = self.client_post(invite_link, {'email': email})

self.assertEqual(result.status_code, 200)
self.assert_in_response("Whoops. The confirmation link has expired.", result)
self.assert_in_response("The confirmation link has expired or been deactivated.", result)

def test_invalid_multiuse_link(self):
# type: () -> None
Expand Down Expand Up @@ -1555,6 +1555,26 @@ def test_replace_subdomain_in_confirmation_link(self) -> None:
'from_confirmation': '1'}, subdomain="zephyr")
self.assert_in_success_response(["We couldn't find your confirmation link"], result)

def test_failed_signup_due_to_empty_realm_in_prereg_user(self) -> None:
"""
Largely to test a transitional state, where we started requiring the
realm in PreregistrationUser (if realm_creation is False), and wanted
to make sure we had properly disabled any existing confirmation links that
didn't have the realm set.
"""
email = "newuser@zulip.com"
password = "password"
self.client_post('/accounts/home/', {'email': email})
PreregistrationUser.objects.update(realm=None)
result = self.client_post(
'/accounts/register/',
{'password': password,
'key': find_key_by_email(email),
'terms': True,
'full_name': "New User",
'from_confirmation': '1'})
self.assert_in_success_response(["The confirmation link has expired or been deactivated."], result)

def test_failed_signup_due_to_restricted_domain(self):
# type: () -> None
realm = get_realm('zulip')
Expand Down
4 changes: 3 additions & 1 deletion zerver/views/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ def accounts_register(request):
realm = None
else:
realm = get_realm(get_subdomain(request))
if prereg_user.realm is not None and prereg_user.realm != realm:
if prereg_user.realm is None:
return render(request, 'confirmation/link_expired.html')
if prereg_user.realm != realm:
return render(request, 'confirmation/link_does_not_exist.html')

if realm and not email_allowed_for_realm(email, realm):
Expand Down

0 comments on commit 960d736

Please sign in to comment.