ropensci-archive / roweb2

:no_entry: ARCHIVED :no_entry:
https://github.com/ropensci/roweb3
25 stars 72 forks source link

Technote about credentials #713

Closed jeroen closed 4 years ago

jeroen commented 4 years ago

Preview here: https://deploy-preview-713--ropensci.netlify.app/technotes/2020/07/07/github-pat/

@jennybc @hadley does this answer most of your questions? I'd like to keep the post fairly introductory, and refer to the git-credential docs and package documentation for technical details.

jeroen commented 4 years ago

Do you endorse that? Is that worth mentioning as a fairly direct substitute for inlining a PAT in ~/.Renviron?

I think so. Myself I call set_github_pat() only once I need the GITHUB_PAT. There is no need to have the GITHUB_PAT in my R session all the time.

Then again if you do a lot of work with the GH api, it probably makes sense to do it in your .Rprofile.

hadley commented 4 years ago

I'd prefer not to endorse, or at least endorse with conditions, because I think it's a transitional state — the packages that use the env var (gh, remotes, and usethis?) should pretty rapidly transition to using credentials so that the PAT is only retrieved when necessary. I'd prefer to do one big push to get everyone's PAT out of .Renviron, so I think that should wait until we've incorporated the pluming into the major packages.

jennybc commented 4 years ago

I'd prefer not to endorse, or at least endorse with conditions, because I think it's a transitional state — the packages that use the env var (gh, remotes, and usethis?) should pretty rapidly transition to using credentials so that the PAT is only retrieved when necessary. I'd prefer to do one big push to get everyone's PAT out of .Renviron, so I think that should wait until we've incorporated the pluming into the major packages.

I want to make sure I follow you. Currently set_github_pat() takes the PAT from the credential store and puts it into the GITHUB_PAT env var. There's not a function marketed as simply returning the PAT, although obviously there could be. We use the session's env var as the cache.

Are you proposing this:

jennybc commented 4 years ago

Or maybe @Hadley is saying that usethis should be the one making the credentials::set_github_pat() call, if the env var doesn't already hold a PAT? So, we still use the env var mechanism, but instead of making the PAT "always available", any package that needs the PAT is responsible for firing it.

hadley commented 4 years ago

Right, I'm just saying we should be responsible for it — i.e. if you've correctly setup your GitHub PAT with credentials, all our packages should just work without additional user intervention.

jeroen commented 4 years ago

usethis should be the one making the credentials::set_github_pat() call, if the env var doesn't already hold a PAT? So, we still use the env var mechanism, but instead of making the PAT "always available", any package that needs the PAT is responsible for firing it.

Yes I think this is the best solution.

One important limitation to be aware of is that most credential helpers do not allow you to programatically insert a password or token into the store. The user must interactively type the password or token in the dialog. So we still need to support the GITHUB_PAT environment variable for those cases where the token is not provided by an interactive user, but set externally, for example in CI environments.

jeroen commented 4 years ago

This is now live: https://ropensci.org/technotes/2020/07/07/github-pat/ . Thanks for the feedback!