PowerShell / Modules

MIT License
112 stars 25 forks source link

[SecretsManagement][Feature Request] Secrets Vault Initialization for Extension Authors #36

Closed JustinGrote closed 4 years ago

JustinGrote commented 4 years ago

Summary of the new feature/enhancement

As a Secrets Management Extension author, I want a way to validate Register-Secretsvault at runtime so I can let the user immediately know if he supplied the wrong format password, if the path is invalid, etc.

Proposed technical implementation details (optional)

Pull in Register-SecretVault from the implementing module, so that logic can be placed there rather than prior to every single other command.

Jaykul commented 4 years ago

The current implementation forces the logic to be in every single command anyway -- but even if that was not fixed, I agree:

There should be a Register (or Initialize) on the vault extensions which Register-SecretsVault would call during initialization to ensure that the VaultParameters work and prevent registration if they do not. Otherwise, if you make a mistake, you're stuck in this weird loop calling: Register, Get, Unregister over and over until you get your connection information correct (whether that's an API token, a URI, credentials, or the path to a .kdbx database).

PaulHigin commented 4 years ago

I don't understand what the proposed solution is. But I thought about having a fifth required vault function that validates the registered vault (it is currently commented out in the source code, 'ValidateExtension'). The function could return a simple boolean plus as much diagnostic error information as needed.

I am not sure about preventing vault registration ... it would be a shame to deny registration just because a site is momentarily down. But maybe it could be an option (or again the use of a '-Force' parameter to always register).

JustinGrote commented 4 years ago

@PaulHigin Somehow being able to return some sort of validation message would be helpful so extension authors can shorten the feedback loop to users if they mis-enter or forget to include a mandatory vault parameter. It could be as simple as some basic vaultparameter validation (e.g. you put special characters in an Azure Key Vault Name) and go so far as do a full validation. The depth of the implementation would be up to the Author.

Jaykul commented 4 years ago

it would be a shame to deny registration just because a site is momentarily down

But what if it's because the URL was wrong? Or API key you put in your additional parameters was wrong?

The point is to let the vault module (author) decide whether the registration parameters work or not -- and provide feedback to the users at the right time: when the user is entering the registration parameters.

Otherwise we're going to get the errors later, when we think we're doing actions on secrets!

PaulHigin commented 4 years ago

Added support for TestSecretVault function in binary and script extension modules. It currently returns True or False, and can return multiple errors (or write to error stream for script/cmdlet version of the function) for diagnostics.

Currently this new function is only called from a new SecretManagement module cmdlet, Test-SecretVault. We decided not to make TestSecretVault success a condition for registering an extension vault. If the test fails for a temporary problem (network issue, other set up steps that still need to be performed), we don't want to prevent the extension from being registered. So, currently a user has to manually call Test-SecretVault cmdlet and unregister the extension if needed.

But this behavior can be tweaked as we move forward.

JustinGrote commented 4 years ago

@PaulHigin Thanks! I'll give this a shot with keepass and see how well it works in practice.