From e4950f47fe08067681c5f1b2e81ea59bc914b5bd Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 12:10:18 -0400
Subject: [PATCH 01/18] Allow nested Google Groups

---
 oauthenticator/google.py | 48 +++++++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 5d901cd9..367ad76b 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -105,6 +105,17 @@ def _userdata_url_default(self):
         """,
     )
 
+    allow_nested_groups = Bool(
+        Set(Unicode()),
+        config=True,
+        help="""
+        Allow members of nested Google groups to sign in.
+
+        Use of this requires configuration of `gsuite_administrator` and
+        `google_service_account_keys`.
+        """,
+    )
+
     strip_domain = Bool(
         config=True,
         help="""
@@ -353,16 +364,10 @@ def _service_client(self, service_name, service_version, credentials, http=None)
             cache_discovery=False,
             http=http,
         )
-
-    def _fetch_user_groups(self, user_email, user_email_domain, http=None):
+    def _setup_service(self, user_email_domain, http=None):
         """
-        Return a set with the google groups a given user is a member of
+        Set up the service client for Google API.
         """
-        # FIXME: When this function is used and waiting for web request
-        #        responses, JupyterHub gets blocked from doing other things.
-        #        Ideally the web requests should be made using an async client
-        #        that can be awaited while JupyterHub handles other things.
-        #
         credentials = self._service_client_credentials(
             scopes=[f"{self.google_api_url}/auth/admin.directory.group.readonly"],
             user_email_domain=user_email_domain,
@@ -373,13 +378,36 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None):
             credentials=credentials,
             http=http,
         )
+        return service
+
+    def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_groups=None):
+        """
+        Return a set with the google groups a given user is a member of, including nested groups if allowed.
+        """
+        if checked_groups is None:
+            checked_groups = set()
+
+        
+        service = self._setup_service(user_email_domain, http)
 
         resp = service.groups().list(userKey=user_email).execute()
         user_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}])
         }
-        self.log.debug(f"user_email {user_email} is a member of {user_groups}")
-        return user_groups
+
+        # Add the current user's groups to the checked groups
+        checked_groups.update(user_groups)
+
+        # Recursively check for nested groups if allowed
+        if self.allow_nested_groups:
+            for group in user_groups:
+                if group not in checked_groups:
+                    nested_groups = self._fetch_user_groups(group, user_email_domain, http, checked_groups)
+                    checked_groups.update(nested_groups)
+
+        self.log.debug(f"user_email {user_email} is a member of {checked_groups}")
+        return checked_groups
+
 
 
 class LocalGoogleOAuthenticator(LocalAuthenticator, GoogleOAuthenticator):

From 5b22e2e931b395b39e61f78c4ef6c6977db005b0 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 12:27:31 -0400
Subject: [PATCH 02/18] Improvements

---
 oauthenticator/google.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 367ad76b..cd13afd6 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -106,7 +106,6 @@ def _userdata_url_default(self):
     )
 
     allow_nested_groups = Bool(
-        Set(Unicode()),
         config=True,
         help="""
         Allow members of nested Google groups to sign in.
@@ -387,10 +386,10 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
         if checked_groups is None:
             checked_groups = set()
 
-        
-        service = self._setup_service(user_email_domain, http)
+        if not hasattr(self, 'service'):
+            self.service = self._setup_service(user_email_domain, http)
 
-        resp = service.groups().list(userKey=user_email).execute()
+        resp = self.service.groups().list(userKey=user_email).execute()
         user_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}])
         }

From efd4e7cb60e8c2ee58debbd0678e64a53d51fb79 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 12:47:20 -0400
Subject: [PATCH 03/18] Add prints

---
 oauthenticator/google.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index cd13afd6..19c9c81c 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -393,11 +393,13 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
         user_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}])
         }
+        print(user_groups)
 
         # Add the current user's groups to the checked groups
         checked_groups.update(user_groups)
 
         # Recursively check for nested groups if allowed
+        print(self.allow_nested_groups)
         if self.allow_nested_groups:
             for group in user_groups:
                 if group not in checked_groups:

From d68fb4342ccbffcee5dc909f2c55583144268ffa Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 13:16:41 -0400
Subject: [PATCH 04/18] Add domain

---
 oauthenticator/google.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 19c9c81c..423f7a98 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -393,17 +393,15 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
         user_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}])
         }
-        print(user_groups)
 
         # Add the current user's groups to the checked groups
         checked_groups.update(user_groups)
 
         # Recursively check for nested groups if allowed
-        print(self.allow_nested_groups)
         if self.allow_nested_groups:
             for group in user_groups:
                 if group not in checked_groups:
-                    nested_groups = self._fetch_user_groups(group, user_email_domain, http, checked_groups)
+                    nested_groups = self._fetch_user_groups(group + user_email_domain, user_email_domain, http, checked_groups)
                     checked_groups.update(nested_groups)
 
         self.log.debug(f"user_email {user_email} is a member of {checked_groups}")

From a0ce23ac0cc70fc56dc70099de43aa9203710c51 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 13:20:01 -0400
Subject: [PATCH 05/18] Comment out `checked_groups`

---
 oauthenticator/google.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 423f7a98..4d0a8bc2 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -395,7 +395,7 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
         }
 
         # Add the current user's groups to the checked groups
-        checked_groups.update(user_groups)
+        # checked_groups.update(user_groups)
 
         # Recursively check for nested groups if allowed
         if self.allow_nested_groups:

From 5a5284204b5ad7a26304bbc7148dc33c11ddb2f5 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 13:22:32 -0400
Subject: [PATCH 06/18] Re-add `@`

---
 oauthenticator/google.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 4d0a8bc2..b43d7d1f 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -394,14 +394,11 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
             g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}])
         }
 
-        # Add the current user's groups to the checked groups
-        # checked_groups.update(user_groups)
-
         # Recursively check for nested groups if allowed
         if self.allow_nested_groups:
             for group in user_groups:
                 if group not in checked_groups:
-                    nested_groups = self._fetch_user_groups(group + user_email_domain, user_email_domain, http, checked_groups)
+                    nested_groups = self._fetch_user_groups(group + "@" + user_email_domain, user_email_domain, http, checked_groups)
                     checked_groups.update(nested_groups)
 
         self.log.debug(f"user_email {user_email} is a member of {checked_groups}")

From 7a2c30563d6021421471b754b16195fbd51d9d2d Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 13:28:57 -0400
Subject: [PATCH 07/18] Ensure `email` is not `None`

---
 oauthenticator/google.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index b43d7d1f..1b8060bd 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -391,7 +391,7 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
 
         resp = self.service.groups().list(userKey=user_email).execute()
         user_groups = {
-            g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}])
+            g['email'].split('@')[0] for g in resp.get('groups', []) if g.get('email')
         }
 
         # Recursively check for nested groups if allowed

From c0598b46f43fbcdb5b448166a6f1b81efde2e0cc Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 13:39:06 -0400
Subject: [PATCH 08/18] Separate checked from processed groups

---
 oauthenticator/google.py | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 1b8060bd..c716fa53 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -379,12 +379,14 @@ def _setup_service(self, user_email_domain, http=None):
         )
         return service
 
-    def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_groups=None):
+    def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_groups=None, processed_groups=None):
         """
         Return a set with the google groups a given user is a member of, including nested groups if allowed.
         """
         if checked_groups is None:
             checked_groups = set()
+        if processed_groups is None:
+            processed_groups = set()
 
         if not hasattr(self, 'service'):
             self.service = self._setup_service(user_email_domain, http)
@@ -394,12 +396,20 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
             g['email'].split('@')[0] for g in resp.get('groups', []) if g.get('email')
         }
 
+        self.log.debug(f"Fetched groups for {user_email}: {user_groups}")
+
+        # Add the current user's groups to the checked groups
+        checked_groups.update(user_groups)
+
+        self.log.debug(f"Checked groups after update: {checked_groups}")
+
         # Recursively check for nested groups if allowed
         if self.allow_nested_groups:
             for group in user_groups:
-                if group not in checked_groups:
-                    nested_groups = self._fetch_user_groups(group + "@" + user_email_domain, user_email_domain, http, checked_groups)
+                if group not in processed_groups:
+                    nested_groups = self._fetch_user_groups(group + "@" + user_email_domain, user_email_domain, http, checked_groups, processed_groups)
                     checked_groups.update(nested_groups)
+                    processed_groups.add(group)
 
         self.log.debug(f"user_email {user_email} is a member of {checked_groups}")
         return checked_groups

From ebdad2627fc57e1ee89e44d97bb32371ded48e7e Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 18:00:38 -0400
Subject: [PATCH 09/18] Run hooks

---
 oauthenticator/google.py | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index c716fa53..d82dc061 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -363,6 +363,7 @@ def _service_client(self, service_name, service_version, credentials, http=None)
             cache_discovery=False,
             http=http,
         )
+
     def _setup_service(self, user_email_domain, http=None):
         """
         Set up the service client for Google API.
@@ -379,7 +380,14 @@ def _setup_service(self, user_email_domain, http=None):
         )
         return service
 
-    def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_groups=None, processed_groups=None):
+    def _fetch_user_groups(
+        self,
+        user_email,
+        user_email_domain,
+        http=None,
+        checked_groups=None,
+        processed_groups=None,
+    ):
         """
         Return a set with the google groups a given user is a member of, including nested groups if allowed.
         """
@@ -398,16 +406,19 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
 
         self.log.debug(f"Fetched groups for {user_email}: {user_groups}")
 
-        # Add the current user's groups to the checked groups
         checked_groups.update(user_groups)
-
         self.log.debug(f"Checked groups after update: {checked_groups}")
 
-        # Recursively check for nested groups if allowed
         if self.allow_nested_groups:
             for group in user_groups:
                 if group not in processed_groups:
-                    nested_groups = self._fetch_user_groups(group + "@" + user_email_domain, user_email_domain, http, checked_groups, processed_groups)
+                    nested_groups = self._fetch_user_groups(
+                        f"{group}@{user_email_domain}",
+                        user_email_domain,
+                        http,
+                        checked_groups,
+                        processed_groups,
+                    )
                     checked_groups.update(nested_groups)
                     processed_groups.add(group)
 
@@ -415,6 +426,5 @@ def _fetch_user_groups(self, user_email, user_email_domain, http=None, checked_g
         return checked_groups
 
 
-
 class LocalGoogleOAuthenticator(LocalAuthenticator, GoogleOAuthenticator):
     """A version that mixes in local system user creation"""

From 833bb08601ed7150bef932f6e14acaf753beeb72 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 23:49:48 -0400
Subject: [PATCH 10/18] Update function name/docs

---
 oauthenticator/google.py            | 53 ++++++++++++++++-------------
 oauthenticator/tests/test_google.py |  2 +-
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index d82dc061..aa7b5d86 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -108,10 +108,15 @@ def _userdata_url_default(self):
     allow_nested_groups = Bool(
         config=True,
         help="""
-        Allow members of nested Google groups to sign in.
-
-        Use of this requires configuration of `gsuite_administrator` and
-        `google_service_account_keys`.
+        Allow members of nested Google groups to sign in and/or administer
+        TLJH. If `True` the authenticator will recursively check a user's 
+        group memberships and allow sign in and/or grant admin rights to
+        a user that is a member of a group that has nested membership in
+        any group in `allowed_google_groups` or `admin_google_groups`.
+
+        Use of this requires configuration of `gsuite_administrator`, and
+        `google_service_account_keys`, plus at least `allowed_google_groups` 
+        or `admin_google_groups`.
         """,
     )
 
@@ -245,7 +250,7 @@ async def update_auth_model(self, auth_model):
 
         user_groups = set()
         if self.allowed_google_groups or self.admin_google_groups:
-            user_groups = self._fetch_user_groups(user_email, user_domain)
+            user_groups = self._fetch_member_groups(user_email, user_domain)
         # sets are not JSONable, cast to list for auth_state
         user_info["google_groups"] = list(user_groups)
 
@@ -380,39 +385,42 @@ def _setup_service(self, user_email_domain, http=None):
         )
         return service
 
-    def _fetch_user_groups(
+    def _fetch_member_groups(
         self,
-        user_email,
+        member_email,
         user_email_domain,
         http=None,
-        checked_groups=None,
-        processed_groups=None,
+        checked_groups=set(),
+        processed_groups=set(),
     ):
         """
-        Return a set with the google groups a given user is a member of, including nested groups if allowed.
+        Return a set with the google groups a given user/group is a member of, including nested groups if allowed.
         """
-        if checked_groups is None:
-            checked_groups = set()
-        if processed_groups is None:
-            processed_groups = set()
-
+        # FIXME: When this function is used and waiting for web request
+        #        responses, JupyterHub gets blocked from doing other things.
+        #        Ideally the web requests should be made using an async client
+        #        that can be awaited while JupyterHub handles other things.
+        #
         if not hasattr(self, 'service'):
             self.service = self._setup_service(user_email_domain, http)
 
-        resp = self.service.groups().list(userKey=user_email).execute()
-        user_groups = {
+        resp = self.service.groups().list(userKey=member_email).execute()
+        member_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', []) if g.get('email')
         }
 
-        self.log.debug(f"Fetched groups for {user_email}: {user_groups}")
+        self.log.debug(f"Fetched groups for {member_email}: {member_groups}")
+
+        checked_groups.update(member_groups)
+        self.log.debug(f"Checked groups after update: {checked_groups}")
 
-        checked_groups.update(user_groups)
         self.log.debug(f"Checked groups after update: {checked_groups}")
 
         if self.allow_nested_groups:
-            for group in user_groups:
+            for group in member_groups:
                 if group not in processed_groups:
-                    nested_groups = self._fetch_user_groups(
+                    processed_groups.add(group)
+                    nested_groups = self._fetch_member_groups(
                         f"{group}@{user_email_domain}",
                         user_email_domain,
                         http,
@@ -420,9 +428,8 @@ def _fetch_user_groups(
                         processed_groups,
                     )
                     checked_groups.update(nested_groups)
-                    processed_groups.add(group)
 
-        self.log.debug(f"user_email {user_email} is a member of {checked_groups}")
+        self.log.debug(f"member_email {member_email} is a member of {checked_groups}")
         return checked_groups
 
 
diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py
index 7dc7c35f..4c3a9e0c 100644
--- a/oauthenticator/tests/test_google.py
+++ b/oauthenticator/tests/test_google.py
@@ -211,7 +211,7 @@ async def test_google(
     handled_user_model = user_model("user1@example.com", "user1")
     handler = google_client.handler_for_user(handled_user_model)
     with mock.patch.object(
-        authenticator, "_fetch_user_groups", lambda *args: {"group1"}
+        authenticator, "_fetch_member_groups", lambda *args: {"group1"}
     ):
         auth_model = await authenticator.get_authenticated_user(handler, None)
 

From cf709af307cc1a3732ee443e0e99506f365463cc Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sat, 21 Sep 2024 23:53:40 -0400
Subject: [PATCH 11/18] Remove copy/paste of log line

---
 oauthenticator/google.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index aa7b5d86..58fe0368 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -408,14 +408,11 @@ def _fetch_member_groups(
         member_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', []) if g.get('email')
         }
-
         self.log.debug(f"Fetched groups for {member_email}: {member_groups}")
 
         checked_groups.update(member_groups)
         self.log.debug(f"Checked groups after update: {checked_groups}")
 
-        self.log.debug(f"Checked groups after update: {checked_groups}")
-
         if self.allow_nested_groups:
             for group in member_groups:
                 if group not in processed_groups:

From 96723dabcc51deae0777a857c4273b0acf4f7dfd Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Sun, 22 Sep 2024 11:16:11 -0400
Subject: [PATCH 12/18] Update `help` docs

---
 oauthenticator/google.py | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 58fe0368..f9d386b1 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -108,15 +108,8 @@ def _userdata_url_default(self):
     allow_nested_groups = Bool(
         config=True,
         help="""
-        Allow members of nested Google groups to sign in and/or administer
-        TLJH. If `True` the authenticator will recursively check a user's 
-        group memberships and allow sign in and/or grant admin rights to
-        a user that is a member of a group that has nested membership in
-        any group in `allowed_google_groups` or `admin_google_groups`.
-
-        Use of this requires configuration of `gsuite_administrator`, and
-        `google_service_account_keys`, plus at least `allowed_google_groups` 
-        or `admin_google_groups`.
+        Include members of nested Google groups in `allowed_google_groups` and
+        `admin_google_groups` to sign in and/or administer JupyterHub.
         """,
     )
 

From 2d9163833613575e04bb91f98e7079f3c0520378 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Thu, 26 Sep 2024 13:05:46 -0400
Subject: [PATCH 13/18] Add tests

---
 oauthenticator/tests/test_google.py | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py
index 4c3a9e0c..d2782e06 100644
--- a/oauthenticator/tests/test_google.py
+++ b/oauthenticator/tests/test_google.py
@@ -3,6 +3,7 @@
 import logging
 import re
 from unittest import mock
+from unittest.mock import AsyncMock
 
 from pytest import fixture, mark, raises
 from traitlets.config import Config
@@ -380,3 +381,33 @@ async def test_deprecated_config(
 
     expected_log_tuple = (test_logger.name, expect_loglevel, expect_message)
     assert expected_log_tuple in captured_log_tuples
+
+
+@mark.asyncio
+async def test_nested_group_memberships():
+    mock_fetch_member_groups = AsyncMock(
+        return_value={'group1', 'nested_group1', 'nested_group2'}
+    )
+
+    authenticator = GoogleOAuthenticator()
+    authenticator._fetch_member_groups = mock_fetch_member_groups
+
+    user_info = {'name': 'testuser', 'email': 'testuser@example.com'}
+
+    result = await authenticator._fetch_member_groups(user_info['email'], 'example.com')
+    assert 'nested_group1' in result
+    assert 'nested_group2' in result
+
+
+@mark.asyncio
+async def test_user_not_in_nested_group():
+    mock_fetch_member_groups = AsyncMock(return_value={'group1', 'group2'})
+
+    authenticator = GoogleOAuthenticator()
+    authenticator._fetch_member_groups = mock_fetch_member_groups
+
+    user_info = {'name': 'testuser', 'email': 'testuser@example.com'}
+
+    result = await authenticator._fetch_member_groups(user_info['email'], 'example.com')
+    assert 'nested_group1' not in result
+    assert 'nested_group2' not in result

From 9f279fe82aaa6130e0a4af5a964745f1f43e2d86 Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Thu, 26 Sep 2024 13:36:24 -0400
Subject: [PATCH 14/18] Remove tests

---
 oauthenticator/tests/test_google.py | 33 +----------------------------
 1 file changed, 1 insertion(+), 32 deletions(-)

diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py
index d2782e06..6554f898 100644
--- a/oauthenticator/tests/test_google.py
+++ b/oauthenticator/tests/test_google.py
@@ -3,7 +3,6 @@
 import logging
 import re
 from unittest import mock
-from unittest.mock import AsyncMock
 
 from pytest import fixture, mark, raises
 from traitlets.config import Config
@@ -257,7 +256,7 @@ async def test_hosted_domain_single_entry(
     expect_admin,
 ):
     """
-    Tests that sign in is restricted to the listed domain and that the username
+    Tests that sign in is restricted fetchto the listed domain and that the username
     represents the part before the `@domain.com` as expected when hosted_domain
     contains a single entry.
     """
@@ -381,33 +380,3 @@ async def test_deprecated_config(
 
     expected_log_tuple = (test_logger.name, expect_loglevel, expect_message)
     assert expected_log_tuple in captured_log_tuples
-
-
-@mark.asyncio
-async def test_nested_group_memberships():
-    mock_fetch_member_groups = AsyncMock(
-        return_value={'group1', 'nested_group1', 'nested_group2'}
-    )
-
-    authenticator = GoogleOAuthenticator()
-    authenticator._fetch_member_groups = mock_fetch_member_groups
-
-    user_info = {'name': 'testuser', 'email': 'testuser@example.com'}
-
-    result = await authenticator._fetch_member_groups(user_info['email'], 'example.com')
-    assert 'nested_group1' in result
-    assert 'nested_group2' in result
-
-
-@mark.asyncio
-async def test_user_not_in_nested_group():
-    mock_fetch_member_groups = AsyncMock(return_value={'group1', 'group2'})
-
-    authenticator = GoogleOAuthenticator()
-    authenticator._fetch_member_groups = mock_fetch_member_groups
-
-    user_info = {'name': 'testuser', 'email': 'testuser@example.com'}
-
-    result = await authenticator._fetch_member_groups(user_info['email'], 'example.com')
-    assert 'nested_group1' not in result
-    assert 'nested_group2' not in result

From 36931caa63291d56a1062354bd6e53f3f7062afd Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Thu, 26 Sep 2024 18:43:34 -0400
Subject: [PATCH 15/18] Add `aiohttp` to `requirements`

---
 requirements.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/requirements.txt b/requirements.txt
index 53cc9bc4..47a34eeb 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,4 @@
+aiohttp
 # jsonschema is used for validating authenticator configurations
 jsonschema
 jupyterhub>=2.2

From 5209682d400846cb98b26e9802cdb9859d5a365a Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Thu, 26 Sep 2024 19:45:43 -0400
Subject: [PATCH 16/18] Remove `aiohttp` from `requirements`

---
 requirements.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 47a34eeb..53cc9bc4 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,3 @@
-aiohttp
 # jsonschema is used for validating authenticator configurations
 jsonschema
 jupyterhub>=2.2

From 125c7012cf7d3b324be06aae925f2ce35918c36e Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Tue, 1 Oct 2024 09:37:24 -0400
Subject: [PATCH 17/18] Apply suggestions from code review

Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
---
 oauthenticator/google.py            | 21 +++++++++++----------
 oauthenticator/tests/test_google.py |  2 +-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index f9d386b1..3faa8924 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -408,16 +408,17 @@ def _fetch_member_groups(
 
         if self.allow_nested_groups:
             for group in member_groups:
-                if group not in processed_groups:
-                    processed_groups.add(group)
-                    nested_groups = self._fetch_member_groups(
-                        f"{group}@{user_email_domain}",
-                        user_email_domain,
-                        http,
-                        checked_groups,
-                        processed_groups,
-                    )
-                    checked_groups.update(nested_groups)
+                if group in processed_groups:
+                    continue
+                processed_groups.add(group)
+                nested_groups = self._fetch_member_groups(
+                    f"{group}@{user_email_domain}",
+                    user_email_domain,
+                    http,
+                    checked_groups,
+                    processed_groups,
+                )
+                checked_groups.update(nested_groups)
 
         self.log.debug(f"member_email {member_email} is a member of {checked_groups}")
         return checked_groups
diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py
index 6554f898..4c3a9e0c 100644
--- a/oauthenticator/tests/test_google.py
+++ b/oauthenticator/tests/test_google.py
@@ -256,7 +256,7 @@ async def test_hosted_domain_single_entry(
     expect_admin,
 ):
     """
-    Tests that sign in is restricted fetchto the listed domain and that the username
+    Tests that sign in is restricted to the listed domain and that the username
     represents the part before the `@domain.com` as expected when hosted_domain
     contains a single entry.
     """

From 6d436325c41ad8f7353b99e8771b46b63b81f23e Mon Sep 17 00:00:00 2001
From: Jordan Bradford <36420801+jrdnbradford@users.noreply.github.com>
Date: Tue, 1 Oct 2024 10:25:57 -0400
Subject: [PATCH 18/18] Use `include` and set `*_groups` to `None`

---
 oauthenticator/google.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/oauthenticator/google.py b/oauthenticator/google.py
index 3faa8924..f06f6aca 100644
--- a/oauthenticator/google.py
+++ b/oauthenticator/google.py
@@ -105,7 +105,7 @@ def _userdata_url_default(self):
         """,
     )
 
-    allow_nested_groups = Bool(
+    include_nested_groups = Bool(
         config=True,
         help="""
         Include members of nested Google groups in `allowed_google_groups` and
@@ -383,8 +383,8 @@ def _fetch_member_groups(
         member_email,
         user_email_domain,
         http=None,
-        checked_groups=set(),
-        processed_groups=set(),
+        checked_groups=None,
+        processed_groups=None,
     ):
         """
         Return a set with the google groups a given user/group is a member of, including nested groups if allowed.
@@ -397,6 +397,9 @@ def _fetch_member_groups(
         if not hasattr(self, 'service'):
             self.service = self._setup_service(user_email_domain, http)
 
+        checked_groups = checked_groups or set()
+        processed_groups = processed_groups or set()
+
         resp = self.service.groups().list(userKey=member_email).execute()
         member_groups = {
             g['email'].split('@')[0] for g in resp.get('groups', []) if g.get('email')
@@ -406,7 +409,7 @@ def _fetch_member_groups(
         checked_groups.update(member_groups)
         self.log.debug(f"Checked groups after update: {checked_groups}")
 
-        if self.allow_nested_groups:
+        if self.include_nested_groups:
             for group in member_groups:
                 if group in processed_groups:
                     continue