pantheon-systems / quicksilver-pushback

Push any commits made on the Pantheon dashboard back to the original GitHub repository.
MIT License
14 stars 13 forks source link

Add Lockr support to store github token encrypted/offsite #8

Open stevector opened 6 years ago

stevector commented 6 years ago

I spoke with @cellar-door at WordCamp this weekend about adding Lockr integration to this package. He suggested making it an optional feature to increase security.

@greg-1-anderson / @cellar-door, I'm interested in your input on how best to do implementation. In hacking around on a multidev over the weekend was able to read the token using

$github_token = exec("wp lockr get key  github_token  ");
$github_token = substr($github_token, strpos($github_token, " ") + 1);

I did that substr command because WP-CLI formats the output as Success: ABC123XYZ

So, I'd rather not rely on the respective module/plugin commands to get the keys because I'd rather not do this kind of munging and in general it seems weird to exec() to retrieve data available through PHP. Especially when the underlying Lockr code doesn't care much about Drupal vs. WordPress.

As an alternative, this quicksilver-pushback package could use the composer suggest key to bring in https://github.com/lockr/lockr-client and fall back to secrets.json if Lockr does not exist.

On the other hand, I might be over engineering when the module/plugin are already available and might still be required to register the site and set the key.

greg-1-anderson commented 6 years ago

I think that using class_exists to determine whether or not to use lockr is a good idea. Perhaps the check might go the other way, though, and use file_exists to check for the Github token first, since that will be faster.

It would be good to have an encrypted-at-rest solution available here. Even better than that, the existing solution is fragile, as the Github token file is obliterated if, say, you copy files from live to the mulitdev site. Using lockr would not suffer from that shortcoming.

cellar-door commented 6 years ago

If you have the Lockr client library loaded in via composer and available to the quicksilver-pushback there shouldn't be anything stopping you from registering/setting a key (assuming that you then take the return from the key set and store that in the secrets.json file). It may be entirely possible to take the functionality out of the lockr.php in the WordPress module and put it into a thin layer of logic that abstracts for this. That way it's platform independent and doesn't require using exec()

I'm happy to have our devs create that logic if you think that's the way you want to go. Should be a simple for us to do since we're in it every day

greg-1-anderson commented 6 years ago

One of the main benefits of using lockr here is getting rid of the secrets.json file; it would be great if we could do that with a direct API call. The code here is messy but pretty short; should be straightforward to shoehorn it in. Any help in that direction would be appreciated.

stevector commented 6 years ago

I met with @greg-1-anderson and @ataylorme earlier today. We agreed to start just by updating the code here around reading the key. For writing the key from derivatives of example-wordpress-composer and example-drops-8-composer we will have manual instructions for adding the module/plugin.

@cellar-door, your suggestion sounds good. Could you (or your team) make a PR that copies the relevant pieces from lockr.php of the WordPress plugin?

ataylorme commented 5 years ago

I'm happy to have our devs create that logic if you think that's the way you want to go. Should be a simple for us to do since we're in it every day

@cellar-door bump here - Lockr support in Quicksilver Pushback would be a great feature to spearhead a 2.1 release.