PowerShell / Modules

MIT License
112 stars 25 forks source link

[SecretsManagement][Design] The *Secret commands should include a specific parameter for the registered vault name #49

Closed Jaykul closed 4 years ago

Jaykul commented 4 years ago

Summary of the new feature/enhancement

As a developer of secret vaults In order to support caching configuration The registered name of the vault should be passed in to the vault module when commands are called And it should be a valid file name (i.e. should not contain [IO.Path]::GetInvalidFileNameChars()) or perhaps should be guaranteed alpha-numeric

Proposed technical implementation details

Every call that receives VaultParameters should also receive a VaultName.

Given that #36 is implemented for validation of vault parameters, the vault name would be passed in too. Then the vault provider would be able (but not required) to cache any connection or authentication token that was generated for validation.

NOTE: that this does not require that the Register command is called in every PowerShell session, assuming key vault registration is persisted ...

For really simple use cases, this name could serve as a folder name, for instance, in the TestLocalScript example which currently hard-codes 'TestVault18' ...

For more complex use cases, it could serve as a key into a hashtable of cached connections, and each command would essentially be calling some internal implementation like GetRegisteredSecretsVault ... which could cache connections in the session, or user scope, or machine scope, or ... whatever the vault provider supports.

PaulHigin commented 4 years ago

I went back and forth on this, but eventually left VaultName out because I couldn't think of a good reason to have it. Also, I want to have explicit parameters be mandatory for all vault implementations and VaultName didn't seem to fit. 'VaultName' feels more like optional parameters for specific vault implementations, and can be passed in via the additional parameters dictionary.

Jaykul commented 4 years ago

The problem is that you're already requiring the users to type a name when they register ... and they can register my module multiple times, but I can't tell one registration of my module from another.

So the current design basically forces me to add a mandatory "additional parameter" just so I can distinguish one registration from another. Not only does that force the users to repeat themselves, but the current UX for mandatory additional parameters is basically ... me throwing an exception listing them.

Could we maybe just add the vaultName to the hashtable that already gets passed in?

PaulHigin commented 4 years ago

The extension class and script functions now require a 'VaultName' parameter, which is passed in for each call.