From e739bee00a6bec97c24dddfce0c43d75c08e959c Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sun, 13 Jun 2021 15:00:45 +0000 Subject: [PATCH] poll widget: Add server validation. --- zerver/lib/validator.py | 43 ++++++++++++ zerver/lib/widget.py | 24 +++++++ zerver/tests/test_widgets.py | 127 ++++++++++++++++++++++++++++++++++- zerver/views/submessage.py | 17 ++++- 4 files changed, 207 insertions(+), 4 deletions(-) diff --git a/zerver/lib/validator.py b/zerver/lib/validator.py index 143247e9fc..9022b56ccf 100644 --- a/zerver/lib/validator.py +++ b/zerver/lib/validator.py @@ -456,6 +456,49 @@ def check_widget_content(widget_content: object) -> Dict[str, Any]: raise ValidationError("unknown widget type: " + widget_type) +def validate_poll_data(poll_data: object, is_widget_author: bool) -> None: + check_dict([("type", check_string)])("poll data", poll_data) + + assert isinstance(poll_data, dict) + + if poll_data["type"] == "vote": + checker = check_dict_only( + [ + ("type", check_string), + ("key", check_string), + ("vote", check_int_in([1, -1])), + ] + ) + checker("poll data", poll_data) + return + + if poll_data["type"] == "question": + if not is_widget_author: + raise ValidationError("You can't edit a question unless you are the author.") + + checker = check_dict_only( + [ + ("type", check_string), + ("question", check_string), + ] + ) + checker("poll data", poll_data) + return + + if poll_data["type"] == "new_option": + checker = check_dict_only( + [ + ("type", check_string), + ("option", check_string), + ("idx", check_int), + ] + ) + checker("poll data", poll_data) + return + + raise ValidationError(f"Unknown type for poll data: {poll_data['type']}") + + # Converter functions for use with has_request_variables def to_non_negative_int(s: str, max_int_size: int = 2 ** 32 - 1) -> int: x = int(s) diff --git a/zerver/lib/widget.py b/zerver/lib/widget.py index 0bbf122547..c3be37810c 100644 --- a/zerver/lib/widget.py +++ b/zerver/lib/widget.py @@ -79,6 +79,30 @@ def do_widget_post_save_actions(send_request: SendMessageRequest) -> None: send_request.submessages = SubMessage.get_raw_db_rows([message_id]) +def get_widget_type(*, message_id: int) -> Optional[str]: + submessage = ( + SubMessage.objects.filter( + message_id=message_id, + msg_type="widget", + ) + .only("content") + .first() + ) + + if submessage is None: + return None + + try: + data = json.loads(submessage.content) + except Exception: + return None + + try: + return data["widget_type"] + except Exception: + return None + + def is_widget_message(message: Message) -> bool: # Right now all messages that are widgetized use submessage, and vice versa. return message.submessage_set.exists() diff --git a/zerver/tests/test_widgets.py b/zerver/tests/test_widgets.py index 1203e3b152..584395f32a 100644 --- a/zerver/tests/test_widgets.py +++ b/zerver/tests/test_widgets.py @@ -3,11 +3,12 @@ from typing import Any, Dict import orjson from django.core.exceptions import ValidationError +from django.http import HttpResponse from zerver.lib.test_classes import ZulipTestCase from zerver.lib.validator import check_widget_content -from zerver.lib.widget import get_widget_data -from zerver.models import SubMessage +from zerver.lib.widget import get_widget_data, get_widget_type +from zerver.models import SubMessage, UserProfile class WidgetContentTestCase(ZulipTestCase): @@ -223,3 +224,125 @@ class WidgetContentTestCase(ZulipTestCase): submessage = SubMessage.objects.get(message_id=message.id) self.assertEqual(submessage.msg_type, "widget") self.assertEqual(orjson.loads(submessage.content), expected_submessage_content) + + def test_poll_permissions(self) -> None: + cordelia = self.example_user("cordelia") + hamlet = self.example_user("hamlet") + stream_name = "Verona" + content = "/poll Preference?\n\nyes\nno" + + payload = dict( + type="stream", + to=stream_name, + client="test suite", + topic="whatever", + content=content, + ) + result = self.api_post(cordelia, "/api/v1/messages", payload) + self.assert_json_success(result) + + message = self.get_last_message() + + def post(sender: UserProfile, data: Dict[str, object]) -> HttpResponse: + payload = dict( + message_id=message.id, msg_type="widget", content=orjson.dumps(data).decode() + ) + return self.api_post(sender, "/api/v1/submessage", payload) + + result = post(cordelia, dict(type="question", question="Tabs or spaces?")) + self.assert_json_success(result) + + result = post(hamlet, dict(type="question", question="Tabs or spaces?")) + self.assert_json_error(result, "You can't edit a question unless you are the author.") + + def test_poll_type_validation(self) -> None: + sender = self.example_user("cordelia") + stream_name = "Verona" + content = "/poll Preference?\n\nyes\nno" + + payload = dict( + type="stream", + to=stream_name, + client="test suite", + topic="whatever", + content=content, + ) + result = self.api_post(sender, "/api/v1/messages", payload) + self.assert_json_success(result) + + message = self.get_last_message() + + def post_submessage(content: str) -> HttpResponse: + payload = dict( + message_id=message.id, + msg_type="widget", + content=content, + ) + return self.api_post(sender, "/api/v1/submessage", payload) + + def assert_error(content: str, error: str) -> None: + result = post_submessage(content) + self.assert_json_error_contains(result, error) + + assert_error("bogus", "Invalid json for submessage") + assert_error('""', "not a dict") + assert_error("[]", "not a dict") + + assert_error('{"type": "bogus"}', "Unknown type for poll data: bogus") + assert_error('{"type": "vote"}', "key is missing") + assert_error('{"type": "vote", "key": "1,1,", "vote": 99}', "Invalid poll data") + + assert_error('{"type": "question"}', "key is missing") + assert_error('{"type": "question", "question": 7}', "not a string") + + assert_error('{"type": "new_option"}', "key is missing") + assert_error('{"type": "new_option", "idx": 7, "option": 999}', "not a string") + assert_error('{"type": "new_option", "idx": "bogus", "option": "maybe"}', "not an int") + + def assert_success(data: Dict[str, object]) -> None: + content = orjson.dumps(data).decode() + result = post_submessage(content) + self.assert_json_success(result) + + # Note that we only validate for types. The server code may, for, + # example, allow a vote for a non-existing option, and we rely + # on the clients to ignore those. + + assert_success(dict(type="vote", key="1,1", vote=1)) + assert_success(dict(type="new_option", idx=7, option="maybe")) + assert_success(dict(type="question", question="what's for dinner?")) + + def test_get_widget_type(self) -> None: + sender = self.example_user("cordelia") + stream_name = "Verona" + # We test for both trailing and leading spaces, along with blank lines + # for the poll options. + content = "/poll Preference?\n\nyes\nno" + + payload = dict( + type="stream", + to=stream_name, + client="test suite", + topic="whatever", + content=content, + ) + result = self.api_post(sender, "/api/v1/messages", payload) + self.assert_json_success(result) + + message = self.get_last_message() + + [submessage] = SubMessage.objects.filter(message_id=message.id) + + self.assertEqual(get_widget_type(message_id=message.id), "poll") + + submessage.content = "bogus non json" + submessage.save() + self.assertEqual(get_widget_type(message_id=message.id), None) + + submessage.content = '{"bogus": 1}' + submessage.save() + self.assertEqual(get_widget_type(message_id=message.id), None) + + submessage.content = '{"widget_type": "todo"}' + submessage.save() + self.assertEqual(get_widget_type(message_id=message.id), "todo") diff --git a/zerver/views/submessage.py b/zerver/views/submessage.py index 1e58a534e0..de84457e04 100644 --- a/zerver/views/submessage.py +++ b/zerver/views/submessage.py @@ -1,13 +1,16 @@ import orjson +from django.core.exceptions import ValidationError from django.db import transaction from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ from zerver.decorator import REQ, has_request_variables from zerver.lib.actions import do_add_submessage, verify_submessage_sender +from zerver.lib.exceptions import JsonableError from zerver.lib.message import access_message from zerver.lib.response import json_error, json_success -from zerver.lib.validator import check_int +from zerver.lib.validator import check_int, validate_poll_data +from zerver.lib.widget import get_widget_type from zerver.models import UserProfile @@ -30,10 +33,20 @@ def process_submessage( ) try: - orjson.loads(content) + widget_data = orjson.loads(content) except orjson.JSONDecodeError: return json_error(_("Invalid json for submessage")) + widget_type = get_widget_type(message_id=message.id) + + is_widget_author = message.sender_id == user_profile.id + + if widget_type == "poll": + try: + validate_poll_data(poll_data=widget_data, is_widget_author=is_widget_author) + except ValidationError as error: + raise JsonableError(error.message) + do_add_submessage( realm=user_profile.realm, sender_id=user_profile.id,