Skip to content

Commit

Permalink
Device code flow fixes when broker is enabled (#4846)
Browse files Browse the repository at this point in the history 
* Get test app ready adding Device code flow option

* Initial code to add AccountSource to cache

* Complete implementation and add test

* update strings

* Test code cleanup

* Update Account.cs
  • Loading branch information
@ashok672
ashok672 authored Jul 18, 2024
1 parent c1a99c4 commit 574f2e6
Show file tree
Hide file tree
Showing 18 changed files with 214 additions and 17 deletions.
1 change: 1 addition & 0 deletions src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ public static bool TryConvertToMsalAccount(
nativeAccount.HomeAccountid,
nativeAccount.UserName,
nativeAccount.Environment,
null,
new Dictionary<string, string>() {
{
clientId,
Expand Down
10 changes: 9 additions & 1 deletion src/client/Microsoft.Identity.Client/Account.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,20 @@ internal sealed class Account : IAccount
/// <param name="homeAccountId">Home account ID in "uid.utid" format; can be null, for example when migrating the ADAL v3 cache.</param>
/// <param name="username"><see href="https://learn.microsoft.com/windows/win32/secauthn/user-name-formats#user-principal-name">UPN-style</see>, can be null</param>
/// <param name="environment">Identity provider for the account, e.g., <c>login.microsoftonline.com</c>.</param>
/// <param name="accountsource">The initial flow that established the account. e.g., device code flow.</param>
/// <param name="wamAccountIds">Map of (<c>client_id</c>, <c>wam_account_id</c>)</param>
/// <param name="tenantProfiles">Map of (<c>tenant_id</c>, <c>tenant_profile</c>)</param>
public Account(
string homeAccountId,
string username,
string environment,
string environment,
string accountsource = null,
IDictionary<string, string> wamAccountIds = null,
IEnumerable<TenantProfile> tenantProfiles = null)
{
Username = username;
Environment = environment;
AccountSource = accountsource;
HomeAccountId = AccountId.ParseFromString(homeAccountId);
WamAccountIds = wamAccountIds;
TenantProfiles = tenantProfiles;
Expand All @@ -44,6 +47,11 @@ public Account(
/// </summary>
public string Environment { get; }

/// <summary>
/// Gets the source of the account. For example, device code flow, broker etc.
/// </summary>
public string AccountSource { get; }

/// <summary>
/// Gets additional account identifiers, such as object ID, tenant ID, and the unique identifier.
/// </summary>
Expand Down
15 changes: 13 additions & 2 deletions src/client/Microsoft.Identity.Client/Cache/Items/MsalAccountCacheItem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ internal MsalAccountCacheItem(
string preferredCacheEnv,
string clientInfo,
string homeAccountId,
string accountSource,
IdToken idToken,
string preferredUsername,
string tenantId,
Expand All @@ -59,6 +60,7 @@ internal MsalAccountCacheItem(
idToken?.GetUniqueId(),
clientInfo,
homeAccountId,
accountSource,
idToken?.Name,
preferredUsername,
tenantId,
Expand All @@ -72,6 +74,7 @@ internal MsalAccountCacheItem(
string localAccountId,
string rawClientInfo,
string homeAccountId,
string accountSource,
string name,
string preferredUsername,
string tenantId,
Expand All @@ -85,6 +88,7 @@ internal MsalAccountCacheItem(
localAccountId,
rawClientInfo,
homeAccountId,
accountSource,
name,
preferredUsername,
tenantId,
Expand All @@ -96,13 +100,15 @@ internal MsalAccountCacheItem(
internal MsalAccountCacheItem(
string environment,
string tenantId,
string homeAccountId,
string homeAccountId,
string accountSource,
string preferredUsername)
: this()
{
Environment = environment;
TenantId = tenantId;
HomeAccountId = homeAccountId;
AccountSource = accountSource;
PreferredUsername = preferredUsername;

InitCacheKey();
Expand All @@ -114,6 +120,7 @@ internal MsalAccountCacheItem(
internal string GivenName { get; set; }
internal string FamilyName { get; set; }
internal string LocalAccountId { get; set; }
internal string AccountSource { get; set; }
internal string AuthorityType { get; set; }

/// <summary>
Expand All @@ -133,6 +140,7 @@ private void Init(
string localAccountId,
string rawClientInfo,
string homeAccountId,
string accountSource,
string name,
string preferredUsername,
string tenantId,
Expand All @@ -149,6 +157,7 @@ private void Init(
GivenName = givenName;
FamilyName = familyName;
HomeAccountId = homeAccountId;
AccountSource = accountSource;
WamAccountIds = wamAccountIds;

InitCacheKey();
Expand Down Expand Up @@ -202,7 +211,8 @@ internal static MsalAccountCacheItem FromJObject(JObject j)
LocalAccountId = JsonHelper.ExtractExistingOrEmptyString(j, StorageJsonKeys.LocalAccountId),
AuthorityType = JsonHelper.ExtractExistingOrEmptyString(j, StorageJsonKeys.AuthorityType),
TenantId = JsonHelper.ExtractExistingOrEmptyString(j, StorageJsonKeys.Realm),
WamAccountIds = JsonHelper.ExtractInnerJsonAsDictionary(j, StorageJsonKeys.WamAccountIds)
WamAccountIds = JsonHelper.ExtractInnerJsonAsDictionary(j, StorageJsonKeys.WamAccountIds),
AccountSource = JsonHelper.ExtractExistingOrEmptyString(j, StorageJsonKeys.AccountSource)
};

item.PopulateFieldsFromJObject(j);
Expand All @@ -223,6 +233,7 @@ internal override JObject ToJObject()
SetItemIfValueNotNull(json, StorageJsonKeys.LocalAccountId, LocalAccountId);
SetItemIfValueNotNull(json, StorageJsonKeys.AuthorityType, AuthorityType);
SetItemIfValueNotNull(json, StorageJsonKeys.Realm, TenantId);
SetItemIfValueNotNull(json, StorageJsonKeys.AccountSource, AccountSource);
if (WamAccountIds != null && WamAccountIds.Any())
{
#if SUPPORTS_SYSTEM_TEXT_JSON
Expand Down
1 change: 1 addition & 0 deletions src/client/Microsoft.Identity.Client/Cache/StorageJsonKeys.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internal static class StorageJsonKeys
public const string KeyId = "kid";
public const string TokenType = "token_type";
public const string WamAccountIds = "wam_account_ids";
public const string AccountSource = "account_source";

// todo(cache): this needs to be added to the spec. needed for OBO flow on .NET.
public const string UserAssertionHash = "user_assertion_hash";
Expand Down
8 changes: 7 additions & 1 deletion src/client/Microsoft.Identity.Client/IAccount.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ public interface IAccount
/// AccountId of the home account for the user. This uniquely identifies the user across AAD tenants.
/// </summary>
/// <remarks>Can be null, for example if this account was migrated to MSAL.NET from ADAL.NET v3's token cache</remarks>
AccountId HomeAccountId { get; }
AccountId HomeAccountId { get; }

/// <summary>
/// The initial flow that established the account. For example, device code flow.
/// </summary>
/// <remarks>Can be null. Currently only device code flow updates this property with a valid string</remarks>
string AccountSource { get; }
}
}
5 changes: 4 additions & 1 deletion src/client/Microsoft.Identity.Client/Internal/Requests/Silent/SilentRequest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
UiRequiredExceptionClassification.AcquireTokenSilentFailed);
}

if (isBrokerConfigured)
bool isAccountSourceDeviceCodeFlow = !string.IsNullOrEmpty(AuthenticationRequestParameters.Account.AccountSource) &&
AuthenticationRequestParameters.Account.AccountSource == "device_code_flow";

if (isBrokerConfigured && !isAccountSourceDeviceCodeFlow)
{
_logger.Info("Broker is configured and enabled, attempting to use broker instead.");
var brokerResult = await _brokerStrategyLazy.Value.ExecuteAsync(cancellationToken).ConfigureAwait(false);
Expand Down
12 changes: 10 additions & 2 deletions src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,12 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem, Account>> IToke
homeAccountId);

Dictionary<string, string> wamAccountIds = TokenResponseHelper.GetWamAccountIds(requestParams, response);
string accountSource = requestParams.ApiId == ApiEvent.ApiIds.AcquireTokenByDeviceCode ? "device_code_flow" : null;
msalAccountCacheItem = new MsalAccountCacheItem(
instanceDiscoveryMetadata.PreferredCache,
response.ClientInfo,
homeAccountId,
accountSource,
idToken,
username,
tenantId,
Expand All @@ -149,10 +151,12 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem, Account>> IToke
}
}


account = new Account(
homeAccountId,
username,
instanceDiscoveryMetadata.PreferredNetwork,
accountSource,
wamAccountIds,
tenantProfiles?.Values);
}
Expand Down Expand Up @@ -988,6 +992,7 @@ async Task<IEnumerable<IAccount>> ITokenCacheInternal.GetAccountsAsync(Authentic
requestParameters.AppConfig.MultiCloudSupportEnabled ?
account.Environment : // If multi cloud support is enabled keep the cached environment
environment, // Preserve the environment passed in by the user
account.AccountSource,
account.WamAccountIds,
tenantProfiles?.Values);

Expand Down Expand Up @@ -1020,6 +1025,7 @@ async Task<IEnumerable<IAccount>> ITokenCacheInternal.GetAccountsAsync(Authentic
cachedAccount.HomeAccountId,
cachedAccount.PreferredUsername,
cachedAccount.Environment,
cachedAccount.AccountSource,
cachedAccount.WamAccountIds,
tenantProfiles?.Values);

Expand Down Expand Up @@ -1067,7 +1073,7 @@ private static void UpdateMapWithAdalAccountsWithClientInfo(
if (!clientInfoToAccountMap.ContainsKey(accountIdentifier))
{
clientInfoToAccountMap[accountIdentifier] = new Account(
accountIdentifier, pair.Value.DisplayableId, envFromRequest);
accountIdentifier, pair.Value.DisplayableId, envFromRequest, null);
}
}
}
Expand All @@ -1084,7 +1090,7 @@ private static void UpdateWithAdalAccountsWithoutClientInfo(
{
if (!string.IsNullOrEmpty(user.DisplayableId) && !uniqueUserNames.Contains(user.DisplayableId))
{
accounts.Add(new Account(null, user.DisplayableId, envFromRequest));
accounts.Add(new Account(null, user.DisplayableId, envFromRequest, null));
uniqueUserNames.Add(user.DisplayableId);
}
}
Expand Down Expand Up @@ -1153,12 +1159,14 @@ async Task<Account> ITokenCacheInternal.GetAccountAssociatedWithAccessTokenAsync
msalAccessTokenCacheItem.Environment,
msalAccessTokenCacheItem.TenantId,
msalAccessTokenCacheItem.HomeAccountId,
requestParameters.Account?.AccountSource,
requestParameters.Account?.Username));

return new Account(
msalAccessTokenCacheItem.HomeAccountId,
accountCacheItem?.PreferredUsername,
accountCacheItem?.Environment,
accountCacheItem?.AccountSource,
accountCacheItem?.WamAccountIds,
tenantProfiles?.Values);
}
Expand Down
3 changes: 3 additions & 0 deletions tests/Microsoft.Identity.Test.Common/Core/Mocks/TokenCacheHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ internal static void PopulateCache(
clientInfo,
homeAccId,
null,
null,
displayableId,
utid,
null,
Expand Down Expand Up @@ -298,6 +299,7 @@ internal static (MsalAccessTokenCacheItem AT, MsalRefreshTokenCacheItem RT, Msal
homeAccountId,
null,
null,
null,
TestConstants.Utid,
null,
null,
Expand Down Expand Up @@ -346,6 +348,7 @@ public static void AddAccountToCache(
$"{uid}.{utid}",
null,
null,
null,
utid,
null,
null,
Expand Down
48 changes: 48 additions & 0 deletions ...icrosoft.Identity.Test.Integration.netcore/SeleniumTests/DeviceCodeFlowIntegrationTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Identity.Client;
using Microsoft.Identity.Client.Broker;
using Microsoft.Identity.Client.Kerberos;
using Microsoft.Identity.Test.Common;
using Microsoft.Identity.Test.Common.Core.Helpers;
Expand Down Expand Up @@ -43,6 +44,14 @@ public async Task DeviceCodeFlowTestAsync()
await AcquireTokenWithDeviceCodeFlowAsync(labResponse, "aad user").ConfigureAwait(false);
}

[TestMethod]
[Timeout(2 * 60 * 1000)] // 2 min timeout
public async Task SilentTokenAfterDeviceCodeFlowWithBrokerTestAsync()
{
LabResponse labResponse = await LabUserHelper.GetDefaultUserAsync().ConfigureAwait(false);
await AcquireTokenSilentAfterDeviceCodeFlowWithBrokerAsync(labResponse, "aad user").ConfigureAwait(false);
}

[TestMethod]
[Timeout(2 * 60 * 1000)] // 2 min timeout
[TestCategory(TestCategories.Arlington)]
Expand Down Expand Up @@ -114,6 +123,45 @@ private async Task AcquireTokenWithDeviceCodeFlowAsync(LabResponse labResponse,
TestCommon.ValidateNoKerberosTicketFromAuthenticationResult(result);
}

private async Task AcquireTokenSilentAfterDeviceCodeFlowWithBrokerAsync(LabResponse labResponse, string userType)
{
Trace.WriteLine($"Calling AcquireTokenSilentAfterDeviceCodeFlowWithBrokerAsync with {0}", userType);
BrokerOptions options = new BrokerOptions(BrokerOptions.OperatingSystems.Windows);
var builder = PublicClientApplicationBuilder.Create(labResponse.App.AppId).WithTestLogging().WithBroker(options);

switch (labResponse.User.AzureEnvironment)
{
case AzureEnvironment.azureusgovernment:
builder.WithAuthority(labResponse.Lab.Authority + labResponse.Lab.TenantId);
break;
default:
break;
}

var pca = builder.Build();
var userCacheAccess = pca.UserTokenCache.RecordAccess();

var result = await pca.AcquireTokenWithDeviceCode(s_scopes, deviceCodeResult =>
{
SeleniumExtensions.PerformDeviceCodeLogin(deviceCodeResult, labResponse.User, TestContext, false);
return Task.FromResult(0);
}).ExecuteAsync(CancellationToken.None).ConfigureAwait(false);

Trace.WriteLine("Running asserts");

userCacheAccess.AssertAccessCounts(0, 1);
Assert.IsFalse(userCacheAccess.LastAfterAccessNotificationArgs.IsApplicationCache);

Assert.IsNotNull(result);
Assert.IsTrue(result.Account.AccountSource == "device_code_flow");
Assert.IsTrue(!string.IsNullOrEmpty(result.AccessToken));
TestCommon.ValidateNoKerberosTicketFromAuthenticationResult(result);

var silentTokenResult = await pca.AcquireTokenSilent(s_scopes, result.Account).ExecuteAsync(CancellationToken.None).ConfigureAwait(false);
Assert.IsNotNull(silentTokenResult);
Assert.IsTrue(!string.IsNullOrEmpty(silentTokenResult.AccessToken));
}

#region Azure AD Kerberos Feature Tests
[IgnoreOnOneBranch]
[RunOn(TargetFrameworks.NetCore)]
Expand Down
1 change: 1 addition & 0 deletions tests/Microsoft.Identity.Test.Unit/CacheTests/MsalTokenCacheKeysTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public void MsalAccountCacheKey()
"login.microsoftonline.com",
"contoso.com",
"uid.utid",
null,
"localId");

var iOSKey = item.iOSCacheKey;
Expand Down
4 changes: 4 additions & 0 deletions tests/Microsoft.Identity.Test.Unit/CacheTests/UnifiedSchemaValidationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ public void TestSchemaComplianceForAccount_WhenMSSTSResponse_WithAADAccount()
MsalEnvironment,
response.ClientInfo,
homeAccountId,
null,
idToken,
"[email protected]",
AadTenantId,
Expand Down Expand Up @@ -497,6 +498,7 @@ public void TestSchemaComplianceForAccount_WhenMSSTSResponse_WithB2CAccount()
MsalEnvironment,
response.ClientInfo,
homeAccountId,
null,
IdToken.Parse(response.IdToken),
"Missing from the token response",
B2CTenantId,
Expand Down Expand Up @@ -671,6 +673,7 @@ public void TestSchemaComplianceForAccount_WhenMSSTSResponse_WithB2CAccountAndTe
MsalEnvironment,
response.ClientInfo,
homeAccountId,
null,
IdToken.Parse(response.IdToken),
"Missing from the token response",
B2CTenantId,
Expand Down Expand Up @@ -881,6 +884,7 @@ public void TestSchemaComplianceForAccount_WhenMSSTSResponse_WithAADAccountAndFo
MsalEnvironment,
response.ClientInfo,
homeAccountId,
null,
IdToken.Parse(response.IdToken),
"[email protected]",
AadTenantId,
Expand Down
1 change: 1 addition & 0 deletions tests/Microsoft.Identity.Test.Unit/PublicApiTests/AccountTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ public void TestGetAccounts()
homeAccountId2,
null,
null,
null,
"uTId1",
null,
null,
Expand Down
2 changes: 1 addition & 1 deletion tests/devapps/DesktopTestApp/MainForm.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace DesktopTestApp
{
public partial class MainForm : Form
{
private const string PublicClientId = "6afec070-b576-4a2f-8d95-41f317b28e06";
private const string PublicClientId = "4b0db8c2-9f26-4417-8bde-3f0e3656f8e0";
private string _b2CClientId = "e3b9ad76-9763-4827-b088-80c7a7888f79";
public const string B2CCustomDomainClientId = "64a88201-6bbd-49f5-ab46-9153798493fd ";
private const string _ciamAuthority = "https://msidlabciam1.ciamlogin.com";
Expand Down
Loading

0 comments on commit 574f2e6

Please sign in to comment.