feature: activate PKCE by default in requests to external OIDC providers

[strehle]

PKCE for OIDC/OPENID clients is available now since years in UAA but currently only on/active for public usage Add now an own option to have it on also for all other authentications. Default on, but if a client does not want it, you can switch it off. See https://oauth.net/2/pkce/,
PKCE is not only for public usages but it is part of standard frameworks (spring and golang)

https://oauth.net/2.1/

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/185865696

The labels on this github issue will be updated when the story is started.

[strehle]

@peterhaochen47 this belongs to PKCE story, similar to cloudfoundry/cf-uaa-lib#90 , because it is PKCE as client

[peterhaochen47]

To clarify, this PR is only about when UAA is acting as an OAuth client, authenticating with an external OAuth server (as opposed to UAA clients authenticating with UAA), correct? In that case, if the external OAuth server does NOT support PKCE, would this be a breaking change?

Also, when an external OAuth server does not support PKCE, would it just ignore the code_verifier & code_challenge and still issue the token/code, or would it fail the auth request?

[strehle]

To clarify, this PR is only about when UAA is acting as an OAuth client, authenticating with an external OAuth server (as opposed to UAA clients authenticating with UAA), correct? In that case, if the external OAuth server does NOT support PKCE, would this be a breaking change?

not if the external OIDC provider is standard complaint and even with old UAA I could not find an issue. The parameters should be ignored, if the server does not support it.

Also, when an external OAuth server does not support PKCE, would it just ignore the code_verifier & code_challenge and still issue the token/code, or would it fail the auth request?

Ignore it.

PKCE was designed to be downwards compatible and it only works if both parties support it.
UAA uses PKCE already if you have public usages, and this experience showed that we can add it also for other usages, so from my side I dont see a breaking change,
If you have a veto here, we can move to false as default, e.g. private boolean pkce = false;
But we should allow the PKCE usage also in combination of authenticated clients,

[peterhaochen47]

@strehle I see, thank you. I tried to edit the PR title to clarify what this feature is.

My question is similar to this one about cf-uaac:
If, as you said, the PKCE is fully backward compatible (both clients and auth-server have to support it for it to take effect; and the auth request will not fail if one of them does not support it), then what is the purpose of adding the config config.pkce from the user perspective? Why not just always try to use PKCE?

[strehle]

@strehle I see, thank you. I tried to edit the PR title to clarify what this feature is.

My question is similar to this one about cf-uaac: If, as you said, the PKCE is fully backward compatible (both clients and auth-server have to support it for it to take effect; and the auth request will not fail if one of them does not support it), then what is the purpose of adding the config config.pkce from the user perspective? Why not just always try to use PKCE?

in UAAC I am with you to enable PKCE always because uaac is for UAA only and here I can say it works. Therefore changed cloudfoundry/cf-uaac#123 to use always PKCE.

The OIDC flow is for other OIDC providers and there it might be not safe even if they should be compatible.
The new options is on by default, but if somebody see an error, then PKCE could be deactivated again. Simply because I dont know which OIDC provder is used I would go with this option, what do you think?

[peterhaochen47]

I see. My concern is that removing an added config is a lot of work (you have to deprecate it, and advance major version, etc.).
But if you believe that there're some unknown risks (even though theoretically there shouldn't be) then adding an option is fine. Your call!