Skip to content

Commit 50b10d6

Browse files
msJinLeimsJinLei
authored andcommitted
Address review comments
Co-authored-by: Yeming Liu <[email protected]> Address review comments
1 parent a72f3d2 commit 50b10d6

26 files changed

+336
-151
lines changed

src/Accounts/Accounts.Test/AutosaveTests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ public AutosaveTests(ITestOutputHelper output)
4141
XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output));
4242
commandRuntimeMock = new MockCommandRuntime();
4343
dataStore = new MemoryDataStore();
44-
ResetState();
4544
keyStore = SetMockedAzKeyStore();
4645
}
4746

@@ -51,7 +50,7 @@ private AzKeyStore SetMockedAzKeyStore()
5150
storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object);
5251
storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]);
5352
storageMocker.Setup(f => f.WriteData(It.IsAny<byte[]>())).Callback((byte[] s) => {});
54-
var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object);
53+
var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", true, storageMocker.Object);
5554
return keyStore;
5655
}
5756

src/Accounts/Accounts.Test/ContextCmdletTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public ContextCmdletTests(ITestOutputHelper output)
6464
Mock<IStorage> storageMocker = new Mock<IStorage>();
6565
AzKeyStore azKeyStore = null;
6666
string profilePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), Resources.AzureDirectoryName);
67-
azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, storageMocker.Object);
67+
azKeyStore = new AzKeyStore(profilePath, AzureSession.Instance.KeyStoreFile, true, storageMocker.Object);
6868
AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => azKeyStore, true);
6969
}
7070

src/Accounts/Accounts.Test/ProfileCmdletTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ private AzKeyStore SetMockedAzKeyStore()
5555
storageMocker.Setup(f => f.Create()).Returns(storageMocker.Object);
5656
storageMocker.Setup(f => f.ReadData()).Returns(new byte[0]);
5757
storageMocker.Setup(f => f.WriteData(It.IsAny<byte[]>())).Callback((byte[] s) => { });
58-
var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", storageMocker.Object);
58+
var keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, "azkeystore", false, storageMocker.Object);
5959
return keyStore;
6060
}
6161

src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ public override void ExecuteCmdlet()
425425
azureAccount.SetProperty(AzureAccount.Property.CertificatePath, resolvedPath);
426426
if (CertificatePassword != null)
427427
{
428-
keyStore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword);
428+
keyStore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, azureAccount.Id, Tenant), CertificatePassword);
429429
if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected)
430430
{
431431
WriteWarning(string.Format(Resources.ServicePrincipalWarning, AzureSession.Instance.KeyStoreFile, AzureSession.Instance.ARMProfileDirectory));
@@ -451,7 +451,7 @@ public override void ExecuteCmdlet()
451451

452452
if (azureAccount.Type == AzureAccount.AccountType.ServicePrincipal && password != null)
453453
{
454-
keyStore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret
454+
keyStore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret
455455
,azureAccount.Id, Tenant), password);
456456
if (GetContextModificationScope() == ContextModificationScope.CurrentUser && !keyStore.IsProtected)
457457
{
@@ -713,7 +713,7 @@ public void OnImport()
713713
}
714714

715715
AzKeyStore keyStore = null;
716-
keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile);
716+
keyStore = new AzKeyStore(AzureSession.Instance.ARMProfileDirectory, AzureSession.Instance.KeyStoreFile, autoSaveEnabled);
717717
AzureSession.Instance.RegisterComponent(AzKeyStore.Name, () => keyStore);
718718

719719
if (!InitializeProfileProvider(autoSaveEnabled))

src/Accounts/Accounts/AutoSave/DisableAzureRmContextAutosave.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ void DisableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextA
9292
builder.Reset();
9393
}
9494

95+
if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore))
96+
{
97+
keystore.DisableSyncToStorage();
98+
}
99+
95100
if (writeAutoSaveFile)
96101
{
97102
FileUtilities.EnsureDirectoryExists(session.ProfileDirectory);

src/Accounts/Accounts/AutoSave/EnableAzureRmContextAutosave.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ void EnableAutosave(IAzureSession session, bool writeAutoSaveFile, out ContextAu
102102
AzureSession.Instance.RegisterComponent(PowerShellTokenCacheProvider.PowerShellTokenCacheProviderKey, () => newCacheProvider, true);
103103
}
104104

105+
if (AzureSession.Instance.TryGetComponent(AzKeyStore.Name, out AzKeyStore keystore))
106+
{
107+
keystore.EnableSyncToStorage();
108+
}
109+
105110
if (writeAutoSaveFile)
106111
{
107112
try

src/Accounts/Accounts/ChangeLog.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
* 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]
2323
* Enabled credential to be found only by applicationId while tenant was not matched when accquire token. [#20484]
2424
* When Az.Accounts ran in parallel, the waiters were allowed to wait infinitely to avoid throw exception in automation enviroment. [#20455]
25-
* Used Lazy load for AzKeyStore.
26-
* Used update on change mechanism for AzKeyStore and remove `Flush` interface.
2725

2826
## Version 2.11.1
2927
* Fixed an issue where Az.Accounts cannot be imported correctly. [#20615]

src/Accounts/Accounts/Context/ImportAzureRMContext.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,13 @@ void CopyProfile(AzureRmProfile source, IProfileOperations target)
7878
var secret = account.GetProperty(AzureAccount.Property.ServicePrincipalSecret);
7979
if (!string.IsNullOrEmpty(secret))
8080
{
81-
keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, context.Value.Tenant?.Id)
81+
keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, context.Value.Tenant?.Id)
8282
, secret.ConvertToSecureString());
8383
}
8484
var password = account.GetProperty(AzureAccount.Property.CertificatePassword);
8585
if (!string.IsNullOrEmpty(password))
8686
{
87-
keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, context.Value.Tenant?.Id)
87+
keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, context.Value.Tenant?.Id)
8888
,password.ConvertToSecureString());
8989
}
9090
}

src/Accounts/Accounts/Context/SetAzureRMContext.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,13 +97,13 @@ public override void ExecuteCmdlet()
9797
var secret = account.GetProperty(AzureAccount.Property.ServicePrincipalSecret);
9898
if (!string.IsNullOrEmpty(secret))
9999
{
100-
keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, Context.Tenant?.Id)
100+
keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, Context.Tenant?.Id)
101101
, secret.ConvertToSecureString());
102102
}
103103
var password = account.GetProperty(AzureAccount.Property.CertificatePassword);
104104
if (!string.IsNullOrEmpty(password))
105105
{
106-
keyStore.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, Context.Tenant?.Id)
106+
keyStore.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, Context.Tenant?.Id)
107107
, password.ConvertToSecureString());
108108
}
109109
}

src/Accounts/Authentication.ResourceManager/AzureRmProfile.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,13 @@ private IAzureContext MigrateSecretToKeyStore(IAzureContext context, AzKeyStore
225225
var account = context.Account;
226226
if (account.IsPropertySet(AzureAccount.Property.ServicePrincipalSecret))
227227
{
228-
keystore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First())
228+
keystore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.ServicePrincipalSecret, account.Id, account.GetTenants().First())
229229
, account.ExtendedProperties.GetProperty(AzureAccount.Property.ServicePrincipalSecret).ConvertToSecureString());
230230
account.ExtendedProperties.Remove(AzureAccount.Property.ServicePrincipalSecret);
231231
}
232232
if (account.IsPropertySet(AzureAccount.Property.CertificatePassword))
233233
{
234-
keystore?.SaveCredential(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First())
234+
keystore?.SaveSecureString(new ServicePrincipalKey(AzureAccount.Property.CertificatePassword, account.Id, account.GetTenants().First())
235235
, account.ExtendedProperties.GetProperty(AzureAccount.Property.CertificatePassword).ConvertToSecureString());
236236
account.ExtendedProperties.Remove(AzureAccount.Property.CertificatePassword);
237237
}

0 commit comments

Comments
 (0)