xenofem/python-zulip-api
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

botserver: Validate token before accepting message.

Browse source 
Previously, the botserver would accept any message sent
to it. This was a security hazard, since an attacker could
impersonate arbitrary users with arbitrary messages. We only
want the Zulip instance where a bot is registered to be able
to send out messages for that bot. To do this, this commits
adds a check for the security token associated with each
outgoing webhook bot. For each bot, its token is stored in
the botserverrc file. The server sends the token along with
each message.
This commit is contained in:
Robert Hönig 2018-05-30 11:29:13 +02:00 committed by showell
parent efe5b04374
commit f4e0808a87
3 changed files with 42 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
zulip_botserver/tests/test.conf
View file

@ -2,7 +2,9 @@
key=value
email=helloworld-bot@zulip.com
site=http://localhost
token=abcd1234
[giphy]
key=value2
email=giphy-bot@zulip.com
site=http://localhost
token=abcd1234

35
zulip_botserver/tests/test_server.py
View file

@ -26,13 +26,15 @@ class BotServerTests(BotServerTestCase):
'email': 'helloworld-bot@zulip.com',
'key': '123456789qwertyuiop',
'site': 'http://localhost',
'token': 'abcd1234',
}
}
self.assert_bot_server_response(available_bots=available_bots,
bots_config=bots_config,
event=dict(message={'content': "@**test** test message"},
bot_email='helloworld-bot@zulip.com',
trigger='mention'),
trigger='mention',
token='abcd1234'),
expected_response="beep boop",
check_success=True)
@ -43,17 +45,20 @@ class BotServerTests(BotServerTestCase):
'email': 'helloworld-bot@zulip.com',
'key': '123456789qwertyuiop',
'site': 'http://localhost',
'token': 'abcd1234',
},
'help': {
'email': 'help-bot@zulip.com',
'key': '123456789qwertyuiop',
'site': 'http://localhost',
'token': 'abcd1234',
}
}
self.assert_bot_server_response(available_bots=available_bots,
event=dict(message={'content': "@**test** test message"},
bot_email='helloworld-bot@zulip.com',
trigger='mention'),
trigger='mention',
token='abcd1234'),
expected_response="beep boop",
bots_config=bots_config,
check_success=True)
@ -64,6 +69,7 @@ class BotServerTests(BotServerTestCase):
'email': 'helloworld-bot@zulip.com',
'key': '123456789qwertyuiop',
'site': 'http://localhost',
'token': 'abcd1234',
},
}
self.assert_bot_server_response(available_bots=['helloworld'],
@ -72,6 +78,24 @@ class BotServerTests(BotServerTestCase):
bots_config=bots_config,
check_success=False)
def test_wrong_bot_token(self) -> None:
available_bots = ['helloworld']
bots_config = {
'helloworld': {
'email': 'helloworld-bot@zulip.com',
'key': '123456789qwertyuiop',
'site': 'http://localhost',
'token': 'abcd1234',
}
}
self.assert_bot_server_response(available_bots=available_bots,
bots_config=bots_config,
event=dict(message={'content': "@**test** test message"},
bot_email='helloworld-bot@zulip.com',
trigger='mention',
token='wrongtoken'),
check_success=False)
@mock.patch('logging.error')
@mock.patch('zulip_bots.lib.StateHandler')
def test_wrong_bot_credentials(self, mock_StateHandler: mock.Mock, mock_LoggingError: mock.Mock) -> None:
@ -81,6 +105,7 @@ class BotServerTests(BotServerTestCase):
'email': 'helloworld-bot@zulip.com',
'key': '123456789qwertyuiop',
'site': 'http://localhost',
'token': 'abcd1234',
}
}
# TODO: The following passes mypy, though the six stubs don't match the
@ -93,7 +118,8 @@ class BotServerTests(BotServerTestCase):
available_bots=available_bots,
event=dict(message={'content': "@**test** test message"},
bot_email='helloworld-bot@zulip.com',
trigger='mention'),
trigger='mention',
token='abcd1234'),
bots_config=bots_config))
@mock.patch('sys.argv', ['zulip-bot-server', '--config-file', '/foo/bar/baz.conf'])
@ -115,11 +141,13 @@ class BotServerTests(BotServerTestCase):
'email': 'helloworld-bot@zulip.com',
'key': 'value',
'site': 'http://localhost',
'token': 'abcd1234',
},
'giphy': {
'email': 'giphy-bot@zulip.com',
'key': 'value2',
'site': 'http://localhost',
'token': 'abcd1234',
}
}
assert json.dumps(bot_conf1, sort_keys=True) == json.dumps(expected_config1, sort_keys=True)
@ -129,6 +157,7 @@ class BotServerTests(BotServerTestCase):
'email': 'helloworld-bot@zulip.com',
'key': 'value',
'site': 'http://localhost',
'token': 'abcd1234',
}
}
assert json.dumps(bot_conf2, sort_keys=True) == json.dumps(expected_config2, sort_keys=True)

10
zulip_botserver/zulip_botserver/server.py
View file

@ -6,7 +6,7 @@ import os
from flask import Flask, request
from importlib import import_module
from typing import Any, Dict, Union, List, Optional
from werkzeug.exceptions import BadRequest
from werkzeug.exceptions import BadRequest, Unauthorized
from zulip import Client
from zulip_bots import lib
@ -22,6 +22,7 @@ def read_config_file(config_file_path: str, bot_name: Optional[str]=None) -> Dic
"email": parser.get(section, 'email'),
"key": parser.get(section, 'key'),
"site": parser.get(section, 'site'),
"token": parser.get(section, 'token'),
}
if bot_name is not None:
logging.warning("Single bot mode is enabled")
@ -102,16 +103,21 @@ bots_config = {} # type: Dict[str, Dict[str, str]]
@app.route('/', methods=['POST'])
def handle_bot() -> Union[str, BadRequest]:
def handle_bot() -> Union[str, BadRequest, Unauthorized]:
event = request.get_json(force=True)
for bot_name, config in bots_config.items():
if config['email'] == event['bot_email']:
bot = bot_name
bot_config = config
break
else:
return BadRequest("Cannot find a bot with email {} in the Botserver "
"configuration file. Do the emails in your botserverrc "
"match the bot emails on the server?".format(event['bot_email']))
if bot_config['token'] != event['token']:
return Unauthorized("Request token does not match token found for bot {} in the "
"Botserver configuration file. Do the outgoing webhooks in "
"Zulip point to the right Botserver?".format(event['bot_email']))
lib_module = app.config.get("BOTS_LIB_MODULES", {})[bot]
bot_handler = app.config.get("BOT_HANDLERS", {})[bot]
message_handler = app.config.get("MESSAGE_HANDLERS", {})[bot]