Merge pull request #540 from ubclaunchpad/temp-fix/permissions

temporary fix: disable deletion of existing permissions
ubclaunchpad · Oct 5, 2020 · 4394c76 · 4394c76
2 parents ca02676 + 47d3b3a
commit 4394c76
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 33 deletions.
diff --git a/app/controller/command/commands/team.py b/app/controller/command/commands/team.py
@@ -786,23 +786,5 @@ def refresh_all_drive_permissions(self):
             return
 
         all_teams: List[Team] = self.facade.query(Team)
-        leads_team: Team = None
-        admin_team: Team = None
         for t in all_teams:
-            if t.github_team_name == self.config.github_team_leads:
-                leads_team = t
-                continue
-            if t.github_team_name == self.config.github_team_admin:
-                admin_team = t
-                continue
             sync_team_email_perms(self.gcp, self.facade, t)
-
-        # Workaround for https://github.com/ubclaunchpad/rocket2/issues/497:
-        # We sort the teams such that special-permissions teams are sync'd
-        # last, so that inherited permissions are not overwritten in child
-        # folders.
-        #
-        # TODO: If a proper fix is implemented, remove this and related code
-        for t in [leads_team, admin_team]:
-            if t is not None:
-                sync_team_email_perms(self.gcp, self.facade, t)
diff --git a/interface/gcp.py b/interface/gcp.py
@@ -3,6 +3,11 @@
 from googleapiclient.discovery import Resource
 import logging
 
+# Set to False to resolve https://github.com/ubclaunchpad/rocket2/issues/510
+# temporarily. Longer-term fix is being tracked in the actual problem,
+# https://github.com/ubclaunchpad/rocket2/issues/497
+DELETE_OLD_DRIVE_PERMISSIONS = False
+
 
 class GCPInterface:
     """Utility class for calling Google Cloud Platform (GCP) APIs."""
@@ -12,7 +17,11 @@ def __init__(self, drive_client: Resource, subject=None):
         self.drive = drive_client
         self.subject = subject
 
-    def set_drive_permissions(self, scope, drive_id, emails: List[str]):
+    def set_drive_permissions(self,
+                              scope,
+                              drive_id,
+                              emails: List[str],
+                              delete_permissions=DELETE_OLD_DRIVE_PERMISSIONS):
         """
         Creates permissions for the given emails, and removes everyone not
         on the list.
@@ -79,19 +88,22 @@ def set_drive_permissions(self, scope, drive_id, emails: List[str]):
 
         # Delete old permissions
         # See http://googleapis.github.io/google-api-python-client/docs/dyn/drive_v3.permissions.html#delete # noqa
-        deleted_shares = 0
-        for p_id in to_delete:
-            try:
-                self.drive.permissions()\
-                    .delete(fileId=drive_id,
-                            permissionId=p_id,
-                            supportsAllDrives=True)\
-                    .execute()
-                deleted_shares += 1
-            except Exception as e:
-                logging.error(f"Failed to delete permission {p_id} for drive "
-                              + f"item ({scope}, {drive_id}): {e}")
-        logging.info(f"Deleted {deleted_shares} permissions for {scope}")
+        if delete_permissions is True:
+            deleted_shares = 0
+            for p_id in to_delete:
+                try:
+                    self.drive.permissions()\
+                        .delete(fileId=drive_id,
+                                permissionId=p_id,
+                                supportsAllDrives=True)\
+                        .execute()
+                    deleted_shares += 1
+                except Exception as e:
+                    logging.error(f"Failed to delete permission {p_id} for "
+                                  + f"drive item ({scope}, {drive_id}): {e}")
+            logging.info(f"Deleted {deleted_shares} permissions for {scope}")
+        else:
+            logging.info("DELETE_OLD_DRIVE_PERMISSIONS is set to false")
 
 
 def new_share_message(scope):

diff --git a/tests/interface/gcp_test.py b/tests/interface/gcp_test.py
@@ -48,7 +48,7 @@ def test_set_drive_permissions(self):
         self.gcp.set_drive_permissions('team', 'abcde', [
             '[email protected]',
             '[email protected]',
-        ])
+        ], delete_permissions=True)
 
         # initial list
         mock_perms.list.assert_called()