When creating realm with the ldap backend, the registration flow didn't
properly handle some things - the user wouldn't be set as realm admin,
initial subscriptions and messages weren't created, and the redirect
wasn't happening properly in the case of subdomains.
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 small block of code was over-indented. It should be run in this
part of the function unconditionally, not inside an "else" block.
We obviously want it to run regardless of whether
request.POST.get('from_confirmation')
is True or not.
This changes the way django_to_ldap_username works to make sure the ldap
username it returns actually has a corresponding ldap entry and raise an
exception if that's not possible. It seems to be a more sound approach
than just having it return its best guess - which was the case so far.
Now there is a guarantee that what it returns is the username of an
actual ldap user.
This allows communicating to the registration flow when the email being
registered doesn't belong to ldap, which then will proceed to register
it via the normal email backend flow - finally fixing the bug where you
couldn't register a non-ldap email even with the email backend enabled.
These changes to the behavior of django_to_ldap_username require small
refactorings in a couple of other functions that call it, as well as
adapting some tests to these changes. Finally, additional tests are
added for the above-mentioned registration flow behavior and some
related corner-cases.
Previously, the logic for determining whether to provide an LDAP
password prompt on the registration page was incorrectly including it
if any LDAP authentication was backend enabled, even if LDAP was
configured with the populate-only backend that is not responsible for
authentication (just for filling in name and custom profile fields).
We fix this by correcting the conditional, and add a test.
There's still follow-up work to do here: We may still end up
presenting a registration form in situations where it's useless
because we got all the data from SAML + LDAP. But that's for a future
issue.
This fixes a bug reported in #13275.
This makes the implementation of `get_realm` consistent with its
declared return type of `Realm` rather than `Optional[Realm]`.
Fixes#12263.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Previously, we had some expensive-to-calculate keys in
zulip_default_context, especially around enabled authentication
backends, which in total were a significant contributor to the
performance of various logged-out pages. Now, these keys are only
computed for the login/registration pages where they are needed.
This is a moderate performance optimization for the loading time of
many logged-out pages.
Closes#11929.
The hope is that by having a shorter list of initial streams, it'll
avoid some potential confusion confusion about the value of topics.
At the very least, having 5 streams each with 1 topic was not a good
way to introduce Zulip.
This commit minimizes changes to the message content in
`send_initial_realm_messages` to keep the diff readable. Future commits will
reshape the content.
For Google auth, the multiuse invite key should be stored in the
csrf_state sent to google along with other values like is_signup,
mobile_flow_otp.
For social auth, the multiuse invite key should be passed as params to
the social-auth backend. The passing of the key is handled by
social_auth pipeline and made available to us when the auth is
completed.
This causes changing the email_address_visibility field to actually
modify what user_profile.email values are generated for users, both on
user creation and afterwards as email addresses are edited.
The overall feature isn't yet complete, but this brings us pretty close.
Previously, zerver.views.registration.confirmation_key was only
available in development; now we make that more structurally clear by
moving it to the special zerver/views/development directory.
Fixes#11256.
Since we have already added the `invite_as` field to models, we can now
replace usage of `invite_as_admin` properly with its equivalent `invite_as
== PreregistrationUser.INVITE_AS['REALM_ADMIN']`.
Hence, also removed now redundant `invite_as`.
It appears that our i18n logic was only using the recipient's language
for logged-in emails, so even properly tagged for translation and
translated emails for functions like "Find my team" and "password
reset" were being always sent in English.
With great work by Vishnu Ks on the tests and the to_emails code path.
A key part of this is the new helper, get_user_by_delivery_email. Its
verbose name is important for clarity; it should help avoid blind
copy-pasting of get_user (which we'll also want to rename).
Unfortunately, it requires detailed understanding of the context to
figure out which one to use; each is used in about half of call sites.
Another important note is that this PR doesn't migrate get_user calls
in the tests except where not doing so would cause the tests to fail.
This probably deserves a follow-up refactor to avoid bugs here.
This adds a function that sends provided email to all administrators
of a realm, but in a single email. As a result, send_email now takes
arguments to_user_ids and to_emails instead of to_user_id and
to_email.
We adjust other APIs to match, but note that send_future_email does
not yet support the multiple recipients model for good reasons.
Tweaked by tabbott to modify `manage.py deliver_email` to handle
backwards-compatibily for any ScheduledEmail objects already in the
database.
Fixes#10896.
We're trying to sweep "subject" out of the codebase,
even when it has nothing to do our legacy "subject"
field. The rewording here will prevent some linter
noise.
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.
Right now it only has one function, but the function
we removed never really belonged in actions.py, and
now we have better test coverage on actions.py, which
is an important module to get to 100%.
Previously, if you had LDAPAuthBackend enabled, we basically blocked
any other auth backends from working at all, by requiring the user's
login flow include verifying the user's LDAP password.
We still want to enforce that in the case that the account email
matches LDAP_APPEND_DOMAIN, but there's a reasonable corner case:
Having effectively guest users from outside the LDAP domain.
We don't want to allow creating a Zulip-level password for a user
inside the LDAP domain, so we still verify the LDAP password in that
flow, but if the email is allowed to register (due to invite or
whatever) but is outside the LDAP domain for the organization, we
allow it to create an account and set a password.
For the moment, this solution only covers EmailAuthBackend. It's
likely that just extending the list of other backends we check for in
the new conditional on `email_auth_backend` would be correct, but we
haven't done any testing for those cases, and with auth code paths,
it's better to disallow than allow untested code paths.
Fixes#9422.
This should significantly improve the user experience for creating
additional accounts on zulipchat.com.
Currently, disabled in production pending some work on visual styling.
This will let us defer configuring outbound email to the end of the
install procedure, so we can greatly simplify it by consolidating
several scripted steps.
The new flow could be simplified further by giving the user the full
form in the first place, rather than first a form for just their
email address and then a form with the other details. We'll leave
that improvement for a separate change.
Now, there's just one spot at the beginning of the function where we
inspect the string key the user gave us; and after that point, we not
only have validated that string but in fact are working from our own
record that it pointed to, not the string itself.
This simplifies the code a bit, e.g. by not repeatedly searching the
database for the key (and hoping everything agrees so that we keep
getting the same row), and it will simplify adding logic to inspect
row attributes like `presume_email_valid`.
There's no use case for presenting a key that's invalid; if we haven't
given the user a valid key, we needn't send them to a URL that
presents an invalid one. And the code is simpler to think about if
the only keys that can exist (after the validation at the top of the
function) are valid ones.
Apart from the case where creation_key is None, but invalid, and
settings.OPEN_REALM_CREATION is True so that we'd previously let the
invalid key slide, this is a pure refactor.
This is a little cleaner in that the try/except blocks for
SMTPException are a lot narrower; and it'll facilitate an upcoming
change to sometimes skip sending mail.
Eventually this check for the realm will be done in get_object_from_key
itself. Rewriting this to fit the pattern in get_object_from_key.
No change to behavior.
Commit d4ee3023 and its parent have the history behind this code.
Since d4ee3023^, all new PreregistrationUser objects, except those for
realm creation, have a non-None `realm`. Since d4ee3023, any legacy
PreregistrationUsers, with a `realm` of None despite not being for
realm creation, are treated as expired. Now, we ignore them
completely, and remove any that exist from the database.
The user-visible effect is to change the error message for
registration (or invitation) links created before d4ee3023^ to be
"link does not exist", rather than "link expired".
This change will at most affect users upgrading straight from 1.7 or
earlier to 1.8 (rather than from 1.7.1), but I think that's not much
of a concern (such installations are probably long-running
installations, without many live registration or invitation links).
[greg: tweaked commit message]
[Modified by greg to (1) keep `USERNAME_FIELD = 'email'`,
(2) silence the corresponding system check, and (3) ban
reusing a system bot's email address, just like we do in
realm creation.]
As we migrate to allow reuse of the same email with multiple realms,
we need to replace the old "no email reuse" validators. Because
stealing the email for a system bot would be problematic, we still ban
doing so.
This commit only affects the realm creation logic, not registering an
account in an existing realm.
The one thing this bit of logic is used for is to decide whether
there's an existing user which is a mirror dummy that we should
activate. This change causes us to ignore such an existing user if
it's on some other realm, and go straight into `do_create_user`.
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]
We would allow a user with a valid invitation for one realm to use it
on a different realm instead. On a server with multiple realms, an
authorized user of one realm could use this (by sending invites to
other email addresses they control) to create accounts on other
realms. (CVE-2017-0910)
With this commit, when sending an invitation, we record the inviting
user's realm on the PreregistrationUser row; and when registering a
user, we check that the PregistrationUser realm matches the realm the
user is trying to register on. This resolves CVE-2017-0910 for
newly-sent invitations; the next commit completes the fix.
[greg: rewrote commit message]
Previously, this was a ValidationError, but that doesn't really make
sense, since this condition reflects an actual bug in the code.
Because this happened to be our only test coverage the ValidationError
catch on line 84 of registration.py, we add nocoverage there for now.
I remember being really confused by this function in the past, and I finally
figured it out. It should be removed, and the dev_url added by
00-realm-creation should call a function that just gets the confirmation_key
from outbox like all of the backend tests, but until then this comment
should help.
This change:
* Prevents weird potential attacks like taking a valid confirmation link
(say an unsubscribe link), and putting it into the URL of a multiuse
invite link. I don't know of any such attacks one could do right now, but
reasoning about it is complicated.
* Makes the code easier to read, and in the case of confirmation/views.py,
exposes something that needed refactoring anyway (USER_REGISTRATION and
INVITATION should have different endpoints, and both of those endpoints
should be in zerver/views/registration, not this file).
Most of these have more to do with authentication in general than with
registering a new account. `create_preregistration_user` could go
either way; we move it to `auth` so we can make the imports go only in
one direction.
Tweaked by tabbott to have the field before the invitation is
completed be called invite_as_admins, not invited_as_admins, for
readability.
Fixes#6834.
Wherever possible, we always want to move checking for error
conditions to the views code, so that we don't need to worry about
handling failures with (in this case) a user that's half-created
because a DefaultStreamGroup doesn't exist.
This effectively implements the feature of default stream groups,
except for a UI, nice styling, etc.
Note that we're careful to not have this do anything in an
organization that doesn't have any default stream groups.
While it's totally fine to put a leading '.' before the cookie domain
for normal hostnames and browsers will just strip them, if you're
using an IP address, it doesn't work, because .127.0.0.1 (for example)
is just invalid, and the cookie won't be set.
This fixes an issue where after installing with an IP address, realm
creation would end with being stuck at a blank page for
/accounts/login/subdomain/.
While our recent changing to hide /register means we don't need a nice
pretty error message here, eventually we'll want to clean up the error
message.
Fixes#7047.
This new function extractions the bit of logic we use after creating a
new user account to log them in and send them to the home page,
without emailing the user about their new login.
Use this new variable to determine if the user already exists while
doing registration. While doing login through GitHub if we press
*Go back to login*, we pass email using email variable. As a result,
the login page starts showing the "User already exists error" if we
don't change the variable.
Mateusz Mandera
Tim Abbott
Anders Kaseorg
Hemanth V. Alluri
Rishi Gupta
Vishnu Ks
Anders Kaseorg
Harshit Bansal
Vaibhav
Shubham Dhama
Raymond Akornor
Steve Howell
Shubham Padia
rht
neiljp (Neil Pilgrim)
Greg Price
Aditya Bansal
Vishnu Ks
Umair Khan