[java-idp-plugin-webauthn] branch main updated: Improve Javadoc

Phil Smart philip.smart at jisc.ac.uk
Wed Jun 19 15:41:18 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=9160d2c8ad77bba7d8aeeb7befa9e07c01cfc00f

The following commit(s) were added to refs/heads/main by this push:
     new 9160d2c  Improve Javadoc
9160d2c is described below

commit 9160d2c8ad77bba7d8aeeb7befa9e07c01cfc00f
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed Jun 19 16:41:15 2024 +0100

    Improve Javadoc
---
 .../authn/webauthn/authn/AssertionResult.java      |  2 ++
 .../client/WebAuthnAuthenticationClient.java       | 10 ++++------
 .../WebAuthnAuthenticationClientFactory.java       |  1 -
 .../context/WebAuthnRegistrationContext.java       |  2 +-
 .../StorageServiceCredentialRepository.java        | 23 +++++++++++++---------
 .../impl/YubicoWebAuthnAuthenticationClient.java   |  4 +++-
 .../IdPStorageServiceCredentialRespository.java    |  9 ++++++---
 7 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/authn/AssertionResult.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/authn/AssertionResult.java
index 4192e2e..64137ed 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/authn/AssertionResult.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/authn/AssertionResult.java
@@ -22,6 +22,8 @@ import net.shibboleth.shared.annotation.constraint.NotEmpty;
 /**
  * The result of calling {@link WebAuthnAuthenticationClient#validateAuthenticatorAssertionResponse(String, byte[], 
  * com.yubico.webauthn.data.PublicKeyCredentialRequestOptions, com.yubico.webauthn.data.PublicKeyCredential)}
+ * 
+ * <p>Note, this is similar to Yubico's AssertionResult but, importantly, can be instantiated publicly.</p>
  */
 //TODO javadoc etc. when finalised
 public class AssertionResult {
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java
index 607708c..8ebfd86 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClient.java
@@ -21,12 +21,10 @@ import net.shibboleth.idp.plugin.authn.webauthn.exception.RegistrationFailureExc
 import net.shibboleth.idp.plugin.authn.webauthn.exception.WebAuthnAuthenticationClientException;
 
 /**
- * A client that manages a WebAuthn authentication or registration ceremony.
+ * A client that manages WebAuthn authentication and registration ceremonies.
  * 
- * <p>Tied to the Yubico library data model, other clients will need to be adapted to support this model, and provide
- * translations between their internal representation and the types required.</p>
- * 
- * TODO: should be go all in for Yubico data model here, otherwise we need our own variant of PublicKeyCredential etc.
+ * <p>Uses the Yubico library data model. Clients will need to be adapted to and support this model, if required 
+ * providing translations between their internal representation and the Yubico types.</p>
  * 
  * <p>Implementations must be thread-safe</p>
  */
@@ -90,7 +88,7 @@ public interface WebAuthnAuthenticationClient {
      * 
      * @return a registration result iff the registration was valid
      * 
-     * @throws RegistrationFailureException if the registration is not valid
+     * @throws RegistrationFailureException if the registration is invalid.
      */
      @Nonnull RegistrationResult validateAuthenticatorAttestationResponse(
              @Nonnull final PublicKeyCredentialCreationOptions publicKeyCredentialCreationOptions, 
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClientFactory.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClientFactory.java
index f15b6d3..55a8d3f 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClientFactory.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/WebAuthnAuthenticationClientFactory.java
@@ -2,7 +2,6 @@ package net.shibboleth.idp.plugin.authn.webauthn.client;
 
 import javax.annotation.Nonnull;
 
-import net.shibboleth.idp.plugin.authn.webauthn.client.WebAuthnAuthenticationClient;
 import net.shibboleth.idp.plugin.authn.webauthn.exception.WebAuthnAuthenticationClientException;
 
 /**
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java
index 2905995..129ec50 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java
@@ -35,7 +35,7 @@ public final class WebAuthnRegistrationContext extends BaseWebAuthnContext {
     @Nullable private PublicKeyCredentialCreationOptions publicKeyCredentialCreationOptions;
            
     /** 
-     * The credential public key which is the result of registration of a new key pair generated
+     * The credential public key which is the result of a registration of a new key pair generated
      * by the authenticator.
      */
     @Nullable private RegistrationResult registrationResult;
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/StorageServiceCredentialRepository.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/StorageServiceCredentialRepository.java
index e82ec66..ec443d0 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/StorageServiceCredentialRepository.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/StorageServiceCredentialRepository.java
@@ -20,6 +20,8 @@ import java.util.Set;
 import javax.annotation.Nonnull;
 import javax.annotation.concurrent.ThreadSafe;
 
+import org.opensaml.storage.StorageService;
+
 import com.yubico.webauthn.CredentialRepository;
 import com.yubico.webauthn.data.ByteArray;
 
@@ -28,7 +30,9 @@ import net.shibboleth.shared.annotation.constraint.NotLive;
 import net.shibboleth.shared.annotation.constraint.Unmodifiable;
 
 /**
- * An extension of the {@link CredentialRepository} interface to support additional read and add operations.
+ * An extension of the {@link CredentialRepository} interface to support additional read and write operations.
+ * 
+ * <p>Implementations are intended to work with a {@link StorageService}, although that is not a strict requirement.</p>
  * 
  * <p>Implementations of this interface are required to be thread-safe.</p>
  */
@@ -46,17 +50,17 @@ public interface StorageServiceCredentialRepository extends CredentialRepository
     Set<CredentialRegistration> getRegistrationsByUsername(@Nonnull final String username);
 
     /**
-     * Add a new credential registration.
+     * Add a new credential registration for a given user, by their username.
      * 
-     * @param username the username to add the registration for
-     * @param credential the credential to register
+     * @param username the user to add the registration for
+     * @param credential the credential to add
      * 
      * @return true iff the registration was added. False otherwise.
      */
     boolean addRegistrationByUsername(@Nonnull final String username, @Nonnull final CredentialRegistration credential);
     
     /**
-     * Update the signature counter of the credential that belongs to the user.
+     * Update the signature counter of the credential that belongs to the given user.
      * 
      * @param username the username of the user to update the signature counter for
      * @param credentialId the identifier of the credential to update the signature counter for
@@ -80,7 +84,7 @@ public interface StorageServiceCredentialRepository extends CredentialRepository
             @Nonnull final ByteArray credentialId);
 
     /**
-     * Remove the given registration for the give user.
+     * Remove the registration for the given user.
      * 
      * @param username the user to remove the registration for
      * @param credentialRegistration the credential to remove
@@ -97,7 +101,8 @@ public interface StorageServiceCredentialRepository extends CredentialRepository
      * 
      * @param credentialId the credential to remove
      * 
-     * @return the number of credentials that have been removed. Should always be 1 if removed, 0 if not.
+     * @return the number of credentials that have been removed. Should be 1 if removed from a single user, more than 1
+     *          if removed from more than one user, and 0 if not removed from any user.
      */
     int removeRegistrationByCredentialId(@Nonnull final ByteArray credentialId);    
     
@@ -106,7 +111,7 @@ public interface StorageServiceCredentialRepository extends CredentialRepository
      * Get all credential registrations known to the repository. Typically used to provide an administrative view
      * of the repository.
      * 
-     * @return all credential registrations know to the system.
+     * @return all credential registrations known to the system.
      */
     @Nonnull @NotLive @Unmodifiable Set<CredentialRegistration> getAllRegistrations();
 
@@ -114,7 +119,7 @@ public interface StorageServiceCredentialRepository extends CredentialRepository
      * Remove the credential with the given credential Id for the given user.
      * 
      * @param username the user to remove the credential from
-     * @param credentialId the Id of the credential to remove
+     * @param credentialId the identifier of the credential to remove
      * 
      * @return true iff the credential was remove, false otherwise.
      */
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 ee18734..0b24d9a 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
@@ -170,7 +170,7 @@ public class YubicoWebAuthnAuthenticationClient implements WebAuthnAuthenticatio
                 log.debug("Attempting validation of credential with username '{}' and userHandle '{}'",
                         username, userId);
             }
-
+            
             final com.yubico.webauthn.AssertionResult result = rp.finishAssertion(FinishAssertionOptions.builder()
                     .request(requestAssertion)
                     .response(authenticatorAssertionResponse)
@@ -210,6 +210,8 @@ public class YubicoWebAuthnAuthenticationClient implements WebAuthnAuthenticatio
                     .request(publicKeyCredentialCreationOptions)
                     .response(authenticatorAttestationResponse)
                     .build());
+           
+            RegistrationResult.builder().withCredential(authenticatorAttestationResponse).build();
 
             final RegistrationResult registrationResult = RegistrationResult.builder()
                     .withAttestationTrusted(result.isAttestationTrusted())
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
index b62a7be..2df3481 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
@@ -47,6 +47,7 @@ import net.shibboleth.idp.plugin.authn.webauthn.exception.CredentialRepositoryEx
 import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
 import net.shibboleth.idp.plugin.authn.webauthn.storage.StorageServiceCredentialRepository;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.annotation.constraint.NotLive;
 import net.shibboleth.shared.annotation.constraint.ThreadSafeAfterInit;
 import net.shibboleth.shared.annotation.constraint.Unmodifiable;
@@ -75,7 +76,7 @@ public class IdPStorageServiceCredentialRespository extends AbstractIdentifiable
     private static final Logger log = LoggerFactory.getLogger(IdPStorageServiceCredentialRespository.class);
     
     /** The context to use to partition the storage records.*/
-    private static final String STORAGE_CONTEXT = "net.shibboleth.idp.plugin.authn.webauthn";
+    @Nonnull @NotEmpty private static final String STORAGE_CONTEXT = "net.shibboleth.idp.plugin.authn.webauthn";
     
     /** Storage record serializer. */
     @NonnullAfterInit private StorageSerializer<Set<CredentialRegistration>> serializer;
@@ -90,7 +91,7 @@ public class IdPStorageServiceCredentialRespository extends AbstractIdentifiable
     @NonnullAfterInit private ReentrantReadWriteLock lock;
     
     /**
-     * Set the storage service to store registered credentials. 
+     * Set the storage service to store credentials. 
      * 
      * @param service the storageService to set.
      */
@@ -298,9 +299,11 @@ public class IdPStorageServiceCredentialRespository extends AbstractIdentifiable
         try {
             readLock.lock();
             final Set<CredentialRegistration> existingRegistrations = getRegistrationsByUsername(username);
-            return existingRegistrations.stream()
+            final Optional<CredentialRegistration> registration = existingRegistrations.stream()
                     .filter(credReg -> id.equals(credReg.getCredential().getCredentialId()))
                     .findFirst();
+            assert registration != null;
+            return registration;
         } finally {
             readLock.unlock();
         }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list