From 94608854bdfab9948f804efd2d7dabb6beae60a2 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 12 May 2023 13:04:38 -0700 Subject: [PATCH 1/2] Stop requiring a configured oauth client prior to `token_fetch()` Fixes #160 --- R/gm_auth.R | 53 +++++++++++++++++++------------- tests/testthat/_snaps/gm_auth.md | 48 +++++++++++++++++++++++------ tests/testthat/helper.R | 14 --------- tests/testthat/test-gm_auth.R | 53 ++++++++++++++++++++++++-------- 4 files changed, 109 insertions(+), 59 deletions(-) diff --git a/R/gm_auth.R b/R/gm_auth.R index e746c1d..109d3f1 100644 --- a/R/gm_auth.R +++ b/R/gm_auth.R @@ -63,17 +63,7 @@ gm_auth <- function(email = gm_default_email(), gargle::check_is_service_account(path, hint = "gm_auth_configure") scopes <- gm_scopes(scopes) - # preserving previous behavior, in which gm_auth() errors if no OAuth client - # is pre-configured - # this has downsides (see https://github.com/r-lib/gmailr/issues/160), but - # tackling the service account problem is a separate piece of work client <- gm_oauth_client() - if (is.null(client)) { - cli::cli_abort( - "Must create an OAuth client and register it with {.fun gm_auth_configure}." - ) - } - cred <- gargle::token_fetch( scopes = scopes, app = client, @@ -84,20 +74,39 @@ gm_auth <- function(email = gm_default_email(), use_oob = use_oob, token = token ) - if (!inherits(cred, "Token2.0")) { - cli::cli_abort(c( - "Can't get Google credentials.", - "i" = "Are you running {.pkg gmailr} in a non-interactive \\ - session? Consider:", - "*" = "Call {.fun gm_auth} directly with all necessary specifics.", - "i" = "See gargle's \"Non-interactive auth\" vignette for more details:", - "i" = "{.url https://gargle.r-lib.org/articles/non-interactive-auth.html}" - )) + + if (inherits(cred, "Token2.0")) { + .auth$set_cred(cred) + .auth$set_auth_active(TRUE) + return(invisible()) } - .auth$set_cred(cred) - .auth$set_auth_active(TRUE) - invisible() + no_client <- is.null(client) + no_client_msg <- c( + "x" = "No OAuth client has been configured.", + "i" = "To auth with the user flow, you must register an OAuth client with \\ + {.fun gm_auth_configure}.", + "i" = "See the article \"Set up an OAuth client\" for how to get a client:", + " " = "{.url https://gmailr.r-lib.org/dev/articles/oauth-client.html}" + ) + + non_interactive_msg <- c( + "!" = "{.pkg gmailr} appears to be running in a non-interactive session \\ + and it can't auto-discover credentials.", + " " = "You may need to call {.fun gm_auth} directly with all necessary \\ + specifics.", + "i" = "See gargle's \"Non-interactive auth\" vignette for more details:", + "i" = "{.url https://gargle.r-lib.org/articles/non-interactive-auth.html}" + ) + + cli::cli_abort(c( + "Can't get Google credentials.", + if (no_client) no_client_msg, + if (!is_interactive()) non_interactive_msg, + "i" = "For general auth troubleshooting, set \\ + {.code options(gargle_verbosity = \"debug\")} to see more detailed + debugging information." + )) } #' Clear current token diff --git a/tests/testthat/_snaps/gm_auth.md b/tests/testthat/_snaps/gm_auth.md index b359232..0d3ff09 100644 --- a/tests/testthat/_snaps/gm_auth.md +++ b/tests/testthat/_snaps/gm_auth.md @@ -1,20 +1,30 @@ -# gm_auth_configure() errors for key, secret, appname, app +# gm_auth() errors if OAuth client is passed to `path` Code - gm_auth_configure(key = "KEY", secret = "SECRET") + gm_auth(path = system.file("extdata", + "client_secret_installed.googleusercontent.com.json", package = "gargle")) Condition - Error: - ! The use of `key`, `secret`, `appname`, and `app` with `gm_auth_configure()` was deprecated in gmailr 2.0.0 and is now defunct. - i Please use the `path` (strongly recommended) or `client` argument instead. + Error in `gm_auth()`: + ! `path` does not represent a service account. + Did you provide the JSON for an OAuth client instead of for a service account? + Use `gm_auth_configure()` to configure the OAuth client. -# gm_oauth_app() is deprecated +# gm_auth() errors informatively Code - absorb <- gm_oauth_app() + gm_auth() Condition - Warning: - `gm_oauth_app()` was deprecated in gmailr 2.0.0. - i Please use `gm_oauth_client()` instead. + Error in `gm_auth()`: + ! Can't get Google credentials. + x No OAuth client has been configured. + i To auth with the user flow, you must register an OAuth client with `gm_auth_configure()`. + i See the article "Set up an OAuth client" for how to get a client: + + ! gmailr appears to be running in a non-interactive session and it can't auto-discover credentials. + You may need to call `gm_auth()` directly with all necessary specifics. + i See gargle's "Non-interactive auth" vignette for more details: + i + i For general auth troubleshooting, set `options(gargle_verbosity = "debug")` to see more detailed debugging information. # gm_auth_configure() works @@ -32,6 +42,24 @@ Error in `gm_auth_configure()`: ! Must supply either `client` or `path`. +# gm_auth_configure() errors for key, secret, appname, app + + Code + gm_auth_configure(key = "KEY", secret = "SECRET") + Condition + Error: + ! The use of `key`, `secret`, `appname`, and `app` with `gm_auth_configure()` was deprecated in gmailr 2.0.0 and is now defunct. + i Please use the `path` (strongly recommended) or `client` argument instead. + +# gm_oauth_app() is deprecated + + Code + absorb <- gm_oauth_app() + Condition + Warning: + `gm_oauth_app()` was deprecated in gmailr 2.0.0. + i Please use `gm_oauth_client()` instead. + # gm_scopes() reveals gmail scopes Code diff --git a/tests/testthat/helper.R b/tests/testthat/helper.R index d8b061c..ffbd307 100644 --- a/tests/testthat/helper.R +++ b/tests/testthat/helper.R @@ -1,21 +1,7 @@ if (gargle:::secret_can_decrypt("gmailr")) { - # provide the actual token, so we don't need to get into the oauth client token <- unserialize(gzcon(rawConnection( gargle:::secret_read("gmailr", "gmailr-dev-token") ))) - - # https://github.com/r-lib/gmailr/issues/160 - fake_client <- gargle::gargle_oauth_client( - id = "PLACEHOLDER", - secret = "PLACEHOLDER" - ) - gm_auth_configure(fake_client) - # Alternatively, I could do this: - # gm_auth_configure(client = token$app) - # But that is somewhat misleading, i.e. it suggests that the configured client - # needs to match that of the token, which it does not. The configured client - # is never needed. - gm_auth(token = token) # TODO: Think about approaches other than this. diff --git a/tests/testthat/test-gm_auth.R b/tests/testthat/test-gm_auth.R index 9c8844b..0ea83bd 100644 --- a/tests/testthat/test-gm_auth.R +++ b/tests/testthat/test-gm_auth.R @@ -1,24 +1,31 @@ -test_that("gm_auth_configure() errors for key, secret, appname, app", { +# gm_auth() ---- +test_that("gm_auth() errors if OAuth client is passed to `path`", { expect_snapshot( error = TRUE, - gm_auth_configure(key = "KEY", secret = "SECRET") - ) - expect_error(gm_auth_configure(appname = "APPNAME")) - google_app <- httr::oauth_app( - "gmailr", - key = "KEYKEYKEY", - secret = "SECRETSECRETSECRET" + gm_auth( + path = system.file( + "extdata", "client_secret_installed.googleusercontent.com.json", + package = "gargle" + ) + ) ) - expect_error(gm_auth_configure(app = google_app)) }) -test_that("gm_oauth_app() is deprecated", { - withr::local_options(lifecycle_verbosity = "warning") - expect_snapshot(absorb <- gm_oauth_app()) +test_that("gm_auth() errors informatively", { + credentials_nope <- function(scopes, ...) { NULL } + gargle::local_cred_funs(funs = list(credentials_nope = credentials_nope)) + local_mocked_bindings(gm_default_oauth_client = function() NULL) + local_interactive(FALSE) + + expect_snapshot( + error = TRUE, + gm_auth() + ) }) +# gm_auth_configure() ---- test_that("gm_auth_configure() works", { - # unset GMAILR_APP + withr::local_envvar(GMAILR_OAUTH_CLIENT = NA) withr::local_envvar(GMAILR_APP = NA) old_client <- gm_oauth_client() withr::defer(gm_auth_configure(client = old_client)) @@ -46,6 +53,26 @@ test_that("gm_auth_configure() works", { ) }) +test_that("gm_auth_configure() errors for key, secret, appname, app", { + expect_snapshot( + error = TRUE, + gm_auth_configure(key = "KEY", secret = "SECRET") + ) + expect_error(gm_auth_configure(appname = "APPNAME")) + google_app <- httr::oauth_app( + "gmailr", + key = "KEYKEYKEY", + secret = "SECRETSECRETSECRET" + ) + expect_error(gm_auth_configure(app = google_app)) +}) + +test_that("gm_oauth_app() is deprecated", { + withr::local_options(lifecycle_verbosity = "warning") + expect_snapshot(absorb <- gm_oauth_app()) +}) + +# gm_scopes() ---- test_that("gm_scopes() reveals gmail scopes", { expect_snapshot(gm_scopes()) }) From 065fba052d8066e7ea9da3d9ff412386cc439dac Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Fri, 12 May 2023 14:39:21 -0700 Subject: [PATCH 2/2] Work on the NEWS --- NEWS.md | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8ac5c33..d620aa7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -15,14 +15,35 @@ Version 1.3.0 of gargle introduced some changes around OAuth and gmailr is synci - The first argument is now named `client`, which is morally equivalent to the previous `app`, i.e. this is essentially a name change. - The `key`, `secret`, `appname`, and `app` arguments are deprecated. - Our strong recommendation is to use the `path` argument, e.g.: - + - Our strong recommendation is to use the `path` argument, either explicitly:: ``` r gm_auth_configure(path = "path/to/my-oauth-client.json") ``` + or implicitly: + ``` r + gm_auth_configure() + ``` + which works because of the new default: + `gm_auth_configure(path = gm_default_oauth_client()`. +* `gm_default_oauth_client()` is a new helper that searches for the JSON file + representing an OAuth client in a sequence of locations. The (file)path of + least resistance is to place this file in the directory returned by + `rappdirs::user_data_dir("gmailr")`. Another alternative is to record its + filepath in the `GMAILR_OAUTH_CLIENT` environment variable. For backwards + compatibility, the `GMAILR_APP` environment variable is still consulted, but + generates a warning (#166). ## Other changes +* `gm_auth()` no longer checks for an OAuth client before calling + `gargle::token_fetch()`. This allows other auth methods to work, which by and + large don't need an OAuth client, such as `gargle::credentials_byo_oauth2()` + (#160, #186). + + Since the lack of an OAuth client undoubtedly remains the most common reason + for `gm_auth()` to fail, its error message includes some specific content if + no OAuth client has been configured. + * `gm_scopes()` can now take a character vector of scopes, each of which can be an actual scope or a short alias, e.g., `"gmail.readonly"`, which identifies a scope associated with the Gmail API. When called without arguments, @@ -54,13 +75,6 @@ Version 1.3.0 of gargle introduced some changes around OAuth and gmailr is synci affected R versions, the examples are automatically converted to a regular section with a note that they might not work. -* `gm_default_oauth_client()` is a new helper that searches for the JSON file - representing an OAuth client in a sequence of locations. The (file)path of - least resistance is to place this file in the directory returned by - `rappdirs::user_data_dir("gmailr")`. Another alternative is to record its - filepath in the `GMAILR_OAUTH_CLIENT` environment variable. For backwards - compatibility, the `GMAILR_APP` environment variable is still consulted, but - generates a warning (#166). # gmailr 1.0.1