Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# vmxr 0.1.1.9000 (development)

* `vmx_login()` device-code prompt now matches the actual flow (GEN-2378).
Because the browser is opened at `verification_uri_complete` (the URL that
already embeds the `user_code`), the consent page shows the code **pre-filled**
and never asks the user to paste it. `vmx_login()` now prints its own guidance
before the flow — "check the code shown in the browser matches, then approve",
plus a note that you may not be prompted to paste it — reframing the misleading
"copy … and paste when requested" line that upstream `httr2` prints. The
guidance shows only in the interactive/browser branch; the non-interactive
fallback (plain `verification_uri`, where you *do* enter the code) is left to
httr2's accurate "Visit … and enter code" message.
* OIDC access tokens now self-heal on a reused client (GEN-2344). `vmx_client()`
no longer freezes the OIDC access token at construction: the bearer token is
resolved **per request** via a provider closure, so a long-lived
Expand Down
37 changes: 37 additions & 0 deletions R/oidc.R
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,39 @@ vmx_oidc_config <- function(issuer = NULL, client_id = NULL, scopes = NULL) {
httr2::oauth_client(id = config$client_id, token_url = token_endpoint, auth = "body")
}

# Pre-frame httr2's device-code prompt so it matches what actually happens here
# (GEN-2378). httr2::oauth_flow_device() hard-codes, in its *interactive* branch,
# "Copy <code> and paste when requested by the browser"
# (r-lib/httr2 R/oauth-flow-device.R) -- but it opens `verification_uri_complete`,
# the URL that already embeds the `user_code`, so Authentik's consent page shows
# the code pre-filled and never asks the user to paste it. That "copy ... and
# paste" line therefore describes a step that doesn't happen. We can't reword or
# suppress httr2's line (no override argument; it wraps a bare readline), so we
# print our own guidance immediately before it, reframing the step as "verify the
# code matches, then approve" and warning that no paste prompt may appear.
#
# Only the interactive/browser branch is misleading. When httr2 runs
# non-interactively it instead prints "Visit <url> and enter code <code>" against
# the plain `verification_uri` (no embedded code), where entering the code IS the
# real step -- so we stay silent there and let httr2's accurate instruction stand
# (the paste/enter guidance belongs only on that fallback branch). We gate on the
# same predicate httr2 uses for `open_browser` (rlang::is_interactive()) so our
# framing tracks its branch exactly.
.vmx_oidc_device_prompt <- function(interactive = rlang::is_interactive()) {
if (!isTRUE(interactive)) {
return(invisible())
}
cli::cli_bullets(c(
"i" = "A browser window will open to confirm your VeloMetrix sign-in.",
"*" = "Check that the security code shown in the browser matches the code printed below, then approve the request.",
"i" = paste0(
"You may not be prompted to enter or paste the code \u2014 it's ",
"already included in the sign-in URL, so the page may show it pre-filled."
)
))
invisible()
}

# Wrap httr2's RFC 8628 device-code flow in a mockable binding. Kept minimal so
# tests can stub it (the real flow opens a browser and blocks on polling).
.vmx_oidc_device_flow <- function(oauth_client, device_url, scopes) {
Expand Down Expand Up @@ -356,6 +389,10 @@ vmx_login <- function(issuer = NULL, client_id = NULL, scopes = NULL,
cache_path <- cache_path %||% .vmx_oidc_cache_path()
endpoints <- .vmx_oidc_discover(config)
oauth_client <- .vmx_oidc_client(config, endpoints$token)
# Reframe httr2's misleading "copy & paste the code" prompt before it prints
# (GEN-2378): the browser opens the pre-filled URL, so the user verifies and
# approves rather than pasting.
.vmx_oidc_device_prompt()
raw <- tryCatch(
.vmx_oidc_device_flow(oauth_client, endpoints$device, config$scopes),
error = function(e) {
Expand Down
48 changes: 48 additions & 0 deletions tests/testthat/test-oidc.R
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,51 @@ test_that("device-flow failures are wrapped as vmx_auth_error", {
expect_s3_class(err, "vmx_auth_error")
expect_match(conditionMessage(err), "device-code login failed")
})

# -- GEN-2378: device-code prompt wording ------------------------------------
# httr2::oauth_flow_device() hard-codes a "Copy <code> and paste when requested"
# line, but it opens the pre-filled verification_uri_complete, so no paste ever
# happens. .vmx_oidc_device_prompt() pre-frames that as verify-and-approve in the
# interactive/browser branch, and stays silent in the non-interactive branch
# (where httr2's own "Visit <url> and enter code" instruction is accurate).

test_that("device prompt reframes the browser flow as verify-and-approve (GEN-2378)", {
# Widen cli + collapse whitespace so console line-wrapping can't split a phrase
# mid-word and break the substring matches below.
withr::local_options(cli.width = 10000)
out <- gsub("\\s+", " ", paste(cli::cli_fmt(.vmx_oidc_device_prompt(interactive = TRUE)), collapse = " "))
# Tells the user to verify the code matches and approve -- not to paste it.
expect_match(out, "matches", ignore.case = TRUE)
expect_match(out, "approve", ignore.case = TRUE)
# The modified note: warn that no paste/enter prompt may appear (code pre-filled).
expect_match(out, "pre-filled", ignore.case = TRUE)
expect_match(out, "may not be prompted", ignore.case = TRUE)
# It must NOT instruct copy-and-paste (the misleading step this issue removes).
expect_false(grepl("paste when requested", out, ignore.case = TRUE))
})

test_that("device prompt stays silent in the non-interactive fallback branch (GEN-2378)", {
# httr2 prints "Visit <url> and enter code <code>" there, which is accurate, so
# vmxr adds nothing (the paste/enter instruction belongs only on that branch).
expect_identical(cli::cli_fmt(.vmx_oidc_device_prompt(interactive = FALSE)), character(0))
})

test_that("vmx_login shows the reframed device guidance before running the flow (GEN-2378)", {
cache <- withr::local_tempfile(fileext = ".json")
local_oidc_env(cache)
seen <- character(0)
testthat::local_mocked_bindings(
.vmx_oidc_discover = function(config) list(device = "https://auth.test/device", token = "https://auth.test/token"),
# Record ordering: the guidance must precede the (browser-opening) flow.
.vmx_oidc_device_prompt = function(interactive = rlang::is_interactive()) {
seen <<- c(seen, "prompt")
invisible()
},
.vmx_oidc_device_flow = function(oauth_client, device_url, scopes) {
seen <<- c(seen, "flow")
list(access_token = "acc", refresh_token = "ref", expires_in = 600, token_type = "Bearer")
}
)
suppressMessages(vmx_login())
expect_identical(seen, c("prompt", "flow"))
})