mirror of
https://github.com/element-hq/synapse.git
synced 2024-11-22 09:35:45 +03:00
Add additional validation to pusher URLs. (#8865)
Pusher URLs now must end in `/_matrix/push/v1/notify` per the specification.
This commit is contained in:
parent
df3e6a23a7
commit
b774c555d8
6 changed files with 107 additions and 31 deletions
1
changelog.d/8865.bugfix
Normal file
1
changelog.d/8865.bugfix
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Add additional validation to pusher URLs to be compliant with the specification.
|
|
@ -15,5 +15,4 @@
|
||||||
|
|
||||||
|
|
||||||
class PusherConfigException(Exception):
|
class PusherConfigException(Exception):
|
||||||
def __init__(self, msg):
|
"""An error occurred when creating a pusher."""
|
||||||
super().__init__(msg)
|
|
||||||
|
|
|
@ -14,6 +14,7 @@
|
||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
import logging
|
import logging
|
||||||
|
import urllib.parse
|
||||||
|
|
||||||
from prometheus_client import Counter
|
from prometheus_client import Counter
|
||||||
|
|
||||||
|
@ -97,9 +98,22 @@ class HttpPusher:
|
||||||
if self.data is None:
|
if self.data is None:
|
||||||
raise PusherConfigException("data can not be null for HTTP pusher")
|
raise PusherConfigException("data can not be null for HTTP pusher")
|
||||||
|
|
||||||
|
# Validate that there's a URL and it is of the proper form.
|
||||||
if "url" not in self.data:
|
if "url" not in self.data:
|
||||||
raise PusherConfigException("'url' required in data for HTTP pusher")
|
raise PusherConfigException("'url' required in data for HTTP pusher")
|
||||||
self.url = self.data["url"]
|
|
||||||
|
url = self.data["url"]
|
||||||
|
if not isinstance(url, str):
|
||||||
|
raise PusherConfigException("'url' must be a string")
|
||||||
|
url_parts = urllib.parse.urlparse(url)
|
||||||
|
# Note that the specification also says the scheme must be HTTPS, but
|
||||||
|
# it isn't up to the homeserver to verify that.
|
||||||
|
if url_parts.path != "/_matrix/push/v1/notify":
|
||||||
|
raise PusherConfigException(
|
||||||
|
"'url' must have a path of '/_matrix/push/v1/notify'"
|
||||||
|
)
|
||||||
|
|
||||||
|
self.url = url
|
||||||
self.http_client = hs.get_proxied_blacklisted_http_client()
|
self.http_client = hs.get_proxied_blacklisted_http_client()
|
||||||
self.data_minus_url = {}
|
self.data_minus_url = {}
|
||||||
self.data_minus_url.update(self.data)
|
self.data_minus_url.update(self.data)
|
||||||
|
|
|
@ -18,6 +18,7 @@ from twisted.internet.defer import Deferred
|
||||||
|
|
||||||
import synapse.rest.admin
|
import synapse.rest.admin
|
||||||
from synapse.logging.context import make_deferred_yieldable
|
from synapse.logging.context import make_deferred_yieldable
|
||||||
|
from synapse.push import PusherConfigException
|
||||||
from synapse.rest.client.v1 import login, room
|
from synapse.rest.client.v1 import login, room
|
||||||
from synapse.rest.client.v2_alpha import receipts
|
from synapse.rest.client.v2_alpha import receipts
|
||||||
|
|
||||||
|
@ -34,6 +35,11 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
user_id = True
|
user_id = True
|
||||||
hijack_auth = False
|
hijack_auth = False
|
||||||
|
|
||||||
|
def default_config(self):
|
||||||
|
config = super().default_config()
|
||||||
|
config["start_pushers"] = True
|
||||||
|
return config
|
||||||
|
|
||||||
def make_homeserver(self, reactor, clock):
|
def make_homeserver(self, reactor, clock):
|
||||||
self.push_attempts = []
|
self.push_attempts = []
|
||||||
|
|
||||||
|
@ -46,15 +52,49 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
m.post_json_get_json = post_json_get_json
|
m.post_json_get_json = post_json_get_json
|
||||||
|
|
||||||
config = self.default_config()
|
hs = self.setup_test_homeserver(proxied_blacklisted_http_client=m)
|
||||||
config["start_pushers"] = True
|
|
||||||
|
|
||||||
hs = self.setup_test_homeserver(
|
|
||||||
config=config, proxied_blacklisted_http_client=m
|
|
||||||
)
|
|
||||||
|
|
||||||
return hs
|
return hs
|
||||||
|
|
||||||
|
def test_invalid_configuration(self):
|
||||||
|
"""Invalid push configurations should be rejected."""
|
||||||
|
# Register the user who gets notified
|
||||||
|
user_id = self.register_user("user", "pass")
|
||||||
|
access_token = self.login("user", "pass")
|
||||||
|
|
||||||
|
# Register the pusher
|
||||||
|
user_tuple = self.get_success(
|
||||||
|
self.hs.get_datastore().get_user_by_access_token(access_token)
|
||||||
|
)
|
||||||
|
token_id = user_tuple.token_id
|
||||||
|
|
||||||
|
def test_data(data):
|
||||||
|
self.get_failure(
|
||||||
|
self.hs.get_pusherpool().add_pusher(
|
||||||
|
user_id=user_id,
|
||||||
|
access_token=token_id,
|
||||||
|
kind="http",
|
||||||
|
app_id="m.http",
|
||||||
|
app_display_name="HTTP Push Notifications",
|
||||||
|
device_display_name="pushy push",
|
||||||
|
pushkey="a@example.com",
|
||||||
|
lang=None,
|
||||||
|
data=data,
|
||||||
|
),
|
||||||
|
PusherConfigException,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Data must be provided with a URL.
|
||||||
|
test_data(None)
|
||||||
|
test_data({})
|
||||||
|
test_data({"url": 1})
|
||||||
|
# A bare domain name isn't accepted.
|
||||||
|
test_data({"url": "example.com"})
|
||||||
|
# A URL without a path isn't accepted.
|
||||||
|
test_data({"url": "http://example.com"})
|
||||||
|
# A url with an incorrect path isn't accepted.
|
||||||
|
test_data({"url": "http://example.com/foo"})
|
||||||
|
|
||||||
def test_sends_http(self):
|
def test_sends_http(self):
|
||||||
"""
|
"""
|
||||||
The HTTP pusher will send pushes for each message to a HTTP endpoint
|
The HTTP pusher will send pushes for each message to a HTTP endpoint
|
||||||
|
@ -84,7 +124,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "http://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -119,7 +159,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# One push was attempted to be sent -- it'll be the first message
|
# One push was attempted to be sent -- it'll be the first message
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(self.push_attempts[0][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
self.push_attempts[0][2]["notification"]["content"]["body"], "Hi!"
|
self.push_attempts[0][2]["notification"]["content"]["body"], "Hi!"
|
||||||
)
|
)
|
||||||
|
@ -139,7 +181,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# Now it'll try and send the second push message, which will be the second one
|
# Now it'll try and send the second push message, which will be the second one
|
||||||
self.assertEqual(len(self.push_attempts), 2)
|
self.assertEqual(len(self.push_attempts), 2)
|
||||||
self.assertEqual(self.push_attempts[1][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[1][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
self.push_attempts[1][2]["notification"]["content"]["body"], "There!"
|
self.push_attempts[1][2]["notification"]["content"]["body"], "There!"
|
||||||
)
|
)
|
||||||
|
@ -196,7 +240,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "http://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -232,7 +276,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# Check our push made it with high priority
|
# Check our push made it with high priority
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(self.push_attempts[0][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
||||||
|
|
||||||
# Add yet another person — we want to make this room not a 1:1
|
# Add yet another person — we want to make this room not a 1:1
|
||||||
|
@ -270,7 +316,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
# Advance time a bit, so the pusher will register something has happened
|
# Advance time a bit, so the pusher will register something has happened
|
||||||
self.pump()
|
self.pump()
|
||||||
self.assertEqual(len(self.push_attempts), 2)
|
self.assertEqual(len(self.push_attempts), 2)
|
||||||
self.assertEqual(self.push_attempts[1][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[1][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "high")
|
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "high")
|
||||||
|
|
||||||
def test_sends_high_priority_for_one_to_one_only(self):
|
def test_sends_high_priority_for_one_to_one_only(self):
|
||||||
|
@ -312,7 +360,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "http://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -328,7 +376,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# Check our push made it with high priority — this is a one-to-one room
|
# Check our push made it with high priority — this is a one-to-one room
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(self.push_attempts[0][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
||||||
|
|
||||||
# Yet another user joins
|
# Yet another user joins
|
||||||
|
@ -347,7 +397,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
# Advance time a bit, so the pusher will register something has happened
|
# Advance time a bit, so the pusher will register something has happened
|
||||||
self.pump()
|
self.pump()
|
||||||
self.assertEqual(len(self.push_attempts), 2)
|
self.assertEqual(len(self.push_attempts), 2)
|
||||||
self.assertEqual(self.push_attempts[1][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[1][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
|
|
||||||
# check that this is low-priority
|
# check that this is low-priority
|
||||||
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
|
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
|
||||||
|
@ -394,7 +446,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "http://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -410,7 +462,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# Check our push made it with high priority
|
# Check our push made it with high priority
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(self.push_attempts[0][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
||||||
|
|
||||||
# Send another event, this time with no mention
|
# Send another event, this time with no mention
|
||||||
|
@ -419,7 +473,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
# Advance time a bit, so the pusher will register something has happened
|
# Advance time a bit, so the pusher will register something has happened
|
||||||
self.pump()
|
self.pump()
|
||||||
self.assertEqual(len(self.push_attempts), 2)
|
self.assertEqual(len(self.push_attempts), 2)
|
||||||
self.assertEqual(self.push_attempts[1][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[1][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
|
|
||||||
# check that this is low-priority
|
# check that this is low-priority
|
||||||
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
|
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
|
||||||
|
@ -467,7 +523,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "http://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -487,7 +543,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# Check our push made it with high priority
|
# Check our push made it with high priority
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(self.push_attempts[0][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high")
|
||||||
|
|
||||||
# Send another event, this time as someone without the power of @room
|
# Send another event, this time as someone without the power of @room
|
||||||
|
@ -498,7 +556,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
# Advance time a bit, so the pusher will register something has happened
|
# Advance time a bit, so the pusher will register something has happened
|
||||||
self.pump()
|
self.pump()
|
||||||
self.assertEqual(len(self.push_attempts), 2)
|
self.assertEqual(len(self.push_attempts), 2)
|
||||||
self.assertEqual(self.push_attempts[1][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[1][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
|
|
||||||
# check that this is low-priority
|
# check that this is low-priority
|
||||||
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
|
self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low")
|
||||||
|
@ -572,7 +632,7 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "http://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -591,7 +651,9 @@ class HTTPPusherTests(HomeserverTestCase):
|
||||||
|
|
||||||
# Check our push made it
|
# Check our push made it
|
||||||
self.assertEqual(len(self.push_attempts), 1)
|
self.assertEqual(len(self.push_attempts), 1)
|
||||||
self.assertEqual(self.push_attempts[0][1], "example.com")
|
self.assertEqual(
|
||||||
|
self.push_attempts[0][1], "http://example.com/_matrix/push/v1/notify"
|
||||||
|
)
|
||||||
|
|
||||||
# Check that the unread count for the room is 0
|
# Check that the unread count for the room is 0
|
||||||
#
|
#
|
||||||
|
|
|
@ -67,7 +67,7 @@ class PusherShardTestCase(BaseMultiWorkerStreamTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "https://push.example.com/push"},
|
data={"url": "https://push.example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -109,7 +109,7 @@ class PusherShardTestCase(BaseMultiWorkerStreamTestCase):
|
||||||
http_client_mock.post_json_get_json.assert_called_once()
|
http_client_mock.post_json_get_json.assert_called_once()
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
http_client_mock.post_json_get_json.call_args[0][0],
|
http_client_mock.post_json_get_json.call_args[0][0],
|
||||||
"https://push.example.com/push",
|
"https://push.example.com/_matrix/push/v1/notify",
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
event_id,
|
event_id,
|
||||||
|
@ -161,7 +161,7 @@ class PusherShardTestCase(BaseMultiWorkerStreamTestCase):
|
||||||
http_client_mock2.post_json_get_json.assert_not_called()
|
http_client_mock2.post_json_get_json.assert_not_called()
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
http_client_mock1.post_json_get_json.call_args[0][0],
|
http_client_mock1.post_json_get_json.call_args[0][0],
|
||||||
"https://push.example.com/push",
|
"https://push.example.com/_matrix/push/v1/notify",
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
event_id,
|
event_id,
|
||||||
|
@ -183,7 +183,7 @@ class PusherShardTestCase(BaseMultiWorkerStreamTestCase):
|
||||||
http_client_mock2.post_json_get_json.assert_called_once()
|
http_client_mock2.post_json_get_json.assert_called_once()
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
http_client_mock2.post_json_get_json.call_args[0][0],
|
http_client_mock2.post_json_get_json.call_args[0][0],
|
||||||
"https://push.example.com/push",
|
"https://push.example.com/_matrix/push/v1/notify",
|
||||||
)
|
)
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
event_id,
|
event_id,
|
||||||
|
|
|
@ -1256,7 +1256,7 @@ class PushersRestTestCase(unittest.HomeserverTestCase):
|
||||||
device_display_name="pushy push",
|
device_display_name="pushy push",
|
||||||
pushkey="a@example.com",
|
pushkey="a@example.com",
|
||||||
lang=None,
|
lang=None,
|
||||||
data={"url": "example.com"},
|
data={"url": "https://example.com/_matrix/push/v1/notify"},
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue