feat: google drive permissions sync #484

bobheadxi · 2020-09-24T08:15:05Z

Pull Request

Description

Give a brief description of your changes:

Enables an optional step to team refresh, enabled by providing GCP_SERVICE_ACCOUNT_CREDENTIALS, to synchronize team folders by sharing each team's configured folder to all emails provided by that team's members.

The current intended use case for this is only for the top-level folders indicated in https://docs.ubclaunchpad.com/handbook/tools/drive - i.e I will be assigning the following folders:

all: Projects
leads: UBC Launch Pad
design: Design
strategy: Strategy

As such, no care has been taken for certain edge cases (ie "what if I don't want to unshare with anyone not on a rocket team?") - for this iteration of this feature, I am going to keep it simple.

The goal is to remove one of the last steps that leads have to do from onboarding - see ubclaunchpad/LP-website#175 . It currently requires a refresh - see #490, #495

Testing

If testing this change requires extra setup, please document it here:

Simple unit test provided. ~~Working on a clicky-click test as well.~~

Update - verified this works!

Ticket(s)

Closes #284

(Create a copy of that line for each Github Issue affected,
and replace "Affects" with "Closes" if merging this will close the relevant ticket.)

codecov · 2020-09-24T09:29:19Z

Codecov Report

Merging #484 into master will decrease coverage by 1.17%.
The diff coverage is 63.82%.

@@            Coverage Diff             @@
##           master     #484      +/-   ##
==========================================
- Coverage   94.83%   93.66%   -1.18%     
==========================================
  Files          44       45       +1     
  Lines        2403     2495      +92     
  Branches      303      320      +17     
==========================================
+ Hits         2279     2337      +58     
- Misses         80      110      +30     
- Partials       44       48       +4

Impacted Files	Coverage Δ
app/controller/command/commands/team.py	`84.17% <27.02%> (-5.89%)`	⬇️
interface/gcp.py	`85.71% <85.71%> (ø)`
app/controller/command/parser.py	`96.66% <100.00%> (+0.11%)`	⬆️
app/model/team.py	`98.50% <100.00%> (+0.06%)`	⬆️
config/__init__.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5af07ad...62b9e2e. Read the comment docs.

…ermissions-sync

chuck-sys

Needs a bit more testing.

chuck-sys · 2020-09-24T15:32:15Z

app/controller/command/commands/team.py

+    def refresh_drive_permissions(self):
+        """
+        Refresh Google Drive permissions based on user role. If no GCP client
+        is provided, this function is a no-op.
+        """
+
+        if self.gcp is None:
+            logging.debug("GCP not enabled, skipping drive permissions")
+            return
+
+        all_teams: List[Team] = self.facade.query(Team)
+        for t in all_teams:
+            if len(t.folder) == 0:
+                continue
+
+            emails: List[str] = []
+            for m in t.members:
+                if len(m.email) > 0:
+                    emails.append(m.email)
+
+            if len(emails) > 0:
+                logging.info("Synchronizing permissions for "
+                             + f"{t.github_team_name}'s folder ({t.folder}) "
+                             + f"to {emails}")
+                self.gcp.set_drive_permissions(
+                    t.folder, t.github_team_name, emails)


You may want to run a more specialized version of this function, specific to the team that called with a --folder, to sync permissions for that team's team members only.

Something similar to what happens when we do /rocket user edit --github and we do an invite for that github user to join our organization.

Do you think it's necessary at the moment? Happy to implement this in a follow-up instead :)

app/model/team.py

…ermissions-sync

chuck-sys

Okay so here's the thing. /rocket team refresh takes around 20 seconds to execute (last I checked). The more members we have in Launch Pad, the longer it takes, O(n). We'll still need ways for users to receive the permission sharing automatically, whether if it is a user just adding their emails, or a team adding a folder ID into the system.

I am very adamant about not using /rocket team refresh unless we really need to because ... there are lots of side effects that I don't know about, that may be disruptive. For example, today I found out that it would forcefully add a user back into the organization if the user was in ddb but was already out of the org (one possibility would be if the user ID was still in a team in ddb).

Will have to implement this as a follow-up. For now I'm approving this.

chuck-sys

Okay so here's the thing. /rocket team refresh takes around 20 seconds to execute (last I checked). The more members we have in Launch Pad, the longer it takes, O(n). We'll still need ways for users to receive the permission sharing automatically, whether if it is a user just adding their emails, or a team adding a folder ID into the system.

I am very adamant about not using /rocket team refresh unless we really need to because ... there are lots of side effects that I don't know about, that may be disruptive. For example, today I found out that it would forcefully add a user back into the organization if the user was in ddb but was already out of the org (one possibility would be if the user ID was still in a team in ddb).

Will have to implement this as a follow-up. For now I'm approving this.

bobheadxi · 2020-09-25T05:23:34Z

@cheukyin699 agreed and noted! I've created #490 to cover that

bobheadxi requested a review from chuck-sys September 24, 2020 08:15

bobheadxi requested review from a team as code owners September 24, 2020 08:15

bobheadxi marked this pull request as draft September 24, 2020 08:15

bobheadxi mentioned this pull request Sep 24, 2020

add email to rocket setup ubclaunchpad/LP-website#174

Merged

bobheadxi added 3 commits September 24, 2020 16:32

chore: remove old pydocstyle check

c89c6f0

chore: move credentials to own ignore category

e4dfd13

chore: unify linting

a99f9d5

bobheadxi force-pushed the drive-permissions-sync branch 4 times, most recently from ea7e807 to b971333 Compare September 24, 2020 09:25

bobheadxi force-pushed the drive-permissions-sync branch 2 times, most recently from 319d1df to becd800 Compare September 24, 2020 13:17

feat: add optional google drive permissions sync

ccee0bd

bobheadxi force-pushed the drive-permissions-sync branch from becd800 to ccee0bd Compare September 24, 2020 14:32

bobheadxi mentioned this pull request Sep 24, 2020

docs: update local dev steps, unify permissions in config #485

Merged

Merge branch 'master' of github.com:ubclaunchpad/rocket2 into drive-p…

154031c

…ermissions-sync

bobheadxi marked this pull request as ready for review September 24, 2020 14:59

chuck-sys requested changes Sep 24, 2020

View reviewed changes

bobheadxi added 2 commits September 25, 2020 09:36

interface: add more logging for drive permissions

eeb42dd

model: add folder to team from/to dict

a4ac605

bobheadxi force-pushed the drive-permissions-sync branch from e5aae9a to a4ac605 Compare September 25, 2020 01:36

bobheadxi added 3 commits September 25, 2020 11:25

config: add support for gcp actor

8da0c01

gcp: fix all usages of gcp drive apis

133171e

Merge branch 'master' of github.com:ubclaunchpad/rocket2 into drive-p…

0b6ac69

…ermissions-sync

bobheadxi requested a review from chuck-sys September 25, 2020 03:28

tests: fix gcp test

62b9e2e

bobheadxi mentioned this pull request Sep 25, 2020

note that github, drive access should only be done through rocket ubclaunchpad/LP-website#175

Merged

bobheadxi added the 👁Ready for Review label Sep 25, 2020

bobheadxi requested a review from johnagl September 25, 2020 03:58

chuck-sys mentioned this pull request Sep 25, 2020

Cheukyin699/fix docs again and again #489

Merged

chuck-sys reviewed Sep 25, 2020

View reviewed changes

chuck-sys approved these changes Sep 25, 2020

View reviewed changes

bobheadxi mentioned this pull request Sep 25, 2020

reduce reliance on 'team refresh' #490

Open

2 tasks

bobheadxi merged commit a7cfd72 into master Sep 25, 2020

bobheadxi deleted the drive-permissions-sync branch September 25, 2020 05:23

bobheadxi mentioned this pull request Sep 25, 2020

trigger permissions sync when user edits their email #495

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: google drive permissions sync #484

feat: google drive permissions sync #484

bobheadxi commented Sep 24, 2020 •

edited

Loading

codecov bot commented Sep 24, 2020 •

edited

Loading

chuck-sys left a comment

chuck-sys Sep 24, 2020

bobheadxi Sep 25, 2020

chuck-sys left a comment •

edited

Loading

chuck-sys left a comment

bobheadxi commented Sep 25, 2020

feat: google drive permissions sync #484

feat: google drive permissions sync #484

Conversation

bobheadxi commented Sep 24, 2020 • edited Loading

Pull Request

Description

Testing

Ticket(s)

codecov bot commented Sep 24, 2020 • edited Loading

Codecov Report

chuck-sys left a comment

Choose a reason for hiding this comment

chuck-sys Sep 24, 2020

Choose a reason for hiding this comment

bobheadxi Sep 25, 2020

Choose a reason for hiding this comment

chuck-sys left a comment • edited Loading

Choose a reason for hiding this comment

chuck-sys left a comment

Choose a reason for hiding this comment

bobheadxi commented Sep 25, 2020

bobheadxi commented Sep 24, 2020 •

edited

Loading

codecov bot commented Sep 24, 2020 •

edited

Loading

chuck-sys left a comment •

edited

Loading