Suggestion: Pass through auth_url from oauth_client$auth_params to oauth_flow/req_oauth

[thohan88]

This may just make sense to me, but I think it would be nice if auth_url (and potentially pkce and redirect_uri) could be included in auth_params. This would make oauth_client a self-contained object which is easier to pass around, especially in the context of multiple clients.

Current situation: Currently, when using oauth_client(), auth_url pkce and redirect_uri need to be specified separately in the flow:

github_client <- oauth_client(
  id = "",
  secret = "",
  token_url = "https://github.com/login/oauth/access_token"
)

oauth_flow_auth_code(
  client = github_client,
  auth_url = "https://github.com/login/oauth/authorize",
  redirect_uri = "http://127.0.0.1:1410"
  pkce = FALSE
)

Proposed Change Override arguments specified within auth_params of the oauth_client object, so that the flow can be simplified:

github_client <- oauth_client(
  id = "",
  secret = "",
  token_url = "https://github.com/login/oauth/access_token",
  auth_params = list(
    auth_url = "https://github.com/login/oauth/authorize",
    redirect_uri = "http://localhost:1410",
    pkce = FALSE
  )
)

oauth_flow_auth_code(client = github_client)

Caveats The above example would of course break if you attempt a device flow:

oauth_flow_device_code(client = github_client)

Which could be overriden with:

oauth_flow_device_code(client = github_client, auth_url = "https://github.com/login/device/code")

Alternatively, one could have auth_url_auth_code and auth_url_device_code, but this feels more clunky. I think most users will rarely mix auth code and device code flows, but not sure here.

Context I'm working on a PR draft for Shiny integration that supports multiple providers. During this process, I found it cleaner and more intuitive to provide a list of oauth_clients rather than managing parameters separately for each client. Fully understand there may be good reasons they are separate today.

[hadley]

I'll contemplate this more when I'm next deep in OAuth, but my recollection that this is deliberate, in order to keep the parameters better matching the objects in the OAuth specs. (I also vaguely remember that httr lumped them altogether and that seemed like a bad idea once I understand OAuth better)

[thohan88]

After working a bit further on this, I agree!

I settled on a separate oauth_shiny_client() instead in #518.