Azure not supported for WebUI PKCE Auth Code Flow

[Kerwood]

Describe the bug

In the upcoming v2.10.0-rc1 release, @Marvin9 added support for using authorization code flow with PKCE in the webUI, which is lovely. ❤️

• feat: PKCE authentication flow for web logins #9890 #15889

Unfortunately Azure is non-compliant with RFC8414 standard because the /.well-known/openid-configuration endpoint is missing the code_challenge_methods_supported property.
The ArgoCD login flow is stopped checking for that specific property here.

argo-cd/ui/src/app/login/components/utils.ts

Lines 76 to 80 in 3224102

	if (!authorizationServer?.code_challenge_methods_supported?.includes('S256')) {
	throw new PKCELoginError('Authorization Server does not support S256 code challenge method');
	}

Other projects seem to have the same issue, like kubelogin.

• Flag to force PKCE int128/kubelogin#858

This was brought to the attention of Microsoft in January 2021 and I do not think that they intend to do anything about it.

• https://learn.microsoft.com/en-us/answers/questions/218113/openid-connect-authorization-code-flow-with-proof

Proposal

Maybe have a setting something like, forcePKCECodeChallengeMethod: S256 that would skip the code_challenge_methods_supported property check, or just skipPKCECodeChallengeMethod: true ?

[Kerwood]

@michael-basil

[michael-basil]

I have verified the same issue and agree with the suggestion of a flag to mindfully force the flow.

🙏

[Marvin9]

What if we know in the frontend itself if IdP is Azure then we by default skip the error assuming that PKCE still works even if /.well-known/openid-configuration omit code_challenge_methods_supported. I don't see benefit of additional flag for exception like this. Any thoughts on it?

[michael-basil]

It would seem this was debated a fair amount with the kubelogin project (dropping the link again):

• Flag to force PKCE int128/kubelogin#858

That project chose to go with: "--oidc-use-pkce"

To follow this convention that others in the industry may be conditioned for the following flag (or similar) would seem to follow suit:

• skipPKCECodeChallengeMethodVerification

[Kerwood]

What if we know in the frontend itself if IdP is Azure then we by default skip the error assuming that PKCE still works even if /.well-known/openid-configuration omit code_challenge_methods_supported. I don't see benefit of additional flag for exception like this. Any thoughts on it?

Personally I think that creating a snowflake solution for a specific provider is a bad idea. This could end up with all sorts of snowflake code for different IDP's. I think it would be better to create a generic setting.

@michael-basil are you proposing that the setting shoud be skipPKCECodeChallengeMethodVerification ?

[michael-basil]

I am suggesting the same and with the name to skip verification or would be simpler in configuration management over time.

For example, we test before we introduce new Argo versions, but the typical engineer wouldn't know the details of PKCE.

They would know that it doesn't work though and then we really are in the weeds at that point anyway.

So I am partial to the basic approach of kubelogin though not fixed on the name, just what came naturally.

[michael-basil]

Implementation of both flags as optional.

One flag to skip verification and one flag to force a method ...

This is the most robust and durable after thinking it through further.

[Kerwood]

"Skip code challenge verification" sounds good to me.

[michael-basil]

Wouldn't the proof of key code exchange be the challenge?

This is skipping the method verification.

Perhaps I am understanding this differently?

[Kerwood]

tl;dr
I think either

• there should not be a frontend check for code_challenge_methods_supported property
• or there should be a setting to skip the check
• or there should be a setting to set the challenge method manually and therefore the check is skipped.

[Marvin9]

@michael-basil can you enable auth in api server and then run? You can either use ARGOCD_E2E_DISABLE_AUTH environment variable or use --disable-auth flag in api-server of test/container/Procfile.

[michael-basil]

The flag is set to false in the Makefile:

ARGOCD_E2E_DISABLE_AUTH=false \

Are you suggesting to set the value to true. Seems counterintuitive on first glance.

I figure when I can see the debug messages I will be able to trace through the code to see where it's going haywire.

[Marvin9]

What I meant is set DISABLE_AUTH to false.

I usually don't run that way so I might be wrong. But what I see in Makefile is ARGOCD_E2E_DISABLE_AUTH=false \ in start-e2e-local. Since you run make start, you would need to either set false in global variable used in run-in-test-server.

If you already tried and issue persist then let me know if I can help.

[Kerwood]

@michael-basil
I don't mean to be rude but you are getting a bit off topic here. You are posting a lot of information that doesn't really have anything to do with the main issue. Please keep your posts relevant.

The main issue/discussion here is: PKCE auth flow in not working with Azure because the /.well-known/openid-configuration endpoint does not support the code_challenge_methods_supported property. Which setting should we implement to skip the .includes('S256') check.

Here's a few proposals.

• there should not be a frontend check for code_challenge_methods_supported property
• or there should be a setting to skip the check
• or there should be a setting to set the challenge method manually and therefore the check is skipped.

[michael-basil]

@Kerwood in response to your suggestion I deleted a couple of my posts (as GitHub comments can be refactored):

TL/DR:

• I support the proposals you have outlined
• I have verified the viability of the first proposal described as there should not be a frontend check with an Azure IDP:
  • Running argocd and Kubernetes locally from my machine (via make start)
  • Running argocd as an image deployed to a remote Kubernetes cluster aka (via make image)

In alignment with the code contribution sentiment of sometimes it's helpful to have some PoC code along with a proposal I am working towards a draft pull request to support the described proposal(s) to help bolster the case.

If anyone would like to collaborate to pair on this coding effort please reach out in CNCF Slack by DM or #argo-contributors.

[michael-basil]

I tested this fix off of the master branch. It worked well! Would someone be able to point me to determine when this would go into a release?