Improve request parser for better compatibility with Socket Mode

Author: seratch

slack_bolt/request/internals.py

@@ -36,9 +36,9 @@ def parse_body(body: str, content_type: Optional[str]) -> Dict[str, Any]:
     ) or body.startswith("{"):
         return json.loads(body)
     else:
-        if "payload" in body:
+        if "payload" in body:  # This is not JSON format yet
             params = dict(parse_qsl(body))
-            if "payload" in params:
+            if params.get("payload") is not None:
                 return json.loads(params.get("payload"))
             else:
                 return {}
@@ -56,72 +56,72 @@ def extract_is_enterprise_install(payload: Dict[str, Any]) -> Optional[bool]:
 
 
 def extract_enterprise_id(payload: Dict[str, Any]) -> Optional[str]:
-    if "enterprise" in payload:
+    if payload.get("enterprise") is not None:
         org = payload.get("enterprise")
         if isinstance(org, str):
             return org
         elif "id" in org:
             return org.get("id")  # type: ignore
-    if "authorizations" in payload and len(payload["authorizations"]) > 0:
+    if payload.get("authorizations") is not None and len(payload["authorizations"]) > 0:
         # To make Events API handling functioning also for shared channels,
         # we should use .authorizations[0].enterprise_id over .enterprise_id
         return extract_enterprise_id(payload["authorizations"][0])
     if "enterprise_id" in payload:
         return payload.get("enterprise_id")
-    if "team" in payload and "enterprise_id" in payload["team"]:
+    if payload.get("team") is not None and "enterprise_id" in payload["team"]:
         # In the case where the type is view_submission
         return payload["team"].get("enterprise_id")
-    if "event" in payload:
+    if payload.get("event") is not None:
         return extract_enterprise_id(payload["event"])
     return None
 
 
 def extract_team_id(payload: Dict[str, Any]) -> Optional[str]:
-    if "team" in payload:
+    if payload.get("team") is not None:
         team = payload.get("team")
         if isinstance(team, str):
             return team
         elif team and "id" in team:
             return team.get("id")
-    if "authorizations" in payload and len(payload["authorizations"]) > 0:
+    if payload.get("authorizations") is not None and len(payload["authorizations"]) > 0:
         # To make Events API handling functioning also for shared channels,
         # we should use .authorizations[0].team_id over .team_id
         return extract_team_id(payload["authorizations"][0])
     if "team_id" in payload:
         return payload.get("team_id")
-    if "event" in payload:
+    if payload.get("event") is not None:
         return extract_team_id(payload["event"])
-    if "user" in payload:
+    if payload.get("user") is not None:
         return payload.get("user")["team_id"]
     return None
 
 
 def extract_user_id(payload: Dict[str, Any]) -> Optional[str]:
-    if "user" in payload:
+    if payload.get("user") is not None:
         user = payload.get("user")
         if isinstance(user, str):
             return user
         elif "id" in user:
             return user.get("id")  # type: ignore
     if "user_id" in payload:
         return payload.get("user_id")
-    if "event" in payload:
+    if payload.get("event") is not None:
         return extract_user_id(payload["event"])
     return None
 
 
 def extract_channel_id(payload: Dict[str, Any]) -> Optional[str]:
-    if "channel" in payload:
+    if payload.get("channel") is not None:
         channel = payload.get("channel")
         if isinstance(channel, str):
             return channel
         elif "id" in channel:
             return channel.get("id")  # type: ignore
     if "channel_id" in payload:
         return payload.get("channel_id")
-    if "event" in payload:
+    if payload.get("event") is not None:
         return extract_channel_id(payload["event"])
-    if "item" in payload:
+    if payload.get("item") is not None:
         # reaction_added: body["event"]["item"]
         return extract_channel_id(payload["item"])
     return None

tests/slack_bolt/request/test_internals.py

@@ -50,6 +50,35 @@ def teardown_method(self):
         },
     ]
 
+    enterprise_no_channel_requests = [
+        {
+            "type": "shortcut",
+            "token": "xxx",
+            "action_ts": "1606983924.521157",
+            "team": {"id": "T111", "domain": "ddd"},
+            "user": {"id": "U111", "username": "use", "team_id": "T111"},
+            "is_enterprise_install": False,
+            "enterprise": {"id": "E111", "domain": "eee"},
+            "callback_id": "run-socket-mode",
+            "trigger_id": "111.222.xxx",
+        },
+    ]
+
+    no_enterprise_no_channel_requests = [
+        {
+            "type": "shortcut",
+            "token": "xxx",
+            "action_ts": "1606983924.521157",
+            "team": {"id": "T111", "domain": "ddd"},
+            "user": {"id": "U111", "username": "use", "team_id": "T111"},
+            "is_enterprise_install": False,
+            # This may be "null" in Socket Mode
+            "enterprise": None,
+            "callback_id": "run-socket-mode",
+            "trigger_id": "111.222.xxx",
+        },
+    ]
+
     def test_channel_id_extraction(self):
         for req in self.requests:
             channel_id = extract_channel_id(req)
@@ -59,16 +88,34 @@ def test_user_id_extraction(self):
         for req in self.requests:
             user_id = extract_user_id(req)
             assert user_id == "U111"
+        for req in self.enterprise_no_channel_requests:
+            user_id = extract_user_id(req)
+            assert user_id == "U111"
+        for req in self.no_enterprise_no_channel_requests:
+            user_id = extract_user_id(req)
+            assert user_id == "U111"
 
     def test_team_id_extraction(self):
         for req in self.requests:
             team_id = extract_team_id(req)
             assert team_id == "T111"
+        for req in self.enterprise_no_channel_requests:
+            team_id = extract_team_id(req)
+            assert team_id == "T111"
+        for req in self.no_enterprise_no_channel_requests:
+            team_id = extract_team_id(req)
+            assert team_id == "T111"
 
     def test_enterprise_id_extraction(self):
         for req in self.requests:
-            team_id = extract_enterprise_id(req)
-            assert team_id == "E111"
+            enterprise_id = extract_enterprise_id(req)
+            assert enterprise_id == "E111"
+        for req in self.enterprise_no_channel_requests:
+            enterprise_id = extract_enterprise_id(req)
+            assert enterprise_id == "E111"
+        for req in self.no_enterprise_no_channel_requests:
+            enterprise_id = extract_enterprise_id(req)
+            assert enterprise_id is None
 
     def test_is_enterprise_install_extraction(self):
         for req in self.requests: