Skip to content

Commit c2b5c29

Browse files
seratchseratch
authored
Add more tests & fix a data deletion bug in AmazonS3InstallationStore (#1081)
1 parent 57d8dd6 commit c2b5c29

7 files changed

Lines changed: 376 additions & 5 deletions

File tree

‎setup.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@
2828
"black==21.7b0",
2929
"psutil>=5,<6",
3030
"databases>=0.3",
31+
# used only under slack_sdk/*_store
32+
"boto3<=2",
33+
# TODO: Upgrade to v2
34+
"moto<2", # For AWS tests
3135
]
3236
codegen_dependencies = [
3337
"black==21.7b0",

‎slack_sdk/errors/__init__.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,7 @@ class SlackObjectFormationError(SlackClientError):
5252

5353

5454
class SlackClientConfigurationError(SlackClientError):
55-
"""Error raised when attempting to send messages over the websocket when the
56-
connection is closed."""
55+
"""Error raised because of invalid configuration on the client side:
56+
* when attempting to send messages over the websocket when the connection is closed.
57+
* when external system (e.g., Amazon S3) configuration / credentials are not correct
58+
"""

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

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from botocore.client import BaseClient
77

8+
from slack_sdk.errors import SlackClientConfigurationError
89
from slack_sdk.oauth.installation_store.async_installation_store import (
910
AsyncInstallationStore,
1011
)
@@ -252,7 +253,7 @@ def delete_bot(
252253
)
253254
except Exception as e: # skipcq: PYL-W0703
254255
message = f"Failed to find bot installation data for enterprise: {e_id}, team: {t_id}: {e}"
255-
self.logger.warning(message)
256+
raise SlackClientConfigurationError(message)
256257

257258
async def async_delete_installation(
258259
self,
@@ -282,15 +283,52 @@ def delete_installation(
282283
Bucket=self.bucket_name,
283284
Prefix=f"{workspace_path}/installer-{user_id or ''}",
284285
)
286+
deleted_keys = []
285287
for content in objects.get("Contents", []):
286288
key = content.get("Key")
287289
if key is not None:
288290
self.logger.info(f"Going to delete installation ({key})")
289291
try:
290292
self.s3_client.delete_object(
291293
Bucket=self.bucket_name,
292-
Key=content.get("Key"),
294+
Key=key,
293295
)
296+
deleted_keys.append(key)
297+
except Exception as e: # skipcq: PYL-W0703
298+
message = f"Failed to find bot installation data for enterprise: {e_id}, team: {t_id}: {e}"
299+
raise SlackClientConfigurationError(message)
300+
301+
try:
302+
no_user_id_key = key.replace(f"-{user_id}", "")
303+
if not no_user_id_key.endswith("installer-latest"):
304+
self.s3_client.delete_object(
305+
Bucket=self.bucket_name,
306+
Key=no_user_id_key,
307+
)
308+
deleted_keys.append(no_user_id_key)
294309
except Exception as e: # skipcq: PYL-W0703
295310
message = f"Failed to find bot installation data for enterprise: {e_id}, team: {t_id}: {e}"
296-
self.logger.warning(message)
311+
raise SlackClientConfigurationError(message)
312+
313+
# Check the remaining installation data
314+
objects = self.s3_client.list_objects(
315+
Bucket=self.bucket_name,
316+
Prefix=f"{workspace_path}/installer-",
317+
MaxKeys=10, # the small number would be enough for this purpose
318+
)
319+
keys = [
320+
c.get("Key")
321+
for c in objects.get("Contents", [])
322+
if c.get("Key") not in deleted_keys
323+
]
324+
# If only installer-latest remains, we should delete the one as well
325+
if len(keys) == 1 and keys[0].endswith("installer-latest"):
326+
content = objects.get("Contents", [])[0]
327+
try:
328+
self.s3_client.delete_object(
329+
Bucket=self.bucket_name,
330+
Key=content.get("Key"),
331+
)
332+
except Exception as e: # skipcq: PYL-W0703
333+
message = f"Failed to find bot installation data for enterprise: {e_id}, team: {t_id}: {e}"
334+
raise SlackClientConfigurationError(message)

‎tests/slack_sdk/audit_logs/mock_web_api_server.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ def do_GET(self):
4141
return
4242

4343
try:
44+
if self.path == "/error":
45+
self.send_response(500)
46+
self.set_common_headers()
47+
self.wfile.write("unexpected response body".encode("utf-8"))
48+
return
49+
4450
if self.path == "/timeout":
4551
time.sleep(2)
4652

‎tests/slack_sdk/audit_logs/test_client.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import unittest
2+
from urllib.error import URLError
23

34
from slack_sdk.audit_logs import AuditLogsClient, AuditLogsResponse
45
from tests.slack_sdk.audit_logs.mock_web_api_server import (
@@ -31,3 +32,14 @@ def test_schemas(self):
3132
resp: AuditLogsResponse = self.client.schemas()
3233
self.assertEqual(200, resp.status_code)
3334
self.assertIsNotNone(resp.body.get("schemas"))
35+
36+
def test_url_error(self):
37+
invalid_url = "http://localhost:9999/"
38+
client = AuditLogsClient(token="xoxp-", base_url=invalid_url)
39+
with self.assertRaises(URLError):
40+
client.logs(limit=1, action="user_login")
41+
42+
def test_http_error(self):
43+
resp: AuditLogsResponse = self.client.api_call(path="error")
44+
self.assertEqual(500, resp.status_code)
45+
self.assertEqual("unexpected response body", resp.raw_body)

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

Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
import unittest
2+
3+
import boto3
4+
from moto import mock_s3
5+
from slack_sdk.oauth.installation_store import Installation
6+
from slack_sdk.oauth.installation_store.amazon_s3 import AmazonS3InstallationStore
7+
8+
9+
class TestAmazonS3(unittest.TestCase):
10+
mock_s3 = mock_s3()
11+
bucket_name = "test-bucket"
12+
13+
def setUp(self):
14+
self.mock_s3.start()
15+
s3 = boto3.resource("s3")
16+
bucket = s3.Bucket(self.bucket_name)
17+
bucket.create(CreateBucketConfiguration={"LocationConstraint": "af-south-1"})
18+
19+
def tearDown(self):
20+
self.mock_s3.stop()
21+
22+
def build_store(self) -> AmazonS3InstallationStore:
23+
return AmazonS3InstallationStore(
24+
s3_client=boto3.client("s3"),
25+
bucket_name=self.bucket_name,
26+
client_id="111.222",
27+
)
28+
29+
def test_instance(self):
30+
store = self.build_store()
31+
self.assertIsNotNone(store)
32+
33+
def test_save_and_find(self):
34+
store = self.build_store()
35+
installation = Installation(
36+
app_id="A111",
37+
enterprise_id="E111",
38+
team_id="T111",
39+
user_id="U111",
40+
bot_id="B111",
41+
bot_token="xoxb-111",
42+
bot_scopes=["chat:write"],
43+
bot_user_id="U222",
44+
)
45+
store.save(installation)
46+
47+
# find bots
48+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
49+
self.assertIsNotNone(bot)
50+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
51+
self.assertIsNone(bot)
52+
bot = store.find_bot(enterprise_id=None, team_id="T111")
53+
self.assertIsNone(bot)
54+
55+
# delete bots
56+
store.delete_bot(enterprise_id="E111", team_id="T222")
57+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
58+
self.assertIsNone(bot)
59+
60+
# find installations
61+
i = store.find_installation(enterprise_id="E111", team_id="T111")
62+
self.assertIsNotNone(i)
63+
i = store.find_installation(enterprise_id="E111", team_id="T222")
64+
self.assertIsNone(i)
65+
i = store.find_installation(enterprise_id=None, team_id="T111")
66+
self.assertIsNone(i)
67+
68+
i = store.find_installation(
69+
enterprise_id="E111", team_id="T111", user_id="U111"
70+
)
71+
self.assertIsNotNone(i)
72+
i = store.find_installation(
73+
enterprise_id="E111", team_id="T111", user_id="U222"
74+
)
75+
self.assertIsNone(i)
76+
i = store.find_installation(
77+
enterprise_id="E111", team_id="T222", user_id="U111"
78+
)
79+
self.assertIsNone(i)
80+
81+
# delete installations
82+
store.delete_installation(enterprise_id="E111", team_id="T111", user_id="U111")
83+
i = store.find_installation(
84+
enterprise_id="E111", team_id="T111", user_id="U111"
85+
)
86+
self.assertIsNone(i)
87+
i = store.find_installation(enterprise_id="E111", team_id="T111")
88+
self.assertIsNone(i)
89+
90+
# delete all
91+
store.save(installation)
92+
store.delete_all(enterprise_id="E111", team_id="T111")
93+
94+
i = store.find_installation(enterprise_id="E111", team_id="T111")
95+
self.assertIsNone(i)
96+
i = store.find_installation(
97+
enterprise_id="E111", team_id="T111", user_id="U111"
98+
)
99+
self.assertIsNone(i)
100+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
101+
self.assertIsNone(bot)
102+
103+
def test_org_installation(self):
104+
store = self.build_store()
105+
installation = Installation(
106+
app_id="AO111",
107+
enterprise_id="EO111",
108+
user_id="UO111",
109+
bot_id="BO111",
110+
bot_token="xoxb-O111",
111+
bot_scopes=["chat:write"],
112+
bot_user_id="UO222",
113+
is_enterprise_install=True,
114+
)
115+
store.save(installation)
116+
117+
# find bots
118+
bot = store.find_bot(enterprise_id="EO111", team_id=None)
119+
self.assertIsNotNone(bot)
120+
bot = store.find_bot(
121+
enterprise_id="EO111", team_id="TO222", is_enterprise_install=True
122+
)
123+
self.assertIsNotNone(bot)
124+
bot = store.find_bot(enterprise_id="EO111", team_id="TO222")
125+
self.assertIsNone(bot)
126+
bot = store.find_bot(enterprise_id=None, team_id="TO111")
127+
self.assertIsNone(bot)
128+
129+
# delete bots
130+
store.delete_bot(enterprise_id="EO111", team_id="TO222")
131+
bot = store.find_bot(enterprise_id="EO111", team_id=None)
132+
self.assertIsNotNone(bot)
133+
134+
store.delete_bot(enterprise_id="EO111", team_id=None)
135+
bot = store.find_bot(enterprise_id="EO111", team_id=None)
136+
self.assertIsNone(bot)
137+
138+
# find installations
139+
i = store.find_installation(enterprise_id="EO111", team_id=None)
140+
self.assertIsNotNone(i)
141+
i = store.find_installation(
142+
enterprise_id="EO111", team_id="T111", is_enterprise_install=True
143+
)
144+
self.assertIsNotNone(i)
145+
i = store.find_installation(enterprise_id="EO111", team_id="T222")
146+
self.assertIsNone(i)
147+
i = store.find_installation(enterprise_id=None, team_id="T111")
148+
self.assertIsNone(i)
149+
150+
i = store.find_installation(
151+
enterprise_id="EO111", team_id=None, user_id="UO111"
152+
)
153+
self.assertIsNotNone(i)
154+
i = store.find_installation(
155+
enterprise_id="E111",
156+
team_id="T111",
157+
is_enterprise_install=True,
158+
user_id="U222",
159+
)
160+
self.assertIsNone(i)
161+
i = store.find_installation(enterprise_id=None, team_id="T222", user_id="U111")
162+
self.assertIsNone(i)
163+
164+
# delete installations
165+
store.delete_installation(enterprise_id="E111", team_id=None)
166+
i = store.find_installation(enterprise_id="E111", team_id=None)
167+
self.assertIsNone(i)
168+
169+
# delete all
170+
store.save(installation)
171+
store.delete_all(enterprise_id="E111", team_id=None)
172+
173+
i = store.find_installation(enterprise_id="E111", team_id=None)
174+
self.assertIsNone(i)
175+
i = store.find_installation(enterprise_id="E111", team_id=None, user_id="U111")
176+
self.assertIsNone(i)
177+
bot = store.find_bot(enterprise_id=None, team_id="T222")
178+
self.assertIsNone(bot)
179+
180+
def test_save_and_find_token_rotation(self):
181+
store = self.build_store()
182+
installation = Installation(
183+
app_id="A111",
184+
enterprise_id="E111",
185+
team_id="T111",
186+
user_id="U111",
187+
bot_id="B111",
188+
bot_token="xoxe.xoxp-1-initial",
189+
bot_scopes=["chat:write"],
190+
bot_user_id="U222",
191+
bot_refresh_token="xoxe-1-initial",
192+
bot_token_expires_in=43200,
193+
)
194+
store.save(installation)
195+
196+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
197+
self.assertIsNotNone(bot)
198+
self.assertEqual(bot.bot_refresh_token, "xoxe-1-initial")
199+
200+
# Update the existing data
201+
refreshed_installation = Installation(
202+
app_id="A111",
203+
enterprise_id="E111",
204+
team_id="T111",
205+
user_id="U111",
206+
bot_id="B111",
207+
bot_token="xoxe.xoxp-1-refreshed",
208+
bot_scopes=["chat:write"],
209+
bot_user_id="U222",
210+
bot_refresh_token="xoxe-1-refreshed",
211+
bot_token_expires_in=43200,
212+
)
213+
store.save(refreshed_installation)
214+
215+
# find bots
216+
bot = store.find_bot(enterprise_id="E111", team_id="T111")
217+
self.assertIsNotNone(bot)
218+
self.assertEqual(bot.bot_refresh_token, "xoxe-1-refreshed")
219+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
220+
self.assertIsNone(bot)
221+
bot = store.find_bot(enterprise_id=None, team_id="T111")
222+
self.assertIsNone(bot)
223+
224+
# delete bots
225+
store.delete_bot(enterprise_id="E111", team_id="T222")
226+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
227+
self.assertIsNone(bot)
228+
229+
# find installations
230+
i = store.find_installation(enterprise_id="E111", team_id="T111")
231+
self.assertIsNotNone(i)
232+
i = store.find_installation(enterprise_id="E111", team_id="T222")
233+
self.assertIsNone(i)
234+
i = store.find_installation(enterprise_id=None, team_id="T111")
235+
self.assertIsNone(i)
236+
237+
i = store.find_installation(
238+
enterprise_id="E111", team_id="T111", user_id="U111"
239+
)
240+
self.assertIsNotNone(i)
241+
i = store.find_installation(
242+
enterprise_id="E111", team_id="T111", user_id="U222"
243+
)
244+
self.assertIsNone(i)
245+
i = store.find_installation(
246+
enterprise_id="E111", team_id="T222", user_id="U111"
247+
)
248+
self.assertIsNone(i)
249+
250+
# delete installations
251+
store.delete_installation(enterprise_id="E111", team_id="T111", user_id="U111")
252+
i = store.find_installation(
253+
enterprise_id="E111", team_id="T111", user_id="U111"
254+
)
255+
self.assertIsNone(i)
256+
i = store.find_installation(enterprise_id="E111", team_id="T111")
257+
self.assertIsNone(i)
258+
259+
# delete all
260+
store.save(installation)
261+
store.delete_all(enterprise_id="E111", team_id="T111")
262+
263+
i = store.find_installation(enterprise_id="E111", team_id="T111")
264+
self.assertIsNone(i)
265+
i = store.find_installation(
266+
enterprise_id="E111", team_id="T111", user_id="U111"
267+
)
268+
self.assertIsNone(i)
269+
bot = store.find_bot(enterprise_id="E111", team_id="T222")
270+
self.assertIsNone(bot)

0 commit comments

Comments
 (0)