Add openId scope to all requests via ATHandlerBase [do not merge before discussing]

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Core/Cache/AdalTokenCacheKey.cs

@@ -57,18 +57,18 @@ internal enum TokenSubjectType
     internal sealed class AdalTokenCacheKey
     {
         internal AdalTokenCacheKey(string authority, string resource, string clientId, TokenSubjectType tokenSubjectType, AdalUserInfo adalUserInfo)
-            : this(authority, resource, clientId, tokenSubjectType, (adalUserInfo != null) ? adalUserInfo.UniqueId : null, (adalUserInfo != null) ? adalUserInfo.DisplayableId : null)
+            : this(authority, resource, clientId, tokenSubjectType, adalUserInfo?.UniqueId, (adalUserInfo != null) ? adalUserInfo?.DisplayableId : null)
         {
         }
 
         internal AdalTokenCacheKey(string authority, string resource, string clientId, TokenSubjectType tokenSubjectType, string uniqueId, string displayableId)
         {
-            this.Authority = authority;
-            this.Resource = resource;
-            this.ClientId = clientId;
-            this.TokenSubjectType = tokenSubjectType;
-            this.UniqueId = uniqueId;
-            this.DisplayableId = displayableId;
+            Authority = authority;
+            Resource = resource;
+            ClientId = clientId;
+            TokenSubjectType = tokenSubjectType;
+            UniqueId = uniqueId;
+            DisplayableId = displayableId;
         }
 
         public string Authority { get; }
@@ -93,7 +93,7 @@ internal AdalTokenCacheKey(string authority, string resource, string clientId, T
         public override bool Equals(object obj)
         {
             AdalTokenCacheKey other = obj as AdalTokenCacheKey;
-            return (other != null) && this.Equals(other);
+            return (other != null) && Equals(other);
         }
 
         /// <summary>
@@ -111,12 +111,12 @@ public bool Equals(AdalTokenCacheKey other)
             }
 
             return other != null
-                && other.Authority == this.Authority
-                && this.ResourceEquals(other.Resource)
-                && this.ClientIdEquals(other.ClientId)
-                && other.UniqueId == this.UniqueId
-                && this.DisplayableIdEquals(other.DisplayableId)
-                && other.TokenSubjectType == this.TokenSubjectType;
+                && other.Authority == Authority
+                && ResourceEquals(other.Resource)
+                && ClientIdEquals(other.ClientId)
+                && other.UniqueId == UniqueId
+                && DisplayableIdEquals(other.DisplayableId)
+                && other.TokenSubjectType == TokenSubjectType;
         }
 
         /// <summary>
@@ -128,28 +128,28 @@ public bool Equals(AdalTokenCacheKey other)
         public override int GetHashCode()
         {
             const string delimiter = ":::";
-            var hashString = this.Authority + delimiter
-                           + this.Resource.ToLowerInvariant() + delimiter
-                           + this.ClientId.ToLowerInvariant() + delimiter
-                           + this.UniqueId + delimiter
-                           + this.DisplayableId?.ToLowerInvariant() + delimiter
-                           + (int) this.TokenSubjectType;
+            var hashString = Authority + delimiter
+                           + Resource.ToLowerInvariant() + delimiter
+                           + ClientId.ToLowerInvariant() + delimiter
+                           + UniqueId + delimiter
+                           + DisplayableId?.ToLowerInvariant() + delimiter
+                           + (int) TokenSubjectType;
             return hashString.GetHashCode();
         }
 
         internal bool ResourceEquals(string otherResource)
         {
-            return (string.Compare(otherResource, this.Resource, StringComparison.OrdinalIgnoreCase) == 0);
+            return (string.Compare(otherResource, Resource, StringComparison.OrdinalIgnoreCase) == 0);
         }
 
         internal bool ClientIdEquals(string otherClientId)
         {
-            return (string.Compare(otherClientId, this.ClientId, StringComparison.OrdinalIgnoreCase) == 0);
+            return (string.Compare(otherClientId, ClientId, StringComparison.OrdinalIgnoreCase) == 0);
         }
 
         internal bool DisplayableIdEquals(string otherDisplayableId)
         {
-            return (string.Compare(otherDisplayableId, this.DisplayableId, StringComparison.OrdinalIgnoreCase) == 0);
+            return (string.Compare(otherDisplayableId, DisplayableId, StringComparison.OrdinalIgnoreCase) == 0);
         }
 
         private string DebuggerDisplay

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenByAuthorizationCodeHandler.cs

@@ -65,34 +65,34 @@ public AcquireTokenByAuthorizationCodeHandler(
 
             this.redirectUri = redirectUri;
 
-            this.LoadFromCache = false;
+            LoadFromCache = false;
 
-            this.SupportADFS = true;
+            SupportADFS = true;
         }
 
         protected override void AddAdditionalRequestParameters(DictionaryRequestParameters requestParameters)
         {
             requestParameters[OAuthParameter.GrantType] = OAuthGrantType.AuthorizationCode;
-            requestParameters[OAuthParameter.Code] = this.authorizationCode;
-            requestParameters[OAuthParameter.RedirectUri] = this.redirectUri.OriginalString;
+            requestParameters[OAuthParameter.Code] = authorizationCode;
+            requestParameters[OAuthParameter.RedirectUri] = redirectUri.OriginalString;
         }
 
         protected override async Task PostTokenRequestAsync(AdalResultWrapper resultEx)
         {
             await base.PostTokenRequestAsync(resultEx).ConfigureAwait(false);
             AdalUserInfo adalUserInfo = resultEx.Result.UserInfo;
-            this.UniqueId = (adalUserInfo == null) ? null : adalUserInfo.UniqueId;
-            this.DisplayableId = (adalUserInfo == null) ? null : adalUserInfo.DisplayableId;
+            UniqueId = (adalUserInfo == null) ? null : adalUserInfo.UniqueId;
+            DisplayableId = (adalUserInfo == null) ? null : adalUserInfo.DisplayableId;
             if (resultEx.ResourceInResponse != null)
             {
-                this.Resource = resultEx.ResourceInResponse;
+                Resource = resultEx.ResourceInResponse;
                 RequestContext.Logger.Verbose("Resource value in the token response was used for storing tokens in the cache");
             }
 
             // If resource is not passed as an argument and is not returned by STS either, 
             // we cannot store the token in the cache with null resource.
             // TODO: Store refresh token though if STS supports MRRT.
-            this.StoreToCache = this.StoreToCache && (this.Resource != null);
+            StoreToCache = StoreToCache && (Resource != null);
         }
     }
 }

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenHandlerBase.cs

@@ -326,7 +326,8 @@ protected virtual async Task PostTokenRequestAsync(AdalResultWrapper resultEx)
         {
             var requestParameters = new DictionaryRequestParameters(Resource, ClientKey)
             {
-                { OAuthParameter.ClientInfo, "1" }
+                { OAuthParameter.ClientInfo, "1" },
+                { OAuthParameter.Scope, OAuthValue.ScopeOpenId }
             };
             AddAdditionalRequestParameters(requestParameters);
             return await SendHttpMessageAsync(requestParameters).ConfigureAwait(false);

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenOnBehalfHandler.cs

@@ -73,7 +73,6 @@ protected override void AddAdditionalRequestParameters(DictionaryRequestParamete
             requestParameters[OAuthParameter.GrantType] = OAuthGrantType.JwtBearer;
             requestParameters[OAuthParameter.Assertion] = _userAssertion.Assertion;
             requestParameters[OAuthParameter.RequestedTokenUse] = OAuthRequestedTokenUse.OnBehalfOf;
-            requestParameters[OAuthParameter.Scope] = OAuthValue.ScopeOpenId;
         }
     }
 }

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/NonInteractive/AcquireTokenIWAHandler.cs

@@ -118,9 +118,6 @@ protected override void AddAdditionalRequestParameters(DictionaryRequestParamete
                 requestParameters[OAuthParameter.Assertion] =
                     Convert.ToBase64String(Encoding.UTF8.GetBytes(_userAssertion.Assertion));
             }
-
-            // To request id_token in response
-            requestParameters[OAuthParameter.Scope] = OAuthValue.ScopeOpenId;
         }
     }
 }

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/NonInteractive/AcquireTokenUserAssertionHandler.cs

@@ -71,10 +71,6 @@ protected override void AddAdditionalRequestParameters(DictionaryRequestParamete
         {
             requestParameters[OAuthParameter.GrantType] = _userAssertion.AssertionType;
             requestParameters[OAuthParameter.Assertion] = Convert.ToBase64String(Encoding.UTF8.GetBytes(_userAssertion.Assertion));
-
-
-            // To request id_token in response
-            requestParameters[OAuthParameter.Scope] = OAuthValue.ScopeOpenId;
         }
     }
 }

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/NonInteractive/AcquireTokenUsernamePasswordHandler.cs

@@ -116,9 +116,6 @@ protected override void AddAdditionalRequestParameters(DictionaryRequestParamete
                 requestParameters[OAuthParameter.Username] = _userPasswordInput.UserName;
                 requestParameters[OAuthParameter.Password] = new string(_userPasswordInput.PasswordToCharArray());
             }
-
-            // To request id_token in response
-            requestParameters[OAuthParameter.Scope] = OAuthValue.ScopeOpenId;
         }
     }
 }

src/Microsoft.IdentityModel.Clients.ActiveDirectory/TokenCache.cs

@@ -690,8 +690,8 @@ internal void StoreToCacheCommon(AdalResultWrapper result, string authority, str
                         IdToken.TryParse(result.Result.IdToken, out IdToken idToken);
 
                         CacheFallbackOperations.WriteMsalRefreshToken(TokenCacheAccessor, result, authority, clientId, displayableId,
-                            result.Result.UserInfo.GivenName,
-                            result.Result.UserInfo.FamilyName,
+                            result.Result.UserInfo?.GivenName,
+                            result.Result.UserInfo?.FamilyName,
                             idToken?.GetUniqueId());
                     }
                 }

tests/Test.ADAL.NET.Unit.net45/TokenCacheUnitTests.cs

@@ -36,6 +36,8 @@
 using Microsoft.Identity.Core;
 using Microsoft.Identity.Core.Cache;
 using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Cache;
+using System;
+using Test.ADAL.NET.Common.Mocks;
 
 namespace Test.ADAL.NET.Unit
 {
@@ -186,11 +188,72 @@ public void ParallelStoreTest()
         }
 
         [TestMethod]
-        [Description("Test to ensure the token cache doesnt throw an exception when cleared")]
+        [Description("Test to ensure the token cache doesn't throw an exception when cleared")]
         [TestCategory("AdalDotNet.Unit")]
         public void TokenCacheClearTest()
         {
             TokenCacheTests.TokenCacheClearTest(File.ReadAllBytes("oldcache.serialized"));
         }
+
+        [TestMethod]
+        [Description("Test to ensure a null IdToken is handled correctly")]
+        [TestCategory("AdalDotNet.Unit")]
+        public void NullIdTokenCacheTest()
+        {
+            // arrange & act
+            TokenCache tokenCache = new TokenCache();
+
+            WriteMsalRefreshToken(CreateAdalResultWrapper(), tokenCache.TokenCacheAccessor);
+
+            // assert
+            // no IdToken present, CacheFallbackOperations exits early
+            Assert.AreEqual(tokenCache.TokenCacheAccessor.AccountCount, 0);
+            Assert.AreEqual(tokenCache.TokenCacheAccessor.RefreshTokenCount, 0);
+        }
+
+        [TestMethod]
+        [Description("Test to ensure WriteMsalRefreshToken handles presence of IdToken correctly")]
+        [TestCategory("AdalDotNet.Unit")]
+        public void IdTokenReturnedCacheTest()
+        {
+            // arrange
+            TokenCache tokenCache = new TokenCache();
+
+            var result = CreateAdalResultWrapper();
+
+            result.Result.IdToken = "some-id-token";
+
+            // act
+            WriteMsalRefreshToken(result, tokenCache.TokenCacheAccessor);
+
+            // assert
+            // IdToken present
+            Assert.AreEqual(tokenCache.TokenCacheAccessor.AccountCount, 1);
+            Assert.AreEqual(tokenCache.TokenCacheAccessor.RefreshTokenCount, 1);
+        }
+
+        private AdalResultWrapper CreateAdalResultWrapper()
+        {
+            return new AdalResultWrapper
+            {
+                RefreshToken = "some-rt",
+                RawClientInfo = MockHelpers.CreateClientInfo(AdalTestConstants.DefaultUniqueId, AdalTestConstants.DefaultUniqueTenantIdentifier),
+                ResourceInResponse = AdalTestConstants.DefaultResource,
+                Result = new AdalResult("Bearer", "some-access-token", DateTimeOffset.UtcNow, (DateTimeOffset.UtcNow + TimeSpan.FromMinutes(180)))
+            };
+        }
+
+        private void WriteMsalRefreshToken(AdalResultWrapper result, ITokenCacheAccessor tokenCacheAccessor)
+        {
+            CacheFallbackOperations.WriteMsalRefreshToken(
+                tokenCacheAccessor,
+                result,
+                AdalTestConstants.DefaultAuthorityCommonTenant,
+                AdalTestConstants.DefaultClientId,
+                AdalTestConstants.DefaultDisplayableId,
+                null,
+                null,
+                AdalTestConstants.DefaultUniqueId);
+        }
     }
 }