Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue when returned token user id does not match case of existing account user id #51

Merged
merged 6 commits into from
Dec 18, 2014
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion src/Common/Commands.Common.Test/Common/ProfileClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ public void AddAzureAccountReturnsAccountWithAllSubscriptionsInCsmMode()
Assert.True(account.GetSubscriptions(client.Profile).Any(s => s.Id == new Guid(csmSubscription1.SubscriptionId)));
}

/// <summary>
/// Verify that if a user has a different identity in one tenant, the identity is not added if it has no
/// access to subscriptions
/// </summary>
[Fact]
public void AddAzureAccountWithImpersonatedGuestWithNoSubscriptions()
{
Expand Down Expand Up @@ -326,6 +330,10 @@ public void AddAzureAccountWithImpersonatedGuestWithNoSubscriptions()
Assert.Equal("UserA", subrdfe1.Account);
}

/// <summary>
/// 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
/// </summary>
[Fact]
public void AddAzureAccountWithImpersonatedGuestWithSubscriptions()
{
Expand All @@ -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);
Expand All @@ -363,6 +372,39 @@ public void AddAzureAccountWithImpersonatedGuestWithSubscriptions()
Assert.Equal("UserA", subrdfe1.Account);
Assert.Equal("UserB", subGuest.Account);
}
/// <summary>
/// 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
/// </summary>
[Fact]
public void AddAzureAccountIsCaseInsensitive()
{
SetMocks(new[] { rdfeSubscription1, guestRdfeSubscription }.ToList(), new List<Azure.Subscriptions.Models.Subscription>(), 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);

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()
Expand Down
10 changes: 5 additions & 5 deletions src/Common/Commands.Common/Common/ProfileClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Account Ids do not match.

}
Expand Down Expand Up @@ -918,7 +918,7 @@ private IEnumerable<AzureSubscription> 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;
}
Expand Down Expand Up @@ -984,7 +984,7 @@ private IEnumerable<AzureSubscription> 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;
}
Expand Down