[java-idp-plugin-webauthn] 03/03: Fix Javadoc, Add missing annotations, cleanup log output
Phil Smart
philip.smart at jisc.ac.uk
Fri Mar 15 10:25:26 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=f73a3edff527287477644e67f9720acf1730ffe3
commit f73a3edff527287477644e67f9720acf1730ffe3
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Mar 15 10:25:20 2024 +0000
Fix Javadoc, Add missing annotations, cleanup log output
---
.../webauthn/context/WebAuthnRegistrationContext.java | 2 +-
.../idp/plugin/authn/webauthn/admin/impl/AddUserId.java | 3 +--
.../impl/ValidateAuthenticatorAttestationResponse.java | 13 ++++++++++---
.../impl/AbstractWebAuthnAuthenticationAction.java | 16 ++++++++--------
.../idp/plugin/authn/webauthn/impl/WebAuthnEncoder.java | 8 +++++---
.../impl/IdPStorageServiceCredentialRespository.java | 7 +++++--
6 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java
index 78aefef..883c97f 100644
--- a/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java
+++ b/webauthn-api/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/context/WebAuthnRegistrationContext.java
@@ -195,7 +195,7 @@ public final class WebAuthnRegistrationContext extends BaseWebAuthnContext {
*
* @return the credential nickname.
*/
- public String getCredentialNickname() {
+ @Nullable public String getCredentialNickname() {
return credentialNickname;
}
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 f375a19..08b7621 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
@@ -44,7 +44,6 @@ import net.shibboleth.shared.primitive.LoggerFactory;
*
* <p>Note, the same user should use the same UserHandle.</p>
*
- * @event {@link WebAuthnRegistrationEventIds#INVALID_REGISTRATION_CTX}
* @event {@link WebAuthnRegistrationEventIds#INVALID_REGISTRATION}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
* @post a UserHandle is added to the registration context
@@ -89,7 +88,7 @@ public class AddUserId extends AbstractWebAuthnRegistrationAction {
username = context.getUsername();
if (username == null) {
log.error("{} Username not available in registration context", getLogPrefix());
- ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION_CTX);
+ ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
return false;
}
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 b7a3f58..e30a2f7 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
@@ -88,12 +88,19 @@ public class ValidateAuthenticatorAttestationResponse extends AbstractWebAuthnRe
try {
final RegistrationResult credentialPublicKey =
- getWebAuthnClient().validateAuthenticatorAttestationResponse(pkCredCreationOptions, attestation);
+ getWebAuthnClient().validateAuthenticatorAttestationResponse(pkCredCreationOptions, attestation);
+
+ // If untrusted attestations are not allowed, it will not get this far. This is only useful to log if
+ // untrusted attestations are allowed and the metadata has been loaded and the attestation has been
+ // checked.
+ log.debug("{} Was attestation for authenticator '{}' trusted? {}", getLogPrefix(),
+ credentialPublicKey.getAaguid().getHex(),
+ credentialPublicKey.isAttestationTrusted() ? "Yes" : "No");
// If valid. Add back to context
context.setRegistrationResult(credentialPublicKey);
- log.info("Public Key Registration was valid");
+ log.info("{} Public Key Registration was valid", getLogPrefix());
} catch (final RegistrationFailureException e) {
- log.warn("{} Public key credential creation options was invalid", getLogPrefix(), e);
+ log.warn("{} Public key registration failed", getLogPrefix(), e);
ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
return;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnAuthenticationAction.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnAuthenticationAction.java
index 61af477..3678978 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnAuthenticationAction.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/AbstractWebAuthnAuthenticationAction.java
@@ -56,7 +56,7 @@ public abstract class AbstractWebAuthnAuthenticationAction extends AbstractAuthe
/** Class logger. */
@Nonnull @NotEmpty private final Logger log = LoggerFactory.getLogger(AbstractWebAuthnAuthenticationAction.class);
- /** Lookup strategy to locate the webauthn authentication context. */
+ /** Lookup strategy to locate the WebAuthn authentication context. */
@Nonnull private Function<ProfileRequestContext,WebAuthnAuthenticationContext> webauthnContextLookupStrategy;
/** The WebAuthn authentication Context.*/
@@ -69,7 +69,7 @@ public abstract class AbstractWebAuthnAuthenticationAction extends AbstractAuthe
/**
* Set the WebAuthn client used to handle registration and authentication ceremonies.
*
- * @param client The webauthnClient to set.
+ * @param client The WebAuthn client to set.
*/
public void setWebAuthnClient(@Nonnull final WebAuthnAuthenticationClient client) {
checkSetterPreconditions();
@@ -79,9 +79,9 @@ public abstract class AbstractWebAuthnAuthenticationAction extends AbstractAuthe
/**
* Get the WebAuthn client used to handle registration and authentication ceremonies.
*
- * @return the webAuthnClient.
+ * @return the WebAuthn client.
*/
- @NonnullBeforeExec public WebAuthnAuthenticationClient getWebAuthnClient() {
+ @NonnullBeforeExec protected WebAuthnAuthenticationClient getWebAuthnClient() {
checkComponentActive();
return webAuthnClient;
}
@@ -96,7 +96,7 @@ public abstract class AbstractWebAuthnAuthenticationAction extends AbstractAuthe
/**
- * Set WebAuthn authentication context lookup strategy to use.
+ * Set the WebAuthn authentication context lookup strategy to use.
*
* @param strategy lookup strategy
*/
@@ -157,7 +157,7 @@ public abstract class AbstractWebAuthnAuthenticationAction extends AbstractAuthe
*
* @param profileRequestContext the current IdP profile request context
* @param authenticationContext the current authentication context
- * @param context the webauthn authentication context
+ * @param context the WebAuthn authentication context
*
* @return true iff execution should continue
*/
@@ -168,12 +168,12 @@ public abstract class AbstractWebAuthnAuthenticationAction extends AbstractAuthe
}
/**
- * Performs this webauthn authentication action using the supplied webauthn context. Implementations
+ * Performs this WebAuthn authentication action using the supplied WebAuthn context. Implementations
* should override this method.
*
* @param profileRequestContext the current IdP profile request context
* @param authenticationContext the current authentication context
- * @param context the webauthn authentication context
+ * @param context the WebAuthn authentication context
*/
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final AuthenticationContext authenticationContext,
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnEncoder.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnEncoder.java
index fd206ef..b12c3ea 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnEncoder.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnEncoder.java
@@ -70,10 +70,13 @@ public final class WebAuthnEncoder {
* @return is the credential discoverable (a 'passkey' or resident key) true, false, or unknown
*/
@Nonnull public static String formatDiscoverable(@Nullable final Optional<Boolean> discoverable) {
+ // Optional should not be nullable, but we can not guarantee the call-site for this
if (discoverable == null) {
return "not-set";
}
- return discoverable.isEmpty() ? "unknown" : Boolean.toString(discoverable.get());
+ final String formatted = discoverable.isEmpty() ? "unknown" : Boolean.toString(discoverable.get());
+ assert formatted != null;
+ return formatted;
}
/**
@@ -86,8 +89,7 @@ public final class WebAuthnEncoder {
public static String formatTransports(@Nullable final Set<AuthenticatorTransport> transports) {
if (transports == null) {
return "";
- }
-
+ }
return transports.stream().map(AuthenticatorTransport::getId).collect(Collectors.joining(","));
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
index 2689843..e4ea264 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/storage/impl/IdPStorageServiceCredentialRespository.java
@@ -56,10 +56,13 @@ import net.shibboleth.shared.logic.ConstraintViolationException;
/**
* A {@link CredentialRepository} adaptor for the Shibboleth {@link StorageService}.
*
+ * <p>Username is used as the key to the storage context. The object to store (value of the storage record) is a set of
+ * {@link CredentialRegistration registered credentials}. Storage records do not expire.</p>
+ *
* <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 after init.</p>
+ * <p>This repository is thread-safe after it is initialised.</p>
*/
@ThreadSafeAfterInit
public class IdPStorageServiceCredentialRespository extends AbstractIdentifiableInitializableComponent
@@ -202,7 +205,7 @@ public class IdPStorageServiceCredentialRespository extends AbstractIdentifiable
*
* @param userHandle the userHandle to match
*
- * @return registrations that match that userHandle from any
+ * @return registrations that match that userHandle from any user.
*/
private Collection<CredentialRegistration> getRegistrationsByUserHandle(final ByteArray userHandle) {
checkComponentActive();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list