search

r-lib / httr

httr: a friendly http package for R
https://httr.r-lib.org
Other
985 stars 1.99k forks source link

Feedback on secrets vignette #465

Closed jennybc closed 1 year ago

jennybc commented 7 years ago

Possible (re-)organization that works well with current content:

Some of the local practices (for example, listing secret files in .gitignore) have an implication for package design: if you help people create a secret file, it's also your duty to edit .gitignore for them. httr does this but packages might facilitate other workflows that write secret info to file.

This advice is over-simple if you need your secrets in order to build vignettes or run tests:

If you're making a package: make sure they are listed in .Rbuildignore so they don't accidentally get included in a public R package.

I don't think you can just .Rbuildignore secrets because you need them available to check and build your package locally and, especially, on travis, etc. I do a real dance around CRAN submissions for this very reason, so I'd love to know if I'm just doing it wrong!

Note that .Renviron is only processed on startup, so you’ll need to restart R to see changes.

I'm pretty sure it also must end in a newline, which has caused many a person to pull out their hair when creating .Renviron for the first time.

I now know how to do that travis stuff on appveyor as well, fwiw. I could add that?

hadley commented 7 years ago

Hmmm, now you're making me wonder how bigrquery actually works...

Ah ok - I think the key idea is to store the encrypted file in the package, and the encrypted file outside of the package. So for bigrquery I have "tests/testthat/service-token.json.enc" which is included in the package, and in git. On travis, I unencrypt as needed, and locally I just have an unecrypted version of the file saved in the same place ("~/.bigrquery-token.json")

What do you do?

A similar section for appveyor would be great! How does encryption work there?

jennybc commented 7 years ago

What do you do?

Locally, I have unencrypted and encrypted file in tests/testthat/. The unencrypted file is gitignored but not Rbuildignored. When I build a tarball for CRAN I have a clunky two-step process and, in the middle, I remove the unencrypted file. It never occurred to me to store it outside the package 😔 but yes that would simplify things.

To dissect your approach:

This should definitely be the recommended pattern!

A similar section for appveyor would be great! How does encryption work there?

Very similar to travis. You probably don't want gory details in this vignette but I could make a PR that puts travis and appveyor on slightly more equal footing and add some appropriate links.

My new insight is to always make the decryption of secure files conditional on private vars being available (so, not a PR). This way PRs aren't doomed to fail: here's a bigrquery PR that fails exactly for this reason, so you'll never get to see the results of R CMD check and the non-skipped tests:

https://travis-ci.org/rstats-db/bigrquery/jobs/247213505

hadley commented 5 years ago

Have we made any changes to our workflow in the ~18 months (!!) since you wrote this?

jennybc commented 5 years ago

I think my last synthesis still basically holds up.

hadley commented 5 years ago

Oh now I remember what I was thinking of: the newer approach in bigrquery where I use symmetric encryption. e.g. https://github.com/r-dbi/bigrquery/commit/aefd5722fbfb8917208f2392b8c5d1c6a85ff1e2

hadley commented 5 years ago

https://github.com/r-dbi/bigrquery/blob/master/R/secret.R

hadley commented 5 years ago

@jennybc could we point to the gargle secrets vignette? or integrate the two somehow?

jennybc commented 5 years ago

Yeah I'll tackle this. I still have to develop gargle advice on what to do with OAuth tokens. Even if it's a suboptimal workflow (vs service tokens), some people will prefer it and it may actually be the only option with some APIs.

hadley commented 1 year ago

httr has been superseded in favour of httr2, so is no longer under active development. If this problem is still important to you in httr2, I'd suggest filing an issue offer there 😄. Thanks for using httr!