Skip to content

Fix for issue 273 #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions Microsoft.Identity.Web/AuthorizeForScopesAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
{
Expand All @@ -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();
Expand Down