Determine usability of Arcus.Security

[stijnmoreels]

Arcus.Security has been arround for some time, but the maintainence is not always easy to garantuee. To continue the simplification and usability of the library, some decisions should be made. This discussion item acts as a parent to discus/argument against certain proposal ideas noted here.

To make Arcus.Security more easier, modular to implement in new and existing projects, we should:

• ❌ Remove the Arcus.Security.AzureFunctions project: as this has no addtional value, besides a delegated extensions.
  • ➡️ We can safely remove this entire project in the solution, as seperate projects means seperate NuGet packages.
• ❌ Remove Arcus.Observability in Arcus.Security.Core`: since custom observability is being in review, and to have less a hard-link between Arcus libraries, we should make sure that we don't depend directly on this library. Currently, it is only being used for auditing reasons, when getting secrets, this can be done differently in a number of ways.
• ❌ Reduce GetSecret... calls in ISecretProvider: currently, there are number of ways to get secrets. These should be reviewed and simplified greatly.
  • GetSecret(Async), GetRawSecret(Async), should become GetSecretAsync/GetSecret in the ISecretProvider interface. This removes the need for having a ISyncSecretProvider interface and lets implementations having built-in capabilities for sync/async choices, instead of being aware of two seperate interfaces.
  • Add implicit operator for Secret, so that it is easier to transform to string, but remove all the GetRawSecret... calls, as it is mostly duplicated logic.
• ❌ Reduce the ISecretStore methods and become main point of contact: instead of using the ISecretProvider directly for users to use, we should think about using the ISecretStore as the central point. It would allow easier understanding of the secret store, as well as providing functionality to users without having the secret providers to update each time.
  • ❌ Use only GetProvider(...) and GetProvider<T>(...), and remove the cached-versions.
  • ➕ Add GetSecret(Async)
• ❌ Remove the IVersionedSecretProvider interface, in favor of using the secret providers directly. Only the Key vault secret provider, is currently making sense of using versions.
• ❌ Use a DelegateSecretProvider instead of a mutated/cached variant, using a functional way instead.

[fgheysels]

I agree with most of the points above.
However, you propose to remove the ISecretProvider interface and work with ISecretStore instead. However, you're also talking about introducing a DelegateSecretProvider ? Can't we make caching something that is being configured on the ISecretStore ?

[stijnmoreels]

Yes, the DelegateSecretProvider could act as a 'delegate' for anything that needs to 'enrich' during the secret retrievel: secret name conversion, caching... it hides a bit the infrastructure from the 'core' secret retrieval. But yes, from a user perspective, the configuration still happens on a secret provider-basis as not all providers require/make-sense to add caching, maybe.

[stijnmoreels]

Proposal would be something like this, where there only exists two interfaces: ISecretProvider and ISecretStore:

/// <summary>
/// Represents a provider that can retrieve secrets based on a given name.
/// </summary>
public interface ISecretProvider
{
    /// <summary>
    /// Gets a stored secret by its name.
    /// </summary>
    /// <param name="secretName">The </param>
    /// <param name="configureOptions"></param>
    Task<SecretResult> GetSecretAsync(string secretName, Action<SecretOptions> configureOptions)
    {
        return Task.FromResult(GetSecret(secretName, configureOptions));
    }

    /// <summary>
    /// Gets a stored secret by its name.
    /// </summary>
    /// <param name="secretName"></param>
    /// <param name="configureOptions"></param>
    /// <returns></returns>
    SecretResult GetSecret(string secretName, Action<SecretOptions> configureOptions);
}

/// <summary>
/// Represents the central point of contact to retrieve secrets from registered <see cref="ISecretProvider"/>s in the user application.
/// </summary>
public interface ISecretStore : ISecretProvider
{
    /// <summary>
    /// Gets the registered named <see cref="ISecretProvider"/> from the secret store.
    /// </summary>
    /// <typeparam name="TProvider">The concrete type of the secret provider implementation.</typeparam>
    /// <param name="providerName">
    ///     The name of the concrete secret provider implementation;
    ///     uses the FQN (fully-qualified name) of the type in case none is provided.
    /// </param>
    /// <exception cref="ArgumentException">Thrown when the <paramref name="providerName"/> is blank.</exception>
    TProvider GetProvider<TProvider>(string providerName) where TProvider : ISecretProvider;
}

But before we go any further, I would want to have some clarity from management as to the budget of Arcus; as this will again take quite some effort.

[stijnmoreels]

See https://github.com/stijnmoreels/arcus.security/tree/idea/ideal-secret-store for more details.