[java-idp-plugin-webauthn] branch main updated: Add IdP storage service adaptor

Phil Smart philip.smart at jisc.ac.uk
Fri Jan 26 17:01:34 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=143bfb053562bd87d15b81bbccb97279ba353df5

The following commit(s) were added to refs/heads/main by this push:
     new 143bfb0  Add IdP storage service adaptor
143bfb0 is described below

commit 143bfb053562bd87d15b81bbccb97279ba353df5
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Jan 26 17:01:27 2024 +0000

    Add IdP storage service adaptor
    
         - an IdP storage service implementation can now be used as a Yubico
        Credential repository.
         - Needs work or possibly a re-think if we want to expose different
        APIs. But this works as a first step.
---
 .../exception/CredentialRepositoryException.java   |  63 ++++
 .../webauthn/storage/CredentialRegistration.java   | 331 +++++++++++++----
 .../StorageServiceCredentialRepository.java        |  52 ++-
 webauthn-impl/pom.xml                              |  10 +-
 .../admin/impl/StorePublicKeyCredential.java       |  75 +---
 .../impl/CredentialRegistrationSerializer.java     |  88 +++++
 .../IdPStorageServiceCredentialRespository.java    | 353 ++++++++++++++++++
 .../storage/impl/InMemoryRegistrationStorage.java  |   7 +-
 ...bauthnPublicKeyCredentialStorageSerializer.java |   2 +-
 .../META-INF/net.shibboleth.idp/postconfig.xml     |  12 +-
 .../idp/plugin/authn/webauthn/module.properties    |  14 +-
 .../authn/webauthn/views/webauthn-register.vm      |   4 +-
 .../views/webauthn-registration-outcomes.vm        |   2 +-
 .../YubicoWebauthnAuthenticationClientTest.java    |  99 +++--
 .../authn/webauthn/impl/AbstractWebAuthnTest.java  |  35 +-
 .../impl/CredentialRegistrationSerializerTest.java | 124 +++++++
 ...IdPStorageServiceCredentialRespositoryTest.java | 397 +++++++++++++++++++++
 17 files changed, 1455 insertions(+), 213 deletions(-)

diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/CredentialRepositoryException.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/CredentialRepositoryException.java
new file mode 100644
index 0000000..c2aad9a
--- /dev/null
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/exception/CredentialRepositoryException.java
@@ -0,0 +1,63 @@
+/*
+ * 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.exception;
+
+import javax.annotation.Nullable;
+
+/**
+ * An unchecked exception that is throw when there is an unrecoverable error occurs handling requests to the 
+ * credential repository.
+ */
+public class CredentialRepositoryException extends RuntimeException {
+    
+    /** Serial version UID. */
+    private static final long serialVersionUID = -4999178337302857500L;
+
+    /**
+     * Constructor.
+     */
+    public CredentialRepositoryException() {
+        super();
+    }
+
+    /**
+     * Constructor.
+     * 
+     * @param message exception message
+     */
+    public CredentialRepositoryException(@Nullable final String message) {
+        super(message);
+    }
+
+    /**
+     * Constructor.
+     * 
+     * @param wrappedException exception to be wrapped by this one
+     */
+    public CredentialRepositoryException(@Nullable final Exception wrappedException) {
+        super(wrappedException);
+    }
+
+    /**
+     * Constructor.
+     * 
+     * @param message exception message
+     * @param wrappedException exception to be wrapped by this one
+     */
+    public CredentialRepositoryException(@Nullable final String message, @Nullable final Exception wrappedException) {
+        super(message, wrappedException);
+    }
+
+}
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java
index 3a569a2..5338c77 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/CredentialRegistration.java
@@ -18,126 +18,315 @@
 package net.shibboleth.idp.plugin.authn.webauthn.storage;
 
 import java.time.Instant;
+import java.util.Collections;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.SortedSet;
-import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
+import javax.annotation.concurrent.ThreadSafe;
 
+import com.fasterxml.jackson.annotation.JsonGetter;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder;
 import com.yubico.webauthn.RegisteredCredential;
 import com.yubico.webauthn.data.AuthenticatorTransport;
 import com.yubico.webauthn.data.PublicKeyCredentialDescriptor;
 import com.yubico.webauthn.data.UserIdentity;
 
 /**
- * Influenced by the CredentialRegistration class in the Yubico demo libraries.
+ * Registration record used to hold registered credentials.
  * 
- * Used to hold registrations, and for easy extraction of values for display.
+ * <p>Equality is determined by comparing the wrapped {@link RegisteredCredential credential}.</p>
  */
-//TODO need our own storage record, so this should be test only and then replaced with the actual one eventually
-//TODO make this more official and undeprecate
-//TODO needs a builder
- at Deprecated
+ at ThreadSafe
+ at Immutable
+ at JsonDeserialize(builder = CredentialRegistration.Builder.class)
 public class CredentialRegistration {
 
-    UserIdentity userIdentity;
-    Optional<String> credentialNickname;
-    @Nonnull SortedSet<AuthenticatorTransport> transports;
-    Instant registrationTime;
-    /** Is the credential a discovery type (passkey). Empty if not known.*/
-    Optional<Boolean> discoverable;
-    RegisteredCredential credential;
-    Optional<Object> attestationMetadata;
-    /** Was the user verified during registration.*/
-    boolean userVerified;
-    
-    
-    public CredentialRegistration(final UserIdentity userIdentity, final Optional<String> credentialNickname,
-            @Nonnull final SortedSet<AuthenticatorTransport> transports, final Instant registrationTime, 
-            final RegisteredCredential credential,
-            final Optional<Object> attestationMetadata, final Optional<Boolean> isDiscoverable,
-            final boolean isUserVerified) {
-        super();
-        this.userIdentity = userIdentity;
-        this.credentialNickname = credentialNickname;
-        this.transports = transports;
-        this.registrationTime = registrationTime;
-        this.credential = credential;
-        this.attestationMetadata = attestationMetadata;
-        discoverable = isDiscoverable;
-        userVerified = isUserVerified;
+    /** The users identity. */
+    @Nonnull private final UserIdentity userIdentity;
+
+    /** An optional nickname of the credential. */
+    @Nullable private final String credentialNickname;
+
+    /**
+     * The set of {@link AuthenticatorTransport transports} the authenticator can
+     * use to communicate with the client.
+     */
+    @Nonnull private final SortedSet<AuthenticatorTransport> transports;
+
+    /** The time the registration took place. */
+    @Nonnull private final Instant registrationTime;
+
+    /** Is the credential a discovery type (passkey). Empty if not known. */
+    @Nonnull private final Optional<Boolean> discoverable;
+
+    /** The credential to register. */
+    @Nonnull private final RegisteredCredential credential;
+
+    /** Optional attestation metadata about the authenticator. */
+    @Nullable private final Object attestationMetadata;
+
+    /** Was the user verified during registration. */
+    private final boolean userVerified;
+
+    /**
+     * 
+     * Builder constructor.
+     *
+     * @param builder the builder
+     */
+    private CredentialRegistration(final Builder builder) {
+        this.userIdentity = builder.userIdentity;
+        this.transports = builder.transports;
+        this.registrationTime = builder.registrationTime;
+        this.credential = builder.credential;
+        this.credentialNickname = builder.credentialNickname;
+        this.discoverable = builder.discoverable;
+        this.attestationMetadata = builder.attestationMetadata;
+        this.userVerified = builder.userVerified;
     }
-    
+
+    @JsonGetter("userVerified")
     public boolean isUserVerified() {
         return userVerified;
     }
-    
-    public Optional<Boolean> isDiscoverable(){
+
+    @JsonGetter("discoverable")
+    @Nonnull public Optional<Boolean> isDiscoverable() {
         return discoverable;
     }
     
-    public String getDiscoverableAsString(){
-        return isDiscoverable().isPresent() ? isDiscoverable().get().toString() : "unknown";
-    }
-    
-    public String getNickname(){
-        return credentialNickname.orElse("");
-    }
-    
-    public String getCredentialIdBase64Url() {
-        return credential.getCredentialId().getBase64Url();
+    //TODO replace with view encoder?
+    @JsonIgnore
+    @Nonnull public String getDiscoverableAsString() {
+        return discoverable.isEmpty() ? "unknown" : Boolean.toString(discoverable.get());
     }
-    
-    public String getTransportsString() {
-        return transports.stream().map(AuthenticatorTransport::getId).collect(Collectors.joining(","));
-    }
-    
-    public CredentialRegistration() {
-        transports = new TreeSet<AuthenticatorTransport>();
+
+    @JsonGetter("nickname")
+    public String getNickname() {
+        return credentialNickname;
     }
 
-    public String getRegistrationTimestamp() {
-        return registrationTime.toString();
+    @JsonGetter("registrationTime")
+    public Instant getRegistrationTime() {
+        return registrationTime;
     }
 
+    @JsonIgnore
     public String getUsername() {
         return userIdentity.getName();
     }
 
+    @JsonGetter("userIdentity")
     public UserIdentity getUserIdentity() {
         return userIdentity;
     }
 
+    @JsonGetter("credential")
     public RegisteredCredential getCredential() {
         return credential;
     }
-    
+
+    @JsonGetter("transports")
     public SortedSet<AuthenticatorTransport> getTransports() {
         return transports;
     }
     
-    @Nullable public PublicKeyCredentialDescriptor toPublicKeyCredentialDescriptor() {
-        return  PublicKeyCredentialDescriptor.builder()
+    // TODO do we need these here. They should be transformed where needed
+    @JsonIgnore
+    public String getCredentialIdBase64Url() {
+        return credential.getCredentialId().getBase64Url();
+    }
+
+    @JsonIgnore
+    public String getTransportsString() {
+        return transports.stream().map(AuthenticatorTransport::getId).collect(Collectors.joining(","));
+    }
+
+    /**
+     * Convert the credential registration into a {@link PublicKeyCredentialDescriptor}.
+     * 
+     * @return the created {@link PublicKeyCredentialDescriptor}
+     */
+    @JsonIgnore
+    @Nullable
+    public PublicKeyCredentialDescriptor toPublicKeyCredentialDescriptor() {
+        return PublicKeyCredentialDescriptor.builder()
                 .id(credential.getCredentialId())
                 .transports(transports)
                 .build();
     }
     
-    public CredentialRegistration withCredential(final RegisteredCredential newRegCred) {
-        final CredentialRegistration newReg = new CredentialRegistration();
-        newReg.attestationMetadata = attestationMetadata;
-        newReg.userIdentity = userIdentity;
-        newReg.registrationTime = registrationTime;
-        newReg.transports = transports;
-        newReg.credentialNickname = credentialNickname;
-        newReg.discoverable = discoverable;
-        newReg.userVerified = userVerified;
-        // With the new credential
-        newReg.credential = newRegCred;
-        return newReg;
-    }
     
 
+    /** {@inheritDoc} */
+    @Override
+    public int hashCode() {
+        return Objects.hash(credential);
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean equals(final Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        final CredentialRegistration other = (CredentialRegistration) obj;
+        return Objects.equals(credential, other.credential);
+    }
+
+    /**
+     * Copy this instance into a new credential registration instance replacing the registered credential with that
+     * given.
+     * 
+     * @param newRegisteredCred the new credential
+     * 
+     * @return a new {@link CredentialRegistration} instance
+     */
+    @JsonIgnore
+    public CredentialRegistration withCredential(final RegisteredCredential newRegisteredCred) {
+        return CredentialRegistration.builder()
+                .withUserIdentity(userIdentity)
+                .withTransports(transports)
+                .withRegistrationTime(registrationTime)
+                // The credential here is the new one
+                .withCredential(newRegisteredCred)
+                .withAttestationMetadata(attestationMetadata)
+                .withCredentialNickname(credentialNickname)
+                .withDiscoverable(discoverable)
+                .withUserVerified(userVerified).build();
+    }
+
+    /** Builder stage.*/
+    public static IUserIdentityStage builder() {
+        return new Builder();
+    }
+
+    /** Builder stage.*/
+    public interface IUserIdentityStage {
+        public ITransportsStage withUserIdentity(UserIdentity userIdentity);
+    }
+
+    /** Builder stage.*/
+    public interface ITransportsStage {
+        public IRegistrationTimeStage withTransports(SortedSet<AuthenticatorTransport> transports);
+    }
+
+    /** Builder stage.*/
+    public interface IRegistrationTimeStage {
+        public ICredentialStage withRegistrationTime(Instant registrationTime);
+    }
+
+    /** Builder stage.*/
+    public interface ICredentialStage {
+        public IBuildStage withCredential(RegisteredCredential credential);
+    }
+
+    /** Builder stage.*/
+    public interface IBuildStage {
+        public IBuildStage withCredentialNickname(String credentialNickname);
+
+        public IBuildStage withDiscoverable(Optional<Boolean> discoverable);
+
+        public IBuildStage withAttestationMetadata(Object attestationMetadata);
+
+        public IBuildStage withUserVerified(boolean userVerified);
+
+        @Nonnull public CredentialRegistration build();
+    }
+
+    /** Builder.*/
+    @JsonPOJOBuilder(buildMethodName = "build", withPrefix = "with")
+    public static final class Builder
+            implements IUserIdentityStage, ITransportsStage, IRegistrationTimeStage, ICredentialStage, IBuildStage {
+        private UserIdentity userIdentity;
+        private SortedSet<AuthenticatorTransport> transports;
+        private Instant registrationTime;
+        private RegisteredCredential credential;
+        private String credentialNickname;
+        private Optional<Boolean> discoverable;
+        private Object attestationMetadata;
+        private boolean userVerified;
+
+        /** Constructor.*/
+        private Builder() {
+            // Create empty, corresponds to 'unknown'
+            discoverable = Optional.empty();
+            
+            userVerified = false;
+            transports = Collections.emptySortedSet();
+        }
+
+        @Override
+        @JsonProperty("userIdentity")
+        public ITransportsStage withUserIdentity(final UserIdentity user) {
+            userIdentity = user;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("transports")
+        public IRegistrationTimeStage withTransports(final SortedSet<AuthenticatorTransport> authenticatorTransports) {
+            transports = authenticatorTransports;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("registrationTime")
+        public ICredentialStage withRegistrationTime(final Instant time) {
+            registrationTime = time;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("credential")
+        public IBuildStage withCredential(final RegisteredCredential cred) {
+            credential = cred;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("nickname")
+        public IBuildStage withCredentialNickname(final String credNickname) {
+            credentialNickname = credNickname;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("discoverable")
+        public IBuildStage withDiscoverable(final Optional<Boolean> isDiscoverable) {
+            discoverable = isDiscoverable;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("attestationMetadata")
+        public IBuildStage withAttestationMetadata(final Object attestationMtd) {
+            attestationMetadata = attestationMtd;
+            return this;
+        }
+
+        @Override
+        @JsonProperty("userVerified")
+        public IBuildStage withUserVerified(final boolean isUserVerified) {
+            userVerified = isUserVerified;
+            return this;
+        }
+
+        @Override
+        public CredentialRegistration build() {
+            return new CredentialRegistration(this);
+        }
+    }
+
 }
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 368d58c..fe572f1 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
@@ -14,25 +14,59 @@
 
 package net.shibboleth.idp.plugin.authn.webauthn.storage;
 
-import java.util.Collection;
 import java.util.Optional;
+import java.util.Set;
+
+import javax.annotation.Nonnull;
 
 import com.yubico.webauthn.CredentialRepository;
 import com.yubico.webauthn.data.ByteArray;
 
 /**
- * An IdP extension of the Yubico {@link CredentialRepository} interface to
- * support additional operations required by the IdP.
+ * An extension of the {@link CredentialRepository} interface to support additional read and add operations.
+ * 
+ * <p>Implementations of this interface are required to be thread-safe.</p>
  */
 public interface StorageServiceCredentialRepository extends CredentialRepository {
 
-    Collection<CredentialRegistration> getRegistrationsByUsername(final String username);
+    /**
+     * Get credential registrations by username.
+     * 
+     * @param username the username to find registrations for
+     * 
+     * @return the set of registered credentials associated to the user
+     */
+    Set<CredentialRegistration> getRegistrationsByUsername(@Nonnull final String username);
 
-    boolean addRegistrationByUsername(final String username, final CredentialRegistration reg);
+    /**
+     * Add a new credential registration.
+     * 
+     * @param username the username to add the registration for
+     * @param credential the credential to register
+     * 
+     * @return true iff the registration was added. False otherwise.
+     */
+    boolean addRegistrationByUsername(@Nonnull final String username, @Nonnull final CredentialRegistration credential);
 
-    Optional<CredentialRegistration> getRegistrationByUsernameAndCredentialId(final String username,
-            final ByteArray id);
+    /**
+     * Get the credential belonging to the user by its credential identifier.
+     * 
+     * @param username the username to find the credential for
+     * @param id the identifier of the credential to find
+     * 
+     * @return the credential if found, otherwise an empty {@link Optional}.
+     */
+    Optional<CredentialRegistration> getRegistrationByUsernameAndCredentialId(@Nonnull final String username,
+            @Nonnull final ByteArray id);
 
-    boolean removeRegistrationByUsername(
-                    final String username, final CredentialRegistration credentialRegistration);
+    /**
+     * Remove the given registration for the give user.
+     * 
+     * @param username the user to remove the registration for
+     * @param credentialRegistration the credential to remove
+     * 
+     * @return true iff the credential was removed, false otherwise.
+     */
+    boolean removeRegistrationByUsername(@Nonnull final String username, 
+            @Nonnull final CredentialRegistration credentialRegistration);
 }
diff --git a/webauthn-impl/pom.xml b/webauthn-impl/pom.xml
index c7a2930..36ee0f8 100644
--- a/webauthn-impl/pom.xml
+++ b/webauthn-impl/pom.xml
@@ -31,17 +31,17 @@
             <artifactId>webauthn-server-core</artifactId>
             <scope>compile</scope>
         </dependency>
+        <dependency><!-- TODO check the IdP provides this -->
+            <groupId>com.fasterxml.jackson.datatype</groupId>
+            <artifactId>jackson-datatype-jdk8</artifactId>
+            <scope>compile</scope>
+        </dependency>
         <!-- Runtime dependencies -->
         <dependency>
             <groupId>com.fasterxml.jackson.dataformat</groupId>
             <artifactId>jackson-dataformat-cbor</artifactId>
             <scope>runtime</scope>
         </dependency>
-        <dependency>
-            <groupId>com.fasterxml.jackson.datatype</groupId>
-            <artifactId>jackson-datatype-jdk8</artifactId>
-            <scope>runtime</scope>
-        </dependency>
         <dependency>
             <groupId>com.upokecenter</groupId>
             <artifactId>cbor</artifactId>
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java
index b39d5f4..25a6f20 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/StorePublicKeyCredential.java
@@ -18,8 +18,7 @@
 package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
 
 import java.time.Instant;
-import java.util.Optional;
-import java.util.SortedSet;
+import java.util.Set;
 import java.util.TreeSet;
 
 import javax.annotation.Nonnull;
@@ -34,15 +33,13 @@ import org.slf4j.Logger;
 
 import com.yubico.webauthn.RegisteredCredential;
 import com.yubico.webauthn.RegistrationResult;
-import com.yubico.webauthn.data.AuthenticatorTransport;
 import com.yubico.webauthn.data.ByteArray;
 import com.yubico.webauthn.data.UserIdentity;
 
 import net.shibboleth.idp.authn.AuthnEventIds;
-import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnPublicKeyCredentialRecord;
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
-import net.shibboleth.idp.plugin.authn.webauthn.storage.impl.WebauthnPublicKeyCredentialStorageSerializer;
+import net.shibboleth.idp.plugin.authn.webauthn.storage.impl.CredentialRegistrationSerializer;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.logic.Constraint;
@@ -54,21 +51,18 @@ import net.shibboleth.shared.primitive.LoggerFactory;
 public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction {
 
     /** Class logger. */
-    @Nonnull
-    private final Logger log = LoggerFactory.getLogger(StorePublicKeyCredential.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(StorePublicKeyCredential.class);
 
     /** Backing service. */
-    @NonnullAfterInit
-    private StorageService storageService;    
+    @NonnullAfterInit private StorageService storageService;    
     
 
     /** Storage record serializer. */
-    @Nonnull
-    private final StorageSerializer<WebAuthnPublicKeyCredentialRecord> serializer;
+    @Nonnull  private final StorageSerializer<Set<CredentialRegistration>> serializer;
 
     /** Constructor. */
     public StorePublicKeyCredential() {
-        serializer = new WebauthnPublicKeyCredentialStorageSerializer();
+        serializer = new CredentialRegistrationSerializer();
     }
 
     /**
@@ -124,17 +118,19 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
                     .id(new ByteArray(context.getUserHandle()))
                     .build();
             
-            // TODO fixup the record we will use to store registrations
-            final SortedSet<AuthenticatorTransport> transports = 
-                    registrationResult.getKeyId().getTransports().orElse(new TreeSet<>());
-            
-            final CredentialRegistration registration = new CredentialRegistration(user, 
-                    Optional.of(context.getCredentialNickname()), 
-                    transports, Instant.now(), credential, Optional.empty(), registrationResult.isDiscoverable(),
-                    registrationResult.isUserVerified());
-            
-            getCredentialRepository().addRegistrationByUsername(username, registration);
+            final CredentialRegistration registration = CredentialRegistration.builder()
+                    .withUserIdentity(user)
+                    .withTransports(registrationResult.getKeyId().getTransports().orElse(new TreeSet<>()))
+                    .withRegistrationTime(Instant.now())
+                    .withCredential(credential)
+                    //TODO this should not be null, we should support attestation even if not initially used
+                    .withAttestationMetadata(null)
+                    .withCredentialNickname(context.getCredentialNickname())
+                    .withDiscoverable(registrationResult.isDiscoverable())
+                    .withUserVerified(registrationResult.isUserVerified())
+                    .build();
             
+            getCredentialRepository().addRegistrationByUsername(username, registration);            
             
             log.debug("{} Added public key credential registration for user '{}' and key '{}'. Using a "
                     + "discoverable credential '{}', and user verification '{}'", 
@@ -150,41 +146,6 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
         }
         
 
-        // TODO one key per user, and overwrite the old one, this is obviously all
-        // wrong?
-//        try {
-//            final StorageRecord<Object> existing = storageService.read("webauthn-keys", username);
-//            if (existing != null) {
-//                final boolean updated = storageService.update("webauthn-keys", username,
-//                        new WebAuthnPublicKeyCredentialRecord(publicKey, credentialId), serializer, null);
-//                if (updated) {
-//                    log.debug("Updated webauthn-keys for user {}", context.getUsername());
-//                } else {
-//                    log.error("Unable to update public key registration");
-//                    // TODO Event type is wrong
-//                    ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_AUTHN_CTX);
-//                    return;
-//                }
-//            } else {
-//                final boolean created = storageService.create("webauthn-keys", username,
-//                        new WebAuthnPublicKeyCredentialRecord(publicKey, credentialId), serializer, null);
-//                if (created) {
-//                    log.debug("Stored webauthn-keys for user {}", context.getUsername());
-//                } else {
-//                    log.error("Unable to store public key registration");
-//                    // TODO Event type is wrong
-//                    ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.INVALID_AUTHN_CTX);
-//                    return;
-//                }
-//            }
-//
-//        } catch (final IOException e) {
-//            log.error("Unable to store public key registration", e);
-//            // TODO Event type is wrong
-//            ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
-//            return;
-//        }
-
     }
 
 }
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/CredentialRegistrationSerializer.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/CredentialRegistrationSerializer.java
new file mode 100644
index 0000000..bb8f0c8
--- /dev/null
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/CredentialRegistrationSerializer.java
@@ -0,0 +1,88 @@
+/*
+ * 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.storage.impl;
+
+import java.io.IOException;
+import java.util.Set;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.storage.StorageSerializer;
+
+import com.fasterxml.jackson.annotation.JsonInclude.Include;
+import com.fasterxml.jackson.core.Base64Variants;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.json.JsonMapper;
+import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+
+import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
+import net.shibboleth.shared.annotation.constraint.NotLive;
+import net.shibboleth.shared.annotation.constraint.Unmodifiable;
+import net.shibboleth.shared.collection.CollectionSupport;
+import net.shibboleth.shared.component.AbstractInitializableComponent;
+
+
+/**
+ * Serialize the WebauthnPublicKeyCredentialRecord to a string using Hex encoding.
+ */
+public class CredentialRegistrationSerializer extends AbstractInitializableComponent 
+        implements StorageSerializer<Set<CredentialRegistration>> {
+    
+    /** The CBOR friendly json mapper.*/
+    protected ObjectMapper jsonMapper;
+    
+    public CredentialRegistrationSerializer() {
+        jsonMapper = JsonMapper.builder()
+                .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, true)
+                .serializationInclusion(Include.NON_ABSENT)
+                .defaultBase64Variant(Base64Variants.MODIFIED_FOR_URL)
+                .addModule(new Jdk8Module())
+                .addModule(new JavaTimeModule())
+                .build();
+    }
+
+    @Override
+    public String serialize(final Set<CredentialRegistration> instance) throws IOException {
+        checkComponentActive();
+        final String valueAsString = jsonMapper.writeValueAsString(instance);
+        if (valueAsString == null) {
+            throw new IOException("Unable to serialize credential registration collection");
+        }
+        return valueAsString;
+    }
+
+    @Override
+    @Nonnull @Unmodifiable @NotLive
+    public Set<CredentialRegistration> deserialize(final long version, final String context, final String key, final String value,
+            final Long expiration) throws IOException {
+        checkComponentActive();
+        try {
+            //TODO the other properties?
+            final Set<CredentialRegistration> registrations = 
+                    jsonMapper.readValue(value, new TypeReference<Set<CredentialRegistration>>() {});
+            if (registrations == null) {
+                // Unlikely it gets here
+                throw new IOException("Unable to read credential registrations");
+            }
+            return CollectionSupport.copyToSet(registrations);
+        } catch (final Exception e) {
+           throw new IOException("Error reading JSON string into a Credential Registration",e);
+        }
+    }
+
+}
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
new file mode 100644
index 0000000..585b688
--- /dev/null
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
@@ -0,0 +1,353 @@
+/*
+ * 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.storage.impl;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.stream.Collectors;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import org.opensaml.storage.EnumeratableStorageService;
+import org.opensaml.storage.StorageCapabilities;
+import org.opensaml.storage.StorageRecord;
+import org.opensaml.storage.StorageSerializer;
+import org.opensaml.storage.StorageService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.yubico.webauthn.CredentialRepository;
+import com.yubico.webauthn.RegisteredCredential;
+import com.yubico.webauthn.data.ByteArray;
+import com.yubico.webauthn.data.PublicKeyCredentialDescriptor;
+
+import net.shibboleth.idp.plugin.authn.webauthn.exception.CredentialRepositoryException;
+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.collection.CollectionSupport;
+import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.logic.ConstraintViolationException;
+
+/**
+ * A {@link CredentialRepository} adaptor for the Shibboleth {@link StorageService}.
+ * 
+ * <p>Note, any exception is wrapped in an unchecked {@link CredentialRepositoryException}. If the caller does not deem 
+ * this terminal, they should catch and handle that error appropriately.</p>
+ * 
+ * <p>This repository is thread-safe.</p>
+ */
+ at ThreadSafe
+public class IdPStorageServiceCredentialRespository extends AbstractIdentifiableInitializableComponent
+            implements StorageServiceCredentialRepository {
+    
+    /** Class logger.*/
+    private static final Logger log = LoggerFactory.getLogger(IdPStorageServiceCredentialRespository.class);
+    
+    /** The context to use to partition the storage records.*/
+    private static final String STORAGE_CONTEXT = "webauthn";
+    
+    /** Storage record serializer. */
+    @NonnullAfterInit private StorageSerializer<Set<CredentialRegistration>> serializer;
+    
+    /** The composed storage Service.*/
+    @NonnullAfterInit private EnumeratableStorageService storageService;
+    
+    /** A shared lock to synchronize access to read and write operations. */
+    @NonnullAfterInit private ReadWriteLock lock;
+    
+    /**
+     * Set the storage service to store registered credentials. 
+     * 
+     * @param storageService the storageService to set.
+     */
+    public void setStorageService(@Nonnull final StorageService service) {
+        checkSetterPreconditions();
+        Constraint.isNotNull(service, "The Storage Service can not be null");
+        if (service instanceof final EnumeratableStorageService ess) {
+            storageService = ess;            
+        } else {
+            throw new ConstraintViolationException("Credential repository requires an EnumeratableStorageService type");
+        }
+        final StorageCapabilities caps = storageService.getCapabilities();
+        if (caps instanceof StorageCapabilities) {
+            Constraint.isTrue(caps.isServerSide(), "StorageService cannot be client-side");
+            if (!caps.isClustered()) {
+                log.info("Use of non-clustered storage service will result in per-node lockout behavior");
+            }
+        }
+    }
+    
+    /**
+     * Set the storage service serializer to handle {@link CredentialRegistration}s. 
+     * 
+     * @param serializer the serializer to set.
+     */
+    public void setSerializer(@Nonnull final StorageSerializer<Set<CredentialRegistration>> storageSerializer) {
+        checkSetterPreconditions();
+        serializer = Constraint.isNotNull(storageSerializer, "serializer can not be null");
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    protected void doInitialize() throws ComponentInitializationException {        
+        super.doInitialize();
+        if (serializer == null) {
+            throw new ComponentInitializationException("Storage serializer can not be null");
+        }
+        if (storageService == null) {
+            throw new ComponentInitializationException("Storage service can not be null");
+        }
+        lock = new ReentrantReadWriteLock(true);
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    protected void doDestroy() {
+        lock = null;
+        super.doDestroy();
+    }
+    
+    /** {@inheritDoc} */
+    @Override
+    public Set<PublicKeyCredentialDescriptor> getCredentialIdsForUsername(@Nullable final String username) {
+        checkComponentActive();
+        if (username == null) {
+            return CollectionSupport.emptySet();
+        }
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            return getRegistrationsByUsername(username).stream().map(reg -> reg.toPublicKeyCredentialDescriptor())
+                    .collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableSet())).get();  
+        } finally {
+            readLock.unlock();
+        }
+
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public Optional<ByteArray> getUserHandleForUsername(@Nullable final String username) {
+        checkComponentActive();
+        if (username == null) {
+            return Optional.empty();
+        }
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            // Find a userhandle from any credential, they should be the same per user
+            return getRegistrationsByUsername(username).stream().findAny().map(reg -> reg.getUserIdentity().getId()); 
+        } finally {
+            readLock.unlock();
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public Optional<String> getUsernameForUserHandle(final ByteArray userHandle) {
+        checkComponentActive();
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            return getRegistrationsByUserHandle(userHandle).stream().findAny().map(CredentialRegistration::getUsername);
+        } finally {
+            readLock.unlock();
+        }
+        
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public Optional<RegisteredCredential> lookup(final ByteArray credentialId, final ByteArray userHandle) {
+        checkComponentActive();
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            return lookupAll(credentialId).stream()
+                .filter(cred -> cred.getUserHandle().equals(userHandle))
+                .findAny();
+        } finally {
+            readLock.unlock();
+        }
+    }
+    
+    /**
+     * Get registrations from any user by userHandle.
+     * 
+     * @param userHandle the userHandle to match
+     * 
+     * @return registrations that match that userHandle from any 
+     */
+    private Collection<CredentialRegistration> getRegistrationsByUserHandle(final ByteArray userHandle) {
+        checkComponentActive();
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            final Set<CredentialRegistration> foundCredentials = new HashSet<>();
+            for (final Iterator<String> i = storageService.getContextKeys(STORAGE_CONTEXT, null).iterator(); 
+                    i.hasNext();) {
+                final String usernameKey = i.next();
+                assert usernameKey != null;
+                final Set<CredentialRegistration> foundCredentialsForUser = 
+                        getRegistrationsByUsername(usernameKey)
+                        .stream()
+                        .filter(cred -> userHandle.equals(cred.getUserIdentity().getId()))
+                        .collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableSet())).get();
+                foundCredentials.addAll(foundCredentialsForUser);
+            }
+            return foundCredentials;
+        } catch (final IOException e) {
+            throw new CredentialRepositoryException(e);            
+        } finally {
+            readLock.unlock();
+        }
+      }
+
+    /** {@inheritDoc} */
+    @Override
+    @Nonnull public Set<RegisteredCredential> lookupAll(final ByteArray credentialId) {
+        checkComponentActive();
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            final Set<RegisteredCredential> foundCredentials = new HashSet<>();
+            for (final Iterator<String> i = storageService.getContextKeys(STORAGE_CONTEXT, null).iterator(); 
+                    i.hasNext();) {
+                final String usernameKey = i.next();
+                assert usernameKey != null;
+                final Set<RegisteredCredential> foundCredentialsForUser = 
+                        getRegistrationsByUsername(usernameKey)
+                        .stream()
+                        .filter(reg -> reg.getCredential().getCredentialId().equals(credentialId))
+                        .map(CredentialRegistration::getCredential)
+                        .collect(CollectionSupport.nonnullCollector(Collectors.toUnmodifiableSet())).get();
+                foundCredentials.addAll(foundCredentialsForUser);
+            }
+            return foundCredentials;
+        } catch (final IOException e) {
+            throw new CredentialRepositoryException(e);
+        } finally {
+            readLock.unlock();
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    @Nonnull public Set<CredentialRegistration> getRegistrationsByUsername(final String username) {
+        checkComponentActive();
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            final StorageRecord<Set<CredentialRegistration>> registration = 
+                    storageService.read(STORAGE_CONTEXT, username);
+            if (registration != null) {
+                return registration.getValue(serializer, STORAGE_CONTEXT, username);
+            }
+            return CollectionSupport.emptySet();
+        } catch (final IOException e) {
+            throw new CredentialRepositoryException(e);
+        } finally {
+            readLock.unlock();
+        }
+    }    
+
+    /** {@inheritDoc} */
+    @Override
+    public Optional<CredentialRegistration> getRegistrationByUsernameAndCredentialId(
+            final String username, final ByteArray id) {
+        checkComponentActive();
+        final Lock readLock = lock.readLock();
+        try {
+            readLock.lock();
+            final Set<CredentialRegistration> existingRegistrations = getRegistrationsByUsername(username);
+            return existingRegistrations.stream()
+                    .filter(credReg -> id.equals(credReg.getCredential().getCredentialId()))
+                    .findFirst();
+        } finally {
+            readLock.unlock();
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean addRegistrationByUsername(
+            @Nonnull final String username, @Nonnull final CredentialRegistration reg) {
+        checkComponentActive();
+        final Lock writeLock = lock.writeLock();
+        try {
+            writeLock.lock();
+            
+            final Set<CredentialRegistration> existingRegistrations = getRegistrationsByUsername(username);
+            if (!existingRegistrations.isEmpty()) {
+                final Set<CredentialRegistration> updateSet = new LinkedHashSet<>(existingRegistrations);
+                updateSet.add(reg);
+                return storageService.update(STORAGE_CONTEXT, username, updateSet, serializer, null);                
+            } else {
+                final Set<CredentialRegistration> addSet = new LinkedHashSet<>(1);
+                addSet.add(reg);
+                return storageService.create(STORAGE_CONTEXT, username, addSet, serializer, null);
+            }
+        } catch (final IOException e) {
+            throw new CredentialRepositoryException(e);            
+        } finally {
+            writeLock.unlock();
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean removeRegistrationByUsername(
+            final String username, final CredentialRegistration credentialRegistration) {
+        checkComponentActive();
+        final Lock writeLock = lock.writeLock();
+        try {
+            writeLock.lock();
+            final Set<CredentialRegistration> existingRegistrations = getRegistrationsByUsername(username);
+            if (!existingRegistrations.isEmpty()) {
+                final Set<CredentialRegistration> updateSet = new LinkedHashSet<>(existingRegistrations);
+                updateSet.remove(credentialRegistration);
+                if (updateSet.isEmpty()) {
+                    //remove the entire storage record
+                    storageService.delete(STORAGE_CONTEXT, username);
+                } else {
+                    //else, add back what remains
+                    assert serializer != null;
+                    return storageService.update(STORAGE_CONTEXT, username, updateSet, serializer, null);   
+                }                            
+            } 
+            // Nothing to do if the registration does not exist
+            return false;
+            
+        } catch (final IOException e) {
+            throw new CredentialRepositoryException(e);            
+        } finally {
+            writeLock.unlock();
+        }         
+    }
+
+}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/InMemoryRegistrationStorage.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/InMemoryRegistrationStorage.java
index fe7c09c..4f44e5e 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/InMemoryRegistrationStorage.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/InMemoryRegistrationStorage.java
@@ -47,7 +47,10 @@ import com.yubico.webauthn.data.PublicKeyCredentialDescriptor;
 import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
 import net.shibboleth.idp.plugin.authn.webauthn.storage.StorageServiceCredentialRepository;
 
-
+/**
+ * Use {@link IdPStorageServiceCredentialRespository}
+ */
+ at Deprecated
 public class InMemoryRegistrationStorage implements StorageServiceCredentialRepository {
 
   private final Cache<String, Set<CredentialRegistration>> storage =
@@ -138,7 +141,7 @@ public class InMemoryRegistrationStorage implements StorageServiceCredentialRepo
     }
   }
 
-  public Collection<CredentialRegistration> getRegistrationsByUsername(final String username) {
+  public Set<CredentialRegistration> getRegistrationsByUsername(final String username) {
     try {
       return storage.get(username, HashSet::new);
     } catch (final ExecutionException e) {
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/WebauthnPublicKeyCredentialStorageSerializer.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/WebauthnPublicKeyCredentialStorageSerializer.java
index 38a2541..238924c 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/WebauthnPublicKeyCredentialStorageSerializer.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/WebauthnPublicKeyCredentialStorageSerializer.java
@@ -35,7 +35,7 @@ public class WebauthnPublicKeyCredentialStorageSerializer extends AbstractInitia
     implements StorageSerializer<WebAuthnPublicKeyCredentialRecord> {
     
     /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(WebauthnPublicKeyCredentialStorageSerializer.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(CredentialRegistrationSerializer.class);
 
     @Override
     public String serialize(final WebAuthnPublicKeyCredentialRecord instance) throws IOException {
diff --git a/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml b/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
index c91210e..272895a 100644
--- a/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net.shibboleth.idp/postconfig.xml
@@ -94,9 +94,17 @@
         p:objectMapper-ref="shibboleth.authn.WebAuthn.JSONObjectMapper" 
         p:credentialRepository-ref="shibboleth.authn.webauthn.DefaultCredentialRepository"/>
     
-    <!--  TODO replace with our own -->
+        
     <bean id="shibboleth.authn.webauthn.DefaultCredentialRepository" scope="singleton"
-        class="net.shibboleth.idp.plugin.authn.webauthn.storage.impl.InMemoryRegistrationStorage"/>
+        class="net.shibboleth.idp.plugin.authn.webauthn.storage.impl.IdPStorageServiceCredentialRespository"
+        p:storageService-ref="shibboleth.authn.webauthn.DefaultCredentialRepositoryStorageService"
+        p:serializer-ref="shibboleth.authn.webauthn.DefaultCredentialRepositoryStorageSerializer"/>
+        
+     <bean id="shibboleth.authn.webauthn.DefaultCredentialRepositoryStorageService"
+        class="org.opensaml.storage.impl.MemoryStorageService"/>
+        
+     <bean id="shibboleth.authn.webauthn.DefaultCredentialRepositoryStorageSerializer"
+        class="net.shibboleth.idp.plugin.authn.webauthn.storage.impl.CredentialRegistrationSerializer"/>
     
            
      <!-- 
diff --git a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/module.properties b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/module.properties
index a8eb51c..d924e7b 100644
--- a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/module.properties
+++ b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/module.properties
@@ -19,28 +19,28 @@ idp.authn.WebAuthn.2.dest = conf/authn/webauthn-config.xml
 idp.authn.WebAuthn.2.replace = false
 
 idp.authn.WebAuthn.3.src = /net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-authn.vm
-idp.authn.WebAuthn.3.dest = views//webauthn/webauthn-authn.vm
+idp.authn.WebAuthn.3.dest = views/webauthn/webauthn-authn.vm
 
 idp.authn.WebAuthn.4.src = /net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
-idp.authn.WebAuthn.4.dest = views//webauthn/webauthn-register.vm
+idp.authn.WebAuthn.4.dest = views/webauthn/webauthn-register.vm
 
 idp.authn.WebAuthn.5.src = /net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-registered.vm
 idp.authn.WebAuthn.5.dest = views/webauthn/webauthn-registered.vm
 
 idp.authn.WebAuthn.6.src = /net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-selector.vm
-idp.authn.WebAuthn.6.dest = views//webauthn/webauthn-selector.vm
+idp.authn.WebAuthn.6.dest = views/webauthn/webauthn-selector.vm
 
 idp.authn.WebAuthn.7.src = /net/shibboleth/idp/plugin/authn/webauthn/css/webauthn.css
 idp.authn.WebAuthn.7.dest = edit-webapp/css/webauthn.css
 idp.authn.WebAuthn.7.postenable = Customize edit-webapp/css/webauthn.css and rebuild war to deploy.
 
-idp.authn.WebAuthn.8.src = /net/shibboleth/idp/plugin/authn/webauthn/js/webauthn-json.js
-idp.authn.WebAuthn.8.dest = edit-webapp/js/webauthn-json.js
+idp.authn.WebAuthn.8.src = /net/shibboleth/idp/plugin/authn/webauthn/js/webauthn-json.browser-ponyfill.js
+idp.authn.WebAuthn.8.dest = edit-webapp/js/webauthn-json.browser-ponyfill.js
 
 idp.authn.WebAuthn.9.src = /net/shibboleth/idp/plugin/authn/webauthn/js/webauthn-support.js
 idp.authn.WebAuthn.9.dest = edit-webapp/js/webauthn-support.js
 
-idp.authn.WebAuthn.9.src = /net/shibboleth/idp/plugin/authn/webauthn/js/webauthn-json.js.map
-idp.authn.WebAuthn.9.dest = edit-webapp/js/webauthn-json.js.map
+idp.authn.WebAuthn.9.src = /net/shibboleth/idp/plugin/authn/webauthn/js/webauthn-json.browser-ponyfill.js.map
+idp.authn.WebAuthn.9.dest = edit-webapp/js/webauthn-json.browser-ponyfill.js.map
 
 
diff --git a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
index 34401a2..50b6bf2 100644
--- a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
+++ b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-register.vm
@@ -116,9 +116,9 @@
                              #foreach($cred in $webauthnRegContext.existingCredentials)
                                 <tr>
                                     <td>$encoder.encodeForHTML($cred.nickname)</td>
-                                    <td>$encoder.encodeForHTML($cred.transportsString)</td>
+                                    <td>$encoder.encodeForHTML($cred.transports)</td>
                                     <td>$encoder.encodeForHTML($cred.discoverableAsString)</td>
-                                    <td>$encoder.encodeForHTML($cred.registrationTimestamp)</td>
+                                    <td>$encoder.encodeForHTML($cred.registrationTime)</td>
                                     <td>
                                     <form id="deleteKeyForm" action="$flowExecutionUrl" method="post">
                                         #parse("csrf/csrf.vm")
diff --git a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-registration-outcomes.vm b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-registration-outcomes.vm
index a43f1d3..5167bfc 100644
--- a/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-registration-outcomes.vm
+++ b/webauthn-impl/src/main/resources/net/shibboleth/idp/plugin/authn/webauthn/views/webauthn-registration-outcomes.vm
@@ -67,7 +67,7 @@
                             <td>$encoder.encodeForHTML($cred.transportsString)</td>
                             <td>$encoder.encodeForHTML($cred.discoverableAsString)</td>
                             <td>$encoder.encodeForHTML($cred.userVerified)</td>
-                            <td>$encoder.encodeForHTML($cred.registrationTimestamp)</td>
+                            <td>$encoder.encodeForHTML($cred.registrationTime)</td>
                         </tr>
                         #end
                     </table>
diff --git a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java
index 64732c3..cd1f4c8 100644
--- a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java
+++ b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebauthnAuthenticationClientTest.java
@@ -18,14 +18,10 @@ import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
 
 import java.time.Instant;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.TreeSet;
 
-import javax.annotation.Nonnull;
-
-import org.slf4j.Logger;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
@@ -52,39 +48,22 @@ import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnTest;
 import net.shibboleth.idp.plugin.authn.webauthn.impl.MockAuthenticator;
 import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
 import net.shibboleth.idp.plugin.authn.webauthn.storage.impl.InMemoryRegistrationStorage;
-import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.codec.Base64Support;
-import net.shibboleth.shared.primitive.LoggerFactory;
 
 /**
  * Tests for {@link YubicoWebauthnAuthenticationClient}. To some extend this is testing the Yubico libraries work
  * correctly. But it does ensure the client has been constructed to use those libraries correctly.
  */
-public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest {
-    
-    private final static String ORIGIN = "https://idp.example.com";
-    
-    private final static String RPID = "idp.example.com";
-    
-    private final static String CHALLENGE_B64 = "dGhpc2lzBaNoYWxsZW5nZQ==";
+public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest {    
     
     private final static String CHALLENGE_2_B64 = "8gneM8yvE20CqnSCUkyD";
     
-    private final static String USER_HANDLE_B64 = "dGhpc2lzYWNoYWxsZW5nZQ==";   
-    
-    private final static String USERNAME = "test-user";
-    
-    /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(YubicoWebauthnAuthenticationClientTest.class);
-    
     private YubicoWebauthnAuthenticationClient client;   
     
     private PublicKeyCredentialCreationOptions credentialCreationOptions;
     
     private PublicKeyCredentialRequestOptions credentialRequestOptions;
     
-    
-    
     private InMemoryRegistrationStorage storage;
     
     private UserIdentity userIdentity;
@@ -200,11 +179,19 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                 .userHandle(new ByteArray(Base64Support.decode(USER_HANDLE_B64)))
                 .publicKeyCose(attestation.getResponse().getParsedAuthenticatorData()
                         .getAttestedCredentialData().get().getCredentialPublicKey())
-                .build();
+                .build();      
+
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickname"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(), Optional.of(true),
-                true);
+        final CredentialRegistration reg =CredentialRegistration.builder()
+                .withUserIdentity(userIdentity)
+                .withTransports(new TreeSet<AuthenticatorTransport>())
+                .withRegistrationTime(Instant.now())
+                .withCredential(credential)
+                .withAttestationMetadata(null)
+                .withCredentialNickname("Nickname")
+                .withDiscoverable(Optional.of(Boolean.TRUE))
+                .withUserVerified(true)
+                .build();
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -242,9 +229,16 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                         .getAttestedCredentialData().get().getCredentialPublicKey())
                 .build();
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickname"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(),Optional.of(true),
-                true);
+        final CredentialRegistration reg =CredentialRegistration.builder()
+                .withUserIdentity(userIdentity)
+                .withTransports(new TreeSet<AuthenticatorTransport>())
+                .withRegistrationTime(Instant.now())
+                .withCredential(credential)
+                .withAttestationMetadata(null)
+                .withCredentialNickname("Nickname")
+                .withDiscoverable(Optional.of(Boolean.TRUE))
+                .withUserVerified(true)
+                .build();
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -280,9 +274,16 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                         .getAttestedCredentialData().get().getCredentialPublicKey())
                 .build();
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickname"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(),Optional.of(true),
-                true);
+        final CredentialRegistration reg =CredentialRegistration.builder()
+                .withUserIdentity(userIdentity)
+                .withTransports(new TreeSet<AuthenticatorTransport>())
+                .withRegistrationTime(Instant.now())
+                .withCredential(credential)
+                .withAttestationMetadata(null)
+                .withCredentialNickname("Nickname")
+                .withDiscoverable(Optional.of(Boolean.TRUE))
+                .withUserVerified(true)
+                .build();
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -319,9 +320,16 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
                         .getAttestedCredentialData().get().getCredentialPublicKey())
                 .build();
         
-        final CredentialRegistration reg = new CredentialRegistration(userIdentity, Optional.of("Nickanme"), 
-                new TreeSet<AuthenticatorTransport>(), Instant.now(), credential, Optional.empty(),Optional.of(true),
-                true);
+        final CredentialRegistration reg =CredentialRegistration.builder()
+                .withUserIdentity(userIdentity)
+                .withTransports(new TreeSet<AuthenticatorTransport>())
+                .withRegistrationTime(Instant.now())
+                .withCredential(credential)
+                .withAttestationMetadata(null)
+                .withCredentialNickname("Nickname")
+                .withDiscoverable(Optional.of(Boolean.TRUE))
+                .withUserVerified(true)
+                .build();
         
         storage.addRegistrationByUsername(USERNAME, reg);
         
@@ -337,25 +345,6 @@ public class YubicoWebauthnAuthenticationClientTest extends AbstractWebAuthnTest
             credentialRequestOptions, assertion);
         
     }
-    
-    /**
-     * Create a client data JSON object as a Java Map.
-     * 
-     * @param type the operation type
-     * @param origin the origin
-     * @param challenge the challenge
-     * @return the clientData map
-     */
-    private Map<String, String> createClientData(@Nonnull @NotEmpty final String type, 
-            @Nonnull @NotEmpty final String origin,
-            @Nonnull @NotEmpty final String challenge){
-        final HashMap<String, String> obj = new HashMap<>();
-        obj.put("challenge",challenge);
-        obj.put("origin", origin);
-        obj.put("type", type);
-        return obj;        
-    }
-    
-    // Needs to do crypto checks
+
 
 }
diff --git a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnTest.java b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnTest.java
index 5e03116..4ffd63f 100644
--- a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnTest.java
+++ b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnTest.java
@@ -16,7 +16,11 @@ package net.shibboleth.idp.plugin.authn.webauthn.impl;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+
+import javax.annotation.Nonnull;
 
 import org.opensaml.profile.context.ProfileRequestContext;
 import org.springframework.webflow.execution.RequestContext;
@@ -35,10 +39,21 @@ import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnAuthenticationCo
 import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
 import net.shibboleth.idp.profile.context.navigate.WebflowRequestContextProfileRequestContextLookup;
 import net.shibboleth.idp.profile.testing.RequestContextBuilder;
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
 
 /** Abstract class for tests that require context setup.*/
 public abstract class AbstractWebAuthnTest {
     
+    protected final static String ORIGIN = "https://idp.example.com";
+    
+    protected final static String RPID = "idp.example.com";
+    
+    protected final static String CHALLENGE_B64 = "dGhpc2lzBaNoYWxsZW5nZQ==";    
+    
+    protected final static String USER_HANDLE_B64 = "dGhpc2lzYWNoYWxsZW5nZQ==";   
+    
+    protected final static String USERNAME = "test-user";
+    
     /** The profile request context to use.*/
     protected ProfileRequestContext prc;
     
@@ -104,7 +119,25 @@ public abstract class AbstractWebAuthnTest {
     }  
     
     
-   
+    
+    /**
+     * Create a client data JSON object as a Java Map.
+     * 
+     * @param type the operation type
+     * @param origin the origin
+     * @param challenge the challenge
+     * @return the clientData map
+     */
+    protected Map<String, String> createClientData(@Nonnull @NotEmpty final String type, 
+            @Nonnull @NotEmpty final String origin,
+            @Nonnull @NotEmpty final String challenge){
+        final HashMap<String, String> obj = new HashMap<>();
+        obj.put("challenge",challenge);
+        obj.put("origin", origin);
+        obj.put("type", type);
+        return obj;        
+    }
+
      
     
 
diff --git a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/CredentialRegistrationSerializerTest.java b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/CredentialRegistrationSerializerTest.java
new file mode 100644
index 0000000..37403cf
--- /dev/null
+++ b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/CredentialRegistrationSerializerTest.java
@@ -0,0 +1,124 @@
+/*
+ * 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.storage.impl;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+import java.time.Instant;
+import java.util.Map;
+import java.util.Optional;
+import java.util.TreeSet;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.yubico.webauthn.RegisteredCredential;
+import com.yubico.webauthn.data.AuthenticatorAttestationResponse;
+import com.yubico.webauthn.data.AuthenticatorTransport;
+import com.yubico.webauthn.data.ByteArray;
+import com.yubico.webauthn.data.ClientRegistrationExtensionOutputs;
+import com.yubico.webauthn.data.PublicKeyCredential;
+import com.yubico.webauthn.data.UserIdentity;
+
+import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnTest;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.MockAuthenticator;
+import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
+import net.shibboleth.shared.codec.Base64Support;
+import net.shibboleth.shared.collection.CollectionSupport;
+
+/**
+ * Tests for {@link CredentialRegistrationSerializer}
+ */
+public class CredentialRegistrationSerializerTest extends AbstractWebAuthnTest {
+    
+    private CredentialRegistrationSerializer serializer;
+    
+    private UserIdentity user;
+    
+    private CredentialRegistration reg;
+    
+    @Override
+    @BeforeMethod
+    public void setup() throws Exception {
+        super.setup();
+        mockAuthenticator = new MockAuthenticator(RPID);
+        serializer = new CredentialRegistrationSerializer();  
+        serializer.initialize();
+        
+        user = UserIdentity.builder()
+                .name("jdoe")
+                .displayName("John Doe")
+                .id(new ByteArray("userhandle".getBytes()))
+                .build();
+        
+        final Map<String, String> clientDataCreate = createClientData("webauthn.create", ORIGIN, CHALLENGE_B64);
+        
+        final PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs> attestation = 
+                mockAuthenticator.createAuthenticatorAttestationResponse(CHALLENGE_B64, clientDataCreate, 
+                        Base64Support.decode(USER_HANDLE_B64));
+        
+        final RegisteredCredential credential = RegisteredCredential.builder()
+                .credentialId(attestation.getId())
+                .userHandle(new ByteArray(Base64Support.decode(USER_HANDLE_B64)))
+                .publicKeyCose(attestation.getResponse().getParsedAuthenticatorData()
+                        .getAttestedCredentialData().get().getCredentialPublicKey())
+                .build();
+         
+         reg = CredentialRegistration.builder()
+                 .withUserIdentity(user)
+                 .withTransports(new TreeSet<AuthenticatorTransport>())
+                 .withRegistrationTime(Instant.now())
+                 .withCredential(credential)
+                 .withAttestationMetadata(null)
+                 .withCredentialNickname("nickname")
+                 .withDiscoverable(Optional.of(Boolean.TRUE))
+                 .withUserVerified(true)
+                 .build();
+        
+    }
+    
+    @Test
+    public void testSerialize() throws Exception {
+        
+        final String serialized = serializer.serialize(CollectionSupport.setOf(reg));
+        assertNotNull(serialized);
+        assertTrue(serialized.contains("nickname"));
+        assertTrue(serialized.contains("displayName"));
+        assertTrue(serialized.contains("John Doe"));
+        assertTrue(serialized.contains("credentialId"));
+        assertTrue(serialized.contains("name"));
+        
+    }
+    
+    @Test
+    public void testSerializeThenDeserialize() throws Exception {
+        
+        final String serialized = serializer.serialize(CollectionSupport.setOf(reg));
+        assertNotNull(serialized);
+        
+        final var registrationDeserialized = serializer.deserialize(0, serialized, serialized, serialized, null);
+        assertNotNull(registrationDeserialized);
+        assertEquals(registrationDeserialized.size(), 1);
+        assertEquals(registrationDeserialized.iterator().next().getUsername(), "jdoe");
+        assertEquals(registrationDeserialized.iterator().next().getNickname(), "nickname");
+        assertNotNull(registrationDeserialized.iterator().next().getCredential());
+        assertEquals(registrationDeserialized.iterator().next().isDiscoverable().get(), true);
+        assertEquals(registrationDeserialized.iterator().next().isUserVerified(), true);        
+        
+    }
+
+}
diff --git a/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespositoryTest.java b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespositoryTest.java
new file mode 100644
index 0000000..abdf228
--- /dev/null
+++ b/webauthn-impl/src/test/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespositoryTest.java
@@ -0,0 +1,397 @@
+/*
+ * 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.storage.impl;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Map;
+import java.util.Optional;
+import java.util.TreeSet;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+
+import org.opensaml.storage.StorageService;
+import org.opensaml.storage.impl.MemoryStorageService;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.yubico.webauthn.RegisteredCredential;
+import com.yubico.webauthn.data.AuthenticatorAttestationResponse;
+import com.yubico.webauthn.data.AuthenticatorTransport;
+import com.yubico.webauthn.data.ByteArray;
+import com.yubico.webauthn.data.ClientRegistrationExtensionOutputs;
+import com.yubico.webauthn.data.PublicKeyCredential;
+import com.yubico.webauthn.data.UserIdentity;
+
+import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnTest;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.MockAuthenticator;
+import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
+
+/**
+ * Tests for {@link IdPStorageServiceCredentialRespository}.
+ */
+public class IdPStorageServiceCredentialRespositoryTest extends AbstractWebAuthnTest {
+    
+    private UserIdentity user;
+    
+    private StorageService storageService;
+    
+    private IdPStorageServiceCredentialRespository repo;
+    
+    
+    @Override
+    @BeforeMethod
+    public void setup() throws Exception {
+        super.setup();
+        
+        storageService = new MemoryStorageService();
+        ((MemoryStorageService)storageService).setId("in-memory-storage");
+        ((MemoryStorageService)storageService).initialize();
+        final var storageSerializer = new CredentialRegistrationSerializer();
+        storageSerializer.initialize();
+        repo = new IdPStorageServiceCredentialRespository(); 
+        repo.setId("test-repo");
+        repo.setSerializer(storageSerializer);
+        repo.setStorageService(storageService);
+        repo.initialize();
+        mockAuthenticator = new MockAuthenticator(RPID);
+   }
+    
+    private CredentialRegistration createRegistration(
+            final String name, final String displayName, final byte[] userHandle) throws Exception {
+        
+        final Map<String, String> clientDataCreate = createClientData("webauthn.create", ORIGIN, CHALLENGE_B64);
+        
+        user = UserIdentity.builder()
+                .name(name)
+                .displayName(displayName)
+                .id(new ByteArray(userHandle))
+                .build();
+        
+        // Need to register a new credential first
+        final PublicKeyCredential<AuthenticatorAttestationResponse, ClientRegistrationExtensionOutputs> attestation = 
+                mockAuthenticator.createAuthenticatorAttestationResponse(CHALLENGE_B64, clientDataCreate, userHandle);
+        
+        final RegisteredCredential credential = RegisteredCredential.builder()
+                .credentialId(attestation.getId())
+                .userHandle(new ByteArray(userHandle))
+                .publicKeyCose(attestation.getResponse().getParsedAuthenticatorData()
+                        .getAttestedCredentialData().get().getCredentialPublicKey())
+                .build();
+        
+        return CredentialRegistration.builder()
+                .withUserIdentity(user)
+                .withTransports(new TreeSet<AuthenticatorTransport>())
+                .withRegistrationTime(Instant.now())
+                .withCredential(credential)
+                .withAttestationMetadata(null)
+                .withCredentialNickname("nickname")
+                .withDiscoverable(Optional.of(Boolean.TRUE))
+                .withUserVerified(true)
+                .build();
+        
+    }
+    
+    @Test
+    public void testAddRegistrationByUsername() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        final var registrations = repo.getRegistrationsByUsername("jdoe");
+        assertNotNull(registrations);
+        assertEquals(registrations.size(), 1);
+        assertEquals(registrations.iterator().next().getUsername(),"jdoe");
+        assertEquals(registrations.iterator().next().getCredential().getCredentialId(),
+                registration.getCredential().getCredentialId());
+    }
+    
+    @Test
+    public void testAddTwoRegistrationsByUsername() throws Exception {
+        
+        final CredentialRegistration registration = 
+                createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        // Now add another        
+        final CredentialRegistration registrationTwo = 
+                createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registrationTwo);
+
+        
+        final var registrations = repo.getRegistrationsByUsername("jdoe");
+        assertNotNull(registrations);
+        assertEquals(registrations.size(), 2);
+        final var iterator = registrations.iterator();
+        final var credReg = iterator.next();
+        assertEquals(credReg.getUsername(),"jdoe");
+        // Registrations are not guranteed to be ordered, so just check if either applies
+        assertTrue(registrations.stream().anyMatch(cred -> 
+            cred.getCredential().getCredentialId().equals(registration.getCredential().getCredentialId())));
+       
+        assertTrue(registrations.stream().anyMatch(cred -> 
+            cred.getCredential().getCredentialId().equals(registrationTwo.getCredential().getCredentialId())));
+
+    }
+    
+    @Test
+    public void testRemoveRegistrationByUsername() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        var registrations = repo.getRegistrationsByUsername("jdoe");
+        assertNotNull(registrations);
+        assertEquals(registrations.size(), 1);
+        final var iterator = registrations.iterator();
+        final var credReg = iterator.next();
+        assertEquals(credReg.getUsername(),"jdoe");
+        assertEquals(credReg.getCredential().getCredentialId(),
+                registration.getCredential().getCredentialId());
+        
+        repo.removeRegistrationByUsername("jdoe", registration);
+        
+        registrations = repo.getRegistrationsByUsername("jdoe");
+        assertNotNull(registrations);
+        assertEquals(registrations.size(), 0);
+    }
+    
+    @Test
+    public void testRemoveOneRegistrationFromTwoByUsername() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        // Now add another        
+        final CredentialRegistration registrationTwo = 
+                createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registrationTwo);
+        
+        var registrations = repo.getRegistrationsByUsername("jdoe");
+        assertNotNull(registrations);
+        assertEquals(registrations.size(), 2);
+        
+        repo.removeRegistrationByUsername("jdoe", registrationTwo);
+        
+        registrations = repo.getRegistrationsByUsername("jdoe");
+        assertNotNull(registrations);
+        assertEquals(registrations.size(), 1);
+        final var iterator = registrations.iterator();
+        final var credReg = iterator.next();
+        // Check the only left is the second
+        assertEquals(credReg.getUsername(),"jdoe");
+        assertEquals(credReg.getCredential().getCredentialId(),
+                registration.getCredential().getCredentialId());
+        
+    }
+    
+    @Test
+    public void testGetUserHandleForUsername() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);        
+        
+        final var userHandle = repo.getUserHandleForUsername("jdoe");
+        assertNotNull(userHandle);
+        assertTrue(userHandle.isPresent());
+        assertEquals(userHandle.get(), registration.getUserIdentity().getId());
+        
+    }
+    
+    @Test
+    public void testGetRegistrationByUsernameAndCredentialId() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);        
+        
+        final var registrations = repo.getRegistrationByUsernameAndCredentialId("jdoe", 
+                registration.getCredential().getCredentialId());
+        assertNotNull(registrations);
+        assertTrue(registrations.isPresent());
+        assertEquals(registrations.get().getCredential().getCredentialId(), 
+                registration.getCredential().getCredentialId());
+        
+    }
+    
+    @Test
+    public void testGetRegistrationByUsernameAndCredentialId_NoneFound() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);        
+        
+        final var registrations = repo.getRegistrationByUsernameAndCredentialId("not-found", 
+                registration.getCredential().getCredentialId());
+        assertNotNull(registrations);
+        assertFalse(registrations.isPresent());        
+    }
+    
+    @Test
+    public void testGetCredentialIdsForUsername() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);        
+        
+        final var credentialDescriptors = repo.getCredentialIdsForUsername("jdoe");
+        assertNotNull(credentialDescriptors);
+        assertEquals(credentialDescriptors.size(), 1);
+        assertEquals(credentialDescriptors.iterator().next().getId(), registration.getCredential().getCredentialId());
+        
+    }
+    
+    
+    
+    @Test
+    public void testLookupAll() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        // Now add another for jdoe        
+        final CredentialRegistration registrationTwo = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registrationTwo);
+        
+        // Add one for pdoe
+        final CredentialRegistration registrationthree = 
+                createRegistration("pdoe", "John Doe", "user-handle-pdoe".getBytes());
+        repo.addRegistrationByUsername("pdoe", registrationthree);
+        
+        final var registrationsForFirst = repo.lookupAll(registration.getCredential().getCredentialId());
+        assertNotNull(registrationsForFirst);
+        assertEquals(registrationsForFirst.size(), 1);
+        
+        final var registrationsForThird = repo.lookupAll(registrationthree.getCredential().getCredentialId());
+        assertNotNull(registrationsForThird);
+        assertEquals(registrationsForThird.size(), 1);        
+        
+    }
+    
+    @Test
+    public void testLookup() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        // Now add another for jdoe        
+        final CredentialRegistration registrationTwo = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registrationTwo);
+        
+        // Add one for pdoe
+        final CredentialRegistration registrationthree = 
+                createRegistration("pdoe", "John Doe", "user-handle-pdoe".getBytes());
+        repo.addRegistrationByUsername("pdoe", registrationthree);
+        
+        final var registrationsForFirst = 
+                repo.lookup(registration.getCredential().getCredentialId(), registration.getUserIdentity().getId());
+        assertTrue(registrationsForFirst.isPresent());
+
+        final var registrationsForThird = 
+                repo.lookup(registrationthree.getCredential().getCredentialId(), registrationthree.getUserIdentity().getId());
+        assertTrue(registrationsForThird.isPresent());
+        
+        // This credential does not belong to jdoe
+        final var registrationsForEmpty = 
+                repo.lookup(registration.getCredential().getCredentialId(), registrationthree.getUserIdentity().getId());
+        assertFalse(registrationsForEmpty.isPresent());
+        
+        // This credential does not belong to pdoe
+        final var registrationsForEmptyPdoe = 
+                repo.lookup(registrationthree.getCredential().getCredentialId(), registration.getUserIdentity().getId());
+        assertFalse(registrationsForEmptyPdoe.isPresent());
+        
+    }
+    
+    @Test
+    public void testGetUsernameForUserHandle() throws Exception {     
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registration);
+        
+        // Now add another for jdoe        
+        final CredentialRegistration registrationTwo = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        repo.addRegistrationByUsername("jdoe", registrationTwo);
+        
+        // Add one for pdoe
+        final CredentialRegistration registrationthree = 
+                createRegistration("pdoe", "John Doe", "user-handle-pdoe".getBytes());
+        repo.addRegistrationByUsername("pdoe", registrationthree);
+        
+        final var usernameJdoe = repo.getUsernameForUserHandle(registration.getUserIdentity().getId());
+        assertTrue(usernameJdoe.isPresent());
+        assertEquals(usernameJdoe.get(), "jdoe");
+        
+        final var usernamePdoe = repo.getUsernameForUserHandle(registrationthree.getUserIdentity().getId());
+        assertTrue(usernamePdoe.isPresent());
+        assertEquals(usernamePdoe.get(), "pdoe");
+        
+        // Check nothing comes back for an unrecognised userhandle
+        final var usernameNotfound= repo.getUsernameForUserHandle(new ByteArray("not-found".getBytes()));
+        assertFalse(usernameNotfound.isPresent());        
+        
+    }
+    
+    /* Failure here would be non-deterministic if it happened.*/
+    @Test
+    public final void testThreadSafetyAdd() throws Exception {
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        final CredentialRegistration registrationTwo = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        final CredentialRegistration registrationThree = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+     
+        final ExecutorService service = Executors.newFixedThreadPool(3);
+        final Collection<Future<Boolean>> futures = new ArrayList<>(3);
+        
+        futures.add(service.submit(()->repo.addRegistrationByUsername("jdoe", registration)));
+        futures.add(service.submit(()->repo.addRegistrationByUsername("jdoe", registrationTwo)));
+        futures.add(service.submit(()->repo.addRegistrationByUsername("jdoe", registrationThree)));
+
+        for (final Future<Boolean> f : futures) {
+            final Boolean success = f.get();
+            assertTrue(success);
+         }        
+        assertEquals(repo.getCredentialIdsForUsername("jdoe").size(), 3);
+
+    }
+    
+    /* Failure here would be non-deterministic if it happened.*/
+    @Test
+    public final void testThreadSafetyAddRead() throws Exception {
+        
+        final CredentialRegistration registration = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        final CredentialRegistration registrationTwo = createRegistration("jdoe", "John Doe", "user-handle".getBytes());
+        
+        final ExecutorService service = Executors.newFixedThreadPool(3);
+        final Collection<Future<Boolean>> futures = new ArrayList<>(3);
+        
+        futures.add(service.submit(()->repo.addRegistrationByUsername("jdoe", registration)));
+        futures.add(service.submit(()-> !repo.getRegistrationsByUsername("jdoe").isEmpty()));
+        futures.add(service.submit(()->repo.addRegistrationByUsername("jdoe", registrationTwo)));
+
+        for (final Future<Boolean> f : futures) {
+            final Boolean success = f.get();
+            assertTrue(success);
+         }        
+        assertEquals(repo.getCredentialIdsForUsername("jdoe").size(), 2);
+
+    }
+    
+
+}

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


More information about the commits mailing list