microsoftgraph / msgraph-sdk-dotnet-auth

Archived - use the TokenCredential classes provided by Azure.Identity. https://docs.microsoft.com/en-us/dotnet/api/overview/azure/identity-readme
https://graph.microsoft.com
MIT License
78 stars 19 forks source link

Unpredictable Authorization_RequestDenied failures sometimes when adding users to AAD #47

Closed joshfriend closed 3 years ago

joshfriend commented 5 years ago

I occasionally (but not 100% of the time) get an error when attempting to add a user to AAD from my web API using on-behalf-of authentication:

await _graphServiceClient.Users.Request().AddAsync(user)
Status Code: Forbidden
Microsoft.Graph.ServiceException: Code: Authorization_RequestDenied
Message: Insufficient privileges to complete the operation.
Inner error
   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)
   ...

I found one github issue that seemed very similar to what I was seeing: https://github.com/Azure-Samples/active-directory-java-graphapi-web/issues/4 I tried any applicable suggestions from that thread as well as various others I found on StackOverflow, but nothing has worked so far.

The issue is not specific to any one user calling the API, and I have not found a pattern for when it happens. The error message above is also not very helpful in narrowing down the cause.

In ConfigureServices, I've set up AAD and the Graph SDK client as follows:

services.Configure<ConfidentialClientApplicationOptions>(Configuration.GetSection("MsGraph"));

services.AddScoped<IGraphServiceClient>(serviceProvider => {
  var clientApp = serviceProvider.GetRequiredService<IConfidentialClientApplication>();
  var oboProvider = new OnBehalfOfProvider(clientApp, new[] {"User.ReadWrite.All"});
  return new GraphServiceClient(oboProvider);
});

services.AddAuthentication(AzureADDefaults.JwtBearerAuthenticationScheme)
  .AddAzureADBearer(options => Configuration.Bind("AzureAd", options));

services.Configure<JwtBearerOptions>(AzureADDefaults.JwtBearerAuthenticationScheme, options => {
  options.Authority = options.Authority + "/v2.0/";
  options.TokenValidationParameters.ValidateIssuer = true;
  options.SaveToken = true;
  options.Events = new JwtBearerEvents();
  options.Events.OnTokenValidated = async cxt => {
    var clientApp = cxt.HttpContext.RequestServices
      .GetRequiredService<IConfidentialClientApplication>();

    // perform OBO flow with client app, which will cache the appropriate token for us
    // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/on-behalf-of
    var bearerToken = ((JwtSecurityToken) cxt.SecurityToken).RawData;
    await clientApp.AcquireTokenOnBehalfOf(
      new[] {"User.ReadWrite.All"},
      new UserAssertion(bearerToken)
    ).ExecuteAsync();
  };
});

Here's the versions of the SDKs related to graph/auth that I am using in the project.

<PackageReference Include="Microsoft.Graph" Version="1.15.0" />
<PackageReference Include="Microsoft.Graph.Auth" Version="1.0.0-preview.0" />
<PackageReference Include="Microsoft.IdentityModel.Clients.ActiveDirectory" Version="4.5.1" />
<PackageReference Include="Microsoft.AspNetCore.Authentication.AzureAD.UI" Version="2.2.0" />

I have granted admin consent for the necessary permissions that the application is using:

AB#7208

darrelmiller commented 5 years ago

@jmprieur Do we have any answers to this issue?

joshfriend commented 5 years ago

@darrelmiller can you clarify the addition of the "service bug" label? Is this a known issue with AAD, and where can i find out more information about it?

darrelmiller commented 5 years ago

@joshfriend I can't confirm that it is a service bug, however, I am fairly confident that it isn't an issue with the Graph Auth Provider. We use the "service bug" tag to indicate it is not something that our team can fix. I tagged Jean Marc as he owns the MSAL.NET library that our Auth Provider uses. He would have a better idea if it actually is a service issue or an MSAL issue. However, I believe Jean Marc is on vacation so it may take a few days to get a response.

joshfriend commented 5 years ago

@darrelmiller @jmprieur I'm still having this issue and multiple attempts by myself and several coworkers to work around it have failed.

jmprieur commented 5 years ago

@joshfriend : is it possible to get the information in the MsalServiceException (inner exception of the Graph Exception)? my first intuition is that that users for whom the OBO happen don't have enought permissions: remember that the effective permission for a delegated permission = intersection of permission for the app and permission of the user. Even if the app has the permissions for a resource, if the user does not, then the service will return "Insufficient privileges to complete the operation."

joshfriend commented 5 years ago

I found a few users in the directory who lacked the user creation permissions. I also created a new user without the permission and tried to use it to create a new user, which triggered the "insufficient privileges" error, then adding the permissions allowed the user to finally create a user. I did have to restart the webservice after adding the permission to the user, presumably because of the token cache.

When I created this issue, I'm pretty sure I'd experienced the permission denied response with my own user (which is a global admin for the directory), but that was a long time ago and I have no evidence for that anymore so I'll close this issue.

joshfriend commented 5 years ago

Reopening because we are experiencing the issue again. This time I am 100% sure that the user who is trying and failing to create new users in Graph has the "User administrator" assigned role in the directory.

joshfriend commented 5 years ago

The SDK retrieves and caches auth tokens for the calling user properly, but once we get outside of the OnTokenValidated callback where that happens, it is unable to know what user account to use. When performing a call to Graph, inside IClientApplicationBaseExtensions. GetAccessTokenSilentAsync, the value of msalAuthProviderOption.UserAccount is null so for some reason the SDK decides to use whatever account is first in the cache.

I'm really confused why it's set up that way, it's leading to very strange and unpredictable behavior in my experience.

I also don't know how I can ensure that the call to httpRequestMessage.GetMsalAuthProviderOption() in OnBehalfOfProvider.AuthenticateRequestAsync() will retrieve the correct user assertion instead of returning null

joshfriend commented 5 years ago

I can make this bug happen by logging in first as an unprivileged user after starting the app, then logging out and back in as a user who has user creation privileges. The SDK will use the cached token for the unprivileged user even though the OnTokenValidated call to AcquireTokenOnBehalfOf succeeds with the correct user assertion

joshfriend commented 5 years ago

The combination of some graph requests in my application missing WithUserAssertion() on the request builder, the SDK using just any token from the cache if a user assertion was missing, and the lack of an exception when that happens has caused me lots of pain for months, but I think I finally have it figured out.

Please make the client throw an exception if the auth provider is on-behalf-of flow and no user assertion was provided in the request, or figure out the user assertion automatically. Using the first token from the cache is a pretty terrible result in this case.

It looks like there used to be an exception in this case (#31) but it was removed?

It's obvious in hindsight, but docs stating the importance of WithUserAssertion() on request builders for certain flows would have been very useful.

maisarissi commented 3 years ago

Hi @joshfriend

Thank you for reaching out and opening this issue. This client library will not leave the preview state. Microsoft.Graph v4 now integrates with Azure.Identity which supports a wide variety of authentication flows out of the box. We suggest that you migrate to v4 + Azure.Identity. Read more about it in this issue.

This issue won't be fixed, and the repository will be archived.