-
Notifications
You must be signed in to change notification settings - Fork 25
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
Refactor: gdrive tools auth #2544
base: master
Are you sure you want to change the base?
Conversation
I strongly recommend that all credentials (encrypted or not) be removed from this repository as soon as possible - see advice from Google. The secrets for each client ID should be rotated. The recommended way to authenticate is via Application Default Credentials, but this may need to be considered carefully and done as a separate PR. I'd be happy to assist with all of the above. |
The problem with application default credentials is that every user would have to log onto the google apis dashboard and generate personal credentials, which is quite a technical barrier for users who just want to clone the repo and start syncing their drive content into the app builder. I'm not really sure what the best option would be (seems like what we need is discouraged by google generally), but I'm guessing maybe one of the following might work:
There might be other workarounds, I'm really not up-to-date on google auth methods. Would be keen to hear what you discover and if you have any recommendations for how we can still have a running service without the credentials available |
Looking more at the examples in https://developers.google.com/identity/protocols/oauth2/javascript-implicit-flow, I think it might still be possible to just use the Although at that point I'm guessing we would still want it deployed as a lightweight app/endpoint so that we can restrict domain access to it |
Whatever happens regarding authentication, the existing credentials should be removed from this repo by this PR. Then, new credentials should be created and securely sent to those that need it. I'm assuming those that need credentials, at the moment, are in the IDEMS team or can be contacted directly. I suggest a new issue be created to tackle the auth issue more generally, including whether Application Default Credentials could be part of the solution. |
Removing the credentials as part of this PR doesn't make sense given that this PR removes the alternative workflow that makes use of encrypted credentials. So it essentially would be breaking both methods and not providing any alternatives (no benefit to manually sharing credentials with developers when they can already access the same credentials using the private key to decrypt). So I'll mark this PR as draft for now pending decisions and raise a separate PR to remove the dev credentials so any knock-ons can be better established. #2556 I'm still pretty sure application default credentials are no use here as they authenticate on behalf of the cloud project, and don't provide access to individual user's google drives which is what is required for syncing content. So most likely best option will be a standalone server app to handle authentication where we can embed the credentials directly, but can open up a separate issue to discuss #2557 |
PR Checklist
Description
Noted on mattermost thread, the current system used to authorize access to google drive uses a credential that is encrypted within the scripts config folder. There is also a second set of credentials unencrypted in the gdrive-tools workspace.
This PR removes the encrypted credentials from scripts, in favour of the unencrypted credentials that already exist in the
gdrive-tools
workspace. In turn this removes the need to decrypt the scripts config workspace (code removed and main readme updated accordingly)Author Notes
This should mean that any new author should be able to run the platform without the config
private.key
(still required deploymentprivate.key
if using encrypted deployments)Existing users will likely be prompted to re-authenticate to google drive using the alternate unencrypted credential (no special requirements, just follow prompts when next syncing)
The old credentials
private.key
andtoken.json
file in thepackages/scripts/config
are no longer needed, however I've kept the folder gitignored to prevent anyone accidentally committing the files (can safely be deleted and won't generate in future)Review Notes
If checking out repo for the first time should be able to just import a deployment (with deployment private key if required) and then run sync (should auto-prompt authorization). Hopefully this is a smoother workflow than previous
Dev Notes
I've still kept the option to override credential paths from within the gdrive-tools workspace (in case we ever publish to npm and want to still keep local when calling from scripts), however for now I've removed all options from deployment config and scripts code for working with custom token and credential paths as I fail to see any use-case for them and it over-complicates the code somewhat.
I'm not sure exactly what the implications might be of using the unencrypted dev credentials, however I can't imagine there being much difference between platform developers using these and platform developers using the encrypted version (possibly google starts to limit credentials if a large number of people use them). In any case that issue would be better-served in the long term by registering as a verified google drive application and not rely on local app credentials
As an additional bit of tidying, I noticed the
gdrive-tools
workspace was originally setup to be called fromcli
, however also exposed direct bindings to methods too (lib). The scripts have been using lib bindings for a while now (refactored in #1552) as these support passing filter functions as a function and not stringified base64, so I've opted to remove the cli bindings as I don't think they serve much purpose.Git Issues
Closes #
Screenshots/Videos
If useful, provide screenshot or capture to highlight main changes