Skip to content
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

Graph SDK is not working together with Microsoft.AspNetCore.Identity.EntityFrameworkCore #1140

Closed
wmmihaa opened this issue Sep 23, 2021 · 15 comments
Assignees

Comments

@wmmihaa
Copy link

wmmihaa commented Sep 23, 2021

Describe the bug
I'm trying to use the Graph API to search for users on behalf of the logged in user (User.Read.All).

I can create a new ASP.Net 5 MVC project and get everything working with the Graph SDK (Microsoft.Identity.Web.MicrosoftGraphExtensions), but when trying to add the same configuration and logic to an existing ASP.Net 5 MVC application it fails. The difference between these applications is that the existing application is using Microsoft.AspNetCore.Identity.EntityFrameworkCore to persist user information.

appsettings.json:

{
 "AzureAd": {
    
    "Instance": "https://login.microsoftonline.com/",
    "Domain": "XXX",
    "TenantId": "...",
    "ClientId": "...",
    "ClientSecret": "...",
    "CallbackPath": "/signin-oidc/XXX",
    "ClientCapabilities": [ "cp1" ]
  },
  "DownstreamApi": {
    "BaseUrl": "https://graph.microsoft.com/v1.0",
    "Scopes": "user.read user.read.all"
  },
}

Startup.cs (ConfiigureServices)

services.AddSingleton<IConfiguration>(Configuration);
services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();

services.AddDbContext<GreenEdgeDbContext>(options =>
    options.UseSqlServer(
        Configuration.GetConnectionString("DefaultConnection")));

services.AddIdentity<ApplicationUser, IdentityRole>()
    .AddEntityFrameworkStores<GreenEdgeDbContext>();

services.AddTransient<IEmailSender, EmailSender>();

services.ConfigureApplicationCookie(options =>
{
    // Cookie settings
    options.Cookie.HttpOnly = true;
    options.ExpireTimeSpan = TimeSpan.FromMinutes(30);

    options.LoginPath = "/Identity/Account/Login";
    options.AccessDeniedPath = "/Identity/Account/AccessDenied";
    options.SlidingExpiration = true;
});

string[] initialScopes = Configuration.GetValue<string>("DownstreamApi:Scopes")?.Split(' ');

services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
    .AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd"))
    .EnableTokenAcquisitionToCallDownstreamApi(initialScopes)
    .AddMicrosoftGraph(Configuration.GetSection("DownstreamApi"))
    .AddInMemoryTokenCaches();

services.AddControllersWithViews();
services.AddRazorPages();

As users login they get redirected to AAD from the ExternalLoginModel::OnPost (Skaffolding) and after successful signin redirected back to OnGetCallbackAsync.

As the user reach the controller, we try to search for users using the Graph API:

[RequireHttps]
[Authorize]
public class AdminController : BaseController
{
    private readonly ILogger<AdminController> _logger;
    private readonly ICosmosDBHelper _cosmosDBHelper;
    private readonly MicroServiceBusHelper _microServiceBusHelper;
    private readonly GraphServiceClient _graphServiceClient;

    public AdminController(GreenEdgeDbContext context, 
        ILogger<AdminController> logger, IConfiguration configuration, 
        ICosmosDBHelper cosmosDBHelper, 
        MicroServiceBusHelper microServiceBusHelper,
        GraphServiceClient graphServiceClient) :base(context, configuration)
    {
        _logger = logger;
        _cosmosDBHelper = cosmosDBHelper;
        _microServiceBusHelper = microServiceBusHelper;
        _graphServiceClient = graphServiceClient;
    }
    public async Task<IActionResult> Index()
    {
       // This is where it fails
        var users = await _graphServiceClient.Users
                .Request()
                .Filter("displayName eq 'HAKANSSON Mikael'")
                .GetAsync();

        return View();
    }
...
}

This fails with:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Identity.Web.MergedOptions.PrepareAuthorityInstanceForMsal()
   at Microsoft.Identity.Web.TokenAcquisition.BuildConfidentialClientApplication(MergedOptions mergedOptions)
   at Microsoft.Identity.Web.TokenAcquisition.GetOrBuildConfidentialClientApplication(MergedOptions mergedOptions)
   at Microsoft.Identity.Web.TokenAcquisition.GetAuthenticationResultForUserAsync(IEnumerable`1 scopes, String authenticationScheme, String tenantId, String userFlow, ClaimsPrincipal user, TokenAcquisitionOptions tokenAcquisitionOptions)
   at Microsoft.Identity.Web.TokenAcquisitionAuthenticationProvider.AuthenticateRequestAsync(HttpRequestMessage request)
   at Microsoft.Graph.AuthenticationHandler.SendAsync(HttpRequestMessage httpRequestMessage, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.SendAsyncCore(HttpRequestMessage request, HttpCompletionOption completionOption, Boolean async, Boolean emitTelemetryStartStop, CancellationToken cancellationToken)
   at Microsoft.Graph.HttpProvider.SendRequestAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)

Desktop (please complete the following information):

  • OS: [e.g. Windows 10]
  • Browser [chrome, safari & firefox]
@ghost ghost added the Needs: Triage label Sep 23, 2021
@andrueastman
Copy link
Member

Hey @wmmihaa,

Thanks for raising this issue. Are you able to provide more information on the version of the SDK you are using?

@wmmihaa
Copy link
Author

wmmihaa commented Sep 23, 2021

Are you able to provide more information on the version of the SDK you are using?

My apologies @andrueastman ...
Microsoft.AspNetCore.Authentication.OpenIdConnect - 5.0.10
Microsoft.AspNetCore.Authentication.WsFederation - 5.0.10
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore - 5.0.10
Microsoft.Identity.Web - 1.17.0
Microsoft.Identity.Web.UI - 1.17.0
Microsoft.Identity.Web.MicrosoftGraph - 1.17.0

Same in both projects...

@andrueastman
Copy link
Member

Hey @wmmihaa,

Taking a look at the configuration of the sample app here, the configuration is set up as

            services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApp(Configuration)/// This line is different for you
                .EnableTokenAcquisitionToCallDownstreamApi(initialScopes)
                .AddMicrosoftGraph(Configuration.GetSection("DownstreamApi"))
                .AddInMemoryTokenCaches();

From the exception trace it seems that some configuration value is not setup correctly causing the error. Any chance the suggestion above works out?

@wmmihaa
Copy link
Author

wmmihaa commented Sep 24, 2021

No Configuration is just a configuration section. The code is the same.

@andrueastman
Copy link
Member

The reason I ask is that from the source of the function throwing the error here, the Instance property is probably not loaded correctly from the configuration.

Are you able to verify that this is happening as expected?

@wmmihaa
Copy link
Author

wmmihaa commented Sep 24, 2021

The reason I ask is that from the source of the function throwing the error here, the Instance property is probably not loaded correctly from the configuration.

Are you able to verify that this is happening as expected?

It did not make any difference, and besides the same code works in the new ASP.Net 5 MVC project.

@wmmihaa
Copy link
Author

wmmihaa commented Sep 24, 2021

I've made this video which I hope will be of help: https://youtu.be/luQ75XSsPr8

It might have something to do with the fact that I have to disable cookieScheme, but I don't know.

HTH

@andrueastman
Copy link
Member

Thanks so much for the extra info @wmmihaa.
I will try to replicate the same on my side and feedback to you soon.

@andrueastman
Copy link
Member

Hey @wmmihaa

It seems that using Microsoft.Identity.Web and calling services.AddIdentity<ApplicationUser, IdentityRole>() results to multiple cookie and authentication provider registrations and therefore causes errors. In your case, it seems that the authentication provider you registered isn't the one being picked up at runtime hence the exception.

You should be able to work around this with the suggestion on the link above. Essentially, your code should be something like

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddDbContext<GreenEdgeDbContext>(options =>
                options.UseSqlServer(Configuration.GetValue<string>("DefaultConnection")));
            
            // add the identity like this
            services.AddDefaultIdentity<ApplicationUser>( options =>
            {
                options.SignIn.RequireConfirmedAccount = false;
            })
            .AddDefaultUI()
            .AddEntityFrameworkStores<GreenEdgeDbContext>();

            services.AddControllersWithViews(options =>
            {
                var policy = new AuthorizationPolicyBuilder()
                    .RequireAuthenticatedUser()
                    .Build();
                options.Filters.Add(new AuthorizeFilter(policy));
            });

            services.AddRazorPages()
                  .AddMicrosoftIdentityUI();

            // Add the UI support to handle claims challenges
            services.AddServerSideBlazor()
               .AddMicrosoftIdentityConsentHandler();

            string[] initialScopes = Configuration.GetValue<string>("DownstreamApi:Scopes")?.Split(' ');

            services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApp(Configuration, cookieScheme: null)    // set cookie scheme to null to prevent another
                .EnableTokenAcquisitionToCallDownstreamApi(initialScopes)
                .AddMicrosoftGraph(Configuration.GetSection("DownstreamApi"))
                .AddInMemoryTokenCaches();
        }

A similar issue is documented at the link below
AzureAD/microsoft-identity-web#133

@wmmihaa
Copy link
Author

wmmihaa commented Sep 27, 2021

Thank you @andrueastman for you effort, but your code bypasses the whole scaffolding part such as ExternalLogin... Meaning the ExternalLogin callback is never called.

@wmmihaa
Copy link
Author

wmmihaa commented Sep 28, 2021

@andrueastman Please re-open this issue

@andrueastman
Copy link
Member

Thanks for extra information @wmmihaa. Sorry, I didn't realize that you wished to go that route.

I believe you should be able select the relevant authentication provider registration when making the call to graph by modifying the graph call to be

var users = await _graphServiceClient.Users
                    .Request()
                    .WithAuthenticationScheme(OpenIdConnectDefaults.AuthenticationScheme) // select the auth scheme
                    .Filter("displayName eq 'HAKANSSON Mikael'")
                    .GetAsync();

Any chance this works out for you?

If not, are you able to share a sample project to show how you have configured it with your UserModel?

@wmmihaa
Copy link
Author

wmmihaa commented Sep 28, 2021

I added the WithAuthenticationScheme to the request and now I get a "No account or login hint was passed to the AcquireTokenSilent call." exception:

image

MSAL.NetCore.4.36.0.0.MsalUiRequiredException: 
	ErrorCode: user_null
Microsoft.Identity.Client.MsalUiRequiredException: No account or login hint was passed to the AcquireTokenSilent call. 
   at Microsoft.Identity.Client.Internal.Requests.Silent.SilentRequest.ExecuteAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.Internal.Requests.Silent.SilentRequest.ExecuteAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.RunAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.ApiConfig.Executors.ClientApplicationBaseExecutor.ExecuteAsync(AcquireTokenCommonParameters commonParameters, AcquireTokenSilentParameters silentParameters, CancellationToken cancellationToken)
   at Microsoft.Identity.Web.TokenAcquisition.GetAuthenticationResultForWebAppWithAccountFromCacheAsync(IConfidentialClientApplication application, ClaimsPrincipal claimsPrincipal, IEnumerable`1 scopes, String authority, MergedOptions mergedOptions, String userFlow, TokenAcquisitionOptions tokenAcquisitionOptions)
   at Microsoft.Identity.Web.TokenAcquisition.GetAuthenticationResultForUserAsync(IEnumerable`1 scopes, String authenticationScheme, String tenantId, String userFlow, ClaimsPrincipal user, TokenAcquisitionOptions tokenAcquisitionOptions)
	StatusCode: 0 
	ResponseBody:  
	Headers: 

@andrueastman
Copy link
Member

andrueastman commented Sep 29, 2021

Hey @wmmihaa,

It seems that the generated default scaffolding for ExternalLogin.cshtml.cs does not pass across the all the claims from the SignIn needed for MSAL library to find the user.

You could possibly try to modify the OnGetCallbackAsync function to have something similar to this section of code (after _signInManager.GetExternalLoginInfoAsync() returns a valid result) during the sign in so that the additional claims are included and let me know how it works out for you.

// Sign in the user with this external login provider if the user already has a login.
var result = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: false, bypassTwoFactor : true);
if (result.Succeeded)
{
     // Add these two lines to ensure all the claims are passed accross
    var user = await _signInManager.UserManager.FindByLoginAsync(info.LoginProvider, info.ProviderKey);
    await _signInManager.SignInWithClaimsAsync(user, false, info.Principal.Claims);
    _logger.LogInformation("{Name} logged in with {LoginProvider} provider.", info.Principal.Identity.Name, info.LoginProvider);
    return LocalRedirect(returnUrl);
}

@wmmihaa
Copy link
Author

wmmihaa commented Sep 29, 2021

@andrueastman You are a genius!
This workaround was brilliant. Thank you so much for all the effort!

@wmmihaa wmmihaa closed this as completed Sep 29, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants