[OID4VCI] Migrate OID4VCIssuerWellKnownProviderTest by tdiesler · Pull Request #47313 · keycloak/keycloak

tdiesler · 2026-03-20T09:41:29Z

Ogenbertrand

Hello @tdiesler !
Thanks for the PR, i left a few comments could you check them ?

Ogenbertrand · 2026-03-24T13:54:23Z

        String acceptHeader = session.getContext().getRequestHeaders().getHeaderString(HttpHeaders.ACCEPT);
        boolean preferJwt = acceptHeader != null && acceptHeader.contains(MediaType.APPLICATION_JWT);
-        boolean signedMetadataEnabled = Boolean.parseBoolean(realm.getAttribute(SIGNED_METADATA_ENABLED_ATTR));
+        boolean signedMetadataEnabled = realm.getAttribute(SIGNED_METADATA_ENABLED_ATTR, true);


Defaulting this to true changes signed issuer metadata from opt-in to opt-out.

Yes, and to be honest I don't quite understand why "opt-out" is even necessary. The caller decides (with the Accept header) which format they want.

Is there a reason why we would deny signed metadata when asked for?

Thanks for the feedback @tdiesler !

This is my understanding of this part from the spec, I may be wrong please correct me.

Accept: application/jwt only tells the issuer that the wallet can consume signed metadata, it does not require the issuer to return it. In 11.2.2 the spec makes unsigned JSON mandatory and signed metadata optional: the issuer MUST support application/json, but only MAY support signed metadata, and matching the requested media type is only RECOMMENDED when supported. Then 11.2.3 treats signed metadata as a trust-bearing artifact that the wallet must validate and trust, so this is an issuer capability/policy choice, not just a formatting choice. Because of that, defaulting SIGNED_METADATA_ENABLED_ATTR to true is still a behavioral change from opt-in to opt-out, the spec allows signed metadata, but it does not require us to enable it by default.

Sure, but we do support it. Hence, we can match the requested media type as RECOMMENDED. I'm looking for a valid reason on why it is beneficial to turn it off, is there?

No further points thanks for clarifying me.

What do we do here ? Leave it as is or remove SIGNED_METADATA_ENABLED_ATTR altogether?

I'll say we should remove it SIGNED_METADATA_ENABLED_ATTR.

+1 for remove SIGNED_METADATA_ENABLED_ATTR . In case that caller uses Accept: application/jwt request header, the response might be signed. With any other Accept header (or without Accept), it might be better to respond with unsigned response. That is also what is done by OIDC well-known endpoint AFAIK.

If the attribute is removed in this PR, it would be needed to update also admin console UI and remove the attribute from the UI and make sure that attributes like "Signed Metadata Lifespan" and "Signed Metadata Signing Algorithm" are always visible in the admin console UI

+1 for remove SIGNED_METADATA_ENABLED_ATTR . In case that caller uses Accept: application/jwt request header, the response might be signed. With any other Accept header (or without Accept), it might be better to respond with unsigned response. That is also what is done by OIDC well-known endpoint AFAIK.

If the attribute is removed in this PR, it would be needed to update also admin console UI and remove the attribute from the UI and make sure that attributes like "Signed Metadata Lifespan" and "Signed Metadata Signing Algorithm" are always visible in the admin console UI

I'll handle this in the UI @mposolda !

tdiesler · 2026-03-25T14:09:32Z

@Ogenbertrand is this good to go or shall we remove SIGNED_METADATA_ENABLED_ATTR?

mposolda

Added comment inline to the existing conversation.

mposolda · 2026-03-26T07:34:13Z

        String acceptHeader = session.getContext().getRequestHeaders().getHeaderString(HttpHeaders.ACCEPT);
        boolean preferJwt = acceptHeader != null && acceptHeader.contains(MediaType.APPLICATION_JWT);
-        boolean signedMetadataEnabled = Boolean.parseBoolean(realm.getAttribute(SIGNED_METADATA_ENABLED_ATTR));
+        boolean signedMetadataEnabled = realm.getAttribute(SIGNED_METADATA_ENABLED_ATTR, true);


+1 for remove SIGNED_METADATA_ENABLED_ATTR . In case that caller uses Accept: application/jwt request header, the response might be signed. With any other Accept header (or without Accept), it might be better to respond with unsigned response. That is also what is done by OIDC well-known endpoint AFAIK.

If the attribute is removed in this PR, it would be needed to update also admin console UI and remove the attribute from the UI and make sure that attributes like "Signed Metadata Lifespan" and "Signed Metadata Signing Algorithm" are always visible in the admin console UI

Ogenbertrand · 2026-03-26T12:12:01Z

I opened this ticket for the UI @mposolda, @tdiesler: #47476.
If you approve, i'll start working on it.

tdiesler · 2026-03-26T13:02:47Z

I opened this ticket for the UI @mposolda, @tdiesler: #47476. If you approve, i'll start working on it.

Can we get this merged without the UI work?

mposolda · 2026-03-27T09:48:23Z

I opened this ticket for the UI @mposolda, @tdiesler: #47476. If you approve, i'll start working on it.

Can we get this merged without the UI work?

Will be better to rather not merge this without UI work as then we effectively introduce bug by merging this PR (due the fact that admin console UI would have a switch with no functionality and meaning).

@Ogenbertrand If it is OK for you, you can possibly cherry-pick commits by @tdiesler from this branch and add your commit(s) on top of it with updates to UI. Then you send the dedicated PR with both changes from this PR and UI changes. Then we can close this PR. Does this work?

mposolda · 2026-03-27T10:02:22Z

I opened this ticket for the UI @mposolda, @tdiesler: #47476. If you approve, i'll start working on it.

Can we get this merged without the UI work?

Will be better to rather not merge this without UI work as then we effectively introduce bug by merging this PR (due the fact that admin console UI would have a switch with no functionality and meaning).

@Ogenbertrand If it is OK for you, you can possibly cherry-pick commits by @tdiesler from this branch and add your commit(s) on top of it with updates to UI. Then you send the dedicated PR with both changes from this PR and UI changes. Then we can close this PR. Does this work?

Just seeing that you already sent the UI PR #47515 :-) Addressing UI changes beforehand works as well instead of both changes (Server and UI) in the same PR. I hope I can merge this PR once UI PR #47515 is OK and merged.

Ogenbertrand · 2026-03-27T10:06:29Z

I opened this ticket for the UI @mposolda, @tdiesler: #47476. If you approve, i'll start working on it.

Can we get this merged without the UI work?

Will be better to rather not merge this without UI work as then we effectively introduce bug by merging this PR (due the fact that admin console UI would have a switch with no functionality and meaning).

@Ogenbertrand If it is OK for you, you can possibly cherry-pick commits by @tdiesler from this branch and add your commit(s) on top of it with updates to UI. Then you send the dedicated PR with both changes from this PR and UI changes. Then we can close this PR. Does this work?

I missed this comment @mposolda, i'll focus on addressing any comment i recieve here: #47515 today, sorry i missed your message.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2026-03-27T13:32:36Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testLoginAfterLogoutFromDifferentTab

Keycloak CI - Forms IT (chrome)

org.openqa.selenium.TimeoutException: 
timeout: Timed out receiving message from renderer: 10.000
  (Session info: chrome=146.0.7680.164)
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.17.0-1008-azure', java.version: '25.0.2'
...

java.lang.RuntimeException: Error during saving the screenshot!
	at org.arquillian.extension.recorder.screenshooter.browser.impl.BrowserScreenshooter.takeScreenshot(BrowserScreenshooter.java:127)
	at org.arquillian.extension.recorder.screenshooter.browser.impl.TakeScreenshotAndReportService.takeScreenshotAndReport(TakeScreenshotAndReportService.java:60)
	at org.arquillian.extension.recorder.screenshooter.browser.impl.ScreenshotTaker.onTakeScreenshot(ScreenshotTaker.java:79)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
...

Conversation

tdiesler commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ogenbertrand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ogenbertrand Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ogenbertrand Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tdiesler commented Mar 25, 2026

Uh oh!

mposolda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ogenbertrand commented Mar 26, 2026

Uh oh!

tdiesler commented Mar 26, 2026

Uh oh!

mposolda commented Mar 27, 2026

Uh oh!

mposolda commented Mar 27, 2026

Uh oh!

Ogenbertrand commented Mar 27, 2026

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Mar 27, 2026

Unreported flaky test detected

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testLoginAfterLogoutFromDifferentTab

org.keycloak.testsuite.forms.MultipleTabsLoginTest#loginActionWithoutExecution

org.keycloak.testsuite.forms.MultipleTabsLoginTest#multipleTabsParallelLoginTestWithAuthSessionExpiredAndRequiredAction

org.keycloak.testsuite.forms.MultipleTabsLoginTest#expiredAuthenticationAction_expiredCodeExpiredExecution

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testWithAuthSessionExpiredInTheMiddle_badRedirectUri

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testEmptyBaseUrl

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testRestartFailureWithDifferentClientAfterAuthSessionExpiration

org.keycloak.testsuite.forms.MultipleTabsLoginTest#expiredAuthenticationAction_currentCodeExpiredExecution

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testLogoutDifferentBrowserWithAuthenticationSessionStillPresent

org.keycloak.testsuite.forms.MultipleTabsLoginTest#multipleTabsLoginAndPassiveCheck

org.keycloak.testsuite.forms.MultipleTabsLoginTest#multipleTabsParallelLoginTestWithAuthSessionExpiredInTheMiddle

org.keycloak.testsuite.forms.MultipleTabsLoginTest#loginActionWithoutExecutionInRequiredActions

org.keycloak.testsuite.forms.MultipleTabsLoginTest#multipleTabsParallelLoginTestWithAuthSessionExpiredAndRefreshInTab1

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testLoginPageRefresh

org.keycloak.testsuite.forms.MultipleTabsLoginTest#loginWithDifferentClients

org.keycloak.testsuite.forms.MultipleTabsLoginTest#multipleTabsParallelLoginTestWithAuthSessionExpiredAndRegisterClick

org.keycloak.testsuite.forms.MultipleTabsLoginTest#multipleTabsParallelLoginTest

org.keycloak.testsuite.forms.MultipleTabsLoginTest#expiredAuthenticationAction_expiredCodeCurrentExecution

org.keycloak.testsuite.forms.MultipleTabsLoginTest#testInjectRedirectUriInClientDataAfterAuthSessionExpiration

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

keycloak-github-bot Bot commented Mar 27, 2026

Unreported flaky test detected

org.keycloak.testsuite.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled

tdiesler commented Mar 20, 2026 •

edited

Loading

Ogenbertrand Mar 25, 2026 •

edited

Loading

Ogenbertrand Mar 25, 2026 •

edited

Loading