update to msal4.30 + pkce support #1152
Conversation
| @@ -151,12 +152,14 @@ internal partial class TokenAcquisition : ITokenAcquisitionInternal | |||
| { | |||
| var application = GetOrBuildConfidentialClientApplication(); | |||
|
|
|||
| context.TokenEndpointRequest.Parameters.TryGetValue(OAuthConstants.CodeVerifierKey, out string? codeVerifier); | |||
There was a problem hiding this comment.
can move this inline, but left for testing.
...soft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
| @@ -151,12 +152,14 @@ internal partial class TokenAcquisition : ITokenAcquisitionInternal | |||
| { | |||
| var application = GetOrBuildConfidentialClientApplication(); | |||
|
|
|||
| context.TokenEndpointRequest.Parameters.TryGetValue(OAuthConstants.CodeVerifierKey, out string? codeVerifier); | |||
There was a problem hiding this comment.
We shouldn't remove line 434 of src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs
otherwise
.AddMicrosoftIdentityWebApp()
and then
services.ConfigureOptions<MicrosoftIdentityOptions>(options =>
{
options.UsePkce = false;
});would not do the right thing.
|
How do you know that this works? |
|
@bgavrilMS : you run the WebAppCallsWebApi projects and use the todo-list button to call the web API. The other thing to test is that if you override OpenIdConnectOptions.UsePkce to false, this still works (and it does) |
|
@bgavrilMS we discussed thursday, the testing would have to happen in MSAL.NET, which I think you added. I tested it manually, as we discussed, i put a break point and checked the code verifier was included and also checked when it's not. everything would break if it didn't work. I'm not testing cca.GetAuthorizationRequestUrl, as we don't use it. |
|
@jennyf19 - you have an E2E / Selenium test I believe. Will it use PKCE now? |
|
@bgavrilMS I don't understand what the issue is...? We use PKCE now always, so it just works. Not sure what you are asking. |
|
@jennyf19 great work, can we pick it up in a nightly build package or should we wait for next Nuget release? |
|
@tedvanderveen i've been focused on getting an enhancement out for handling multiple auth schemes....I will sync with @jmprieur and maybe we'll do a release with this and a few other completed items in 1.10 project board so we aren't holding things up for the larger enhancement work with auth schemes. |
|
@tedvanderveen actually, follow 1.9.2, which will have the PCKE included. We will try to have it out next week (1st week of May). |
|
@jennyf19 looks like 1.9.2 is ready to roll? 🤞🏽 Great work! |
|
@tedvanderveen 1.9.2 is out. should be on nuget shortly: Included in 1.9.2 release |
@jmprieur have only tested sign-in (obviously, to start), but thought you might be interested as well.
#470