Optionally enforce the OIDC 'sub' claim

[sberyozkin]

Fixes #35397.

This PR supports an optional enforcement of the tokens containing a sub claim, and adds tests to confirm that if the token and UserInfo sub claims do not match then 401 is returned.

Along the way, it optimizes the nonce verification added with one of the recent PRs, in #35039 (where an extra token parsing was done before the required token verification involving the same token parsed again). Now it is done in the same ``OidcProvider#verifyJwtTokenInternal` place.

[sberyozkin]

Thanks @gastaldi.

We've just had a brief chat with Pedro as well, concern is, one more property. Pedro is right but I'm just terrified of breaking user applications, the experience shows enforcing things for not-necessarily essential things, ex, we don't know if having sub is important to the actual endpoint, and with the large variety of supported providers there is a chance we'll break things if we enforce it by default.

Also, the linking of the id token sub and UserInfo is enforced in https://www.certification.openid.net/plan-detail.html?plan=RPUGzpD20SQYS&public=true, oidcc-client-test-userinfo-invalid-sub:

The client is expected to make a userinfo request  and verify the 'sub' 
value of the UserInfo Response by comparing it with the ID Token's 'sub'
 value. The client must identify the invalid 'sub' value and reject the 
UserInfo Response.

Lets wait till Pedro comments further

Cheers

[sberyozkin]

HI @pedroigor, I suppose, as far as the user experience is concerned, a property like the one added in this PR can be defaulted to true if we observe users keeping manually configuring it, but for the moment I'm hesitating. May be we should let it settle first

[pedroigor]

For the reasons you pointed out, we can't be so strict in regards to the standards so that we can support different OPs.

LGTM.

[sberyozkin]

Thanks @pedroigor, I'll keep it open till Monday in case we come up with some more comments, cheers

[sberyozkin]

Hey @pedroigor @gastaldi , as far as the docs are concerned, this is the one I have in mind, #32052, I'll have to find some time to prioritize

[geoand]

What's the status of this one?

[sberyozkin]

Hi @geoand I was about to merge, but then thought about adding 2 more unit tests, let me do it and I'll merge afterwards

[sberyozkin]

Added 2 simple unit tests to OidcProviderTest to complement the integration tests

[quarkus-bot]

Failing Jobs - Building aece027

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	Build	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 17
✔️	JVM Tests - JDK 20

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/devmode 

📦 integration-tests/devmode

✖ io.quarkus.test.devui.DevUIHibernateSearchSmokeTest.testGetNumberOfIndexedEntityTypes - More details - Source on GitHub

java.lang.RuntimeException: java.lang.OutOfMemoryError: Metaspace
	at io.quarkus.deployment.dev.IsolatedDevModeMain.accept(IsolatedDevModeMain.java:447)
	at io.quarkus.deployment.dev.IsolatedDevModeMain.accept(IsolatedDevModeMain.java:56)