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

[Identity] Migrate AuthorizationCodeCredential to use MSAL #15188

Closed
sadasant opened this issue May 8, 2021 · 3 comments · Fixed by #15851
Closed

[Identity] Migrate AuthorizationCodeCredential to use MSAL #15188

sadasant opened this issue May 8, 2021 · 3 comments · Fixed by #15851
Assignees
Labels
Azure.Identity blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. tracking-external-issue

Comments

@sadasant
Copy link
Contributor

sadasant commented May 8, 2021

AuthorizationCodeCredential currently does not use MSAL. It uses the older approach we had, where we called to the HTTP methods ourselves directly. Moving this to MSAL should be very easy, since we'll be using MSAL's acquireTokenByCode, HOWEVER, acquireTokenByCode does not implement PKCE by default, we'll need to do the same we'll be doing for InteractiveBrowserCredential for Node, meaning this issue: #15168 (we should probably re-use code for this, of course).

@sadasant sadasant added Client This issue points to a problem in the data-plane of the library. Azure.Identity blocking-release Blocks release labels May 8, 2021
@sadasant sadasant added this to the [2021] June milestone May 8, 2021
@sadasant sadasant self-assigned this May 8, 2021
@sadasant sadasant modified the milestones: [2021] June, [2021] July May 12, 2021
@sadasant
Copy link
Contributor Author

sadasant commented Jun 23, 2021

I think I found a bug on MSAL, but I could be wrong.


When using acquireTokenByCode, specifically the example here: lib/msal-common/docs/request.md. If I build the auth code URL instead of calling to MSAL’s getAuthCodeUrl(), meaning, if I use an URL similar to the following during the redirect: https://login.microsoftonline.com/<tenant>/oauth2/v2.0/authorize?client_id=<client-id>&response_type=code&redirect_uri=<redirect-uri>&scope=<scope>

and then I call to the confidential app’s acquireTokenByCode with parameters:

{
  scopes: [ 'https://vault.azure.net/.default' ],
  redirectUri: 'http://localhost:8080/authresponse',
  code: '<received-code>'
}

I get an undefined result?

@sadasant
Copy link
Contributor Author

At this point, I'm seeing two things:

  • Neither of msal-node's methods on the confidential client, getAuthCodeUrl, and acquireTokenByCode, let me change the tenant ID in the URL they work with.
  • Nonetheless, acquireTokenByCode seems to be sending the right request to the server with the client_secret, but the service is giving me this error: AADSTS700025: Client is public so neither 'client_assertion' nor 'client_secret' should be presented.

I've shared the code, my local configuration, and the configuration I'm using on the Azure portal with the MSAL team and with the .NET SDK team and everything seems correct. We don't yet understand what is going on. It seems likely to be an issue with how my app registration is set up, but it could be something else, or even a bug in the code, either on how the requests are represented or on the flow.

I've reached out to the MSAL team asking them if they could provide us some small documentation on how to set up acquireTokenByCode for the confidential client. Once we have more information, I'll continue working on this issue.

@sadasant sadasant modified the milestones: [2021] July, Backlog Jun 25, 2021
@sadasant
Copy link
Contributor Author

sadasant commented Aug 9, 2021

The code in #15851 should work. I did test it some months ago, but I’m asking @KarishmaGhiya to verify.

@xirzec xirzec removed this from the Backlog milestone May 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. tracking-external-issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants