-
Notifications
You must be signed in to change notification settings - Fork 0
Fixing Re-authentication with passkeys #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: improve-auth-user-experience
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |||||
| import org.keycloak.models.credential.WebAuthnCredentialModel; | ||||||
| import org.keycloak.models.utils.KeycloakModelUtils; | ||||||
| import org.keycloak.organization.authentication.authenticators.browser.OrganizationAuthenticatorFactory; | ||||||
| import org.keycloak.protocol.oidc.OIDCLoginProtocol; | ||||||
| import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; | ||||||
| import org.keycloak.representations.idm.AuthenticatorConfigRepresentation; | ||||||
| import org.keycloak.representations.idm.OrganizationDomainRepresentation; | ||||||
|
|
@@ -193,10 +194,10 @@ public void passwordLoginWithNonDiscoverableKey() throws Exception { | |||||
| MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); | ||||||
| loginPage.loginUsername(USERNAME); | ||||||
|
|
||||||
| // now the passkeys username password page should be presented with username selected and passkeys disabled | ||||||
| // now the passkeys username password page should be presented with username selected. Passkeys still enabled | ||||||
| loginPage.assertCurrent(); | ||||||
| MatcherAssert.assertThat(loginPage.getAttemptedUsername(), Matchers.is("userwebauthn")); | ||||||
| Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); | ||||||
| MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); | ||||||
| loginPage.login("invalid-password"); | ||||||
| loginPage.assertCurrent(); | ||||||
| MatcherAssert.assertThat(loginPage.getPasswordInputError(), Matchers.is("Invalid password.")); | ||||||
|
|
@@ -207,7 +208,7 @@ public void passwordLoginWithNonDiscoverableKey() throws Exception { | |||||
|
|
||||||
| // correct login now | ||||||
| MatcherAssert.assertThat(loginPage.getAttemptedUsername(), Matchers.is("userwebauthn")); | ||||||
| Assert.assertThrows(NoSuchElementException.class, () -> driver.findElement(By.xpath("//form[@id='webauth']"))); | ||||||
| MatcherAssert.assertThat(driver.findElement(By.xpath("//form[@id='webauth']")), Matchers.notNullValue()); | ||||||
| loginPage.login(getPassword(USERNAME)); | ||||||
| appPage.assertCurrent(); | ||||||
| events.expectLogin() | ||||||
|
|
@@ -263,4 +264,59 @@ public void passwordLoginWithExternalKey() throws Exception { | |||||
| logout(); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Test users is able to authenticate with passkey during re-authentication (for example when OIDC parameter prompt=login is used) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix grammatical error in comment. The comment has a grammatical issue: "Test users is able" should be either "Test user is able" or "Tests that users are able". Apply this diff to correct the grammar: - // Test users is able to authenticate with passkey during re-authentication (for example when OIDC parameter prompt=login is used)
+ // Tests that users are able to authenticate with passkey during re-authentication (for example when OIDC parameter prompt=login is used)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| @Test | ||||||
| public void webauthnLoginWithDiscoverableKey_reauthentication() throws IOException { | ||||||
| getVirtualAuthManager().useAuthenticator(DefaultVirtualAuthOptions.PASSKEYS.getOptions()); | ||||||
|
|
||||||
| // set passwordless policy for discoverable keys | ||||||
| try (Closeable c = getWebAuthnRealmUpdater() | ||||||
| .setWebAuthnPolicyRpEntityName("localhost") | ||||||
| .setWebAuthnPolicyRequireResidentKey(Constants.WEBAUTHN_POLICY_OPTION_YES) | ||||||
| .setWebAuthnPolicyUserVerificationRequirement(Constants.WEBAUTHN_POLICY_OPTION_REQUIRED) | ||||||
| .setWebAuthnPolicyPasskeysEnabled(Boolean.TRUE) | ||||||
| .update()) { | ||||||
|
|
||||||
| checkWebAuthnConfiguration(Constants.WEBAUTHN_POLICY_OPTION_YES, Constants.WEBAUTHN_POLICY_OPTION_REQUIRED); | ||||||
|
|
||||||
| registerDefaultUser(); | ||||||
|
|
||||||
| UserRepresentation user = userResource().toRepresentation(); | ||||||
| MatcherAssert.assertThat(user, Matchers.notNullValue()); | ||||||
|
|
||||||
| logout(); | ||||||
| events.clear(); | ||||||
|
|
||||||
| // the user should be automatically logged in using the discoverable key | ||||||
| oauth.openLoginForm(); | ||||||
| WaitUtils.waitForPageToLoad(); | ||||||
|
|
||||||
| appPage.assertCurrent(); | ||||||
|
|
||||||
| events.expectLogin() | ||||||
| .user(user.getId()) | ||||||
| .detail(Details.USERNAME, user.getUsername()) | ||||||
| .detail(Details.CREDENTIAL_TYPE, WebAuthnCredentialModel.TYPE_PASSWORDLESS) | ||||||
| .detail(WebAuthnConstants.USER_VERIFICATION_CHECKED, "true") | ||||||
| .assertEvent(); | ||||||
|
|
||||||
| // Re-authentication now with prompt=login. Passkeys login should be possible. | ||||||
| oauth.loginForm() | ||||||
| .prompt(OIDCLoginProtocol.PROMPT_VALUE_LOGIN) | ||||||
| .open(); | ||||||
| WaitUtils.waitForPageToLoad(); | ||||||
|
|
||||||
| appPage.assertCurrent(); | ||||||
|
|
||||||
| events.expectLogin() | ||||||
| .user(user.getId()) | ||||||
| .detail(Details.USERNAME, user.getUsername()) | ||||||
| .detail(Details.CREDENTIAL_TYPE, WebAuthnCredentialModel.TYPE_PASSWORDLESS) | ||||||
| .detail(WebAuthnConstants.USER_VERIFICATION_CHECKED, "true") | ||||||
| .assertEvent(); | ||||||
|
|
||||||
| logout(); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Method call missing required argument.
The method
isConditionalPasskeysEnabled()is defined in the parent classUsernamePasswordFormwith a required parameterUserModel user(line 160 in UsernamePasswordForm.java), but it's being called here without any arguments. This will cause a compilation error.Apply this diff to fix the method call:
📝 Committable suggestion
🤖 Prompt for AI Agents