Skip to content

Commit bb51d60

Browse files
ogailogail
committed
Merge pull request #25 from Azure/fixAAD
Fix aad impersonation issue
2 parents 17ea233 + a76423b commit bb51d60

File tree

4 files changed

+218
-63
lines changed

4 files changed

+218
-63
lines changed

‎src/Common/Commands.Common.Test/Common/ProfileClientTests.cs‎

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using System.Collections.Generic;
1717
using System.IO;
1818
using System.Linq;
19+
using Microsoft.Azure.Subscriptions.Models;
1920
using Microsoft.WindowsAzure.Commands.Common.Models;
2021
using Microsoft.WindowsAzure.Commands.Common.Test.Mocks;
2122
using Microsoft.WindowsAzure.Commands.Profile;
@@ -46,6 +47,10 @@ public class ProfileClientTests
4647
private AzureSubscription azureSubscription3withoutUser;
4748
private AzureEnvironment azureEnvironment;
4849
private AzureAccount azureAccount;
50+
private TenantIdDescription commonTenant;
51+
private TenantIdDescription guestTenant;
52+
private Subscriptions.Models.SubscriptionListOperationResponse.Subscription guestRdfeSubscription;
53+
private Subscription guestCsmSubscription;
4954

5055
public ProfileClientTests()
5156
{
@@ -285,6 +290,80 @@ public void AddAzureAccountReturnsAccountWithAllSubscriptionsInCsmMode()
285290
Assert.True(account.GetSubscriptions(client.Profile).Any(s => s.Id == new Guid(csmSubscription1.SubscriptionId)));
286291
}
287292

293+
[Fact]
294+
public void AddAzureAccountWithImpersonatedGuestWithNoSubscriptions()
295+
{
296+
SetMocks(new[] { rdfeSubscription1 }.ToList(), new List<Azure.Subscriptions.Models.Subscription>(),
297+
new[] { commonTenant, guestTenant }.ToList(),
298+
(userAccount, environment, tenant) =>
299+
{
300+
var token = new MockAccessToken
301+
{
302+
UserId = tenant == commonTenant.TenantId ? userAccount.Id : "UserB",
303+
AccessToken = "def",
304+
LoginType = LoginType.OrgId
305+
};
306+
userAccount.Id = token.UserId;
307+
return token;
308+
});
309+
MockDataStore dataStore = new MockDataStore();
310+
dataStore.VirtualStore[oldProfileDataPath] = oldProfileData;
311+
ProfileClient.DataStore = dataStore;
312+
ProfileClient client = new ProfileClient();
313+
314+
var account = client.AddAccountAndLoadSubscriptions(new AzureAccount { Id = "UserA", Type = AzureAccount.AccountType.User }, AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud], null);
315+
316+
Assert.Equal("UserA", account.Id);
317+
Assert.Equal(1, account.GetSubscriptions(client.Profile).Count);
318+
var subrdfe1 = account.GetSubscriptions(client.Profile).FirstOrDefault(s => s.Id == new Guid(rdfeSubscription1.SubscriptionId));
319+
var userA = client.GetAccount("UserA");
320+
var userB = client.GetAccount("UserB");
321+
Assert.NotNull(userA);
322+
Assert.NotNull(userB);
323+
Assert.Contains<string>(rdfeSubscription1.SubscriptionId, userA.GetPropertyAsArray(AzureAccount.Property.Subscriptions), StringComparer.OrdinalIgnoreCase);
324+
Assert.False(userB.HasSubscription(new Guid(rdfeSubscription1.SubscriptionId)));
325+
Assert.NotNull(subrdfe1);
326+
Assert.Equal("UserA", subrdfe1.Account);
327+
}
328+
329+
[Fact]
330+
public void AddAzureAccountWithImpersonatedGuestWithSubscriptions()
331+
{
332+
SetMocks(new[] { rdfeSubscription1, guestRdfeSubscription }.ToList(), new List<Azure.Subscriptions.Models.Subscription>(), new[] { commonTenant, guestTenant }.ToList(),
333+
(userAccount, environment, tenant) =>
334+
{
335+
var token = new MockAccessToken
336+
{
337+
UserId = tenant == commonTenant.TenantId ? userAccount.Id : "UserB",
338+
AccessToken = "def",
339+
LoginType = LoginType.OrgId
340+
};
341+
userAccount.Id = token.UserId;
342+
return token;
343+
});
344+
MockDataStore dataStore = new MockDataStore();
345+
dataStore.VirtualStore[oldProfileDataPath] = oldProfileData;
346+
ProfileClient.DataStore = dataStore;
347+
ProfileClient client = new ProfileClient();
348+
349+
var account = client.AddAccountAndLoadSubscriptions(new AzureAccount { Id = "UserA", Type = AzureAccount.AccountType.User }, AzureEnvironment.PublicEnvironments[EnvironmentName.AzureCloud], null);
350+
351+
Assert.Equal("UserA", account.Id);
352+
Assert.Equal(1, account.GetSubscriptions(client.Profile).Count);
353+
var subrdfe1 = account.GetSubscriptions(client.Profile).FirstOrDefault(s => s.Id == new Guid(rdfeSubscription1.SubscriptionId));
354+
var userA = client.GetAccount("UserA");
355+
var userB = client.GetAccount("UserB");
356+
var subGuest = userB.GetSubscriptions(client.Profile).FirstOrDefault(s => s.Id == new Guid(guestRdfeSubscription.SubscriptionId));
357+
Assert.NotNull(userA);
358+
Assert.NotNull(userB);
359+
Assert.Contains<string>(rdfeSubscription1.SubscriptionId, userA.GetPropertyAsArray(AzureAccount.Property.Subscriptions), StringComparer.OrdinalIgnoreCase);
360+
Assert.Contains<string>(guestRdfeSubscription.SubscriptionId, userB.GetPropertyAsArray(AzureAccount.Property.Subscriptions), StringComparer.OrdinalIgnoreCase);
361+
Assert.NotNull(subrdfe1);
362+
Assert.NotNull(subGuest);
363+
Assert.Equal("UserA", subrdfe1.Account);
364+
Assert.Equal("UserB", subGuest.Account);
365+
}
366+
288367
[Fact]
289368
public void GetAzureAccountReturnsAccountWithSubscriptions()
290369
{
@@ -1139,7 +1218,7 @@ public void SelectAzureSubscriptionByIdWorks()
11391218
cmdlt.InvokeBeginProcessing();
11401219
cmdlt.ExecuteCmdlet();
11411220
cmdlt.InvokeEndProcessing();
1142-
1221+
11431222
Assert.Equal(tempSubscriptions[2].Id, AzureSession.CurrentContext.Subscription.Id);
11441223
}
11451224

@@ -1184,21 +1263,40 @@ public void ImportPublishSettingsAddsSecondCertificate()
11841263
}
11851264

11861265
private void SetMocks(List<WindowsAzure.Subscriptions.Models.SubscriptionListOperationResponse.Subscription> rdfeSubscriptions,
1187-
List<Azure.Subscriptions.Models.Subscription> csmSubscriptions)
1266+
List<Azure.Subscriptions.Models.Subscription> csmSubscriptions,
1267+
List<Azure.Subscriptions.Models.TenantIdDescription> tenants = null,
1268+
Func<AzureAccount, AzureEnvironment, string, IAccessToken> tokenProvider = null)
11881269
{
11891270
ClientMocks clientMocks = new ClientMocks(new Guid(defaultSubscription));
11901271

11911272
clientMocks.LoadRdfeSubscriptions(rdfeSubscriptions);
11921273
clientMocks.LoadCsmSubscriptions(csmSubscriptions);
1274+
clientMocks.LoadTenants(tenants);
11931275

11941276
AzureSession.ClientFactory = new MockClientFactory(new object[] { clientMocks.RdfeSubscriptionClientMock.Object,
11951277
clientMocks.CsmSubscriptionClientMock.Object });
11961278

1197-
AzureSession.AuthenticationFactory = new MockTokenAuthenticationFactory();
1279+
var mockFactory = new MockTokenAuthenticationFactory();
1280+
if (tokenProvider != null)
1281+
{
1282+
mockFactory.TokenProvider = tokenProvider;
1283+
}
1284+
1285+
AzureSession.AuthenticationFactory = mockFactory;
11981286
}
11991287

12001288
private void SetMockData()
12011289
{
1290+
commonTenant = new TenantIdDescription
1291+
{
1292+
Id = "Common",
1293+
TenantId = "Common"
1294+
};
1295+
guestTenant = new TenantIdDescription
1296+
{
1297+
Id = "Guest",
1298+
TenantId = "Guest"
1299+
};
12021300
rdfeSubscription1 = new Subscriptions.Models.SubscriptionListOperationResponse.Subscription
12031301
{
12041302
SubscriptionId = "16E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1E",
@@ -1213,6 +1311,13 @@ private void SetMockData()
12131311
SubscriptionStatus = Subscriptions.Models.SubscriptionStatus.Active,
12141312
ActiveDirectoryTenantId = "Common"
12151313
};
1314+
guestRdfeSubscription = new Subscriptions.Models.SubscriptionListOperationResponse.Subscription
1315+
{
1316+
SubscriptionId = "26E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1C",
1317+
SubscriptionName = "RdfeSub2",
1318+
SubscriptionStatus = Subscriptions.Models.SubscriptionStatus.Active,
1319+
ActiveDirectoryTenantId = "Guest"
1320+
};
12161321
csmSubscription1 = new Azure.Subscriptions.Models.Subscription
12171322
{
12181323
Id = "Subscriptions/36E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1E",
@@ -1234,6 +1339,13 @@ private void SetMockData()
12341339
State = "Active",
12351340
SubscriptionId = "46E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1E"
12361341
};
1342+
guestCsmSubscription = new Azure.Subscriptions.Models.Subscription
1343+
{
1344+
Id = "Subscriptions/76E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1D",
1345+
DisplayName = "CsmGuestSub",
1346+
State = "Active",
1347+
SubscriptionId = "76E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1D"
1348+
};
12371349
azureSubscription1 = new AzureSubscription
12381350
{
12391351
Id = new Guid("56E3F6FD-A3AA-439A-8FC4-1F5C41D2AD1E"),

‎src/Common/Commands.Common.Test/Mocks/ClientMocks.cs‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ private SubscriptionCloudCredentials CreateCredentials(Guid subscriptionId)
5858
}
5959

6060
public static CloudException Make404Exception()
61-
{
61+
{
6262
return CloudException.Create(
6363
new HttpRequestMessage(),
6464
null,
@@ -75,18 +75,7 @@ public void LoadCsmSubscriptions(List<Azure.Subscriptions.Models.Subscription> s
7575
Subscriptions = subscriptions
7676
}));
7777

78-
var tenantOperationsMock = new Mock<Azure.Subscriptions.ITenantOperations>();
79-
tenantOperationsMock.Setup(f => f.ListAsync(new CancellationToken()))
80-
.Returns(Task.Factory.StartNew(() => new Azure.Subscriptions.Models.TenantListResult
81-
{
82-
TenantIds = new[] { new Azure.Subscriptions.Models.TenantIdDescription
83-
{
84-
Id = "1", TenantId = "1"
85-
}}.ToList()
86-
}));
87-
8878
CsmSubscriptionClientMock.Setup(f => f.Subscriptions).Returns(subscriptionOperationsMock.Object);
89-
CsmSubscriptionClientMock.Setup(f => f.Tenants).Returns(tenantOperationsMock.Object);
9079
}
9180

9281
public void LoadRdfeSubscriptions(List<WindowsAzure.Subscriptions.Models.SubscriptionListOperationResponse.Subscription> subscriptions)
@@ -100,5 +89,26 @@ public void LoadRdfeSubscriptions(List<WindowsAzure.Subscriptions.Models.Subscri
10089

10190
RdfeSubscriptionClientMock.Setup(f => f.Subscriptions).Returns(subscriptionOperationsMock.Object);
10291
}
92+
93+
public void LoadTenants(List<Azure.Subscriptions.Models.TenantIdDescription> tenantIds = null)
94+
{
95+
96+
tenantIds = tenantIds ?? new[]
97+
{
98+
new Azure.Subscriptions.Models.TenantIdDescription
99+
{
100+
Id = "Common",
101+
TenantId = "Common"
102+
}
103+
}.ToList();
104+
var tenantOperationsMock = new Mock<Azure.Subscriptions.ITenantOperations>();
105+
tenantOperationsMock.Setup(f => f.ListAsync(new CancellationToken()))
106+
.Returns(Task.Factory.StartNew(() => new Azure.Subscriptions.Models.TenantListResult
107+
{
108+
TenantIds = tenantIds
109+
}));
110+
111+
CsmSubscriptionClientMock.Setup(f => f.Tenants).Returns(tenantOperationsMock.Object);
112+
}
103113
}
104114
}

‎src/Common/Commands.Common.Test/Mocks/MockTokenAuthenticationFactory.cs‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15+
using System;
1516
using System.Security;
1617
using System.Security.Cryptography.X509Certificates;
1718
using Microsoft.WindowsAzure.Commands.Common.Models;
@@ -23,6 +24,8 @@ public class MockTokenAuthenticationFactory : IAuthenticationFactory
2324
{
2425
public IAccessToken Token { get; set; }
2526

27+
public Func<AzureAccount, AzureEnvironment, string, IAccessToken> TokenProvider { get; set; }
28+
2629
public MockTokenAuthenticationFactory()
2730
{
2831
Token = new MockAccessToken
@@ -31,6 +34,13 @@ public MockTokenAuthenticationFactory()
3134
LoginType = LoginType.OrgId,
3235
AccessToken = "abc"
3336
};
37+
38+
TokenProvider = (account, environment, tenant) => Token = new MockAccessToken
39+
{
40+
UserId = account.Id,
41+
LoginType = LoginType.OrgId,
42+
AccessToken = Token.AccessToken
43+
};
3444
}
3545

3646
public MockTokenAuthenticationFactory(string userId, string accessToken)
@@ -50,14 +60,7 @@ public IAccessToken Authenticate(AzureAccount account, AzureEnvironment environm
5060
account.Id = "test";
5161
}
5262

53-
Token = new MockAccessToken
54-
{
55-
UserId = account.Id,
56-
LoginType = LoginType.OrgId,
57-
AccessToken = Token.AccessToken
58-
};
59-
60-
return Token;
63+
return TokenProvider(account, environment, tenant);
6164
}
6265

6366
public SubscriptionCloudCredentials GetSubscriptionCloudCredentials(AzureContext context)

0 commit comments

Comments
 (0)