[java-idp-plugin-webauthn] branch main updated: Guarantee nonnull fields in registration result
Phil Smart
philip.smart at jisc.ac.uk
Wed Aug 7 14:51:07 UTC 2024
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-idp-plugin-webauthn.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-webauthn.git;a=commit;h=fa7cf8c1109d6d286d31e19962c4e7e94268afef
The following commit(s) were added to refs/heads/main by this push:
new fa7cf8c Guarantee nonnull fields in registration result
fa7cf8c is described below
commit fa7cf8c1109d6d286d31e19962c4e7e94268afef
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Aug 7 15:51:05 2024 +0100
Guarantee nonnull fields in registration result
---
.../authn/webauthn/admin/RegistrationResult.java | 126 +++++++++++++--------
.../webauthn/admin/RegistrationResultTest.java | 47 ++++++++
.../impl/YubicoWebAuthnAuthenticationClient.java | 10 +-
3 files changed, 135 insertions(+), 48 deletions(-)
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResult.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResult.java
index 229db10..bb5d822 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResult.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResult.java
@@ -30,6 +30,7 @@ import com.yubico.webauthn.data.PublicKeyCredential;
import com.yubico.webauthn.data.PublicKeyCredentialDescriptor;
import net.shibboleth.idp.plugin.authn.webauthn.client.WebAuthnAuthenticationClient;
+import net.shibboleth.shared.logic.Constraint;
/**
* The result of a call to {@link WebAuthnAuthenticationClient#validateAuthenticatorAttestationResponse
@@ -37,8 +38,6 @@ import net.shibboleth.idp.plugin.authn.webauthn.client.WebAuthnAuthenticationCli
*/
@ThreadSafe
@Immutable
-//TODO credential is just held here, it comes from the original response. In which case this class is mostly convient to
-// access fields inside the credential.
public class RegistrationResult {
/**
@@ -69,16 +68,6 @@ public class RegistrationResult {
this.credential = builder.credential;
}
- /**
- * Get the builder used to construct this object.
- *
- * @return the builder
- */
- @Nonnull public static Builder builder() {
- return new Builder();
- }
-
-
/**
* Is the attestation signature valid. Does it link to a trusted root attestation.
*
@@ -187,59 +176,106 @@ public class RegistrationResult {
return credential;
}
- /** The builder.*/
- public static final class Builder {
- /** Is the attestation signature valid. Does it link to a trusted root attestation.*/
+ /**
+ * Get the builder used to construct this object.
+ *
+ * @return the builder
+ */
+ @Nonnull public static IAttestationTrustedStage builder() {
+ return new Builder();
+ }
+
+ /** Builder stage.*/
+ public interface IAttestationTrustedStage {
+ /**
+ * Is the attestation signature valid. Does it link to a trusted root attestation?
+ * @param attestationTrusted is the attestation trusted.
+ * @return this builder
+ */
+ public IAttestationTypeStage withAttestationTrusted(boolean attestationTrusted);
+ }
+
+ /** Builder stage.*/
+ public interface IAttestationTypeStage {
+
+ /**
+ * Set the attestation type.
+ * @param attestationType the attestation type
+ * @return this builder
+ */
+ public ICredentialStage withAttestationType(final @Nonnull AttestationType attestationType);
+ }
+
+ /** Builder stage.*/
+ public interface ICredentialStage {
+ /**
+ * Set the public key credential attestation response
+ *
+ * @param credential the attestation response
+ * @return this builder
+ */
+ public IBuildStage withCredential(@Nonnull final
+ PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs> credential);
+ }
+
+
+ public interface IBuildStage {
+ public RegistrationResult build();
+ }
+
+ /** Builder.*/
+ public static final class Builder
+ implements IAttestationTrustedStage, IAttestationTypeStage, ICredentialStage, IBuildStage {
+ /**
+ * Is the attestation signature valid. Does it link to a trusted root attestation.
+ *
+ * <p>Note, this is different than if the assertion signature is valid.</p>
+ */
private boolean attestationTrusted;
- /** The attestation type that was used for this credential.*/
- private AttestationType attestationType;
+ /**
+ * The attestation type that was used for this credential. This only applies to attestation statements
+ * iff requested.
+ *
+ * @return the attestation type.
+ */
+ @Nonnull private AttestationType attestationType;
/** The verified attestation response.*/
+ @Nonnull
private PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs> credential;
-
+
/** Constructor.*/
+ @SuppressWarnings("null")
private Builder() {
}
- /**
- * Is the attestation signature valid. Does it link to a trusted root attestation?
- *
- * @param attTrust is the attestation trusted.
- *
- * @return this builder
- */
- public Builder withAttestationTrusted(final boolean attTrust) {
- attestationTrusted = attTrust;
+ @Override
+ public IAttestationTypeStage withAttestationTrusted(final boolean attestationTrusted) {
+ this.attestationTrusted = attestationTrusted;
return this;
}
- /**
- * Set the attestation type.
- *
- * @param attType the attestation type
- *
- * @return this builder
- */
- public Builder withAttestationType(final AttestationType attType) {
- attestationType = attType;
+ @Override
+ public ICredentialStage withAttestationType(@Nonnull final AttestationType attType) {
+ this.attestationType = Constraint.isNotNull(attType, "AttestationType can not be null");
return this;
}
- /**
- *
- * @param cred the attestation response
- *
- * @return this builder
- */
- public Builder withCredential(
- final PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs> cred) {
- credential = cred;
+ @Override
+ public IBuildStage withCredential(
+ @Nonnull final PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs>
+ cred) {
+ this.credential = Constraint.isNotNull(cred, "Credential can not be null");
return this;
}
+
+ @Override
public RegistrationResult build() {
return new RegistrationResult(this);
}
}
+
+
}
diff --git a/webauthn-api/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResultTest.java b/webauthn-api/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResultTest.java
new file mode 100644
index 0000000..a002489
--- /dev/null
+++ b/webauthn-api/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/admin/RegistrationResultTest.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.idp.plugin.authn.webauthn.admin;
+
+import static org.testng.Assert.assertNotNull;
+
+import org.testng.annotations.Test;
+
+import com.yubico.webauthn.data.AttestationType;
+
+import net.shibboleth.shared.logic.ConstraintViolationException;
+
+/**
+ * Tests for {@link RegistrationResult}
+ */
+public class RegistrationResultTest {
+
+ private RegistrationResult regResult;
+
+ @SuppressWarnings("null")
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testRegistrationResultConstruction_NullCredential() {
+ regResult = RegistrationResult.builder().withAttestationTrusted(true).withAttestationType(AttestationType.NONE)
+ .withCredential(null).build();
+ assertNotNull(regResult);
+ }
+
+ @SuppressWarnings("null")
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testRegistrationResultConstruction_NullType() {
+ regResult = RegistrationResult.builder().withAttestationTrusted(true).withAttestationType(null)
+ .withCredential(null).build();
+ assertNotNull(regResult);
+ }
+}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java
index 0b24d9a..59c4e85 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java
@@ -27,6 +27,7 @@ import com.yubico.webauthn.AssertionRequest;
import com.yubico.webauthn.FinishAssertionOptions;
import com.yubico.webauthn.FinishRegistrationOptions;
import com.yubico.webauthn.RelyingParty;
+import com.yubico.webauthn.data.AttestationType;
import com.yubico.webauthn.data.AuthenticatorAssertionResponse;
import com.yubico.webauthn.data.AuthenticatorAttestationResponse;
import com.yubico.webauthn.data.AuthenticatorSelectionCriteria;
@@ -210,12 +211,15 @@ public class YubicoWebAuthnAuthenticationClient implements WebAuthnAuthenticatio
.request(publicKeyCredentialCreationOptions)
.response(authenticatorAttestationResponse)
.build());
-
- RegistrationResult.builder().withCredential(authenticatorAttestationResponse).build();
+
+ final AttestationType type = result.getAttestationType();
+ if (type == null) {
+ throw new RegistrationFailureException("Attestation type was null");
+ }
final RegistrationResult registrationResult = RegistrationResult.builder()
.withAttestationTrusted(result.isAttestationTrusted())
- .withAttestationType(result.getAttestationType())
+ .withAttestationType(type)
.withCredential(authenticatorAttestationResponse)
.build();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list