ropensci / vcr

Record HTTP calls and replay them
https://docs.ropensci.org/vcr
Other
77 stars 12 forks source link

Problem with `filter_sensitive_data` not replacing keys #167

Open juliasilge opened 4 years ago

juliasilge commented 4 years ago

Hello! 👋 We are trying to get vcr tests set up in the qualtRics package and are running into some challenges. The first one is that filter_sensitive_data does not seem to actually be filtering out the important strings. This work is being done in ropensci/qualtRics#140 so you can check out what's in the current helper-qualtRics.R file if you like:

library("vcr")
invisible(vcr::vcr_configure(
  dir = "../fixtures",
  preserve_exact_body_bytes = TRUE,
  filter_sensitive_data = list("<<<my_api_key>>>" = Sys.getenv('QUALTRICS_API_KEY'),
                               "<<<my_base_url>>>" = Sys.getenv('QUALTRICS_BASE_URL'))
))

When I run devtools::test() I do have new fixtures being created for the two functions we are currently trying to test using vcr, but they have my API key and personal URL in them recorded on disk. They must be in the environment at the time that the API call is being made or otherwise the call would fail.

Do you have any ideas? Have you seen other problems with the sensitive data issue?

sckott commented 4 years ago

thanks for this @juliasilge

is there any chance I can try to replicate the issue? I don't have a qualtrics account, but is there a sandbox qualtrics instance I can use?

juliasilge commented 4 years ago

The free accounts unfortunately don't allow API access. I will try to see if there is some way to share info.

sckott commented 4 years ago

it looks like in all of the tests in that PR that the qualtrics_api_credentials fxn is called https://github.com/ropensci/qualtRics/pull/140/files?file-filters%5B%5D=.R&file-filters%5B%5D=No+extension#diff-5b39ae76315a9d1bdbaba3ee3f352b5aR4 before the test runs - but internally the fxn is looking for the env var that it reads from your .Renviron file, not the one that was just set in the R session

if i comment out the line calling qualtrics_api_credentials the correct env var is found and I see X-API-TOKEN: <<<my_api_key>>> in the fixture file

sckott commented 4 years ago

or keep the qualtrics_api_credentials() fxn call but then do the readEnviron thing right after (i can't remember the exact fxn name)

sckott commented 4 years ago

@juliasilge did that help?

juliasilge commented 4 years ago

Thank you so much for looking at this!

I didn't clarify before that I was removing that line with the qualtrics_api_credentials() to set up the fixture, just like you described. The fixture is created fine but my API token is not replaced. If I run devtools::test() with this:

test_that("all_mailinglists returns a tbl_df with expected column names and types", {

  readRenviron("~/.Renviron")

  vcr::use_cassette("all_mailinglists", {
    x <- all_mailinglists()
  })

  expect_s3_class(x, c("tbl_df","tbl","data.frame"))
  expect_named(x, c("libraryId", "id", "name", "category", "folder"))
  expect_type(x$libraryId, "character")
  expect_type(x$id, "character")
  expect_type(x$name, "character")
  expect_type(x$category, "character")
  expect_type(x$folder, "character")

})

Then a fixture all_mailinglists.yml is created in my /tests/fixtures folder but is still has my API key and URL in it. 😕

Another contributor is trying to just use webmockr instead and we are having trouble with that too. 😬 This is at least more reproducible so I will open an issue over there.

sckott commented 4 years ago

Thanks for the update. Do you have other instances of qualtrics_api_credentials() called in any of your tests? If so, wouldn't that set the api key to a different value?

juliasilge commented 4 years ago

OK, it took some doing but I think we got vcr up and running for qualtRics.

I did have to do some manual editing of the fixture .yml functions to make URLs that webmockr would recognize as registered requests, as touched on in ropensci/webmockr#102. My workflow was to:

Feel free to close this issue if you like. It seems like the main underlying problem for us was webmockr and registered/unregistered requests? I think that vcr is behaving as expected although I am not 100% sure.

sckott commented 4 years ago

Thanks Julia. Hmm, that workflow shouldn't be necessary. I'm not quite sure what's going on - you shouldn't have to manually edit the fixtures, so we have more work to do here to make it so that you just wrap your calls in use_cassette and you're done.

Run devtools::test() and see which ones failed because of unregistered requests

i see that you have some test files that use webmockr and some that use vcr. Are you saying above that you get webmockr errors for the tests that use vcr?

juliasilge commented 4 years ago

Yes, that's right -- webmockr errors (unregistered requests) on the vcr tests.

sckott commented 4 years ago

Weird. I don't think that should be happening. I'm having a hard time replicating your errors.. It seems you still have those calls to qualtrics_api_credentials in the test files even though you said above you were going to remove those. maybe I'm missing something?

juliasilge commented 4 years ago

I remove those lines when I want to use real API credentials that are in my .Renviron file. Calling qualtrics_api_credentials() stores values in those environment variables (and optionally, writes them to .Renviron). The qualtRics functions won't run unless there is something in the environment variables.

The tests succeed now because the fake values are loaded into the environment variables, and the fake API calls are loaded via vcr.

sckott commented 4 years ago

Okay, thanks for the explanation. Sorry about this not working super cleanly.

May be related to https://github.com/ropensci/vcr/issues/137

maelle commented 3 years ago

We actually closed #137 by adding docs, and reading this issue I am not sure it was related but I might be missing something.

In (small) packages where I have done vcr stuff with secrets

if(!nzchar(Sys.setenv("APIKET")) {
  Sys.setenv("APIKEY" = "foobar")
}

in order to fool the package (not vcr).

I'll go read the PR thread in qualtRics.

sckott commented 3 years ago

thxs @maelle

maelle commented 3 years ago

In the end I wasn't able to find out what the vcr/webmockr problem was here.

sckott commented 3 years ago

ugh, well thanks for trying