Merge pull request #449 from octoenergy/add_option_to_check_github_team

Add option to check GitHub team

docs/source/github.md

@@ -39,3 +39,33 @@ To use this expanded user information, you will need to subclass your
 current spawner and modify the subclass to read these fields from
 `auth_state` and then use this information to provision your Notebook or
 Lab user.
+
+## Restricting access
+
+### Organizations
+
+If you would like to restrict access to members of specific GitHub organizations
+you can pass a list of organization names to `allowed_organizations`.
+
+For example, the below will ensure that only members of `org_a` or
+`org_b` will be authorized to access.
+
+`c.GitHubOAuthenticator.allowed_organizations = ["org_a", "org_b"]`
+
+### Teams
+
+It is also possible to restrict access to members of specific teams within
+organizations using the syntax: `<organization>:<team-name>`.
+
+For example, the below will only allow members of `org_a`, or
+`team_1` in `org_b` access. Members of `org_b` but not `team_1` will be
+unauthorized to access.
+
+`c.GitHubOAuthenticator.allowed_organizations = ["org_a", "org_b:team_1"]`
+
+### Notes
+
+- Restricting access by either organization or team requires the `read:org`
+  scope
+- Ensure you use the organization/team name as it appears in the GitHub url
+  - E.g. Use `jupyter` instead of `Project Jupyter`

oauthenticator/github.py

@@ -225,13 +225,13 @@ async def _check_membership_allowed_organizations(
         headers = _api_headers(access_token)
         # Check membership of user `username` for organization `org` via api [check-membership](https://developer.github.com/v3/orgs/members/#check-membership)
         # With empty scope (even if authenticated by an org member), this
-        #  will only await public org members.  You want 'read:org' in order
-        #  to be able to iterate through all members.
-        check_membership_url = "%s/orgs/%s/members/%s" % (
-            self.github_api,
-            org,
-            username,
-        )
+        # will only await public org members.  You want 'read:org' in order
+        # to be able to iterate through all members. If you would only like to
+        # allow certain teams within an organisation, specify
+        # allowed_organisations = {org_name:team_name}
+
+        check_membership_url = self._build_check_membership_url(org, username)
+
         req = HTTPRequest(
             check_membership_url,
             method="GET",
@@ -260,6 +260,13 @@ async def _check_membership_allowed_organizations(
             )
         return False
 
+    def _build_check_membership_url(self, org: str, username: str) -> str:
+        if ":" in org:
+            org, team = org.split(":")
+            return f"{self.github_api}/orgs/{org}/teams/{team}/members/{username}"
+        else:
+            return f"{self.github_api}/orgs/{org}/members/{username}"
+
 
 class LocalGitHubOAuthenticator(LocalAuthenticator, GitHubOAuthenticator):
 

oauthenticator/tests/test_github.py

@@ -7,6 +7,7 @@
 from urllib.parse import urlparse
 
 from pytest import fixture
+from pytest import mark
 from tornado.httpclient import HTTPResponse
 from tornado.httputil import HTTPHeaders
 from traitlets.config import Config
@@ -71,69 +72,95 @@ async def test_allowed_org_membership(github_client):
 
     ## Mock Github API
 
-    teams = {
+    orgs = {
         'red': ['grif', 'simmons', 'donut', 'sarge', 'lopez'],
         'blue': ['tucker', 'caboose', 'burns', 'sheila', 'texas'],
     }
 
+    org_teams = {'blue': {'alpha': ['tucker', 'caboose', 'burns']}}
+
     member_regex = re.compile(r'/orgs/(.*)/members')
 
-    def team_members(paginate, request):
+    def org_members(paginate, request):
         urlinfo = urlparse(request.url)
-        team = member_regex.match(urlinfo.path).group(1)
+        org = member_regex.match(urlinfo.path).group(1)
 
-        if team not in teams:
+        if org not in orgs:
             return HTTPResponse(request, 404)
 
         if not paginate:
-            return [user_model(m) for m in teams[team]]
+            return [user_model(m) for m in orgs[org]]
         else:
             page = parse_qs(urlinfo.query).get('page', ['1'])
             page = int(page[0])
-            return team_members_paginated(
-                team, page, urlinfo, functools.partial(HTTPResponse, request)
+            return org_members_paginated(
+                org, page, urlinfo, functools.partial(HTTPResponse, request)
             )
 
-    def team_members_paginated(team, page, urlinfo, response):
-        if page < len(teams[team]):
+    def org_members_paginated(org, page, urlinfo, response):
+        if page < len(orgs[org]):
             headers = make_link_header(urlinfo, page + 1)
-        elif page == len(teams[team]):
+        elif page == len(orgs[org]):
             headers = {}
         else:
             return response(400)
 
         headers.update({'Content-Type': 'application/json'})
 
-        ret = [user_model(teams[team][page - 1])]
+        ret = [user_model(orgs[org][page - 1])]
 
         return response(
             200,
             headers=HTTPHeaders(headers),
             buffer=BytesIO(json.dumps(ret).encode('utf-8')),
         )
 
-    membership_regex = re.compile(r'/orgs/(.*)/members/(.*)')
+    org_membership_regex = re.compile(r'/orgs/(.*)/members/(.*)')
 
-    def team_membership(request):
+    def org_membership(request):
         urlinfo = urlparse(request.url)
-        urlmatch = membership_regex.match(urlinfo.path)
-        team = urlmatch.group(1)
+        urlmatch = org_membership_regex.match(urlinfo.path)
+        org = urlmatch.group(1)
         username = urlmatch.group(2)
-        print('Request team = %s, username = %s' % (team, username))
-        if team not in teams:
-            print('Team not found: team = %s' % (team))
+        print('Request org = %s, username = %s' % (org, username))
+        if org not in orgs:
+            print('Org not found: org = %s' % (org))
+            return HTTPResponse(request, 404)
+        if username not in orgs[org]:
+            print('Member not found: org = %s, username = %s' % (org, username))
             return HTTPResponse(request, 404)
-        if username not in teams[team]:
-            print('Member not found: team = %s, username = %s' % (team, username))
+        return HTTPResponse(request, 204)
+
+    team_membership_regex = re.compile(r'/orgs/(.*)/teams/(.*)/members/(.*)')
+
+    def team_membership(request):
+        urlinfo = urlparse(request.url)
+        urlmatch = team_membership_regex.match(urlinfo.path)
+        org = urlmatch.group(1)
+        team = urlmatch.group(2)
+        username = urlmatch.group(3)
+        print('Request org = %s, team = %s username = %s' % (org, team, username))
+        if org not in orgs:
+            print('Org not found: org = %s' % (org))
+            return HTTPResponse(request, 404)
+        if team not in org_teams[org]:
+            print('Team not found in org: team = %s, org = %s' % (team, org))
+            return HTTPResponse(request, 404)
+        if username not in org_teams[org][team]:
+            print(
+                'Member not found: org = %s, team = %s, username = %s'
+                % (org, team, username)
+            )
             return HTTPResponse(request, 404)
         return HTTPResponse(request, 204)
 
     ## Perform tests
 
     for paginate in (False, True):
         client_hosts = client.hosts['api.github.com']
-        client_hosts.append((membership_regex, team_membership))
-        client_hosts.append((member_regex, functools.partial(team_members, paginate)))
+        client_hosts.append((team_membership_regex, team_membership))
+        client_hosts.append((org_membership_regex, org_membership))
+        client_hosts.append((member_regex, functools.partial(org_members, paginate)))
 
         authenticator.allowed_organizations = ['blue']
 
@@ -156,10 +183,42 @@ def team_membership(request):
         user = await authenticator.authenticate(handler)
         assert user['name'] == 'donut'
 
+        # test team membership
+        authenticator.allowed_organizations = ['blue:alpha', 'red']
+
+        handler = client.handler_for_user(user_model('tucker'))
+        user = await authenticator.authenticate(handler)
+        assert user['name'] == 'tucker'
+
+        handler = client.handler_for_user(user_model('grif'))
+        user = await authenticator.authenticate(handler)
+        assert user['name'] == 'grif'
+
+        handler = client.handler_for_user(user_model('texas'))
+        user = await authenticator.authenticate(handler)
+        assert user is None
+
         client_hosts.pop()
         client_hosts.pop()
 
 
+@mark.parametrize(
+    "org, username, expected",
+    [
+        ("blue", "texas", "https://api.github.com/orgs/blue/members/texas"),
+        (
+            "blue:alpha",
+            "tucker",
+            "https://api.github.com/orgs/blue/teams/alpha/members/tucker",
+        ),
+        ("red", "grif", "https://api.github.com/orgs/red/members/grif"),
+    ],
+)
+def test_build_check_membership_url(org, username, expected):
+    output = GitHubOAuthenticator()._build_check_membership_url(org, username)
+    assert output == expected
+
+
 def test_deprecated_config(caplog):
     cfg = Config()
     cfg.GitHubOAuthenticator.github_organization_whitelist = ["jupy"]