Feature request: merge with Credentials package

[restonslacker]

Thanks to a lot of legwork by my co-conspirator @mattpollock, he was able to get a number of R packages publicly released by MITRE. One of those packages is credentials which we started before getPass was available on CRAN. There is a lot of duplicated functionality between the two so rather than pushing credentials to CRAN, we were wondering if you would be open to a PR (or series of) that added some functions from credentials.

The biggest difference is that the main functions in credentials provide a list with both the username and the password in the return value. There is also a shiny + miniUI variant of username/password acquisition. The other difference is that credentials exposes the functions for each acquisition method rather than the autodetect method used by getPass.

What we would like to do:

• Add getCredentials() public function which returns a list as described above.
• Add guessUser() public helper function.
• Add the shiny + miniUI acquisition method
• Move rstudioapi to SUGGESTS (we think you only use this in one file and it would be a small change)

I'm happy to discuss any details of this or come up with alternatives if you are interested.

[mattpollock]

To clarify, get_credentials does an auto detect similar to getPass but also exports the method-specific collectors. Another difference is that get_credentials accepts a preferences list to let package authors exercise some control over how credentials are gathered.

I see getPass (returning only a password) and getCredentials (returning a list with username, password, and a logical indicating whether the pw can be cached) as complementary functions. A getPass-specific Shiny collector could be written that does not allow users to input username or click the "cache password" checkbox.

[snoweye]

• Can those functions work without testthat and knitr?
• Can those functions released in BSD-2 which getPass currently uses? Or, we have change to APL2?

[wrathematics]

Thanks for contacting us. I'm definitely not opposed to working together, although a separate package for comprehensively handling user credentials could easily make sense as well. One that, in addition to your additions, automatically handled hashing/authenticating could be useful, for example.

At some point I convinced myself that rstudioapi had to be a hard import. I'll take another look though.

[restonslacker]

@snoweye those are both in SUGGESTS and so end users aren't forced to install them. The functions that I mentioned can work without them if this is an issue for some reason. testthat is used by the test suite in the package to make sure changes don't unexpectedly break existing functions. If we move forward with integration, those tests may no longer be valid. Of course, in our discussions we may decide to write new or updated tests. knitr is used to build the package vignette. The vignette may need to be updated but I'm in favor of keeping more documentation instead of less and if others agree, we would need to leave it in.

I don't have a definitive answer on the license issue at this time. I'd have to look into it more but iirc both licenses are pretty permissive.

@wrathematics we discussed leaving it as a separate package but think the principal downside is that bug reports might get filed in the wrong place. since we didn't foresee changing the existing getPass public API, it seemed like adding functionality to an already existing package was slightly preferable. If you are willing to export the individual acquisition functions, we can explore this route. Ultimately either solution is fine with us, but we wanted to discuss with you all to find the best solution for the R community. :)

[wrathematics]

You don't need testthat for tests or knitr for vignettes though. We both feel pretty negatively about those packages (I have very strong negative feelings about testthat in particular). This package currently has a latex vignette, although we used to use knitr. Dropping knitr halved the time spent checking the package on travis, for example.

[snoweye]

An extension/advance package on top of either or both packages may be easier for both. Unless these can get very popular, don't need to worry about wrong bug reports ...

[mattpollock]

To make that work we would still need to modify the namespace here so that credentials could depend on it. I'm awaiting guidance about the licensing issue. Expect to hear back early next week (Tuesday or Wednesday). I agree that the bug report issue is likely not going to be a big deal, but I would still like to avoid entirely duplicating what's already been released.

[snoweye]

• I were not sure Suggests can pass R CMD check --as-cran ... I thought Imports was probably the minimum.
• I pretty sure shiny and minUI should not be moved to getPass, so that changing credentials (or a new package, getPass2?) by importing getPass if needed would be a better way to go because a lot of packages don't fit to web ... @wrathematics or me may help if you have a plan to go on CRAN.