diff --git a/src/client/Microsoft.Identity.Client/AuthenticationResult.cs b/src/client/Microsoft.Identity.Client/AuthenticationResult.cs index d531e19e23..fa984fb9f4 100644 --- a/src/client/Microsoft.Identity.Client/AuthenticationResult.cs +++ b/src/client/Microsoft.Identity.Client/AuthenticationResult.cs @@ -211,9 +211,16 @@ internal AuthenticationResult() { } public bool IsExtendedLifeTimeToken { get; } /// - /// Gets the Unique Id of the account. It can be null. When the is not null, this is its ID, that - /// is its ObjectId claim, or if that claim is null, the Subject claim. + /// Gets the Unique Id of the account in this + /// It is set as the oid (ObjectId) claim, or if that claim is null, as the sub (Subject) claim which is guaranteed not-null. /// + /// + /// The oid claim identifies a user in all apps - Microsoft Identity Providers issue ID tokens with this claim, although it can be null in rare cases. + /// The sub claim is "a locally unique and never reassigned identifier within the Issuer for the End-User" as per https://openid.net/specs/openid-connect-core-1_0.html and it is a + /// mandatory claim with OIDC compliant issuers. + /// Guest AAD accounts have different oid claim values in each tenant. Use to uniquely identify users across tenants. + /// See https://docs.microsoft.com/azure/active-directory/develop/id-tokens#payload-claims + /// public string UniqueId { get; } /// diff --git a/src/client/Microsoft.Identity.Client/Cache/Items/MsalAccountCacheItem.cs b/src/client/Microsoft.Identity.Client/Cache/Items/MsalAccountCacheItem.cs index 0c0128e40b..225e34f78c 100644 --- a/src/client/Microsoft.Identity.Client/Cache/Items/MsalAccountCacheItem.cs +++ b/src/client/Microsoft.Identity.Client/Cache/Items/MsalAccountCacheItem.cs @@ -56,7 +56,7 @@ internal MsalAccountCacheItem( { Init( preferredCacheEnv, - idToken?.ObjectId, + idToken?.GetUniqueId(), clientInfo, homeAccountId, idToken?.Name, diff --git a/src/client/Microsoft.Identity.Client/Internal/IdToken.cs b/src/client/Microsoft.Identity.Client/Internal/IdToken.cs index 1fef82e00a..bf0cffcfee 100644 --- a/src/client/Microsoft.Identity.Client/Internal/IdToken.cs +++ b/src/client/Microsoft.Identity.Client/Internal/IdToken.cs @@ -41,8 +41,21 @@ internal class IdToken { private const string DefaultIssuser = "LOCAL AUTHORITY"; + /// + /// The OID claim is a unique identifier (GUID) for the user object in Azure AD. + /// Guest Users have different OID. + /// This is a stable ID across all apps. + /// + /// IMPORTANT: There are rare cases where this is missing! + /// + /// + /// Avoid using as it is not guaranteed non-null. Use instead. + /// public string ObjectId { get; private set; } + /// + /// The sub claim is a unique identifier for user + app. + /// public string Subject { get; private set; } public string TenantId { get; private set; } diff --git a/src/client/Microsoft.Identity.Client/TenantProfile.cs b/src/client/Microsoft.Identity.Client/TenantProfile.cs index eac8ca0433..cab2aedc71 100644 --- a/src/client/Microsoft.Identity.Client/TenantProfile.cs +++ b/src/client/Microsoft.Identity.Client/TenantProfile.cs @@ -26,6 +26,7 @@ internal TenantProfile(MsalIdTokenCacheItem msalIdTokenCacheItem) /// This ID uniquely identifies the user across applications - two different applications signing in the same user will receive the same value in the oid claim. /// The user will have a different object ID in each tenant - they're considered different accounts, even though the user logs into each account with the same credentials. /// + /// This claim is issued by Microsoft Identity Providers and can be null. Fallback to the sub claim, which is scoped to a user and an app. public string Oid => _msalIdTokenCacheItem?.IdToken.ObjectId; /// diff --git a/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs b/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs index e3d9341c93..5f87b884b9 100644 --- a/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs +++ b/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs @@ -358,7 +358,7 @@ private void SaveToLegacyAdalCache( InstanceDiscoveryMetadataEntry instanceDiscoveryMetadata) { if (msalRefreshTokenCacheItem?.RawClientInfo != null && - msalIdTokenCacheItem?.IdToken?.ObjectId != null && + msalIdTokenCacheItem?.IdToken?.GetUniqueId() != null && IsLegacyAdalCacheEnabled(requestParams)) { @@ -373,7 +373,7 @@ private void SaveToLegacyAdalCache( msalRefreshTokenCacheItem, msalIdTokenCacheItem, authorityWithPreferredCache.AuthorityInfo.CanonicalAuthority.ToString(), - msalIdTokenCacheItem.IdToken.ObjectId, + msalIdTokenCacheItem.IdToken.GetUniqueId(), response.Scope); } else diff --git a/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHelpers.cs b/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHelpers.cs index 3507100683..d70b7df29b 100644 --- a/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHelpers.cs +++ b/tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHelpers.cs @@ -45,6 +45,18 @@ public static string GetFociTokenResponse() "\",\"id_token_expires_in\":\"3600\"}"; } + public static string GetTokenResponseWithNoOidClaim() + { + return "{\"token_type\": \"Bearer\"," + + "\"scope\": \"https://management.core.windows.net//user_impersonation https://management.core.windows.net//.default\"," + + "\"expires_in\": 4771," + + "\"ext_expires_in\": 4771," + + "\"access_token\": \"secret\"," + + "\"refresh_token\": \"secret\"," + + "\"id_token\": \"eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6InVFQnBBVkVBTVBaUVpGS2lJRFVWdjFEdFRkayJ9.eyJhdWQiOiI0OTliODRhYy0xMzIxLTQyN2YtYWExNy0yNjdjYTY5NzU3OTgiLCJpc3MiOiJodHRwczovL2xvZ2luLndpbmRvd3MtcHBlLm5ldC85OGVjYjBlZi1iYjhkLTQyMTYtYjQ1YS03MGRmOTUwZGM2ZTMvdjIuMCIsImlhdCI6MTY4NDMxMzA1NCwibmJmIjoxNjg0MzEzMDU0LCJleHAiOjE2ODQzMTY5NTQsImFhaSI6InRlbmFudDogODExOTg5MGItNGMzZi00NjVkLTk4NDAtNTk5MTMxZDE0ZDk4LCBvYmplY3Q6IDRlNjJhMmI0LTBiZTYtNDI0Yi1hMDg0LWYyZmYwOTIxOGUyMyIsImF1dGhfdGltZSI6MTY4NDMxMzMzOCwiaG9tZV9vaWQiOiI0ZTYyYTJiNC0wYmU2LTQyNGItYTA4NC1mMmZmMDkyMThlMjMiLCJob21lX3B1aWQiOiIxMDAzREZGRDAwOURGODQ3IiwiaG9tZV90aWQiOiI4MTE5ODkwYi00YzNmLTQ2NWQtOTg0MC01OTkxMzFkMTRkOTgiLCJpZHAiOiJodHRwczovL3N0cy53aW5kb3dzLXBwZS5uZXQvODExOTg5MGItNGMzZi00NjVkLTk4NDAtNTk5MTMxZDE0ZDk4LyIsIm5hbWUiOiJ0ZXN0X3Rlc3RfY3NwXzAyMiBUZWNobmljaWFuIiwibm9uY2UiOiIzMGJiYTU0Ny05MjViLTQxZWItOTFiYy05YTM1YTY3M2JmZDkiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJ1c2VyXzRlNjJhMmI0MGJlNjQyNGJhMDg0ZjJmZjA5MjE4ZTIzQHN1YmdkYXAuYWxpbmMueHl6IiwicmgiOiIwLkFBRUE3N0RzbUkyN0ZrSzBXbkRmbFEzRzQ2eUVtMGtoRTM5Q3FoY21mS2FYVjVnQkFKUS4iLCJzdWIiOiJBdWpMRFFwNXlSTVJjR3BQY0RCZnQ5TmI1dUZTS1l4RFpxNjUtZWJmSGxzIiwidGlkIjoiOThlY2IwZWYtYmI4ZC00MjE2LWI0NWEtNzBkZjk1MGRjNmUzIiwidXRpIjoiODZyeGRJTlhiMDJMY3RIMDltVW1BQSIsInZlciI6IjIuMCIsIndpZHMiOlsiODhkOGUzZTMtOGY1NS00YTFlLTk1M2EtOWI5ODk4Yjg4NzZiIiwiMDgzNzJiODctN2QwMi00ODJhLTllMDItZmIwM2VhNWZlMTkzIl0sInhtc19tcGNpIjoyNTkyMDAsInhtc19wY2kiOjM2MDAsInhtc193c2l0IjoxODAwfQ.no_signature\"," + + "\"client_info\": \"eyJ1aWQiOiI0ZTYyYTJiNC0wYmU2LTQyNGItYTA4NC1mMmZmMDkyMThlMjMiLCJ1dGlkIjoiODExOTg5MGItNGMzZi00NjVkLTk4NDAtNTk5MTMxZDE0ZDk4In0\"}"; + } + public static readonly string DefaultEmtpyFailureErrorMessage = "{\"the-error-is-not-here\":\"erorwithouterrorfield\",\"error_description\":\"AADSTS991: " + "This is an error message which doesn't contain the error field. " + diff --git a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs index 52ae1d60ea..dfb446f07c 100644 --- a/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs +++ b/tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs @@ -1037,6 +1037,53 @@ await app.AcquireTokenByAuthorizationCode(TestConstants.s_scope, TestConstants.D } } + [TestMethod] + // regression test for https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/4140 + public async Task IdTokenHasNoOid_ADALSerialization_Async() + { + // Arrange + using (var httpManager = new MockHttpManager()) + { + httpManager.AddInstanceDiscoveryMockHandler("https://login.windows-ppe.net/98ecb0ef-bb8d-4216-b45a-70df950dc6e3/"); + + var app = ConfidentialClientApplicationBuilder.Create(TestConstants.ClientId) + .WithAuthority("https://login.windows-ppe.net/98ecb0ef-bb8d-4216-b45a-70df950dc6e3/") + .WithClientSecret(TestConstants.ClientSecret) + .WithHttpManager(httpManager) + .Build(); + + byte[] tokenCacheInAdalFormat = null; + app.UserTokenCache.SetAfterAccess( + (args) => + { + if (args.HasStateChanged) + { + tokenCacheInAdalFormat = args.TokenCache.SerializeAdalV3(); + }; + }); + + var handler = httpManager.AddMockHandler( + new MockHttpMessageHandler() + { + ExpectedMethod = HttpMethod.Post, + ResponseMessage = MockHelpers.CreateSuccessResponseMessage(MockHelpers.GetTokenResponseWithNoOidClaim()) + }); + + + // Act + var result = await app.AcquireTokenByAuthorizationCode(new[] { "https://management.core.windows.net//.default" }, "code") + .ExecuteAsync() + .ConfigureAwait(false); + + // Assert + Assert.IsNotNull(tokenCacheInAdalFormat); + + Assert.AreEqual("AujLDQp5yRMRcGpPcDBft9Nb5uFSKYxDZq65-ebfHls", result.UniqueId); + Assert.AreEqual("AujLDQp5yRMRcGpPcDBft9Nb5uFSKYxDZq65-ebfHls", result.ClaimsPrincipal.FindFirst("sub").Value); + Assert.IsNull(result.ClaimsPrincipal.FindFirst("oid")); + } + } + [DataTestMethod] [DataRow(true)] [DataRow(false)] @@ -1805,7 +1852,7 @@ public async Task ValidateAppTokenProviderAsync() .BuildConcrete(); // AcquireToken from app provider - AuthenticationResult result = await app.AcquireTokenForClient(TestConstants.s_scope) + AuthenticationResult result = await app.AcquireTokenForClient(TestConstants.s_scope) .ExecuteAsync(new CancellationToken()).ConfigureAwait(false); Assert.IsNotNull(result.AccessToken); @@ -1865,7 +1912,7 @@ public async Task ValidateAppTokenProviderAsync() Assert.AreEqual(4, callbackInvoked); } - + private AppTokenProviderResult GetAppTokenProviderResult(string differentScopesForAt = "", long? refreshIn = 1000) {