Merge pull request #280 from Azure-Samples/tibre/273

Fix for issue 273

Author: Tiago Brenck

Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs

@@ -15,10 +15,8 @@
 
 namespace Microsoft.Identity.Web
 {
-    // TODO: rename to EnsureScopesAttribute ? or MsalAuthorizeForScopesAttribute  or AuthorizeForScopesAttribute
-
     /// <summary>
-    /// Filter used on a controller action to trigger an incremental consent.
+    /// Filter used on a controller action to trigger incremental consent.
     /// </summary>
     /// <example>
     /// The following controller action will trigger
@@ -42,21 +40,25 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute
         public string ScopeKeySection { get; set; }
 
         /// <summary>
-        /// Handles the MsaUiRequiredExeception
+        /// Handles the MsalUiRequiredException
         /// </summary>
         /// <param name="context">Context provided by ASP.NET Core</param>
         public override void OnException(ExceptionContext context)
         {
             MsalUiRequiredException msalUiRequiredException = context.Exception as MsalUiRequiredException;
+
             if (msalUiRequiredException == null)
             {
                 msalUiRequiredException = context.Exception?.InnerException as MsalUiRequiredException;
             }
 
             if (msalUiRequiredException != null)
             {
-                if (CanBeSolvedByReSignInUser(msalUiRequiredException))
+                if (CanBeSolvedByReSignInOfUser(msalUiRequiredException))
                 {
+                    // Do not re-use the attribute param Scopes. For more info: https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/issues/273
+                    string[] scopes = null;
+
                     // the users cannot provide both scopes and ScopeKeySection at the same time
                     if (!string.IsNullOrWhiteSpace(ScopeKeySection) && Scopes != null && Scopes.Length > 0)
                     {
@@ -74,45 +76,52 @@ public override void OnException(ExceptionContext context)
                             throw new InvalidOperationException($"The {nameof(ScopeKeySection)} is provided but the IConfiguration instance is not present in the services collection");
                         }
 
-                        Scopes = new string[] { configuration.GetValue<string>(ScopeKeySection) };
+                        scopes = new string[] { configuration.GetValue<string>(ScopeKeySection) };
                     }
 
-                    var properties = BuildAuthenticationPropertiesForIncrementalConsent(Scopes, msalUiRequiredException, context.HttpContext);
+                    else
+                        scopes = Scopes;
+
+                    var properties = BuildAuthenticationPropertiesForIncrementalConsent(scopes, msalUiRequiredException, context.HttpContext);
                     context.Result = new ChallengeResult(properties);
                 }
             }
 
             base.OnException(context);
         }
 
-        private bool CanBeSolvedByReSignInUser(MsalUiRequiredException ex)
+        private bool CanBeSolvedByReSignInOfUser(MsalUiRequiredException ex)
         {
             // ex.ErrorCode != MsalUiRequiredException.UserNullError indicates a cache problem.
             // When calling an [Authenticate]-decorated controller we expect an authenticated
             // user and therefore its account should be in the cache. However in the case of an
             // InMemoryCache, the cache could be empty if the server was restarted. This is why
             // the null_user exception is thrown.
 
-            return ex.ErrorCode.ContainsAny(new [] { MsalError.UserNullError, MsalError.InvalidGrantError });
+            return ex.ErrorCode.ContainsAny(new[] { MsalError.UserNullError, MsalError.InvalidGrantError });
         }
 
         /// <summary>
-        /// Build Authentication properties needed for an incremental consent.
+        /// Build Authentication properties needed for incremental consent.
         /// </summary>
         /// <param name="scopes">Scopes to request</param>
         /// <param name="ex">MsalUiRequiredException instance</param>
         /// <param name="context">current http context in the pipeline</param>
         /// <returns>AuthenticationProperties</returns>
         private AuthenticationProperties BuildAuthenticationPropertiesForIncrementalConsent(
-            string[] scopes, MsalUiRequiredException ex, HttpContext context)
+            string[] scopes,
+            MsalUiRequiredException ex,
+            HttpContext context)
         {
             var properties = new AuthenticationProperties();
 
-            // Set the scopes, including the scopes that ADAL.NET / MASL.NET need for the Token cache
-            string[] additionalBuildInScopes =
-                {OidcConstants.ScopeOpenId, OidcConstants.ScopeOfflineAccess, OidcConstants.ScopeProfile};
+            // Set the scopes, including the scopes that ADAL.NET / MSAL.NET need for the token cache
+            string[] additionalBuiltInScopes =
+                {OidcConstants.ScopeOpenId,
+                OidcConstants.ScopeOfflineAccess,
+                OidcConstants.ScopeProfile};
             properties.SetParameter<ICollection<string>>(OpenIdConnectParameterNames.Scope,
-                                                         scopes.Union(additionalBuildInScopes).ToList());
+                                                         scopes.Union(additionalBuiltInScopes).ToList());
 
             // Attempts to set the login_hint to avoid the logged-in user to be presented with an account selection dialog
             var loginHint = context.User.GetLoginHint();