Pass and use rememberMe option in passkeys authenticators#47940
Conversation
Closes keycloak#45104 Signed-off-by: rmartinc <rmartinc@redhat.com>
Closes keycloak#45104 Signed-off-by: rmartinc <rmartinc@redhat.com>
|
Rebased and conflicts resolved. There seems to be a lot of changes in the TS... 😄 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR propagates the rememberMe option through passkeys/WebAuthn authentication so “Remember me” behaves consistently when users authenticate via passkeys from forms that display the checkbox.
Changes:
- Add client-side forwarding of
rememberMefrom the visible checkbox to the WebAuthn (webauth) form submission. - Centralize server-side remember-me handling in
AuthenticatorUtilsand apply it to the WebAuthn passwordless authenticator. - Migrate/update passkeys tests to the newer
tests/webauthnmodule and extend coverage for remember-me with passkeys.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| themes/src/main/resources/theme/base/login/resources/js/webauthnAuthenticate.js | Adds forwarding of rememberMe via a hidden input on WebAuthn submit. |
| testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java | Removes old Arquillian-based test (migrated to new suite). |
| testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/webauthn/passwordless/PasskeysUsernameFormTest.java | Removes old Arquillian-based test (migrated to new suite). |
| tests/webauthn/src/test/java/org/keycloak/tests/webauthn/passwordless/PasskeysUsernamePasswordFormTest.java | New/updated tests including remember-me assertions for passkeys. |
| tests/webauthn/src/test/java/org/keycloak/tests/webauthn/passwordless/PasskeysUsernameFormTest.java | New/updated tests including remember-me at username/password steps. |
| tests/webauthn/src/test/java/org/keycloak/tests/webauthn/AbstractWebAuthnVirtualTest.java | Refactors realm setup for new tests, adds flows/users, adjusts required actions. |
| test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginUsernamePage.java | Adds username error/autocomplete accessors and remember-me helpers for username-only page. |
| test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/LoginPage.java | Adds password-only fill helper and exposes password-field error separately. |
| test-framework/ui/src/main/java/org/keycloak/testframework/ui/page/AbstractLoginPage.java | Adds accessors for shared info/error message banners. |
| test-framework/core/src/main/java/org/keycloak/testframework/realm/RealmConfigBuilder.java | Adds builder method for passwordless passkeys-enabled policy. |
| test-framework/core/src/main/java/org/keycloak/testframework/events/EventAssertion.java | Exposes asserted event via getEvent() for follow-up operations in tests. |
| services/src/main/java/org/keycloak/authentication/authenticators/util/AuthenticatorUtils.java | Centralizes remember-me form processing logic. |
| services/src/main/java/org/keycloak/authentication/authenticators/browser/WebAuthnPasswordlessAuthenticator.java | Processes rememberMe when present in WebAuthn passwordless submissions. |
| services/src/main/java/org/keycloak/authentication/authenticators/browser/AbstractUsernameFormAuthenticator.java | Switches to the shared remember-me processor and adjusts brute-force helper reference. |
| loginPage.submit(); | ||
| loginPage.assertCurrent(); | ||
| MatcherAssert.assertThat(loginPage.getUsernameAutocomplete(), Matchers.is("username webauthn")); | ||
| MatcherAssert.assertThat(loginPage.getUsernameInputError(), Matchers.is("Invalid username or email.")); |
There was a problem hiding this comment.
Out-of-scope: Does it mean that we can do the username enumeration with the UsernameForm?
There was a problem hiding this comment.
Yes, if you set up the username (alone) authenticator you are allowing user enumeration instead of the username/password. The same that happens in google and other sites that uses the separated username form.
There was a problem hiding this comment.
@rmartinc It's quite unfortunate, but not sure if we could provide a better approach here and propagate the invalid user to credential authenticators/validators... it might be quite difficult and fragile... Do we know if we at least inform the administrators about the possible enumeration?
There was a problem hiding this comment.
Maybe, but in general if you use the separate username form it is more or less expected that user enumeration is present. If you avoid that (for example allowing any username at that page and giving the error later in the password), the user experience is bad (because a user that types the username wrong becomes crazy to detect that the error was in the previous page). But this is unrelated to the present issue.
Closes keycloak#45104 Signed-off-by: rmartinc <rmartinc@redhat.com>
|
@keycloak/core-clients-maintainers @keycloak/core-iam-maintainers Do you want to review this one? |
|
It should not break orgs because we recently added a test for remember me when using org flow. |
Closes #45104
The PR adds the
rememberMefor passkeys in the forms that already show the remember me checkbox (username&password and username alone). With this the only one that maybe is missing is the standalone webauthn authenticator when used for passwordless. But this one never showed the checkbox so I would go initially with this. It adds therememberMeto the webauthn form if it was present in the original form.The PR also upgrades the two tests that were modified. They are very different so I decided to send 3 commits (fix, tests mv, tests modifications). You can review the changes for the tests in the last commit. If we want to maintain the history for the tests file, please do not squash it when merging.
Don't know if we should add some doc changes. Let me know what you think.