Skip to content

Commit

Permalink
Improve token rotation error handling and installation error text (#861)
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch committed Mar 15, 2023
1 parent 84bb73d commit 8334d17
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 10 deletions.
12 changes: 11 additions & 1 deletion slack_bolt/authorization/async_authorize.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from logging import Logger
from typing import Optional, Callable, Awaitable, Dict, Any, List

from slack_sdk.errors import SlackApiError
from slack_sdk.errors import SlackApiError, SlackTokenRotationError
from slack_sdk.oauth.installation_store import Bot, Installation
from slack_sdk.oauth.installation_store.async_installation_store import (
AsyncInstallationStore,
Expand Down Expand Up @@ -274,6 +274,11 @@ async def __call__(
user_token = refreshed.user_token
user_scopes = refreshed.user_scopes

except SlackTokenRotationError as rotation_error:
# When token rotation fails, it is usually unrecoverable
# So, this built-in middleware gives up continuing with the following middleware and listeners
self.logger.error(f"Failed to rotate tokens due to {rotation_error}")
return None
except NotImplementedError as _:
self.find_installation_available = False

Expand Down Expand Up @@ -307,6 +312,11 @@ async def __call__(
bot_token = refreshed.bot_token
bot_scopes = refreshed.bot_scopes

except SlackTokenRotationError as rotation_error:
# When token rotation fails, it is usually unrecoverable
# So, this built-in middleware gives up continuing with the following middleware and listeners
self.logger.error(f"Failed to rotate tokens due to {rotation_error}")
return None
except NotImplementedError as _:
self.find_bot_available = False
except Exception as e:
Expand Down
12 changes: 11 additions & 1 deletion slack_bolt/authorization/authorize.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from logging import Logger
from typing import Optional, Callable, Dict, Any, List

from slack_sdk.errors import SlackApiError
from slack_sdk.errors import SlackApiError, SlackTokenRotationError
from slack_sdk.oauth import InstallationStore
from slack_sdk.oauth.installation_store.models.bot import Bot
from slack_sdk.oauth.installation_store.models.installation import Installation
Expand Down Expand Up @@ -271,6 +271,11 @@ def __call__(
user_token = refreshed.user_token
user_scopes = refreshed.user_scopes

except SlackTokenRotationError as rotation_error:
# When token rotation fails, it is usually unrecoverable
# So, this built-in middleware gives up continuing with the following middleware and listeners
self.logger.error(f"Failed to rotate tokens due to {rotation_error}")
return None
except NotImplementedError as _:
self.find_installation_available = False

Expand Down Expand Up @@ -304,6 +309,11 @@ def __call__(
bot_token = refreshed.bot_token
bot_scopes = refreshed.bot_scopes

except SlackTokenRotationError as rotation_error:
# When token rotation fails, it is usually unrecoverable
# So, this built-in middleware gives up continuing with the following middleware and listeners
self.logger.error(f"Failed to rotate tokens due to {rotation_error}")
return None
except NotImplementedError as _:
self.find_bot_available = False
except Exception as e:
Expand Down
3 changes: 2 additions & 1 deletion slack_bolt/middleware/authorization/async_internals.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from slack_bolt.middleware.authorization.internals import _build_error_text
from slack_bolt.request.async_request import AsyncBoltRequest
from slack_bolt.response import BoltResponse

Expand All @@ -18,5 +19,5 @@ def _build_error_response() -> BoltResponse:
# show an ephemeral message to the end-user
return BoltResponse(
status=200,
body=":x: Please install this app into the workspace :bow:",
body=_build_error_text(),
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from slack_bolt.response import BoltResponse
from .async_authorization import AsyncAuthorization
from .async_internals import _build_error_response, _is_no_auth_required
from .internals import _is_no_auth_test_call_required
from .internals import _is_no_auth_test_call_required, _build_error_text
from ...authorization import AuthorizeResult
from ...authorization.async_authorize import AsyncAuthorize

Expand Down Expand Up @@ -91,6 +91,9 @@ async def async_process(
"Although the app should be installed into this workspace, "
"the AuthorizeResult (returned value from authorize) for it was not found."
)
if req.context.response_url is not None:
await req.context.respond(_build_error_text())
return BoltResponse(status=200, body="")
return _build_error_response()

except SlackApiError as e:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from slack_sdk.web.async_slack_response import AsyncSlackResponse
from slack_sdk.errors import SlackApiError
from .async_internals import _build_error_response, _is_no_auth_required
from .internals import _to_authorize_result, _is_no_auth_test_call_required
from .internals import _to_authorize_result, _is_no_auth_test_call_required, _build_error_text
from ...authorization import AuthorizeResult


Expand Down Expand Up @@ -57,6 +57,9 @@ async def async_process(
else:
# Just in case
self.logger.error("auth.test API call result is unexpectedly None")
if req.context.response_url is not None:
await req.context.respond(_build_error_text())
return BoltResponse(status=200, body="")
return _build_error_response()
except SlackApiError as e:
self.logger.error(f"Failed to authorize with the given token ({e})")
Expand Down
9 changes: 8 additions & 1 deletion slack_bolt/middleware/authorization/internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,18 @@ def _is_no_auth_test_call_required(req: Union[BoltRequest, "AsyncBoltRequest"])
return _is_no_auth_test_events(req)


def _build_error_text() -> str:
return (
":warning: We apologize, but for some unknown reason, your installation with this app is no longer available. "
"Please reinstall this app into your workspace :bow:"
)


def _build_error_response() -> BoltResponse:
# show an ephemeral message to the end-user
return BoltResponse(
status=200,
body=":x: Please install this app into the workspace :bow:",
body=_build_error_text(),
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
_build_error_response,
_is_no_auth_required,
_is_no_auth_test_call_required,
_build_error_text,
)
from ...authorization import AuthorizeResult
from ...authorization.authorize import Authorize
Expand Down Expand Up @@ -93,6 +94,9 @@ def process(
"Although the app should be installed into this workspace, "
"the AuthorizeResult (returned value from authorize) for it was not found."
)
if req.context.response_url is not None:
req.context.respond(_build_error_text())
return BoltResponse(status=200, body="")
return _build_error_response()

except SlackApiError as e:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
_is_no_auth_required,
_to_authorize_result,
_is_no_auth_test_call_required,
_build_error_text,
)
from ...authorization import AuthorizeResult

Expand Down Expand Up @@ -71,6 +72,9 @@ def process(
else:
# Just in case
self.logger.error("auth.test API call result is unexpectedly None")
if req.context.response_url is not None:
req.context.respond(_build_error_text())
return BoltResponse(status=200, body="")
return _build_error_response()
except SlackApiError as e:
self.logger.error(f"Failed to authorize with the given token ({e})")
Expand Down
2 changes: 1 addition & 1 deletion tests/scenario_tests/test_authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def test_failure(self):
request = self.build_valid_request()
response = app.dispatch(request)
assert response.status == 200
assert response.body == ":x: Please install this app into the workspace :bow:"
assert response.body == ""
assert self.mock_received_requests.get("/auth.test") == None

def test_bot_context_attributes(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/scenario_tests_async/test_authorize.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ async def test_failure(self):
request = self.build_valid_request()
response = await app.async_dispatch(request)
assert response.status == 200
assert response.body == ":x: Please install this app into the workspace :bow:"
assert response.body == ""
assert self.mock_received_requests.get("/auth.test") == None

@pytest.mark.asyncio
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from slack_sdk import WebClient

from slack_bolt.middleware import SingleTeamAuthorization
from slack_bolt.middleware.authorization.internals import _build_error_text
from slack_bolt.request import BoltRequest
from slack_bolt.response import BoltResponse
from tests.mock_web_api_server import (
Expand Down Expand Up @@ -42,4 +43,4 @@ def test_failure_pattern(self):
resp = authorization.process(req=req, resp=resp, next=next)

assert resp.status == 200
assert resp.body == ":x: Please install this app into the workspace :bow:"
assert resp.body == _build_error_text()
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from slack_bolt.middleware.authorization.async_single_team_authorization import (
AsyncSingleTeamAuthorization,
)
from slack_bolt.middleware.authorization.internals import _build_error_text
from slack_bolt.request.async_request import AsyncBoltRequest
from slack_bolt.response import BoltResponse
from tests.mock_web_api_server import (
Expand Down Expand Up @@ -56,4 +57,4 @@ async def test_failure_pattern(self):
resp = await authorization.async_process(req=req, resp=resp, next=next)

assert resp.status == 200
assert resp.body == ":x: Please install this app into the workspace :bow:"
assert resp.body == _build_error_text()

0 comments on commit 8334d17

Please sign in to comment.