From 8b7fe5b950eb350e37f6f81e6a1a2814cd8167b5 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 13 Apr 2023 11:36:23 +0900 Subject: [PATCH] Improve the default OAuth page content renderer not to embed external parameters as-is (#1352) --- docs-src/oauth/index.rst | 15 +- integration_tests/samples/oauth/oauth_v2.py | 13 +- .../samples/oauth/oauth_v2_async.py | 13 +- .../samples/oauth/oauth_v2_legacy.py | 224 ------------------ .../samples/openid_connect/flask_example.py | 2 +- .../samples/openid_connect/sanic_example.py | 17 +- .../samples/token_rotation/oauth.py | 2 +- .../samples/token_rotation/oauth_async.py | 6 +- .../token_rotation/oauth_sqlalchemy.py | 2 +- .../samples/token_rotation/oauth_sqlite3.py | 2 +- .../redirect_uri_page_renderer/__init__.py | 9 +- .../redirect_uri_page_renderer/test_init.py | 2 +- 12 files changed, 51 insertions(+), 256 deletions(-) delete mode 100644 integration_tests/samples/oauth/oauth_v2_legacy.py diff --git a/docs-src/oauth/index.rst b/docs-src/oauth/index.rst index 910207bf3..19992361d 100644 --- a/docs-src/oauth/index.rst +++ b/docs-src/oauth/index.rst @@ -34,6 +34,7 @@ The code snippet below demonstrates how to build it using `Flask ' \ + return f'' \ f'' When accessing ``https://(your domain)/slack/install``, you will see "Add to Slack" button in the webpage. You can start the app's installation flow by clicking the button. @@ -90,13 +91,11 @@ The redirection gives you a ``code`` parameter. You can exchange the value for a redirect_uri=redirect_uri, code=request.args["code"] ) - - installed_enterprise = oauth_response.get("enterprise", {}) + installed_enterprise = oauth_response.get("enterprise") or {} is_enterprise_install = oauth_response.get("is_enterprise_install") - installed_team = oauth_response.get("team", {}) - installer = oauth_response.get("authed_user", {}) - incoming_webhook = oauth_response.get("incoming_webhook", {}) - + installed_team = oauth_response.get("team") or {} + installer = oauth_response.get("authed_user") or {} + incoming_webhook = oauth_response.get("incoming_webhook") or {} bot_token = oauth_response.get("access_token") # NOTE: oauth.v2.access doesn't include bot_id in response bot_id = None @@ -137,7 +136,7 @@ The redirection gives you a ``code`` parameter. You can exchange the value for a return make_response(f"Try the installation again (the state value is already expired)", 400) error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) + return make_response(f"Something is wrong with the installation (error: {html.escape(error)})", 400) Token Lookup ************************************************* diff --git a/integration_tests/samples/oauth/oauth_v2.py b/integration_tests/samples/oauth/oauth_v2.py index 764c5af85..5741b4ef8 100644 --- a/integration_tests/samples/oauth/oauth_v2.py +++ b/integration_tests/samples/oauth/oauth_v2.py @@ -1,6 +1,7 @@ # --------------------- # Flask App for Slack OAuth flow # --------------------- +import html # pip3 install flask from flask import Flask, request, make_response @@ -41,7 +42,7 @@ def oauth_start(): state = state_store.issue() url = authorization_url_generator.generate(state) return ( - f'' + f'' f'' ) @@ -57,11 +58,11 @@ def oauth_callback(): oauth_response = client.oauth_v2_access(client_id=client_id, client_secret=client_secret, code=code) logger.info(f"oauth.v2.access response: {oauth_response}") - installed_enterprise = oauth_response.get("enterprise", {}) + installed_enterprise = oauth_response.get("enterprise") or {} is_enterprise_install = oauth_response.get("is_enterprise_install") - installed_team = oauth_response.get("team", {}) - installer = oauth_response.get("authed_user", {}) - incoming_webhook = oauth_response.get("incoming_webhook", {}) + installed_team = oauth_response.get("team") or {} + installer = oauth_response.get("authed_user") or {} + incoming_webhook = oauth_response.get("incoming_webhook") or {} bot_token = oauth_response.get("access_token") # NOTE: oauth.v2.access doesn't include bot_id in response @@ -105,7 +106,7 @@ def oauth_callback(): return redirect_page_renderer.render_failure_page("the state value is already expired") error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) + return redirect_page_renderer.render_failure_page(error) # --------------------- diff --git a/integration_tests/samples/oauth/oauth_v2_async.py b/integration_tests/samples/oauth/oauth_v2_async.py index 3c7e2442a..c43add6f3 100644 --- a/integration_tests/samples/oauth/oauth_v2_async.py +++ b/integration_tests/samples/oauth/oauth_v2_async.py @@ -1,7 +1,7 @@ # --------------------- # Sanic App for Slack OAuth flow # --------------------- - +import html import logging import os from slack_sdk.web.async_client import AsyncWebClient @@ -44,7 +44,7 @@ async def oauth_start(req: Request): url = authorization_url_generator.generate(state) return HTTPResponse( status=200, - body=f'' + body=f'' f'', ) @@ -61,6 +61,7 @@ async def oauth_callback(req: Request): logger.info(f"oauth.v2.access response: {oauth_response}") installed_enterprise = oauth_response.get("enterprise") or {} + is_enterprise_install = oauth_response.get("is_enterprise_install") installed_team = oauth_response.get("team") or {} installer = oauth_response.get("authed_user") or {} incoming_webhook = oauth_response.get("incoming_webhook") or {} @@ -85,6 +86,8 @@ async def oauth_callback(req: Request): incoming_webhook_url=incoming_webhook.get("url"), incoming_webhook_channel_id=incoming_webhook.get("channel_id"), incoming_webhook_configuration_url=incoming_webhook.get("configuration_url"), + is_enterprise_install=is_enterprise_install, + token_type=oauth_response.get("token_type"), ) installation_store.save(installation) html = redirect_page_renderer.render_success_page( @@ -111,7 +114,11 @@ async def oauth_callback(req: Request): ) error = req.args.get("error") if "error" in req.args else "" - return HTTPResponse(status=400, body=f"Something is wrong with the installation (error: {error})") + return HTTPResponse( + status=400, + headers={"Content-Type": "text/html; charset=utf-8"}, + body=redirect_page_renderer.render_failure_page(error), + ) # --------------------- diff --git a/integration_tests/samples/oauth/oauth_v2_legacy.py b/integration_tests/samples/oauth/oauth_v2_legacy.py deleted file mode 100644 index d9a8b1b40..000000000 --- a/integration_tests/samples/oauth/oauth_v2_legacy.py +++ /dev/null @@ -1,224 +0,0 @@ -# --------------------- -# Flask App for Slack OAuth flow -# --------------------- - -# pip3 install flask -from flask import Flask, request, make_response - -app = Flask(__name__) -app.debug = True - -import logging -import os -from uuid import uuid4 -from slack_sdk import WebClient - -client_id = os.environ["SLACK_TEST_CLIENT_ID"] -client_secret = os.environ["SLACK_TEST_CLIENT_SECRET"] -redirect_uri = os.environ["SLACK_TEST_REDIRECT_URI"] -scopes = "app_mentions:read,chat:write" -user_scopes = "search:read" - -logger = logging.getLogger(__name__) - - -class StateService: - def __init__(self): - # no expiration implemented - self.state_values = [] - - def generate(self): - new_value = str(uuid4()) - self.state_values.append(new_value) - return new_value - - def consume(self, state) -> bool: - is_valid = state in self.state_values - if is_valid: - self.state_values.remove(state) - return is_valid - - -class Database: - def __init__(self): - self.tokens = {} - - def save(self, oauth_v2_response): - team_id = oauth_v2_response["team"]["id"] - installer = oauth_v2_response["authed_user"] - self.tokens[team_id] = { - "bot_token": oauth_v2_response["access_token"], - "bot_user_id": oauth_v2_response["bot_user_id"], - "user_id": installer["id"], - "user_token": installer["access_token"] if "access_token" in installer else None, - } - logger.debug(f"all rows: {list(self.tokens.keys())}") - - def find_bot_token(self, team_id: str) -> str: - logger.debug(f"all rows: {list(self.tokens.keys())}, team_id: {team_id}") - installation = self.tokens[team_id] if team_id in self.tokens else None - if installation: - return installation["bot_token"] - else: - return None - - -state_service = StateService() -database = Database() - - -@app.route("/slack/oauth/start", methods=["GET"]) -def oauth_start(): - state = state_service.generate() - return ( - f'' - f'' - ) - - -@app.route("/slack/oauth/callback", methods=["GET"]) -def oauth_callback(): - # Retrieve the auth code and state from the request params - if "code" in request.args: - state = request.args["state"] - if state_service.consume(state): - code = request.args["code"] - client = WebClient() # no prepared token needed for this app - response = client.oauth_v2_access( - client_id=client_id, - client_secret=client_secret, - redirect_uri=redirect_uri, - code=code, - ) - logger.info(f"oauth.v2.access response: {response}") - database.save(response) - return "Thanks for installing this app!" - else: - return make_response(f"Try the installation again (the state value is already expired)", 400) - - error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) - - -# --------------------- -# Flask App for Slack events -# --------------------- - -import hmac -import hashlib -from time import time -import json - - -def verify_slack_request(signing_secret: str, request_body: str, timestamp: str, signature: str) -> bool: - """Slack Request Verification - - For more information: https://github.com/slackapi/python-slack-events-api - """ - - if abs(time() - int(timestamp)) > 60 * 5: - return False - - if hasattr(hmac, "compare_digest"): - req = str.encode("v0:" + str(timestamp) + ":") + request_body - request_hash = "v0=" + hmac.new(str.encode(signing_secret), req, hashlib.sha256).hexdigest() - return hmac.compare_digest(request_hash, signature) - else: - # So, we'll compare the signatures explicitly - req = str.encode("v0:" + str(timestamp) + ":") + request_body - request_hash = "v0=" + hmac.new(str.encode(signing_secret), req, hashlib.sha256).hexdigest() - - if len(request_hash) != len(signature): - return False - result = 0 - if isinstance(request_hash, bytes) and isinstance(signature, bytes): - for x, y in zip(request_hash, signature): - result |= x ^ y - else: - for x, y in zip(request_hash, signature): - result |= ord(x) ^ ord(y) - return result == 0 - - -from slack_sdk.errors import SlackApiError - -signing_secret = os.environ["SLACK_SIGNING_SECRET"] - - -@app.route("/slack/events", methods=["POST"]) -def slack_app(): - if not verify_slack_request( - signing_secret=signing_secret, - request_body=request.get_data(), - timestamp=request.headers.get("X-Slack-Request-Timestamp"), - signature=request.headers.get("X-Slack-Signature"), - ): - return make_response("invalid request", 403) - - if "command" in request.form and request.form["command"] == "/do-something": - trigger_id = request.form["trigger_id"] - try: - team_id = request.form["team_id"] - bot_token = database.find_bot_token(team_id) - logger.debug(f"token: {bot_token}") - if not bot_token: - return make_response("Please install this app first!", 200) - - client = WebClient(token=bot_token) - response = client.views_open( - trigger_id=trigger_id, - view={ - "type": "modal", - "callback_id": "modal-id", - "title": {"type": "plain_text", "text": "Awesome Modal"}, - "submit": {"type": "plain_text", "text": "Submit"}, - "close": {"type": "plain_text", "text": "Cancel"}, - "blocks": [ - { - "type": "input", - "block_id": "b-id", - "label": { - "type": "plain_text", - "text": "Input label", - }, - "element": { - "action_id": "a-id", - "type": "plain_text_input", - }, - } - ], - }, - ) - return make_response("", 200) - except SlackApiError as e: - code = e.response["error"] - return make_response(f"Failed to open a modal due to {code}", 200) - - elif "payload" in request.form: - payload = json.loads(request.form["payload"]) - if payload["type"] == "view_submission" and payload["view"]["callback_id"] == "modal-id": - submitted_data = payload["view"]["state"]["values"] - print(submitted_data) # {'b-id': {'a-id': {'type': 'plain_text_input', 'value': 'your input'}}} - return make_response("", 200) - - return make_response("", 404) - - -if __name__ == "__main__": - # export SLACK_TEST_CLIENT_ID=123.123 - # export SLACK_TEST_CLIENT_SECRET=xxx - # export SLACK_TEST_REDIRECT_URI=https://{yours}.ngrok.io/slack/oauth/callback - # export SLACK_SIGNING_SECRET=*** - # export FLASK_ENV=development - - app.run("localhost", 3000) - - # python3 integration_tests/samples/oauth/oauth_v2.py - # ngrok http 3000 - # https://{yours}.ngrok.io/slack/oauth/start diff --git a/integration_tests/samples/openid_connect/flask_example.py b/integration_tests/samples/openid_connect/flask_example.py index 770b4a2ef..644ed31dd 100644 --- a/integration_tests/samples/openid_connect/flask_example.py +++ b/integration_tests/samples/openid_connect/flask_example.py @@ -91,7 +91,7 @@ def oauth_callback(): return redirect_page_renderer.render_failure_page("The state value is already expired") error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) + return redirect_page_renderer.render_failure_page(error) if __name__ == "__main__": diff --git a/integration_tests/samples/openid_connect/sanic_example.py b/integration_tests/samples/openid_connect/sanic_example.py index bb62f6a9f..a7d0f645e 100644 --- a/integration_tests/samples/openid_connect/sanic_example.py +++ b/integration_tests/samples/openid_connect/sanic_example.py @@ -98,20 +98,27 @@ async def oauth_callback(req: Request): except Exception: logger.exception("Failed to perform openid.connect.token API call") - return redirect_page_renderer.render_failure_page("Failed to perform openid.connect.token API call") + html = redirect_page_renderer.render_failure_page("Failed to perform openid.connect.token API call") + return HTTPResponse( + status=400, + headers={"Content-Type": "text/html; charset=utf-8"}, + body=html, + ) else: html = redirect_page_renderer.render_failure_page("The state value is already expired") return HTTPResponse( status=400, - headers={ - "Content-Type": "text/html; charset=utf-8", - }, + headers={"Content-Type": "text/html; charset=utf-8"}, body=html, ) error = req.args.get("error") if "error" in req.args else "" - return HTTPResponse(status=400, body=f"Something is wrong with the installation (error: {error})") + return HTTPResponse( + status=400, + headers={"Content-Type": "text/html; charset=utf-8"}, + body=redirect_page_renderer.render_failure_page(error), + ) if __name__ == "__main__": diff --git a/integration_tests/samples/token_rotation/oauth.py b/integration_tests/samples/token_rotation/oauth.py index 2f59cc923..4fb977244 100644 --- a/integration_tests/samples/token_rotation/oauth.py +++ b/integration_tests/samples/token_rotation/oauth.py @@ -245,7 +245,7 @@ def oauth_callback(): return redirect_page_renderer.render_failure_page("the state value is already expired") error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) + return redirect_page_renderer.render_failure_page(error) if __name__ == "__main__": diff --git a/integration_tests/samples/token_rotation/oauth_async.py b/integration_tests/samples/token_rotation/oauth_async.py index 5540c4aa3..319c718f9 100644 --- a/integration_tests/samples/token_rotation/oauth_async.py +++ b/integration_tests/samples/token_rotation/oauth_async.py @@ -251,7 +251,11 @@ async def oauth_callback(req: Request): ) error = req.args.get("error") if "error" in req.args else "" - return HTTPResponse(status=400, body=f"Something is wrong with the installation (error: {error})") + return HTTPResponse( + status=400, + headers={"Content-Type": "text/html; charset=utf-8"}, + body=redirect_page_renderer.render_failure_page(error), + ) if __name__ == "__main__": diff --git a/integration_tests/samples/token_rotation/oauth_sqlalchemy.py b/integration_tests/samples/token_rotation/oauth_sqlalchemy.py index cb9fe0e6f..df52beac7 100644 --- a/integration_tests/samples/token_rotation/oauth_sqlalchemy.py +++ b/integration_tests/samples/token_rotation/oauth_sqlalchemy.py @@ -274,7 +274,7 @@ def oauth_callback(): return redirect_page_renderer.render_failure_page("the state value is already expired") error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) + return redirect_page_renderer.render_failure_page(error) if __name__ == "__main__": diff --git a/integration_tests/samples/token_rotation/oauth_sqlite3.py b/integration_tests/samples/token_rotation/oauth_sqlite3.py index 005b64b81..7b7acd989 100644 --- a/integration_tests/samples/token_rotation/oauth_sqlite3.py +++ b/integration_tests/samples/token_rotation/oauth_sqlite3.py @@ -267,7 +267,7 @@ def oauth_callback(): return redirect_page_renderer.render_failure_page("the state value is already expired") error = request.args["error"] if "error" in request.args else "" - return make_response(f"Something is wrong with the installation (error: {error})", 400) + return redirect_page_renderer.render_failure_page(error) if __name__ == "__main__": diff --git a/slack_sdk/oauth/redirect_uri_page_renderer/__init__.py b/slack_sdk/oauth/redirect_uri_page_renderer/__init__.py index 87b943a33..bc630953f 100644 --- a/slack_sdk/oauth/redirect_uri_page_renderer/__init__.py +++ b/slack_sdk/oauth/redirect_uri_page_renderer/__init__.py @@ -1,3 +1,4 @@ +import html from typing import Optional @@ -35,7 +36,7 @@ def render_success_page( return f""" - +