Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Validate device_keys for C-S /keys/query requests
Browse files Browse the repository at this point in the history
Closes #10354

A small, not particularly critical fix. I'm interested in seeing if we
can find a more systematic approach though.

Wishlist:
1. Declaratively specify the data we expect
2. Automatic validation
3. Gradually roll this out across synapse
4. don't eat too many CPU cycles
5. type hints available to Python to mypy/PyCharm etc can make use of the data

A quick search mentions jsonschema, fastjsonschema, pydantic. Attrs has
this in but I think it's quite verbose to get validation?
  • Loading branch information
David Robertson committed Aug 12, 2021
1 parent 4a76d01 commit 427c0de
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 1 deletion.
7 changes: 7 additions & 0 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ def error_dict(self):
return cs_error(self.msg, self.errcode)


class InvalidAPICallError(SynapseError):
"""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.
Expand Down
14 changes: 13 additions & 1 deletion synapse/rest/client/v2_alpha/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import logging

from synapse.api.errors import SynapseError
from synapse.api.errors import SynapseError, InvalidAPICallError
from synapse.http.servlet import (
RestServlet,
parse_integer,
Expand Down Expand Up @@ -163,6 +163,18 @@ async def on_POST(self, request):
device_id = requester.device_id
timeout = parse_integer(request, "timeout", 10 * 1000)
body = parse_json_object_from_request(request)

try:
device_keys = body["device_keys"]
except KeyError:
raise InvalidAPICallError("device_keys is a required body parameter")

for mxid, device_ids in device_keys.items():
if not isinstance(device_ids, list):
raise InvalidAPICallError(
f"device_ids for {mxid} should be a list of 0 or more strings, not {device_ids!r}",
)

result = await self.e2e_keys_handler.query_devices(
body, timeout, user_id, device_id
)
Expand Down
77 changes: 77 additions & 0 deletions tests/rest/client/v2_alpha/test_keys.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
from http import HTTPStatus

from synapse.api.errors import Codes
from synapse.rest import admin
from synapse.rest.client.v1 import login
from synapse.rest.client.v2_alpha import keys
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,
)

0 comments on commit 427c0de

Please sign in to comment.