diff --git a/R/oauth-client.R b/R/oauth-client.R index 6c8581cdd..3df0acb9c 100644 --- a/R/oauth-client.R +++ b/R/oauth-client.R @@ -69,6 +69,17 @@ oauth_client <- function( "{.code auth = 'jwt_sig'} requires a claim specification in {.arg auth_params}." ) } + + if (is_list(auth_params$claim)) { + auth_params$claim <- exec("jwt_claim", !!!auth_params$claim) + } else if (is.function(auth_params$claim)) { + auth_params$claim <- auth_params$claim() + } else { + cli::cli_abort( + "{.value claim} in {.arg auth_params} + must be a list or function." + ) + } } auth <- paste0("oauth_client_req_auth_", auth) diff --git a/R/oauth-flow-jwt.R b/R/oauth-flow-jwt.R index edfb2576e..d5ef9a6bc 100644 --- a/R/oauth-flow-jwt.R +++ b/R/oauth-flow-jwt.R @@ -13,11 +13,6 @@ #' @family OAuth flows #' @inheritParams req_perform #' @inheritParams req_oauth_auth_code -#' @param claim A list of claims. If all elements of the claim set are static -#' apart from `iat`, `nbf`, `exp`, or `jti`, provide a list and -#' [jwt_claim()] will automatically fill in the dynamic components. -#' If other components need to vary, you can instead provide a zero-argument -#' callback function which should call `jwt_claim()`. #' @param signature Function use to sign `claim`, e.g. [jwt_encode_sig()]. #' @param signature_params Additional arguments passed to `signature`, e.g. #' `size`, `header`. @@ -27,8 +22,13 @@ #' req_auth <- function(req) { #' req_oauth_bearer_jwt( #' req, -#' client = oauth_client("example", "https://example.com/get_token"), -#' claim = jwt_claim() +#' client = oauth_client( +#' "example", +#' "https://example.com/get_token", +#' auth_params = list( +#' claim = jwt_claim() +#' ) +#' ) #' ) #' } #' @@ -37,7 +37,6 @@ req_oauth_bearer_jwt <- function( req, client, - claim, signature = "jwt_encode_sig", signature_params = list(), scope = NULL, @@ -45,14 +44,13 @@ req_oauth_bearer_jwt <- function( ) { params <- list( client = client, - claim = claim, signature = signature, signature_params = signature_params, scope = scope, token_params = token_params ) - cache <- cache_mem(client, claim) + cache <- cache_mem(client) req_oauth(req, "oauth_flow_bearer_jwt", params, cache = cache) } @@ -60,7 +58,6 @@ req_oauth_bearer_jwt <- function( #' @rdname req_oauth_bearer_jwt oauth_flow_bearer_jwt <- function( client, - claim, signature = "jwt_encode_sig", signature_params = list(), scope = NULL, @@ -68,18 +65,21 @@ oauth_flow_bearer_jwt <- function( ) { check_installed("jose") if (is.null(client$key)) { - cli::cli_abort("JWT flow requires {.arg client} with a key.") + cli::cli_abort("JWT flow requires {.arg client} with a key and a claim.") } - if (is_list(claim)) { - claim <- exec("jwt_claim", !!!claim) - } else if (is.function(claim)) { - claim <- claim() - } else { - cli::cli_abort("{.arg claim} must be a list or function.") + if (is.null(client$auth_params$claim)) { + cli::cli_abort( + "JWT flow requires {.arg client} with a claim in {.arg auth_params}." + ) } - jwt <- exec(signature, claim = claim, key = client$key, !!!signature_params) + jwt <- exec( + signature, + claim = client$auth_params$claim, + key = client$key, + !!!signature_params + ) # https://datatracker.ietf.org/doc/html/rfc7523#section-2.1 oauth_client_get_token( diff --git a/man/req_oauth_bearer_jwt.Rd b/man/req_oauth_bearer_jwt.Rd index d0dd1be9b..55b4160ec 100644 --- a/man/req_oauth_bearer_jwt.Rd +++ b/man/req_oauth_bearer_jwt.Rd @@ -8,7 +8,6 @@ req_oauth_bearer_jwt( req, client, - claim, signature = "jwt_encode_sig", signature_params = list(), scope = NULL, @@ -17,7 +16,6 @@ req_oauth_bearer_jwt( oauth_flow_bearer_jwt( client, - claim, signature = "jwt_encode_sig", signature_params = list(), scope = NULL, @@ -29,12 +27,6 @@ oauth_flow_bearer_jwt( \item{client}{An \code{\link[=oauth_client]{oauth_client()}}.} -\item{claim}{A list of claims. If all elements of the claim set are static -apart from \code{iat}, \code{nbf}, \code{exp}, or \code{jti}, provide a list and -\code{\link[=jwt_claim]{jwt_claim()}} will automatically fill in the dynamic components. -If other components need to vary, you can instead provide a zero-argument -callback function which should call \code{jwt_claim()}.} - \item{signature}{Function use to sign \code{claim}, e.g. \code{\link[=jwt_encode_sig]{jwt_encode_sig()}}.} \item{signature_params}{Additional arguments passed to \code{signature}, e.g. @@ -62,8 +54,13 @@ Learn more about the overall OAuth authentication flow in req_auth <- function(req) { req_oauth_bearer_jwt( req, - client = oauth_client("example", "https://example.com/get_token"), - claim = jwt_claim() + client = oauth_client( + "example", + "https://example.com/get_token", + auth_params = list( + claim = jwt_claim() + ) + ) ) } diff --git a/tests/testthat/_snaps/oauth-client.md b/tests/testthat/_snaps/oauth-client.md index 0c3589172..dd8f438b9 100644 --- a/tests/testthat/_snaps/oauth-client.md +++ b/tests/testthat/_snaps/oauth-client.md @@ -70,3 +70,40 @@ * token_url: "http://example.com" * auth : +# validates claim + + Code + oauth_client("test", "http://example.com", key = "abc", auth_params = list( + claim = "123")) + Condition + Error in `oauth_client()`: + ! claim in `auth_params` must be a list or function. + +--- + + Code + oauth_client("test", "http://example.com", key = "abc", auth_params = list( + claim = jwt_claim())) + Output + + * name : "b3d60321e62eb364e9c8dc12ffeec242" + * id : "test" + * key : + * token_url : "http://example.com" + * auth : "oauth_client_req_auth_jwt_sig" + * auth_params: + +--- + + Code + oauth_client("test", "http://example.com", key = "abc", auth_params = list( + claim = jwt_claim)) + Output + + * name : "b3d60321e62eb364e9c8dc12ffeec242" + * id : "test" + * key : + * token_url : "http://example.com" + * auth : "oauth_client_req_auth_jwt_sig" + * auth_params: + diff --git a/tests/testthat/_snaps/oauth-flow-jwt.md b/tests/testthat/_snaps/oauth-flow-jwt.md deleted file mode 100644 index 890c27621..000000000 --- a/tests/testthat/_snaps/oauth-flow-jwt.md +++ /dev/null @@ -1,16 +0,0 @@ -# validates inputs - - Code - oauth_flow_bearer_jwt(client1) - Condition - Error in `oauth_flow_bearer_jwt()`: - ! JWT flow requires `client` with a key. - ---- - - Code - oauth_flow_bearer_jwt(client2, claim = NULL) - Condition - Error in `oauth_flow_bearer_jwt()`: - ! `claim` must be a list or function. - diff --git a/tests/testthat/_snaps/req-body.new.md b/tests/testthat/_snaps/req-body.new.md new file mode 100644 index 000000000..66afa5eab --- /dev/null +++ b/tests/testthat/_snaps/req-body.new.md @@ -0,0 +1,35 @@ +# can't change body type + + Code + req_body_json(req, list(x = 1)) + Condition + Error in `req_body_json()`: + ! Can't change body type from raw to json. + i You must use only one type of `req_body_*()` per request. + +# errors on invalid input + + Code + req_body_file(request_test(), 1) + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + Code + req_body_file(request_test(), "doesntexist") + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + Code + req_body_file(request_test(), ".") + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + +# non-json type errors + + Code + req_body_json(request_test(), mtcars, type = "application/xml") + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + diff --git a/tests/testthat/_snaps/req-options.new.md b/tests/testthat/_snaps/req-options.new.md new file mode 100644 index 000000000..a9cdeb628 --- /dev/null +++ b/tests/testthat/_snaps/req-options.new.md @@ -0,0 +1,13 @@ +# validates inputs + + Code + req_timeout(request_test(), "x") + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + Code + req_timeout(request_test(), 0) + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + diff --git a/tests/testthat/_snaps/req-perform-connection.new.md b/tests/testthat/_snaps/req-perform-connection.new.md new file mode 100644 index 000000000..06154e962 --- /dev/null +++ b/tests/testthat/_snaps/req-perform-connection.new.md @@ -0,0 +1,41 @@ +# validates inputs + + Code + req_perform_connection(1) + Condition + Error in `req_perform_connection()`: + ! `req` must be an HTTP request object, not the number 1. + Code + req_perform_connection(request_test(), 1) + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + +# curl errors become errors + + Code + req_perform_connection(req) + Condition + Error in `req_perform_connection()`: + ! Failed to perform HTTP request. + Caused by error in `open()`: + ! Failed to connect + +# correclty reports curl error with retries (#817) + + Code + req_perform_connection(req) + Condition + Error in `req_perform_connection()`: + ! Failed to perform HTTP request. + Caused by error in `open()`: + ! Failed to connect + +# validates its input + + Code + StreamingBody$new(1) + Condition + Error in `StreamingBody$new()`: + ! `conn` must be a connection, not the number 1. + diff --git a/tests/testthat/_snaps/resp-body.new.md b/tests/testthat/_snaps/resp-body.new.md new file mode 100644 index 000000000..d2346c1b8 --- /dev/null +++ b/tests/testthat/_snaps/resp-body.new.md @@ -0,0 +1,26 @@ +# check argument types before caching + + Code + resp_body_json(1) + Condition + Error in `resp_body_json()`: + ! `resp` must be an HTTP response object, not the number 1. + Code + resp_body_xml(1) + Condition + Error in `resp_body_xml()`: + ! `resp` must be an HTTP response object, not the number 1. + +# content types are checked + + Code + resp_body_json(req_perform(request_test("/xml"))) + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + Code + resp_body_xml(req_perform(request_test("/json"))) + Condition + Error in `example_url()`: + ! The package "webfakes" is required. + diff --git a/tests/testthat/test-oauth-client.R b/tests/testthat/test-oauth-client.R index 0cd5cf6ed..39f7eb15a 100644 --- a/tests/testthat/test-oauth-client.R +++ b/tests/testthat/test-oauth-client.R @@ -68,3 +68,35 @@ test_that("can authenticate using header or body", { list(client_id = I("id"), client_secret = I("secret")) ) }) + +test_that("validates claim", { + expect_snapshot( + oauth_client( + "test", + "http://example.com", + key = "abc", + auth_params = list(claim = "123") + ), + error = TRUE + ) + + expect_snapshot( + oauth_client( + "test", + "http://example.com", + key = "abc", + auth_params = list(claim = jwt_claim()) + ), + error = FALSE + ) + + expect_snapshot( + oauth_client( + "test", + "http://example.com", + key = "abc", + auth_params = list(claim = jwt_claim) + ), + error = FALSE + ) +}) diff --git a/tests/testthat/test-oauth-flow-jwt.R b/tests/testthat/test-oauth-flow-jwt.R index f8aae05c8..e3ab9e839 100644 --- a/tests/testthat/test-oauth-flow-jwt.R +++ b/tests/testthat/test-oauth-flow-jwt.R @@ -34,16 +34,3 @@ test_that("can generate token and use it automatically", { expect_type(resp, "list") expect_equal(resp$email_verified, TRUE) }) - -test_that("validates inputs", { - client1 <- oauth_client("test", "http://example.com") - expect_snapshot(oauth_flow_bearer_jwt(client1), error = TRUE) - - client2 <- oauth_client( - "test", - "http://example.com", - key = "abc", - auth_params = list(claim = "123") - ) - expect_snapshot(oauth_flow_bearer_jwt(client2, claim = NULL), error = TRUE) -})