[java-idp-plugin-webauthn] 02/02: Fix Javadoc. Small code improvements.
Phil Smart
philip.smart at jisc.ac.uk
Tue Jun 4 16:25:50 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=732da9ee899e195dd01a8950f41a0805c2102c73
commit 732da9ee899e195dd01a8950f41a0805c2102c73
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Jun 4 17:25:45 2024 +0100
Fix Javadoc. Small code improvements.
---
.../storage/StorageServiceCredentialRepository.java | 2 ++
.../admin/impl/AbstractWebAuthnManagementAction.java | 7 ++++---
.../admin/impl/AbstractWebAuthnRegistrationAction.java | 9 +++++----
.../admin/impl/AddAttestationConveyancePreference.java | 4 ++--
.../impl/AddAuthenticatorAttachmentRequirement.java | 7 ++++---
.../plugin/authn/webauthn/admin/impl/AddDisplayName.java | 7 ++-----
.../webauthn/admin/impl/AddResidentKeyRequirement.java | 2 +-
.../idp/plugin/authn/webauthn/admin/impl/AddUserId.java | 7 ++++---
.../admin/impl/AdminDeletePublicKeyCredential.java | 2 +-
...AdminExtractKeyRemovalInformationFromFormRequest.java | 12 ++++++------
.../admin/impl/AllowCurrentUserAccessPredicate.java | 16 +++++++++-------
.../impl/CreatePublicKeyCredentialCreationOptions.java | 2 +-
.../webauthn/admin/impl/DeletePublicKeyCredential.java | 7 ++++---
.../ExtractAuthenticatorAttestationFromFormRequest.java | 14 +++++++-------
.../ExtractKeyRemovalInformationFromFormRequest.java | 14 +++++++-------
.../admin/impl/ExtractUsernameFromRegistrationForm.java | 2 +-
.../admin/impl/ExtractUsernameSearchFromFormRequest.java | 2 +-
.../webauthn/admin/impl/LookupCredentialsForUser.java | 2 +-
.../admin/impl/PopulateWebAuthnManagementContext.java | 5 +++--
.../admin/impl/PopulateWebAuthnRegistrationContext.java | 8 ++++----
.../authn/webauthn/admin/impl/RandomUserIdGenerator.java | 2 +-
.../webauthn/admin/impl/StorePublicKeyCredential.java | 4 ++--
.../impl/ValidateAuthenticatorAttestationResponse.java | 4 ++--
...AuthnCredentialsAuthenticationFlowLookupStrategy.java | 4 ++--
24 files changed, 76 insertions(+), 69 deletions(-)
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 6b653d7..e82ec66 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
@@ -18,6 +18,7 @@ import java.util.Optional;
import java.util.Set;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.ThreadSafe;
import com.yubico.webauthn.CredentialRepository;
import com.yubico.webauthn.data.ByteArray;
@@ -31,6 +32,7 @@ import net.shibboleth.shared.annotation.constraint.Unmodifiable;
*
* <p>Implementations of this interface are required to be thread-safe.</p>
*/
+ at ThreadSafe
public interface StorageServiceCredentialRepository extends CredentialRepository {
/**
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnManagementAction.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnManagementAction.java
index b8c1427..6e8e8b0 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnManagementAction.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnManagementAction.java
@@ -94,7 +94,8 @@ public abstract class AbstractWebAuthnManagementAction extends BaseWebAuthnActio
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
return false;
- }
+ }
+ assert webauthnManagementContext != null;
return doPreExecute(profileRequestContext, webauthnManagementContext);
}
@@ -105,8 +106,8 @@ public abstract class AbstractWebAuthnManagementAction extends BaseWebAuthnActio
* @param profileRequestContext the current IdP profile request context
*/
@Override
- protected final void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-
+ protected final void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
+ assert webauthnManagementContext != null;
doExecute(profileRequestContext, webauthnManagementContext);
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnRegistrationAction.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnRegistrationAction.java
index 4b01b18..ba61bd5 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnRegistrationAction.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AbstractWebAuthnRegistrationAction.java
@@ -94,7 +94,8 @@ public abstract class AbstractWebAuthnRegistrationAction extends BaseWebAuthnAct
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
return false;
- }
+ }
+ assert webauthnRegistrationContext != null;
return doPreExecute(profileRequestContext, webauthnRegistrationContext);
}
@@ -106,12 +107,12 @@ public abstract class AbstractWebAuthnRegistrationAction extends BaseWebAuthnAct
*/
@Override
protected final void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
-
+ assert webauthnRegistrationContext != null;
doExecute(profileRequestContext, webauthnRegistrationContext);
}
/**
- * Performs this admin action's pre-execute step. Default implementation just returns true.
+ * Performs this registration action's pre-execute step. Default implementation just returns true.
*
* @param profileRequestContext the current IdP profile request context
* @param context the WebAuthn registration context
@@ -124,7 +125,7 @@ public abstract class AbstractWebAuthnRegistrationAction extends BaseWebAuthnAct
}
/**
- * Performs this webauthn authentication action using the supplied webauthn context. Implementations
+ * Performs this webauthn registration action using the supplied webauthn context. Implementations
* should override this method.
*
* @param profileRequestContext the current IdP profile request context
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAttestationConveyancePreference.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAttestationConveyancePreference.java
index 6434351..2e4a9c5 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAttestationConveyancePreference.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAttestationConveyancePreference.java
@@ -31,7 +31,7 @@ import net.shibboleth.shared.logic.ConstraintViolationException;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * Add a Attestation preference to the {@link WebAuthnAuthenticationContext context}. The default preference is 'none'.
+ * Add an attestation preference to the {@link WebAuthnAuthenticationContext context}. The default preference is 'none'.
*
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
* @post <pre>WebAuthnRegistrationContext.getAttestationConveyancePreference() != null</pre>
@@ -74,7 +74,7 @@ public class AddAttestationConveyancePreference extends AbstractWebAuthnRegistra
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final WebAuthnRegistrationContext context) {
- log.debug("{} Attestation conveyance preference is '{}'",getLogPrefix(), attestationConveyancePreference);
+ log.trace("{} Attestation conveyance preference is '{}'",getLogPrefix(), attestationConveyancePreference);
context.setAttestationConveyancePreference(attestationConveyancePreference);
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAuthenticatorAttachmentRequirement.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAuthenticatorAttachmentRequirement.java
index 8e5952a..25f1419 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAuthenticatorAttachmentRequirement.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddAuthenticatorAttachmentRequirement.java
@@ -31,7 +31,8 @@ import net.shibboleth.shared.logic.ConstraintViolationException;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * Add an authenticator attachment requirement to the {@link WebAuthnRegistrationContext context}.
+ * Add an authenticator attachment requirement to the {@link WebAuthnRegistrationContext context}. Default is
+ * {@code null} which translates to a requiremernt of 'any'.
*
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
* @post <pre>WebAuthnRegistrationContext.getAuthenticatorAttachmentRequirement() != null</pre>
@@ -41,7 +42,7 @@ public class AddAuthenticatorAttachmentRequirement extends AbstractWebAuthnRegis
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(AddAuthenticatorAttachmentRequirement.class);
- /** Set the AuthenticatorAttachment requirement. Default is null, so any.*/
+ /** Set the AuthenticatorAttachment requirement. Default is null, so 'any'.*/
@Nullable private AuthenticatorAttachment authenticatorAttachmentRequirement;
/** Constructor.*/
@@ -77,7 +78,7 @@ public class AddAuthenticatorAttachmentRequirement extends AbstractWebAuthnRegis
protected void doExecute(final ProfileRequestContext profileRequestContext,
final WebAuthnRegistrationContext context) {
- log.debug("{} AuthenticatorAttachment is '{}'",getLogPrefix(), authenticatorAttachmentRequirement != null ?
+ log.trace("{} AuthenticatorAttachment is '{}'",getLogPrefix(), authenticatorAttachmentRequirement != null ?
authenticatorAttachmentRequirement : "ANY");
context.setAuthenticatorAttachmentRequirement(authenticatorAttachmentRequirement);
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddDisplayName.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddDisplayName.java
index 207cd17..5de9b5d 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddDisplayName.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddDisplayName.java
@@ -35,8 +35,7 @@ import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action to generate or lookup a user.displayName. The display name is for display purposes only and only used for
- * the PublicKeyCredentialCreationOptions.
+ * An action to generate or lookup a user.displayName. The display name is for display purposes only.
*
* @event {@link WebAuthnRegistrationEventIds#INVALID_REGISTRATION}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
@@ -98,8 +97,6 @@ public class AddDisplayName extends AbstractWebAuthnRegistrationAction {
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final WebAuthnRegistrationContext context) {
-
- // else we build a user.display from the lookup function
final String displayName = displayNameLookupStrategy.apply(profileRequestContext);
if (displayName == null) {
log.trace("{} DisplayName was null for user '{}'", getLogPrefix(), username);
@@ -107,7 +104,7 @@ public class AddDisplayName extends AbstractWebAuthnRegistrationAction {
return;
}
if (displayName.getBytes(StandardCharsets.UTF_8).length > 64) {
- log.warn("{} DisplayName exceeds 64 bytes and might get truncated by the authenticator", getLogPrefix());
+ log.trace("{} DisplayName exceeds 64 bytes and might get truncated by the authenticator", getLogPrefix());
}
log.trace("{} Populating DisplayName '{}'",getLogPrefix(),displayName);
context.setDisplayName(displayName);
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddResidentKeyRequirement.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddResidentKeyRequirement.java
index 6cddc22..94e5724 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddResidentKeyRequirement.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddResidentKeyRequirement.java
@@ -71,7 +71,7 @@ public class AddResidentKeyRequirement extends AbstractWebAuthnRegistrationActio
protected void doExecute(final ProfileRequestContext profileRequestContext,
final WebAuthnRegistrationContext context) {
- log.debug("{} ResidentKey is '{}'",getLogPrefix(), residentKeyRequirement);
+ log.trace("{} ResidentKey requirement is '{}'",getLogPrefix(), residentKeyRequirement);
context.setResidentKeyRequirement(residentKeyRequirement);
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
index 73c3dc2..a78adc4 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
@@ -41,11 +41,12 @@ import net.shibboleth.shared.primitive.LoggerFactory;
* map a public key credential to a users session map of public keys, and by the authenticator to map the IdP's ID
* (RelyingParty ID) and the userHandle to a public key credential source (which contains the private key).
*
- * <p>This is used during the registration process, added to PublicKeyCredentialCreationOptions.</p>
+ * <p>Used during the registration process, added to PublicKeyCredentialCreationOptions.</p>
*
* <p>The user.id could contain some form of state if required, but must not contain retrievable PII.</p>
*
- * <p>The same IdP user should have the same user.id.</p>
+ * <p>The same user should have the same user.id. That is, a single user can have more than one registered
+ * credential.</p>
*
*
* @event {@link WebAuthnRegistrationEventIds#INVALID_REGISTRATION}
@@ -122,7 +123,7 @@ public class AddUserId extends AbstractWebAuthnRegistrationAction {
return;
}
if (userId.length > 64) {
- log.warn("{}: User.id is larger than 64 bytes", getLogPrefix());
+ log.trace("{} User.id is larger than 64 bytes", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
return;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminDeletePublicKeyCredential.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminDeletePublicKeyCredential.java
index 50607a2..bc23277 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminDeletePublicKeyCredential.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminDeletePublicKeyCredential.java
@@ -32,7 +32,7 @@ import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action that removes a public key credential from the credential repository based on the credential Id
+ * An action that removes a public key credential from the credential repository based on the credential ID
* found in the management context.
*
* @event {WebAuthnRegistrationEventIds#INVALID_REGISTRATION_CTX}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminExtractKeyRemovalInformationFromFormRequest.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminExtractKeyRemovalInformationFromFormRequest.java
index 7168959..e7f15e8 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminExtractKeyRemovalInformationFromFormRequest.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AdminExtractKeyRemovalInformationFromFormRequest.java
@@ -24,7 +24,7 @@ import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import jakarta.servlet.http.HttpServletRequest;
-import net.shibboleth.idp.authn.AuthnEventIds;
+import net.shibboleth.idp.plugin.authn.webauthn.admin.WebAuthnRegistrationEventIds;
import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnManagementContext;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
@@ -36,9 +36,9 @@ import net.shibboleth.shared.primitive.StringSupport;
/**
- * An action that extracts the credential identifier for removal from the incoming HTTP request.
+ * An action that extracts the credential ID for removal from the incoming HTTP request.
*
- * @event {AuthnEventIds#NO_CREDENTIALS}
+ * @event {AWebAuthnRegistrationEventIds#INVALID_REGISTRATION_CTX}
* @event {EventIds#IO_ERROR}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnManagementContext.class) != null</pre>
* @post add credential ID to remove from the the management context
@@ -79,14 +79,14 @@ public class AdminExtractKeyRemovalInformationFromFormRequest extends AbstractWe
final HttpServletRequest request = getHttpServletRequest();
if (request == null) {
log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+ ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION_CTX);
return;
}
final String credentialId = request.getParameter(credentialIdParameterName);
if (credentialId == null) {
log.debug("{} CredentialID not found in HTTP request",getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+ ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION_CTX);
return;
}
try {
@@ -96,7 +96,7 @@ public class AdminExtractKeyRemovalInformationFromFormRequest extends AbstractWe
log.trace("{} Credential to remove '{}'",getLogPrefix(),credentialId);
} catch (final DecodingException e) {
log.debug("{} Unable to base64 decode credentialID, can not remove credential", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
+ ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION_CTX);
return;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java
index 94299de..e2ee889 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AllowCurrentUserAccessPredicate.java
@@ -37,14 +37,15 @@ import net.shibboleth.shared.primitive.LoggerFactory;
/**
* An access control predicate that should implement comparison logic to allow access only to authenticated users who
- * initiate the WebAuthn registration/authentication process. If the authenticated user is not the same as the user who
- * started the registration/authentication process (as determined by the initial username collection step), access should be denied.
+ * initiated the WebAuthn registration/authentication process. If the authenticated user is not the same as the user who
+ * started the registration/authentication process (as determined by the initial username collection step), access
+ * should be denied.
*
* <p>It is important to prevent users from changing their username between the registration and authentication
- * flows. If this is allowed, it would create a loophole in the WebAuthn authentication process, which could
- * allow a user to bypass it. For instance, a user who starts the registration of a new WebAuthn credential, without any
+ * flows. If this is allowed, it would create a loophole in the WebAuthn authentication process which could
+ * allow a user bypass. For instance, a user who starts the registration of a new WebAuthn credential, without any
* previous WebAuthn credentials, could then login with an inferior authentication mechanism as another user who
- * should only be using WebAuthn.</p>
+ * should only be using WebAuthn (although the overall flow is the responsibility of the MFA flow logic).</p>
*
* <p>More complicated comparison predicates might be needed if either the principal in the subject context and or the
* username in the registration context have been transformed into different forms but are still logically
@@ -56,7 +57,7 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
/** Class logger. */
@Nonnull @NotEmpty private final Logger log = LoggerFactory.getLogger(AllowCurrentUserAccessPredicate.class);
- /** Lookup strategy to locate the webauthn registration context. */
+ /** Lookup strategy to locate the WebAuthn registration context. */
@Nonnull
private Function<ProfileRequestContext,WebAuthnRegistrationContext> webauthnRegistrationContextLookupStrategy;
@@ -146,7 +147,8 @@ public class AllowCurrentUserAccessPredicate extends AbstractIdentifiableInitial
}
// Prioritise the registration context
- final String usernameFromContext = regContext != null ? regContext.getUsername() : webAuthnContext.getUsername();
+ final String usernameFromContext = regContext != null ? regContext.getUsername() :
+ webAuthnContext.getUsername();
final SubjectContext subjectContext = subjectContextLookupStrategy.apply(profileRequestContext);
if (subjectContext == null) {
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java
index 01b5edf..58e6a3e 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/CreatePublicKeyCredentialCreationOptions.java
@@ -47,7 +47,7 @@ import net.shibboleth.shared.primitive.LoggerFactory;
* @event {WebAuthnRegistrationEventIds#INVALID_REGISTRATION_CTX}
* @event {@link EventIds#IO_ERROR}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
- * @post a UserHandle is added to the registration context
+ * @post a PublicKeyCredentialCreationOptions is added to the registration context
*/
public class CreatePublicKeyCredentialCreationOptions extends AbstractWebAuthnRegistrationAction {
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java
index ecb346e..ab00805 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/DeletePublicKeyCredential.java
@@ -33,7 +33,7 @@ import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRegistration;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action that removes a public key credential from the credential repository.
+ * An action that removes a credential registration from the credential repository.
*
* @event {WebAuthnRegistrationEventIds#INVALID_REGISTRATION_CTX}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
@@ -49,8 +49,8 @@ public class DeletePublicKeyCredential extends AbstractWebAuthnRegistrationActio
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final WebAuthnRegistrationContext context) {
- // This should come from the context before the registration page i.e. it should not comes from a form the user
- // can't manipulate.
+ // This should come from the context before the registration page i.e. it should not come from a form the user
+ // can manipulate.
final String username = context.getUsername();
if (username == null) {
log.error("{} Unable to find username in registration context", getLogPrefix());
@@ -58,6 +58,7 @@ public class DeletePublicKeyCredential extends AbstractWebAuthnRegistrationActio
return;
}
+ // This comes from the credential the user input
final byte[] credentialId = context.getCredentialIdToRemove();
if (credentialId == null) {
log.error("{} Unable to find credentialId in registration context", getLogPrefix());
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java
index 3bf1864..8055614 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractAuthenticatorAttestationFromFormRequest.java
@@ -40,8 +40,8 @@ import net.shibboleth.shared.primitive.StringSupport;
/**
- * An action that extracts the Authenticator Attestation Response and credential Nickname from the incoming
- * HTTP request.
+ * An action that extracts the PublicKeyCredential containing the authenticator attestation response from the incoming
+ * HTTP request. Also extracts the user entered credential nickname.
*
* @event {WebAuthnRegistrationEventIds#INVALID_REGISTRATION}
* @event {AuthnEventIds#NO_CREDENTIALS}
@@ -108,16 +108,16 @@ public class ExtractAuthenticatorAttestationFromFormRequest extends AbstractWebA
final String pkCredAttestationJson = request.getParameter(attestationParameterName);
if (StringSupport.trimOrNull(pkCredAttestationJson) == null) {
- log.warn("{} No authenticator attestation response in request", getLogPrefix());
+ log.debug("{} No authenticator attestation response in request", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
return;
}
- log.trace("Public key credential authenticator attestation response in JSON is '{}'",pkCredAttestationJson);
+ log.trace("PublicKeyCredential authenticator response in JSON is '{}'",pkCredAttestationJson);
final String credNickname = request.getParameter(credentialNicknameParameterName);
- log.trace("Public key credential nickname is '{}'",credNickname);
+ log.trace("Credential nickname is '{}'",credNickname);
if (StringSupport.trimOrNull(credNickname) == null) {
- log.warn("{} No nickname in request", getLogPrefix());
+ log.debug("{} No nickname in request", getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
return;
}
@@ -128,7 +128,7 @@ public class ExtractAuthenticatorAttestationFromFormRequest extends AbstractWebA
context.setAuthenticatorAttestationResponse(pkCredAttestation);
context.setCredentialNickname(credNickname);
} catch (final IOException e) {
- log.warn("{} Could not convert AuthenticatorAttestationResponse from request parameter", getLogPrefix(), e);
+ log.debug("{} Could not parse PublicKeyCredential response from request parameter", getLogPrefix(), e);
ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
return;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java
index 4e1ea6c..bb93d80 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractKeyRemovalInformationFromFormRequest.java
@@ -36,12 +36,12 @@ import net.shibboleth.shared.primitive.StringSupport;
/**
- * An action that extracts the credential identifier for removal from the incoming HTTP request.
+ * An action that extracts the credential identifier of the credential to remove from the incoming HTTP request.
*
* @event {AuthnEventIds#NO_CREDENTIALS}
* @event {EventIds#IO_ERROR}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
- * @post add credential ID to remove from the the registration context
+ * @post add the credential identifier to the registration context
*/
public class ExtractKeyRemovalInformationFromFormRequest extends AbstractWebAuthnRegistrationAction {
@@ -62,12 +62,12 @@ public class ExtractKeyRemovalInformationFromFormRequest extends AbstractWebAuth
/**
* Set the name of the credential Id parameter.
*
- * @param field field name
+ * @param parameter parameter name
*/
- public void setCedentialIdParameterName(@Nonnull @NotEmpty final String field) {
+ public void setCredentialIdParameterName(@Nonnull @NotEmpty final String parameter) {
checkSetterPreconditions();
- credentialIdParameterName = Constraint.isNotNull(StringSupport.trimOrNull(field), "Credential ID "
+ credentialIdParameterName = Constraint.isNotNull(StringSupport.trimOrNull(parameter), "Credential ID "
+ "parameter cannot be null or empty");
}
@@ -84,12 +84,12 @@ public class ExtractKeyRemovalInformationFromFormRequest extends AbstractWebAuth
final String credentialId = request.getParameter(credentialIdParameterName);
if (credentialId == null) {
- log.debug("{} CredentialID not found in HTTP request",getLogPrefix());
+ log.debug("{} Credential ID not found in HTTP request",getLogPrefix());
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.NO_CREDENTIALS);
return;
}
try {
- // Should be base64 encoded credential.
+ // Should be a base64 encoded credential.
final byte[] credentialIdAsBytes = Base64Support.decode(credentialId);
context.setCredentialIdToRemove(credentialIdAsBytes);
log.trace("{} Credential to remove '{}'",getLogPrefix(),credentialId);
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameFromRegistrationForm.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameFromRegistrationForm.java
index b3ac5e1..4a3326f 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameFromRegistrationForm.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameFromRegistrationForm.java
@@ -49,7 +49,7 @@ public class ExtractUsernameFromRegistrationForm extends AbstractWebAuthnExtract
/** Strategy used to locate the {@link BaseWebAuthnContext} to operate on. */
@Nonnull private Function<ProfileRequestContext,BaseWebAuthnContext> webAuthnContextLookupStrategy;
- /** Form parameter name to carry username. */
+ /** Form parameter name that carries the username. */
@Nonnull @NotEmpty private String usernameFieldName;
/** Context to operate on. */
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameSearchFromFormRequest.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameSearchFromFormRequest.java
index 328b609..af6b960 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameSearchFromFormRequest.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ExtractUsernameSearchFromFormRequest.java
@@ -83,7 +83,7 @@ public class ExtractUsernameSearchFromFormRequest extends AbstractWebAuthnManage
final String parameterValue = request.getParameter(requestParameterToExtract);
if (parameterValue == null) {
- log.debug("{} {} not found in HTTP request",getLogPrefix(), requestParameterToExtract);
+ log.debug("{} '{}' not found in HTTP request",getLogPrefix(), requestParameterToExtract);
ActionSupport.buildEvent(profileRequestContext, AuthnEventIds.UNKNOWN_USERNAME);
return;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/LookupCredentialsForUser.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/LookupCredentialsForUser.java
index 1d8d5c6..6c31eb4 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/LookupCredentialsForUser.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/LookupCredentialsForUser.java
@@ -51,7 +51,7 @@ public class LookupCredentialsForUser extends AbstractWebAuthnManagementAction {
log.trace("{} No username to search for", getLogPrefix());
context.setFoundCredentials(CollectionSupport.emptyList());
} else {
- log.trace("{} Searching registered credentials for '{}'",getLogPrefix(), userToSearchFor);
+ log.trace("{} Finding registered credentials for '{}'",getLogPrefix(), userToSearchFor);
assert userToSearchFor != null;
final Collection<CredentialRegistration> credentials =
getCredentialRepository().getRegistrationsByUsername(userToSearchFor);
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnManagementContext.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnManagementContext.java
index 8bf5e09..9e1a39e 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnManagementContext.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnManagementContext.java
@@ -32,11 +32,12 @@ import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action to create (or lookup) the {@link WebAuthnManagementContext} and set the admin principal.
+ * An action to create (or lookup) the {@link WebAuthnManagementContext} and set the admin principal name from the
+ * lookup strategy.
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
* @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
- * @post a {@link WebAuthnManagementContext} is created.
+ * @post a {@link WebAuthnManagementContext} is created and the principalName is set.
*/
public class PopulateWebAuthnManagementContext extends AbstractProfileAction {
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnRegistrationContext.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnRegistrationContext.java
index a465a14..1f491b5 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnRegistrationContext.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/PopulateWebAuthnRegistrationContext.java
@@ -34,12 +34,12 @@ import net.shibboleth.shared.logic.PredicateSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action to create (or lookup) the {@link WebAuthnRegistrationContext} and populate
- * with the username found from the lookup strategy.
+ * An action to create (or lookup) the {@link WebAuthnRegistrationContext} and populate with the username found from
+ * the lookup strategy.
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
* @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
- * @post a {@link WebAuthnRegistrationContext} is created, and the username has been populated.
+ * @post a {@link WebAuthnRegistrationContext} is created and the username is populated.
*/
//TODO The population actions are similar, can they be generalised?
public class PopulateWebAuthnRegistrationContext extends AbstractProfileAction {
@@ -101,7 +101,7 @@ public class PopulateWebAuthnRegistrationContext extends AbstractProfileAction {
}
/**
- * Set the lookup strategy to use for the username to match against Duo identity.
+ * Set the lookup strategy to find the username.
*
* @param strategy lookup strategy
*/
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java
index 18d9e05..54238b9 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/RandomUserIdGenerator.java
@@ -27,7 +27,7 @@ import org.slf4j.Logger;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * A user.id generator that generates a 64 byte randomized user.id. Returns {@code null} iff one can not be generated.
+ * A user.id generator that generates a random 64 byte user.id. Returns {@code null} iff one can not be generated.
*/
public final class RandomUserIdGenerator implements Function<ProfileRequestContext, byte[]>{
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 e41d4b2..aeead6b 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
@@ -47,7 +47,7 @@ import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action that stores the public key credential into the credential repository.
+ * An action that adds the public key credential in the registration context to the credential repository.
*
* <p>Importantly, the registration is stored against the user in the context (the authenticated user). This way, even
* if the user changed the webauthn create request in the browser (e.g. to a different userId), it will still be
@@ -127,7 +127,7 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
try {
userIdBase64 = context.getUserId()!=null ? Base64Support.encodeURLSafe(userId) : null;
} catch (final EncodingException e) {
- // Do nothing, just 'null' userId;
+ // Do nothing, just set a 'null' userId;
userIdBase64 = "null";
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java
index 8e7e167..46c88e9 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/ValidateAuthenticatorAttestationResponse.java
@@ -37,8 +37,8 @@ import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * Validate the public key registration attempt by delegating to the WebAuthn client. If valid, store the registration
- * result inside the registration context.
+ * Validate the public key registration attempt by sending the public key credential to the WebAuthn client.
+ * If valid, store the registration result inside the registration context.
*
* @event {WebAuthnRegistrationEventIds#INVALID_REGISTRATION}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/WebAuthnCredentialsAuthenticationFlowLookupStrategy.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/WebAuthnCredentialsAuthenticationFlowLookupStrategy.java
index 61aa861..3388959 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/WebAuthnCredentialsAuthenticationFlowLookupStrategy.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/WebAuthnCredentialsAuthenticationFlowLookupStrategy.java
@@ -40,6 +40,7 @@ import net.shibboleth.shared.primitive.StringSupport;
* {@link WebAuthnRegistrationContext} exists in the {@link ProfileRequestContext} and the user has existing WebAuthn
* credentials. Otherwise the set of {@link #credentialUnavailableFlows} is returned.
*/
+//TODO Not used?
public class WebAuthnCredentialsAuthenticationFlowLookupStrategy extends AbstractIdentifiableInitializableComponent
implements Function<ProfileRequestContext,Set<String>>{
@@ -51,8 +52,7 @@ public class WebAuthnCredentialsAuthenticationFlowLookupStrategy extends Abstrac
private static final String DEFAULT_WEBAUTN_FLOW = "WebAuthn";
/**
- * The set of WebAuthn flows to return if a user has registered WebAuthn credentials.
- * This is defaults to WebAuthn.
+ * The set of WebAuthn flows to return if a user has registered WebAuthn credentials. Defaults to 'WebAuthn'.
*/
@Nonnull @NonnullElements @NotLive @Unmodifiable private Set<String> webAuthnFlows;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list