Raising jsonableError in the authentication form was non-ideal because
it took the user to an ugly page with the returned json.
We also add logging of this rare occurence of the scenario being
handled here.
user_profile.check_password(password) in authenticate of
EmailAuthBackend can raise PasswordTooWeakError; this happens when the
user's password is weaker than the current required policies and needs
to be rehashed (E.g. because, as in Django 3.2, the minimum salt
entropy increased).
This is a very rare case, but still needs a good user-facing error
message. We raise a json error to handle this with a user-facing error
message.
See this comment by Mateusz Mandera for a detailed explanation
about this case along with a traceback it generates.
https://github.com/zulip/zulip/pull/15449#discussion_r448308614
The authenticate function of EmailAuthBackend had request param
type set Optional[HttpRequest] had `None` as default. This
function is never called without a request. So this changes it to
require an HttpRequest parameter.
It was made `Optional` in bc062e1c4d,
because this parameter was new in Django at the time.
We're safe to make it a required argument as everything worked well
before that recent commit and Mateusz Mandera and I checked if it gets
`None` anywhere and found only authenticate of non EmailAuthBackend
gets `None` in some places like `dev_direct_login`.
All the places in tests where this function got `None` as request
were fixed in previous commit.
The function get_role_for_new_user was added to get role from the
invited_as value, as invited_as values were one of (1,2,3,4)
previously, but it was then changed to be the actual role value,
i.e. one of (100, 200, 400, 600), in 1f8f227444.
So, we can safely remove this function now and use invited_as value
directly and handle realm_creation case by using an if condition.
This allows access to be more configurable than just setting one
attribute. This can be configured by setting the setting
AUTH_LDAP_ADVANCED_REALM_ACCESS_CONTROL.
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This adds an option for restricting a ldap user
to only be allowed to login into certain realms.
This is done by configuring an attribute mapping of "org_membership"
to an ldap attribute that will contain the list of subdomains the ldap
user is allowed to access. This is analogous to how it's done in SAML.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
The name used to be included in the id_token, but this seems to have
been changed by Apple and now it's sent in the `user` request param.
https://github.com/python-social-auth/social-core/pull/483 is the
upstream PR for this - but upstream is currently unmaintained, so we
have to monkey patch.
We also alter the tests to reflect this situation. Tests no longer put
the name in the id_token, but rather in the `user` request param in the
browser flow, just like it happens in reality.
An adaptation has to be made in the native flow - since the name won't
be included by Apple in the id_token anymore, the app, when POSTing
to the /complete/apple/ endpoint,
can (and should for better user experience)
add the `user` param formatted as json of
{"email": "hamlet@zulip.com", "name": {"firstName": "Full", "lastName": "Name"}}
dict. This is also reflected by the change in the
native flow tests.
We previously used to to redirect to config error page with
a different URL. This commit renders config error in the same
URL where configuration error is encountered. This way when
conifguration error is fixed the user can refresh to continue
normally or go back to login page from the link provided to
choose any other backend auth.
Also moved those URLs to dev_urls.py so that they can be easily
accessed to work on styling etc.
In tests, removed some of the asserts checking status code to be 200
as the function `assert_in_success_response` does that check.
This was called in both if and else with the same argument.
I believe there's no reason for it to exist twice and having
it just once would be a bit cleaner.
Replace default root logger with zulip.auth.apple for apple auth
in file zproject/backends.py and update the test cases
accordingly in file zerver/tests/test_auth_backends.py
Our intent throughout the codebase is to treat email
case-insensitively.
The only codepath affected by this bug is remote_user_sso, as that's the
only one that currently passes potentially both a user_profile and
ExternalAuthDataDict when creating the ExternalAuthResult. That's why we
add a test specifically for that codepath.
Uses git release as this version 3.4.0 is not released to pypi.
This is required for removing some overriden functions of
apple auth backend class AppleAuthBackend.
With the update we also make following changes:
* Fix full name being populated as "None None".
c5c74f27dd that's included in update assigns first_name and last_name
to None when no name is provided by apple. Due to this our
code is filling return_data['full_name'] to 'None None'.
This commit fixes it by making first and last name strings empty.
* Remove decode_id_token override.
Python social auth merged the PR we sent including the changes
we made to decode_id_token function. So, now there is no
necessity for the override.
* Add _AUDIENCE setting in computed_settings.py.
`decode_id_token` is dependent on this setting.
Fixes#15904.
settings is supposed to be a proper OneLogin_Saml2_Settings object,
rather than an empty dictionary. This bug wasn't easy to spot because
the codepath that causes this to demonstrate runs only if the
SAMLResponse contains encrypted assertions.
In particular importing gitter data leads to having accounts with these
noreply github emails. We generally only want users to have emails that
we can actually send messages to, so we'll keep the old behavior of
disallowing sign up with such an email address. However, if an account
of this type already exists, we should allow the user to have access to
it.
A few major themes here:
- We remove short_name from UserProfile
and add the appropriate migration.
- We remove short_name from various
cache-related lists of fields.
- We allow import tools to continue to
write short_name to their export files,
and then we simply ignore the field
at import time.
- We change functions like do_create_user,
create_user_profile, etc.
- We keep short_name in the /json/bots
API. (It actually gets turned into
an email.)
- We don't modify our LDAP code much
here.
I checked that this does not interfere with the MRO of the auth
backends:
In [1]: import zproject.backends; zproject.backends.GitHubAuthBackend.__mro__
Out[1]:
(zproject.backends.GitHubAuthBackend,
zproject.backends.SocialAuthMixin,
zproject.backends.ZulipAuthMixin,
zproject.backends.ExternalAuthMethod,
abc.ABC,
social_core.backends.github.GithubOAuth2,
social_core.backends.oauth.BaseOAuth2,
social_core.backends.oauth.OAuthAuth,
social_core.backends.base.BaseAuth,
object)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Because of other validation on these values, I don't believe any of
these does anything different, but these changes improve readability
and likely make GitHub's code scanners happy.
Adds the ability to set a SAML attribute which contains a
list of subdomains the user is allowed to access. This allows a Zulip
server with multiple organizations to filter using SAML attributes
which organization each user can access.
Cleaned up and adapted by Mateusz Mandera to fit our conventions and
needs more.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Old: a validator returns None on success and returns an error string
on error.
New: a validator returns the validated value on success and raises
ValidationError on error.
This allows mypy to catch mismatches between the annotated type of a
REQ parameter and the type that the validator actually validates.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Anders Kaseorg
Mateusz Mandera
Dinesh
sahil839
strifel
shanukun
Strifel
palash
Tim Abbott
Steve Howell
arpit551
Brainrecursion