Skip to content

Commit 9ca77da

Browse files
seratchseratch
authored
Fix #454 Add oauth_settings.state_validation_enabled to customize the OAuth flow (#455)
* Fix #454 Add oauth_settings.state_validation_enabled to customize the OAuth flow * Add more tests
1 parent 908daf4 commit 9ca77da

File tree

6 files changed

+237
-70
lines changed

6 files changed

+237
-70
lines changed

‎slack_bolt/oauth/async_oauth_flow.py‎

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -161,30 +161,32 @@ def sqlite3(
161161
# -----------------------------
162162

163163
async def handle_installation(self, request: AsyncBoltRequest) -> BoltResponse:
164-
state = await self.issue_new_state(request)
165-
url = await self.build_authorize_url(state, request)
166-
set_cookie_value = self.settings.state_utils.build_set_cookie_for_new_state(
167-
state
168-
)
164+
set_cookie_value: Optional[str] = None
165+
url = await self.build_authorize_url("", request)
166+
if self.settings.state_validation_enabled is True:
167+
state = await self.issue_new_state(request)
168+
url = await self.build_authorize_url(state, request)
169+
set_cookie_value = self.settings.state_utils.build_set_cookie_for_new_state(
170+
state
171+
)
169172
if self.settings.install_page_rendering_enabled:
170173
html = await self.build_install_page_html(url, request)
171174
return BoltResponse(
172175
status=200,
173176
body=html,
174-
headers={
175-
"Content-Type": "text/html; charset=utf-8",
176-
"Set-Cookie": [set_cookie_value],
177-
},
177+
headers=await self.append_set_cookie_headers(
178+
{"Content-Type": "text/html; charset=utf-8"},
179+
set_cookie_value,
180+
),
178181
)
179182
else:
180183
return BoltResponse(
181184
status=302,
182185
body="",
183-
headers={
184-
"Content-Type": "text/html; charset=utf-8",
185-
"Location": url,
186-
"Set-Cookie": [set_cookie_value],
187-
},
186+
headers=await self.append_set_cookie_headers(
187+
{"Content-Type": "text/html; charset=utf-8", "Location": url},
188+
set_cookie_value,
189+
),
188190
)
189191

190192
# ----------------------
@@ -199,6 +201,13 @@ async def build_authorize_url(self, state: str, request: AsyncBoltRequest) -> st
199201
async def build_install_page_html(self, url: str, request: AsyncBoltRequest) -> str:
200202
return _build_default_install_page_html(url)
201203

204+
async def append_set_cookie_headers(
205+
self, headers: dict, set_cookie_value: Optional[str]
206+
):
207+
if set_cookie_value is not None:
208+
headers["Set-Cookie"] = [set_cookie_value]
209+
return headers
210+
202211
# -----------------------------
203212
# Callback
204213
# -----------------------------
@@ -219,29 +228,30 @@ async def handle_callback(self, request: AsyncBoltRequest) -> BoltResponse:
219228
)
220229

221230
# state parameter verification
222-
state: Optional[str] = request.query.get("state", [None])[0]
223-
if not self.settings.state_utils.is_valid_browser(state, request.headers):
224-
return await self.failure_handler(
225-
AsyncFailureArgs(
226-
request=request,
227-
reason="invalid_browser",
228-
suggested_status_code=400,
229-
settings=self.settings,
230-
default=self.default_callback_options,
231+
if self.settings.state_validation_enabled is True:
232+
state: Optional[str] = request.query.get("state", [None])[0]
233+
if not self.settings.state_utils.is_valid_browser(state, request.headers):
234+
return await self.failure_handler(
235+
AsyncFailureArgs(
236+
request=request,
237+
reason="invalid_browser",
238+
suggested_status_code=400,
239+
settings=self.settings,
240+
default=self.default_callback_options,
241+
)
231242
)
232-
)
233243

234-
valid_state_consumed = await self.settings.state_store.async_consume(state)
235-
if not valid_state_consumed:
236-
return await self.failure_handler(
237-
AsyncFailureArgs(
238-
request=request,
239-
reason="invalid_state",
240-
suggested_status_code=401,
241-
settings=self.settings,
242-
default=self.default_callback_options,
244+
valid_state_consumed = await self.settings.state_store.async_consume(state)
245+
if not valid_state_consumed:
246+
return await self.failure_handler(
247+
AsyncFailureArgs(
248+
request=request,
249+
reason="invalid_state",
250+
suggested_status_code=401,
251+
settings=self.settings,
252+
default=self.default_callback_options,
253+
)
243254
)
244-
)
245255

246256
# run installation
247257
code = request.query.get("code", [None])[0]

‎slack_bolt/oauth/async_oauth_settings.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class AsyncOAuthSettings:
4444
token_rotation_expiration_minutes: int
4545
authorize: AsyncAuthorize
4646
# state parameter related configurations
47+
state_validation_enabled: bool
4748
state_store: AsyncOAuthStateStore
4849
state_cookie_name: str
4950
state_expiration_seconds: int
@@ -76,6 +77,7 @@ def __init__(
7677
installation_store_bot_only: bool = False,
7778
token_rotation_expiration_minutes: int = 120,
7879
# state parameter related configurations
80+
state_validation_enabled: bool = True,
7981
state_store: Optional[AsyncOAuthStateStore] = None,
8082
state_cookie_name: str = OAuthStateUtils.default_cookie_name,
8183
state_expiration_seconds: int = OAuthStateUtils.default_expiration_seconds,
@@ -100,6 +102,7 @@ def __init__(
100102
installation_store: Specify the instance of `InstallationStore` (Default: `FileInstallationStore`)
101103
installation_store_bot_only: Use `InstallationStore#find_bot()` if True (Default: False)
102104
token_rotation_expiration_minutes: Minutes before refreshing tokens (Default: 2 hours)
105+
state_validation_enabled: Set False if your OAuth flow omits the state parameter validation (Default: True)
103106
state_store: Specify the instance of `InstallationStore` (Default: `FileOAuthStateStore`)
104107
state_cookie_name: The cookie name that is set for installers' browser. (Default: "slack-app-oauth-state")
105108
state_expiration_seconds: The seconds that the state value is alive (Default: 600 seconds)
@@ -153,6 +156,7 @@ def __init__(
153156
bot_only=self.installation_store_bot_only,
154157
)
155158
# state parameter related configurations
159+
self.state_validation_enabled = state_validation_enabled
156160
self.state_store = state_store or FileOAuthStateStore(
157161
expiration_seconds=state_expiration_seconds,
158162
client_id=client_id,

‎slack_bolt/oauth/oauth_flow.py‎

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -158,30 +158,33 @@ def sqlite3(
158158
# -----------------------------
159159

160160
def handle_installation(self, request: BoltRequest) -> BoltResponse:
161-
state = self.issue_new_state(request)
162-
url = self.build_authorize_url(state, request)
163-
set_cookie_value = self.settings.state_utils.build_set_cookie_for_new_state(
164-
state
165-
)
161+
set_cookie_value: Optional[str] = None
162+
url = self.build_authorize_url("", request)
163+
if self.settings.state_validation_enabled is True:
164+
state = self.issue_new_state(request)
165+
url = self.build_authorize_url(state, request)
166+
set_cookie_value = self.settings.state_utils.build_set_cookie_for_new_state(
167+
state
168+
)
169+
166170
if self.settings.install_page_rendering_enabled:
167171
html = self.build_install_page_html(url, request)
168172
return BoltResponse(
169173
status=200,
170174
body=html,
171-
headers={
172-
"Content-Type": "text/html; charset=utf-8",
173-
"Set-Cookie": [set_cookie_value],
174-
},
175+
headers=self.append_set_cookie_headers(
176+
{"Content-Type": "text/html; charset=utf-8"},
177+
set_cookie_value,
178+
),
175179
)
176180
else:
177181
return BoltResponse(
178182
status=302,
179183
body="",
180-
headers={
181-
"Content-Type": "text/html; charset=utf-8",
182-
"Location": url,
183-
"Set-Cookie": [set_cookie_value],
184-
},
184+
headers=self.append_set_cookie_headers(
185+
{"Content-Type": "text/html; charset=utf-8", "Location": url},
186+
set_cookie_value,
187+
),
185188
)
186189

187190
# ----------------------
@@ -196,6 +199,11 @@ def build_authorize_url(self, state: str, request: BoltRequest) -> str:
196199
def build_install_page_html(self, url: str, request: BoltRequest) -> str:
197200
return _build_default_install_page_html(url)
198201

202+
def append_set_cookie_headers(self, headers: dict, set_cookie_value: Optional[str]):
203+
if set_cookie_value is not None:
204+
headers["Set-Cookie"] = [set_cookie_value]
205+
return headers
206+
199207
# -----------------------------
200208
# Callback
201209
# -----------------------------
@@ -216,29 +224,30 @@ def handle_callback(self, request: BoltRequest) -> BoltResponse:
216224
)
217225

218226
# state parameter verification
219-
state = request.query.get("state", [None])[0]
220-
if not self.settings.state_utils.is_valid_browser(state, request.headers):
221-
return self.failure_handler(
222-
FailureArgs(
223-
request=request,
224-
reason="invalid_browser",
225-
suggested_status_code=400,
226-
settings=self.settings,
227-
default=self.default_callback_options,
227+
if self.settings.state_validation_enabled is True:
228+
state = request.query.get("state", [None])[0]
229+
if not self.settings.state_utils.is_valid_browser(state, request.headers):
230+
return self.failure_handler(
231+
FailureArgs(
232+
request=request,
233+
reason="invalid_browser",
234+
suggested_status_code=400,
235+
settings=self.settings,
236+
default=self.default_callback_options,
237+
)
228238
)
229-
)
230239

231-
valid_state_consumed = self.settings.state_store.consume(state)
232-
if not valid_state_consumed:
233-
return self.failure_handler(
234-
FailureArgs(
235-
request=request,
236-
reason="invalid_state",
237-
suggested_status_code=401,
238-
settings=self.settings,
239-
default=self.default_callback_options,
240+
valid_state_consumed = self.settings.state_store.consume(state)
241+
if not valid_state_consumed:
242+
return self.failure_handler(
243+
FailureArgs(
244+
request=request,
245+
reason="invalid_state",
246+
suggested_status_code=401,
247+
settings=self.settings,
248+
default=self.default_callback_options,
249+
)
240250
)
241-
)
242251

243252
# run installation
244253
code = request.query.get("code", [None])[0]

‎slack_bolt/oauth/oauth_settings.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class OAuthSettings:
3939
token_rotation_expiration_minutes: int
4040
authorize: Authorize
4141
# state parameter related configurations
42+
state_validation_enabled: bool
4243
state_store: OAuthStateStore
4344
state_cookie_name: str
4445
state_expiration_seconds: int
@@ -71,6 +72,7 @@ def __init__(
7172
installation_store_bot_only: bool = False,
7273
token_rotation_expiration_minutes: int = 120,
7374
# state parameter related configurations
75+
state_validation_enabled: bool = True,
7476
state_store: Optional[OAuthStateStore] = None,
7577
state_cookie_name: str = OAuthStateUtils.default_cookie_name,
7678
state_expiration_seconds: int = OAuthStateUtils.default_expiration_seconds,
@@ -95,6 +97,7 @@ def __init__(
9597
installation_store: Specify the instance of `InstallationStore` (Default: `FileInstallationStore`)
9698
installation_store_bot_only: Use `InstallationStore#find_bot()` if True (Default: False)
9799
token_rotation_expiration_minutes: Minutes before refreshing tokens (Default: 2 hours)
100+
state_validation_enabled: Set False if your OAuth flow omits the state parameter validation (Default: True)
98101
state_store: Specify the instance of `InstallationStore` (Default: `FileOAuthStateStore`)
99102
state_cookie_name: The cookie name that is set for installers' browser. (Default: "slack-app-oauth-state")
100103
state_expiration_seconds: The seconds that the state value is alive (Default: 600 seconds)
@@ -147,6 +150,7 @@ def __init__(
147150
bot_only=self.installation_store_bot_only,
148151
)
149152
# state parameter related configurations
153+
self.state_validation_enabled = state_validation_enabled
150154
self.state_store = state_store or FileOAuthStateStore(
151155
expiration_seconds=state_expiration_seconds,
152156
client_id=client_id,

‎tests/slack_bolt/oauth/test_oauth_flow.py‎

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
from slack_sdk import WebClient
66
from slack_sdk.oauth.installation_store import FileInstallationStore
7-
from slack_sdk.oauth.state_store import FileOAuthStateStore
7+
from slack_sdk.oauth.state_store import (
8+
OAuthStateStore,
9+
FileOAuthStateStore,
10+
)
811
from slack_sdk.signature import SignatureVerifier
912

1013
from slack_bolt import BoltRequest, BoltResponse, App
@@ -56,10 +59,11 @@ def test_handle_installation_default(self):
5659
resp = oauth_flow.handle_installation(req)
5760
assert resp.status == 200
5861
assert resp.headers.get("content-type") == ["text/html; charset=utf-8"]
62+
assert resp.headers.get("set-cookie") is not None
5963
assert "https://slack.com/oauth/v2/authorize?state=" in resp.body
6064

6165
# https://github.com/slackapi/bolt-python/issues/183
62-
# For direct install URL suppport
66+
# For direct install URL support
6367
def test_handle_installation_no_rendering(self):
6468
oauth_flow = OAuthFlow(
6569
settings=OAuthSettings(
@@ -77,6 +81,25 @@ def test_handle_installation_no_rendering(self):
7781
assert resp.status == 302
7882
location_header = resp.headers.get("location")[0]
7983
assert "https://slack.com/oauth/v2/authorize?state=" in location_header
84+
assert resp.headers.get("set-cookie") is not None
85+
86+
def test_handle_installation_no_state_validation(self):
87+
oauth_flow = OAuthFlow(
88+
settings=OAuthSettings(
89+
client_id="111.222",
90+
client_secret="xxx",
91+
scopes=["chat:write", "commands"],
92+
user_scopes=["search:read"],
93+
installation_store=FileInstallationStore(),
94+
install_page_rendering_enabled=False, # disabled
95+
state_validation_enabled=False, # disabled
96+
state_store=None,
97+
)
98+
)
99+
req = BoltRequest(body="")
100+
resp = oauth_flow.handle_installation(req)
101+
assert resp.status == 302
102+
assert resp.headers.get("set-cookie") is None
80103

81104
def test_scopes_as_str(self):
82105
settings = OAuthSettings(
@@ -160,6 +183,52 @@ def test_handle_callback_invalid_state(self):
160183
resp = oauth_flow.handle_callback(req)
161184
assert resp.status == 400
162185

186+
def test_handle_callback_already_expired_state(self):
187+
class MyOAuthStateStore(OAuthStateStore):
188+
def issue(self, *args, **kwargs) -> str:
189+
return "expired_one"
190+
191+
def consume(self, state: str) -> bool:
192+
return False
193+
194+
oauth_flow = OAuthFlow(
195+
settings=OAuthSettings(
196+
client_id="111.222",
197+
client_secret="xxx",
198+
scopes=["chat:write", "commands"],
199+
state_store=MyOAuthStateStore(),
200+
)
201+
)
202+
state = oauth_flow.issue_new_state(None)
203+
req = BoltRequest(
204+
body="",
205+
query=f"code=foo&state={state}",
206+
headers={"cookie": [f"{oauth_flow.settings.state_cookie_name}={state}"]},
207+
)
208+
resp = oauth_flow.handle_callback(req)
209+
assert resp.status == 401
210+
211+
def test_handle_callback_no_state_validation(self):
212+
oauth_flow = OAuthFlow(
213+
client=WebClient(base_url=self.mock_api_server_base_url),
214+
settings=OAuthSettings(
215+
client_id="111.222",
216+
client_secret="xxx",
217+
scopes=["chat:write", "commands"],
218+
installation_store=FileInstallationStore(),
219+
state_validation_enabled=False, # disabled
220+
state_store=None,
221+
),
222+
)
223+
state = oauth_flow.issue_new_state(None)
224+
req = BoltRequest(
225+
body="",
226+
query=f"code=foo&state=invalid",
227+
headers={"cookie": [f"{oauth_flow.settings.state_cookie_name}={state}"]},
228+
)
229+
resp = oauth_flow.handle_callback(req)
230+
assert resp.status == 200
231+
163232
def test_handle_callback_using_options(self):
164233
def success(args: SuccessArgs) -> BoltResponse:
165234
assert args.request is not None

0 commit comments

Comments
 (0)