Merge pull request #2528 from bendikro/master-github-hook-fix

Fix GitHubAuth to retrieve all organizations
buildbot · Dec 8, 2016 · 3626a1d · 3626a1d
2 parents c86127c + b1e3071
commit 3626a1d
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 8 deletions.
diff --git a/master/buildbot/newsfragments/githubauth_orgs.bugfix b/master/buildbot/newsfragments/githubauth_orgs.bugfix
@@ -0,0 +1 @@
+Fix `:py:class:`~buildbot.www.oauth2.GitHubAuth` to retrieve all organizations instead of only those publicly available.
diff --git a/master/buildbot/test/unit/test_www_oauth.py b/master/buildbot/test/unit/test_www_oauth.py
@@ -85,12 +85,12 @@ def test_getGoogleLoginURL(self):
     @defer.inlineCallbacks
     def test_getGithubLoginURL(self):
         res = yield self.githubAuth.getLoginURL('http://redir')
-        exp = ("https://github.com/login/oauth/authorize?scope=user%3Aemail&state="
+        exp = ("https://github.com/login/oauth/authorize?scope=user&state="
                "redirect%3Dhttp%253A%252F%252Fredir&redirect_uri=h%3A%2Fa%2Fb%2F"
                "auth%2Flogin&response_type=code&client_id=ghclientID")
         self.assertEqual(res, exp)
         res = yield self.githubAuth.getLoginURL(None)
-        exp = ("https://github.com/login/oauth/authorize?scope=user%3Aemail&redirect_uri="
+        exp = ("https://github.com/login/oauth/authorize?scope=user&redirect_uri="
                "h%3A%2Fa%2Fb%2Fauth%2Flogin&response_type=code&client_id=ghclientID")
         self.assertEqual(res, exp)
 
@@ -135,13 +135,14 @@ def test_GithubVerifyCode(self):
             [  # /user/emails
                 {'email': 'buzz@bar', 'verified': True, 'primary': False},
                 {'email': 'bar@foo', 'verified': True, 'primary': True}],
-            [dict(  # /users/bar/orgs
-                login="group",)
-             ]])
+            [  # /user/orgs
+                dict(login="hello"),
+                dict(login="grp"),
+            ]])
         res = yield self.githubAuth.verifyCode("code!")
         self.assertEqual({'email': 'bar@foo',
                           'username': 'bar',
-                          'groups': ['group'],
+                          'groups': ["hello", "grp"],
                           'full_name': 'foo bar'}, res)
 
     @defer.inlineCallbacks

diff --git a/master/buildbot/www/oauth2.py b/master/buildbot/www/oauth2.py
@@ -175,7 +175,7 @@ class GitHubAuth(OAuth2Auth):
     name = "GitHub"
     faIcon = "fa-github"
     authUri = 'https://github.com/login/oauth/authorize'
-    authUriAdditionalParams = {'scope': 'user:email'}
+    authUriAdditionalParams = {'scope': 'user'}
     tokenUri = 'https://github.com/login/oauth/access_token'
     resourceEndpoint = 'https://api.github.com'
 
@@ -186,7 +186,8 @@ def getUserInfoFromOAuthClient(self, c):
             if email.get('primary', False):
                 user['email'] = email['email']
                 break
-        orgs = self.get(c, join('/users', user['login'], "orgs"))
+        orgs = self.get(c, '/user/orgs')
+
         return dict(full_name=user['name'],
                     email=user['email'],
                     username=user['login'],