Integrate current auth-username-password-form authenticator with passkeys isConditionalMediationAvailable #38781
Conversation
| * | ||
| * @author rmartinc | ||
| */ | ||
| public class PasskeysUsernamePasswordForm extends UsernamePasswordForm { |
There was a problem hiding this comment.
@rmartinc Nice work! I really like this!
IMHO, in terms of this authenticator, I'd rather think about the passkeys/webauthn functionality as a FEATURE of the UsernamePasswordForm without the need to create a new authenticator.
I'd rather have some checkbox in the UsernamPasswordForm config like 'Or use passkeys'.
AFAIK, we already have quite a lot of authenticators, and it might become unintuitive for admins to know what to use. By limiting the creation of new authenticators and rather enhancing existing ones, we might get better UX. Or maybe even provide some sort of categories for authenticators to filter them out a little bit, based on their usage (but out of the scope).
On the other hand, I understand we might potentially introduce some breaking change for the authenticator impls, but if these changes are quite straightforward, I would not be so scared about that.
WDYT? Or do you have any other concerns?
It's just my humble opinion on this. Let me know what you think.
Great work, though 👍
There was a problem hiding this comment.
Yep, we have managed that idea too. I was adding a configuration option (disabled by default) to add passkeys. It can be done that way too with no problem. But @mposolda preferred this way. 😄 I don't know. I'm OK with both ideas.
There was a problem hiding this comment.
Note that most probably we need to do the same for more authenticators. For example the auth-username-form (the one with only the username field) or the organization (the one used when organizations are enabled in the realm). So the decision will be repeated several times (we can have two variants for each authenticator or an option in each one).
There was a problem hiding this comment.
@rmartinc Yes, there are some similar authenticators created, indeed. Not sure about your plans, but having some general authenticator for these use cases with some additional features(orgs, passkeys, ...) seems more reasonable to me.
So, imagine a case when you'd like to provide the same functionality of passkey to the form authn organization. And what if there's another "feature" created that might be shared?
@mposolda What's your opinion on this? Of course, it might be out of scope of this PR, so we can discuss it somewhere else.
There was a problem hiding this comment.
@mabartos My original preference was possibly checking if we can use something like FormActionSPI . So something, which will allow to build the individual authentication pages as "building blocks", which consists of multiple different pieces . That is also what I've originally commented on the linked issue. That may allow using other similar things (like for example adding "social buttons" to any other authentication page).
Currrently form-action SPI is used only on registration form and I am not sure how feasible it is to use it in the other parts. According to @rmartinc , it might be complicated.
Having config option on every authenticator may work too, but it means that config option needs to be on every "variant" of the authenticator if I understand correctly? Finally having subclasses for each variant (EG. PasskeysUsernamePasswordForm , PasskeysUsernameForm) is also similar option. Both of these have some disadvantage that it needs to change multiple authenticator classes and either include configuration option to them OR introduce subclasses for every form variant where we want to add passkeys.
The "Config option on each authenticator" is probably easier for the administrators as they can just enable passkey. However it might be harder for the maintenance of the code as you need to change sources of every authenticator, which we want to decorate, and add "passkeys" option to it.
|
Rebased to current main. |
…/password with isConditionalMediationAvailable Closes keycloak#29596 Signed-off-by: rmartinc <rmartinc@redhat.com>
…or implementation Use the webauthn passwordless policy to enable passkeys at realm level Closes keycloak#29596 Signed-off-by: rmartinc <rmartinc@redhat.com>
|
OK, I have updated the PR with two changes:
Id nobody says something different I will move this PR out of draft if the CI passes successfully. Simplified steps now:
|
|
@tnorimat Do you please have any feedback to this PR? |
|
@mposolda Yes, I will review the PR in this week. |
|
Hi, thank you for this feature! I’ve tested the feature and I have two pieces of feedback:
|
|
@Romain7495 See issue #40021 for the first question. This issue is only for the username and password form. The rest of forms will be added in different issues. Regarding the second point it depends on the passkey. To be chosen the passkey should be discoverable and with user verification. But this is more a question for the browser than for us, we are using the JS method to ask for keys, the page relies on the browser once the method is called. Did you set the require discoverable to yes and the user verification to required in the policy? I suppose that with those two options the passkey will be created with the correct options or the creation will fail. |
|
@rmartinc thanks for your answser Yes I had already configured the webauthn passwordless policy like this (still same after removing and re creating usb passkeys) |
|
Thanks @Romain7495! I have been investigating the second issue and it seems the problem is intrinsic to the conditional UI interface. More or less it is explained in the following thread in reddit. In that thread they recommend to always add the modal UI too (a direct button to start the modal UI). With this PR we are using only conditional UI. With the authenticator steps you can always add another step with passwordless (try another way button). But I'm thinking about adding the button to start the modal directly. @tnorimat @mposolda WDYT? |
|
@rmartinc I read the reddit thread. I agree with your idea. It would be better if Conditional UI itself had supported the feature of picking up passkeys that satisfies some conditions (e.g., only device bound passkeys) we can set. |
Closes keycloak#29596 Signed-off-by: rmartinc <rmartinc@redhat.com>
|
OK, I have updated the PR with mainly two changes:
This is the login page now after adding the button: @Romain7495 Can you please test that using the new button you can select the usb key? |
|
@rmartinc it works as expected 👍 Thanks
|
|
If we have both icloud passkeys and device bound passkeys, icloud will prompt before the modal like in all other websites that I have tested 👌 after cancelling this icloud popup, we have : |
|
Thanks @Romain7495! @tnorimat @mposolda Please try to do a refresh review. I think that now this is more complete and we can continue the same idea in the rest of the username authenticators. |
|
@rmartinc @tnorimat @Romain7495 Thanks everyone for the review and testing! For me, passkeys work as expected on Ubuntu 24.04 and Chrome 136 and Google phone 8. The Firefox does not work for me on Ubuntu, however Firefox does not work also with https://www.passkeys.io/ as well. With Firefox, I can only use yubikey as 2nd-factor authenticator for WebAuthn (no user saved on passkeys, just a default setup of I hope to merge tomorrow morning CET time (around 12 hours from now). @tnorimat @Romain7495 Please tell me if you want to re-review and re-test this PR and you want more time. I will wait with merging if you let me know and want more time for review and test. Otherwise I hope to merge. |
|
@mposolda Thanks for the update. Everything looks good based on my testing. Tested on macOS (Apple M4):
Note: Mobile testing is limited without HTTPS (required for WebAuthn except on |
|
@Romain7495 @tnorimat Thanks for the additional reviews and the details about testing! |
|
Thanks all for your work, this feature is highly appreciated!! 🙏🙏🙏 |
…keys isConditionalMediationAvailable (keycloak#38781) Closes keycloak#29596 Signed-off-by: rmartinc <rmartinc@redhat.com>
Closes #29596
Draft PR for now that adds a new authenticator that extends the normal username password form authenticator with the
isConditionalMediationAvailablelogin for passkeys. The main ideas are the following:Passkeys Username Password Form.Steps to test this:
--features passkeys.Require discoverable credentialto Yes andUser verification requirementto required.Signing inpage of the account console.Little screencast from my windows VM (don't know why the cursor is lost, but you can see the idea). First normal password login, then the passkey login.
passkeys.mp4