Make redirect_uri the primary interface #294
Conversation
Co-authored-by: Aaron Jacobs <[email protected]>
#Conflicts: # NEWS.md # R/oauth-flow-auth-code.R # tests/testthat/test-oauth-flow-auth-code.R
| #' @param host_ip IP address for the temporary webserver used to capture the | ||
| #' authorization code. | ||
| #' @param type Either `desktop` or `web`. Use desktop when running on the | ||
| #' desktop in an environment where you can redirect the user to `localhost`. | ||
| #' Use `web` when running in a hosted web environment. | ||
| #' @param port Port to bind the temporary webserver to. Used only when | ||
| #' `redirect_uri` is `"http(s)://localhost"`. By default, this uses a random | ||
| #' port. You may need to set it to a fixed port if the API requires that the | ||
| #' `redirect_uri` specified in the client exactly matches the `redirect_uri` | ||
| #' generated by this function. | ||
| #' @param port Port to bind the temporary webserver to. |
There was a problem hiding this comment.
Shouldn't the explicit docs for host_ip and port go away, since they are lumped in with the host_name as deprecated in favor of redirect_uri?
| #' | ||
| #' Alternatively, you can provide a URL to a website that uses javascript to | ||
| #' give the user a code to copy and paste back into the R session (see | ||
| #' <https://www.tidyverse.org/google-callback/>, for an example). This is |
There was a problem hiding this comment.
FWIW gargle includes the source for a similar page (but minus the tidyverse branding), in case you want to do the same or point at that one:
https://github.com/r-lib/gargle/blob/main/inst/pseudo-oob/google-callback/index.html
There was a problem hiding this comment.
I haven't traced through the httr2 code enough yet to know if this is already here: But the gargle example doesn't just emit the naked auth code. It checks for a state and then packages/encodes the state and code.
Which then gets decoded and checked:
There was a problem hiding this comment.
Yeah I added support for that in the original PR. It should work.
There was a problem hiding this comment.
Ah, ok. I was just focusing on the specific diffs in front of my 👀
| lifecycle::deprecate_warn("0.3.0", "oauth_flow_auth_code(host_ip)") | ||
| } | ||
|
|
||
| localhost <- parsed$hostname == "localhost" |
There was a problem hiding this comment.
Feels like you might one day have to ponder what happens if someone provides urn:ietf:wg:oauth:2.0:oob as redirect_uri.
There was a problem hiding this comment.
Yeah and that seems like it will be a relatively straighforward change to make.
Fixes #149