webhooks/github: Restrict membership event scope to teams.

According to GitHub's webhook docs, the scope of a membership
event can only be limited to 'teams', which holds true when a
new member is added to a team. However, we just found a payload
in our logs that indicates that when a user is removed from a
team, the scope of the membership is erroneously set to
'organization', not 'team'. This is most likely a bug on
GitHub's end because such behaviour is a direct violation of
their webhook API event specifications. We account for this
by restricting membership events to teams explicitly, at least
till GitHub's docs suggest otherwise.

zerver/webhooks/github/fixtures/membership_removal.json

@@ -0,0 +1,61 @@
+{
+  "action": "removed",
+  "scope": "organization",
+  "member": {
+    "login": "kdaigle",
+    "id": 2501,
+    "avatar_url": "https://avatars.githubusercontent.com/u/2501?v=3",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/kdaigle",
+    "html_url": "https://github.com/kdaigle",
+    "followers_url": "https://api.github.com/users/kdaigle/followers",
+    "following_url": "https://api.github.com/users/kdaigle/following{/other_user}",
+    "gists_url": "https://api.github.com/users/kdaigle/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/kdaigle/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/kdaigle/subscriptions",
+    "organizations_url": "https://api.github.com/users/kdaigle/orgs",
+    "repos_url": "https://api.github.com/users/kdaigle/repos",
+    "events_url": "https://api.github.com/users/kdaigle/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/kdaigle/received_events",
+    "type": "User",
+    "site_admin": true
+  },
+  "sender": {
+    "login": "baxterthehacker",
+    "id": 6752317,
+    "avatar_url": "https://avatars.githubusercontent.com/u/6752317?v=2",
+    "gravatar_id": "",
+    "url": "https://api.github.com/users/baxterthehacker",
+    "html_url": "https://github.com/baxterthehacker",
+    "followers_url": "https://api.github.com/users/baxterthehacker/followers",
+    "following_url": "https://api.github.com/users/baxterthehacker/following{/other_user}",
+    "gists_url": "https://api.github.com/users/baxterthehacker/gists{/gist_id}",
+    "starred_url": "https://api.github.com/users/baxterthehacker/starred{/owner}{/repo}",
+    "subscriptions_url": "https://api.github.com/users/baxterthehacker/subscriptions",
+    "organizations_url": "https://api.github.com/users/baxterthehacker/orgs",
+    "repos_url": "https://api.github.com/users/baxterthehacker/repos",
+    "events_url": "https://api.github.com/users/baxterthehacker/events{/privacy}",
+    "received_events_url": "https://api.github.com/users/baxterthehacker/received_events",
+    "type": "User",
+    "site_admin": false
+  },
+  "team": {
+    "name": "Contractors",
+    "id": 123456,
+    "slug": "contractors",
+    "permission": "admin",
+    "url": "https://api.github.com/teams/123456",
+    "members_url": "https://api.github.com/teams/123456/members{/member}",
+    "repositories_url": "https://api.github.com/teams/123456/repos"
+  },
+  "organization": {
+    "login": "baxterandthehackers",
+    "id": 7649605,
+    "url": "https://api.github.com/orgs/baxterandthehackers",
+    "repos_url": "https://api.github.com/orgs/baxterandthehackers/repos",
+    "events_url": "https://api.github.com/orgs/baxterandthehackers/events",
+    "members_url": "https://api.github.com/orgs/baxterandthehackers/members{/member}",
+    "public_members_url": "https://api.github.com/orgs/baxterandthehackers/public_members{/member}",
+    "avatar_url": "https://avatars.githubusercontent.com/u/7649605?v=2"
+  }
+}

zerver/webhooks/github/tests.py

@@ -140,8 +140,18 @@ class GithubWebhookTest(WebhookTestCase):
         self.send_and_test_stream_message('issue', expected_topic, expected_message, HTTP_X_GITHUB_EVENT='issues')
 
     def test_membership_msg(self) -> None:
-        expected_message = u"baxterthehacker added [kdaigle](https://github.com/kdaigle) to Contractors team"
+        expected_message = u"baxterthehacker added [kdaigle](https://github.com/kdaigle) to the Contractors team"
-        self.send_and_test_stream_message('membership', self.EXPECTED_TOPIC_ORGANIZATION_EVENTS, expected_message, HTTP_X_GITHUB_EVENT='membership')
+        self.send_and_test_stream_message('membership',
+                                          self.EXPECTED_TOPIC_ORGANIZATION_EVENTS,
+                                          expected_message,
+                                          HTTP_X_GITHUB_EVENT='membership')
+
+    def test_membership_removal_msg(self) -> None:
+        expected_message = u"baxterthehacker removed [kdaigle](https://github.com/kdaigle) from the Contractors team"
+        self.send_and_test_stream_message('membership_removal',
+                                          self.EXPECTED_TOPIC_ORGANIZATION_EVENTS,
+                                          expected_message,
+                                          HTTP_X_GITHUB_EVENT='membership')
 
     def test_member_msg(self) -> None:
         expected_message = u"baxterthehacker added [octocat](https://github.com/octocat) to [public-repo](https://github.com/baxterthehacker/public-repo)"

zerver/webhooks/github/view.py

@@ -75,16 +75,15 @@ def get_closed_pull_request_body(payload: Dict[str, Any],
 def get_membership_body(payload: Dict[str, Any]) -> str:
     action = payload['action']
     member = payload['member']
-    scope = payload['scope']
+    team_name = payload['team']['name']
-    scope_object = payload[scope]
 
-    return u"{} {} [{}]({}) to {} {}".format(
+    return u"{sender} {action} [{username}]({html_url}) {preposition} the {team_name} team".format(
-        get_sender_name(payload),
+        sender=get_sender_name(payload),
-        action,
+        action=action,
-        member['login'],
+        username=member['login'],
-        member['html_url'],
+        html_url=member['html_url'],
-        scope_object['name'],
+        preposition='from' if action == 'removed' else 'to',
-        scope
+        team_name=team_name
     )
 
 def get_member_body(payload: Dict[str, Any]) -> str: