-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
check if we really need drive scope #37
Comments
Ok, discussed with @shcheklein and turns out we absolutely need that scope, gdrive implementation can't work without it. |
Does the new(-ish) recommendation for migrating to drive.file scope allow you to re-open? (I can't actually tell if drive.file scope is newer than this issue.) |
@itcarroll thanks! DVC needs full access to a directory - read and write files into it. Would this scope work for this? Also, when you give access to DVC it means you are giving access to the DVC installed on your machine - these keys are saved to your disk, they are not shared with anyone outside. Do you feel this is still problematic for you? |
Just based on reading the documentation, drive.file scope will let you read/write any files/folders created by the client. And the Google Picker API will let you add additional files/folders to the authorized list. In the workflow I'm imagining, you need the Picker once to authorize abc123 from argument gdrive://abc123. As long as I use the same client, everything in that folder would have been created by it, right? (You should get a second opinion from a real developer!) Yes, I know I'm only giving access to an app running on my computer. But since I haven't read through all the code on iterative/dvc, I don't know what the app is doing with its access. Are you suggesting the app can not delete everything from my drive? |
Including files written by a different person into the same location? We need to do some experiments I think.
@itcarroll it should not (and it's not that easy to make a mistake like this- this would require some code, etc), it's a good point though, better scoped credentials are safer and should be considered as a best practice. Even it's not DVC, those credentials might leak, etc. One more thing to clarify - if we can give access to a Team Drive this way ( |
I've started testing out the "drive.file" scope. If you would like to participate, I've set up a toy git + dvc project where we can collaborate. I've also created a google-cloud-project exclusively to use for this test; it's info is in the git repository. By "you" in the above I don't just mean @shcheklein. Add a reaction below and I'll add you as a repo collaborator, test-user for the google-cloud project, and Editor on a shared Google Drive folder. Obviously I'll have to steal your gmail address from somewhere (e.g. your GitHub profile...) |
@itcarroll qq - do you remember by chance how did you initially give access to the app to the initial "root" directory? Did use some kind of picker for that? |
I think I'm hitting this issue for now https://issuetracker.google.com/u/1/issues/239671507?pli=1 . It would be beneficial indeed to have |
Sorry for slow response!
I used the Google API Python Client in a small script to create a google drive folder using the credentials (client_id and client_secret) of a google cloud project (created as per the DVC instructions). Don't know if that all still works, given the new verification stuff you encountered. |
Currently we require
drive
anddrive.appdata
scopes:https://github.com/iterative/dvc/blob/1d9acdc8f3a1574df2b62593bf223ca885daf337/dvc/tree/gdrive.py#L224
drive
is considered to be restricted https://developers.google.com/drive/api/v2/about-auth , so we need to double check if we really need it and if we don't need it - get rid of it to make it more secure.Discord context https://discordapp.com/channels/485586884165107732/563406153334128681/748161223715192863
UPDATEL:
We can research on using the
drive.file
scope instead: #37The text was updated successfully, but these errors were encountered: