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

[Bug] NullReferenceException when using Microsoft Graph (AppOnly) within IHostedService or anonymous web app controllers #1372

Closed
1 of 8 tasks
Trancesphere opened this issue Aug 5, 2021 · 22 comments
Labels
bug Something isn't working
Milestone

Comments

@Trancesphere
Copy link

Which version of Microsoft Identity Web are you using?
1.15.2

Where is the issue?

  • Web app
    • Sign-in users
    • Sign-in users and call web APIs
  • Web API
    • Protected web APIs (validating tokens)
    • Protected web APIs (validating scopes)
    • Protected web APIs call downstream web APIs
  • Token cache serialization
    • In-memory caches
    • Session caches
    • Distributed caches
  • Other (please describe)

Is this a new or an existing app?
The app is in production (using 1.10.0) and I have upgraded to a new version of Microsoft Identity Web.

Repro

Debug Launch is set to Project.
Launch Browser is unticked.

The below code works fine with 1.10.0.
Anything above 1.10.0 (1.11-1.15.2) throws a NullReferenceException.

Startup.cs

        services.AddMicrosoftIdentityWebApiAuthentication(Configuration)
           .EnableTokenAcquisitionToCallDownstreamApi()
           .AddMicrosoftGraphAppOnly(authProvider => new GraphServiceClient(authProvider))
           .AddInMemoryTokenCaches();

Shortened code from IHostedService:

    private async void DoWork(object state)
    {
        using (var scope = Services.CreateScope())
        {
            var graphClient =
                scope.ServiceProvider
                    .GetRequiredService<GraphServiceClient>();

            var users = await graphClient.Users.Request().GetAsync();
        }
    }

Expected behavior
Return the list of users within Azure AD using Graph App Only

Actual behavior

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.GetAuthenticationResultForAppAsync(String scope, String authenticationScheme, String tenant, 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)
--- End of inner exception stack trace ---
at Microsoft.Graph.HttpProvider.SendRequestAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)
at Microsoft.Graph.HttpProvider.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)
at Microsoft.Graph.BaseRequest.SendRequestAsync(Object serializableObject, CancellationToken cancellationToken, HttpCompletionOption completionOption)
at Microsoft.Graph.BaseRequest.SendAsync[T](Object serializableObject, CancellationToken cancellationToken, HttpCompletionOption completionOption)
at Microsoft.Graph.GraphServiceUsersCollectionRequest.GetAsync(CancellationToken cancellationToken)
at TestProject.BackgroundServices.TimedDirectorySynchroniserService.DoWork(Object state) in C:\Users\User\source\repos\TestProject\TestProject\BackgroundServices\TimedDirectorySynchroniserService.cs:line 48

Possible solution

Additional context / logs / screenshots

@jennyf19 jennyf19 added bug Something isn't working investigate labels Aug 5, 2021
@paolovalladolid
Copy link

I am getting the same exception and stack trace. In my case, this is happening in a console app.

Excerpt from Main method of the console app:

var config = new ConfigurationBuilder()
        .AddJsonFile($"appsettings.json", optional: true, reloadOnChange: true).Build();

var builder = new HostBuilder().ConfigureServices((hostContext, services) =>
{
        services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
               .AddMicrosoftIdentityWebApi(config.GetSection("AzureAd"))
                   .EnableTokenAcquisitionToCallDownstreamApi()
                       .AddMicrosoftGraph(config.GetSection("DownstreamApi"))
                       .AddInMemoryTokenCaches();
 }).UseConsoleLifetime();

Method that calls Azure using GraphServiceClient

public async Task<Microsoft.Graph.Application> GetServiceFromAzureAsync(string serviceId)
{
      var filterString = $"tags/any(c:c eq 'ThreescaleServiceId:{serviceId}')";

      var results = await graphClient.Applications.Request()
          .Filter(filterString)
          .GetAsync();
}

@jennyf19
Copy link
Collaborator

@paolovalladolid do you have a working repo or link to code that i can download and run locally?

@paolovalladolid
Copy link

paolovalladolid commented Aug 12, 2021

@paolovalladolid do you have a working repo or link to code that i can download and run locally?

@jennyf19 I have created a repo for a simple console app that demonstrates the issue with using GraphServiceClient with the IoC framework. Plug in your Azure settings into the appsettings.json of course.

https://github.com/paolovalladolid/PVSimpleMSGraphConsole

@jennyf19
Copy link
Collaborator

@paolovalladolid

  1. How do you add the authorization and authentication middleware? (app.UseAuthentication(); and app.UseAuthorization();) ?

    app.UseAuthentication();
    app.UseAuthorization();

  2. If you request an app only scope, you want to apply .WithAppOnly() to the request:

     var results = await graphClient.Applications.Request()
                 .WithAppOnly()
                 .Filter(filterString)
                 .GetAsync();

@jmprieur jmprieur added external and removed bug Something isn't working labels Aug 17, 2021
@paolovalladolid
Copy link

paolovalladolid commented Aug 17, 2021

@jennyf19 I tried to make the suggested code changes but am having a hard time pushing my changes to GitHub because I can't get my new token to work with GitHub. I modified the classes as follows. I still get the same stack trace:

class Program
  {
    public static async Task Main(string[] args)
    {
      Console.WriteLine("Hello World!");

      var services = Startup.ConfigureServices();
      var serviceProvider = services.BuildServiceProvider();

      Console.WriteLine("My MSGraph app configuration loaded. Begin call to Azure...");

      var msapp = await GetServiceFromAzureAsync(serviceProvider);
    }

    public static async Task<Microsoft.Graph.Application> GetServiceFromAzureAsync(IServiceProvider injectedProvider)
    {

      var serviceId =  2555417833004;
      var filterString = $"tags/any(c:c eq 'ThreescaleServiceId:{serviceId}')";

      using IServiceScope serviceScope = injectedProvider.CreateScope();
      IServiceProvider provider = serviceScope.ServiceProvider;

      var graphClient = provider.GetRequiredService<GraphServiceClient>();

      var results = await graphClient.Applications.Request()
          .WithAppOnly()
          .Filter(filterString)
          .GetAsync();

      return results.First();
    }
  }

public static class Startup
  {
    public static IServiceCollection ConfigureServices()
    {
      var services = new ServiceCollection();

      var config = new ConfigurationBuilder()
      .AddJsonFile($"appsettings.json", optional: true, reloadOnChange: true)
      .Build();

      services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
              .AddMicrosoftIdentityWebApi(config.GetSection("AzureAd"))
                  .EnableTokenAcquisitionToCallDownstreamApi()
                      .AddMicrosoftGraph(config.GetSection("DownstreamApi"))
                      .AddInMemoryTokenCaches();

      return services;
    }

    public static void Configure(IApplicationBuilder app, IWebHostEnvironment env)
    {
      app.UseAuthentication();
      app.UseAuthorization();
    }
  }

@ghost
Copy link

ghost commented Aug 20, 2021

Was getting something similar with a hosted service, workaround for me was to use .AddAuthentication(JwtBearerDefaults.AuthenticationScheme) when configuring services and then ensure that IOptionsMonitor<JwtBearerOptions>.Get(JwtBearerDefaults.AuthenticationScheme) is called before token acquisition within the hosted service .

Seems the merged options aren't initialised until the first web request comes in, thereby breaking daemon scenarios?

@jmprieur
Copy link
Collaborator

Thanks @mrfrankbell
I see, so it's not really a protected web API, @paolovalladolid ?

@jmprieur jmprieur added answered question Further information is requested and removed investigate labels Aug 20, 2021
@ghost
Copy link

ghost commented Aug 21, 2021 via email

@MaxxDelusional
Copy link

I am also experiencing this issue in 1.16.0. ITokenAcquisition.GetAccessTokenForAppAsync throws a null reference exception.

Stack Trace when using ITokenAcquisition.GetAccessTokenForAppAsync

   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.GetAuthenticationResultForAppAsync(String scope, String authenticationScheme, String tenant, TokenAcquisitionOptions tokenAcquisitionOptions)
   at Microsoft.Identity.Web.TokenAcquisition.<GetAccessTokenForAppAsync>d__19.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at [Code Redacted]

If I try adding WithAppOnly to my Graph Request, I get the same error.

Stack Trace when using WithAppOnly

   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.GetAuthenticationResultForAppAsync(String scope, String authenticationScheme, String tenant, TokenAcquisitionOptions tokenAcquisitionOptions)
   at Microsoft.Identity.Web.TokenAcquisitionAuthenticationProvider.<AuthenticateRequestAsync>d__3.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Graph.AuthenticationHandler.<SendAsync>d__16.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at System.Net.Http.HttpClient.<SendAsyncCore>d__85.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at Microsoft.Graph.HttpProvider.<SendRequestAsync>d__19.MoveNext()

@jmprieur
Copy link
Collaborator

@MaxxDelusional
Do you have repro steps?

@MaxxDelusional
Copy link

@jmprieur I was just able to reproduce this new Web Api project.

I think the issue occurs because my API endpoint doesn't require an authenticated user. The API I am creating is hosted privately, and cannot be accessed from outside of my network. I want the API to be able to pull user photos from the graph api, without a user.

appsettings.json

{
  "AzureAd": {
    "Instance": "https://login.microsoftonline.com/",
    "Domain": "[READACTED]",
    "TenantId": "[READACTED]",
    "ClientId": "[READACTED]",
    "CallbackPath": "/signin-oidc",
    "Audience": "[READACTED]",
    "ClientSecret": "[READACTED]"
  },
  "Logging": {
    "LogLevel": {
      "Default": "Information",
      "Microsoft": "Warning",
      "Microsoft.Hosting.Lifetime": "Information"
    }
  },
  "AllowedHosts": "*"
}

TestController.cs

using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Identity.Web;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace IdentityWebGraphTest.Controllers
{

    [ApiController]
    [Route("[controller]")]
    public class TestController : ControllerBase
    {
        [HttpGet]
        [AllowAnonymous]
        public async Task<ActionResult> Get([FromServices] Microsoft.Graph.GraphServiceClient graph)
        {
           //NullReferenceException is thrown here
            var request = await graph.Users["[USER IN MY ORG]"].Photo.Content
               .Request()
               .WithAppOnly()
               .GetAsync();

            return Ok();
        }
    }

}

Startup.cs

using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.HttpsPolicy;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Microsoft.Identity.Web;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace IdentityWebGraphTest
{
    public class Startup
    {
        public Startup(IConfiguration configuration)
        {
            Configuration = configuration;
        }

        public IConfiguration Configuration { get; }

        // This method gets called by the runtime. Use this method to add services to the container.
        public void ConfigureServices(IServiceCollection services)
        {

            services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApi(Configuration.GetSection("AzureAd"))
                .EnableTokenAcquisitionToCallDownstreamApi()
                .AddMicrosoftGraph(defaultScopes: "User.Read User.Read.All")
                .AddInMemoryTokenCaches();


            services.AddControllers();
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }

            app.UseHttpsRedirection();

            app.UseRouting();

            app.UseAuthorization();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
            });
        }
    }
}

@jmprieur
Copy link
Collaborator

jmprieur commented Aug 24, 2021

@MaxxDelusional.
Then you'd want to use app only permissions.

Use

.AddMicrosoftGraphAppOnly()

instead of

 .AddMicrosoftGraph(defaultScopes: "User.Read User.Read.All")

in the startup.cs file

@MaxxDelusional
Copy link

Some of the endpoints in my API do require an authenticated user, is it possible to use both extension methods at the same time? I did a quick test with both, and I still got the NullReferenceException.

Maybe I am misunderstanding something, but why does WithAppOnly require an authenticated user at all?

@jmprieur
Copy link
Collaborator

@MaxxDelusional.
Sorry, I was not clear that you had both app tokens and user tokens. No you can't use both .AddMicrosoftGraphAppOnly() and .AddMicrosoftGraph(defaultScopes: "User.Read User.Read.All")

Let's check for this scenario of anonymous web app controllers wanting to access app only graph scopes

@jmprieur jmprieur changed the title [Bug] NullReferenceException when using Microsoft Graph (AppOnly) within IHostedService [Bug] NullReferenceException when using Microsoft Graph (AppOnly) within IHostedService or anonymous web app controllers Aug 25, 2021
@jmprieur jmprieur added bug Something isn't working and removed question Further information is requested external answered labels Aug 25, 2021
@jmprieur
Copy link
Collaborator

@jennyf19: Changing to a bug for the case of anonymous controllers that want to call Microsoft Graph app only scopes

@jmprieur jmprieur added this to the 1.16 milestone Aug 25, 2021
@jennyf19
Copy link
Collaborator

I tried to repro this with an anonymous controller and a GRPC service, but could not get the same error. @MaxxDelusional could you provide a repro?

@MaxxDelusional
Copy link

I suspect that I am probably doing something wrong.

In any case, here is a simplified repo. You'll need to modify appsettings.json, and specify a userId in TestController.

https://github.com/MaxxDelusional/IdentityWebTest

@jennyf19
Copy link
Collaborator

Thanks for the repro @MaxxDelusional, was very helpful. We have a bug when using an anonymous controller, but as a work around, you can specify the auth scheme in the TestController and things should work:

var request = await _graphServiceClient.Users[userId].Photo.Content
                .Request()
                .WithAppOnly()
                .WithAuthenticationScheme(JwtBearerDefaults.AuthenticationScheme)
                .GetAsync();

also, in Startup.cs, you need to include:

app.UseAuthentication();

to set up all the middleware correctly.

@jmprieur we need to do something better here for when we don't have the token but it is a web API.

@mrichards3
Copy link

Thanks @jennyf19. We were having this same issue using IDownstreamWebApi.CallWebApiForAppAsync from an anonymous controller since version 1.10.

Adding app.UseAuthentication(), and then also explicitly passing the authentication scheme like this avoids the NullReferenceException:

var value = await _downstreamWebApi.CallWebApiForAppAsync(
                "SecureService",
                // Need to explicitly pass the authentication scheme here
                JwtBearerDefaults.AuthenticationScheme,
                options =>
                {
                    ...
                });

@jmprieur
Copy link
Collaborator

Thanks for the investigation and the heads-up @jennyf19
Maybe we should leverage IAuthenticationSchemeProvider.GetDefaultAuthenticateSchemeAsync in ITokenAcquisition?

@jennyf19
Copy link
Collaborator

@MaxxDelusional @paolovalladolid @Trancesphere the fix is in master if you want to try it out. will be in next release, possibly next week or week after (1st/2nd week of sept).

@jennyf19 jennyf19 modified the milestones: 1.16, 1.16.1 Sep 3, 2021
@jennyf19
Copy link
Collaborator

jennyf19 commented Sep 6, 2021

Included in 1.16.1 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants