Microsoft-authentication-library-for-dotnet: .WithCertificate with /common audience scenario is broken

[svrooij]

Which Version of MSAL are you using ?
<PackageReference Include="Microsoft.Identity.Client" Version="2.7.0" />

Platform
netcore 2.2

What authentication flow has the issue?

• Desktop / Mobile
  
  
  
  • [ ] Interactive
  
  
  • [ ] Integrated Windows Auth
  
  
  • [ ] Username Password
  
  
  • [ ] Device code flow (browserless)
  
  
• Web App
  
  
  
  • [x] Authorization code
  
  
  • [ ] OBO
  
  
• Web API
  
  
  
  • [ ] OBO
  
  

Code based on this sample https://github.com/microsoftgraph/aspnetcore-connect-sample

Is this a new or existing app?
It's a new app, I took created a new empty dotnet core web application and took the code from the above sample as an inspiration.

Repro

// Azure options method
// ClientCredentials with Key when filled and with certificate when not filled (both created in te portal correctly!!)
public ClientCredential GetClientCredential() {
    if(!string.IsNullOrEmpty(ClientSecret)) return new ClientCredential(ClientSecret);
    if(!string.IsNullOrEmpty(CertificateThumbprint)) {
        X509Certificate2 cert = GetCertificate();
        if(cert != null)
            return new ClientCredential(new ClientAssertionCertificate(cert));
    }
    throw new ArgumentException("No client secret or certificate configured");
}
// Actual code.
var cca = new ConfidentialClientApplication(
    _azureOptions.ClientId,
    _azureOptions.GetAuthority(),
    redirecUrl,
    _azureOptions.GetClientCredential(),
    new Helpers.SessionTokenCache(identifier, memoryCache).GetCacheInstance(),
    null);

var result = await cca.AcquireTokenByAuthorizationCodeAsync(code, scopes);

Expected behavior
Get an accesstoken

Actual behavior
Exception thrown (when certificate set)

"AADSTS700023: Client assertion audience claim does not match Realm issuer.
Trace ID: 36e0fa3f-437e-45b5-825e-472930cc7100
Correlation ID: fa45528a-9bde-4eaa-9fd1-ab7c44d15348
Timestamp: 2019-02-18 22:35:46Z"

Access Token when the secret is set. (To check the client is created correctly and it doesn't have anything to do with redirect uri, invalid client.....)

Possible Solution
It seems like the ClientAssertionCertificate doesn't accept an ClientID anymore. And according to this page the client ID should be in the assertion.

In the past it did accept a ClientID as shown in the "test code" from earlier.

So I would expect the ClientAssertionCertificate to have an new ClientAssertionCertificate(string clientId, X509Certificate2 certificate) constructor.

Additional context/ Logs / Screenshots
The following code isn't working anymore, and I cannot find when it would be removed.
https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/28473ce9460cfbeb191df346db565926d044f2e7/tests/Microsoft.Identity.Test.LabInfrastructure/KeyVaultSecretsProvider.cs#L145

[jmprieur]

@svrooij : I just checked with the certificate variation of the following sample:https://github.com/azure-samples/active-directory-dotnetcore-daemon-v2

The ClientAssertionCertificate is working fine in daemon applications (BTW the ClientID, in MSAL.NET, contrary to ADAL.NET, is provided at the creation of the application, and therefore does not need to be repeated. See https://aka.ms/msal-net-client-credentials

I suspect that there is a service issue with the redemption of the authorization code when a certificate is used. Will update you.

[svrooij]

I suspect that there is a service issue with the redemption of the authorization code when a certificate is used. Will update you.

Does this mean you think there is something wrong with the v2 endpoints of the Azure ad endpoints?
Or should I change something to the code I used?

You’re right about the part that the ClientID should be set twice that seems unnecessary

[jmprieur]

After investigation:

• the client assertion is indeed bad:

"aud": "https://login.microsoftonline.com/{tenantid}/v2.0"

the library:

1. Looks at /common/
2. takes the issuer field, which happens to be "https://login.microsoftonline.com/{tenantid}/v2.0" in the case where the authority is common (this works fine for a tenanted endpoint)
3. sets the aud claim of the client assertion to the value of this field.

Note that MSAL does the right compliant thing, but OAuth doesn’t have a good mechanism for wildcard URLs like /common that don’t
map to a specific issuer up front.

The proposal is to attempt, in MSAL.NET, to replace "{tenanid}" by "common" in the audience claim of the client assertion (given that we would not know the tenant beforehand)

[jmprieur]

@henrik-me FYI.

[jmprieur]

@svrooij I've provided the analysis above.
is it an option for you to use a tenanted authority (until we fix the issue)

[svrooij]

In my specific application I would need it to work with any tenant.
But in the flow i’m using id-token code I know which tenant it’s because I’m able to extract that from the identity token before exchanging the code for an access token.

Can I specify that upon requesting the access token with the code?

[jmprieur]

yes @svrooij , you can by setting the authority in the constructor to confidential client application to be "https://login.microsoftonline.com/{TenantIdExtractedFromIdToken}/"

[svrooij]

Thanks @jmprieur I just verified that this works.

So this are the options:

1. When using a ClientSecret key, you can both use https://login.microsoftonline.com/common/v2.0 and https://login.microsoftonline.com/{tenantID}/v2.0 as the authority.
2. When using a certificate client secret you can only use https://login.microsoftonline.com/{tenantId}/v2.0 as the authority. https://login.microsoftonline.com/common/v2.0 doesn't work (yet).

The iss claim from the ID-token can be used as Authority. Or if you don't save this, you can use the Issuer of each generated claim.

[jennyf19]

@jmprieur FYI: You can also just specify the tenantId.
https://login.microsoftonline.com/{tenantId}

[jennyf19]

@jmprieur is this needing a fix or documentation? moving to 4.1 milestone, but feel free to close or modify as needed.

[jmprieur]

@jennyf19 ideally we'd make a fix.

[svrooij]

@jmprieur is there something I can do to resolve this issue so it doesn’t get moved every time a new version comes around.
Do you have a link to the git version of the documentation so I can improve it.

[jmprieur]

@svrooij : I've sent a PR to update this page: https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-daemon-app-configuration
It's in the pipe. I could take a few hours to come-up publicly

Note that I'd still want to fix the issue in MSAL.NET @henrik-me @jennyf19

[jmprieur]

@jennyf19 : the proposed fix is described in https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/891#issuecomment-465524589

before setting the "aud" claim, we should replace the "{tenantid}" string by "common".

[jennyf19]

@svrooij @jmprieur
The bug has been fixed in this branch, if you want to try it out. It was interesting that the behavior w/the ClientSecret and ClientCertificate was different.
If you're interested, here is why:
In AuthorityEndpointResolutionManager, we are just checking for {tenant} and replacing the value w/whatever we have for the tenant value (in this case common). But in the issuer, we actually have tenantid
--> https://login.microsoftonline.com/{tenantid}/v2.0, so the value {tenantid} is not replaced with common.

Then, in ClientCredentialHelper, if it’s the ClientCertificate being used, we set the audience to the value of the SelfSignedJwtAudience, which would be https://login.microsoftonline.com/{tenantid}/v2.0 if common is used, because of the above reason (this is now fixed).
However, when doing the ClientSecret flow, we check if the secret is being used and then use the secret as a parameter and exit out, we do not continue and set the audience to any value. Which is why the behavior was different depending on if the secret or certificate was used.
cc: @henrik-me

[jmprieur]

thanks @jennyf19
yes, with a certificate we really compute a client assertion including the issuer and audience
Thanks for fixing this, @jennyf19

[jennyf19]

@svrooij fixed...will be in 4.1 release, out in a few days.

[svrooij]

Thanks @jennyf19 and guys!

I really liked the detailed explanation! Great fix.

[jmprieur]

@svrooij : the fixe is available in MSAL.NET 4.1
Thanks for your patience
cc: @jennyf19

[msabansal]

@jennyf19 @jmprieur The sample from https://github.com/microsoftgraph/aspnetcore-connect-sample (modified to use certificate) still doesn't work with the new version. It only works when the call on this line https://github.com/microsoftgraph/aspnetcore-connect-sample/blob/226d820a5bdf4b2cfde46749107d7cc571fc7761/MicrosoftGraphAspNetCoreConnectSample/Helpers/GraphAuthProvider.cs#L69 uses tenant'ed authority. Is that expected?
Error:

MSAL.NetCore.4.2.1.0.MsalServiceException: 
    ErrorCode: invalid_client
Microsoft.Identity.Client.MsalServiceException: The wrong application (public or confidential) is being used with this authentication flow. Check the configuration of the app being used in the app registration portal. See https://aka.ms/msal-net-invalid-client for details.  Original exception: AADSTS700023: Client assertion audience claim does not match Realm issuer.
Trace ID: fefd1353-253f-4cdc-9527-6d02cbd52500
Correlation ID: 6fef8f37-b8cb-4302-b259-028cb1c018f7
Timestamp: 2019-07-25 19:10:37Z
   at Microsoft.Identity.Client.OAuth2.OAuth2Client.CreateErrorResponse(HttpResponse response, RequestContext requestContext)
   at Microsoft.Identity.Client.OAuth2.OAuth2Client.CreateResponse[T](HttpResponse response, RequestContext requestContext, Boolean addCorrelationId)
   at Microsoft.Identity.Client.OAuth2.OAuth2Client.ExecuteRequestAsync[T](Uri endPoint, HttpMethod method, RequestContext requestContext)
   at Microsoft.Identity.Client.OAuth2.OAuth2Client.GetTokenAsync(Uri endPoint, RequestContext requestContext)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.SendHttpMessageAsync(OAuth2Client client, String tokenEndpoint)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.SendTokenRequestAsync(String tokenEndpoint, IDictionary`2 additionalBodyParameters, CancellationToken cancellationToken)
   at Microsoft.Identity.Client.Internal.Requests.AuthorizationCodeRequest.ExecuteAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.RunAsync(CancellationToken cancellationToken)
   at Microsoft.Identity.Client.ApiConfig.Executors.ConfidentialClientExecutor.ExecuteAsync(AcquireTokenCommonParameters commonParameters, AcquireTokenByAuthorizationCodeParameters authorizationCodeParameters, CancellationToken cancellationToken)
   at MicrosoftGraphAspNetCoreConnectSample.Helpers.GraphAuthProvider.GetUserAccessTokenByAuthorizationCode(String authorizationCode) in C:\Users\sabansal\source\repos\testsample\MicrosoftGraphAspNetCoreConnectSample\Helpers\GraphAuthProvider.cs:line 73
   at MicrosoftGraphAspNetCoreConnectSample.Extensions.AzureAdAuthenticationBuilderExtensions.ConfigureAzureOptions.<Configure>b__4_2(AuthorizationCodeReceivedContext context) in C:\Users\sabansal\source\repos\testsample\MicrosoftGraphAspNetCoreConnectSample\Extensions\AzureAdAuthenticationBuilderExtensions.cs:line 84
   at Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.RunAuthorizationCodeReceivedEventAsync(OpenIdConnectMessage authorizationResponse, ClaimsPrincipal user, AuthenticationProperties properties, JwtSecurityToken jwt)
   at Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectHandler.HandleRemoteAuthenticateAsync()
    StatusCode: 401 
    ResponseBody: {"error":"invalid_client","error_description":"AADSTS700023: Client assertion audience claim does not match Realm issuer.\r\nTrace ID: fefd1353-253f-4cdc-9527-6d02cbd52500\r\nCorrelation ID: 6fef8f37-b8cb-4302-b259-028cb1c018f7\r\nTimestamp: 2019-07-25 19:10:37Z","error_codes":[700023],"timestamp":"2019-07-25 19:10:37Z","trace_id":"fefd1353-253f-4cdc-9527-6d02cbd52500","correlation_id":"6fef8f37-b8cb-4302-b259-028cb1c018f7"} 
    Headers: Cache-Control: no-store, no-cache
Pragma: no-cache

Tried with MSAL.NET version 4.1 and 4.2.1

[henrik-me]

@jennyf19 : reactivating to start investigation.

[henrik-me]

CC: @jmprieur

[bgavrilMS]

When using certificates, MSAL will create and sign a JWT that includes (among others), an audience claim. In MSAL.NET the value of this claim is the issuer, which is taken from the discovery endpoint. The problem arrises when using common.

According to the ESTS team:

ESTS would accept either tenant specific issuer URL or **common token endpoint URL**

Issuer (not good): https://login.microsoftonline.us/common/v2.0
Token Endpoint (seems to work): https://login.microsoftonline.com/common/oauth2/v2.0/token

I can make the following fix:

• if authority is tenanteless, use token endpoint
• otherwise use issuer (as it comes from the discovery endpoint)

However I do not see the logic in this. Why accept the token endpoint in one case and issuer in other?

@jmprieur @henrik-me - following up with the ESTS team.
@rayluo @sangonzal @SomkaPe - in case this affects python / java as well.

Note that in web site scenarios MSAL does not have the tenant ID, although the midleware does have this inside the tid claim.

[rayluo]

@bgavrilMS Interesting topic. Question. If the authority is tenant-specific, would ESTS also accept its token endpoint (https://login.microsoftonline.com/CONTOSO/oauth2/v2.0/token) as a valid aud? Historically ADAL Python always uses token endpoint and it seemingly™️ works, and MSAL Python inherits that practice without giving much thought on it. Personally I do agree with you about your comment on the imparity of the ESTS would accept either tenant specific issuer URL or **common token endpoint URL**.

[svrooij]

I really like all of you guys helping out to my original seemingly easy question/suggestion.

@bgavrilMS if your suggestion works that would be really nice. Think that might also need some extra documentation on the page describing the certificate client credentials. https://docs.microsoft.com/en-us/azure/active-directory/develop/active-directory-certificate-credentials. And a side question, would it then also accept the organizations endpoint?

By the way I think the documentation page is changed since this issue was created, because I cannot remember that it stated that the audience should be the tenanted token endpoint.

[bgavrilMS]

Discussed more around this and it looks like sending the Token Endpoint Uri as Issuer is the right thing to do for this flow. As @rayluo mentioned, sending the token endpoint (tenanted or tenantless) seems to work.

@svrooij - /organizations would have the same issue, it's a "tenantless" authority. But otherwise yes, it will work.

I'm working on a fix. Until then, the workaround remains valid - use a tenanted authority.

Most of the team is on holiday, so next release would be in some ~2 weeks (not a promise). If this is blocking, we'll figure how to release sooner.

[bgavrilMS]

MSAL 4.3.1 released