mirror of
https://github.com/zulip/zulip.git
synced 2025-11-09 08:26:11 +00:00
slack: Provide more information when a Slack token fails to validate.
(cherry picked from commit 4c8915c8e4)
This commit is contained in:
@@ -1466,8 +1466,13 @@ def check_token_access(token: str) -> None:
|
|||||||
data = requests.get(
|
data = requests.get(
|
||||||
"https://slack.com/api/team.info", headers={"Authorization": f"Bearer {token}"}
|
"https://slack.com/api/team.info", headers={"Authorization": f"Bearer {token}"}
|
||||||
)
|
)
|
||||||
if data.status_code != 200 or not data.json()["ok"]:
|
if data.status_code != 200:
|
||||||
raise ValueError(f"Invalid Slack token: {token}")
|
raise ValueError(
|
||||||
|
f"Failed to fetch data (HTTP status {data.status_code}) for Slack token: {token}"
|
||||||
|
)
|
||||||
|
if not data.json()["ok"]:
|
||||||
|
error = data.json()["error"]
|
||||||
|
raise ValueError(f"Invalid Slack token: {token}, {error}")
|
||||||
has_scopes = set(data.headers.get("x-oauth-scopes", "").split(","))
|
has_scopes = set(data.headers.get("x-oauth-scopes", "").split(","))
|
||||||
required_scopes = {"emoji:read", "users:read", "users:read.email", "team:read"}
|
required_scopes = {"emoji:read", "users:read", "users:read.email", "team:read"}
|
||||||
missing_scopes = required_scopes - has_scopes
|
missing_scopes = required_scopes - has_scopes
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ from zerver.data_import.slack import (
|
|||||||
SlackBotEmail,
|
SlackBotEmail,
|
||||||
channel_message_to_zerver_message,
|
channel_message_to_zerver_message,
|
||||||
channels_to_zerver_stream,
|
channels_to_zerver_stream,
|
||||||
|
check_token_access,
|
||||||
convert_slack_workspace_messages,
|
convert_slack_workspace_messages,
|
||||||
do_convert_data,
|
do_convert_data,
|
||||||
fetch_shared_channel_users,
|
fetch_shared_channel_users,
|
||||||
@@ -181,6 +182,62 @@ class SlackImporter(ZulipTestCase):
|
|||||||
self.assertEqual(test_zerver_realm_dict["name"], realm_subdomain)
|
self.assertEqual(test_zerver_realm_dict["name"], realm_subdomain)
|
||||||
self.assertEqual(test_zerver_realm_dict["date_created"], time)
|
self.assertEqual(test_zerver_realm_dict["date_created"], time)
|
||||||
|
|
||||||
|
@responses.activate
|
||||||
|
def test_check_token_access(self) -> None:
|
||||||
|
def token_request_callback(request: PreparedRequest) -> Tuple[int, Dict[str, str], bytes]:
|
||||||
|
auth = request.headers.get("Authorization")
|
||||||
|
if auth == "Bearer xoxb-broken-request":
|
||||||
|
return (400, {}, orjson.dumps({"ok": False, "error": "invalid_auth"}))
|
||||||
|
|
||||||
|
if auth == "Bearer xoxb-invalid-token":
|
||||||
|
return (200, {}, orjson.dumps({"ok": False, "error": "invalid_auth"}))
|
||||||
|
|
||||||
|
if auth == "Bearer xoxb-limited-scopes":
|
||||||
|
return (
|
||||||
|
200,
|
||||||
|
{"x-oauth-scopes": "emoji:read,bogus:scope"},
|
||||||
|
orjson.dumps({"ok": True}),
|
||||||
|
)
|
||||||
|
if auth == "Bearer xoxb-valid-token":
|
||||||
|
return (
|
||||||
|
200,
|
||||||
|
{"x-oauth-scopes": "emoji:read,users:read,users:read.email,team:read"},
|
||||||
|
orjson.dumps({"ok": True}),
|
||||||
|
)
|
||||||
|
else: # nocoverage
|
||||||
|
raise Exception("Unknown token mock")
|
||||||
|
|
||||||
|
responses.add_callback(
|
||||||
|
responses.GET, "https://slack.com/api/team.info", callback=token_request_callback
|
||||||
|
)
|
||||||
|
|
||||||
|
def exception_for(token: str) -> str:
|
||||||
|
with self.assertRaises(Exception) as invalid:
|
||||||
|
check_token_access(token)
|
||||||
|
return invalid.exception.args[0]
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
exception_for("xoxq-unknown"),
|
||||||
|
"Unknown token type -- must start with xoxb- or xoxp-",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
exception_for("xoxb-invalid-token"),
|
||||||
|
"Invalid Slack token: xoxb-invalid-token, invalid_auth",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
exception_for("xoxb-broken-request"),
|
||||||
|
"Failed to fetch data (HTTP status 400) for Slack token: xoxb-broken-request",
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(
|
||||||
|
exception_for("xoxb-limited-scopes"),
|
||||||
|
"Slack token is missing the following required scopes: ['team:read', 'users:read', 'users:read.email']",
|
||||||
|
)
|
||||||
|
|
||||||
|
check_token_access("xoxb-valid-token")
|
||||||
|
|
||||||
def test_get_owner(self) -> None:
|
def test_get_owner(self) -> None:
|
||||||
user_data = [
|
user_data = [
|
||||||
{"is_owner": False, "is_primary_owner": False},
|
{"is_owner": False, "is_primary_owner": False},
|
||||||
@@ -1186,8 +1243,10 @@ class SlackImporter(ZulipTestCase):
|
|||||||
@mock.patch("zerver.data_import.slack.build_avatar_url")
|
@mock.patch("zerver.data_import.slack.build_avatar_url")
|
||||||
@mock.patch("zerver.data_import.slack.build_avatar")
|
@mock.patch("zerver.data_import.slack.build_avatar")
|
||||||
@mock.patch("zerver.data_import.slack.get_slack_api_data")
|
@mock.patch("zerver.data_import.slack.get_slack_api_data")
|
||||||
|
@mock.patch("zerver.data_import.slack.check_token_access")
|
||||||
def test_slack_import_to_existing_database(
|
def test_slack_import_to_existing_database(
|
||||||
self,
|
self,
|
||||||
|
mock_check_token_access: mock.Mock,
|
||||||
mock_get_slack_api_data: mock.Mock,
|
mock_get_slack_api_data: mock.Mock,
|
||||||
mock_build_avatar_url: mock.Mock,
|
mock_build_avatar_url: mock.Mock,
|
||||||
mock_build_avatar: mock.Mock,
|
mock_build_avatar: mock.Mock,
|
||||||
@@ -1385,8 +1444,10 @@ class SlackImporter(ZulipTestCase):
|
|||||||
@mock.patch("zerver.data_import.slack.build_avatar_url")
|
@mock.patch("zerver.data_import.slack.build_avatar_url")
|
||||||
@mock.patch("zerver.data_import.slack.build_avatar")
|
@mock.patch("zerver.data_import.slack.build_avatar")
|
||||||
@mock.patch("zerver.data_import.slack.get_slack_api_data")
|
@mock.patch("zerver.data_import.slack.get_slack_api_data")
|
||||||
|
@mock.patch("zerver.data_import.slack.check_token_access")
|
||||||
def test_slack_import_unicode_filenames(
|
def test_slack_import_unicode_filenames(
|
||||||
self,
|
self,
|
||||||
|
mock_check_token_access: mock.Mock,
|
||||||
mock_get_slack_api_data: mock.Mock,
|
mock_get_slack_api_data: mock.Mock,
|
||||||
mock_build_avatar_url: mock.Mock,
|
mock_build_avatar_url: mock.Mock,
|
||||||
mock_build_avatar: mock.Mock,
|
mock_build_avatar: mock.Mock,
|
||||||
|
|||||||
Reference in New Issue
Block a user