AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.38k stars 338 forks source link

[Bug] OBO with Microsoft Graph (OnBehalfOfProvider) throws NullReferenceException when trying to authenticate #2599

Closed Tinathnath closed 3 years ago

Tinathnath commented 3 years ago

Hi, I'm trying to authenticate to Microsoft Graph with the official SDK and the OnBehalfOf provider. I followed the docs [https://docs.microsoft.com/en-us/graph/sdks/choose-authentication-providers?tabs=CS#OnBehalfOfProvider]() but it doesn't work. I tried multiple configuration scenarios but I still get a NullReferenceException in the process. It seems that the problem is in the GetBodyParameters() method, I think UserAssertion is null (see [https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/master/src/client/Microsoft.Identity.Client/Internal/Requests/OnBehalfOfRequest.cs#L109]())

Logs and Network traces There's no network trace matching login.microsoftonline.com, the error is beeing thrown before token request.

StackTrace:

Status Code: 0
Microsoft.Graph.ServiceException: Code: generalException
Message: An error occurred sending the request.

 ---> Microsoft.Graph.Auth.AuthenticationException: Code: generalException
Message: Unexpected exception occured while authenticating the request.

 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Identity.Client.Internal.Requests.OnBehalfOfRequest.GetBodyParameters()
   at Microsoft.Identity.Client.Internal.Requests.OnBehalfOfRequest.FetchNewAccessTokenAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.Internal.Requests.OnBehalfOfRequest.ExecuteAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.RunAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.ApiConfig.Executors.ConfidentialClientExecutor.ExecuteAsync(AcquireTokenCommonParameters commonParameters, AcquireTokenOnBehalfOfParameters onBehalfOfParameters, CancellationToken cancellationToken)
   at Microsoft.Graph.Auth.OnBehalfOfProvider.GetNewAccessTokenAsync(AuthenticationProviderOption msalAuthProviderOption)
   --- End of inner exception stack trace ---
   at Microsoft.Graph.Auth.OnBehalfOfProvider.GetNewAccessTokenAsync(AuthenticationProviderOption msalAuthProviderOption)
   at Microsoft.Graph.Auth.OnBehalfOfProvider.AuthenticateRequestAsync(HttpRequestMessage httpRequestMessage)
   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.UserRequest.GetAsync(CancellationToken cancellationToken)
   at AadAuthAPI.Controllers.UserController.GetUser()

Which Version of MSAL are you using ? Microsoft.Graph: 3.30.0 Microsoft.Graph.Auth: 1.0.0-preview.0 Microsoft.Identity.Web: 1.9.1 Microsoft.Identity.Web.MicrosoftGraph: 1.9.1

Platform netcore 5

What authentication flow has the issue?

Is this a new or existing app? This is a new app or experiment

Repro

Startup.cs

  AzureAdOptions = Configuration.GetSection(AzureAdConfigSection).Get<AzureAdOptions>();
  services.Configure<AzureAdOptions>(Configuration.GetSection("AzureAd"));

  services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
      .AddMicrosoftIdentityWebApi(Configuration.GetSection("AzureAd"));

In my controller, I can get the user token with:

_ = await HttpContext.AuthenticateAsync();
string token = await HttpContext.GetTokenAsync("access_token");

But when I create my Graph client and execute a request, it fails:

IConfidentialClientApplication confidentialClientApplication = ConfidentialClientApplicationBuilder
    .Create(_aadConfig.ClientId)
    .WithClientSecret(_aadConfig.ClientSecret)
    .WithAuthority("https://login.microsoftonline.com/<tenantId>")
    .WithRedirectUri(_aadConfig.RedirectUri)
    .Build();

OnBehalfOfProvider authProvider = new(confidentialClientApplication, new string[] { "https://graph.microsoft.com/user.read" });
var client = new GraphServiceClient(authProvider);

var user = await client.Me.Request().GetAsync();

It tried with/without RedirectUri and Authority and I always get the same error.

Expected behavior Expect the middleware to get a token with OBO flow to authenticate to Graph with my current API user. When I call the authorization endpoint in Postman with my clientId+secret+user token it works well ([https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-on-behalf-of-flow#first-case-access-token-request-with-a-shared-secret]())

Actual behavior The authentication fails in the Microsoft.Graph.Auth package with OnBehalfOfProvider.

Possible Solution Might be an issue with the UserAssertion object beeing null.

Thank you

bgavrilMS commented 3 years ago

Will take a look. Null refs are most always bugs

jmprieur commented 3 years ago

@Tinathnath Whatever the outcome of the investigation that @bgavrilMS will do, given you already use Microsoft.Identity.Web, the way to go is not to use the OnBehalfOf provider, but directly Microsoft.Identity.Web.MicrosoftGraph (which we developed with the Microsoft Graph SDK team).

The startup.cs: https://github.com/Azure-Samples/active-directory-dotnet-native-aspnetcore-v2/blob/1c0c04fb103dca02f65d41d21ad65f7f743e37e3/2.%20Web%20API%20now%20calls%20Microsoft%20Graph/TodoListService/Startup.cs#L25-L28

            services.AddMicrosoftIdentityWebApiAuthentication(Configuration)
                    .EnableTokenAcquisitionToCallDownstreamApi()
                        .AddMicrosoftGraph(Configuration.GetSection("DownstreamApi"))
                        .AddInMemoryTokenCaches();

And then you inject directly the GraphService client in your controller actions and you use it from there;

https://github.com/Azure-Samples/active-directory-dotnet-native-aspnetcore-v2/blob/1c0c04fb103dca02f65d41d21ad65f7f743e37e3/2.%20Web%20API%20now%20calls%20Microsoft%20Graph/TodoListService/Controllers/TodoListController.cs#L32

Out of curiosity (and to update the documentation), from which documentation did you get the idea of mixing MIcrosoft.Identity.Web and the OnBehalfOf provider? cc: @darrelmiller

Tinathnath commented 3 years ago

@bgavrilMS Thanks ! @jmprieur I tried with Microsoft.Identity.Web.MicrosoftGraph in Startup but forgot to use it via DI, I created my own instance (now that I'm writing it, it doesn't make sense). I'm quite new to MSAL so I tend to mix things up. I thought the AddMicrosoftGraph() helper could only authenticate as a daemon app, but I need to authenticate as a user that's why I used the OnBehalf provider.

bgavrilMS commented 3 years ago

Assuming this is no longer an issue. Please reopen if it is.