Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slack Notifications using Key Module #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cellar-door
Copy link

This provides a simple way to manage and share API keys between Drupal and Quicksilver using the key module. By sharing keys between Drupal and Quicksilver it's now possible to encrypt/decrypt data from Drupal, use shared APIs from Drupal and Quicksilver to automate processes and much more.

They key_get_secrets() function within key.php file can be copy/pasted into anywhere _get_secrets() is used and if the secrets.json file isn't found, or the required key isn't in the secrets file, it uses drush to get the key from the key module.

This opens up the ability to do more secure storage of keys using the Key module and it's extensions like Lockr.

@greg-1-anderson
Copy link
Member

@joshkoenig What do you think about factoring out the lockr / key functionality into the key.php file? Do you think there is a better way this common code can be provided and managed?

@cellar-door
Copy link
Author

My $.02, I think ideally we would have a universal way that secrets could be managed outside of having to have each plugin do the management of it with their own _get_secrets(). The key.php is more of an easy way for folks to see the abstracted code and use it in their plugins rather than a best practice of including the extra file. I'm open to ways to do this more succinctly.

We are already working on extending the Terminus secrets to do CMS and corresponding plugin detection to pull keys from there as needed, similar to how this example does it. Would it be better for no secrets to be managed in quicksilver and all be managed in Terminus?

@greg-1-anderson
Copy link
Member

The issue we have is not with secrets management in Quicksilver, but with external dependencies management in the Quicksilver examples project. Really, the best practice way to manage dependencies in Quicksilver, though, is to write a wp-cli or Drush extension to do the work, and use the WordPress or Drupal site (ideally Composer-managed) to hold the dependencies, which are declared in a composer.json bundled with your cli tool. That way, your Quicksilver webhook becomes just a simple layer that manages parameters and calls the tool, where the real work happens. This ideal situation, though, is complicated by the fact that not everyone uses Composer to manage their Drupal and WordPress sites at the moment, and using Composer in a plug-in or module when the main application is not using Composer causes all sorts of problems.

We have similar limitations with Terminus; I think that this will be easily resolved once we have pantheon-systems/terminus#1035 in core. From there, it would be easy enough to use composer require rather than git clone to pull in plug-ins; that would allow each plug-in to declare its own composer.json file.

@cellar-door
Copy link
Author

+1 for having a plugin manager in Terminus. This would allow folks to only have quicksilver do the function necessary (in this example slack) and leave things like secrets management to the terminus and Drupal/WordPress to manage how they are provided.

In Terminus is the thought then that plugins could extend core functionality? So going back to secrets management, if Quicksilver always just called terminus for it's secrets plugins could then extend that if an alternate source of secret management is applied?

I agree that managing dependencies in Quicksilver and in Terminus independently might get a bit confusing for folks and a central plugin system in Terminus with Quicksilver just adding functionality on top of it would help simplify it. If I'm understanding that's what you're pointing to.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Apr 21, 2016

We've got some confusion with overloaded terms here.

There is a quicksilver terminus plugin, which helps users load Quicksilver webhook examples, or webhooks from personal repositories, into their local working copy of their Pantheon sites. Quicksilver itself is the thing that runs on Pantheon to run your webhooks. Quicksilver can't call Terminus, because Terminus is not installed on the Pantheon platform. If you really needed to, you could require Terminus as a dependency of your Drupal or WordPress site, but this is not an ordinary or recommended way to interact with your workflow.

We could make the terminus quicksilver plugin install dependencies for webhooks at install time. This would work great for something like a key.php file; we could just make a composer project to hold it. However, we have to be a little cautious about using composer require in the terminus quicksilver plugin, as it could cause problems for folks' sites if webhook developers got too overzealous with their requirements, and selected something that conflicts with their site.

The terminus plugin manager would just be for doing things like using an external library with the terminus secrets plugin.

@cellar-door
Copy link
Author

Gotcha - I somehow got confused that Quicksilver could interact with Terminus and its plugin system. Extending Terminus with plugins will allow for people to easily interact with the secrets.json and even reach into their CMS to store them if the plugins/modules are there but doesn't solve the issue at hand which is what to do with a library type function in Quicksilver.

So then would the best thing for now with Quicksilver is to put something like _get_secrets() into the core with the ability for people to extend it in their own plugins by offering additional ways to grab keys through Drush or WP-CLI where they are including the libraries via composer or their own method? This way if I'm writing the slack_notifications I can call _get_secrets() and rather than it throwing a fatal error it would return the error back and the script would handle an error by then invoking the Drush or WP-CLI commands to get the keys from other locations if possible?

In the case of Drupal and Key, the functionality is already in the module and invokable via Drush so it won't need to rely on composer but I understand the larger meta discussion over dependency management that's going on here now.

@greg-1-anderson
Copy link
Member

Yes; in the case of Drupal and Key, ideally the Key module would have a composer.json that included a non-Drupal-specific PHP project via a composer.json file, allowing the same code to be used with other frameworks, like WordPress. Having everything directly in the Drupal module does make things a little easier, in the short term, but factoring it out via Composer is simpler and more maintainable in the long run.

@cellar-door
Copy link
Author

Key is mainly a drupal specific framework for routing keys and providing a pluggable architecture for storage methods and key types. In Wordrpess there's no "Key" equivalent and they frown on framework plugins so we're building the Lockr module to fill that gap and be the key manager as well. With Key, Drush allows universal retrieval and each plugin could be included via compser. In WordPress, our WP-CLI will be specific to Lockr since there's no general framework. So running a drush command against key is the most universal way to implement secret management in Drupal since Key is the router.

All our Lockr plugins/modules share the same core library found at: https://packagist.org/packages/lockr/lockr-client The D8 version will include this library via composer. If using Lockr outside the CMS, one could include the library via composer and interact with it. There will just be a need to store the returned data somewhere like secrets.json since there is an extra layer of encryption on keys stored in Lockr.

The patch here though isn't meant to be specific to Lockr, it's for using Drupal to route the keys through the key module. We can look at other examples using composer etc. if that's a more desired example.

@greg-1-anderson
Copy link
Member

Still need to see how @joshkoenig wants to handle the key.php file here.

@joshkoenig
Copy link
Member

For now we are trying to steer away from premature abstraction, or the presentation of anything "library esque" in Quicksilver. It's early days. I'd prefer having one big file to emphasize that these are scripts and nothing more.

@cellar-door
Copy link
Author

Why don't I do this: I'll push the code back into the original slack_notifications and do a lookup via drush for the module so that _get_secrets() is able to leverage the Key module only if present and required secrets are missing. That should provide the broadest universal application and act as a jumping off point for folks wanting to use other secrets management software they may have.

For Wordpress users we may have to come up with a more universal way since Lockr is going to be the central routing point for keys for the time being.

I've got a few ideas brewing around Terminus now that I think will help provide a more generalized approach as well.

@greg-1-anderson
Copy link
Member

I think the strategy is to do all as you say in ^^, except copy key.php into the key-based slack notification example. If folks want to use this example to add key support for secrets somewhere else, then they'll have to copy this code from this example and paste it into their script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants