AzureAD / microsoft-identity-web

Helps creating protected web apps and web APIs with Microsoft identity platform and Azure AD B2C
MIT License
680 stars 210 forks source link

Redeem authCode failing with error in v2.5.0 #2096

Closed michiproep closed 1 year ago

michiproep commented 1 year ago

Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

2.5.0

Web app

Sign-in users, when the appsetting.json contain "ResponseType": "code",

Description

After upgrading to 2.5.0 signIn is not working anymore. It's failing with

OpenIdConnectProtocolException: Message contains error: 'invalid_client', error_description: 'AADSTS7000218: The request body must contain the following parameter: 'client_assertion' or 'client_secret'. Trace ID: f230550d-2015-4e89-a234-a98a4b718000 Correlation ID: 33e33f36-5a76-4b3d-b8f7-5c56e1724fe7 Timestamp: 2023-03-01 09:56:57Z', error_uri: 'https://login.microsoftonline.com/error?code=7000218'.

although ClientSecret is set in options.

Reproduction steps

just migrate to 2.5.0 and application which overrides the "ResponseType": "code"

Error message

OpenIdConnectProtocolException: Message contains error: 'invalid_client', error_description: 'AADSTS7000218: The request body must contain the following parameter: 'client_assertion' or 'client_secret'. Trace ID: f230550d-2015-4e89-a234-a98a4b718000 Correlation ID: 33e33f36-5a76-4b3d-b8f7-5c56e1724fe7 Timestamp: 2023-03-01 09:56:57Z', error_uri: 'https://login.microsoftonline.com/error?code=7000218'.

Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.RedeemAuthorizationCodeAsync(OpenIdConnectMessage tokenEndpointRequest)
Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.HandleRemoteAuthenticateAsync()

Id Web logs

No response

Relevant code snippets

none

Regression

1.26.0

Expected behavior

Client_assertion/secret parameter is send with redeemCode request.

MrCodeB2 commented 1 year ago

same here

jmprieur commented 1 year ago

@michiproep @MrCodeB2 very strange. How is your appsettings.json? We'll have a look, but meanwhile, as a workaround, do you want to replace the "ClientSecret" configuration by the following block (there is an array as we now allow a series of client credentials, including certificates, workload identity federation etc ...)

"ClientCredentials": [
   {
    "SourceType": "ClientSecret",
    "ClientSecret": "***"
   }
  ]
michiproep commented 1 year ago

Hi @jmprieur , sorry, no luck. Did not help.

jonathan-vogel-siemens commented 1 year ago

can confirm this makes it impossible for users to login. Only thing that helped is reverting back to 1.26.0

jmprieur commented 1 year ago

@michiproep @Jonathan-a35y @MrCodeB2 Is it a blazor app? in which case did you read : https://github.com/AzureAD/microsoft-identity-web/wiki/web-app-troubleshooting#aadsts54005-oauth2-authorization-code-was-already-redeemederror ?

michiproep commented 1 year ago

No, it's a plain asp.net core mvc app. I already send the example app within my email. Remember?

jmprieur commented 1 year ago

@michiproep would you be able to share a repro?

Dzeneralen commented 1 year ago

Using both Microsoft.Identity.Web and Microsoft.identity.Web.UI.

Issue is with scaffolded templates (dotnet new razor2), but the main points are listed below. Secret is not used on last packages, but is on 1.*.

Program.cs

builder.Services
    .AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
    .AddMicrosoftIdentityWebApp(builder.Configuration.GetSection("AzureAd"));

appsettings.json

{
  "AzureAd": {
    "Instance": "https://login.microsoftonline.com/",
    "Domain": "myprivatetenant.onmicrosoft.com",
    "TenantId": "random_guid",
    "ClientId": "random_guid",
    "ClientSecret": "my_super_secret",
    "ResponseType": "code",
    "CallbackPath": "/signin-oidc",
    "ClientCredentials": [
      {
        "SourceType": "ClientSecret",
        "ClientSecret": "my_super_secret"
      }
    ]
  },

}
paulirwin commented 1 year ago

+1, confirmed this breaks sign-in for an ASP.NET Core Web App that is configured to call the Graph API downstream. Downgrading to 1.26.0 fixes it.

jmprieur commented 1 year ago

In the appsettings.json you should have either the ClientSecret, or the client credentials of SourceType: ClientSecret, but not both. Did you set the secret programmatically? or by configuration?

michiproep commented 1 year ago

Well, I tryed all combinations... None of them works. Also setting them in code via confiure(...)

Dzeneralen commented 1 year ago

In the appsettings.json you should have either the ClientSecret, or the client credentials of SourceType: ClientSecret, but not both. Did you set the secret programmatically? or by configuration?

Neither of them work, tried one at a time in appsettings.json. Both are included above just to show how I tried.

jonathan-vogel-siemens commented 1 year ago

I can confirm that ^

paulirwin commented 1 year ago

In the appsettings.json you should have either the ClientSecret, or the client credentials of SourceType: ClientSecret, but not both. Did you set the secret programmatically? or by configuration?

I tried this both ways. I am using user-secrets to test this locally before promoting to a cloud environment. I added the following user secret keys (my existing configuration section is called AzureAd): AzureAd:ClientCredentials:0:SourceType of ClientSecret, and AzureAd:ClientCredentials:0:ClientSecret with the secret value. Let me know if I did something wrong there.

jmprieur commented 1 year ago

@Dzeneralen I just did:

dotnet new razor --auth SingleOrg --callsGraph

Then I've created an app in Azure Ad and configured it with a Client Secret. And it worked fine.

That's my program,cs:

using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.Identity.Web;
using Microsoft.Identity.Web.UI;

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
    .AddMicrosoftIdentityWebApp(builder.Configuration.GetSection("AzureAd"))
     .EnableTokenAcquisitionToCallDownstreamApi(new string[] { "user.read" })
       .AddMicrosoftGraph()
       .AddInMemoryTokenCaches();

builder.Services.AddAuthorization(options =>
{
    // By default, all incoming requests will be authorized according to the default policy.
    options.FallbackPolicy = options.DefaultPolicy;
});
builder.Services.AddRazorPages()
    .AddMicrosoftIdentityUI();

var app = builder.Build();

// Configure the HTTP request pipeline.
if (!app.Environment.IsDevelopment())
{
    app.UseExceptionHandler("/Error");
    // The default HSTS value is 30 days. You may want to change this for production scenarios, see https://aka.ms/aspnetcore-hsts.
    app.UseHsts();
}

app.UseHttpsRedirection();
app.UseStaticFiles();

app.UseRouting();

app.UseAuthorization();

app.MapRazorPages();
app.MapControllers();

app.Run();

and the appsettings.json file

{
/*
The following identity settings need to be configured
before the project can be successfully executed.
For more info see https://aka.ms/dotnet-template-ms-identity-platform 
*/
  "AzureAd": {
    "Instance": "https://login.microsoftonline.com/",
    "Domain": "mydomain",
    "TenantId": "MytenantId",
    "ClientId": "MyClientId",
    "ClientSecret": "*****"
  },
  "Logging": {
    "LogLevel": {
      "Default": "Information",
      "Microsoft.AspNetCore": "Warning"
    }
  },
  "AllowedHosts": "*"
}

The index.cs page is:

using Microsoft.AspNetCore.Mvc.RazorPages;
using Microsoft.Graph;
using Microsoft.Identity.Web;

namespace idweb_bugs.Pages;

[AuthorizeForScopes(Scopes = new string[] { "user.read" })]
public class IndexModel : PageModel
{
    GraphServiceClient _grpahServiceClient;

    private readonly ILogger<IndexModel> _logger;

    public IndexModel(ILogger<IndexModel> logger, GraphServiceClient grpahServiceClient)
    {
        _logger = logger;
        _grpahServiceClient = grpahServiceClient;
    }

    public async Task OnGet()
    {
        var me = await _grpahServiceClient.Me.Request().GetAsync();
    }
}

This works well.

How do you specify your secret? In the appsettings.json file? or by code?

Also don't you have something empty the the dotnet secret manager?

jmprieur commented 1 year ago

I also added this to the appsettings.json (even if this is the default)

   "ResponseType": "code",

and this worked too.

@michiproep @paulirwin @MrCodeB2 @Dzeneralen @Jonathan-a35y: At this point, if one of you has a link to code that repro-es and that we could debug this would help moving forward.

jmprieur commented 1 year ago

@michiproep @paulirwin @MrCodeB2 @Dzeneralen @Jonathan-a35y FYI, we were able to repro, and @jennyf19 has a fix.

we updated slightly the title and the description of the bug with the repro steps.

jennyf19 commented 1 year ago

GitHub automatically closed this, reopening. This will be in the 2.6.1 release which should be out this week 3/23-3/24.

westin-m commented 1 year ago

Included in the 2.6.1 release

michiproep commented 1 year ago

I can confirm that this is working now. But unfortunately, the version 2.6.1 introduced additional bugs: It's about setup and handling OpenIdConnect events. First of all, it's not clear how we can setup the eventHandler with the new version:

and second: It seems like once I'm able to catch the event (currently only via PostConfigure), it get's called multiple times, it breaks the DI scope (e.g. DbContext gets disposed), async await not working anymore.

Sorry to say, but these 2.x versions are a complete mess currently.

jmprieur commented 1 year ago

@michiproep Thanks for the feedback.

As we explained in the release notes, you need to setup your event handlers with the named options (the authentication scheme). You can use OpenIdConnectOptions or MicrosoftIdentityOptions (OpenIdConnectOptions is more performant).

Please see: https://github.com/AzureAD/microsoft-identity-web/wiki/v2.0#breaking-changes-1

We can also help you better if you share repro code (when we met, you said you would). If you'd rather meet again, just let us know. We can too.

michiproep commented 1 year ago

@jmprieur Although, I did send you the code twice already, I now created a repo and send the link via email...

michiproep commented 1 year ago

Regarding my last comment, there are some additional issues created by other users now. e.g. 1992