A bug in the implementation of the can_forge_sender permission
(previously is_api_super_user) resulted in users with this permission
being able to send messages appearing as if sent by a system bots,
including to other organizations hosted by the same Zulip installation.
- The send message API had a bug allowing an api super user to
use forging to send messages to other realms' streams, as a
cross-realm bot. We fix this most directly by eliminating the
realm_str parameter - it is not necessary for any valid current use
case. The email gateway doesn't use this API despite the comment in
that block suggesting otherwise.
- The conditionals inside access_stream_for_send_message are changed up
to improve security. They were generally not ordered very well,
allowing the function to successfully return due to very weak
acceptance conditions - skipping the higher importance checks that
should lead to raising an error.
- The query count in test_subs is decreased because
access_stream_for_send_message returns earlier when doing its check
for a cross-realm bot sender - some subscription checking queries are
skipped.
- A linkifier test in test_message_dict needs to be changed. It didn't
make much sense in the first place, because it was creating a message
by a normal user, to a stream outside of the user's realm. That
shouldn't even be allowed.
Changed the name of the test-user cordelia from `Cordelia Lear` to
`Cordelia, Lear's daughter`.
This change will enable us to test users with escape characters in
their names.
I also updated the Node, Puppeteer, Backend tests and Fixtures to
support this change.
Messages sent by muted users are marked as read
as soon as they are sent (or, more accurately,
while creating the database entries itself), regardless
of type (stream/huddle/PM).
ede73ee4cd, makes it easy to
pass a list to `do_send_messages` containing user-ids for
whom the message should be marked as read.
We add the contents of this list to the set of muter IDs,
and then pass it on to `create_user_messages`.
This benefits from the caching behaviour of `get_muting_users`
and should not cause performance issues long term.
The consequence is that messages sent by muted users will
not contribute to unread counts and notifications.
This commit does not affect the unread messages
(if any) present just before muting, but only handles
subsequent messages. Old unreads will be handled in
further commits.
This adds the is_user_active with the appropriate code for setting the
value correctly in the future. In the following commit a migration to
backfill the value for existing Subscriptions will be added.
To ensure correct user_profile.is_active handling also in tests, we
replace all direct .is_active mutation with calls to appropriate
functions.
This commit adds a new option of STREAM_POST_POLICY_MODERATORS
in stream_post_policy which will allow only realm admins and
moderators to post in that stream.
The comments in stream-policy tests in test_message_send.py specifies
the restriction of creating streams based on stream_post_policy. But
this restriction was removed in 9aaa61963 and we now allow everyone to
create all type of streams. So this commit fixes the stream creation
parts in comments.
This commit updates the stream creation, subscribing others to
stream, wildcard mention settings and stream post policy to allow
realm moderators even if they are new and the respective setting
is set to allow full members only.
This commit renames the is_new_member property in models.py
to is_provisional_member which will return true for any user
who is not a full member. We will add a condition in further
commit such that this returns 'False' for a moderator as we
will initially give all the rights to moderator that a full
member has.
Currently there are only tests for verifying the error case and there
are no tests to check the case where messages are sent successfully
in 'STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS' stream.
This commit adds tests for checking that full members and bots owned
by them can send message successfully in streams with post policy as
'STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS'.
According to tests we should not allow bot without owners to
post in streams with STREAM_POST_POLICY_RESTRICT_NEW_MEMBERS.
But the code does not handle this and the related test passes
and raises error for case of bots without owner because the bot
is itself a new member.
This commit fixes this by adding a condition to check if there
is no bot owner and then raise error if there is no owner.
Adjustments made due to changes in Django 3.0:
(https://docs.djangoproject.com/en/3.0/releases/3.0/)
- test_signup: INTERNAL_RESET_URL_TOKEN was moved to
PasswordResetConfirmView.reset_url_token
- test_message_fetch:
"add_never_cache_headers() and never_cache() now add the private
directive to Cache-Control headers."
- "django.utils.html.escape() now uses html.escape() to escape HTML.
This converts ' to ' instead of the previous equivalent decimal
code '." - this requires adjusting the expected decimal code
in some of the string fixtures in tests.
We change the return type of check_message to be dataclass instead of
Dict[str, Any]. This refactoring helps us to understand the context of the
data structure returned by check_message clearly which was not possible
when using Dict.
SendMessageRequest class is added in zerver/lib/message.py inspite of it
not being used in that file itself just to maintain consistency as other
TypedDicts and dataclasses are defined in that file and to avoid circular
dependency as SendMessageRequest is being used in lib/widget.py as well.
We also rename local variable to 'send_request' for accessing
SendMessageRequest objects.
In 709493cd75 (Feb 2017)
I added code to render_markdown that re-fetched the
sender of the message, to detect whether the message is
a bot.
It's better to just let the ORM fetch this. The
message object should already have sender.
The diff makes it look like we are saving round trips
to the database, which is true in some cases. For
the main message-send codepath, though, we are only
saving a trip to memcached, since the middleware
will have put our sender's user object into the
cache. The test_message_send test calls internally
to check_send_stream_message, so it was actually
hitting the database in render_markdown (prior to
my change).
All the fields of a stream's recipient object can
be inferred from the Stream, so we just make a local
object. Django will create a Message object without
checking that the child Recipient object has been
saved. If that behavior changes in some upgrade,
we should see some pretty obvious symptom, including
query counts changing.
Tweaked by tabbott to add a longer explanatory comment, and delete a
useless old comment.
For streams in which only full members are allowed to post,
we block guest users from posting there.
Guests users were blocked from posting to admin only streams
already. So now, guest users can only post to
STREAM_POST_POLICY_EVERYONE streams.
This is not a new feature but a bugfix which should have
happened when implementing full member stream policy / guest users.
We call build_message_send_dict from check_message instead of
do_send_messages.
This is a prep commit for adding a new setting for handling
wildcard mentions in large streams.
This commit renames 'test_message_to_self' and
'test_api_message_to_self' tests to
'test_message_to_stream_by_name' and
'test_api_message_to_stream_by_name' to depict
the actual purpose of these tests.
The exception trace only goes from where the exception was thrown up
to where the `logging.exception` call is; any context as to where
_that_ was called from is lost, unless `stack_info` is passed as well.
Having the stack is particularly useful for Sentry exceptions, which
gain the full stack trace.
Add `stack_info=True` on all `logging.exception` calls with a
non-trivial stack; we omit `wsgi.py`. Adjusts tests to match.
In this commit, we grant guest users access to stream history,
send message and common stream data of web-public streams.
This is part of PR #14638 that aims to allow guest users to
browse and subscribe to web-public streams.
Some parameters such as `to` and `topic` have been intentionally
undocumentecd hence fail request validation. So mark tests which
fail due to this accordingly.
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.
This commit moves InternalPrepTest test class to test_message_send.py
because it tests internal_send_* and internal_prep_* functions which are
used for internal message sending in zulip.
This commit moves few tests related to testing proper sending of private
messages from PrivateMessagesTest class in test_messages.py to a new class
in test_message_send.py.
The commit moves, test_create_mirror_user_despite_race which is not related
to message sending from MessagePOSTTest class in test_message_send.py to
test_mirror_users.py.
Mateusz Mandera
Arun Sankar
Abhijeet Prasad Bodas
sahil839
shanukun
Anders Kaseorg
Steve Howell
Aman Agrawal
palash
Alex Vandiver
Clara Dantas
orientor
Tim Abbott
Mohit Gupta