Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

send_message_backend: Remove the realm_str API param. 

This already became useless in 6e11754642,
as detailed in the API changelog entry here. At this point, we should
eliminate this param and the weird code around it.

This commit also deletes the associated tests added in
6e11754642, since with realm_str removed,
they make no sense anymore (and actually fail with an OpenAPI error due
to using params not used in the API). Hypothetically they could be
translated to use the subdomain= kwarg, but that also doesn't make
sense, since at that point they'd be just testing the case of a user
making an API request on a different subdomain than their current one
and that's just redundant and already tested generally in
test_decorators.
This commit is contained in:
Mateusz Mandera 2023-02-28 00:47:40 +01:00 committed by Tim Abbott
parent 82379c31e4
commit f0f02d05ab
4 changed files with 7 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api_docs/changelog.md
View File

@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 7.0
**Feature level 166**
* [`POST /messages`](/api/send-message): Eliminated the `realm_str` parameter. This parameter
was already redundant due to it needing to match the realm of the user making the request,
otherwise returning an authorization error. With this, the parameter is removed, meaning
that if provided in the API request, it'll be ignored.
**Feature level 165**
* [`PATCH /user_groups/{user_group_id}`](/api/update-user-group): The

2
version.py
View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the
# new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 165
API_FEATURE_LEVEL = 166
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

76
zerver/tests/test_message_send.py
View File

@ -27,7 +27,6 @@ from zerver.actions.message_send import (
internal_send_stream_message_by_name,
send_rate_limited_pm_notification_to_bot_owner,
)
from zerver.actions.realm_domains import do_add_realm_domain
from zerver.actions.realm_settings import do_set_realm_property
from zerver.actions.streams import do_change_stream_post_policy
from zerver.actions.users import do_change_can_forge_sender, do_deactivate_user
@ -1061,81 +1060,6 @@ class MessagePOSTTest(ZulipTestCase):
)
self.assert_json_error(result, "User not authorized for this query")
def test_send_message_as_not_superuser_to_different_domain(self) -> None:
self.login("hamlet")
result = self.client_post(
"/json/messages",
{
"type": "stream",
"to": "Verona",
"content": "Test message",
"topic": "Test topic",
"realm_str": "mit",
},
)
self.assert_json_error(result, "User not authorized for this query")
def test_send_message_with_can_forge_sender_to_different_domain(self) -> None:
user = self.example_user("default_bot")
do_change_can_forge_sender(user, True)
# To a non-existing realm:
result = self.api_post(
user,
"/api/v1/messages",
{
"type": "stream",
"to": "Verona",
"content": "Test message",
"topic": "Test topic",
"realm_str": "non-existing",
},
)
self.assert_json_error(result, "User not authorized for this query")
# To an existing realm:
zephyr_realm = get_realm("zephyr")
result = self.api_post(
user,
"/api/v1/messages",
{
"type": "stream",
"to": "Verona",
"content": "Test message",
"topic": "Test topic",
"realm_str": zephyr_realm.string_id,
},
)
self.assert_json_error(result, "User not authorized for this query")
def test_send_message_forging_message_to_another_realm(self) -> None:
"""
Test for a specific vulnerability that allowed a .can_forge_sender
user to forge a message as a cross-realm bot to a stream in another realm,
by setting up an appropriate RealmDomain and specifying JabberMirror as client
to cause the vulnerable codepath to be executed.
"""
user = self.example_user("default_bot")
do_change_can_forge_sender(user, True)
zephyr_realm = get_realm("zephyr")
self.make_stream("Verona", zephyr_realm)
do_add_realm_domain(zephyr_realm, "zulip.com", False, acting_user=None)
result = self.api_post(
user,
"/api/v1/messages",
{
"type": "stream",
"to": "Verona",
"client": "JabberMirror",
"content": "Test message",
"topic": "Test topic",
"forged": "true",
"sender": "notification-bot@zulip.com",
"realm_str": zephyr_realm.string_id,
},
)
self.assert_json_error(result, "User not authorized for this query")
def test_send_message_when_sender_is_not_set(self) -> None:
result = self.api_post(
self.mit_user("starnine"),

5
zerver/views/message_send.py
View File

@ -198,7 +198,6 @@ def send_message_backend(
topic_name: Optional[str] = REQ_topic(),
message_content: str = REQ("content"),
widget_content: Optional[str] = REQ(default=None, documentation_pending=True),
realm_str: Optional[str] = REQ("realm_str", default=None, documentation_pending=True),
local_id: Optional[str] = REQ(default=None),
queue_id: Optional[str] = REQ(default=None),
delivery_type: str = REQ("delivery_type", default="send_now", documentation_pending=True),
@ -238,10 +237,6 @@ def send_message_backend(
raise JsonableError(_("User not authorized for this query"))
realm = user_profile.realm
if realm_str and realm_str != user_profile.realm.string_id:
# The realm_str parameter does nothing, because it has to match
# the user's realm - but we keep it around for backward compatibility.
raise JsonableError(_("User not authorized for this query"))
if client.name in ["zephyr_mirror", "irc_mirror", "jabber_mirror", "JabberMirror"]:
# Here's how security works for mirroring: