Merge pull request #1177 from uc-cdis/fix/google-group-update-single-…

…user

fix(PXP-11385): Adds single user google group updates

fence/resources/google/access_utils.py

@@ -4,7 +4,6 @@
 """
 import backoff
 import time
-import flask
 from urllib.parse import unquote
 import traceback
 
@@ -17,7 +16,7 @@
 from cdislogging import get_logger
 
 from fence.config import config
-from fence.errors import NotFound, NotSupported, UserError
+from fence.errors import NotFound, NotSupported
 from fence.models import (
     User,
     Project,
@@ -29,7 +28,6 @@
 )
 from fence.resources.google.utils import (
     get_db_session,
-    get_users_from_google_members,
     get_monitoring_service_account_email,
     is_google_managed_service_account,
 )
@@ -42,93 +40,80 @@ class GoogleUpdateException(Exception):
     pass
 
 
-def bulk_update_google_groups(google_bulk_mapping):
+def update_google_groups_for_users(google_single_user_mapping):
     """
-    Update Google Groups based on mapping provided from Group -> Users.
+    Update Google Groups for a single user based on the provided mapping.
 
     Args:
-        google_bulk_mapping (dict): {"[email protected]": ["member1", "member2"]}
+        google_single_user_mapping (dict): {"user_email": ["[email protected]"]}
     """
     google_project_id = (
         config["STORAGE_CREDENTIALS"].get("google", {}).get("google_project_id")
     )
     google_update_failures = False
     with GoogleCloudManager(google_project_id) as gcm:
-        for group, expected_members in google_bulk_mapping.items():
-            expected_members = set(expected_members)
-            logger.debug(f"Starting diff for group {group}...")
-
-            # get members list from google
-            members_from_google = []
-
+        for user_email, groups in google_single_user_mapping.items():
+            logger.debug(f"Updating groups for user {user_email}...")
+            expected_groups = set(groups)
+            # Get the groups the user is currently in
             try:
-                members_from_google = _get_members_from_google_group(gcm, group)
+                user_current_groups = _get_groups_for_user(gcm, user_email)
             except Exception as exc:
                 logger.error(
-                    f"ERROR: FAILED TO GET MEMBERS FROM GOOGLE GROUP {group}! "
-                    f"This sync will SKIP "
-                    f"the above user to try and update other authorization "
-                    f"(rather than failing early). The error will be re-raised "
-                    f"after attempting to update all other users. Exc: "
+                    f"ERROR: FAILED TO GET GROUPS FOR USER {user_email}! "
                     f"{traceback.format_exc()}"
                 )
                 google_update_failures = True
+                user_current_groups = []
 
-            google_members = set(member.get("email") for member in members_from_google)
+            logger.info(f"User's current groups: {user_current_groups}")
 
-            logger.debug(f"Google membership for {group}: {google_members}")
-            logger.debug(f"Expected membership for {group}: {expected_members}")
+            # Determine which groups to add the user to and which to remove them from
+            current_groups = set(user_current_groups)
 
-            # diff between expected group membership and actual membership
-            to_delete = set.difference(google_members, expected_members)
-            to_add = set.difference(expected_members, google_members)
-            no_update = set.intersection(google_members, expected_members)
+            groups_to_add = expected_groups - current_groups
+            groups_to_remove = current_groups - expected_groups
 
-            logger.info(f"All already in group {group}: {no_update}")
+            logger.info(f"Groups to add for {user_email}: {groups_to_add}")
+            logger.info(f"Groups to remove for {user_email}: {groups_to_remove}")
 
-            # do add
-            for member_email in to_add:
-                logger.info(f"Adding to group {group}: {member_email}")
+            for group in groups_to_add:
+                logger.info(f"Adding {user_email} to group {group}")
                 try:
-                    _add_member_to_google_group(gcm, member_email, group)
+                    _add_member_to_google_group(gcm, user_email, group)
                 except Exception as exc:
                     logger.error(
-                        f"ERROR: FAILED TO ADD MEMBER {member_email} TO GOOGLE "
-                        f"GROUP {group}! This sync will SKIP "
-                        f"the above user to try and update other authorization "
-                        f"(rather than failing early). The error will be re-raised "
-                        f"after attempting to update all other users. Exc: "
+                        f"ERROR: FAILED TO ADD USER {user_email} TO GOOGLE "
+                        f"GROUP {group}! This sync will continue to update other users. Exc: "
                         f"{traceback.format_exc()}"
                     )
                     google_update_failures = True
 
-            # do remove
-            for member_email in to_delete:
-                logger.info(f"Removing from group {group}: {member_email}")
-
+            # Remove the user from groups they should not be in
+            for group in groups_to_remove:
+                logger.info(f"Removing {user_email} from group {group}")
                 try:
-                    _remove_member_from_google_group(gcm, member_email, group)
+                    _remove_member_from_google_group(gcm, user_email, group)
                 except Exception as exc:
                     logger.error(
-                        f"ERROR: FAILED TO REMOVE MEMBER {member_email} FROM "
-                        f"GOOGLE GROUP {group}! This sync will SKIP "
-                        f"the above user to try and update other authorization "
-                        f"(rather than failing early). The error will be re-raised "
-                        f"after attempting to update all other users. Exc: "
+                        f"ERROR: FAILED TO REMOVE USER {user_email} FROM "
+                        f"GOOGLE GROUP {group}! This sync will continue to update other users. Exc: "
                         f"{traceback.format_exc()}"
                     )
                     google_update_failures = True
 
-            if google_update_failures:
-                raise GoogleUpdateException(
-                    f"FAILED TO UPDATE GOOGLE GROUPS (see previous errors)."
-                )
-
+        if google_update_failures:
+            raise GoogleUpdateException(
+                f"FAILED TO UPDATE GOOGLE GROUPS FOR USER {user_email} (see previous errors)."
+            )
 
 @backoff.on_exception(backoff.expo, Exception, **DEFAULT_BACKOFF_SETTINGS)
 def _get_members_from_google_group(gcm, group):
     return gcm.get_group_members(group)
 
+@backoff.on_exception(backoff.expo, Exception, **DEFAULT_BACKOFF_SETTINGS)
+def _get_groups_for_user(gcm, user):
+    return gcm.get_groups_for_user(user)
 
 @backoff.on_exception(backoff.expo, Exception, **DEFAULT_BACKOFF_SETTINGS)
 def _add_member_to_google_group(gcm, add_member_to_group, group):

fence/resources/google/utils.py

@@ -582,21 +582,33 @@ def _update_service_account_db_entry(
     return service_account_db_entry
 
 
-def get_or_create_proxy_group_id(expires=None, user_id=None, username=None):
+def get_or_create_proxy_group_id(expires=None, user_id=None, username=None, session=None, storage_manager=None):
     """
     If no username returned from token or database, create a new proxy group
     for the given user. Also, add the access privileges.
 
+    Args:
+        user_id: Fence user ID of the user
+        username: username for the user
+        session: a sqlalchemy session
+        storage_manager: StorageManager for the backend to create proxy groups, usually google.
     Returns:
         int: id of (possibly newly created) proxy group associated with user
     """
-    proxy_group_id = _get_proxy_group_id(user_id=user_id, username=username)
+    db_session = session or current_app.scoped_session()
+    manager = storage_manager or flask.current_app.storage_manager
+
+    logger.info(f"Proxy Group: {user_id}, {username}")
+    proxy_group_id = _get_proxy_group_id(user_id=user_id, username=username, session=db_session)
+    logger.info(f"{proxy_group_id}")
     if not proxy_group_id:
         try:
-            user_by_id = query_for_user_by_id(current_app.scoped_session(), user_id)
+            user_by_id = query_for_user_by_id(db_session, user_id)
+            logger.info(f"user_by_id: {user_by_id}")
             user_by_username = query_for_user(
-                session=current_app.scoped_session(), username=username
+                session=db_session, username=username
             )
+            logger.info(f"user_by_username: {user_by_username}")
         except Exception:
             user_by_id = None
             user_by_username = None
@@ -616,10 +628,10 @@ def get_or_create_proxy_group_id(expires=None, user_id=None, username=None):
                 f"username={username}, nor was there a current_token"
             )
 
-        proxy_group_id = _create_proxy_group(user_id, username).id
+        proxy_group_id = _create_proxy_group(user_id, username, session=db_session).id
 
         privileges = (
-            current_app.scoped_session()
+            db_session
             .query(AccessPrivilege)
             .filter(AccessPrivilege.user_id == user_id)
         )
@@ -629,25 +641,25 @@ def get_or_create_proxy_group_id(expires=None, user_id=None, username=None):
 
             for sa in storage_accesses:
                 if sa.provider.name == STORAGE_ACCESS_PROVIDER_NAME:
-                    flask.current_app.storage_manager.logger.info(
+                    manager.logger.info(
                         "grant {} access {} to {} in {}".format(
                             username, p.privilege, p.project_id, p.auth_provider
                         )
                     )
 
-                    flask.current_app.storage_manager.grant_access(
+                    manager.grant_access(
                         provider=(sa.provider.name),
                         username=username,
                         project=p.project,
                         access=p.privilege,
-                        session=current_app.scoped_session(),
+                        session=db_session,
                         expires=expires,
                     )
 
     return proxy_group_id
 
 
-def _get_proxy_group_id(user_id=None, username=None):
+def _get_proxy_group_id(user_id=None, username=None, session=None):
     """
     Get users proxy group id from the current token, if possible.
     Otherwise, check the database for it.
@@ -661,9 +673,9 @@ def _get_proxy_group_id(user_id=None, username=None):
         user_id = user_id or current_token["sub"]
 
         try:
-            user = query_for_user_by_id(current_app.scoped_session(), user_id)
+            user = query_for_user_by_id(session, user_id)
             if not user:
-                user = query_for_user(current_app.scoped_session(), username)
+                user = query_for_user(session, username)
         except Exception:
             user = None
 
@@ -673,7 +685,7 @@ def _get_proxy_group_id(user_id=None, username=None):
     return proxy_group_id
 
 
-def _create_proxy_group(user_id, username):
+def _create_proxy_group(user_id, username, session):
     """
     Create a proxy group for the given user
 
@@ -696,11 +708,11 @@ def _create_proxy_group(user_id, username):
     )
 
     # link proxy group to user
-    user = current_app.scoped_session().query(User).filter_by(id=user_id).first()
+    user = session.query(User).filter_by(id=user_id).first()
     user.google_proxy_group_id = proxy_group.id
 
-    current_app.scoped_session().add(proxy_group)
-    current_app.scoped_session().commit()
+    session.add(proxy_group)
+    session.commit()
 
     logger.info(
         "Created proxy group {} for user {} with id {}.".format(

fence/resources/storage/__init__.py

@@ -398,10 +398,9 @@ def _update_access_to_bucket(
             bucket_privileges = bucket_access_group.privileges or []
             if set(bucket_privileges).issubset(access):
                 bucket_name = bucket_access_group.email
-
                 if google_bulk_mapping is not None:
-                    google_bulk_mapping.setdefault(bucket_name, []).append(
-                        storage_username
+                    google_bulk_mapping.setdefault(storage_username, []).append(
+                        bucket_name
                     )
                     self.logger.info(
                         "User {}'s Google proxy group ({}) added to bulk mapping for Google Bucket Access Group {}.".format(