Use sub claim if oid is missing (#4141)

* Use sub claim if oid is missing

* tag

---------

Co-authored-by: Gladwin Johnson <90415114+gladjohn@users.noreply.github.com>

src/client/Microsoft.Identity.Client/AuthenticationResult.cs

@@ -211,9 +211,16 @@ internal AuthenticationResult() { }
         public bool IsExtendedLifeTimeToken { get; }
 
         /// <summary>
-        /// Gets the Unique Id of the account. It can be null. When the <see cref="IdToken"/> is not <c>null</c>, this is its ID, that
-        /// is its ObjectId claim, or if that claim is <c>null</c>, the Subject claim.
+        /// Gets the Unique Id of the account in this <see cref="TenantId" />
+        /// It is set as the oid (ObjectId) claim, or if that claim is <c>null</c>, as the sub (Subject) claim which is guaranteed not-null.
         /// </summary>
+        /// <remarks>
+        /// 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 <see cref="Account.HomeAccountId"/> to uniquely identify users across tenants.
+        /// See https://docs.microsoft.com/azure/active-directory/develop/id-tokens#payload-claims
+        /// </remarks>
         public string UniqueId { get; }
 
         /// <summary>

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,

src/client/Microsoft.Identity.Client/Internal/IdToken.cs

@@ -41,8 +41,21 @@ internal class IdToken
     {
         private const string DefaultIssuser = "LOCAL AUTHORITY";
 
+        /// <summary>
+        /// 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! 
+        /// </summary>
+        /// <remarks>
+        /// Avoid using as it is not guaranteed non-null. Use <see cref="GetUniqueId"/> instead.
+        /// </remarks>
         public string ObjectId { get; private set; }
 
+        /// <summary>
+        /// The sub claim is a unique identifier for user + app. 
+        /// </summary>
         public string Subject { get; private set; }
 
         public string TenantId { get; private set; }

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. 
         /// </summary>
+        /// <remarks>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.</remarks>
         public string Oid => _msalIdTokenCacheItem?.IdToken.ObjectId;
 
         /// <summary>

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

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. " +

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)
         {