Stop requiring a configured oauth client prior to token_fetch() (#186)

* Stop requiring a configured oauth client prior to `token_fetch()` Fixes #160 * Work on the NEWS
r-lib · May 12, 2023 · c3fee30 · c3fee30
1 parent 510294d
commit c3fee30
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 68 deletions.
diff --git 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
 

diff --git 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
@@ -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:
+        <https://gmailr.r-lib.org/dev/articles/oauth-client.html>
+      ! 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 <https://gargle.r-lib.org/articles/non-interactive-auth.html>
+      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
@@ -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
@@ -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())
 })