Repository navigation
Validate device_keys for C-S /keys/query requests #10593
Changes from all commits
427c0de
69116a5
f71b822
c6a24e4
588437c
f4d710b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Reject Client-Server /keys/query requests which provide device_ids incorrectly. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,6 +147,14 @@ def error_dict(self): | |
| return cs_error(self.msg, self.errcode) | ||
|
|
||
|
|
||
| class InvalidAPICallError(SynapseError): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like if we're going to introduce a new subclass here we should consider using it more widely. (Or at least file an issue about it.)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fair. I had in mind #8445 here---e.g. that any systematic approach to validation could make use of this subclass.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that makes sense as doing it as part of that issue! |
||
| """You called an existing API endpoint, but fed that endpoint | ||
| invalid or incomplete data.""" | ||
|
|
||
| def __init__(self, msg: str): | ||
| super().__init__(HTTPStatus.BAD_REQUEST, msg, Codes.BAD_JSON) | ||
|
|
||
|
|
||
| class ProxiedRequestError(SynapseError): | ||
| """An error from a general matrix endpoint, eg. from a proxied Matrix API call. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| from http import HTTPStatus | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs a license header. |
||
|
|
||
| from synapse.api.errors import Codes | ||
| from synapse.rest import admin | ||
| from synapse.rest.client import keys, login | ||
|
|
||
| from tests import unittest | ||
|
|
||
|
|
||
| class KeyQueryTestCase(unittest.HomeserverTestCase): | ||
| servlets = [ | ||
| keys.register_servlets, | ||
| admin.register_servlets_for_client_rest_resource, | ||
| login.register_servlets, | ||
| ] | ||
|
|
||
| def test_rejects_device_id_ice_key_outside_of_list(self): | ||
| self.register_user("alice", "wonderland") | ||
| alice_token = self.login("alice", "wonderland") | ||
| bob = self.register_user("bob", "uncle") | ||
| channel = self.make_request( | ||
| "POST", | ||
| "/_matrix/client/r0/keys/query", | ||
| { | ||
| "device_keys": { | ||
| bob: "device_id1", | ||
| }, | ||
| }, | ||
| alice_token, | ||
| ) | ||
| self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) | ||
| self.assertEqual( | ||
| channel.json_body["errcode"], | ||
| Codes.BAD_JSON, | ||
| channel.result, | ||
| ) | ||
|
|
||
| def test_rejects_device_key_given_as_map_to_bool(self): | ||
| self.register_user("alice", "wonderland") | ||
| alice_token = self.login("alice", "wonderland") | ||
| bob = self.register_user("bob", "uncle") | ||
| channel = self.make_request( | ||
| "POST", | ||
| "/_matrix/client/r0/keys/query", | ||
| { | ||
| "device_keys": { | ||
| bob: { | ||
| "device_id1": True, | ||
| }, | ||
| }, | ||
| }, | ||
| alice_token, | ||
| ) | ||
|
|
||
| self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) | ||
| self.assertEqual( | ||
| channel.json_body["errcode"], | ||
| Codes.BAD_JSON, | ||
| channel.result, | ||
| ) | ||
|
|
||
| def test_requires_device_key(self): | ||
| """`device_keys` is required. We should complain if it's missing.""" | ||
| self.register_user("alice", "wonderland") | ||
| alice_token = self.login("alice", "wonderland") | ||
| channel = self.make_request( | ||
| "POST", | ||
| "/_matrix/client/r0/keys/query", | ||
| {}, | ||
| alice_token, | ||
| ) | ||
| self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) | ||
| self.assertEqual( | ||
| channel.json_body["errcode"], | ||
| Codes.BAD_JSON, | ||
| channel.result, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.