Skip to content

Commit d82de19

Browse files
seratchseratch
authored
Fix #1441 Built-in InstallationStores fail to resolve a valid bot token when both bot and user-only installations co-exist in database tables (#1442)
1 parent c76c275 commit d82de19

File tree

8 files changed

+262
-24
lines changed

8 files changed

+262
-24
lines changed

‎slack_sdk/oauth/installation_store/amazon_s3/__init__.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ def save(self, installation: Installation):
105105
self.logger.debug(f"S3 put_object response: {response}")
106106

107107
def save_bot(self, bot: Bot):
108+
if bot.bot_token is None:
109+
self.logger.debug("Skipped saving a new row because of the absense of bot token in it")
110+
return
111+
108112
none = "none"
109113
e_id = bot.enterprise_id or none
110114
t_id = bot.team_id or none
@@ -215,10 +219,13 @@ def find_installation(
215219
data = json.loads(body)
216220
installation = Installation(**data)
217221

218-
if installation is not None and user_id is not None:
222+
has_user_installation = user_id is not None and installation is not None
223+
no_bot_token_installation = installation is not None and installation.bot_token is None
224+
should_find_bot_installation = has_user_installation or no_bot_token_installation
225+
if should_find_bot_installation:
219226
# Retrieve the latest bot token, just in case
220227
# See also: https://github.com/slackapi/bolt-python/issues/664
221-
latest_bot_installation = self.find_installation(
228+
latest_bot_installation = self.find_bot(
222229
enterprise_id=enterprise_id,
223230
team_id=team_id,
224231
is_enterprise_install=is_enterprise_install,

‎slack_sdk/oauth/installation_store/file/__init__.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ def save(self, installation: Installation):
7777
f.write(entity)
7878

7979
def save_bot(self, bot: Bot):
80+
if bot.bot_token is None:
81+
self.logger.debug("Skipped saving a new row because of the absense of bot token in it")
82+
return
83+
8084
none = "none"
8185
e_id = bot.enterprise_id or none
8286
t_id = bot.team_id or none
@@ -169,10 +173,13 @@ def find_installation(
169173
data = json.loads(f.read())
170174
installation = Installation(**data)
171175

172-
if installation is not None and user_id is not None:
176+
has_user_installation = user_id is not None and installation is not None
177+
no_bot_token_installation = installation is not None and installation.bot_token is None
178+
should_find_bot_installation = has_user_installation or no_bot_token_installation
179+
if should_find_bot_installation:
173180
# Retrieve the latest bot token, just in case
174181
# See also: https://github.com/slackapi/bolt-python/issues/664
175-
latest_bot_installation = self.find_installation(
182+
latest_bot_installation = self.find_bot(
176183
enterprise_id=enterprise_id,
177184
team_id=team_id,
178185
is_enterprise_install=is_enterprise_install,

‎slack_sdk/oauth/installation_store/sqlalchemy/__init__.py‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ def find_bot(
209209
c.client_id == self.client_id,
210210
c.enterprise_id == enterprise_id,
211211
c.team_id == team_id,
212+
c.bot_token.is_not(None), # the latest one that has a bot token
212213
)
213214
)
214215
.order_by(desc(c.installed_at))
@@ -294,25 +295,24 @@ def find_installation(
294295
installed_at=row["installed_at"],
295296
)
296297

297-
if user_id is not None and installation is not None:
298+
has_user_installation = user_id is not None and installation is not None
299+
no_bot_token_installation = installation is not None and installation.bot_token is None
300+
should_find_bot_installation = has_user_installation or no_bot_token_installation
301+
if should_find_bot_installation:
298302
# Retrieve the latest bot token, just in case
299303
# See also: https://github.com/slackapi/bolt-python/issues/664
300-
where_clause = and_(
301-
c.client_id == self.client_id,
302-
c.enterprise_id == enterprise_id,
303-
c.team_id == team_id,
304-
c.bot_token.is_not(None), # the latest one that has a bot token
304+
latest_bot_installation = self.find_bot(
305+
enterprise_id=enterprise_id,
306+
team_id=team_id,
307+
is_enterprise_install=is_enterprise_install,
305308
)
306-
query = self.installations.select().where(where_clause).order_by(desc(c.installed_at)).limit(1)
307-
with self.engine.connect() as conn:
308-
result: object = conn.execute(query)
309-
for row in result.mappings(): # type: ignore
310-
installation.bot_token = row["bot_token"]
311-
installation.bot_id = row["bot_id"]
312-
installation.bot_user_id = row["bot_user_id"]
313-
installation.bot_scopes = row["bot_scopes"]
314-
installation.bot_refresh_token = row["bot_refresh_token"]
315-
installation.bot_token_expires_at = row["bot_token_expires_at"]
309+
if latest_bot_installation is not None and installation.bot_token != latest_bot_installation.bot_token:
310+
installation.bot_id = latest_bot_installation.bot_id
311+
installation.bot_user_id = latest_bot_installation.bot_user_id
312+
installation.bot_token = latest_bot_installation.bot_token
313+
installation.bot_scopes = latest_bot_installation.bot_scopes
314+
installation.bot_refresh_token = latest_bot_installation.bot_refresh_token
315+
installation.bot_token_expires_at = latest_bot_installation.bot_token_expires_at
316316

317317
return installation
318318

‎slack_sdk/oauth/installation_store/sqlite3/__init__.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ def create_tables(self):
5959
enterprise_url text,
6060
team_id text not null default '',
6161
team_name text,
62-
bot_token text not null,
63-
bot_id text not null,
64-
bot_user_id text not null,
62+
bot_token text,
63+
bot_id text,
64+
bot_user_id text,
6565
bot_scopes text,
6666
bot_refresh_token text, -- since v3.8
6767
bot_token_expires_at datetime, -- since v3.8
@@ -224,6 +224,10 @@ def save(self, installation: Installation):
224224
self.save_bot(installation.to_bot())
225225

226226
def save_bot(self, bot: Bot):
227+
if bot.bot_token is None:
228+
self.logger.debug("Skipped saving a new row because of the absense of bot token in it")
229+
return
230+
227231
with self.connect() as conn:
228232
conn.execute(
229233
"""

‎tests/slack_sdk/oauth/installation_store/test_amazon_s3.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,3 +242,58 @@ def test_save_and_find_token_rotation(self):
242242
self.assertIsNone(i)
243243
bot = store.find_bot(enterprise_id="E111", team_id="T222")
244244
self.assertIsNone(bot)
245+
246+
def test_issue_1441_mixing_user_and_bot_installations(self):
247+
store = self.build_store()
248+
249+
bot_installation = Installation(
250+
app_id="A111",
251+
enterprise_id="E111",
252+
team_id="T111",
253+
user_id="U111",
254+
bot_id="B111",
255+
bot_token="xoxb-111",
256+
bot_scopes=["chat:write"],
257+
bot_user_id="U222",
258+
)
259+
store.save(bot_installation)
260+
261+
# find bots
262+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
263+
self.assertIsNotNone(bot)
264+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
265+
self.assertIsNone(bot)
266+
bot = store.find_bot(enterprise_id=None, team_id="T111")
267+
self.assertIsNone(bot)
268+
269+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
270+
self.assertIsNotNone(installation.bot_token)
271+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
272+
self.assertIsNone(installation)
273+
installation = store.find_installation(enterprise_id=None, team_id="T111")
274+
self.assertIsNone(installation)
275+
276+
user_installation = Installation(
277+
app_id="A111",
278+
enterprise_id="E111",
279+
team_id="T111",
280+
user_id="U111",
281+
bot_scopes=["openid"],
282+
user_token="xoxp-111",
283+
)
284+
store.save(user_installation)
285+
286+
# find bots
287+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
288+
self.assertIsNotNone(bot.bot_token)
289+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
290+
self.assertIsNone(bot)
291+
bot = store.find_bot(enterprise_id=None, team_id="T111")
292+
self.assertIsNone(bot)
293+
294+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
295+
self.assertIsNotNone(installation.bot_token)
296+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
297+
self.assertIsNone(installation)
298+
installation = store.find_installation(enterprise_id=None, team_id="T111")
299+
self.assertIsNone(installation)

‎tests/slack_sdk/oauth/installation_store/test_file.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,58 @@ def test_org_installation(self):
148148
self.assertIsNone(i)
149149
bot = store.find_bot(enterprise_id=None, team_id="T222")
150150
self.assertIsNone(bot)
151+
152+
def test_issue_1441_mixing_user_and_bot_installations(self):
153+
store = FileInstallationStore(client_id="111.222")
154+
155+
bot_installation = Installation(
156+
app_id="A111",
157+
enterprise_id="E111",
158+
team_id="T111",
159+
user_id="U111",
160+
bot_id="B111",
161+
bot_token="xoxb-111",
162+
bot_scopes=["chat:write"],
163+
bot_user_id="U222",
164+
)
165+
store.save(bot_installation)
166+
167+
# find bots
168+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
169+
self.assertIsNotNone(bot)
170+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
171+
self.assertIsNone(bot)
172+
bot = store.find_bot(enterprise_id=None, team_id="T111")
173+
self.assertIsNone(bot)
174+
175+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
176+
self.assertIsNotNone(installation.bot_token)
177+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
178+
self.assertIsNone(installation)
179+
installation = store.find_installation(enterprise_id=None, team_id="T111")
180+
self.assertIsNone(installation)
181+
182+
user_installation = Installation(
183+
app_id="A111",
184+
enterprise_id="E111",
185+
team_id="T111",
186+
user_id="U111",
187+
bot_scopes=["openid"],
188+
user_token="xoxp-111",
189+
)
190+
store.save(user_installation)
191+
192+
# find bots
193+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
194+
self.assertIsNotNone(bot.bot_token)
195+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
196+
self.assertIsNone(bot)
197+
bot = store.find_bot(enterprise_id=None, team_id="T111")
198+
self.assertIsNone(bot)
199+
200+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
201+
self.assertIsNotNone(installation.bot_token)
202+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
203+
self.assertIsNone(installation)
204+
installation = store.find_installation(enterprise_id=None, team_id="T111")
205+
self.assertIsNone(installation)

‎tests/slack_sdk/oauth/installation_store/test_sqlalchemy.py‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from slack_sdk.oauth.installation_store.sqlalchemy import SQLAlchemyInstallationStore
88

99

10-
class TestSQLite3(unittest.TestCase):
10+
class TestSQLAlchemy(unittest.TestCase):
1111
engine: Engine
1212

1313
def setUp(self):
@@ -234,3 +234,58 @@ def test_save_and_find_token_rotation(self):
234234
self.assertIsNone(i)
235235
bot = store.find_bot(enterprise_id="E111", team_id="T222")
236236
self.assertIsNone(bot)
237+
238+
def test_issue_1441_mixing_user_and_bot_installations(self):
239+
store = self.store
240+
241+
bot_installation = Installation(
242+
app_id="A111",
243+
enterprise_id="E111",
244+
team_id="T111",
245+
user_id="U111",
246+
bot_id="B111",
247+
bot_token="xoxb-111",
248+
bot_scopes=["chat:write"],
249+
bot_user_id="U222",
250+
)
251+
store.save(bot_installation)
252+
253+
# find bots
254+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
255+
self.assertIsNotNone(bot)
256+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
257+
self.assertIsNone(bot)
258+
bot = store.find_bot(enterprise_id=None, team_id="T111")
259+
self.assertIsNone(bot)
260+
261+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
262+
self.assertIsNotNone(installation.bot_token)
263+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
264+
self.assertIsNone(installation)
265+
installation = store.find_installation(enterprise_id=None, team_id="T111")
266+
self.assertIsNone(installation)
267+
268+
user_installation = Installation(
269+
app_id="A111",
270+
enterprise_id="E111",
271+
team_id="T111",
272+
user_id="U111",
273+
bot_scopes=["openid"],
274+
user_token="xoxp-111",
275+
)
276+
store.save(user_installation)
277+
278+
# find bots
279+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
280+
self.assertIsNotNone(bot.bot_token)
281+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
282+
self.assertIsNone(bot)
283+
bot = store.find_bot(enterprise_id=None, team_id="T111")
284+
self.assertIsNone(bot)
285+
286+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
287+
self.assertIsNotNone(installation.bot_token)
288+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
289+
self.assertIsNone(installation)
290+
installation = store.find_installation(enterprise_id=None, team_id="T111")
291+
self.assertIsNone(installation)

‎tests/slack_sdk/oauth/installation_store/test_sqlite3.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,3 +235,58 @@ def test_save_and_find_token_rotation(self):
235235
self.assertIsNone(i)
236236
bot = store.find_bot(enterprise_id="E111", team_id="T222")
237237
self.assertIsNone(bot)
238+
239+
def test_issue_1441_mixing_user_and_bot_installations(self):
240+
store = SQLite3InstallationStore(database="logs/test.db", client_id="111.222")
241+
242+
bot_installation = Installation(
243+
app_id="A111",
244+
enterprise_id="E111",
245+
team_id="T111",
246+
user_id="U111",
247+
bot_id="B111",
248+
bot_token="xoxb-111",
249+
bot_scopes=["chat:write"],
250+
bot_user_id="U222",
251+
)
252+
store.save(bot_installation)
253+
254+
# find bots
255+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
256+
self.assertIsNotNone(bot)
257+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
258+
self.assertIsNone(bot)
259+
bot = store.find_bot(enterprise_id=None, team_id="T111")
260+
self.assertIsNone(bot)
261+
262+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
263+
self.assertIsNotNone(installation.bot_token)
264+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
265+
self.assertIsNone(installation)
266+
installation = store.find_installation(enterprise_id=None, team_id="T111")
267+
self.assertIsNone(installation)
268+
269+
user_installation = Installation(
270+
app_id="A111",
271+
enterprise_id="E111",
272+
team_id="T111",
273+
user_id="U111",
274+
bot_scopes=["openid"],
275+
user_token="xoxp-111",
276+
)
277+
store.save(user_installation)
278+
279+
# find bots
280+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
281+
self.assertIsNotNone(bot.bot_token)
282+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
283+
self.assertIsNone(bot)
284+
bot = store.find_bot(enterprise_id=None, team_id="T111")
285+
self.assertIsNone(bot)
286+
287+
installation = store.find_installation(enterprise_id="E111", team_id="T111")
288+
self.assertIsNotNone(installation.bot_token)
289+
installation = store.find_installation(enterprise_id="E111", team_id="T222")
290+
self.assertIsNone(installation)
291+
installation = store.find_installation(enterprise_id=None, team_id="T111")
292+
self.assertIsNone(installation)

0 commit comments

Comments
 (0)