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

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

Closed
thohan88 opened this issue Jun 27, 2024 · 2 comments

Comments

@thohan88
Copy link

thohan88 commented Jun 27, 2024

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
Copy link
Member

hadley commented Jul 10, 2024

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
Copy link
Author

After working a bit further on this, I agree!

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

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

2 participants