diff --git a/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs b/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs index bf166972..1a4bd600 100644 --- a/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs +++ b/Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs @@ -15,10 +15,8 @@ namespace Microsoft.Identity.Web { - // TODO: rename to EnsureScopesAttribute ? or MsalAuthorizeForScopesAttribute or AuthorizeForScopesAttribute - /// - /// Filter used on a controller action to trigger an incremental consent. + /// Filter used on a controller action to trigger incremental consent. /// /// /// The following controller action will trigger @@ -42,12 +40,13 @@ public class AuthorizeForScopesAttribute : ExceptionFilterAttribute public string ScopeKeySection { get; set; } /// - /// Handles the MsaUiRequiredExeception + /// Handles the MsalUiRequiredException /// /// Context provided by ASP.NET Core public override void OnException(ExceptionContext context) { MsalUiRequiredException msalUiRequiredException = context.Exception as MsalUiRequiredException; + if (msalUiRequiredException == null) { msalUiRequiredException = context.Exception?.InnerException as MsalUiRequiredException; @@ -55,8 +54,11 @@ public override void OnException(ExceptionContext context) 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,10 +76,13 @@ 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(ScopeKeySection) }; + scopes = new string[] { configuration.GetValue(ScopeKeySection) }; } - var properties = BuildAuthenticationPropertiesForIncrementalConsent(Scopes, msalUiRequiredException, context.HttpContext); + else + scopes = Scopes; + + var properties = BuildAuthenticationPropertiesForIncrementalConsent(scopes, msalUiRequiredException, context.HttpContext); context.Result = new ChallengeResult(properties); } } @@ -85,7 +90,7 @@ public override void OnException(ExceptionContext context) 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 @@ -93,26 +98,30 @@ private bool CanBeSolvedByReSignInUser(MsalUiRequiredException ex) // 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 }); } /// - /// Build Authentication properties needed for an incremental consent. + /// Build Authentication properties needed for incremental consent. /// /// Scopes to request /// MsalUiRequiredException instance /// current http context in the pipeline /// AuthenticationProperties 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>(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();