sorry, deleted the branch by mistake

Just a side note from a developer experience: extending Keycloak's admin UI is more than hard if you are not a very well experienced JS developer. You should definitely improve here and make it easier for everyone to extend/contribute!

@keycloak/core-clients - can you help out @dasniko with the test, and also comment on if this should be configurable as he describes, or if it could be the new default?

@mposolda @mabartos @rmartinc Would you mind having a look into this PR and the detailed explanation in the corresponding issue and give me a first feedback!?

I can remove the switch again, or make it a select-box, as there are several options according to the spec (see #46959 (comment)).
We can also discuss if we make this behavior the new default (although it'd change current behavior). I'm open to many options.

Currently, also the documentation is still missing. I will add this once we decided how to move on.

I'd really appreciate to get this merged into core, as I see this behavior as a default on more and more websites using passkeys!

Thanks @dasniko for the PR! Trying to answer your questions:

I can remove the switch again, or make it a select-box, as there are several options according to the spec (see #46959 (comment)).

I see the switch OK if we don't want to support more options. But, on the other hand, if later the spec defines more options (for example the immediate explained here) or the other options (silent, required) are required by someone, a select is much more appropriate. So initially I'm more in favor of adding the select box and make it easier to be expanded with more options if needed.

We can also discuss if we make this behavior the new default (although it'd change current behavior). I'm open to many options.

I would maintain the same default conditional for the moment, even the default is optional in the spec. In order to not change the current behavior. I'm open if you think different, but initially I wouldn't change the current default behavior.

Regarding the tests, the tests for passkeys are currently in the old testsuite in this package. I'm moving part of them to the new TS in this other PR #47940. But I don't know if we can test this, because the selenium configuration (when using mediation) always performs the login automatically using the passkey, so probably both options (conditional and optional) will work in the same way. But, at least, one of the tests (for example the default username/password PasskeysUsernamePasswordFormTest) should test that both options (conditional and optional) work OK.

@rmartinc Thanks for your response.
I'll change the implementation to the mediation-select, makes sense.

Regarding the tests, I can see in your PR that PasskeysUsernamePasswordFormTest is moved from the old to the new Test-Framework. So, it would make sense to wait until your PR is merged, rebase my PR and then see what and how is possible to test, right? I don't necessarily want to implement any Selenium tests anymore...

@dasniko yes, it makes sense, but I don't know when PR #47940 will be reviewed and merged. Let's see if it does not take much time. You can just duplicate the discoverable tests (for example webauthnLoginWithDiscoverableKey in both tests) but using the non-default configuration (optional instead of conditional). You can create an intermediary private/protected method and just change the configuration option value. I have not tested but my impression is that selenium is going to not differentiate them (because, as you see in the tests, when a discoverable key is used with the correct options, the passkey is directly used, so I suppose is going to perform the same automatic login).

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.model.session.UserSessionProviderModelTest#testCreateUserSessionsParallel

Keycloak CI - Store Model Tests

java.lang.AssertionError: 
threads didn't terminate in time: [main (RUNNABLE):
	at java.management@25.0.2/sun.management.ThreadImpl.dumpThreads0(Native Method)
	at java.management@25.0.2/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:505)
	at java.management@25.0.2/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:493)
...

Report flaky test

org.keycloak.testsuite.model.session.UserSessionProviderModelTest#testCreateUserSessionsParallel

Keycloak CI - Store Model Tests

java.lang.AssertionError: 
threads didn't terminate in time: [main (RUNNABLE):
	at java.management@25.0.2/sun.management.ThreadImpl.dumpThreads0(Native Method)
	at java.management@25.0.2/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:505)
	at java.management@25.0.2/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:493)
...

Report flaky test

@rmartinc I've extended your tests in the new TS and made them a @ParameterizedTest, please have a look.

The failing JavaScript tests/checks are not related to my changes, seems there's a failure with an npm lib!?

The new flaky tests are also not related to my changes.

@rmartinc Thanks for your feedback, valid points, also from CoPilot. I just pushed some fixes/improvements.

I don't like much the user experience in one aspect after this change. Set to optional for example, if the user decide to close the webauthn modal, the modal is shown again and again when the user performs any action, for example the user fails in the password. It's a bit annoying.

I don't know if we need to add something to not show the modal if the user declines to use the passkey when the same auth session. One possible solution was using the cookie KC_AUTH_SESSION_HASH and storing something in the local store. But I don't know if I'm over-reacting.

I understand your concerns. And I did some tests on other websites I know using the optional mediation directly. They behave the same: every time I get on the login page, no matter why, and I do have a registered passkey for this domain, I'll get the modal.
For me personally, that's not a problem, because if I do have a registered passkey for a domain/website, I do want to use it, I don't want to use other credential methods. But perhaps that's only my personal opinion!?
So, I'm completely open to whatever. IMHO this is not required, but if you decide to go with a proposed storage solution (I'd prefer the session storage, no the local storage), I'll also fine with it and will implement it.

@dasniko Thanks for the PR!

@rmartinc I'm about to look at this today.

@dasniko I've tried the solution, and first of all, very nice job!

However, I find the opening of the modal annoying too :/ I think we can be opinionated in this case, and rather improve the UX than provide a standard way of how others do it.

For the default browser flow(UnPwdForm), when the user puts in a wrong password (it happens many times; count me in as well), the modal is shown again and again - and it is very annoying.

every time I get on the login page, no matter why, and I do have a registered passkey for this domain, I'll get the modal.

@dasniko If you already have a registered passkey for the domain, it's somewhat expected that you'll use it, as it's almost frictionless. However, when there's a cross-platform device (like my phone), I don't want to be bothered by the modal so many times either (we need to show the modal for the first time to check if the cross-platform device has it registered). So, if the passwordless policy allows only the platform passkeys, and there's the registered passkey, I'm ok to show the modal again even when the user puts a wrong password, as the passkey could be perceived as a much better and easier method to log in. But, there might be some other user trying to log in from the same device (public places, library,...) - So, even maybe in this case, I'd not always show the modal (even when the password is incorrectly put).

Summary: I think it'd be good to show it once(per authn session?), and when the user clicks 'Cancel', just do not show it again. If they want to change their minds, they still can use the button 'Sign in with passkeys', which has great visibility on the login page.

So, agree with the @rmartinc view on this :) But it's just my 2 cents.

WDYT?

Summary: I think it'd be good to show it once(per authn session?), and when the user clicks 'Cancel', just do not show it again. If they want to change their minds, they still can use the button 'Sign in with passkeys', which has great visibility on the login page.

As already mentioned, I'm totally fine with keeping the modal closed if the user closes it manually. So, I'm going to implement it.

Done, I added the dismissal check.
I pushed it in a separate commit, not yet squashed, for easier review.

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.model.singleUseObject.SingleUseObjectModelTest#testCluster

Keycloak CI - Store Model Tests

java.lang.AssertionError: 
threads didn't terminate in time: [main (RUNNABLE):
	at java.management@25.0.2/sun.management.ThreadImpl.dumpThreads0(Native Method)
	at java.management@25.0.2/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:505)
	at java.management@25.0.2/sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:493)
...

Report flaky test

org.keycloak.testsuite.model.user.UserModelTest#testAddDirtyRemoveFederationUsersInTheSameGroupConcurrent

Keycloak CI - Store Model Tests

java.lang.NullPointerException: Cannot invoke "org.keycloak.models.KeycloakSessionFactory.create()" because "factory" is null
	at org.keycloak.models.utils.KeycloakModelUtils.runJobInTransactionWithResult(KeycloakModelUtils.java:463)
	at org.keycloak.models.utils.KeycloakModelUtils.runJobInTransactionWithResult(KeycloakModelUtils.java:447)
	at org.keycloak.testsuite.model.KeycloakModelTest.inComittedTransaction(KeycloakModelTest.java:590)
	at org.keycloak.testsuite.model.KeycloakModelTest.inComittedTransaction(KeycloakModelTest.java:586)
...

Report flaky test

org.keycloak.testsuite.model.session.UserSessionPersisterProviderTest#testOnRealmRemoved

Keycloak CI - Store Model Tests

org.infinispan.remoting.RemoteException: ISPN000217: Received exception from node-50, see cause for remote stack trace
	at org.infinispan.remoting.transport.ResponseCollectors.wrapRemoteException(ResponseCollectors.java:26)
	at org.infinispan.remoting.transport.ValidSingleResponseCollector.withException(ValidSingleResponseCollector.java:37)
	at org.infinispan.remoting.transport.ValidSingleResponseCollector.addResponse(ValidSingleResponseCollector.java:21)
	at org.infinispan.remoting.transport.impl.SingleTargetRequest.addResponse(SingleTargetRequest.java:69)
...

Report flaky test

JFYI - As a follow-up, we could also not show the modal when the 'Cross-platform' passkeys are disabled, and there's no platform authenticator available (PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable()).

Because what's the reason to show the modal when we know only the platform authenticator can be used, but there's no way to use it?

@dasniko @rmartinc WDYT?

JFYI - As a follow-up, we could also not show the modal when the 'Cross-platform' passkeys are disabled, and there's no platform authenticator available (PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable()).

Because what's the reason to show the modal when we know only the platform authenticator can be used, but there's no way to use it?

@dasniko @rmartinc WDYT?

Yeah, makes sense, IMHO.
Would you create the follow-up issue?

JFYI - As a follow-up, we could also not show the modal when the 'Cross-platform' passkeys are disabled, and there's no platform authenticator available (PublicKeyCredential.isUserVerifyingPlatformAuthenticatorAvailable()).
Because what's the reason to show the modal when we know only the platform authenticator can be used, but there's no way to use it?
@dasniko @rmartinc WDYT?

Yeah, makes sense, IMHO. Would you create the follow-up issue?

Yes, we don't want to bother you more in this PR. We will create another issue if we think more checks are needed.

Hopefully everything is now contained.
Should I squash the commits and rebase on main? Or will you do it when merging?

Would you create the follow-up issue?

Related to this:

• Best practices for detecting support for Passkeys - support for isUserVerifyingPlatformAuthenticatorAvailable()  #29558

@dasniko Could you please resolve the conflict? Thanks!