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

Create a comand for automated remote configuration (esp. for options requiring authorization) #3455

Open
x0b opened this issue Aug 18, 2019 · 21 comments

Comments

@x0b
Copy link
Contributor

x0b commented Aug 18, 2019

What is your current rclone version (output from rclone version)?

rclone v1.48.0-DEV
- os/arch: windows/amd64
- go version: go1.12.7

However, the behavior was probably introduced in #2472 / 1.43

What problem are you are trying to solve?

Configuring new remotes without using an (unstructured) interactive config session.

Currently, there is at least one remote type (onedrive) that requires using the interactive config dialog to have a working remote. This remote type requires authentication for some of the required configuration options, drive_type and drive_id.

Side note: rclone is a bit inconsistent here. rclone config providers shows these options as "Required": "false".

Effectively, to automate the setup of this remote type you'd have to parse and special-case for this remote type. This affects all GUI-like wrappers for rclone.

How do you think rclone should be changed to solve that?

Auto-configure if possible. If the user only has one drive type and one drive id in his account, this is easy.

For more complicated cases, there should be a command that similar to interactive config, with each invocation returning the next interactive configuration option in a structured format, preferably json.

Imagined User Flow

The imagined feature works like the current implementation until 4., where the additional required parameters can be queried and set.

  1. rclone config create <name> <type> [options]
  2. User is directed to login url and authorizes rclone
  3. rclone saves authorization token
  4. rclone config continue <name> [<key> <value>] [flags]
    Repeat 4. until all required options are set.

E.g. for onedrive
4. rclone config continue <name>
<---

{
   "Status": "more_required",
   "Name": "drive_type",
   "Help": "The type of the drive",
   "Type": "array",
   "Value": [
      {"Value": "onedrive", "Type": "select" }, 
      {"Value": "sharepoint", "Type": "select" }, 
      {"Value": "driveid", "Type": "string" }, 
      {"Value": "siteid", "Type": "string" }, 
      {"Value": "search", "Type": "string" }]
}
  1. rclone config continue <name> "drive_type" "onedrive"
    <---
{
   "Status": "more_required",
   "Name": "drive_id",
   "Help": "The ID of the drive to use",
   "Type": "array",
   "Values": ["752034815", "277173438"]
}
  1. rclone config continue <name> "drive_id" "752034815"
    <---
{
   "Status": "success"
}
@ncw ncw added this to the Help Wanted milestone Aug 20, 2019
@ncw
Copy link
Member

ncw commented Aug 20, 2019

Yes this is a known weakness of the config system.

For autoconfig then rclone normally sets the

  --auto-confirm   If enabled, do not request console confirmation.

This answers Yes to most yes/no questions and is enough to get us through most configuration questions.

You can preload answers to these yes/no questions in the config, using the answer in the tag, eg config_drive_ok set into the config.

backend/onedrive/onedrive.go:                   if !config.ConfirmWithConfig(m, "config_drive_ok", true) {
lib/oauthutil/oauthutil.go:             if !config.ConfirmWithConfig(m, "config_refresh_token", true) {
lib/oauthutil/oauthutil.go:             return config.ConfirmWithConfig(m, "config_is_local", true)

None of that is very elegant or extensible, so a better system would be great!

I'd like it to work via the API and the command line ideally.

So onto your idea...

To make rclone config work in that way would need some way of persisting the state so far - the config file would seem the obvious choice.

The actual things returned should probably be an rclone config option

At the moment the Config() callback has the parameters

Config func(name string, config configmap.Mapper)

This should really return an error instead of exiting fatally, and it should return a pointer to an Option for the next question, so something like this

Config func(name string, config configmap.Mapper, question, answer string) (*fs.Option, error)

On initial call you'd call it

  • Config(name, config, "", "")
  • If it returned non-nil fs.Option then it would save the config
  • on rclone config continue name question answer you would reload the config then call Config(name, config, question, answer) and repeat if necessary

An API like that could work with the API the command line or interactively.

@negative0 have you any thoughts on this? This would make the currently difficult to configure remotes in the web gui (eg onedrive) possible. Assume that we made all the rclone config continue calls into an API?

@x0b
Copy link
Contributor Author

x0b commented Aug 21, 2019

I realized that my initial idea was more complicated that necessary.

To enable configuration with web or other gui state keeping beyond a rclone process lifetime is not actually required. At least for my use case, it could work just like the current rclone config edit, just in a machine readable/writable format with a defined set of input types and values.

@ncw
Copy link
Member

ncw commented Aug 21, 2019

I realized that my initial idea was more complicated that necessary.

To enable configuration with web or other gui state keeping beyond a rclone process lifetime is not actually required. At least for my use case, it could work just like the current rclone config edit, just in a machine readable/writable format with a defined set of input types and values.

Would you want to run it via the API or via the command line?

@x0b
Copy link
Contributor Author

x0b commented Aug 21, 2019

Command line is probably harder to consume since it would require a message separation mechanism (delimiter or a length prefix).

So I would preferably run it over an rc api. And --loopback would either not work or be the same as a command line version.

@ncw
Copy link
Member

ncw commented Aug 22, 2019

If this was only going to work via the API then we could use a simpler method...

However this is a reasonably tricky problem either way... I'll think some more on it!

@x0b
Copy link
Contributor Author

x0b commented Apr 21, 2021

I'm currently planning on moving RCX from the command line to the RC interface. One of the blockers is the ability to configure OAuth based remotes using RC. The last time I tried this using config/create, rclone printed the output on the rcd-hosting process, including the authorization url. This is not very useful.

I would therefore simplify the feature request to the following changes:

  1. When calling config/create and OAuth is required, return the OAuth url in the answer to the config/create call.
  2. Introduce a parameter callback_uri to the config/create method. After authorization, rclone redirects to this URI. Pass a parameter state to this URI which indicates if the authorization attempt was successful.

If this was only going to work via the API then we could use a simpler method...

Do you remember what you had in mind? For my use case, that's all I need.

@ncw
Copy link
Member

ncw commented Apr 22, 2021

I'm currently planning on moving RCX from the command line to the RC interface. One of the blockers is the ability to configure OAuth based remotes using RC. The last time I tried this using config/create, rclone printed the output on the rcd-hosting process, including the authorization url. This is not very useful.

Indeed!

Does rclone's attempt to open a browser window work? If it does or could be made to work, then that would fix the problem for most backends.

I would therefore simplify the feature request to the following changes:

  1. When calling config/create and OAuth is required, return the OAuth url in the answer to the config/create call.
  2. Introduce a parameter callback_uri to the config/create method. After authorization, rclone redirects to this URI. Pass a parameter state to this URI which indicates if the authorization attempt was successful.

This won't quite fulfill all the configuration needs - some remotes have a post-token config step - eg google drive lets you choose a team drive (as you noted in your original post).

If this was only going to work via the API then we could use a simpler method...

Do you remember what you had in mind? For my use case, that's all I need.

If it is only going to work via the API it doesn't need to persist stuff which makes the coding much easier.

I think my preference would be to implement something like your original proposal a state based call and response system for the extra config.

Though I did just have this idea... We are asking for user interaction via the browser for oauth so rclone could do its post config steps in the browser, so direct with the user?

@x0b
Copy link
Contributor Author

x0b commented Apr 22, 2021

Does rclone's attempt to open a browser window work? If it does or could be made to work, then that would fix the problem for most backends.

Unfortunately no (#3686), RCX currently emulates an interactive user to "solve" this issue.

Though I did just have this idea... We are asking for user interaction via the browser for oauth so rclone could do its post config steps in the browser, so direct with the user?

I also had that idea. Potential downside: no localization. Other than that it should work.

I think my preference would be to implement something like your original proposal a state based call and response system for the extra config.

No objections.

@ncw
Copy link
Member

ncw commented Apr 25, 2021

Does rclone's attempt to open a browser window work? If it does or could be made to work, then that would fix the problem for most backends.

Unfortunately no (#3686), RCX currently emulates an interactive user to "solve" this issue.

I just read through that thread. Should we put the effort into making it work? Would it solve your problem?

I can have a go at hacking in the "am", "start",

I don't think the skratchdot/open-golang module is being maintained.

Though I did just have this idea... We are asking for user interaction via the browser for oauth so rclone could do its post config steps in the browser, so direct with the user?

I also had that idea. Potential downside: no localization. Other than that it should work.

OK

I think my preference would be to implement something like your original proposal a state based call and response system for the extra config.

No objections.

👍

@ncw ncw self-assigned this Apr 25, 2021
@x0b
Copy link
Contributor Author

x0b commented Apr 26, 2021

I can have a go at hacking in the "am", "start"

No, I tried that (#3686 (comment)). It didn't work because am start only works for root (and maybe adb).

Details, if you really want to go down the rabbit hole.

...the direct route with opening the browser from golang won't work due to the android permission model.

What would most likely work is if rclone would ask the parent app process to open the browser (like termux, but without an additional xdg-open shim binary). However, at that point we're back where we started since we would need to define an interface for that message flow, i.e. an RC interface.

In other words, rclone could do exactly the same as termux:

am broadcast --user 0 \
 -a android.intent.action.VIEW \
 -n $HOST_APP_ID/$BROADCAST_RECEIVER  \
 -d "$URL" \
 > /dev/null

But this does not open the browser - it just sends a message to the parent app that the browser should be opened with $URL. Which would have the same effect as RC, just over a different, Android-specific IPC channel.

So the answer to this question...

Should we put the effort into making it work?

.... is still no.


Maybe I should explain why I re-activated this issue now. From my first tests it looks like compiling and using rclone as a shared library on Android massively improves command latency and removes TCP/IP as source of failures.

However, in a shared library world, there is no stand-alone rclone process any more, which means no clever workarounds with reading from and writing to rclones' standard input and output pipes.

BTW: I just tested this - the web GUI has the same problem when running in docker (i.e. broken OAuth when creating a remote config). RC based config & OAuth would help there as well.

@ncw
Copy link
Member

ncw commented Apr 27, 2021

I can have a go at hacking in the "am", "start"

No, I tried that (#3686 (comment)). It didn't work because am start only works for root (and maybe adb).

Details, if you really want to go down the rabbit hole.

Very interesting details - thanks!

So the answer to this question...

Should we put the effort into making it work?

.... is still no.

:-)

Maybe I should explain why I re-activated this issue now. From my first tests it looks like compiling and using rclone as a shared library on Android massively improves command latency and removes TCP/IP as source of failures.

That is really useful to know - thanks.

However, in a shared library world, there is no stand-alone rclone process any more, which means no clever workarounds with reading from and writing to rclones' standard input and output pipes.

Gotcha.

BTW: I just tested this - the web GUI has the same problem when running in docker (i.e. broken OAuth when creating a remote config). RC based config & OAuth would help there as well.

I think this is broken for two reasons 1) the webserver not opening and 2) rclone's webserver socket not being exported.

Are you still happy for rclone's webserver to

  • redirect the oauth to the correct place (so you open http://127.0.0.1:53682/auth?state=XXX in the browser)
  • receive the oauth result (directly from the browser)

Assuming that is the case, let's look at your original proposal again

  1. When calling config/create and OAuth is required, return the OAuth url in the answer to the config/create call.

Rclone would need to

  • set up oauth
  • start webserver running in the background
  • return URL to start the process
  1. Introduce a parameter callback_uri to the config/create method. After authorization, rclone redirects to this URI. Pass a parameter state to this URI which indicates if the authorization attempt was successful.

A call back URI would be easy enough - is that easy for you? I guess it gets rid of the return to rclone screen. We could make the text in that configurable if that would be useful so it could at least say return to RCX.

However I think polling the API might be more regular - that could then return the success/failure and the next step of config (if any).

We could do both easily enough.

This needs some thought about how long that webserver stays running for - should it time out?

If this was only going to work via the API then we could use a simpler method...

Do you remember what you had in mind? For my use case, that's all I need.

I think we should target this just at the RC API. This means not having to persist anything to disk and with librclone will be easy for any integration eventually.

Rclone changes needed

We need to think about the extra config steps. Here is what we might need to do for rclone.

  • split the backend config into two calls OAuth and Config. The OAuth call will run the Oauth server as it does at the moment with the above modification. The Config call will be a call and response system for setting the extra config once the token is set.

The OAuth call can just return parameters suitable for calling oauthutil.Config - these might be nil if the backend decides it doesn't want to do oauth (eg Google Drive with a service account). This can then be split up as required for the new API.

The Config call should have input something like this

  • State string where we are in the process - start the process with ""
  • Result string result of the previous config step or "" on first call
{
   "State": "more_required", 
   "Result": "previous result",
}

This should then return

  • State string - state value to use on next call - "" for end
  • Error string - to show to user (may contain \n) if the config value passed in was incorrect for some reason
  • Help string - to show to user (may contain \n)
  • []Choices object - choices for the user to choose from (may be empty)
  • FreeForm bool - if set allows the user to type any string not just choose one of the choices

Note that all rclone's config is done with strings which makes life easy.

{
   "State": "more_required", 
   "Error": "",
   "Help": "The type of the drive",
   "Choices": [
      {"Value": "onedrive", "Description": "Onedrive main site" }, 
      {"Value": "sharepoint", "Description": "Sharepoint other site" }, 
      {"Value": "driveid", "Description": "Drive ID or something" }, 
   ],
   "FreeForm": false
}

Config should be called until it returns state as empty string.

I think we can massage each backend into this state. The OAuth call will simplify the code in most backends.

How does that sound to you?

It would be possible to make this work with the command line too if intermediate state is persisted into the config file.

@x0b
Copy link
Contributor Author

x0b commented Apr 28, 2021

Sounds good, the only things to add:

A call back URI would be easy enough - is that easy for you? I guess it gets rid of the return to rclone screen. We could make the text in that configurable if that would be useful so it could at least say return to RCX.

With a callback URI, users could automatically be redirected into the app and the app would know when the OAuth attempt is finished. The other ideas (custom text, polling API) would also work, although with a user experience that is not quite as smooth. Not every user understands that they should close the browser tab and return to the app after the OAuth authorization process.

This needs some thought about how long that webserver stays running for - should it time out?

That is one option. Alternatively, the webserver could be stopped over RC. This is what RCX currently does (by killing the rclone process) if a user starts another OAuth config without completing the first one.

If there's something a non-go programmer like me can do for this feature just point me in the right direction.

ncw added a commit that referenced this issue May 2, 2021
This is a very large change which turns the post Config function in
backends into a state based call and response system so that
alternative user interfaces can be added.

The existing config logic has been converted, but it is quite
complicated and folloup commits will likely be needed to fix it!

Follow up commits will add a command line and API based way of using
this configuration system.
@ncw
Copy link
Member

ncw commented May 2, 2021

I have pushed the first draft of a new config system in the branch fix-3455-config-api

There are some docs on how it works.

I've also converted all the backends to use the new system of config.

@buengese I was wondering if you've got the time to check this code out and have a look at it? In particular the config for the Jottacloud backend was extremely complicated and I've only been able to test some of the paths since I don't have access to the other types of jotta backends. If you take a look at the Config function in the jottacloud backend

// Config runs the backend configuration protocol
func Config(ctx context.Context, name string, m configmap.Mapper, config fs.ConfigIn) (*fs.ConfigOut, error) {

You can get an idea of the code. Unfortunately I had to inline quite a lot of factored out functions to get them returning the questions to the user in the right place.

Any help much appreciated!

@buengese
Copy link
Member

buengese commented May 3, 2021

I'll take a look at it later today. I've been meaning to have another look at Jottacloud authentication for a while now. It's been a bit of mess for a while now and can probably be simplified a bit.

@buengese
Copy link
Member

buengese commented May 4, 2021

I've added a commit to your branch that fixes the jottacloud legacy config and also done some minor rearrangement. Should probably just be squashed in. All paths I could test are now working.

@ncw
Copy link
Member

ncw commented May 7, 2021

I've merged that now thanks @buengese I'm going to try to land this branch soon so we can fix any breakage before the release!

ncw added a commit that referenced this issue May 11, 2021
This is a very large change which turns the post Config function in
backends into a state based call and response system so that
alternative user interfaces can be added.

The existing config logic has been converted, but it is quite
complicated and folloup commits will likely be needed to fix it!

Follow up commits will add a command line and API based way of using
this configuration system.
ncw added a commit that referenced this issue May 11, 2021
This adds a mechanism to add external interfaces to rclone using the
state based configuration.
@ncw
Copy link
Member

ncw commented May 14, 2021

I've merged the state of play so far into master so this can get more testing before the release. I suspect this will break stuff so testing appreciated!

@x0b
Copy link
Contributor Author

x0b commented May 14, 2021

This looks really good, finally managed to test it a bit (over HTTP). Three observations:

  1. Google drive: When answering 'teamdrive?' yes if the account has no team drives, I receive an error response and State is empty. The remote is then not saved and I need to restart the whole process to retry. When doing the same on the interactive config, rclone also complains, but then still allows me to save the remote without the team drive. Intentional?
  2. Is onedrive broken or am I doing something wrong? Fatal error: failed to configure OneDrive: empty token found - please run "rclone config reconnect onedrive-test1:" - both in interactive and over rc.
  3. Docs: Some examples as to how a complete JSON response looks would be nice. I also take from your example code that 'Exclusive': true means that clients should validate => more documentation.

ncw added a commit that referenced this issue May 17, 2021
This bug was caused as part of the config rework
@ncw
Copy link
Member

ncw commented May 17, 2021

@x0b wrote:

This looks really good, finally managed to test it a bit (over HTTP).

Great!

Three observations:

  1. Google drive: When answering 'teamdrive?' yes if the account has no team drives, I receive an error response and State is empty. The remote is then not saved and I need to restart the whole process to retry. When doing the same on the interactive config, rclone also complains, but then still allows me to save the remote without the team drive. Intentional?

I've been unable to replicate this with bin/config.py.

I get the error

{'Error': 'No Shared Drives found in your account',
 'Option': None,
 'Result': '',
 'State': ''}

But the config is saved. I tried this over http to a persistent rclone rcd as well as via the command line with rc --loopback.

Can you replicate the problem with config.py?

  1. Is onedrive broken or am I doing something wrong? Fatal error: failed to configure OneDrive: empty token found - please run "rclone config reconnect onedrive-test1:" - both in interactive and over rc.

Onedrive is broken :-)

I've fixed that now - will merge to master once the tests have passed.

  1. Docs: Some examples as to how a complete JSON response looks would be nice. I also take from your example code that 'Exclusive': true means that clients should validate => more documentation.

I updated the docs a bit like this - let me know if there is anything missing.


The flag --non-interactive is for use by applications that wish to
configure rclone themeselves, rather than using rclone's text based
configuration questions. If this flag is set, and rclone needs to ask
the user a question, a JSON blob will be returned with the question in
it.

This will look something like (some irrelevant detail removed):

{
    "State": "*oauth-islocal,teamdrive,,",
    "Option": {
        "Name": "config_is_local",
        "Help": "Use auto config?\n * Say Y if not sure\n * Say N if you are working on a remote or headless machine\n",
        "Default": true,
        "Examples": [
            {
                "Value": "true",
                "Help": "Yes"
            },
            {
                "Value": "false",
                "Help": "No"
            }
        ],
        "Required": false,
        "IsPassword": false,
        "Type": "bool",
        "Exclusive": true,
    },
    "Error": "",
}

The format of Option is the same as returned by rclone config providers. The question should be asked to the user and returned to
rclone as the --result option along with the --state parameter.

The keys of Option are used as follows:

  • Name - name of variable - show to user
  • Help - help text. Hard wrapped at 80 chars. Any URLs should be clicky.
  • Default - default value - return this if the user just wants the default.
  • Examples - the user should be able to choose one of these
  • Required - the value should be non-empty
  • IsPassword - the value is a password and should be edited as such
  • Type - type of value, eg bool, string, int and others
  • Exclusive - if set no free-form entry allowed only the Examples
  • Irrelevant keys Provider, ShortOpt, Hide, NoPrefix, Advanced

If Error is set then it should be shown to the user at the same
time as the question.

rclone config update name --continue --state "*oauth-islocal,teamdrive,," --result "true"

Note that when using --continue all passwords should be passed in
the clear (not obscured). Any default config values should be passed
in with each invocation of --continue.

At the end of the non interactive process, rclone will return a result
with State as empty string.

If --all is passed then rclone will ask all the config questions,
not just the post config questions. Any parameters are used as
defaults for questions as usual.

Note that bin/config.py in the rclone source implements this protocol
as a readable demonstration.


Here is the config.py demo program

def ask(opt):

ncw added a commit that referenced this issue May 17, 2021
This bug was caused as part of the config rework
@ncw
Copy link
Member

ncw commented May 17, 2021

I've merged the fixes for the above now to master.

ncw added a commit that referenced this issue Aug 11, 2021
…5525

In this commit the config system was re-arranged

    94dbfa4 fs: change Config callback into state based callback #3455

This passed the password as a temporary config parameter but forgot to
reveal it in the API call.
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
This is a very large change which turns the post Config function in
backends into a state based call and response system so that
alternative user interfaces can be added.

The existing config logic has been converted, but it is quite
complicated and folloup commits will likely be needed to fix it!

Follow up commits will add a command line and API based way of using
this configuration system.
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
…#3455

This adds a mechanism to add external interfaces to rclone using the
state based configuration.
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
…estions rclone#3455

This also factors the config questions into a state based mechanism so
a backend can be configured using the same dialog as rclone config but
remotely.
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
This bug was caused as part of the config rework
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
ncw added a commit that referenced this issue Aug 14, 2021
…5525

In this commit the config system was re-arranged

    94dbfa4 fs: change Config callback into state based callback #3455

This passed the password as a temporary config parameter but forgot to
reveal it in the API call.
ncw added a commit that referenced this issue Aug 14, 2021
…5525

In this commit the config system was re-arranged

    94dbfa4 fs: change Config callback into state based callback #3455

This passed the password as a temporary config parameter but forgot to
reveal it in the API call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants