From 3c9ca1e527b19bb72c4835ccdbd22336c1fed180 Mon Sep 17 00:00:00 2001 From: Trevor Payne Date: Fri, 16 Apr 2021 12:29:41 -0500 Subject: [PATCH] Resoves #688 Added some QoL improvements to SecretsManager meant to protect the integrity of the providers dictionary from accidental manipulation Debug statement improvements Improvements to verbosity of console command returns for the SecretsManager --- .../Factory/DeviceFactory.cs | 7 +- .../Secrets/CrestronSecretsProvider.cs | 29 +++--- .../Secrets/Interfaces.cs | 2 +- .../Secrets/SecretsManager.cs | 97 ++++++++++++++----- 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Factory/DeviceFactory.cs b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Factory/DeviceFactory.cs index d4382442..ec041853 100644 --- a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Factory/DeviceFactory.cs +++ b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Factory/DeviceFactory.cs @@ -93,7 +93,8 @@ namespace PepperDash.Essentials.Core { if (prop.Name.ToLower() == "secret") { - var secret = GetSecret(JsonConvert.DeserializeObject(prop.Children().First().ToString())); + var secret = GetSecret(prop.Children().First().ToObject()); + //var secret = GetSecret(JsonConvert.DeserializeObject(prop.Children().First().ToString())); prop.Parent.Replace(secret); } var recurseProp = prop.Value as JObject; @@ -152,7 +153,9 @@ namespace PepperDash.Essentials.Core } catch (Exception ex) { - Debug.Console(2, "Issue with getting device - {0}", ex.Message); + Debug.Console(0, Debug.ErrorLogLevel.Error, "Exception occurred while creating device {0}: {1}", dc.Key, ex.Message); + + Debug.Console(2, "{0}", ex.StackTrace); return null; } } diff --git a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/CrestronSecretsProvider.cs b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/CrestronSecretsProvider.cs index 609aba64..3e0a5964 100644 --- a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/CrestronSecretsProvider.cs +++ b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/CrestronSecretsProvider.cs @@ -10,19 +10,22 @@ namespace PepperDash.Essentials.Core { public string Key { get; set; } //Added for reference - //private readonly bool _secureSupported; + private static readonly bool SecureSupported; public CrestronSecretsProvider(string key) { Key = key; + } + + static CrestronSecretsProvider() + { //Added for future encrypted reference - //_secureSupported = CrestronSecureStorage.Supported; + SecureSupported = CrestronSecureStorage.Supported; - //if (_secureSupported) - //{ - // return; - //} CrestronDataStoreStatic.InitCrestronDataStore(); - + if (SecureSupported) + { + //doThingsFuture + } } /// @@ -30,23 +33,23 @@ namespace PepperDash.Essentials.Core /// /// Secret Key /// Secret Value - public void SetSecret(string key, object value) + public bool SetSecret(string key, object value) { var secret = value as string; if (String.IsNullOrEmpty(secret)) { Debug.Console(2, this, "Unable to set secret for {0}:{1} - value is empty.", Key, key); - return; + return false; } var setErrorCode = CrestronDataStoreStatic.SetLocalStringValue(key, secret); switch (setErrorCode) { case CrestronDataStore.CDS_ERROR.CDS_SUCCESS: - Debug.Console(2, this,"Secret Successfully Set for {0}:{1}", Key, key); - break; + Debug.Console(1, this,"Secret Successfully Set for {0}:{1}", Key, key); + return true; default: Debug.Console(2, this, Debug.ErrorLogLevel.Notice, "Unable to set secret for {0}:{1} - {2}", Key, key, setErrorCode.ToString()); - break; + return false; } } @@ -68,7 +71,7 @@ namespace PepperDash.Essentials.Core default: Debug.Console(0, this, Debug.ErrorLogLevel.Notice, "Unable to retrieve secret for {0}:{1} - {2}", Key, key, getErrorCode.ToString()); - return new CrestronSecret(key, String.Empty, this); + return null; } } } diff --git a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/Interfaces.cs b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/Interfaces.cs index 86ae65cf..43c6a230 100644 --- a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/Interfaces.cs +++ b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/Interfaces.cs @@ -7,7 +7,7 @@ namespace PepperDash.Essentials.Core /// public interface ISecretProvider : IKeyed { - void SetSecret(string key, object value); + bool SetSecret(string key, object value); ISecret GetSecret(string key); } diff --git a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/SecretsManager.cs b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/SecretsManager.cs index f50051da..bcb46ff5 100644 --- a/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/SecretsManager.cs +++ b/essentials-framework/Essentials Core/PepperDashEssentialsBase/Secrets/SecretsManager.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using Crestron.SimplSharp; using PepperDash.Core; @@ -9,15 +8,16 @@ namespace PepperDash.Essentials.Core { public static class SecretsManager { - public static List Secrets { get; set; } + public static Dictionary Secrets { get; private set; } /// /// Initialize the SecretsManager /// public static void Initialize() { - Secrets = new List {new CrestronSecretsProvider("default")}; - + + AddSecretProvider("default", new CrestronSecretsProvider("default")); + CrestronConsole.AddNewConsoleCommand(SetSecretProcess, "setsecret", "Adds secrets to secret provider", ConsoleAccessLevelEnum.AccessOperator); @@ -29,18 +29,24 @@ namespace PepperDash.Essentials.Core CrestronConsole.AddNewConsoleCommand(DeleteSecretProcess, "deletesecret", "Deletes secrets in secret provider", ConsoleAccessLevelEnum.AccessAdministrator); - + } + static SecretsManager() + { + Secrets = new Dictionary(); } /// - /// Method to return a ISecretProvider to Set, Get, and Delete Secrets + /// Get Secret Provider from dictionary by key /// - /// Secret Provider Key - /// + /// Dictionary Key for provider + /// ISecretProvider public static ISecretProvider GetSecretProviderByKey(string key) { - var secret = Secrets.FirstOrDefault(o => o.Key == key); + ISecretProvider secret; + + Secrets.TryGetValue(key, out secret); + if (secret == null) { Debug.Console(1, "SecretsManager unable to retrieve SecretProvider with the key '{0}'", key); @@ -48,6 +54,44 @@ namespace PepperDash.Essentials.Core return secret; } + /// + /// Add secret provider to secrets dictionary + /// + /// Key of new entry + /// New Provider Entry + public static void AddSecretProvider(string key, ISecretProvider provider) + { + if (!Secrets.ContainsKey(key)) + { + Secrets.Add(key, provider); + Debug.Console(1, "Secrets provider '{0}' added to SecretsManager", key); + } + Debug.Console(0, Debug.ErrorLogLevel.Notice, "Unable to add Provider '{0}' to Secrets. Provider with that key already exists", key ); + } + + /// + /// Add secret provider to secrets dictionary, with optional overwrite parameter + /// + /// Key of new entry + /// New provider entry + /// true to overwrite any existing providers in the dictionary + public static void AddSecretProvider(string key, ISecretProvider provider, bool overwrite) + { + if (!Secrets.ContainsKey(key)) + { + Secrets.Add(key, provider); + Debug.Console(1, "Secrets provider '{0}' added to SecretsManager", key); + + } + if (overwrite) + { + Secrets.Add(key, provider); + Debug.Console(1, Debug.ErrorLogLevel.Notice, "Provider with the key '{0}' already exists in secrets. Overwriting with new secrets provider.", key); + + } + Debug.Console(0, Debug.ErrorLogLevel.Notice, "Unable to add Provider '{0}' to Secrets. Provider with that key already exists", key); + } + private static void SetSecretProcess(string cmd) { string response; @@ -76,7 +120,7 @@ namespace PepperDash.Essentials.Core } - var provider = Secrets.FirstOrDefault(o => o.Key == args[0]); + var provider = GetSecretProviderByKey(args[0]); if (provider == null) { @@ -92,11 +136,14 @@ namespace PepperDash.Essentials.Core if (provider.GetSecret(key) == null) { - provider.SetSecret(key, secret); - response = - String.Format( + + response = provider.SetSecret(key, secret) + ? String.Format( "Secret successfully set for {0}:{1}", - provider.Key, key); + provider.Key, key) + : String.Format( + "Unable to set secret for {0}:{1}", + provider.Key, key); CrestronConsole.ConsoleCommandResponse(response); return; } @@ -137,7 +184,7 @@ namespace PepperDash.Essentials.Core } - var provider = Secrets.FirstOrDefault(o => o.Key == args[0]); + var provider = GetSecretProviderByKey(args[0]); if (provider == null) { @@ -153,10 +200,12 @@ namespace PepperDash.Essentials.Core if (provider.GetSecret(key) != null) { - provider.SetSecret(key, secret); - response = - String.Format( - "Secret successfully updated for {0}:{1}", + response = provider.SetSecret(key, secret) + ? String.Format( + "Secret successfully set for {0}:{1}", + provider.Key, key) + : String.Format( + "Unable to set secret for {0}:{1}", provider.Key, key); CrestronConsole.ConsoleCommandResponse(response); return; @@ -199,7 +248,7 @@ namespace PepperDash.Essentials.Core } - var provider = Secrets.FirstOrDefault(o => o.Key == args[0]); + var provider = GetSecretProviderByKey(args[0]); if (provider == null) { @@ -214,11 +263,15 @@ namespace PepperDash.Essentials.Core provider.SetSecret(key, ""); - response = - String.Format( + response = provider.SetSecret(key, "") + ? String.Format( "Secret successfully deleted for {0}:{1}", + provider.Key, key) + : String.Format( + "Unable to delete secret for {0}:{1}", provider.Key, key); CrestronConsole.ConsoleCommandResponse(response); + return; }