Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with filter_sensitive_data not replacing keys #167

Open
juliasilge opened this issue May 2, 2020 · 17 comments
Open

Problem with filter_sensitive_data not replacing keys #167

juliasilge opened this issue May 2, 2020 · 17 comments

Comments

@juliasilge
Copy link

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
Copy link
Collaborator

sckott commented May 3, 2020

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
Copy link
Author

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

@sckott
Copy link
Collaborator

sckott commented May 4, 2020

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
Copy link
Collaborator

sckott commented May 4, 2020

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
Copy link
Collaborator

sckott commented May 6, 2020

@juliasilge did that help?

@juliasilge
Copy link
Author

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
Copy link
Collaborator

sckott commented May 7, 2020

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
Copy link
Author

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:

  • Create each fixture for each unit test interactively and make sure they succeeded (no use of devtools::test() for fixture creation)
  • Run devtools::test() and see which ones failed because of unregistered requests
  • Edit the fixtures by hand one by one to fix the failing tests
  • Eventually, success 🎉

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
Copy link
Collaborator

sckott commented May 8, 2020

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
Copy link
Author

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

@sckott
Copy link
Collaborator

sckott commented May 8, 2020

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
Copy link
Author

juliasilge commented May 8, 2020

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
Copy link
Collaborator

sckott commented May 15, 2020

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

May be related to #137

@maelle
Copy link
Member

maelle commented Nov 19, 2020

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

  • I tweaked the vcr configuration as mentioned at the beginning of this thread
  • I have these lines in a setup file
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
Copy link
Collaborator

sckott commented Nov 19, 2020

thxs @maelle

@maelle
Copy link
Member

maelle commented Nov 22, 2020

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

@sckott
Copy link
Collaborator

sckott commented Nov 24, 2020

ugh, well thanks for trying

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants