This sidesteps tricky escaping issues, and will make it easier to
build a strict Content-Security-Policy.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Fixes#9401.
This adds a FAKE_EMAIL_DOMAIN setting, which should be used if
EXTERNAL_HOST is not a valid domain, and something else is needed to
form bot and dummy user emails (if email visibility is turned off).
It defaults to EXTERNAL_HOST.
get_fake_email_domain() should be used to get this value. It validates
that it's correctly set - that it can be used to form valid emails.
If it's not set correctly, an exception is raised. This is the right
approach, because it's undesirable to have the server seemingly
peacefully operating with that setting misconfigured, as that could
mask some hidden sneaky bugs due to UserProfiles with invalid emails,
which would blow up the moment some code that does validate the emails
is called.
Without disturbing the flow of the existing code for configuring
embedded bots too much, we now use the config_options feature to
allow incoming webhook type bot to be configured via. the "/bots"
endpoint of the API.
This is a prep commit to allow us to validate user provided bot
config data using the same function for incoming webhook type
bots alongside embedded bots (as opposed to creating a new
function just for incoming webhook bots).
Django’s default FileSystemFinder disallows STATICFILES_DIRS from
containing STATIC_ROOT (by raising an ImproperlyConfigured exception),
because STATIC_ROOT is supposed to be the result of collecting all the
static files in the project, not one of the potentially many sources
of static files.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This function is an alternative to get_admin_users that we use in all
places where we explicitly want only human administrative users (not
administrative bots). The following commits will rename
get_admin_users for better clarity.
Mostly rewritten by Tim Abbott to ensure it correctly implements the
desired security model.
Administrators should have access to users' real email address so that
they can contact users out-of-band.
Clients won't have access to user email addresses, and thus won't be
able to compute gravatars.
The tests for this are a bit messy, in large part because our tests
for get_events call subsections of it, rather than the main function.
This commit fixes an error in the logic for allowing admins to edit any
user's CPF (custom profile field) values. The logic allowing users to
edit their own CPF values is however sound. What happens is that of all
the CPF types, for "choice fields" as well as "URL" and "date fields",
when the value is reset/deleted/cleared by the admin in the Admin UI
(organization settings), the frontend would send a null (empty string)
value to the backend for that custom profile field (as this is, after
all, the new value in this case). This would then triggers the backend
validators to return an error message.
We fix this by using the method check_remove_custom_profile_field_value,
that both code paths (user editing their own CPFs and admin editing a
user's CPF) can call.
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 supports guest user in the user-info-form-modal as well as in the
role section of the admin-user-table.
With some fixes by Tim Abbott and Shubham Dhama.
Bots are not allowed to use the same name as
other users in the realm (either bot or human).
This is kind of a big commit, but I wanted to
combine the post/patch (aka add/edit) checks
into one commit, since it's a change in policy
that affects both codepaths.
A lot of the noise is in tests. We had good
coverage on the previous code, including some places
like event testing where we were expediently
not bothering to use different names for
different bots in some longer tests. And then
of course I test some new scenarios that are relevant
with the new policy.
There are two new functions:
check_bot_name_available:
very simple Django query
check_change_bot_full_name:
this diverges from the 3-line
check_change_full_name, where the latter
is still used for the "humans" use case
And then we just call those in appropriate places.
Note that there is still a loophole here
where you can get two bots with the same
name if you reactivate a bot named Fred
that was inactive when the second bot named
Fred was created. Also, we don't attempt
to fix historical data. So this commit
shouldn't be considered any kind of lockdown,
it's just meant to help people from
inadvertently creating two bots of the same
name where they don't intend to. For more
context, we are continuing to allow two
human users in the same realm to have the
same full name, and our code should generally
be tolerant of that possibility. (A good
example is our new mention syntax, which disambiguates
same-named people using ids.)
It's also worth noting that our web app client
doesn't try to scrub full_name from its payload in
situations where the user has actually only modified other
fields in the "Edit bot" UI. Starting here
we just handle this on the server, since it's
easy to fix there, and even if we fixed it in the web
app, there's no guarantee that other clients won't be
just as brute force. It wasn't exactly broken before,
but we'd needlessly write rows to audit tables.
Fixes#10509
Currently, if there is only one admin in realm and admin tries
to updates any non-adminuser's full name it throws error,
"Cannot remove only realm admin". Because in `/json/users/<user_id>`
api check_if_last_admin_is_changed is checked even if property
is_admin is not changed.
This commit fix this issue and add tests for it.
This is a preparatory commit for upcoming changes to move
/avatar/ to be a logged in or API accessible endpoint.
Basically we rename this variable because the new name is more
appropriate in the situation. Also user_profile will be used to
hold the user_profile of person accessing the endpoint in coming up
commit.
random_api_key, the function we use to generate random tokens for API
keys, has been moved to zerver/lib/utils.py because it's used in more
parts of the codebase (apart from user creation), and having it in
zerver/lib/create_user.py was prone to cyclic dependencies.
The function has also been renamed to generate_api_key to have an
imperative name, that makes clearer what it does.
Now reading API keys from a user is done with the get_api_key wrapper
method, rather than directly fetching it from the user object.
Also, every place where an action should be done for each API key is now
using get_all_api_keys. This method returns for the moment a single-item
list, containing the specified user's API key.
This commit is the first step towards allowing users have multiple API
keys.
It's sorta an unusual state to get into, to have a user own a
deactivated bot, when they can't create a bot of that type, but
definitely a valid possibility that we should be checking for.
Fixes#10087.
This adds a common function `access_user_by_id` to access user id
within same realm, complete with a full suite of unit tests.
Tweaked by tabbott to make the test much more readable.
This adds a common function `access_bot_by_id` to access bot id within
same realm. It probably fixes some corner case bugs where we weren't
checking for deactivated bots when regenerating API keys.
These decorators will be part of the process for disabling access to
various features for guest users.
Adding this decorator to the subscribe endpoint breaks the guest users
test we'd just added for the subscribe code path; we address this by
adding a more base-level test on filter_stream_authorization.
This fixes a bug where the endpoint for editing bot users would allow
an organization administrator to edit the full name of a bot user.
A combination of this an another recently fixed bug made it possible
for this process to set a `bot_owner` for a non-bot user; so we also
include a migration to fix that for any users that might have had our
model invariants corrupted in that way.
"incorrect" here means rejected by a bot's validate_config() method.
A common scenario for this is validating API keys before the bot is
created. If validate_config() fails, the bot will not be created.
Rishi Gupta
Mateusz Mandera
Tim Abbott
Yago González
Anders Kaseorg
Hemanth V. Alluri
Mohit Gupta
Joshua Pan
aswanthkoleri
Anders Kaseorg
Yashashvi Dave
Pragati Agrawal
Steve Howell
Aditya Bansal
Vishnu Ks
Shubham Dhama
Robert Hönig