There was a bug here that would trigger an exception inside
`sync_user_profile_custom_fields`, causing it to get logged with
logging.warning, when an attribute configured for SAML custom profile
field sync was missing from a SAMLResponse or had an empty value.
`sync_user_profile_custom_fields` expects valid values, and None is not
valid.
We could consider a slightly different behavior here instead - when an
attribute is sent with no value in the SAMLResponse, that means the attr
has no value in the IdP's user directory - so perhaps a better behavior
would be to also remove the custom profile field value in Zulip. However
there are two issues with that:
1. It's not necessarily the best behavior, because an organization might
want the "user doesn't have this attribute set at the IdP level" state
to just mean that the user should be free to set the value manually in
Zulip if they wish. And having that value get reset on every login would
then be an issue. The implementation in this commit is consistent with
this philosophy.
2. There's some implementation difficulty - upstream
`self.get_attr(...)`, which we use for reading the attr value from the
SAMLResponse, doesn't distinguish between an attribute being sent with
no value and the attribute not being sent at all - in both cases it
returns None. So we'd need some extra work here with parsing the
SAMLResponse properly, to be able to know when the custom profile field
should get cleared.
Replace the SOCIAL_AUTH_SYNC_CUSTOM_ATTRS_DICT with
SOCIAL_AUTH_SYNC_ATTRS_DICT, designed to support also regular user attrs
like role or full name (in the future).
Custom attributes can stay configured as they were and will get merged
into SOCIAL_AUTH_SYNC_ATTRS_DICT in computed_settings, or can be
specified in SOCIAL_AUTH_SYNC_ATTRS_DICT directly with "custom__"
prefix.
The role sync is plumbed through to user creation, so users can
immediately be created with their intended role as provided by the IdP
when they're creating their account, even when doing this flow without
an invitiation.
This bug was introduced in da9e4e6e54.
validate validate_plan_for_authentication_methods is already called
inside validate_authentication_methods_dict_from_api, conditionally on
settings.BILLING_ENABLED. This additional, redundant call runs
regardless of BILLING_ENABLED, and thus prevents a self-hosted server
from enabling certain backends in the organization settings UI.
The impact of this is limited - in order to encounter this bug, a
self-hosted server would have to first disable the backend in the UI, as
self-hosted realms are created with all backend flags enabled. A backend
doesn't show up in the org settings UI until it is first enabled in
AUTHENTICATION_BACKENDS in settings.py - that's why this is a rare
state. A sequence of steps like this has to be followed to reproduce:
1. Add the backend to AUTHENTICATION_BACKENDS in settings.py.
2. Disable the backend in the org settings UI.
3. Now try to re-enable it, which fails due to the bug.
SHA1PasswordHasher will be removed in Django 5.1. MD5PasswordHasher
will remain for the purpose of speeding up tests.
Followup to commit ac5161f439 (#29620).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Creates a new "realm_deactivated" email that can be sent to realm
owners as part of `do_deactivate_realm`, via a boolean flag,
`email_owners`.
This flag is set to `False` when `do_deactivate_realm` is used for
realm exports or changing a realm's subdomain, so that the active
organization owners are not emailed in those cases.
This flag is optional for the `deactivate_realm` management command,
but as there is no active user passed in that case, then the email
is sent without referencing who deactivated the realm.
It is passed as `True` for the support analytics view, but the email
that is generated does not include information about the support
admin user who completed the request for organization deactivation.
When an active organization owner deactivates the organization, then
the flag is `True` and an email is sent to them as well as any other
active organization owners, with a slight variation in the email text
for those two cases.
Adds specific tests for when `email_owners` is passed as `True`. All
existing tests for other functionality of `do_deactivate_user` pass
the flag as `False`.
Adds `localize` from django.util.formats as a jinja env filter so
that the dates in these emails are internationlized for the owner's
default language setting in the "realm_deactivated" email templates.
Fixes#24685.
It's going to be helpful in the future to record the reason for realm
deactivation.
- For information tracking
- For making a distinction between cases where we can allow realm owners
to reactivate their realm via a self-serve flow (e.g.
"owner_request") vs where we can't (ToS abuse).
This commit adds include_realm_default_subscriptions parameter
to the invite endpoints and the corresponding field in
PreregistrationUser and MultiuseInvite objects. This field will
be used to subscribe the new users to the default streams at the
time of account creation and not to the streams that were default
when sending the invite.
The naming `uri` is deprecated while `url` should be used in order to
satisfy URL standards. For this reason, four endpoints are affected:
* The response content of three endpoints `/server_settings`,
`/register` and `/realm` that contain a field `realm_uri` is
changed to `realm_url`.
* In one of the common fields for all mobile push notifications payloads,
`realm_url` field is now added as an alias to `realm_uri`.
For backwards compatibility, we keep the field `realm_uri` and add
an alias `realm_url`.
Co-authored-by: Junyao Chen <junyao.chen@socitydao.org>
In #23380, we are changing all occurrences of uri with url in order to
follow the latest URL standard. Previous PRs #25038 and #25045 has
replaced the occurences of uri that has no direct relation with realm.
This commit changes just the model property, which has no API
compatibility concerns.
This prevents users from hammering the invitation endpoint, causing
races, and inviting more users than they should otherwise be allowed
to.
Doing this requires that we not raise InvitationError when we have
partially succeeded; that behaviour is left to the one callsite of
do_invite_users.
Reported by Lakshit Agarwal (@chiekosec).
Only affects zulipchat, by being based on the BILLING_ENABLED setting.
The restricted backends in this commit are
- AzureAD - restricted to Standard plan
- SAML - restricted to Plus plan, although it was already practically
restricted due to requiring server-side configuration to be done by us
This restriction is placed upon **enabling** a backend - so
organizations that already have a backend enabled, will continue to be
able to use it. This allows us to make exceptions and enable a backend
for an org manually via the shell, and to grandfather organizations into
keeping the backend they have been relying on.
The endpoint was lacking validation that the authentication_methods dict
submitted by the user made sense. So e.g. it allowed submitting a
nonsense key like NoSuchBackend or modifying the realm's configured
authentication methods for a backend that's not enabled on the server,
which should not be allowed.
Both were ultimately harmless, because:
1. Submitting NoSuchBackend would luckily just trigger a KeyError inside
the transaction.atomic() block in do_set_realm_authentication_methods
so it would actually roll back the database changes it was trying to
make. So this couldn't actually create some weird
RealmAuthenticationMethod entries.
2. Silently enabling or disabling e.g. GitHub for a realm when GitHub
isn't enabled on the server doesn't really change anything. And this
action is only available to the realm's admins to begin with, so
there's no attack vector here.
test_supported_backends_only_updated wasn't actually testing anything,
because the state it was asserting:
```
self.assertFalse(github_auth_enabled(realm))
self.assertTrue(dev_auth_enabled(realm))
self.assertFalse(password_auth_enabled(realm))
```
matched the desired state submitted to the API...
```
result = self.client_patch(
"/json/realm",
{
"authentication_methods": orjson.dumps(
{"Email": False, "Dev": True, "GitHub": False}
).decode()
},
)
```
so we just replace it with a new test that tests the param validation.
As explained in the comment, this is to prevent bugs where some strange
combination of codepaths could end up calling do_login without basic
validation of e.g. the subdomain. The usefulness of this will be
extended with the upcoming commit to add the ability to configure custom
code to wrap authenticate() calls in. This will help ensure that some
codepaths don't slip by the mechanism, ending up logging in a user
without the chance for the custom wrapper to run its code.
This test is ancient and patches so much that it's almost unreadable,
while being redundant considering we have comprehensive tests via the
SocialAuthBase subclasses. The one missing case was the one with the
backend we disabled. We replace that with a proper
test_social_auth_backend_disabled test in SocialAuthBase.
The original behavior of this setting was to disable LDAP
authentication for any realms not configured to use it. This was an
arbitrary choice, and its only value was to potentially help catch
typos for users who are lazy about testing their configuration.
Since it makes it a very inconvenient to potentially host multiple
organizations with different LDAP configurations, remove that
behavior.
This is a prep commit for adding feature of restricting
user access to guests such that we can keep the code
easy to read and understand when that feature is added.
This fixes a regression introduced in
9954db4b59, where the realm's default
language would be ignored for users created via API/LDAP/SAML,
resulting in all such users having English as their default language.
The API/LDAP/SAML account creation code paths don't have a request,
and thus cannot pull default language from the user's browser.
We have the `realm.default_language` field intended for this use case,
but it was not being passed through the system.
Rather than pass `realm.default_language` through from each caller, we
make the low-level user creation code set this field, as that seems
more robust to the creation of future callers.
This is an exception that we should be generally catching like the
others, which will give our standard /login/ redirect and proper logging
- as opposed to a 500 if we don't catch.
Addresses directly a bug we occurred in the wild, where a SAMLResponse
was submitted without issuers specified in a valid way, causing this
exception. The added test tests this specific type of scenario.
Mateusz Mandera
Anders Kaseorg
Lauryn Menard
Alex Vandiver
Sahil Batra
Alya Abbott
Vector73
Prakhar Pratyush
Tim Abbott
Simon Michalke