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] Null reference exception calling AcquireTokenByAuthorizationCode when client secret is not specified (need to improve error message) #66

Closed
1 task
winterlimelight opened this issue Oct 27, 2019 · 15 comments
Assignees
Labels
Milestone

Comments

@winterlimelight
Copy link

Which Version of MSAL are you using ?
4.5.1

Platform
.NET Core 3

What authentication flow has the issue?

  • Web App
    • Authorization code

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

Repro
This is based on https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/Acquiring-tokens-with-authorization-codes-on-web-apps but using WithB2CAuthority rather than WithAuthority.

IConfidentialClientApplication cca = ConfidentialClientApplicationBuilder
	.Create(AzureAdB2COptions.ClientId)
	.WithRedirectUri(AzureAdB2COptions.RedirectUri)
	.WithB2CAuthority(AzureAdB2COptions.Authority)
	.Build();

var builder = cca.AcquireTokenByAuthorizationCode(new[] { "openid" }, context.ProtocolMessage.Code);
AuthenticationResult result = await builder.ExecuteAsync();

Expected behavior
I was expecting to retrieve an access token in line with https://docs.microsoft.com/en-us/azure/active-directory-b2c/active-directory-b2c-reference-oauth-code#2-get-a-token

Actual behavior
Exception is thrown by ExecuteAsync(). Stack-trace as follows (awaits removed):

at Microsoft.Identity.Client.Internal.ClientCredentialWrapper.get_Thumbprint()
at Microsoft.Identity.Client.Internal.JsonWebToken.JWTHeaderWithCertificate..ctor(ClientCredentialWrapper credential, Boolean sendCertificate)
at Microsoft.Identity.Client.Internal.JsonWebToken.EncodeHeaderToJson(ClientCredentialWrapper credential, Boolean sendCertificate)
at Microsoft.Identity.Client.Internal.JsonWebToken.Encode(ClientCredentialWrapper credential, Boolean sendCertificate)
at Microsoft.Identity.Client.Internal.JsonWebToken.Sign(ClientCredentialWrapper credential, Boolean sendCertificate)
at Microsoft.Identity.Client.Internal.Requests.ClientCredentialHelper.CreateClientCredentialBodyParameters(ICoreLogger logger, ICryptographyManager cryptographyManager, ClientCredentialWrapper clientCredential, String clientId, AuthorityEndpoints endpoints, Boolean sendX5C)
at Microsoft.Identity.Client.Internal.Requests.RequestBase.<SendTokenRequestAsync>d__21.MoveNext()
at Microsoft.Identity.Client.Internal.Requests.AuthorizationCodeRequest.<ExecuteAsync>d__3.MoveNext()
at Microsoft.Identity.Client.Internal.Requests.RequestBase.<RunAsync>d__14.MoveNext()
at Microsoft.Identity.Client.ApiConfig.Executors.ConfidentialClientExecutor.<ExecuteAsync>d__2.MoveNext()

Possible Solution
I had a look at the master (512d74e) and at RequestBase.cs line 300 there is a condition if (AuthenticationRequestParameters.ClientCredential != null) which must be succeeding to get that stack trace. I would have expected this to be null as I'm executing AcquireTokenByAuthorizationCode and not a client credentials grant.

The WebApp-OpenIDConnect-DotNet sample includes .WithClientSecret(AzureAdB2COptions.ClientSecret) and using this makes the exception go away, but doesn't resolve the problem because it generates an error in B2C: "'AADB2C90079: Clients must send a client_secret when redeeming a confidential grant".

@winterlimelight winterlimelight changed the title [Bug] [Bug] Null reference exception calling AcquireTokenByAuthorizationCode Oct 28, 2019
@bgavrilMS
Copy link
Member

bgavrilMS commented Oct 28, 2019

Null refs are always bugs. Suggest P1.

@jmprieur
Copy link
Collaborator

@winterlimelight, in a confidential client, you need to provide either a client secret or a client certificate. See https://docs.microsoft.com/en-us/azure/active-directory-b2c/tutorial-register-applications#create-a-client-secret, add it to the config file, and then in the code use:

IConfidentialClientApplication cca = ConfidentialClientApplicationBuilder
	.Create(AzureAdB2COptions.ClientId)
	.WithRedirectUri(AzureAdB2COptions.RedirectUri)
	.WithB2CAuthority(AzureAdB2COptions.Authority)
        .WithClientSecret(AzureAdB2COptions.ClientSecret)
	.Build();

Note that you are not using a client credential grant (that's not a daemon app), but you're use a confidential client application (a Web app). It requires client credentials. See https://docs.microsoft.com/azure/active-directory/develop/msal-net-initializing-client-applications#initializing-a-confidential-client-application-from-code

@bgavrilMS : fair enough that we can have a meaningful message that for confidental client apps, the client secret or cert or client assertion is needed (I thought we did already)

@winterlimelight
Copy link
Author

When I included .WithClientSecret(AzureAdB2COptions.ClientSecret) I got an error from B2C: "'AADB2C90079: Clients must send a client_secret when redeeming a confidential grant".

@winterlimelight
Copy link
Author

ClientSecret is set and has a value from Azure Portal > Azure AD B2C - Applications > (app-name) - Keys. I am not using MSALPerUserMemoryTokenCache.

@henrik-me
Copy link
Contributor

@winterlimelight : I'm not sure I understand your scenario. What are you ultimately trying to achieve here? To me it seems a bit like we are mixing confidential client with the auth code for a public client scenario. Can you pls. help with some details on the scenario you are trying to achieve? I think I know but I would really like this to be explicit.

@winterlimelight
Copy link
Author

winterlimelight commented Oct 30, 2019

I have Azure B2C as an authorization server. I have a Razor Pages website, most of which requires Authorization. My intention is to remain stateless, so I want to get to a point where each request contains a signed access token created by B2C that identifies the user.

The idea is that when the user reaches an authorized page, they need to be sent to B2C to authenticate, and that finish up sending tokens to the razor pages back-end, which will then return the access token to the user to send with a future request.

Being Razor Pages, my client has the ability to store confidential information, so it is therefore a confidential client as defined by RFC 6749 section 2.1, and authorization code grant seems suitable as section 4.1 indicates: "The authorization code grant type is used to obtain both access tokens and refresh tokens and is optimized for confidential clients" (RFC 6749, sect 4.1).

Basically I'm looking to do this: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/Acquiring-tokens-with-authorization-codes-on-web-apps but using B2C and without all the token caching.

@winterlimelight
Copy link
Author

I think I've resolved my issue.

Somewhere in the process of copy-n-paste I'd lost the line context.HandleCodeRedemption(result.AccessToken, result.IdToken);.
This, I now understand, is important because we are replacing the OpenIdConnect default handling with MSAL, and therefore have to tell it we've done so by calling HandleCodeRedemption, otherwise it tries to do it itself, and being misconfigured for default handling, sends back an error.

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 6, 2019

Thanks for the update, @winterlimelight
Ideally, if we had repro steps, we could avoid the null reference exception, but since you're unblocked proposing to close this issue (unless we get clear repro steps)

@rijulluman-msft
Copy link

I am facing a similar issue currently

---> (Inner Exception #0) System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.Identity.Client.Internal.ClientCredentialWrapper.get_Thumbprint()
   at Microsoft.Identity.Client.Internal.JsonWebToken.JWTHeaderWithCertificate..ctor(ClientCredentialWrapper credential, Boolean sendCertificate)
   at Microsoft.Identity.Client.Internal.JsonWebToken.Encode(ClientCredentialWrapper credential, Boolean sendCertificate)
   at Microsoft.Identity.Client.Internal.JsonWebToken.Sign(ClientCredentialWrapper credential, Boolean sendCertificate)
   at Microsoft.Identity.Client.Internal.Requests.ClientCredentialHelper.CreateClientCredentialBodyParameters(ICoreLogger logger, ICryptographyManager cryptographyManager, ClientCredentialWrapper clientCredential, String clientId, AuthorityEndpoints endpoints, Boolean sendX5C)
   at Microsoft.Identity.Client.OAuth2.TokenClient.<SendTokenRequestAsync>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.<SendTokenRequestAsync>d__21.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.ClientCredentialRequest.<FetchNewAccessTokenAsync>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.ClientCredentialRequest.<ExecuteAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.<RunAsync>d__14.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Identity.Client.ApiConfig.Executors.ConfidentialClientExecutor.<ExecuteAsync>d__3.MoveNext()<---

@rmotyka
Copy link

rmotyka commented Mar 19, 2020

In appsettings.json in AzureAd add ClientSecret with proper value it helped me with the same exception.

@pdemro
Copy link

pdemro commented Mar 27, 2020

In appsettings.json in AzureAd add ClientSecret with proper value it helped me with the same exception.

Make sure your ClientSecret/ClientId are not missing! This was my issue as well.

@jmprieur jmprieur transferred this issue from AzureAD/microsoft-authentication-library-for-dotnet Mar 28, 2020
@jmprieur jmprieur reopened this Mar 28, 2020
@jmprieur
Copy link
Collaborator

Re-opening to investigate.
Be more robust on ClientSecret / ClientID

@kdblocher
Copy link

@pdemro @rmotyka this was my issue as well - I was storing my ClientSecret in the user secrets area and after a folder rename / environment change I started getting this exception.

I think informing the caller of a lack of the client secret far earlier would help quite a bit. The error message is quite cryptic.

@jmprieur jmprieur added enhancement New feature or request supportability and removed investigate labels Apr 15, 2020
@jmprieur jmprieur changed the title [Bug] Null reference exception calling AcquireTokenByAuthorizationCode [Bug] Null reference exception calling AcquireTokenByAuthorizationCode when client secret is not specified (need to improve error message) Apr 15, 2020
@jmprieur jmprieur added this to the 0.2 - preview milestone Apr 15, 2020
@jmprieur
Copy link
Collaborator

jmprieur commented May 7, 2020

@winterlimelight
@rijulluman-msft
@rmotyka
@pdemro

Fixed in Microsoft.Identity.Web 0.1.2-preview (released today)

@jmprieur jmprieur closed this as completed May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants