From 5c67ead959b295a3e2bc33277486aab833282303 Mon Sep 17 00:00:00 2001 From: Magnesium12 Date: Wed, 21 Dec 2022 13:23:23 +0530 Subject: [PATCH 1/5] feat: Convert to Flask server. --- bot/github/authenticator.py | 4 ++-- bot/github/parser.py | 6 ++--- bot/slack/runner.py | 11 +++++---- bot/utils/json.py | 10 ++++----- main.py | 45 ++++++++++++++++++++----------------- requirements.txt | 4 ++-- tests/slack/test_runner.py | 16 ++++++------- tests/utils/test_json.py | 4 ++-- 8 files changed, 52 insertions(+), 48 deletions(-) diff --git a/bot/github/authenticator.py b/bot/github/authenticator.py index d31f119..3919a3a 100644 --- a/bot/github/authenticator.py +++ b/bot/github/authenticator.py @@ -4,7 +4,7 @@ import requests import sentry_sdk -from bottle import redirect +from flask import redirect from .base import GitHubBase @@ -33,7 +33,7 @@ def redirect_to_oauth_flow(self, repository: str): f"https://redirect.mdgspace.org/{self.base_url}" f"/github/auth/redirect/{repository}", } - redirect(endpoint + "?" + urllib.parse.urlencode(params)) + return redirect(endpoint + "?" + urllib.parse.urlencode(params)) def set_up_webhooks(self, code: str, repository: str) -> str: try: diff --git a/bot/github/parser.py b/bot/github/parser.py index 1d1fd7f..8392e38 100644 --- a/bot/github/parser.py +++ b/bot/github/parser.py @@ -10,7 +10,7 @@ from typing import Type import sentry_sdk -from bottle import LocalRequest +from flask.wrappers import Request from ..models.github import Commit, EventType, Issue, PullRequest, Ref, Repository, User from ..models.github.event import GitHubEvent @@ -69,7 +69,7 @@ def parse(self, event_type, raw_json) -> GitHubEvent | None: return None - def verify(self, request: LocalRequest) -> tuple[bool, str]: + def verify(self, request: Request) -> tuple[bool, str]: """ Verifies incoming GitHub event. @@ -91,7 +91,7 @@ def verify(self, request: LocalRequest) -> tuple[bool, str]: expected_digest = headers["X-Hub-Signature-256"].split('=', 1)[-1] digest = hmac.new( secret.encode(), - request.body.getvalue(), + request.get_data(), hashlib.sha256, ).hexdigest() is_valid = hmac.compare_digest(expected_digest, digest) diff --git a/bot/slack/runner.py b/bot/slack/runner.py index f24b74b..3e5eb46 100644 --- a/bot/slack/runner.py +++ b/bot/slack/runner.py @@ -5,10 +5,9 @@ import hmac import time import urllib.parse -from io import BytesIO from typing import Any -from bottle import MultiDict, WSGIHeaderDict +from werkzeug.datastructures import Headers, ImmutableMultiDict from ..models.github import EventType, convert_keywords_to_events from ..utils.json import JSON @@ -37,8 +36,8 @@ def __init__( def verify( self, - body: BytesIO, - headers: WSGIHeaderDict, + body: bytes, + headers: Headers, ) -> tuple[bool, str]: """ Checks validity of incoming Slack request. @@ -59,7 +58,7 @@ def verify( return False, "Request is too old" expected_digest = headers["X-Slack-Signature"].split('=', 1)[-1] - sig_basestring = ('v0:' + timestamp + ':').encode() + body.getvalue() + sig_basestring = ('v0:' + timestamp + ':').encode() + body digest = hmac.new(self.secret, sig_basestring, hashlib.sha256).hexdigest() is_valid = hmac.compare_digest(expected_digest, digest) @@ -69,7 +68,7 @@ def verify( return True, "Request is secure and valid" - def run(self, raw_json: MultiDict) -> dict[str, Any] | None: + def run(self, raw_json: ImmutableMultiDict) -> dict[str, Any] | None: """ Runs Slack slash commands sent to the bot. :param raw_json: Slash command data sent by Slack. diff --git a/bot/utils/json.py b/bot/utils/json.py index 72f3389..9960ca2 100644 --- a/bot/utils/json.py +++ b/bot/utils/json.py @@ -4,7 +4,7 @@ from typing import Any -from bottle import MultiDict +from werkzeug.datastructures import ImmutableMultiDict class JSON: @@ -40,10 +40,10 @@ def get(k): return keys[0].upper() @staticmethod - def from_multi_dict(multi_dict: MultiDict): + def from_multi_dict(multi_dict: ImmutableMultiDict): """ - Converts `bottle.MultiDict` to `JSON`. - :param multi_dict: Incoming `MultiDict`. - :return: `JSON` object containing the data from the `MultiDict`. + Converts `werkzeug.datastructures.ImmutableMultiDict` to `JSON`. + :param multi_dict: Incoming `ImmutableMultiDict`. + :return: `JSON` object containing the data from the `ImmutableMultiDict`. """ return JSON({key: multi_dict[key] for key in multi_dict.keys()}) diff --git a/main.py b/main.py index 5bcade3..1a858fb 100644 --- a/main.py +++ b/main.py @@ -1,7 +1,7 @@ """ Execution entrypoint for the project. -Sets up a `bottle` server with three endpoints: "/", "/github/events" and "/slack/commands". +Sets up a `Flask` server with three endpoints: "/", "/github/events" and "/slack/commands". "/" is used for testing and status checks. @@ -17,19 +17,19 @@ from typing import Any, Optional, Union import sentry_sdk -from bottle import get, post, request -from bottle import response as http_response -from bottle import run from dotenv import load_dotenv -from sentry_sdk.integrations.bottle import BottleIntegration +from flask import Flask, make_response, request +from sentry_sdk.integrations.flask import FlaskIntegration from bot.github import GitHubApp from bot.models.github.event import GitHubEvent from bot.slack import SlackBot from bot.utils.log import Logger +app = Flask(__name__) -@get("/") + +@app.route("/") def test_get() -> str: """ First test endpoint. @@ -38,7 +38,7 @@ def test_get() -> str: return "This server is running!" -@post("/") +@app.route("/", methods=['POST']) def test_post() -> str: """ Second test endpoint. @@ -54,7 +54,7 @@ def test_post() -> str: f"I'll guess your name!\nYour name is... {name}!") -@post("/github/events") +@app.route("/github/events", methods=['POST']) def manage_github_events(): """ Uses `GitHubApp` to verify, parse and cast the payload into a `GitHubEvent`. @@ -63,8 +63,9 @@ def manage_github_events(): is_valid_request, error_message = github_app.verify(request) if not is_valid_request: - http_response.status = "400 Bad Request" - return error_message + response = make_response(error_message) + response.status_code = 400 + return response event: Optional[GitHubEvent] = github_app.parse( event_type=request.headers["X-GitHub-Event"], @@ -74,8 +75,12 @@ def manage_github_events(): if event is not None: slack_bot.inform(event) + # After creation of Github-Webhook (through api), + # github sends a ping request to payload uri (for its verification) and expects an HTTP 200 OK response. + return make_response("Correct Payload URI", 200) + -@post("/slack/commands") +@app.route("/slack/commands", methods=['POST']) def manage_slack_commands() -> Union[dict, str, None]: """ Uses a `SlackBot` instance to run the slash command triggered by the user. @@ -84,7 +89,7 @@ def manage_slack_commands() -> Union[dict, str, None]: """ is_valid_request, message = slack_bot.verify( - body=request.body, + body=request.get_data(), headers=request.headers, ) if not is_valid_request: @@ -105,20 +110,20 @@ def manage_slack_commands() -> Union[dict, str, None]: } # Unlike GitHub webhooks, Slack does not send the data in `requests.json`. - # Instead, the data is passed in `request.forms`. - response: dict[str, Any] | None = slack_bot.run(raw_json=request.forms) + # Instead, the data is passed in `request.form`. + response: dict[str, Any] | None = slack_bot.run(raw_json=request.form) return response -@get("/github/auth") +@app.route("/github/auth") def initiate_auth(): - github_app.redirect_to_oauth_flow(request.params.get("repository")) + return github_app.redirect_to_oauth_flow(request.args.get("repository")) -@get("/github/auth/redirect//") +@app.route("/github/auth/redirect//") def complete_auth(owner, repo): return github_app.set_up_webhooks( - code=request.query.get("code"), + code=request.args.get("code"), repository=f"{owner}/{repo}", ) @@ -132,7 +137,7 @@ def complete_auth(owner, repo): if (not debug) and ("SENTRY_DSN" in os.environ): sentry_sdk.init( dsn=os.environ["SENTRY_DSN"], - integrations=[BottleIntegration()], + integrations=[FlaskIntegration()], ) slack_bot = SlackBot( @@ -148,4 +153,4 @@ def complete_auth(owner, repo): client_secret=os.environ["GITHUB_APP_CLIENT_SECRET"], ) - run(host="", port=port, debug=debug) + app.run(host="", port=port, debug=debug) diff --git a/requirements.txt b/requirements.txt index ccc6875..dd28cc0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ -bottle==0.12.23 +Flask==2.2.2 peewee==3.15.4 python-dotenv==0.21.0 requests~=2.28.1 -sentry-sdk[bottle]==1.10.1 +sentry-sdk[flask]==1.12.1 slackclient==2.9.4 diff --git a/tests/slack/test_runner.py b/tests/slack/test_runner.py index 51337fc..e52c5ab 100644 --- a/tests/slack/test_runner.py +++ b/tests/slack/test_runner.py @@ -2,7 +2,7 @@ from unittest import skip from unittest.mock import patch -from bottle import MultiDict +from werkzeug.datastructures import ImmutableMultiDict from bot.models.github import convert_keywords_to_events from bot.models.slack import Channel @@ -32,7 +32,7 @@ def setUp(self): @patch("bot.slack.runner.Storage") def test_run_calls_subscribe(self, MockSubscriptionStorage): - raw_json = MultiDict(self.data["run|calls_subscribe"][0]) + raw_json = ImmutableMultiDict(self.data["run|calls_subscribe"][0]) with patch.object(self.logger, "log_command") as mock_logger: with patch.object(self.runner, "run_subscribe_command") as mock_function: @@ -46,7 +46,7 @@ def test_run_calls_subscribe(self, MockSubscriptionStorage): @patch("bot.slack.runner.Storage") def test_run_calls_unsubscribe(self, MockSubscriptionStorage): - raw_json = MultiDict(self.data["run|calls_unsubscribe"][0]) + raw_json = ImmutableMultiDict(self.data["run|calls_unsubscribe"][0]) with patch.object(self.logger, "log_command") as mock_logger: with patch.object(self.runner, "run_unsubscribe_command") as mock_function: @@ -60,7 +60,7 @@ def test_run_calls_unsubscribe(self, MockSubscriptionStorage): @patch("bot.slack.runner.Storage") def test_run_calls_list(self, _): - raw_json = MultiDict(self.data["run|calls_list"][0]) + raw_json = ImmutableMultiDict(self.data["run|calls_list"][0]) with patch.object(self.logger, "log_command") as mock_logger: with patch.object(self.runner, "run_list_command") as mock_function: @@ -71,7 +71,7 @@ def test_run_calls_list(self, _): @patch("bot.slack.runner.Storage") def test_run_calls_help(self, _): - raw_json = MultiDict(self.data["run|calls_help"][0]) + raw_json = ImmutableMultiDict(self.data["run|calls_help"][0]) with patch.object(self.logger, "log_command") as mock_logger: with patch.object(self.runner, "run_help_command") as mock_function: @@ -83,13 +83,13 @@ def test_run_calls_help(self, _): def test_run_doesnt_call(self, _): with patch.object(self.logger, "log_command") as mock_logger: # Wrong command - raw_json = MultiDict(self.data["run|doesnt_call"][0]) + raw_json = ImmutableMultiDict(self.data["run|doesnt_call"][0]) self.assertIsNone(self.runner.run(raw_json)) # No args for subscribe or unsubscribe - raw_json = MultiDict(self.data["run|doesnt_call"][1]) + raw_json = ImmutableMultiDict(self.data["run|doesnt_call"][1]) self.assertIsNone(self.runner.run(raw_json)) - raw_json = MultiDict(self.data["run|doesnt_call"][2]) + raw_json = ImmutableMultiDict(self.data["run|doesnt_call"][2]) self.assertIsNone(self.runner.run(raw_json)) mock_logger.assert_not_called() diff --git a/tests/utils/test_json.py b/tests/utils/test_json.py index f8140f9..91820fc 100644 --- a/tests/utils/test_json.py +++ b/tests/utils/test_json.py @@ -1,6 +1,6 @@ import unittest -from bottle import MultiDict +from werkzeug.datastructures import ImmutableMultiDict from bot.utils.json import JSON @@ -32,7 +32,7 @@ def test_getitem_multiple_not_found(self): self.assertEqual("NAME", json["name", "login"]) def test_from_multi_dict(self): - multi_dict = MultiDict({ + multi_dict = ImmutableMultiDict({ "name": "exampleuser", "login": "example_user" }) From ebbfd10de5f70394d52f4b2cc6d0c0a385c46d23 Mon Sep 17 00:00:00 2001 From: Magnesium12 Date: Sun, 25 Dec 2022 21:29:56 +0530 Subject: [PATCH 2/5] chore: Make suggested changes. --- bot/views.py | 6 ++++++ main.py | 31 ++++--------------------------- 2 files changed, 10 insertions(+), 27 deletions(-) create mode 100644 bot/views.py diff --git a/bot/views.py b/bot/views.py new file mode 100644 index 0000000..fc7863b --- /dev/null +++ b/bot/views.py @@ -0,0 +1,6 @@ +def test_get(): + """ + First test endpoint. + :return: Plaintext confirming server status. + """ + return "This server is running!" diff --git a/main.py b/main.py index 1a858fb..93f566f 100644 --- a/main.py +++ b/main.py @@ -21,6 +21,7 @@ from flask import Flask, make_response, request from sentry_sdk.integrations.flask import FlaskIntegration +from bot import views from bot.github import GitHubApp from bot.models.github.event import GitHubEvent from bot.slack import SlackBot @@ -28,30 +29,7 @@ app = Flask(__name__) - -@app.route("/") -def test_get() -> str: - """ - First test endpoint. - :return: Plaintext confirming server status. - """ - return "This server is running!" - - -@app.route("/", methods=['POST']) -def test_post() -> str: - """ - Second test endpoint. - :return: Status confirmation plaintext containing name supplied in request body. - """ - try: - name: str = request.json["name"] - except KeyError: - name: str = "(empty JSON)" - except TypeError: - name: str = "(invalid JSON)" - return (f"This server is working, and to prove it to you, " - f"I'll guess your name!\nYour name is... {name}!") +app.add_url_rule("/", view_func=views.test_get) @app.route("/github/events", methods=['POST']) @@ -74,10 +52,9 @@ def manage_github_events(): if event is not None: slack_bot.inform(event) + return make_response("OK", 200) - # After creation of Github-Webhook (through api), - # github sends a ping request to payload uri (for its verification) and expects an HTTP 200 OK response. - return make_response("Correct Payload URI", 200) + return make_response("Unrecognized Event", 204) @app.route("/slack/commands", methods=['POST']) From 706f90710c03972cd36d5f33d895ebe7048cca8d Mon Sep 17 00:00:00 2001 From: Aditya Rajput <77491630+BURG3R5@users.noreply.github.com> Date: Mon, 26 Dec 2022 19:46:51 +0530 Subject: [PATCH 3/5] refactor: Simplify Flask responses. --- main.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/main.py b/main.py index 93f566f..057db98 100644 --- a/main.py +++ b/main.py @@ -41,9 +41,7 @@ def manage_github_events(): is_valid_request, error_message = github_app.verify(request) if not is_valid_request: - response = make_response(error_message) - response.status_code = 400 - return response + return make_response(error_message, 400) event: Optional[GitHubEvent] = github_app.parse( event_type=request.headers["X-GitHub-Event"], @@ -52,9 +50,9 @@ def manage_github_events(): if event is not None: slack_bot.inform(event) - return make_response("OK", 200) + return "Informed appropriate channels" - return make_response("Unrecognized Event", 204) + return "Unrecognized Event" @app.route("/slack/commands", methods=['POST']) From dd967d2efd86490013b51405ac9964f637cbcddd Mon Sep 17 00:00:00 2001 From: Aditya Rajput <77491630+BURG3R5@users.noreply.github.com> Date: Mon, 26 Dec 2022 19:48:16 +0530 Subject: [PATCH 4/5] chore(deps): Add `Werkzeug` dependency. --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index dd28cc0..b8f0660 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,3 +4,4 @@ python-dotenv==0.21.0 requests~=2.28.1 sentry-sdk[flask]==1.12.1 slackclient==2.9.4 +Werkzeug~=2.2.2 From ed87bdd2cb2f607c476e37974068cb1d1598bc78 Mon Sep 17 00:00:00 2001 From: Aditya Rajput <77491630+BURG3R5@users.noreply.github.com> Date: Mon, 26 Dec 2022 20:11:20 +0530 Subject: [PATCH 5/5] refactor: Update env variables to fit Flask scheme. --- main.py | 5 ++--- samples/.env | 3 ++- samples/.env.dev | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/main.py b/main.py index 057db98..10352fb 100644 --- a/main.py +++ b/main.py @@ -106,8 +106,7 @@ def complete_auth(owner, repo): if __name__ == "__main__": load_dotenv(Path(".") / ".env") - debug = os.environ["DEBUG"] == "1" - port = int(os.environ.get("CONTAINER_PORT", 5000)) + debug = os.environ["FLASK_DEBUG"] == "True" if (not debug) and ("SENTRY_DSN" in os.environ): sentry_sdk.init( @@ -128,4 +127,4 @@ def complete_auth(owner, repo): client_secret=os.environ["GITHUB_APP_CLIENT_SECRET"], ) - app.run(host="", port=port, debug=debug) + app.run(port=int(os.environ.get("CONTAINER_PORT", 5000))) diff --git a/samples/.env b/samples/.env index c3dbe49..0944a75 100644 --- a/samples/.env +++ b/samples/.env @@ -1,6 +1,7 @@ BASE_URL=subdomain.domain.tld/path1/path2 CONTAINER_PORT=5000 -DEBUG=1 +FLASK_APP=main +FLASK_DEBUG=True GITHUB_APP_CLIENT_ID=0123456789abcdefghij GITHUB_APP_CLIENT_SECRET=e2fbe2fbe2fbe2fbe2fbe2e2fbe2fbe2e2fbe2fb GITHUB_WEBHOOK_SECRET=3dbd2c253813c65b296b7acf67470b7e7bc116e3 diff --git a/samples/.env.dev b/samples/.env.dev index 331dec4..b5a18da 100644 --- a/samples/.env.dev +++ b/samples/.env.dev @@ -1,7 +1,8 @@ SLACK_APP_ID=A0101010101 BASE_URL=blah-111-22-333-444.ngrok.io CONTAINER_PORT=5000 -DEBUG=1 +FLASK_APP=main +FLASK_DEBUG=True GITHUB_APP_CLIENT_ID=0123456789abcdefghij GITHUB_APP_CLIENT_SECRET=e2fbe2fbe2fbe2fbe2fbe2e2fbe2fbe2e2fbe2fb GITHUB_WEBHOOK_SECRET=3dbd2c253813c65b296b7acf67470b7e7bc116e3