arthur-shaw / susoapi

R interface for Survey Solutions' APIs
https://arthur-shaw.github.io/susoapi/
Other
9 stars 5 forks source link

`set_credentials` throws error when HOME is dynamic #44

Open elliottob opened 5 months ago

elliottob commented 5 months ago

Objective 🎯

Store user credentials consistently

User stories 📇

If a user is using renv, the .Renviron file should be created to the project directory, or append credentials to existing .Renviron. Expecting the environmental variable HOME is not consistent across platforms or business computer systems. Issue pops up when Sys.getenv("HOME") responds with "C:\Users\${USERNAME}". It also makes more sense to use the .Renviron in a R project home directory using here::here() instead.

Tasks ⚙️

Development 👨‍💻

I suggest using here::here() instead of Sys.getent("HOME"). See https://github.com/arthur-shaw/susoapi/blob/f878729a77fc472bd3547a91ae35cc358f7a0128/R/authenticate.R#L21C5-L21C36.

arthur-shaw commented 5 months ago

@elliottob , thanks for the helpful bug report.

Based on your report, I see a definite technical problem and a potential design problem.

The technical problem, as I understand it, is that set_credentials() only works when code relies on user-wide .Renviron file (that is in the user's home directory). Put another way, the function does not (yet) support a project-specific .Renviron file, where the project home is different than the user/system home.

Expecting the environmental variable HOME is not consistent across platforms or business computer systems.

Is there another technical problem that I'm missing? Indeed HOME will be a different path on different systems, but my understanding is that Sys.getenv("HOME") will resolve that (i.e., point to the right user-specific place on each OS)

The design problem--perhaps not something this package should worry about--is that a project-level .Renviron will store API secrets in a file that sits in the root of the project directory. While a user can--and should--gitignore the file, I don't see anything in the standard R project setup tools that would tun the safety on the footgun of tracking secrets.

To unpack that, I don't see any guardrails to prevent leaking secrets stored in a project-level .Renviron. Here's some pseudo-code on the project setup steps I imagine folks would follow:

# instantiate a project
usethis::create_project("foo_bar_baz")

# set up renv
renv::init()

# set project-level secrets as environment variables while in the project session
usethis::edit_r_environ(scope = "project")

# remember to gitignore `.Renviron` but share secrets safely

In general--or in your context--what is the best practice for protecting secrets for (renv) projects? I'd adopted .Renviron to make them accessible system-wide, but keep them out of a place where secrets could be accidentally tracked or leaked. However, I do see the challenge of how to do something similar for a project.

During the initial project design, I vascillated about whether to go for .Renviron or {keyring}. Ultimately, I opted for the former since it was something I (mostly) understood and that I could write code to manage. Also, {keyring} was new at the time. Plus, I wasn't sure if/how it would work on locked-down corporate devices (e.g., OS credential store might not be accessible to R processes, or at all).

Now, I wonder if that design decision was wrong--or if the package simply needs to handle project-level .Renviron files, as your issue seems to suggest.

I'd greatly appreciate your suggestions, since your report suggests you have experience grappling with this. If a project-level .Renviron is enough, I think your suggestion points in the right direction. The function might need to use the {rstudioapi} package to discern whether execution occurs within a project environment or not, so that the function is looking in the right place in light of the execution context. If we need a different solution for projects, I'll start researching this again and/or draw on your experience.