OAuth docs #334

hadley · 2023-10-08T20:07:08Z

To do:

Add a section on caching that's inherited across all pages
Check all args passed through
Move cache_key and cache_disk to end of arg list
Review reference index
Update vignette

Fixes #330

hadley · 2023-10-08T22:24:03Z

@llrs please take a look and let me know if this makes it more clear how to actually authenticate a request with OAuth.

@maelle I'd love your feedback too, if you have time.

R/oauth-flow-auth-code.R

Co-authored-by: Maximilian Girlich <maximilian.girlich@metoda.com>

llrs

I see that now most of the documentation has moved to the req_oauth_auth_code help page and then some sections reused in other pages (But there are some missing the cache section).

I see oauth_flow_auth_code_url is now internal but not oauth_flow_auth_code_listen, oauth_flow_auth_code_parse or oauth_flow_auth_code_pkce. I'm not sure if this was on purpose or not.

I've left some comments in the vignette.
There is also a comment on a help page that hasn't changed in this PR, but I think it is in line with the title of the PR, and might help too other readers.

Thanks for keeping me in the loop.

I tried building the pkgdown to easily review the changes but I couldn't build it:

pkgdown::build_site()
## ...
## Reading 'vignettes/httr2.Rmd'
## -- RMarkdown error -------------------------------------------------------------
## 
## Quitting from lines 35-37 [unnamed-chunk-2] (httr2.Rmd)
## --------------------------------------------------------------------------------
## Error: 
## ! in callr subprocess.
## Caused by error in `map(.x, .f, ..., .progress = .progress)`:
## ! In index: 3.
## ℹ See `$stdout` for standard output.
## Type .Last.error to see the more details.

vignettes/articles/oauth.Rmd

R/oauth-flow-refresh.R

hadley · 2023-10-11T21:07:10Z

Thanks @llrs! All the flows that can use caching should have the caching section, but quite a few don't support caching.

FWIW it's not the functions that are internal or doc, but the doc page, and all those functions are documented together.

#Conflicts: # NEWS.md

maelle

Great to see more OAuth docs!

Before reviewing I re-read my own blog post and saw that in a comment I had wondered whether things would be simpler with httr2 😉 r-hub/blog#155 (comment)

I added a few comments.

vignettes/articles/oauth.Rmd

maelle · 2023-10-12T05:56:38Z

vignettes/articles/oauth.Rmd

+This is going to be painful, but unfortunately there's no way around it.
+I recommend using `with_verbosity()` so you can see exactly what httr2 is sending to the server.
+You'll then need to carefully compare this to the API documentation and play "spot the difference".
+You're very welcome to file an issue and I'll do my best to help you out.


also point to the community forum? it might be less intimidating.

Less intimidating, but I don't read them so also less helpful 😆

maelle · 2023-10-12T05:58:52Z

R/oauth-flow-auth-code.R

+#'
+#' Learn more about the overall OAuth authentication flow in `vignette("oauth")`.
+#'
+#' # Caching


this reminds me of one topic that is often a hurdle: how to use OAuth on a server, how to provide the token (maybe just a link to https://gargle.r-lib.org/articles/managing-tokens-securely.html since it shows the idea)

vignettes/articles/oauth.Rmd

jonthegeek

This is great! It balances "oauth is hard and weird" vs "don't be afraid to play with it" well.

vignettes/articles/oauth.Rmd

Co-authored-by: Jon Harmon <jonthegeek@gmail.com>

vignettes/articles/oauth.Rmd

jonthegeek

Even better! My biggest stumbling block learning oauth was getting past "But why are there all these steps???" The hierarchy of tokens helps with that quite a lot! Maybe mention "authorization code" in there as well (as the super short-lived but most capturable one)?

vignettes/articles/oauth.Rmd

maelle

Very useful new section! I added a few more comments, in particular about the refresh token.

vignettes/articles/oauth.Rmd

Co-authored-by: Jon Harmon <jonthegeek@gmail.com>

jonthegeek

Just a couple typos.

vignettes/articles/oauth.Rmd

Co-authored-by: Jon Harmon <jonthegeek@gmail.com>

vignettes/articles/oauth.Rmd

jennybc · 2023-10-16T16:28:11Z

vignettes/articles/oauth.Rmd

-The process varies from API to API, but at the end of it you'll get a client id and in most cases a client secret.
+The first step in working with any OAuth API is to create an **application** **client**.
+It's called an application client because in the wider world OAuth is usually used with a web, phone, or tv app, but here your "app" is going to be your R package.
+For this reason, httr2 calls the application client a **client**, but many APIs will call it an OAuth application.


FWIW it feels like Google thinks of "your piece of software / your project" as the application and then that application might make use of multiple clients, e.g. a desktop client, a web client, etc.

The vocabulary is sort of a nightmare, so, yeah, the main point is preparing an R user to not recognize themselves or their use case in any of the documentation.

vignettes/articles/oauth.Rmd

Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>

#Conflicts: # NEWS.md # man/paginate_req_perform.Rd

jonthegeek

Typo in news. I think I got the correct name for the deprecated function.

jonthegeek · 2023-10-16T18:17:00Z

NEWS.md


 * httr2 now informs the user when a token is cached.

+* `req_perform_stream()` replaces `req_perform_stream()`. `req_perform_stream()`


Suggested change

* `req_perform_stream()` replaces `req_perform_stream()`. `req_perform_stream()`

* `req_perform_stream()` replaces `req_stream()`. `req_stream()`

hadley added 14 commits October 8, 2023 14:19

Update docs for req_oauth_refresh()

ae9608b

Merged origin/main into oauth-docs

1c26fb8

Update req_oauth_auth_code()

8c6fac1

Update oauth_flow_bearer_jwt()

3fd539b

Update oauth_flow_device()

7c49e27

Update oauth_flow_client_credentials()

ee37af6

Make helper for consistent RFC links

30da99f

Update req_oauth_password()

747f02f

Tweak default_redirect_uri() + docs

919deb4

Tweak reference

71e5887

WS

76c11c1

Move caching params to end

40dc1aa

Tweak vignette link

341e4ce

Add section on caching

f95d89c

hadley requested a review from jennybc October 8, 2023 21:21

Much better description of how you actually authenticate a request

b397e6e

hadley marked this pull request as ready for review October 8, 2023 22:24

Transform cache path

b6e9290

mgirlich reviewed Oct 9, 2023

View reviewed changes

R/oauth-flow-auth-code.R Outdated Show resolved Hide resolved

mgirlich reviewed Oct 9, 2023

View reviewed changes

R/oauth-flow-auth-code.R Outdated Show resolved Hide resolved

mgirlich reviewed Oct 9, 2023

View reviewed changes

R/oauth-flow-auth-code.R Outdated Show resolved Hide resolved

mgirlich reviewed Oct 9, 2023

View reviewed changes

R/oauth-flow-auth-code.R Outdated Show resolved Hide resolved

hadley and others added 3 commits October 10, 2023 07:26

Apply suggestions from code review

6b6b25a

Co-authored-by: Maximilian Girlich <maximilian.girlich@metoda.com>

Use fixed = TRUE

0d59028

Re-document

4057a32

llrs reviewed Oct 10, 2023

View reviewed changes

vignettes/articles/oauth.Rmd Outdated Show resolved Hide resolved

vignettes/articles/oauth.Rmd Outdated Show resolved Hide resolved

R/oauth-flow-refresh.R Outdated Show resolved Hide resolved

hadley added 2 commits October 11, 2023 16:00

Vignette clarifications

fb36f65

Clarify docs

1d92698

hadley added 3 commits October 11, 2023 20:51

Add news bullet

94f6a9e

Merge commit 'defedecfdf6cdc4d32fbb9cf3a9c2ec5c1af4607'

9b800f2

#Conflicts: # NEWS.md

More polishing

a0f73f8

maelle reviewed Oct 12, 2023

View reviewed changes

jonthegeek reviewed Oct 12, 2023

View reviewed changes

hadley and others added 2 commits October 13, 2023 06:40

Apply suggestions from code review

f8d728a

Co-authored-by: Jon Harmon <jonthegeek@gmail.com>

Reviewer feedback

2547043

hadley commented Oct 12, 2023

View reviewed changes

vignettes/articles/oauth.Rmd Show resolved Hide resolved

jonthegeek reviewed Oct 12, 2023

View reviewed changes

maelle reviewed Oct 13, 2023

View reviewed changes

hadley and others added 2 commits October 13, 2023 19:09

Apply suggestions from code review

2407e8e

Co-authored-by: Jon Harmon <jonthegeek@gmail.com>

More feedback from reviews

cf926bc

jonthegeek approved these changes Oct 13, 2023

View reviewed changes

vignettes/articles/oauth.Rmd Outdated Show resolved Hide resolved

vignettes/articles/oauth.Rmd Outdated Show resolved Hide resolved

Apply suggestions from code review

87e040d

Co-authored-by: Jon Harmon <jonthegeek@gmail.com>

jennybc approved these changes Oct 16, 2023

View reviewed changes

hadley and others added 3 commits October 16, 2023 13:11

Apply suggestions from code review

1666b05

Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>

Merge commit 'c2559b68a8410c1a04d494960ccaa6e5502b9a9f'

7db8f95

#Conflicts: # NEWS.md # man/paginate_req_perform.Rd

Polish news

6562980

jonthegeek approved these changes Oct 16, 2023

View reviewed changes

hadley merged commit 0059238 into main Oct 16, 2023

hadley deleted the oauth-docs branch October 16, 2023 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAuth docs #334

OAuth docs #334

hadley commented Oct 8, 2023 •

edited

Loading

hadley commented Oct 8, 2023

llrs left a comment •

edited by hadley

Loading

hadley commented Oct 11, 2023

maelle left a comment

maelle Oct 12, 2023

hadley Oct 12, 2023

maelle Oct 12, 2023

jonthegeek left a comment

jonthegeek left a comment

maelle left a comment

jonthegeek left a comment

jennybc Oct 16, 2023

jonthegeek left a comment

jonthegeek Oct 16, 2023


		* httr2 now informs the user when a token is cached.

		* `req_perform_stream()` replaces `req_perform_stream()`. `req_perform_stream()`

	* `req_perform_stream()` replaces `req_perform_stream()`. `req_perform_stream()`
	* `req_perform_stream()` replaces `req_stream()`. `req_stream()`

OAuth docs #334

OAuth docs #334

Conversation

hadley commented Oct 8, 2023 • edited Loading

hadley commented Oct 8, 2023

llrs left a comment • edited by hadley Loading

Choose a reason for hiding this comment

hadley commented Oct 11, 2023

maelle left a comment

Choose a reason for hiding this comment

maelle Oct 12, 2023

Choose a reason for hiding this comment

hadley Oct 12, 2023

Choose a reason for hiding this comment

maelle Oct 12, 2023

Choose a reason for hiding this comment

jonthegeek left a comment

Choose a reason for hiding this comment

jonthegeek left a comment

Choose a reason for hiding this comment

maelle left a comment

Choose a reason for hiding this comment

jonthegeek left a comment

Choose a reason for hiding this comment

jennybc Oct 16, 2023

Choose a reason for hiding this comment

jonthegeek left a comment

Choose a reason for hiding this comment

jonthegeek Oct 16, 2023

Choose a reason for hiding this comment

hadley commented Oct 8, 2023 •

edited

Loading

llrs left a comment •

edited by hadley

Loading