diff --git a/config/src/integration-test/java/org/springframework/security/config/annotation/configurers/WebAuthnWebDriverTests.java b/config/src/integration-test/java/org/springframework/security/config/annotation/configurers/WebAuthnWebDriverTests.java index b0a035ef6b2..baa4476750d 100644 --- a/config/src/integration-test/java/org/springframework/security/config/annotation/configurers/WebAuthnWebDriverTests.java +++ b/config/src/integration-test/java/org/springframework/security/config/annotation/configurers/WebAuthnWebDriverTests.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.openqa.selenium.By; import org.openqa.selenium.WebDriverException; @@ -55,6 +56,7 @@ import org.springframework.security.provisioning.InMemoryUserDetailsManager; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.util.StringUtils; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.filter.DelegatingFilterProxy; import org.springframework.web.servlet.config.annotation.EnableWebMvc; @@ -67,7 +69,7 @@ * * @author Daniel Garnier-Moiroux */ -@org.junit.jupiter.api.Disabled +@Disabled class WebAuthnWebDriverTests { private String baseUrl; @@ -82,6 +84,8 @@ class WebAuthnWebDriverTests { private static final String PASSWORD = "password"; + private String authenticatorId = null; + @BeforeAll static void startChromeDriverService() throws Exception { driverService = new ChromeDriverService.Builder().usingAnyFreePort().build(); @@ -144,7 +148,7 @@ void cleanupDriver() { @Test void loginWhenNoValidAuthenticatorCredentialsThenRejects() { createVirtualAuthenticator(true); - this.driver.get(this.baseUrl); + this.getAndWait("/", "/login"); this.driver.findElement(signinWithPasskeyButton()).click(); await(() -> assertThat(this.driver.getCurrentUrl()).endsWith("/login?error")); } @@ -153,7 +157,7 @@ void loginWhenNoValidAuthenticatorCredentialsThenRejects() { void registerWhenNoLabelThenRejects() { login(); - this.driver.get(this.baseUrl + "/webauthn/register"); + this.getAndWait("/webauthn/register"); this.driver.findElement(registerPasskeyButton()).click(); assertHasAlertStartingWith("error", "Error: Passkey Label is required"); @@ -163,7 +167,7 @@ void registerWhenNoLabelThenRejects() { void registerWhenAuthenticatorNoUserVerificationThenRejects() { createVirtualAuthenticator(false); login(); - this.driver.get(this.baseUrl + "/webauthn/register"); + this.getAndWait("/webauthn/register"); this.driver.findElement(passkeyLabel()).sendKeys("Virtual authenticator"); this.driver.findElement(registerPasskeyButton()).click(); @@ -178,7 +182,8 @@ void registerWhenAuthenticatorNoUserVerificationThenRejects() { *
  • Step 1: Log in with username / password
    • *
  • Step 2: Register a credential from the virtual authenticator
    • *
  • Step 3: Log out
    • - *
  • Step 4: Log in with the authenticator
    • + *
  • Step 4: Log in with the authenticator (no allowCredentials)
    • + *
  • Step 5: Log in again with the same authenticator (with allowCredentials)
    • * */ @Test @@ -190,7 +195,7 @@ void loginWhenAuthenticatorRegisteredThenSuccess() { login(); // Step 2: register a credential from the virtual authenticator - this.driver.get(this.baseUrl + "/webauthn/register"); + this.getAndWait("/webauthn/register"); this.driver.findElement(passkeyLabel()).sendKeys("Virtual authenticator"); this.driver.findElement(registerPasskeyButton()).click(); @@ -212,9 +217,58 @@ void loginWhenAuthenticatorRegisteredThenSuccess() { logout(); // Step 4: log in with the virtual authenticator - this.driver.get(this.baseUrl + "/webauthn/register"); + this.getAndWait("/webauthn/register", "/login"); this.driver.findElement(signinWithPasskeyButton()).click(); await(() -> assertThat(this.driver.getCurrentUrl()).endsWith("/webauthn/register?continue")); + + // Step 5: authenticate while being already logged in + // This simulates some use-cases with MFA. Since the user is already logged in, + // the "allowCredentials" property is populated + this.getAndWait("/login"); + this.driver.findElement(signinWithPasskeyButton()).click(); + await(() -> assertThat(this.driver.getCurrentUrl()).endsWith("/")); + } + + @Test + void registerWhenAuthenticatorAlreadyRegisteredThenRejects() { + createVirtualAuthenticator(true); + login(); + registerAuthenticator("Virtual authenticator"); + + // Cannot re-register the same authenticator because excludeCredentials + // is not empty and contains the given authenticator + this.driver.findElement(passkeyLabel()).sendKeys("Same authenticator"); + this.driver.findElement(registerPasskeyButton()).click(); + + await(() -> assertHasAlertStartingWith("error", "Registration failed")); + } + + @Test + void registerSecondAuthenticatorThenSucceeds() { + createVirtualAuthenticator(true); + login(); + + registerAuthenticator("Virtual authenticator"); + this.getAndWait("/webauthn/register"); + List passkeyRows = this.driver.findElements(passkeyTableRows()); + assertThat(passkeyRows).hasSize(1) + .first() + .extracting((row) -> row.findElement(firstCell())) + .extracting(WebElement::getText) + .isEqualTo("Virtual authenticator"); + + // Create second authenticator and register + removeAuthenticator(); + createVirtualAuthenticator(true); + registerAuthenticator("Second virtual authenticator"); + + this.getAndWait("/webauthn/register"); + + passkeyRows = this.driver.findElements(passkeyTableRows()); + assertThat(passkeyRows).hasSize(2) + .extracting((row) -> row.findElement(firstCell())) + .extracting(WebElement::getText) + .contains("Second virtual authenticator"); } /** @@ -231,11 +285,14 @@ void loginWhenAuthenticatorRegisteredThenSuccess() { * "https://chromedevtools.github.io/devtools-protocol/tot/WebAuthn/">https://chromedevtools.github.io/devtools-protocol/tot/WebAuthn/ */ private void createVirtualAuthenticator(boolean userIsVerified) { + if (StringUtils.hasText(this.authenticatorId)) { + throw new IllegalStateException("Authenticator already exists, please remove it before re-creating one"); + } HasCdp cdpDriver = (HasCdp) this.driver; cdpDriver.executeCdpCommand("WebAuthn.enable", Map.of("enableUI", false)); // this.driver.addVirtualAuthenticator(createVirtualAuthenticatorOptions()); //@formatter:off - cdpDriver.executeCdpCommand("WebAuthn.addVirtualAuthenticator", + Map cmdResponse = cdpDriver.executeCdpCommand("WebAuthn.addVirtualAuthenticator", Map.of( "options", Map.of( @@ -248,21 +305,38 @@ private void createVirtualAuthenticator(boolean userIsVerified) { ) )); //@formatter:on + this.authenticatorId = cmdResponse.get("authenticatorId").toString(); + } + + private void removeAuthenticator() { + HasCdp cdpDriver = (HasCdp) this.driver; + cdpDriver.executeCdpCommand("WebAuthn.removeVirtualAuthenticator", + Map.of("authenticatorId", this.authenticatorId)); + this.authenticatorId = null; } private void login() { - this.driver.get(this.baseUrl); + this.getAndWait("/", "/login"); this.driver.findElement(usernameField()).sendKeys(USERNAME); this.driver.findElement(passwordField()).sendKeys(PASSWORD); this.driver.findElement(signinWithUsernamePasswordButton()).click(); + // Ensure login has completed + await(() -> assertThat(this.driver.getCurrentUrl()).doesNotContain("/login")); } private void logout() { - this.driver.get(this.baseUrl + "/logout"); + this.getAndWait("/logout"); this.driver.findElement(logoutButton()).click(); await(() -> assertThat(this.driver.getCurrentUrl()).endsWith("/login?logout")); } + private void registerAuthenticator(String passkeyName) { + this.getAndWait("/webauthn/register"); + this.driver.findElement(passkeyLabel()).sendKeys(passkeyName); + this.driver.findElement(registerPasskeyButton()).click(); + await(() -> assertThat(this.driver.getCurrentUrl()).endsWith("/webauthn/register?success")); + } + private AbstractStringAssert assertHasAlertStartingWith(String alertType, String alertMessage) { WebElement alert = this.driver.findElement(new By.ById(alertType)); assertThat(alert.isDisplayed()) @@ -289,6 +363,15 @@ private void await(Supplier> assertion) { }); } + private void getAndWait(String endpoint) { + this.getAndWait(endpoint, endpoint); + } + + private void getAndWait(String endpoint, String redirectUrl) { + this.driver.get(this.baseUrl + endpoint); + this.await(() -> assertThat(this.driver.getCurrentUrl()).endsWith(redirectUrl)); + } + private static By.ById passkeyLabel() { return new By.ById("label"); } @@ -325,6 +408,10 @@ private static By.ByCssSelector logoutButton() { return new By.ByCssSelector("button"); } + private static By.ByCssSelector deletePasskeyButton() { + return new By.ByCssSelector("table > tbody > tr > button"); + } + /** * The configuration for WebAuthN tests. It accesses the Server's current port, so we * can configurer WebAuthnConfigurer#allowedOrigin diff --git a/webauthn/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java b/webauthn/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java index f9db52cd318..4d18768b347 100644 --- a/webauthn/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java +++ b/webauthn/src/main/java/org/springframework/security/web/webauthn/management/Webauthn4JRelyingPartyOperations.java @@ -95,7 +95,7 @@ public class Webauthn4JRelyingPartyOperations implements WebAuthnRelyingPartyOpe private final PublicKeyCredentialRpEntity rp; - private final ObjectConverter objectConverter = new ObjectConverter(); + private ObjectConverter objectConverter = new ObjectConverter(); private final AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); @@ -137,6 +137,15 @@ public void setWebAuthnManager(WebAuthnManager webAuthnManager) { this.webAuthnManager = webAuthnManager; } + /** + * Sets the {@link ObjectConverter} to use. + * @param objectConverter the {@link ObjectConverter} to use. Cannot be null. + */ + void setObjectConverter(ObjectConverter objectConverter) { + Assert.notNull(objectConverter, "objectConverter cannot be null"); + this.objectConverter = objectConverter; + } + /** * Sets a {@link Consumer} used to customize the * {@link PublicKeyCredentialCreationOptionsBuilder} for @@ -390,7 +399,7 @@ public PublicKeyCredentialUserEntity authenticate(RelyingPartyAuthenticationRequ .getUserVerification() == UserVerificationRequirement.REQUIRED; com.webauthn4j.data.AuthenticationRequest authenticationRequest = new com.webauthn4j.data.AuthenticationRequest( - request.getPublicKey().getId().getBytes(), assertionResponse.getAuthenticatorData().getBytes(), + request.getPublicKey().getRawId().getBytes(), assertionResponse.getAuthenticatorData().getBytes(), assertionResponse.getClientDataJSON().getBytes(), assertionResponse.getSignature().getBytes()); // CollectedClientData and ExtensionsClientOutputs is registration data, and can diff --git a/webauthn/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java b/webauthn/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java index 405449b3519..65213c7b6ab 100644 --- a/webauthn/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java +++ b/webauthn/src/test/java/org/springframework/security/web/webauthn/management/Webauthn4jRelyingPartyOperationsTests.java @@ -27,15 +27,20 @@ import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.dataformat.cbor.CBORFactory; +import com.webauthn4j.WebAuthnManager; import com.webauthn4j.converter.AttestationObjectConverter; import com.webauthn4j.converter.util.ObjectConverter; +import com.webauthn4j.data.AuthenticationData; +import com.webauthn4j.data.AuthenticationRequest; import com.webauthn4j.data.attestation.AttestationObject; +import com.webauthn4j.data.attestation.authenticator.AttestedCredentialData; import com.webauthn4j.data.attestation.authenticator.AuthenticatorData; import com.webauthn4j.data.extension.authenticator.RegistrationExtensionAuthenticatorOutput; import org.assertj.core.api.recursive.comparison.RecursiveComparisonConfiguration; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -44,12 +49,14 @@ import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.userdetails.PasswordEncodedUser; import org.springframework.security.core.userdetails.UserDetails; +import org.springframework.security.web.webauthn.api.AuthenticatorAssertionResponse; import org.springframework.security.web.webauthn.api.AuthenticatorAttestationResponse; import org.springframework.security.web.webauthn.api.AuthenticatorAttestationResponse.AuthenticatorAttestationResponseBuilder; import org.springframework.security.web.webauthn.api.AuthenticatorSelectionCriteria; import org.springframework.security.web.webauthn.api.AuthenticatorTransport; import org.springframework.security.web.webauthn.api.Bytes; import org.springframework.security.web.webauthn.api.CredentialRecord; +import org.springframework.security.web.webauthn.api.ImmutableCredentialRecord; import org.springframework.security.web.webauthn.api.PublicKeyCredential; import org.springframework.security.web.webauthn.api.PublicKeyCredentialCreationOptions; import org.springframework.security.web.webauthn.api.PublicKeyCredentialDescriptor; @@ -57,9 +64,11 @@ import org.springframework.security.web.webauthn.api.PublicKeyCredentialRequestOptions; import org.springframework.security.web.webauthn.api.PublicKeyCredentialRpEntity; import org.springframework.security.web.webauthn.api.PublicKeyCredentialUserEntity; +import org.springframework.security.web.webauthn.api.TestAuthenticationAssertionResponses; import org.springframework.security.web.webauthn.api.TestAuthenticatorAttestationResponses; import org.springframework.security.web.webauthn.api.TestCredentialRecords; import org.springframework.security.web.webauthn.api.TestPublicKeyCredentialCreationOptions; +import org.springframework.security.web.webauthn.api.TestPublicKeyCredentialRequestOptions; import org.springframework.security.web.webauthn.api.TestPublicKeyCredentialUserEntities; import org.springframework.security.web.webauthn.api.TestPublicKeyCredentials; import org.springframework.security.web.webauthn.api.UserVerificationRequirement; @@ -67,7 +76,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyNoInteractions; @ExtendWith(MockitoExtension.class) @@ -587,6 +598,50 @@ void createCredentialRequestOptionsWhenAuthenticated() { .containsExactly(credentialRecord.getCredentialId()); } + // gh-18158 + @Test + void authenticateThenWa4jRequestCredentialIdIsRawIdBytes() throws Exception { + PublicKeyCredentialRequestOptions options = TestPublicKeyCredentialRequestOptions.create().build(); + AuthenticatorAssertionResponse response = TestAuthenticationAssertionResponses + .createAuthenticatorAssertionResponse() + .build(); + PublicKeyCredential credentials = TestPublicKeyCredentials + .createPublicKeyCredential(response) + .build(); + RelyingPartyAuthenticationRequest request = new RelyingPartyAuthenticationRequest(options, credentials); + PublicKeyCredential publicKey = request.getPublicKey(); + + ImmutableCredentialRecord credentialRecord = TestCredentialRecords.fullUserCredential().build(); + given(this.userCredentials.findByCredentialId(publicKey.getRawId())).willReturn(credentialRecord); + ObjectMapper json = mock(ObjectMapper.class); + ObjectMapper cbor = mock(ObjectMapper.class); + given(cbor.getFactory()).willReturn(mock(CBORFactory.class)); + AttestationObject attestationObject = mock(AttestationObject.class); + AuthenticatorData wa4jAuthData = mock(AuthenticatorData.class); + given(attestationObject.getAuthenticatorData()).willReturn(wa4jAuthData); + given(wa4jAuthData.getAttestedCredentialData()).willReturn(mock(AttestedCredentialData.class)); + given(cbor.readValue(credentialRecord.getAttestationObject().getBytes(), AttestationObject.class)) + .willReturn(attestationObject); + this.rpOperations.setObjectConverter(new ObjectConverter(json, cbor)); + + WebAuthnManager manager = mock(WebAuthnManager.class); + ArgumentCaptor wa4jRequest = ArgumentCaptor.forClass(AuthenticationRequest.class); + AuthenticationData wa4jData = mock(AuthenticationData.class); + given(wa4jData.getAuthenticatorData()).willReturn(mock(AuthenticatorData.class)); + given(manager.verify(wa4jRequest.capture(), any())).willReturn(wa4jData); + given(this.userEntities.findById(any())).willReturn(TestPublicKeyCredentialUserEntities.userEntity().build()); + this.rpOperations.setWebAuthnManager(manager); + + this.rpOperations.authenticate(request); + + // this ensures that our next assertion is valid (we want the rawId bytes, not the + // id bytes to be used) + assertThat(publicKey.getRawId().getBytes()).isNotEqualTo(publicKey.getId().getBytes()); + // ensure that the raw id bytes are passed into webauthn4j (not the id bytes which + // are base64 encoded) + assertThat(wa4jRequest.getValue().getCredentialId()).isEqualTo(publicKey.getRawId().getBytes()); + } + private static AuthenticatorAttestationResponse setFlag(byte... flags) throws Exception { AuthenticatorAttestationResponseBuilder authAttResponseBldr = TestAuthenticatorAttestationResponses .createAuthenticatorAttestationResponse();