From 65c1f0c3ce6a617c4d156a2c230a330399dbc2f3 Mon Sep 17 00:00:00 2001 From: msJinLei Date: Mon, 30 Jan 2023 01:40:14 +0800 Subject: [PATCH 1/5] Fix AzKeyStore issues and optimize initialization and update * Enabled credential to be found only by applicationId while tenant was not matched when accquire token. [#20484] * When Az.Accounts ran in parallel, the waiters were allowed to wait infinitely to avoid throw exception in automation enviroment. [#20455] * Used Lazy load for AzKeyStore. * Used update on change mechanism for AzKeyStore and remove `Flush` interface. --- src/Accounts/Accounts.Test/AutosaveTests.cs | 4 +- .../Accounts.Test/ContextCmdletTests.cs | 18 +- .../Accounts.Test/ProfileCmdletTests.cs | 2 +- .../Accounts/Account/ConnectAzureRmAccount.cs | 13 +- .../AutoSave/DisableAzureRmContextAutosave.cs | 5 - .../AutoSave/EnableAzureRmContextAutosave.cs | 7 - src/Accounts/Accounts/ChangeLog.md | 4 + .../Accounts/Context/ImportAzureRMContext.cs | 4 +- .../Accounts/Context/SetAzureRMContext.cs | 4 +- .../AzureRmProfile.cs | 8 +- .../Authentication.Test/AzKeyStorageTest.cs | 94 +++--- .../Authentication.Test/StorageHelperTest.cs | 88 ++++++ .../Factories/AuthenticationFactory.cs | 8 +- .../Identity/AsyncLockWithValue.cs | 18 ++ .../Authentication/KeyStore/AzKeyStore.cs | 210 ++----------- .../Authentication/KeyStore/IKeyStore.cs | 34 +++ .../Authentication/KeyStore/IKeyStoreKey.cs | 2 + .../Authentication/KeyStore/IStorage.cs | 11 +- .../Authentication/KeyStore/IStorageHelper.cs | 35 +++ .../KeyStore/InMemoryKeyStore.cs | 222 ++++++++++++++ .../KeyStore/KeyStoreNotificationArgs.cs | 23 ++ .../KeyStore/SecureStringConverter.cs | 1 + .../KeyStore/ServicePrincipalKey.cs | 9 + .../Authentication/KeyStore/StorageHelper.cs | 282 ++++++++++++++++++ .../Authentication/KeyStore/StorageWrapper.cs | 111 +------ 25 files changed, 843 insertions(+), 374 deletions(-) create mode 100644 src/Accounts/Authentication.Test/StorageHelperTest.cs create mode 100644 src/Accounts/Authentication/KeyStore/IKeyStore.cs create mode 100644 src/Accounts/Authentication/KeyStore/IStorageHelper.cs create mode 100644 src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs create mode 100644 src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs create mode 100644 src/Accounts/Authentication/KeyStore/StorageHelper.cs diff --git a/src/Accounts/Accounts.Test/AutosaveTests.cs b/src/Accounts/Accounts.Test/AutosaveTests.cs index fd3a2df838bb..50ea60b43fe5 100644 --- a/src/Accounts/Accounts.Test/AutosaveTests.cs +++ b/src/Accounts/Accounts.Test/AutosaveTests.cs @@ -51,9 +51,7 @@ private AzKeyStore SetMockedAzKeyStore() storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object); storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]); storageMocker.Setup(f => f.WriteData(It.IsAny())).Callback((byte[] s) => {}); - var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, false, storageMocker.Object); - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); + var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object); return keyStore; } diff --git a/src/Accounts/Accounts.Test/ContextCmdletTests.cs b/src/Accounts/Accounts.Test/ContextCmdletTests.cs index dcbebaf3f36e..7b977a5a7482 100644 --- a/src/Accounts/Accounts.Test/ContextCmdletTests.cs +++ b/src/Accounts/Accounts.Test/ContextCmdletTests.cs @@ -23,17 +23,21 @@ using Microsoft.WindowsAzure.Commands.ScenarioTest; using Microsoft.WindowsAzure.Commands.Test.Utilities.Common; using Microsoft.WindowsAzure.Commands.Utilities.Common; -using Xunit; -using Xunit.Abstractions; using Microsoft.Azure.Commands.Common.Authentication.Abstractions; -using System; +using Microsoft.Azure.Commands.Common.Authentication.Properties; using Microsoft.Azure.Commands.Profile.Context; -using System.Linq; using Microsoft.Azure.Commands.Common.Authentication.ResourceManager; using Microsoft.Azure.Commands.Profile.Common; using Microsoft.Azure.Commands.ScenarioTest.Mocks; using Microsoft.Azure.Commands.TestFx.Mocks; using Microsoft.Azure.Commands.TestFx; +using Microsoft.Azure.Commands.ResourceManager.Common; +using Moq; +using System; +using System.IO; +using System.Linq; +using Xunit; +using Xunit.Abstractions; namespace Microsoft.Azure.Commands.Profile.Test { @@ -56,6 +60,12 @@ public ContextCmdletTests(ITestOutputHelper output) tokenCacheProviderMock = new MockPowerShellTokenCacheProvider(); AzureSession.Instance.RegisterComponent(PowerShellTokenCacheProvider.PowerShellTokenCacheProviderKey, () => tokenCacheProviderMock); Environment.SetEnvironmentVariable("Azure_PS_Data_Collection", "True"); + + Mock storageMocker = new Mock(); + AzKeyStore azKeyStore = null; + string profilePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName); + azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, storageMocker.Object); + AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => azKeyStore, true); } [Fact] diff --git a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs index 1dab2d754f7b..386998a78ffd 100644 --- a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs +++ b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs @@ -55,7 +55,7 @@ private AzKeyStore SetMockedAzKeyStore() storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object); storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]); storageMocker.Setup(f => f.WriteData(It.IsAny())).Callback((byte[] s) => { }); - var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, false, storageMocker.Object); + var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object); return keyStore; } diff --git a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs index 5597cb0e98ec..218fc28abe8f 100644 --- a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs +++ b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs @@ -425,7 +425,7 @@ public override void ExecuteCmdlet() azureAccount.SetProperty(AzureAccount.Property.CertificatePath, resolvedPath); if (CertificatePassword != null) { - keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword); + keyStore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword); if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected) { WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory)); @@ -451,7 +451,7 @@ public override void ExecuteCmdlet() if (azureAccount.Type == AzureAccount.AccountType.ServicePrincipal && password != null) { - keyStore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret + keyStore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret ,azureAccount.Id, Tenant), password); if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected) { @@ -713,9 +713,7 @@ public void OnImport() } AzKeyStore keyStore = null; - keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile, false, autoSaveEnabled); - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); + keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile); AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore); if (!InitializeProfileProvider(autoSaveEnabled)) @@ -724,11 +722,6 @@ public void OnImport() autoSaveEnabled = false; } - if (!keyStore.LoadStorage()) - { - WriteInitializationWarnings(Resources.KeyStoreLoadingError); - } - IAuthenticatorBuilder builder = null; if (!AzureSession.Instance.TryGetComponent(AuthenticatorBuilder.AuthenticatorBuilderKey, out builder)) { diff --git a/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs b/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs index c9f768733cb7..1d69aba78ce5 100644 --- a/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs +++ b/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs @@ -92,11 +92,6 @@ void DisableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextA builder.Reset(); } - if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore)) - { - keystore.DisableAutoSaving(); - } - if (writeAutoSaveFile) { FileUtilities.EnsureDirectoryExists(session.ProfileDirectory); diff --git a/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs b/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs index fae6a7aed5b5..66325469ea71 100644 --- a/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs +++ b/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs @@ -102,13 +102,6 @@ void EnableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextAu AzureSession.Instance.RegisterComponent(PowerShellTokenCacheProvider.PowerShellTokenCacheProviderKey, () => newCacheProvider, true); } - if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore)) - { - keystore.Flush(); - keystore.DisableAutoSaving(); - } - - if (writeAutoSaveFile) { try diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md index 2b2ef8aee096..ebd8bc1d7230 100644 --- a/src/Accounts/Accounts/ChangeLog.md +++ b/src/Accounts/Accounts/ChangeLog.md @@ -20,6 +20,10 @@ ## Upcoming Release * Supported Web Account Manager on ARM64-based Windows systems. Fixed an issue where `Connect-AzAccount` failed with error "Unable to load DLL 'msalruntime_arm64'". [#20700] +* Enabled credential to be found only by applicationId while tenant was not matched when accquire token. [#20484] +* When Az.Accounts ran in parallel, the waiters were allowed to wait infinitely to avoid throw exception in automation enviroment. [#20455] +* Used Lazy load for AzKeyStore. +* Used update on change mechanism for AzKeyStore and remove `Flush` interface. ## Version 2.11.1 * Fixed an issue where Az.Accounts cannot be imported correctly. [#20615] diff --git a/src/Accounts/Accounts/Context/ImportAzureRMContext.cs b/src/Accounts/Accounts/Context/ImportAzureRMContext.cs index 18cf276a6f42..9304d88d13ba 100644 --- a/src/Accounts/Accounts/Context/ImportAzureRMContext.cs +++ b/src/Accounts/Accounts/Context/ImportAzureRMContext.cs @@ -78,13 +78,13 @@ void CopyProfile(AzureRmProfile source, IProfileOperations target) var secret = account.GetProperty(AzureAccount.Property.ServicePrincipalSecret); if (!string.IsNullOrEmpty(secret)) { - keyStore.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, context.Value.Tenant?.Id) + keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, context.Value.Tenant?.Id) , secret.ConvertToSecureString()); } var password = account.GetProperty(AzureAccount.Property.CertificatePassword); if (!string.IsNullOrEmpty(password)) { - keyStore.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, context.Value.Tenant?.Id) + keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, context.Value.Tenant?.Id) ,password.ConvertToSecureString()); } } diff --git a/src/Accounts/Accounts/Context/SetAzureRMContext.cs b/src/Accounts/Accounts/Context/SetAzureRMContext.cs index 716d2e940b75..fab411685fea 100644 --- a/src/Accounts/Accounts/Context/SetAzureRMContext.cs +++ b/src/Accounts/Accounts/Context/SetAzureRMContext.cs @@ -97,13 +97,13 @@ public override void ExecuteCmdlet() var secret = account.GetProperty(AzureAccount.Property.ServicePrincipalSecret); if (!string.IsNullOrEmpty(secret)) { - keyStore.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, Context.Tenant?.Id) + keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, Context.Tenant?.Id) , secret.ConvertToSecureString()); } var password = account.GetProperty(AzureAccount.Property.CertificatePassword); if (!string.IsNullOrEmpty(password)) { - keyStore.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, Context.Tenant?.Id) + keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, Context.Tenant?.Id) , password.ConvertToSecureString()); } } diff --git a/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs b/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs index d6738055f6d6..9c81702be477 100644 --- a/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs +++ b/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs @@ -225,13 +225,13 @@ private IAzureContext MigrateSecretToKeyStore(IAzureContext context, AzKeyStore var account = context.Account; if (account.IsPropertySet(AzureAccount.Property.ServicePrincipalSecret)) { - keystore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First()) + keystore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First()) , account.ExtendedProperties.GetProperty(AzureAccount.Property.ServicePrincipalSecret).ConvertToSecureString()); account.ExtendedProperties.Remove(AzureAccount.Property.ServicePrincipalSecret); } if (account.IsPropertySet(AzureAccount.Property.CertificatePassword)) { - keystore?.SaveKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First()) + keystore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First()) , account.ExtendedProperties.GetProperty(AzureAccount.Property.CertificatePassword).ConvertToSecureString()); account.ExtendedProperties.Remove(AzureAccount.Property.CertificatePassword); } @@ -336,10 +336,6 @@ public void Save(IFileProvider provider, bool serializeCache = true) // so that previous data is overwritten provider.Stream.SetLength(provider.Stream.Position); } - - AzKeyStore keystore = null; - AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out keystore); - keystore?.Flush(); } finally { diff --git a/src/Accounts/Authentication.Test/AzKeyStorageTest.cs b/src/Accounts/Authentication.Test/AzKeyStorageTest.cs index 64f8a6eadb83..3e6165a547b3 100644 --- a/src/Accounts/Authentication.Test/AzKeyStorageTest.cs +++ b/src/Accounts/Authentication.Test/AzKeyStorageTest.cs @@ -11,6 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- +using Microsoft.Azure.Commands.Common.Authentication.Properties; using Microsoft.Azure.Commands.ResourceManager.Common; using Microsoft.WindowsAzure.Commands.Common; using Microsoft.WindowsAzure.Commands.ScenarioTest; @@ -18,8 +19,8 @@ using Newtonsoft.Json; using System; using System.Collections.Generic; +using System.IO; using System.Linq; -using System.Security; using System.Text; using Xunit; @@ -29,7 +30,7 @@ public class AzKeyStorageTest { private Mock storageMocker = null; private List storageChecker = null; - private string dummpyPath = "/home/dummy/.Azure"; + private string dummpyPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName); private string keyStoreFileName = "azkeystore"; public AzKeyStorageTest() @@ -55,25 +56,23 @@ private static bool CompareJsonObjects(string expected, string acutal) [Trait(Category.AcceptanceType, Category.CheckIn)] public void SaveKey() { - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, false, true, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) { - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); - IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); var secret = "secret".ConvertToSecureString(); - store.SaveKey(servicePrincipalKey, secret); + store.SaveCredential(servicePrincipalKey, secret); IKeyStoreKey certificatePassword = new ServicePrincipalKey("CertificatePassword", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); var passowrd = "password".ConvertToSecureString(); - store.SaveKey(certificatePassword, passowrd); + store.SaveCredential(certificatePassword, passowrd); + + var result = Encoding.UTF8.GetString(storageChecker.ToArray()); + const string EXPECTEDSTRING = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""CertificatePassword\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""password\""""},{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; + Assert.True(CompareJsonObjects(EXPECTEDSTRING, result)); - store.Flush(); + store.Clear(); } storageMocker.Verify(); - var result = Encoding.UTF8.GetString(storageChecker.ToArray()); - const string EXPECTEDSTRING = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""CertificatePassword\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""password\""""},{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; - Assert.True(CompareJsonObjects(EXPECTEDSTRING, result)); } [Fact] @@ -82,16 +81,34 @@ public void FindKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, false, true, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) { - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); - store.LoadStorage(); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - var secret = store.GetKey(servicePrincipalKey); + var secret = store.GetCredential(servicePrincipalKey); Assert.Equal("secret", secret.ConvertToString()); + + store.Clear(); + } + storageMocker.Verify(); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void FindFallbackKey() + { + const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secretFallback\""""}]"; + storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + { + storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); + + IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-0000-bad9-b7b93a3e9c5a"); + var secret = store.GetCredential(servicePrincipalKey); + Assert.Equal("secretFallback", secret.ConvertToString()); + + store.Clear(); } storageMocker.Verify(); } @@ -103,15 +120,14 @@ public void FindNoKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, false, true, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) { - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); - store.LoadStorage(); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("CertificatePassword", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - Assert.Throws(() => store.GetKey(servicePrincipalKey)); + Assert.Throws(() => store.GetCredential(servicePrincipalKey)); + + store.Clear(); } storageMocker.Verify(); } @@ -122,21 +138,20 @@ public void RemoveKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, false, true, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) { - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); - store.LoadStorage(); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - store.DeleteKey(servicePrincipalKey); - store.Flush(); + store.RemoveCredential(servicePrincipalKey); + + var result = Encoding.UTF8.GetString(storageChecker.ToArray()); + var objects = JsonConvert.DeserializeObject>(result); + Assert.Empty(objects); + + store.Clear(); } storageMocker.Verify(); - var result = Encoding.UTF8.GetString(storageChecker.ToArray()); - var objects = JsonConvert.DeserializeObject>(result); - Assert.Empty(objects); } [Fact] @@ -145,21 +160,20 @@ public void RemoveNoKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, false, true, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) { - AzKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - AzKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); - store.LoadStorage(); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("CertificatePassword", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - store.DeleteKey(servicePrincipalKey); - store.Flush(); + store.RemoveCredential(servicePrincipalKey); + + var result = Encoding.UTF8.GetString(storageChecker.ToArray()); + var objects = JsonConvert.DeserializeObject>(result); + Assert.Single(objects); + + store.Clear(); } storageMocker.Verify(); - var result = Encoding.UTF8.GetString(storageChecker.ToArray()); - var objects = JsonConvert.DeserializeObject>(result); - Assert.Single(objects); } } } diff --git a/src/Accounts/Authentication.Test/StorageHelperTest.cs b/src/Accounts/Authentication.Test/StorageHelperTest.cs new file mode 100644 index 000000000000..a261cae27470 --- /dev/null +++ b/src/Accounts/Authentication.Test/StorageHelperTest.cs @@ -0,0 +1,88 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- +using Microsoft.Azure.Commands.Common.Authentication.Properties; +using Microsoft.Azure.Commands.ResourceManager.Common; +using Microsoft.WindowsAzure.Commands.ScenarioTest; +using Moq; +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; + +using Xunit; + +namespace Common.Authenticators.Test +{ + public class StorageHelperTest + { + private Mock storageMocker = null; + private Mock keystoreMocker = null; + private string profilePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName); + private string keyStoreFileName = "azkeystore"; + + public StorageHelperTest() + { + storageMocker = new Mock(); + storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object); + keystoreMocker = new Mock(); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void LoadFromStorageTest() + { + const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; + string actual = null; + List storageChecker = new List(); + storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); + storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); + keystoreMocker.Setup(f => f.Deserialize(It.IsAny(), It.IsAny())).Callback( + (byte[] s, bool c) => { actual = Encoding.UTF8.GetString(s); }); + var helper = StorageHelper.GetStorageHelperAsync(true, keyStoreFileName, profilePath + , keystoreMocker.Object, storageMocker.Object).GetAwaiter().GetResult(); + helper.LoadFromCachedStorage(keystoreMocker.Object); + Assert.Equal(EXPECTED, actual); + storageMocker.Verify(); + keystoreMocker.Verify(); + StorageHelper.TryClearLockedStorageHelper(); + } + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void SaveToStorageTest() + { + const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; + List storageChecker = new List(), keystoreChecker = new List(); + keystoreChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); + + storageMocker.Setup(f => f.WriteData(It.IsAny())).Callback( + (byte[] s) => { storageChecker.AddRange(s); }); + keystoreMocker.Setup(f => f.Serialize()).Returns(keystoreChecker.ToArray()); + + var helper = StorageHelper.GetStorageHelperAsync(true, keyStoreFileName, profilePath + , keystoreMocker.Object, storageMocker.Object).GetAwaiter().GetResult(); + var args = new KeyStoreNotificationArgs() + { + KeyStore = keystoreMocker.Object + }; + helper.WriteToCachedStorage(args); + + string actual = Encoding.UTF8.GetString(storageChecker.ToArray()); + Assert.Equal(EXPECTED, actual); + storageMocker.Verify(); + keystoreMocker.Verify(); + StorageHelper.TryClearLockedStorageHelper(); + } + } +} diff --git a/src/Accounts/Authentication/Factories/AuthenticationFactory.cs b/src/Accounts/Authentication/Factories/AuthenticationFactory.cs index 721841e2d8dd..830d0520d6b4 100644 --- a/src/Accounts/Authentication/Factories/AuthenticationFactory.cs +++ b/src/Accounts/Authentication/Factories/AuthenticationFactory.cs @@ -433,9 +433,9 @@ public void RemoveUser(IAzureAccount account, IAzureTokenCache tokenCache) case AzureAccount.AccountType.ServicePrincipal: try { - KeyStore.DeleteKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, + KeyStore.RemoveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().FirstOrDefault())); - KeyStore.DeleteKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, + KeyStore.RemoveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().FirstOrDefault())); } catch @@ -577,7 +577,7 @@ private AuthenticationParameters GetAuthenticationParameters( { try { - password = KeyStore.GetKey(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret + password = KeyStore.GetCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret , account.Id, tenant)); } catch @@ -591,7 +591,7 @@ private AuthenticationParameters GetAuthenticationParameters( { try { - certificatePassword = KeyStore.GetKey(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword + certificatePassword = KeyStore.GetCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword , account.Id, tenant)); } catch diff --git a/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs b/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs index e89ca5eb2394..6ac99a950115 100644 --- a/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs +++ b/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs @@ -126,6 +126,24 @@ private void SetValue(T value) } } + /// + /// Try to reset value and fail if value is locked. + /// + /// + public bool TryClearValue() + { + lock (_syncObj) + { + if (!_isLocked) + { + _value = default(T); + _hasValue = false; + return true; + } + } + return false; + } + /// /// Release the lock and allow next waiter acquire it /// diff --git a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs index 506f535ba225..7c610f20fd6d 100644 --- a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs +++ b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs @@ -11,12 +11,8 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- -using Newtonsoft.Json; using System; -using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Linq; -using System.Text; +using System.Security; namespace Microsoft.Azure.Commands.ResourceManager.Common { @@ -24,215 +20,69 @@ public class AzKeyStore : IDisposable { public const string Name = "AzKeyStore"; - internal class KeyStoreElement - { - public string keyType; - public string keyStoreKey; - public string valueType; - public string keyStoreValue; - } - - private static IDictionary _typeNameMap = new ConcurrentDictionary(); - - private static IDictionary _elementConverterMap = new ConcurrentDictionary(); + public string FileName { get; set; } + public string Directory { get; set; } - public static void RegisterJsonConverter(Type type, string typeName, JsonConverter converter = null) + private IKeyStore _inMemoryStore = null; + public IKeyStore InMemoryStore { - if (string.IsNullOrEmpty(typeName)) - { - throw new ArgumentNullException($"typeName cannot be empty."); - } - if (_typeNameMap.ContainsKey(type)) - { - if (string.Compare(_typeNameMap[type], typeName) != 0) - { - throw new ArgumentException($"{typeName} has conflict with {_typeNameMap[type]} with reference to {type}."); - } - } - else - { - _typeNameMap[type] = typeName; - } - if (converter != null) - { - _elementConverterMap[_typeNameMap[type]] = converter; - } + get => _inMemoryStore; + set => _inMemoryStore = value; } - private IDictionary _credentials = new ConcurrentDictionary(); - private IStorage _storage = null; - - private bool autoSave = true; - private Exception lastError = null; - - public IStorage Storage - { - get => _storage; - set => _storage = value; - } + private IStorage inputStorage = null; public bool IsProtected { - get => Storage.IsProtected; + get; + private set; } - public AzKeyStore() + public AzKeyStore(string directory, string fileName, IStorage storage = null) { + InMemoryStore = new InMemoryKeyStore(); + InMemoryStore.SetBeforeAccess(LoadStorage); - } + FileName = fileName; + Directory = directory; - public AzKeyStore(string directory, string fileName, bool loadStorage = true, bool autoSaveEnabled = true, IStorage inputStorage = null) - { - autoSave = autoSaveEnabled; - Storage = inputStorage ?? new StorageWrapper() - { - FileName = fileName, - Directory = directory - }; - Storage.Create(); - - if (loadStorage&&!LoadStorage()) - { - throw new InvalidOperationException("Failed to load keystore from storage."); - } - } + inputStorage = storage; - private object Deserialize(string typeName, string value) - { - Type t = null; - t = _typeNameMap.FirstOrDefault(item => item.Value == typeName).Key; - - if (t != null) - { - if (_elementConverterMap.ContainsKey(typeName)) - { - return JsonConvert.DeserializeObject(value, t, _elementConverterMap[typeName]); - } - else - { - return JsonConvert.DeserializeObject(value, t); - } - } - return null; + InMemoryKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); + InMemoryKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); } - public bool LoadStorage() - { - try - { - var data = Storage.ReadData(); - if (data != null && data.Length > 0) - { - var rawJsonString = Encoding.UTF8.GetString(data); - var serializableKeyStore = JsonConvert.DeserializeObject(rawJsonString, typeof(List)) as List; - if (serializableKeyStore != null) - { - foreach (var item in serializableKeyStore) - { - IKeyStoreKey keyStoreKey = Deserialize(item.keyType, item.keyStoreKey) as IKeyStoreKey; - if (keyStoreKey == null) - { - throw new ArgumentException($"Cannot parse the keystore {item.keyStoreKey} with the type {item.keyType}."); - } - var keyStoreValue = Deserialize(item.valueType, item.keyStoreValue); - if (keyStoreValue == null) - { - throw new ArgumentException($"Cannot parse the keystore {item.keyStoreValue} with the type {item.valueType}."); - } - _credentials[keyStoreKey] = keyStoreValue; - } - } - } - } - catch (Exception e) - { - lastError = e; - return false; - } - return true; - } - public void ClearCache() + private void LoadStorage(KeyStoreNotificationArgs args) { - _credentials.Clear(); + var asyncHelper = StorageHelper.GetStorageHelperAsync(true, FileName, Directory, args.KeyStore, inputStorage); + var helper = asyncHelper.GetAwaiter().GetResult(); + IsProtected = helper.IsProtected; } public void Clear() { - ClearCache(); - Storage.Clear(); - } - - public void Flush() - { - IList serializableKeyStore = new List(); - foreach (var item in _credentials) - { - var keyType = _typeNameMap[item.Key.GetType()]; - var key = _elementConverterMap.ContainsKey(keyType) ? - JsonConvert.SerializeObject(item.Key, _elementConverterMap[keyType]) : JsonConvert.SerializeObject(item.Key); - if (!string.IsNullOrEmpty(key)) - { - var valueType = _typeNameMap[item.Value.GetType()]; - serializableKeyStore.Add(new KeyStoreElement() - { - keyType = keyType, - keyStoreKey = key, - valueType = valueType, - keyStoreValue = _elementConverterMap.ContainsKey(valueType) ? - JsonConvert.SerializeObject(item.Value, _elementConverterMap[valueType]) : JsonConvert.SerializeObject(item.Value), - }) ; - } - } - var JsonString = JsonConvert.SerializeObject(serializableKeyStore); - Storage.WriteData(Encoding.UTF8.GetBytes(JsonString)); + InMemoryStore.Clear(); } public void Dispose() { - if (autoSave) - { - Flush(); - } - ClearCache(); - } - - public void SaveKey(IKeyStoreKey key, T value) - { - if (!_typeNameMap.ContainsKey(key.GetType()) || !_typeNameMap.ContainsKey(value.GetType())) - { - throw new InvalidOperationException("Please register key & values type before save it."); - } - _credentials[key] = value; - } - - public T GetKey(IKeyStoreKey key) - { - if (!_credentials.ContainsKey(key)) - { - throw new ArgumentException($"{key.ToString()} is not stored in AzKeyStore yet."); - } - return (T)_credentials[key]; - } - - public bool DeleteKey(IKeyStoreKey key) - { - return _credentials.Remove(key); + StorageHelper.TryClearLockedStorageHelper(); } - public void EnableAutoSaving() + public void SaveCredential(IKeyStoreKey key, SecureString value) { - autoSave = true; + InMemoryStore.SaveKey(key, value); } - public void DisableAutoSaving() + public SecureString GetCredential(IKeyStoreKey key) { - autoSave = false; + return InMemoryStore.GetKey(key); } - public Exception GetLastError() + public bool RemoveCredential(IKeyStoreKey key) { - return lastError; + return InMemoryStore.DeleteKey(key); } } } diff --git a/src/Accounts/Authentication/KeyStore/IKeyStore.cs b/src/Accounts/Authentication/KeyStore/IKeyStore.cs new file mode 100644 index 000000000000..e7c1f637bc4e --- /dev/null +++ b/src/Accounts/Authentication/KeyStore/IKeyStore.cs @@ -0,0 +1,34 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + public interface IKeyStore + { + void SaveKey(IKeyStoreKey key, T value); + + T GetKey(IKeyStoreKey key); + + bool DeleteKey(IKeyStoreKey key); + + void Clear(); + + void SetBeforeAccess(KeyStoreCallbak beforeAccess); + + void SetOnUpdate(KeyStoreCallbak onUpdate); + void Deserialize(byte[] Data, bool shouldClearExistingCache); + + byte[] Serialize(); + } +} diff --git a/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs b/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs index 7916b87bc3ef..4dbac64496d3 100644 --- a/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs +++ b/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs @@ -23,5 +23,7 @@ public abstract class IKeyStoreKey public override abstract bool Equals(object obj); public override abstract int GetHashCode(); + + public abstract bool BeEquivalent(object obj); } } diff --git a/src/Accounts/Authentication/KeyStore/IStorage.cs b/src/Accounts/Authentication/KeyStore/IStorage.cs index be9c8187e35c..ab8b6653973c 100644 --- a/src/Accounts/Authentication/KeyStore/IStorage.cs +++ b/src/Accounts/Authentication/KeyStore/IStorage.cs @@ -18,7 +18,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common public interface IStorage { IStorage Create(); - + void Clear(); byte[] ReadData(); @@ -26,12 +26,5 @@ public interface IStorage void VerifyPersistence(); void WriteData(byte[] data); - - Exception GetLastError(); - - bool IsProtected - { - get; - } } -} +} \ No newline at end of file diff --git a/src/Accounts/Authentication/KeyStore/IStorageHelper.cs b/src/Accounts/Authentication/KeyStore/IStorageHelper.cs new file mode 100644 index 000000000000..eaa4c1619726 --- /dev/null +++ b/src/Accounts/Authentication/KeyStore/IStorageHelper.cs @@ -0,0 +1,35 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- +using System; + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + public interface IStorageHelper + { + void Clear(); + + byte[] LoadUnencryptedTokenCache(); + + void SaveUnencryptedTokenCache(byte[] tokenCache); + + void LoadFromCachedStorage(IKeyStore keystore); + + void WriteToCachedStorage(KeyStoreNotificationArgs args); + + bool IsProtected + { + get; + } + } +} \ No newline at end of file diff --git a/src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs b/src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs new file mode 100644 index 000000000000..de4a3be78306 --- /dev/null +++ b/src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs @@ -0,0 +1,222 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- +using Microsoft.Identity.Client; +using Newtonsoft.Json; +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + internal class InMemoryKeyStore : IKeyStore + { + internal class KeyStoreElement + { + public string keyType; + public string keyStoreKey; + public string valueType; + public string keyStoreValue; + } + + private static IDictionary _typeNameMap = new ConcurrentDictionary(); + + private static IDictionary _elementConverterMap = new ConcurrentDictionary(); + + private IDictionary _credentials = new ConcurrentDictionary(); + + private readonly object lockObj = new object(); + + internal KeyStoreCallbak BeforeAccess = null; + + internal KeyStoreCallbak OnUpdate = null; + + public void SaveKey(IKeyStoreKey key, T value) + { + var args = new KeyStoreNotificationArgs() + { + KeyStore = this + }; + BeforeAccess?.Invoke(args) ; + if (!_typeNameMap.ContainsKey(key.GetType()) || !_typeNameMap.ContainsKey(value.GetType())) + { + throw new InvalidOperationException("Please register key & values type before save it."); + } + _credentials[key] = value; + OnUpdate?.Invoke(args); + } + + public T GetKey(IKeyStoreKey key) + { + var args = new KeyStoreNotificationArgs() + { + KeyStore = this + }; + BeforeAccess?.Invoke(args); + + object value = null; + if ( _credentials.TryGetValue(key, out value)) + { + return (T) value; + } + + try + { + var fallBackKey = _credentials.Keys.First(x => x.BeEquivalent(key)); + return (T)_credentials[fallBackKey]; + } + catch (InvalidOperationException) + { + throw new ArgumentException($"{key.ToString()} is not stored in AzKeyStore yet."); + } + } + + public bool DeleteKey(IKeyStoreKey key) + { + var args = new KeyStoreNotificationArgs() + { + KeyStore = this + }; + BeforeAccess?.Invoke(args); + bool ret = false; + ret = _credentials.Remove(key); + OnUpdate?.Invoke(args); + return ret; + } + + public void Deserialize(byte[] data, bool shouldClearExistingCache) + { + lock(lockObj) + { + if (shouldClearExistingCache) + { + _credentials.Clear(); + } + if (data != null && data.Length > 0) + { + var rawJsonString = Encoding.UTF8.GetString(data); + var serializableKeyStore = JsonConvert.DeserializeObject(rawJsonString, typeof(List)) as List; + if (serializableKeyStore != null) + { + foreach (var item in serializableKeyStore) + { + IKeyStoreKey keyStoreKey = DeserializeItem(item.keyType, item.keyStoreKey) as IKeyStoreKey; + if (keyStoreKey == null) + { + throw new ArgumentException($"Cannot parse the keystore {item.keyStoreKey} with the type {item.keyType}."); + } + var keyStoreValue = DeserializeItem(item.valueType, item.keyStoreValue); + if (keyStoreValue == null) + { + throw new ArgumentException($"Cannot parse the keystore {item.keyStoreValue} with the type {item.valueType}."); + } + _credentials[keyStoreKey] = keyStoreValue; + } + } + } + } + } + + public byte[] Serialize() + { + IList serializableKeyStore = new List(); + foreach (var item in _credentials) + { + var keyType = _typeNameMap[item.Key.GetType()]; + var key = _elementConverterMap.ContainsKey(keyType) ? + JsonConvert.SerializeObject(item.Key, _elementConverterMap[keyType]) : JsonConvert.SerializeObject(item.Key); + if (!string.IsNullOrEmpty(key)) + { + var valueType = _typeNameMap[item.Value.GetType()]; + serializableKeyStore.Add(new KeyStoreElement() + { + keyType = keyType, + keyStoreKey = key, + valueType = valueType, + keyStoreValue = _elementConverterMap.ContainsKey(valueType) ? + JsonConvert.SerializeObject(item.Value, _elementConverterMap[valueType]) : JsonConvert.SerializeObject(item.Value), + }); + } + } + var JsonString = JsonConvert.SerializeObject(serializableKeyStore); + return Encoding.UTF8.GetBytes(JsonString); + } + + public void Clear() + { + var args = new KeyStoreNotificationArgs() + { + KeyStore = this + }; + BeforeAccess?.Invoke(args); + _credentials.Clear(); + OnUpdate?.Invoke(args); + + } + + private static object DeserializeItem(string typeName, string value) + { + Type t = null; + t = _typeNameMap.FirstOrDefault(item => item.Value == typeName).Key; + + if (t != null) + { + if (_elementConverterMap.ContainsKey(typeName)) + { + return JsonConvert.DeserializeObject(value, t, _elementConverterMap[typeName]); + } + else + { + return JsonConvert.DeserializeObject(value, t); + } + } + return null; + } + + public static void RegisterJsonConverter(Type type, string typeName, JsonConverter converter = null) + { + if (string.IsNullOrEmpty(typeName)) + { + throw new ArgumentNullException($"typeName cannot be empty."); + } + if (_typeNameMap.ContainsKey(type)) + { + if (string.Compare(_typeNameMap[type], typeName) != 0) + { + throw new ArgumentException($"{typeName} has conflict with {_typeNameMap[type]} with reference to {type}."); + } + } + else + { + _typeNameMap[type] = typeName; + } + if (converter != null) + { + _elementConverterMap[_typeNameMap[type]] = converter; + } + } + + public void SetBeforeAccess(KeyStoreCallbak beforeAccess) + { + BeforeAccess = beforeAccess; + } + + public void SetOnUpdate(KeyStoreCallbak onUpdate) + { + OnUpdate = onUpdate; + } + } +} diff --git a/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs b/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs new file mode 100644 index 000000000000..25cdcf22a50a --- /dev/null +++ b/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs @@ -0,0 +1,23 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + public class KeyStoreNotificationArgs + { + public IKeyStore KeyStore; + } + + public delegate void KeyStoreCallbak(KeyStoreNotificationArgs args); +} diff --git a/src/Accounts/Authentication/KeyStore/SecureStringConverter.cs b/src/Accounts/Authentication/KeyStore/SecureStringConverter.cs index c399dfacd424..fbbf3f60da9a 100644 --- a/src/Accounts/Authentication/KeyStore/SecureStringConverter.cs +++ b/src/Accounts/Authentication/KeyStore/SecureStringConverter.cs @@ -10,6 +10,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +// ---------------------------------------------------------------------------------- using Microsoft.WindowsAzure.Commands.Common; using Newtonsoft.Json; using System; diff --git a/src/Accounts/Authentication/KeyStore/ServicePrincipalKey.cs b/src/Accounts/Authentication/KeyStore/ServicePrincipalKey.cs index 88e8b5dfc1f7..327ce6acfb8c 100644 --- a/src/Accounts/Authentication/KeyStore/ServicePrincipalKey.cs +++ b/src/Accounts/Authentication/KeyStore/ServicePrincipalKey.cs @@ -54,5 +54,14 @@ public override bool Equals(object obj) } return false; } + + public override bool BeEquivalent(object obj) + { + if (obj is ServicePrincipalKey other) + { + return this.name == other.name && this.appId == other.appId; + } + return false; + } } } diff --git a/src/Accounts/Authentication/KeyStore/StorageHelper.cs b/src/Accounts/Authentication/KeyStore/StorageHelper.cs new file mode 100644 index 000000000000..9b97101744a9 --- /dev/null +++ b/src/Accounts/Authentication/KeyStore/StorageHelper.cs @@ -0,0 +1,282 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- +using Microsoft.Azure.PowerShell.Authenticators.Identity; +using Microsoft.Identity.Client.Extensions.Msal; +using Microsoft.IdentityModel.Abstractions; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading.Tasks; + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + public class StorageHelper : IStorageHelper + { + private const string KeyChainServiceName = "Microsoft.Azure.PowerShell"; + + private static readonly Lazy s_staticLogger = new Lazy(() => + { + return new TraceSourceLogger(new TraceSource(nameof(StorageHelper))); + }); + + private readonly StorageCreationProperties _storageCreationProperties; + + internal IStorage PersistanceStore { get; } + + private readonly TraceSourceLogger _logger; + + private bool _protected; + public bool IsProtected + { + get => _protected; + private set => _protected = value; + } + + private static AsyncLockWithValue cacheHelperLock = new AsyncLockWithValue(); + + internal StorageHelper(StorageCreationProperties storageProperties, bool isProtected, IStorage store = null) + { + _logger = s_staticLogger.Value; + _storageCreationProperties = storageProperties; + PersistanceStore = store ?? new StorageWrapper() + { + StorageCreationProperties = _storageCreationProperties, + LoggerSource = _logger.Source + }; + PersistanceStore.Create(); + _protected = isProtected; + } + + private static StorageHelper GetProtectedStorageHelper(string fileName, string directory, IStorage storage = null) + { + var storageProperties = new StorageCreationPropertiesBuilder(fileName, directory) + .WithMacKeyChain(KeyChainServiceName + ".other_secrets", fileName) + .WithLinuxKeyring(fileName, "default", "AzKeyStoreCache", + new KeyValuePair("AzureClientID", "Microsoft.Developer.Azure.PowerShell"), + new KeyValuePair("Microsoft.Developer.Azure.PowerShell", "1.0.0.0")).Build(); + return StorageHelper.Create(storageProperties, true, storage); + } + + private static StorageHelper GetFallbackStorageHelper(string fileName, string directory, IStorage storage = null) + { + var storageProperties = new StorageCreationPropertiesBuilder(fileName, directory) + .WithUnprotectedFile().Build(); + return StorageHelper.Create(storageProperties, false, storage); + } + + private static StorageHelper Create(StorageCreationProperties storageCreationProperties, bool isProtected, IStorage storage = null) + { + if (storageCreationProperties is null) + { + throw new ArgumentNullException(nameof(storageCreationProperties)); + } + + using (CreateCrossPlatLock(storageCreationProperties)) + { + return new StorageHelper(storageCreationProperties, isProtected, storage); + } + } + + #region Public API + public static async Task GetStorageHelperAsync(bool async, string fileName, string directory, IKeyStore keystore, IStorage storage = null) + { + StorageHelper storageHelper = null; + + using (var asyncLock = await cacheHelperLock.GetLockOrValueAsync(async).ConfigureAwait(false)) + { + if (asyncLock.HasValue) + { + return asyncLock.Value; + } + + try + { + storageHelper = GetProtectedStorageHelper(fileName, directory, storage); + storageHelper.VerifyPersistence(); + } + catch (Exception) + { + storageHelper = GetFallbackStorageHelper(fileName, directory, storage); + storageHelper.VerifyPersistence(); + } + storageHelper.RegisterCache(keystore); + storageHelper.LoadFromCachedStorage(keystore); + asyncLock.SetValue(storageHelper); + } + return storageHelper; + } + + public static bool TryClearLockedStorageHelper() + { + return cacheHelperLock.TryClearValue(); + } + + public void Clear() + { + using (CreateCrossPlatLock(_storageCreationProperties)) + { + PersistanceStore.Clear(); + } + } + + public byte[] LoadUnencryptedTokenCache() + { + using (CreateCrossPlatLock(_storageCreationProperties)) + { + return PersistanceStore.ReadData(); + } + } + + public void SaveUnencryptedTokenCache(byte[] tokenCache) + { + using (CreateCrossPlatLock(_storageCreationProperties)) + { + PersistanceStore.WriteData(tokenCache); + } + } + + public void RegisterCache(IKeyStore keystore) + { + if (keystore == null) + { + throw new ArgumentNullException(nameof(keystore)); + } + + _logger.LogInformation($"Registering token cache with on disk storage"); + + keystore.SetOnUpdate(WriteToCachedStorage); + + _logger.LogInformation($"Done initializing"); + } + + public void UnregisterCache(IKeyStore keystore) + { + if (keystore == null) + { + throw new ArgumentNullException(nameof(keystore)); + } + keystore.SetOnUpdate(null); + } + + public void LoadFromCachedStorage(IKeyStore keystore) + { + LogMessage(EventLogLevel.Verbose, $"Before access\nAcquiring lock for token cache"); + + using (CreateCrossPlatLock(_storageCreationProperties)) + { + LogMessage(EventLogLevel.Verbose, $"Before access, the store has changed"); + + byte[] cachedStoreData = null; + try + { + cachedStoreData = PersistanceStore.ReadData(); + } + catch (Exception ex) + { + LogMessage(EventLogLevel.Error, $"Could not read the token cache. Ignoring. Exception: {ex}"); + return; + + } + LogMessage(EventLogLevel.Verbose, $"Read '{cachedStoreData?.Length}' bytes from storage"); + + try + { + LogMessage(EventLogLevel.Verbose, $"Deserializing the store"); + keystore.Deserialize(cachedStoreData, false); + } + catch (Exception e) + { + LogMessage(EventLogLevel.Error, $"An exception was encountered while deserializing the {nameof(StorageHelper)} : {e}"); + LogMessage(EventLogLevel.Error, $"No data found in the store, clearing the cache in memory."); + + PersistanceStore.Clear(); + throw; + } + } + } + + public void WriteToCachedStorage(KeyStoreNotificationArgs args) + { + using (CreateCrossPlatLock(_storageCreationProperties)) + { + LogMessage(EventLogLevel.Verbose, $"After access"); + byte[] data = null; + LogMessage(EventLogLevel.Verbose, $"After access, cache in memory HasChanged"); + try + { + data = args.KeyStore.Serialize(); + } + catch (Exception e) + { + LogMessage(EventLogLevel.Error, $"An exception was encountered while serializing the {nameof(StorageHelper)} : {e}"); + LogMessage(EventLogLevel.Error, $"No data found in the store, clearing the cache in memory."); + + PersistanceStore.Clear(); + throw; + } + + if (data != null) + { + LogMessage(EventLogLevel.Verbose, $"Serializing '{data.Length}' bytes"); + + try + { + PersistanceStore.WriteData(data); + } + catch (Exception) + { + LogMessage(EventLogLevel.Error, $"Could not write the keystore. Ignoring. See previous error message."); + } + } + } + } + #endregion + + private static CrossPlatLock CreateCrossPlatLock(StorageCreationProperties storageCreationProperties) + { + return new CrossPlatLock( + storageCreationProperties.CacheFilePath + ".lockfile", + storageCreationProperties.LockRetryDelay, + storageCreationProperties.LockRetryCount); + } + + public void VerifyPersistence() + { + PersistanceStore.VerifyPersistence(); + } + + private void LogMessage(EventLogLevel level, string message) + { + LogMessage(level, message, _logger); + } + + private static void LogMessage(EventLogLevel level, string message, TraceSourceLogger traceSourceLogger) + { + message = $"[{KeyChainServiceName}] {message}"; + + switch (level) + { + case EventLogLevel.Warning: + traceSourceLogger.LogWarning(message); + break; + case EventLogLevel.Error: + traceSourceLogger.LogError(message); + break; + case EventLogLevel.Verbose: + traceSourceLogger.LogInformation(message); + break; + } + } + } +} \ No newline at end of file diff --git a/src/Accounts/Authentication/KeyStore/StorageWrapper.cs b/src/Accounts/Authentication/KeyStore/StorageWrapper.cs index f5c01f42e3ce..aaa15a7600d2 100644 --- a/src/Accounts/Authentication/KeyStore/StorageWrapper.cs +++ b/src/Accounts/Authentication/KeyStore/StorageWrapper.cs @@ -11,34 +11,19 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- -using Microsoft.Azure.Commands.Common.Authentication.Properties; using Microsoft.Identity.Client.Extensions.Msal; -using System; -using System.Collections.Generic; -using System.Threading; +using System.Diagnostics; namespace Microsoft.Azure.Commands.ResourceManager.Common { class StorageWrapper : IStorage - { - private const string KeyChainServiceName = "Microsoft.Azure.PowerShell"; + { + public StorageCreationProperties StorageCreationProperties { get; set; } - public string FileName { get; set; } - public string Directory { get; set; } - - private Exception _lastError; + public TraceSource LoggerSource { get; set; } private Storage _storage = null; - private bool _protected; - public bool IsProtected - { - get => _protected; - private set => _protected = value; - } - - static ReaderWriterLockSlim storageLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion); - public StorageWrapper() { @@ -46,104 +31,28 @@ public StorageWrapper() public IStorage Create() { - StorageCreationPropertiesBuilder storageProperties = null; - if (!storageLock.TryEnterWriteLock(TimeSpan.Zero)) - { - throw new InvalidOperationException(Resources.StorageLockConflicts); - } - try - { - storageProperties = new StorageCreationPropertiesBuilder(FileName, Directory) - .WithMacKeyChain(KeyChainServiceName + ".other_secrets", FileName) - .WithLinuxKeyring(FileName, "default", "AzKeyStoreCache", - new KeyValuePair("AzureClientID", "Microsoft.Developer.Azure.PowerShell"), - new KeyValuePair("Microsoft.Developer.Azure.PowerShell", "1.0.0.0")); - _storage = Storage.Create(storageProperties.Build()); - VerifyPersistence(); - _protected = true; - } - catch (Exception e) - { - _lastError = e; - storageProperties = new StorageCreationPropertiesBuilder(FileName, Directory).WithUnprotectedFile(); - _storage = Storage.Create(storageProperties.Build()); - _protected = false; - } - finally - { - storageLock.ExitWriteLock(); - } + _storage = Storage.Create(StorageCreationProperties, LoggerSource); return this; } public void Clear() { - if (!storageLock.TryEnterWriteLock(TimeSpan.Zero)) - { - throw new InvalidOperationException(Resources.StorageLockConflicts); - } - try - { - _storage.Clear(); - } - finally - { - storageLock.ExitWriteLock(); - } + _storage.Clear(ignoreExceptions: true); } public byte[] ReadData() { - if (!storageLock.TryEnterReadLock(TimeSpan.Zero)) - { - throw new InvalidOperationException(Resources.StorageLockConflicts); - } - try - { - return _storage.ReadData(); - } - finally - { - storageLock.ExitReadLock(); - } + return _storage.ReadData(); } public void VerifyPersistence() { - if (!storageLock.TryEnterWriteLock(TimeSpan.Zero)) - { - throw new InvalidOperationException(Resources.StorageLockConflicts); - } - try - { - _storage.VerifyPersistence(); - } - finally - { - storageLock.ExitWriteLock(); - } + _storage.VerifyPersistence(); } public void WriteData(byte[] data) { - if (!storageLock.TryEnterWriteLock(TimeSpan.Zero)) - { - throw new InvalidOperationException(Resources.StorageLockConflicts); - } - - try - { - _storage.WriteData(data); - } - finally - { - storageLock.ExitWriteLock(); - } - } - - public Exception GetLastError() - { - return _lastError; + _storage.WriteData(data); } } -} +} \ No newline at end of file From 69bf8f071fcefa6e57629f2c016a5213de56c4ad Mon Sep 17 00:00:00 2001 From: Lei Jin Date: Tue, 31 Jan 2023 10:35:53 +0800 Subject: [PATCH 2/5] Add parallel test case of Az.Accounts to smoke test --- .azure-pipelines/util/smoke-test-steps.yml | 7 +++++++ tools/Test/SmokeTest/InstallAzModules.ps1 | 6 ++++++ tools/Test/SmokeTest/RmCoreSmokeTests.ps1 | 24 +++++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/.azure-pipelines/util/smoke-test-steps.yml b/.azure-pipelines/util/smoke-test-steps.yml index a3d3f007ec1d..8575cc76749d 100644 --- a/.azure-pipelines/util/smoke-test-steps.yml +++ b/.azure-pipelines/util/smoke-test-steps.yml @@ -78,6 +78,13 @@ jobs: Write-Host "List artifacts..." Get-ChildItem "$(Pipeline.Workspace)\\LocalRepo\\" + - task: NuGetCommand@2 + condition: and(succeeded(), eq('${{ parameters.psVersion }}', '5.1.14')) + displayName: 'Download ThreadJob .nupkg File for PowerShell 5.1.14' + inputs: + command: custom + arguments: 'install ThreadJob -directdownload -packagesavemode nupkg -source https://www.powershellgallery.com/api/v2 -OutputDirectory packages' + - task: NuGetCommand@2 condition: and(succeeded(), eq(variables['GalleryName'], 'LocalRepo')) displayName: 'Download Previous Az .nupkg Files' diff --git a/tools/Test/SmokeTest/InstallAzModules.ps1 b/tools/Test/SmokeTest/InstallAzModules.ps1 index f07f244ea514..ebacf1d52484 100644 --- a/tools/Test/SmokeTest/InstallAzModules.ps1 +++ b/tools/Test/SmokeTest/InstallAzModules.ps1 @@ -42,6 +42,12 @@ Register-Gallery $gallery $localRepoLocation Write-Host "Installing Az..." Install-Module -Name Az -Repository $gallery -Scope CurrentUser -AllowClobber -Force + +$file = Get-ChildItem $localRepoLocation | Where-Object {$_.Name -like "ThreadJob*"} +if ($file -ne $null) { + Write-Host "Install ThreadJob..." + Install-Module -Name ThreadJob -Repository $gallery -Scope CurrentUser -AllowClobber -Force +} # Check version Import-Module -MinimumVersion '2.6.0' -Name 'Az' -Force diff --git a/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 b/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 index 630708214406..b7b231f5139b 100644 --- a/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 +++ b/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 @@ -164,7 +164,7 @@ $resourceTestCommands = @( $generalCommands = @( @{ - Name = "Import Az.Accounts in Parallel"; + Name = "Import Az.Accounts in Parallel Job"; Command = { if ($null -ne $env:SYSTEM_DEFINITIONID -or $null -ne $env:Release_DefinitionId -or $null -ne $env:AZUREPS_HOST_ENVIRONMENT) { Write-Warning "Skipping because 'Start-Job' is not supported by design in scenarios where PowerShell is being hosted in other applications." @@ -184,6 +184,28 @@ $generalCommands = @( }; Retry = 0; # no need to retry } + @{ + Name = "Import Az.Accounts in Parallel Thread Job"; + + Command = { + #if ($null -ne $env:SYSTEM_DEFINITIONID -or $null -ne $env:Release_DefinitionId) { + #Write-Warning "Skipping because 'Start-ThreadJob' is not supported by design in scenarios where PowerShell is being hosted in other applications." + #return + #} + $importJobs = @() + 1..50 | ForEach-Object { + $importJobs += Start-ThreadJob -name "import-no.$_" -ScriptBlock { Import-Module Az.Accounts; Get-AzTenant; } + } + $importJobs | Wait-Job + $importJobs | Receive-Job + $importJobs | ForEach-Object { + if ("Completed" -ne $_.State) { + throw "Some jobs have failed." + } + } + }; + Retry = 0; # no need to retry + } ) if($Reverse.IsPresent){ From 867aae8a6d0054f8b01835896a80b0d29455d099 Mon Sep 17 00:00:00 2001 From: Jin Lei <54836179+msJinLei@users.noreply.github.com> Date: Thu, 2 Feb 2023 16:23:37 +0800 Subject: [PATCH 3/5] Address review comments Co-authored-by: Yeming Liu <11371776+isra-fel@users.noreply.github.com> Address review comments --- src/Accounts/Accounts.Test/AutosaveTests.cs | 3 +- .../Accounts.Test/ContextCmdletTests.cs | 2 +- .../Accounts.Test/ProfileCmdletTests.cs | 2 +- .../Accounts/Account/ConnectAzureRmAccount.cs | 6 +- .../AutoSave/DisableAzureRmContextAutosave.cs | 5 + .../AutoSave/EnableAzureRmContextAutosave.cs | 5 + src/Accounts/Accounts/ChangeLog.md | 2 - .../Accounts/Context/ImportAzureRMContext.cs | 4 +- .../Accounts/Context/SetAzureRMContext.cs | 4 +- .../AzureRmProfile.cs | 4 +- .../Authentication.Test/AzKeyStorageTest.cs | 26 +-- .../Authentication.Test/StorageHelperTest.cs | 10 +- .../Factories/AuthenticationFactory.cs | 8 +- .../Identity/AsyncLockWithValue.cs | 18 -- .../KeyStore/AsyncLockWithValue.cs | 212 ++++++++++++++++++ .../Authentication/KeyStore/AzKeyStore.cs | 58 +++-- .../KeyStore/{IKeyStore.cs => IKeyCache.cs} | 2 +- .../Authentication/KeyStore/IKeyStoreKey.cs | 20 +- .../Authentication/KeyStore/IStorage.cs | 1 - .../Authentication/KeyStore/IStorageHelper.cs | 9 +- ...nMemoryKeyStore.cs => InMemoryKeyCache.cs} | 12 +- .../KeyStore/KeyStoreNotificationArgs.cs | 2 +- .../Authentication/KeyStore/StorageHelper.cs | 59 +---- .../Authentication/KeyStore/StorageWrapper.cs | 2 +- tools/Test/SmokeTest/InstallAzModules.ps1 | 3 +- tools/Test/SmokeTest/RmCoreSmokeTests.ps1 | 8 +- 26 files changed, 336 insertions(+), 151 deletions(-) create mode 100644 src/Accounts/Authentication/KeyStore/AsyncLockWithValue.cs rename src/Accounts/Authentication/KeyStore/{IKeyStore.cs => IKeyCache.cs} (97%) rename src/Accounts/Authentication/KeyStore/{InMemoryKeyStore.cs => InMemoryKeyCache.cs} (97%) diff --git a/src/Accounts/Accounts.Test/AutosaveTests.cs b/src/Accounts/Accounts.Test/AutosaveTests.cs index 50ea60b43fe5..0986b9808436 100644 --- a/src/Accounts/Accounts.Test/AutosaveTests.cs +++ b/src/Accounts/Accounts.Test/AutosaveTests.cs @@ -41,7 +41,6 @@ public AutosaveTests(ITestOutputHelper output) XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output)); commandRuntimeMock = new MockCommandRuntime(); dataStore = new MemoryDataStore(); - ResetState(); keyStore = SetMockedAzKeyStore(); } @@ -51,7 +50,7 @@ private AzKeyStore SetMockedAzKeyStore() storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object); storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]); storageMocker.Setup(f => f.WriteData(It.IsAny())).Callback((byte[] s) => {}); - var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object); + var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", true, storageMocker.Object); return keyStore; } diff --git a/src/Accounts/Accounts.Test/ContextCmdletTests.cs b/src/Accounts/Accounts.Test/ContextCmdletTests.cs index 7b977a5a7482..9e4d44bb4cb5 100644 --- a/src/Accounts/Accounts.Test/ContextCmdletTests.cs +++ b/src/Accounts/Accounts.Test/ContextCmdletTests.cs @@ -64,7 +64,7 @@ public ContextCmdletTests(ITestOutputHelper output) Mock storageMocker = new Mock(); AzKeyStore azKeyStore = null; string profilePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName); - azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, storageMocker.Object); + azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, true, storageMocker.Object); AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => azKeyStore, true); } diff --git a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs index 386998a78ffd..fdd16bf07357 100644 --- a/src/Accounts/Accounts.Test/ProfileCmdletTests.cs +++ b/src/Accounts/Accounts.Test/ProfileCmdletTests.cs @@ -55,7 +55,7 @@ private AzKeyStore SetMockedAzKeyStore() storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object); storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]); storageMocker.Setup(f => f.WriteData(It.IsAny())).Callback((byte[] s) => { }); - var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object); + var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, storageMocker.Object); return keyStore; } diff --git a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs index 218fc28abe8f..2e8208f0e560 100644 --- a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs +++ b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs @@ -425,7 +425,7 @@ public override void ExecuteCmdlet() azureAccount.SetProperty(AzureAccount.Property.CertificatePath, resolvedPath); if (CertificatePassword != null) { - keyStore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword); + keyStore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword); if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected) { WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory)); @@ -451,7 +451,7 @@ public override void ExecuteCmdlet() if (azureAccount.Type == AzureAccount.AccountType.ServicePrincipal && password != null) { - keyStore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret + keyStore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret ,azureAccount.Id, Tenant), password); if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected) { @@ -713,7 +713,7 @@ public void OnImport() } AzKeyStore keyStore = null; - keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile); + keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile, autoSaveEnabled); AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore); if (!InitializeProfileProvider(autoSaveEnabled)) diff --git a/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs b/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs index 1d69aba78ce5..a2b9be38aa7a 100644 --- a/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs +++ b/src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs @@ -92,6 +92,11 @@ void DisableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextA builder.Reset(); } + if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore)) + { + keystore.DisableSyncToStorage(); + } + if (writeAutoSaveFile) { FileUtilities.EnsureDirectoryExists(session.ProfileDirectory); diff --git a/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs b/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs index 66325469ea71..830876d6463b 100644 --- a/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs +++ b/src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs @@ -102,6 +102,11 @@ void EnableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextAu AzureSession.Instance.RegisterComponent(PowerShellTokenCacheProvider.PowerShellTokenCacheProviderKey, () => newCacheProvider, true); } + if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore)) + { + keystore.EnableSyncToStorage(); + } + if (writeAutoSaveFile) { try diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md index ebd8bc1d7230..0de24d71e9d9 100644 --- a/src/Accounts/Accounts/ChangeLog.md +++ b/src/Accounts/Accounts/ChangeLog.md @@ -22,8 +22,6 @@ * Supported Web Account Manager on ARM64-based Windows systems. Fixed an issue where `Connect-AzAccount` failed with error "Unable to load DLL 'msalruntime_arm64'". [#20700] * Enabled credential to be found only by applicationId while tenant was not matched when accquire token. [#20484] * When Az.Accounts ran in parallel, the waiters were allowed to wait infinitely to avoid throw exception in automation enviroment. [#20455] -* Used Lazy load for AzKeyStore. -* Used update on change mechanism for AzKeyStore and remove `Flush` interface. ## Version 2.11.1 * Fixed an issue where Az.Accounts cannot be imported correctly. [#20615] diff --git a/src/Accounts/Accounts/Context/ImportAzureRMContext.cs b/src/Accounts/Accounts/Context/ImportAzureRMContext.cs index 9304d88d13ba..f75d2e813fc5 100644 --- a/src/Accounts/Accounts/Context/ImportAzureRMContext.cs +++ b/src/Accounts/Accounts/Context/ImportAzureRMContext.cs @@ -78,13 +78,13 @@ void CopyProfile(AzureRmProfile source, IProfileOperations target) var secret = account.GetProperty(AzureAccount.Property.ServicePrincipalSecret); if (!string.IsNullOrEmpty(secret)) { - keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, context.Value.Tenant?.Id) + keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, context.Value.Tenant?.Id) , secret.ConvertToSecureString()); } var password = account.GetProperty(AzureAccount.Property.CertificatePassword); if (!string.IsNullOrEmpty(password)) { - keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, context.Value.Tenant?.Id) + keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, context.Value.Tenant?.Id) ,password.ConvertToSecureString()); } } diff --git a/src/Accounts/Accounts/Context/SetAzureRMContext.cs b/src/Accounts/Accounts/Context/SetAzureRMContext.cs index fab411685fea..9c0badcf8470 100644 --- a/src/Accounts/Accounts/Context/SetAzureRMContext.cs +++ b/src/Accounts/Accounts/Context/SetAzureRMContext.cs @@ -97,13 +97,13 @@ public override void ExecuteCmdlet() var secret = account.GetProperty(AzureAccount.Property.ServicePrincipalSecret); if (!string.IsNullOrEmpty(secret)) { - keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, Context.Tenant?.Id) + keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, Context.Tenant?.Id) , secret.ConvertToSecureString()); } var password = account.GetProperty(AzureAccount.Property.CertificatePassword); if (!string.IsNullOrEmpty(password)) { - keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, Context.Tenant?.Id) + keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, Context.Tenant?.Id) , password.ConvertToSecureString()); } } diff --git a/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs b/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs index 9c81702be477..a1a53393c7b2 100644 --- a/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs +++ b/src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs @@ -225,13 +225,13 @@ private IAzureContext MigrateSecretToKeyStore(IAzureContext context, AzKeyStore var account = context.Account; if (account.IsPropertySet(AzureAccount.Property.ServicePrincipalSecret)) { - keystore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First()) + keystore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First()) , account.ExtendedProperties.GetProperty(AzureAccount.Property.ServicePrincipalSecret).ConvertToSecureString()); account.ExtendedProperties.Remove(AzureAccount.Property.ServicePrincipalSecret); } if (account.IsPropertySet(AzureAccount.Property.CertificatePassword)) { - keystore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First()) + keystore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First()) , account.ExtendedProperties.GetProperty(AzureAccount.Property.CertificatePassword).ConvertToSecureString()); account.ExtendedProperties.Remove(AzureAccount.Property.CertificatePassword); } diff --git a/src/Accounts/Authentication.Test/AzKeyStorageTest.cs b/src/Accounts/Authentication.Test/AzKeyStorageTest.cs index 3e6165a547b3..ed4fb6b7f38b 100644 --- a/src/Accounts/Authentication.Test/AzKeyStorageTest.cs +++ b/src/Accounts/Authentication.Test/AzKeyStorageTest.cs @@ -56,15 +56,15 @@ private static bool CompareJsonObjects(string expected, string acutal) [Trait(Category.AcceptanceType, Category.CheckIn)] public void SaveKey() { - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, true, storageMocker.Object)) { IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); var secret = "secret".ConvertToSecureString(); - store.SaveCredential(servicePrincipalKey, secret); + store.SaveSecureString(servicePrincipalKey, secret); IKeyStoreKey certificatePassword = new ServicePrincipalKey("CertificatePassword", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); var passowrd = "password".ConvertToSecureString(); - store.SaveCredential(certificatePassword, passowrd); + store.SaveSecureString(certificatePassword, passowrd); var result = Encoding.UTF8.GetString(storageChecker.ToArray()); const string EXPECTEDSTRING = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""CertificatePassword\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""password\""""},{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; @@ -81,12 +81,12 @@ public void FindKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, true, storageMocker.Object)) { storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - var secret = store.GetCredential(servicePrincipalKey); + var secret = store.GetSecureString(servicePrincipalKey); Assert.Equal("secret", secret.ConvertToString()); store.Clear(); @@ -100,12 +100,12 @@ public void FindFallbackKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secretFallback\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, true, storageMocker.Object)) { storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-0000-bad9-b7b93a3e9c5a"); - var secret = store.GetCredential(servicePrincipalKey); + var secret = store.GetSecureString(servicePrincipalKey); Assert.Equal("secretFallback", secret.ConvertToString()); store.Clear(); @@ -120,12 +120,12 @@ public void FindNoKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, true, storageMocker.Object)) { storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("CertificatePassword", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - Assert.Throws(() => store.GetCredential(servicePrincipalKey)); + Assert.Throws(() => store.GetSecureString(servicePrincipalKey)); store.Clear(); } @@ -138,12 +138,12 @@ public void RemoveKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, true, storageMocker.Object)) { storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("ServicePrincipalSecret", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - store.RemoveCredential(servicePrincipalKey); + store.RemoveSecureString(servicePrincipalKey); var result = Encoding.UTF8.GetString(storageChecker.ToArray()); var objects = JsonConvert.DeserializeObject>(result); @@ -160,12 +160,12 @@ public void RemoveNoKey() { const string EXPECTED = @"[{""keyType"":""ServicePrincipalKey"",""keyStoreKey"":""{\""appId\"":\""6c984d31-5b4f-4734-b548-e230a248e347\"",\""tenantId\"":\""54826b22-38d6-4fb2-bad9-b7b93a3e9c5a\"",\""name\"":\""ServicePrincipalSecret\""}"",""valueType"":""SecureString"",""keyStoreValue"":""\""secret\""""}]"; storageChecker.AddRange(Encoding.UTF8.GetBytes(EXPECTED)); - using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, storageMocker.Object)) + using (var store = new AzKeyStore(dummpyPath, keyStoreFileName, true, storageMocker.Object)) { storageMocker.Setup(f => f.ReadData()).Returns(storageChecker.ToArray()); IKeyStoreKey servicePrincipalKey = new ServicePrincipalKey("CertificatePassword", "6c984d31-5b4f-4734-b548-e230a248e347", "54826b22-38d6-4fb2-bad9-b7b93a3e9c5a"); - store.RemoveCredential(servicePrincipalKey); + store.RemoveSecureString(servicePrincipalKey); var result = Encoding.UTF8.GetString(storageChecker.ToArray()); var objects = JsonConvert.DeserializeObject>(result); diff --git a/src/Accounts/Authentication.Test/StorageHelperTest.cs b/src/Accounts/Authentication.Test/StorageHelperTest.cs index a261cae27470..7cde8dd554e6 100644 --- a/src/Accounts/Authentication.Test/StorageHelperTest.cs +++ b/src/Accounts/Authentication.Test/StorageHelperTest.cs @@ -27,7 +27,7 @@ namespace Common.Authenticators.Test public class StorageHelperTest { private Mock storageMocker = null; - private Mock keystoreMocker = null; + private Mock keystoreMocker = null; private string profilePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName); private string keyStoreFileName = "azkeystore"; @@ -35,7 +35,7 @@ public StorageHelperTest() { storageMocker = new Mock(); storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object); - keystoreMocker = new Mock(); + keystoreMocker = new Mock(); } [Fact] @@ -72,11 +72,7 @@ public void SaveToStorageTest() var helper = StorageHelper.GetStorageHelperAsync(true, keyStoreFileName, profilePath , keystoreMocker.Object, storageMocker.Object).GetAwaiter().GetResult(); - var args = new KeyStoreNotificationArgs() - { - KeyStore = keystoreMocker.Object - }; - helper.WriteToCachedStorage(args); + helper.WriteToCachedStorage(keystoreMocker.Object); string actual = Encoding.UTF8.GetString(storageChecker.ToArray()); Assert.Equal(EXPECTED, actual); diff --git a/src/Accounts/Authentication/Factories/AuthenticationFactory.cs b/src/Accounts/Authentication/Factories/AuthenticationFactory.cs index 830d0520d6b4..34772928a5c3 100644 --- a/src/Accounts/Authentication/Factories/AuthenticationFactory.cs +++ b/src/Accounts/Authentication/Factories/AuthenticationFactory.cs @@ -433,9 +433,9 @@ public void RemoveUser(IAzureAccount account, IAzureTokenCache tokenCache) case AzureAccount.AccountType.ServicePrincipal: try { - KeyStore.RemoveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, + KeyStore.RemoveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().FirstOrDefault())); - KeyStore.RemoveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, + KeyStore.RemoveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().FirstOrDefault())); } catch @@ -577,7 +577,7 @@ private AuthenticationParameters GetAuthenticationParameters( { try { - password = KeyStore.GetCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret + password = KeyStore.GetSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret , account.Id, tenant)); } catch @@ -591,7 +591,7 @@ private AuthenticationParameters GetAuthenticationParameters( { try { - certificatePassword = KeyStore.GetCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword + certificatePassword = KeyStore.GetSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword , account.Id, tenant)); } catch diff --git a/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs b/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs index 6ac99a950115..e89ca5eb2394 100644 --- a/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs +++ b/src/Accounts/Authentication/Identity/AsyncLockWithValue.cs @@ -126,24 +126,6 @@ private void SetValue(T value) } } - /// - /// Try to reset value and fail if value is locked. - /// - /// - public bool TryClearValue() - { - lock (_syncObj) - { - if (!_isLocked) - { - _value = default(T); - _hasValue = false; - return true; - } - } - return false; - } - /// /// Release the lock and allow next waiter acquire it /// diff --git a/src/Accounts/Authentication/KeyStore/AsyncLockWithValue.cs b/src/Accounts/Authentication/KeyStore/AsyncLockWithValue.cs new file mode 100644 index 000000000000..4e5d3d6b93c7 --- /dev/null +++ b/src/Accounts/Authentication/KeyStore/AsyncLockWithValue.cs @@ -0,0 +1,212 @@ +// ---------------------------------------------------------------------------------- +// +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- +// +using Microsoft.Azure.PowerShell.Authenticators.Identity; +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Azure.Commands.ResourceManager.Common +{ + /// + /// Primitive that combines async lock and value cache + /// + /// + internal sealed class AsyncLockWithValue + { + private readonly object _syncObj = new object(); + private Queue> _waiters; + private bool _isLocked; + private bool _hasValue; + private T _value; + + /// + /// Method that either returns cached value or acquire a lock. + /// If one caller has acquired a lock, other callers will be waiting for the lock to be released. + /// If value is set, lock is released and all waiters get that value. + /// If value isn't set, the next waiter in the queue will get the lock. + /// + /// + /// + /// + public async ValueTask GetLockOrValueAsync(bool async, CancellationToken cancellationToken = default) + { + TaskCompletionSource valueTcs; + lock (_syncObj) + { + // If there is a value, just return it + if (_hasValue) + { + return new Lock(_value); + } + + // If lock isn't acquire yet, acquire it and return to the caller + if (!_isLocked) + { + _isLocked = true; + return new Lock(this); + } + + // Check cancellationToken before instantiating waiter + cancellationToken.ThrowIfCancellationRequested(); + + // If lock is already taken, create a waiter and wait either until value is set or lock can be acquired by this waiter + if(_waiters is null) + { + _waiters = new Queue>(); + } + // if async == false, valueTcs will be waited only in this thread and only synchronously, so RunContinuationsAsynchronously isn't needed. + valueTcs = new TaskCompletionSource(async ? TaskCreationOptions.RunContinuationsAsynchronously : TaskCreationOptions.None); + _waiters.Enqueue(valueTcs); + } + + try + { + if (async) + { + return await valueTcs.Task.AwaitWithCancellation(cancellationToken); + } + +#pragma warning disable AZC0104 // Use EnsureCompleted() directly on asynchronous method return value. +#pragma warning disable AZC0111 // DO NOT use EnsureCompleted in possibly asynchronous scope. + valueTcs.Task.Wait(cancellationToken); + return valueTcs.Task.EnsureCompleted(); +#pragma warning restore AZC0111 // DO NOT use EnsureCompleted in possibly asynchronous scope. +#pragma warning restore AZC0104 // Use EnsureCompleted() directly on asynchronous method return value. + } + catch (OperationCanceledException) + { + // Throw OperationCanceledException only if another thread hasn't set a value to this waiter + // by calling either Reset or SetValue + if (valueTcs.TrySetCanceled(cancellationToken)) + { + throw; + } + + return valueTcs.Task.Result; + } + } + + /// + /// Set value to the cache and to all the waiters + /// + /// + private void SetValue(T value) + { + Queue> waiters; + lock (_syncObj) + { + _value = value; + _hasValue = true; + _isLocked = false; + if (_waiters == default) + { + return; + } + + waiters = _waiters; + _waiters = default; + } + + while (waiters.Count > 0) + { + waiters.Dequeue().TrySetResult(new Lock(value)); + } + } + + /// + /// Try to reset value and fail if value is locked. + /// + /// + public bool TryClearValue() + { + lock (_syncObj) + { + if (!_isLocked) + { + _value = default(T); + _hasValue = false; + return true; + } + } + return false; + } + + /// + /// Release the lock and allow next waiter acquire it + /// + private void Reset() + { + TaskCompletionSource nextWaiter = UnlockOrGetNextWaiter(); + while (nextWaiter != default && !nextWaiter.TrySetResult(new Lock(this))) + { + nextWaiter = UnlockOrGetNextWaiter(); + } + } + + private TaskCompletionSource UnlockOrGetNextWaiter() + { + lock (_syncObj) + { + if (!_isLocked) + { + return default; + } + + if (_waiters == default) + { + _isLocked = false; + return default; + } + + while (_waiters.Count > 0) + { + var nextWaiter = _waiters.Dequeue(); + if (!nextWaiter.Task.IsCompleted) + { + // Return the waiter only if it wasn't canceled already + return nextWaiter; + } + } + + _isLocked = false; + return default; + } + } + + public readonly struct Lock : IDisposable + { + private readonly AsyncLockWithValue _owner; + public bool HasValue => _owner == default; + public T Value { get; } + + public Lock(T value) + { + _owner = default; + Value = value; + } + + public Lock(AsyncLockWithValue owner) + { + _owner = owner; + Value = default; + } + + public void SetValue(T value) => _owner.SetValue(value); + + public void Dispose() => _owner?.Reset(); + } + } +} diff --git a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs index 7c610f20fd6d..ade907b83f9e 100644 --- a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs +++ b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs @@ -23,11 +23,11 @@ public class AzKeyStore : IDisposable public string FileName { get; set; } public string Directory { get; set; } - private IKeyStore _inMemoryStore = null; - public IKeyStore InMemoryStore + private IKeyCache _inMemoryKeyCache = null; + public IKeyCache InMemoryKeyCache { - get => _inMemoryStore; - set => _inMemoryStore = value; + get => _inMemoryKeyCache; + set => _inMemoryKeyCache = value; } private IStorage inputStorage = null; @@ -38,31 +38,43 @@ public bool IsProtected private set; } - public AzKeyStore(string directory, string fileName, IStorage storage = null) + public AzKeyStore(string directory, string fileName, bool enableContextAutoSaving = true, IStorage storage = null) { - InMemoryStore = new InMemoryKeyStore(); - InMemoryStore.SetBeforeAccess(LoadStorage); + InMemoryKeyCache = new InMemoryKeyCache(); + + if (enableContextAutoSaving) + { + InMemoryKeyCache.SetBeforeAccess(LoadStorage); + InMemoryKeyCache.SetOnUpdate(UpdateStorage); + } FileName = fileName; Directory = directory; inputStorage = storage; - InMemoryKeyStore.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); - InMemoryKeyStore.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); + Common.InMemoryKeyCache.RegisterJsonConverter(typeof(ServicePrincipalKey), typeof(ServicePrincipalKey).Name); + Common.InMemoryKeyCache.RegisterJsonConverter(typeof(SecureString), typeof(SecureString).Name, new SecureStringConverter()); } private void LoadStorage(KeyStoreNotificationArgs args) { - var asyncHelper = StorageHelper.GetStorageHelperAsync(true, FileName, Directory, args.KeyStore, inputStorage); + var asyncHelper = StorageHelper.GetStorageHelperAsync(true, FileName, Directory, args.KeyCache, inputStorage); var helper = asyncHelper.GetAwaiter().GetResult(); IsProtected = helper.IsProtected; } + private void UpdateStorage(KeyStoreNotificationArgs args) + { + var asyncHelper = StorageHelper.GetStorageHelperAsync(false, FileName, Directory, args.KeyCache, inputStorage); + var helper = asyncHelper.GetAwaiter().GetResult(); + helper.WriteToCachedStorage(args.KeyCache); + } + public void Clear() { - InMemoryStore.Clear(); + InMemoryKeyCache.Clear(); } public void Dispose() @@ -70,19 +82,31 @@ public void Dispose() StorageHelper.TryClearLockedStorageHelper(); } - public void SaveCredential(IKeyStoreKey key, SecureString value) + public void SaveSecureString(IKeyStoreKey key, SecureString value) + { + InMemoryKeyCache.SaveKey(key, value); + } + + public SecureString GetSecureString(IKeyStoreKey key) + { + return InMemoryKeyCache.GetKey(key); + } + + public bool RemoveSecureString(IKeyStoreKey key) { - InMemoryStore.SaveKey(key, value); + return InMemoryKeyCache.DeleteKey(key); } - public SecureString GetCredential(IKeyStoreKey key) + public void EnableSyncToStorage() { - return InMemoryStore.GetKey(key); + InMemoryKeyCache.SetBeforeAccess(LoadStorage); + InMemoryKeyCache.SetOnUpdate(UpdateStorage); } - public bool RemoveCredential(IKeyStoreKey key) + public void DisableSyncToStorage() { - return InMemoryStore.DeleteKey(key); + InMemoryKeyCache.SetBeforeAccess(null); + InMemoryKeyCache.SetOnUpdate(null); } } } diff --git a/src/Accounts/Authentication/KeyStore/IKeyStore.cs b/src/Accounts/Authentication/KeyStore/IKeyCache.cs similarity index 97% rename from src/Accounts/Authentication/KeyStore/IKeyStore.cs rename to src/Accounts/Authentication/KeyStore/IKeyCache.cs index e7c1f637bc4e..81d631b0dd35 100644 --- a/src/Accounts/Authentication/KeyStore/IKeyStore.cs +++ b/src/Accounts/Authentication/KeyStore/IKeyCache.cs @@ -14,7 +14,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common { - public interface IKeyStore + public interface IKeyCache { void SaveKey(IKeyStoreKey key, T value); diff --git a/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs b/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs index 4dbac64496d3..c126232d6193 100644 --- a/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs +++ b/src/Accounts/Authentication/KeyStore/IKeyStoreKey.cs @@ -16,14 +16,30 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common { public abstract class IKeyStoreKey { + /// + /// Create key from the data fields of KeyStoreKey. + /// protected abstract string CreateKey(); + /// + /// Convert key to string. + /// public override abstract string ToString(); - public override abstract bool Equals(object obj); - + /// + /// Generate hash code of KeyStoreKey. + /// public override abstract int GetHashCode(); + /// + /// Check whether the current key is exactly equal to another. + /// + public override abstract bool Equals(object obj); + + /// + /// Check whether the current key can be treated as equal to another even though they are not equal. + /// This method can be used as fuzzy search of the keys. + /// public abstract bool BeEquivalent(object obj); } } diff --git a/src/Accounts/Authentication/KeyStore/IStorage.cs b/src/Accounts/Authentication/KeyStore/IStorage.cs index ab8b6653973c..71b7055c6f06 100644 --- a/src/Accounts/Authentication/KeyStore/IStorage.cs +++ b/src/Accounts/Authentication/KeyStore/IStorage.cs @@ -11,7 +11,6 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- -using System; namespace Microsoft.Azure.Commands.ResourceManager.Common { diff --git a/src/Accounts/Authentication/KeyStore/IStorageHelper.cs b/src/Accounts/Authentication/KeyStore/IStorageHelper.cs index eaa4c1619726..49be99e02563 100644 --- a/src/Accounts/Authentication/KeyStore/IStorageHelper.cs +++ b/src/Accounts/Authentication/KeyStore/IStorageHelper.cs @@ -11,7 +11,6 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- -using System; namespace Microsoft.Azure.Commands.ResourceManager.Common { @@ -19,13 +18,9 @@ public interface IStorageHelper { void Clear(); - byte[] LoadUnencryptedTokenCache(); + void LoadFromCachedStorage(IKeyCache keycache); - void SaveUnencryptedTokenCache(byte[] tokenCache); - - void LoadFromCachedStorage(IKeyStore keystore); - - void WriteToCachedStorage(KeyStoreNotificationArgs args); + void WriteToCachedStorage(IKeyCache keycache); bool IsProtected { diff --git a/src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs b/src/Accounts/Authentication/KeyStore/InMemoryKeyCache.cs similarity index 97% rename from src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs rename to src/Accounts/Authentication/KeyStore/InMemoryKeyCache.cs index de4a3be78306..a2c8a78a5d73 100644 --- a/src/Accounts/Authentication/KeyStore/InMemoryKeyStore.cs +++ b/src/Accounts/Authentication/KeyStore/InMemoryKeyCache.cs @@ -11,18 +11,16 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- -using Microsoft.Identity.Client; using Newtonsoft.Json; using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Text; -using System.Threading.Tasks; namespace Microsoft.Azure.Commands.ResourceManager.Common { - internal class InMemoryKeyStore : IKeyStore + internal class InMemoryKeyCache : IKeyCache { internal class KeyStoreElement { @@ -48,7 +46,7 @@ public void SaveKey(IKeyStoreKey key, T value) { var args = new KeyStoreNotificationArgs() { - KeyStore = this + KeyCache = this }; BeforeAccess?.Invoke(args) ; if (!_typeNameMap.ContainsKey(key.GetType()) || !_typeNameMap.ContainsKey(value.GetType())) @@ -63,7 +61,7 @@ public T GetKey(IKeyStoreKey key) { var args = new KeyStoreNotificationArgs() { - KeyStore = this + KeyCache = this }; BeforeAccess?.Invoke(args); @@ -88,7 +86,7 @@ public bool DeleteKey(IKeyStoreKey key) { var args = new KeyStoreNotificationArgs() { - KeyStore = this + KeyCache = this }; BeforeAccess?.Invoke(args); bool ret = false; @@ -159,7 +157,7 @@ public void Clear() { var args = new KeyStoreNotificationArgs() { - KeyStore = this + KeyCache = this }; BeforeAccess?.Invoke(args); _credentials.Clear(); diff --git a/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs b/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs index 25cdcf22a50a..3c87ffe1ba44 100644 --- a/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs +++ b/src/Accounts/Authentication/KeyStore/KeyStoreNotificationArgs.cs @@ -16,7 +16,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common { public class KeyStoreNotificationArgs { - public IKeyStore KeyStore; + public IKeyCache KeyCache; } public delegate void KeyStoreCallbak(KeyStoreNotificationArgs args); diff --git a/src/Accounts/Authentication/KeyStore/StorageHelper.cs b/src/Accounts/Authentication/KeyStore/StorageHelper.cs index 9b97101744a9..85ed4675b180 100644 --- a/src/Accounts/Authentication/KeyStore/StorageHelper.cs +++ b/src/Accounts/Authentication/KeyStore/StorageHelper.cs @@ -11,7 +11,6 @@ // See the License for the specific language governing permissions and // limitations under the License. // ---------------------------------------------------------------------------------- -using Microsoft.Azure.PowerShell.Authenticators.Identity; using Microsoft.Identity.Client.Extensions.Msal; using Microsoft.IdentityModel.Abstractions; using System; @@ -21,7 +20,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common { - public class StorageHelper : IStorageHelper + internal class StorageHelper : IStorageHelper { private const string KeyChainServiceName = "Microsoft.Azure.PowerShell"; @@ -89,7 +88,7 @@ private static StorageHelper Create(StorageCreationProperties storageCreationPro } #region Public API - public static async Task GetStorageHelperAsync(bool async, string fileName, string directory, IKeyStore keystore, IStorage storage = null) + public static async Task GetStorageHelperAsync(bool async, string fileName, string directory, IKeyCache keycache, IStorage storage = null) { StorageHelper storageHelper = null; @@ -110,8 +109,7 @@ public static async Task GetStorageHelperAsync(bool async, strin storageHelper = GetFallbackStorageHelper(fileName, directory, storage); storageHelper.VerifyPersistence(); } - storageHelper.RegisterCache(keystore); - storageHelper.LoadFromCachedStorage(keystore); + storageHelper.LoadFromCachedStorage(keycache); asyncLock.SetValue(storageHelper); } return storageHelper; @@ -130,48 +128,9 @@ public void Clear() } } - public byte[] LoadUnencryptedTokenCache() + public void LoadFromCachedStorage(IKeyCache keycache) { - using (CreateCrossPlatLock(_storageCreationProperties)) - { - return PersistanceStore.ReadData(); - } - } - - public void SaveUnencryptedTokenCache(byte[] tokenCache) - { - using (CreateCrossPlatLock(_storageCreationProperties)) - { - PersistanceStore.WriteData(tokenCache); - } - } - - public void RegisterCache(IKeyStore keystore) - { - if (keystore == null) - { - throw new ArgumentNullException(nameof(keystore)); - } - - _logger.LogInformation($"Registering token cache with on disk storage"); - - keystore.SetOnUpdate(WriteToCachedStorage); - - _logger.LogInformation($"Done initializing"); - } - - public void UnregisterCache(IKeyStore keystore) - { - if (keystore == null) - { - throw new ArgumentNullException(nameof(keystore)); - } - keystore.SetOnUpdate(null); - } - - public void LoadFromCachedStorage(IKeyStore keystore) - { - LogMessage(EventLogLevel.Verbose, $"Before access\nAcquiring lock for token cache"); + LogMessage(EventLogLevel.Verbose, $"Before access\nAcquiring lock for keystore"); using (CreateCrossPlatLock(_storageCreationProperties)) { @@ -184,7 +143,7 @@ public void LoadFromCachedStorage(IKeyStore keystore) } catch (Exception ex) { - LogMessage(EventLogLevel.Error, $"Could not read the token cache. Ignoring. Exception: {ex}"); + LogMessage(EventLogLevel.Error, $"Could not read the keystore. Ignoring. Exception: {ex}"); return; } @@ -193,7 +152,7 @@ public void LoadFromCachedStorage(IKeyStore keystore) try { LogMessage(EventLogLevel.Verbose, $"Deserializing the store"); - keystore.Deserialize(cachedStoreData, false); + keycache.Deserialize(cachedStoreData, false); } catch (Exception e) { @@ -206,7 +165,7 @@ public void LoadFromCachedStorage(IKeyStore keystore) } } - public void WriteToCachedStorage(KeyStoreNotificationArgs args) + public void WriteToCachedStorage(IKeyCache keycache) { using (CreateCrossPlatLock(_storageCreationProperties)) { @@ -215,7 +174,7 @@ public void WriteToCachedStorage(KeyStoreNotificationArgs args) LogMessage(EventLogLevel.Verbose, $"After access, cache in memory HasChanged"); try { - data = args.KeyStore.Serialize(); + data = keycache.Serialize(); } catch (Exception e) { diff --git a/src/Accounts/Authentication/KeyStore/StorageWrapper.cs b/src/Accounts/Authentication/KeyStore/StorageWrapper.cs index aaa15a7600d2..647cabe8acee 100644 --- a/src/Accounts/Authentication/KeyStore/StorageWrapper.cs +++ b/src/Accounts/Authentication/KeyStore/StorageWrapper.cs @@ -16,7 +16,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Common { - class StorageWrapper : IStorage + internal class StorageWrapper : IStorage { public StorageCreationProperties StorageCreationProperties { get; set; } diff --git a/tools/Test/SmokeTest/InstallAzModules.ps1 b/tools/Test/SmokeTest/InstallAzModules.ps1 index ebacf1d52484..8982e959a0a6 100644 --- a/tools/Test/SmokeTest/InstallAzModules.ps1 +++ b/tools/Test/SmokeTest/InstallAzModules.ps1 @@ -44,7 +44,8 @@ Write-Host "Installing Az..." Install-Module -Name Az -Repository $gallery -Scope CurrentUser -AllowClobber -Force $file = Get-ChildItem $localRepoLocation | Where-Object {$_.Name -like "ThreadJob*"} -if ($file -ne $null) { +$installedModule = Get-Module -ListAVailable -Name ThreadJob +if ($file -ne $null -and $installedModule -ne $null) { Write-Host "Install ThreadJob..." Install-Module -Name ThreadJob -Repository $gallery -Scope CurrentUser -AllowClobber -Force } diff --git a/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 b/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 index b7b231f5139b..ea2341b0ddd2 100644 --- a/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 +++ b/tools/Test/SmokeTest/RmCoreSmokeTests.ps1 @@ -164,7 +164,7 @@ $resourceTestCommands = @( $generalCommands = @( @{ - Name = "Import Az.Accounts in Parallel Job"; + Name = "Import Az.Accounts in Parallel (Process)"; Command = { if ($null -ne $env:SYSTEM_DEFINITIONID -or $null -ne $env:Release_DefinitionId -or $null -ne $env:AZUREPS_HOST_ENVIRONMENT) { Write-Warning "Skipping because 'Start-Job' is not supported by design in scenarios where PowerShell is being hosted in other applications." @@ -185,13 +185,9 @@ $generalCommands = @( Retry = 0; # no need to retry } @{ - Name = "Import Az.Accounts in Parallel Thread Job"; + Name = "Import Az.Accounts in Parallel (Thread)"; Command = { - #if ($null -ne $env:SYSTEM_DEFINITIONID -or $null -ne $env:Release_DefinitionId) { - #Write-Warning "Skipping because 'Start-ThreadJob' is not supported by design in scenarios where PowerShell is being hosted in other applications." - #return - #} $importJobs = @() 1..50 | ForEach-Object { $importJobs += Start-ThreadJob -name "import-no.$_" -ScriptBlock { Import-Module Az.Accounts; Get-AzTenant; } From 28b0f897fcf09c582d381bc23c022c00f6016cc8 Mon Sep 17 00:00:00 2001 From: msJinLei Date: Fri, 3 Feb 2023 11:14:15 +0800 Subject: [PATCH 4/5] Fix failed test cases. --- src/Accounts/Accounts.Test/AutosaveTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Accounts/Accounts.Test/AutosaveTests.cs b/src/Accounts/Accounts.Test/AutosaveTests.cs index 0986b9808436..09ffbecb149b 100644 --- a/src/Accounts/Accounts.Test/AutosaveTests.cs +++ b/src/Accounts/Accounts.Test/AutosaveTests.cs @@ -41,6 +41,7 @@ public AutosaveTests(ITestOutputHelper output) XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output)); commandRuntimeMock = new MockCommandRuntime(); dataStore = new MemoryDataStore(); + ResetState(); keyStore = SetMockedAzKeyStore(); } From 47fd8151d34cbb382bf6881819e5451a3a6c4608 Mon Sep 17 00:00:00 2001 From: msJinLei Date: Fri, 3 Feb 2023 13:48:10 +0800 Subject: [PATCH 5/5] Fix an issue of AzKeyStore when context autosaving switching --- src/Accounts/Authentication/KeyStore/AzKeyStore.cs | 6 ++++++ src/Accounts/Authentication/KeyStore/StorageHelper.cs | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs index ade907b83f9e..cb38f0ad9692 100644 --- a/src/Accounts/Authentication/KeyStore/AzKeyStore.cs +++ b/src/Accounts/Authentication/KeyStore/AzKeyStore.cs @@ -97,6 +97,12 @@ public bool RemoveSecureString(IKeyStoreKey key) return InMemoryKeyCache.DeleteKey(key); } + /* Case1: enable --> disable; The methold just unbind the StorageHelper, no influence to InMemoryKeyCache. + * Case2: disable (not enabled before) --> enable; The methold will have the storage data loaded before any access to InMemoryKeyCache; + * InMemoryKeyCache data during the time of disabling will be discarded, which is consistant with the behaviour before AzKeyStore. + * Case3: disable (enabled before) --> enable; The data from storage is already loaded and won't be loaded again. + * Both storage data and InMemoryKeyCache data can be preserved. + */ public void EnableSyncToStorage() { InMemoryKeyCache.SetBeforeAccess(LoadStorage); diff --git a/src/Accounts/Authentication/KeyStore/StorageHelper.cs b/src/Accounts/Authentication/KeyStore/StorageHelper.cs index 85ed4675b180..9921f74443c0 100644 --- a/src/Accounts/Authentication/KeyStore/StorageHelper.cs +++ b/src/Accounts/Authentication/KeyStore/StorageHelper.cs @@ -152,7 +152,8 @@ public void LoadFromCachedStorage(IKeyCache keycache) try { LogMessage(EventLogLevel.Verbose, $"Deserializing the store"); - keycache.Deserialize(cachedStoreData, false); + //Overwrite in memory cache always + keycache.Deserialize(cachedStoreData, true); } catch (Exception e) {