From a956427536d264a644d4c6bdd8ba6f7b224b5db0 Mon Sep 17 00:00:00 2001 From: markcowl Date: Tue, 16 Dec 2014 11:20:38 -0800 Subject: [PATCH 1/6] Fixing issue with tokens that do not normalize user ids --- src/Common/Commands.Common/Common/ProfileClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/Commands.Common/Common/ProfileClient.cs b/src/Common/Commands.Common/Common/ProfileClient.cs index 14b612c9babb..fd2690bc2aea 100644 --- a/src/Common/Commands.Common/Common/ProfileClient.cs +++ b/src/Common/Commands.Common/Common/ProfileClient.cs @@ -863,7 +863,7 @@ private AzureAccount MergeAccountProperties(AzureAccount account1, AzureAccount { throw new ArgumentNullException("account1"); } - if (account1.Id != account2.Id) + if (!string.Equals(account1.Id, account2.Id, StringComparison.InvariantCultureIgnoreCase)) { throw new ArgumentException("Account1 Ids do not match."); } From 12dea2339fe05b612b204a5484b5091542e7b6c6 Mon Sep 17 00:00:00 2001 From: markcowl Date: Tue, 16 Dec 2014 17:43:14 -0800 Subject: [PATCH 2/6] Adding tests to verify the fix --- .../Common/ProfileClientTests.cs | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs b/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs index 222da7747089..3a38698b6219 100644 --- a/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs +++ b/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs @@ -290,6 +290,10 @@ public void AddAzureAccountReturnsAccountWithAllSubscriptionsInCsmMode() Assert.True(account.GetSubscriptions(client.Profile).Any(s => s.Id == new Guid(csmSubscription1.SubscriptionId))); } + /// + /// Verify that if a user has a different identity in one tenant, the identity is not added if it has no + /// access to subscriptions + /// [Fact] public void AddAzureAccountWithImpersonatedGuestWithNoSubscriptions() { @@ -326,6 +330,10 @@ public void AddAzureAccountWithImpersonatedGuestWithNoSubscriptions() Assert.Equal("UserA", subrdfe1.Account); } + /// + /// Verify that multiple accounts can be added if a user has different identitities in different domains, linked to the same login + /// Verify that subscriptions with admin access forall accounts are added + /// [Fact] public void AddAzureAccountWithImpersonatedGuestWithSubscriptions() { @@ -346,7 +354,8 @@ public void AddAzureAccountWithImpersonatedGuestWithSubscriptions() ProfileClient.DataStore = dataStore; ProfileClient client = new ProfileClient(); - var account = client.AddAccountAndLoadSubscriptions(new AzureAccount { Id = "UserA", Type = AzureAccount.AccountType.User }, AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud], null); + var account = client.AddAccountAndLoadSubscriptions(new AzureAccount { Id = "UserA", Type = AzureAccount.AccountType.User }, + AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud], null); Assert.Equal("UserA", account.Id); Assert.Equal(1, account.GetSubscriptions(client.Profile).Count); @@ -363,6 +372,40 @@ public void AddAzureAccountWithImpersonatedGuestWithSubscriptions() Assert.Equal("UserA", subrdfe1.Account); Assert.Equal("UserB", subGuest.Account); } + /// + /// Test that when account is added more than once with different capitalization, only a single account is added + /// and that accounts can be retrieved case-insensitively + /// + [Fact] + public void AddAzureAccountIsCaseInsensitive() + { + SetMocks(new[] { rdfeSubscription1, guestRdfeSubscription }.ToList(), new List(), new[] { commonTenant, guestTenant }.ToList(), + (userAccount, environment, tenant) => + { + var token = new MockAccessToken + { + UserId = tenant == commonTenant.TenantId ? userAccount.Id : "USERA", + AccessToken = "def", + LoginType = LoginType.OrgId + }; + userAccount.Id = token.UserId; + return token; + }); + MockDataStore dataStore = new MockDataStore(); + dataStore.VirtualStore[oldProfileDataPath] = oldProfileData; + ProfileClient.DataStore = dataStore; + ProfileClient client = new ProfileClient(); + + var account = client.AddAccountAndLoadSubscriptions(new AzureAccount { Id = "UserA", Type = AzureAccount.AccountType.User }, + AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud], null); + + Assert.Equal("USERA", account.Id); + var userA = client.GetAccount("UserA"); + var secondUserA = client.GetAccount("USERA"); + Assert.NotNull(userA); + Assert.NotNull(secondUserA); + Assert.Equal(userA.Id, secondUserA.Id); + } [Fact] public void GetAzureAccountReturnsAccountWithSubscriptions() From 64694f4f14bab732da94c2e20a81a9ad8e2dbfe3 Mon Sep 17 00:00:00 2001 From: markcowl Date: Tue, 16 Dec 2014 17:49:01 -0800 Subject: [PATCH 3/6] making id checks consistently case insensitive --- src/Common/Commands.Common/Common/ProfileClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Common/Commands.Common/Common/ProfileClient.cs b/src/Common/Commands.Common/Common/ProfileClient.cs index fd2690bc2aea..f02b4f5e3b19 100644 --- a/src/Common/Commands.Common/Common/ProfileClient.cs +++ b/src/Common/Commands.Common/Common/ProfileClient.cs @@ -918,7 +918,7 @@ private IEnumerable ListResourceManagerSubscriptions(AzureAcc var tenantAccount = new AzureAccount(); CopyAccount(account, tenantAccount); var tenantToken = AzureSession.AuthenticationFactory.Authenticate(tenantAccount, environment, tenant, password, ShowDialog.Never); - if (tenantAccount.Id == account.Id) + if (string.Equals(tenantAccount.Id, account.Id, StringComparison.InvariantCultureIgnoreCase)) { tenantAccount = account; } @@ -984,7 +984,7 @@ private IEnumerable ListServiceManagementSubscriptions(AzureA var tenantAccount = new AzureAccount(); CopyAccount(account, tenantAccount); var tenantToken = AzureSession.AuthenticationFactory.Authenticate(tenantAccount, environment, tenant, password, ShowDialog.Never); - if (tenantAccount.Id == account.Id) + if (string.Equals(tenantAccount.Id, account.Id, StringComparison.InvariantCultureIgnoreCase)) { tenantAccount = account; } From 42e71838bbb8a12d40342bd2981cbc6e651a526f Mon Sep 17 00:00:00 2001 From: markcowl Date: Wed, 17 Dec 2014 09:15:29 -0800 Subject: [PATCH 4/6] Fixing parallel issue with environments and making unit test more robust --- src/Common/Commands.Common.Test/Common/ProfileClientTests.cs | 1 - src/Common/Commands.Common/Common/ProfileClient.cs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs b/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs index 3a38698b6219..bc610519eaae 100644 --- a/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs +++ b/src/Common/Commands.Common.Test/Common/ProfileClientTests.cs @@ -399,7 +399,6 @@ public void AddAzureAccountIsCaseInsensitive() var account = client.AddAccountAndLoadSubscriptions(new AzureAccount { Id = "UserA", Type = AzureAccount.AccountType.User }, AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud], null); - Assert.Equal("USERA", account.Id); var userA = client.GetAccount("UserA"); var secondUserA = client.GetAccount("USERA"); Assert.NotNull(userA); diff --git a/src/Common/Commands.Common/Common/ProfileClient.cs b/src/Common/Commands.Common/Common/ProfileClient.cs index f02b4f5e3b19..9ef947db6a3d 100644 --- a/src/Common/Commands.Common/Common/ProfileClient.cs +++ b/src/Common/Commands.Common/Common/ProfileClient.cs @@ -835,9 +835,9 @@ private AzureEnvironment MergeEnvironmentProperties(AzureEnvironment environment { throw new ArgumentNullException("environment1"); } - if (environment1.Name != environment2.Name) + if (!string.Equals(environment1.Name, environment2.Name, StringComparison.InvariantCultureIgnoreCase)) { - throw new ArgumentException("Subscription Ids do not match."); + throw new ArgumentException("Environment names do not match."); } AzureEnvironment mergedEnvironment = new AzureEnvironment { From a45a0c047efd7001def5f738979af466213a51bd Mon Sep 17 00:00:00 2001 From: markcowl Date: Wed, 17 Dec 2014 16:27:55 -0800 Subject: [PATCH 5/6] Improving error message --- src/Common/Commands.Common/Common/ProfileClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/Commands.Common/Common/ProfileClient.cs b/src/Common/Commands.Common/Common/ProfileClient.cs index 9ef947db6a3d..217b84df6592 100644 --- a/src/Common/Commands.Common/Common/ProfileClient.cs +++ b/src/Common/Commands.Common/Common/ProfileClient.cs @@ -865,7 +865,7 @@ private AzureAccount MergeAccountProperties(AzureAccount account1, AzureAccount } if (!string.Equals(account1.Id, account2.Id, StringComparison.InvariantCultureIgnoreCase)) { - throw new ArgumentException("Account1 Ids do not match."); + throw new ArgumentException("Account Ids do not match."); } if (account1.Type != account2.Type) { From 0a5ea1c80a76ef3d96e0c5ef48bebfcdda3dd96d Mon Sep 17 00:00:00 2001 From: sriramvu Date: Thu, 18 Dec 2014 14:09:57 +0530 Subject: [PATCH 6/6] Corrected format string in framing ClientRequestId --- .../PSRecoveryServicesClient/PSRecoveryServicesClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceManagement/RecoveryServices/Commands.RecoveryServices/PSRecoveryServicesClient/PSRecoveryServicesClient.cs b/src/ServiceManagement/RecoveryServices/Commands.RecoveryServices/PSRecoveryServicesClient/PSRecoveryServicesClient.cs index 8ba046e6aa49..85613f71c947 100644 --- a/src/ServiceManagement/RecoveryServices/Commands.RecoveryServices/PSRecoveryServicesClient/PSRecoveryServicesClient.cs +++ b/src/ServiceManagement/RecoveryServices/Commands.RecoveryServices/PSRecoveryServicesClient/PSRecoveryServicesClient.cs @@ -187,7 +187,7 @@ public string GenerateAgentAuthenticationHeader(string clientRequestId) /// Custom request headers public CustomRequestHeaders GetRequestHeaders() { - this.ClientRequestId = Guid.NewGuid().ToString() + "-" + DateTime.Now.ToString("yyyy-mm-dd HH:mm:ssZ") + "-P"; + this.ClientRequestId = Guid.NewGuid().ToString() + "-" + DateTime.Now.ToUniversalTime().ToString("yyyy-MM-dd HH:mm:ssZ") + "-P"; return new CustomRequestHeaders() {