Skip to content

Commit

Permalink
Add a dedicated error class for OAuth response parsing errors. (#596)
Browse files Browse the repository at this point in the history
Not all OAuth providers in the wild return spec-compliant responses in
the case of an error. A prominent example is Facebook [0], but I've seen
issues with Snowflake OAuth's token exchange flow, too.

I've also seen weird issues in the past like authentication failues
resulting in redirects to human-readable HTML pages rather than proper
OAuth2 error response JSON.

All this to say: I think it can be really useful to have access to the
original response object when trying to diagnose an issue.

So this commit adds a dedicated `httr2_oauth_parse` error class and
embeds the response object in it. This allows consumers to write code
that handles these errors with a lot more context:

    tryCatch(
      some_oauth_using_code(),
      httr2_oauth_parse = function(cnd) {
        cli::cli_abort(
          c(
            "Unexpected failure during an OAuth flow.",
            i = "Status code: {resp_status(cnd$resp)}",
            i = "Response content type: {resp_content_type(cnd$resp)}"
          ),
          parent = cnd
        )
      }
    )

Unit tests are included.

[0]: https://pilcrowonpaper.com/blog/dear-oauth-providers/

Signed-off-by: Aaron Jacobs <[email protected]>
  • Loading branch information
atheriel authored Dec 12, 2024
1 parent e972770 commit 0ea4ec9
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 0 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# httr2 (development version)

* Errors thrown during the parsing of an OAuth response now have a dedicated
`httr2_oauth_parse` error class that includes the original response object
(@atheriel, #596).

# httr2 1.0.7

* `req_perform_promise()` upgraded to use event-driven async based on waiting efficiently on curl socket activity (#579).
Expand Down
4 changes: 4 additions & 0 deletions R/oauth-flow.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ oauth_flow_parse <- function(resp, source, error_call = caller_env()) {
cli::cli_abort(
"Failed to parse response from {.arg {source}} OAuth url.",
parent = err,
resp = resp,
class = "httr2_oauth_parse",
call = error_call
)
}
Expand Down Expand Up @@ -40,6 +42,8 @@ oauth_flow_parse <- function(resp, source, error_call = caller_env()) {
"Failed to parse response from {.arg {source}} OAuth url.",
"*" = "Did not contain {.code access_token}, {.code device_code}, or {.code error} field."
),
resp = resp,
class = "httr2_oauth_parse",
call = error_call
)
}
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/test-oauth-flow.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ test_that("userful errors if response isn't parseable", {
})
})

test_that("can inspect the original response if response isn't parseable", {
resp1 <- response(headers = list(`content-type` = "text/plain"))
resp2 <- response_json(body = list())

tryCatch(
oauth_flow_parse(resp1, "test"),
httr2_oauth_parse = function(cnd) {
expect_equal(cnd$resp, resp1)
}
)
tryCatch(
oauth_flow_parse(resp2, "test"),
httr2_oauth_parse = function(cnd) {
expect_equal(cnd$resp, resp2)
}
)
})

test_that("returns body if known good structure", {
resp <- response_json(body = list(access_token = "10"))
expect_equal(oauth_flow_parse(resp, "test"), list(access_token = "10"))
Expand Down

0 comments on commit 0ea4ec9

Please sign in to comment.