Skip to content
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

Google Drive Integration #867

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

ngharrison
Copy link

This implementation reflects what was done for syncing with Dropbox.

The code is there, but it's not working yet because the mDriveService becomes null before it is used to access the API. I believe this requires better knowledge of the app system that's already set up, so I'm starting the discussion here.

@sjhuang26
Copy link

I personally would love a Google Drive sync feature and am looking to work on this issue.

According to the maintainer of another popular open-source note-taking app [1], Google policy requires an expensive "independent Google-approved audit" to use the necessary APIs.

However, we can use a scope like https://www.googleapis.com/auth/drive.file for which the policy doesn't apply (because the scope is non-restricted) and which should cover all of the user's needs. The app can "view and manage Google Drive files and folders that you have opened or created with this app" [3]. Note that "folders are treated as a type of file" [4].

Even if we needed a restricted scope, according to the official documentation [2], "for apps using restricted scopes, a restricted scope verification must be performed ... if you store restricted scope data on servers (or transmit), then you need to go through a security assessment" from "third-party vendors." Since we don't store user data, we don't need the security assessment.

@nevenz @ngharrison Would a third-party security assessment be required? IMO it's not.

[1] laurent22/joplin#402 (comment)
[2] https://developers.google.com/drive/api/v3/about-auth
[3] https://developers.google.com/identity/protocols/oauth2/scopes#drive
[4] https://developers.google.com/drive/api/v3/about-files

@ngharrison
Copy link
Author

Hey @sjhuang26, I'm not sure about the security assessment. You already know more about it than I do.

As for the code side, I'm not working on this anymore -- other priorities. I made a few more attempts after making this PR to fix the issues I was having and I could push those changes. From the current commit, the code builds, but any Drive functions fail because the Drive object becomes null. I tried moving the creation of that object into GoogleDriveClient.java from GoogleDriveRepoActivity.kt, but I couldn't get it to build.

One of my hopeful assumptions was that GoogleSignIn would handle the authentication tokens for me. I'm not sure if this was one of the problems, but it might have to be handled manually, how DropboxClient.java does it.

@ngharrison
Copy link
Author

Just added a commit of the most recent attempts.

@ngharrison ngharrison force-pushed the master branch 2 times, most recently from 72812eb to f62a351 Compare June 24, 2022 23:32
@ngharrison
Copy link
Author

ngharrison commented Jul 4, 2022

Google Drive integration is now complete and working as far as I've used it. I've made it match the Dropbox functionality and integration within the app as exactly as possible. I haven't made any tests, but if there is actual interest in getting this merged, please let me know.

@sjhuang26 @nevenz

@ngharrison
Copy link
Author

Verifying the app through Google and deciding the right permissions would also need to be handled. Since I've been developing it on my own, I have set up an app certificate through my account, have set the permissions to full drive access, and have been accepting them unverified.

@ngharrison
Copy link
Author

This would close #25.

@codemac
Copy link

codemac commented Dec 4, 2022

There is also the option to just have a page about how to set up your own app certificate for each user?

This is how many open source apps got around the "security audit" requirements.

@marcel-valdez
Copy link

Is anyone working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants