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

[WIP] All Backends: add --backend-token flag - fixes #2849 #2852

Closed

Conversation

jake-stripe
Copy link
Contributor

@jake-stripe jake-stripe commented Dec 20, 2018

Note: The scope of this PR now includes adding a similar token flag to each backend in Rclone. (In addition to the original Dropbox request.)

What is the purpose of this change?

Add a --dropbox-token flag so that a user is able to supply an Access Token (as a JSON blob) outside of the default rclone.conf file. Use of this option (in conjunction with --dropbox-client-id
and --dropbox-client-secret) would allow users to to keep all plaintext credentials off-disk.

Was the change discussed in an issue or in the forum before?

No, just in the Github issue section. Please see #2849.

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)

@ncw
Copy link
Member

ncw commented Dec 30, 2018

Sorry for the delay in replying to this! Christmas plus Norovirus has taken its toll!

What you've written will work fine. I'm just wondering if it can or should be generalised.

All the oauth based remotes could potentially do with this.

$ git grep -l oauth2 backend/
backend/amazonclouddrive/amazonclouddrive.go
backend/box/box.go
backend/drive/drive.go
backend/dropbox/dropbox.go
backend/googlecloudstorage/googlecloudstorage.go
backend/hubic/hubic.go
backend/onedrive/onedrive.go
backend/pcloud/pcloud.go
backend/yandex/yandex.go

So we could extend your patch do do all of the backends like that? Or possibly be a bit more clever and reduce the code duplication a bit.... Maybe making an oauthutil.AddOptions() which would take a fs.Option and return another fs.Option with the oauthutil options in.

Likewise we should be able to do actually setting the token entirely in the oauthutil module I think.

What do you think?

@jake-stripe
Copy link
Contributor Author

Hey Nick,

Happy New Year! No worries about the delay, I've been on holiday as well. I hope you are feeling better post-Norovirus 🤒.

I agree with you that it would be nice to have this feature available regardless of the backend being utilized. I would be happy to take a crack at implementing; though i'm not 100% sure if I'd be able to test all the backends (I only currently have Dropbox + Google Drive creds).

With regard to the current PR and failed Travis CI build, I'm having trouble understanding what the exact error is during the make check run (I understand its breaking on file backend/dropbox/dropbox.go, line 262, char 18, but little else). If you have the time (no rush), would you mind helping me understand the output? Or pointing me at a resource that you maybe would recommend?

Thanks again,

-Jake

@jake-stripe jake-stripe changed the title [WIP] dropbox: add --dropbox-token flag - fixes #2849 [WIP] All Backends: add --backend-token flag - fixes #2849 Jan 2, 2019
@ncw
Copy link
Member

ncw commented Jan 3, 2019

Happy New Year! No worries about the delay, I've been on holiday as well. I hope you are feeling better post-Norovirus .

Thanks :-)

I agree with you that it would be nice to have this feature available regardless of the backend being utilized. I would be happy to take a crack at implementing; though i'm not 100% sure if I'd be able to test all the backends (I only currently have Dropbox + Google Drive creds).

Great!

The daily integration tests will run the backends against all the different providers so that will test that basic functionality isn't broken at least!

With regard to the current PR and failed Travis CI build, I'm having trouble understanding what the exact error is during the make check run (I understand its breaking on file backend/dropbox/dropbox.go, line 262, char 18, but little else). If you have the time (no rush), would you mind helping me understand the output? Or pointing me at a resource that you maybe would recommend?

This is the relevant part of the log. What it means is that errcheck is complaining that config.SetValue returns an error but it didn't get checked.

I'd like to make these code quality checks more obvious - I have a branch with gometalinter doing it which produces better error messages, but I couldn't get it to work with build tags.

errcheck -tags "cmount" ./...
backend/dropbox/dropbox.go:262:18:	config.SetValue(name, config.ConfigToken, opt.Token)
make: *** [check] Error 1

@ncw
Copy link
Member

ncw commented May 11, 2019

Sorry for the loooong delay responding. Do you still want to work on this? I think the feature is a useful one, but only if we can make it for all backends that need it.

@jake-stripe
Copy link
Contributor Author

@ncw I agree that this is an important feature, but unfortunately I won't be able to dedicate time to work on it in the near future. I think it would be best to hand this off to someone else. I'll go ahead and close the request.

@ncw
Copy link
Member

ncw commented May 13, 2019

Thanks for your work on this - I'll pull it into a branch and have a think about how to continue it.

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.

None yet

2 participants