[java-idp-plugin-webauthn] branch main updated: Code, config, and Javadoc cleanup
Phil Smart
philip.smart at jisc.ac.uk
Wed May 1 14:43:20 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=a121786b5efcf510325668279700b5b14dc7e9b1
The following commit(s) were added to refs/heads/main by this push:
new a121786 Code, config, and Javadoc cleanup
a121786 is described below
commit a121786b5efcf510325668279700b5b14dc7e9b1
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Wed May 1 15:43:17 2024 +0100
Code, config, and Javadoc cleanup
---
.../authn/webauthn/admin/impl/AddDisplayName.java | 3 ++-
.../idp/plugin/authn/webauthn/admin/impl/AddUserId.java | 6 ++++--
.../webauthn/admin/impl/StorePublicKeyCredential.java | 17 +++++++++++------
.../impl/ValidateAuthenticatorAttestationResponse.java | 4 ++--
.../client/impl/YubicoWebAuthnAuthenticationClient.java | 3 +--
.../authn/webauthn/impl/GenerateServerChallenge.java | 6 ++++--
.../webauthn-registration-beans.xml | 11 ++++++-----
7 files changed, 30 insertions(+), 20 deletions(-)
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 17a7be7..207cd17 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,7 +35,8 @@ 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.
+ * An action to generate or lookup a user.displayName. The display name is for display purposes only and only used for
+ * the PublicKeyCredentialCreationOptions.
*
* @event {@link 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/AddUserId.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/admin/impl/AddUserId.java
index 1ed0a45..c019875 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
@@ -39,14 +39,16 @@ 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>The user.id could contain some form of state if required, but must not contain retrievable PII.</p>
*
- * <p>The same user should have the same user.id.</p>
+ * <p>The same IdP user should have the same user.id.</p>
*
*
* @event {@link WebAuthnRegistrationEventIds#INVALID_REGISTRATION}
* @pre <pre>ProfileRequestContext.getSubcontext(WebAuthnRegistrationContext.class) != null</pre>
- * @post a UserHandle is added to the registration context
+ * @post a user ID is added to the registration context
*/
public class AddUserId extends AbstractWebAuthnRegistrationAction {
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 c509ade..ede40f0 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
@@ -61,12 +61,19 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
@Nonnull final WebAuthnRegistrationContext context) {
final String username = context.getUsername();
-
if (username == null) {
log.error("Unable to find username in registration context");
ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
return;
- }
+ }
+
+ final String displayName = context.getDisplayName();
+ if (displayName == null) {
+ log.error("Unable to find displayName in registration context");
+ ActionSupport.buildEvent(profileRequestContext, WebAuthnRegistrationEventIds.INVALID_REGISTRATION);
+ return;
+ }
+
final RegistrationResult registrationResult = context.getRegistrationResult();
if (registrationResult == null) {
log.error("Unable to find registration result in registration context");
@@ -80,10 +87,9 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
.publicKeyCose(registrationResult.getPublicKeyCose())
.build();
- //TODO user identity should come from somewhere else?
final UserIdentity user = UserIdentity.builder()
.name(username)
- .displayName(username)
+ .displayName(displayName)
.id(new ByteArray(context.getUserId()))
.build();
@@ -92,7 +98,6 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
final Instant now = Instant.now();
assert now != null;
final Optional<Boolean> isDiscoverable = registrationResult.isDiscoverable();
- assert isDiscoverable != null;
final CredentialRegistration registration = CredentialRegistration.builder()
.withUserIdentity(user)
@@ -108,7 +113,7 @@ public class StorePublicKeyCredential extends AbstractWebAuthnRegistrationAction
getCredentialRepository().addRegistrationByUsername(username, registration);
log.debug("{} Added public key credential registration for user '{}' and key '{}'. Using a "
- + "discoverable credential '{}', and user verification '{}'",
+ + "discoverable credential '{}' and user verification '{}'",
getLogPrefix(), username, registrationResult.getKeyId().getId().getBase64Url(),
registrationResult.isDiscoverable().isPresent() ? registrationResult.isDiscoverable() : "unknown",
registrationResult.isUserVerified());
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 e98fc8f..0ffe4b6 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 it inside the
- * registration context.
+ * Validate the public key registration attempt by delegating 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/client/impl/YubicoWebAuthnAuthenticationClient.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java
index 38b0fec..c0855ee 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/client/impl/YubicoWebAuthnAuthenticationClient.java
@@ -180,10 +180,9 @@ public class YubicoWebAuthnAuthenticationClient implements WebAuthnAuthenticatio
.response(authenticatorAssertionResponse)
.build());
if (result == null) {
- throw new AssertionFailureException("Unable to validate authenticator assertion");
+ throw new AssertionFailureException("Unable to validate authenticator assertion, null result");
}
if (!result.isSuccess()) {
- // TODO why do they use both success and throw an exception?
// I think this is *always* true if valid, and will throw if not valid. But just in case.
throw new AssertionFailureException("Authenticator assertion was not valid");
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
index c97f11d..2b764bb 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/GenerateServerChallenge.java
@@ -35,7 +35,9 @@ import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An action to generate an appropriate server challenge for use.
+ * An action to generate an appropriate server challenge for use in either the PublicKeyCredentialCreationOptions or the
+ * PublicKeyCredentialRequestOptions. A challenge must have high entropy such that it can not be guessed, and should
+ * therefore be at least 16 bytes long.
*
* @event {@link org.opensaml.profile.action.EventIds#PROCEED_EVENT_ID}
* @event {@link org.opensaml.profile.action.EventIds#INVALID_PROFILE_CTX}
@@ -91,7 +93,7 @@ public class GenerateServerChallenge extends AbstractWebAuthnBaseAction {
/** {@inheritDoc} */
@Override
- @Nullable public byte[] apply(final ProfileRequestContext input) {
+ @Nullable public byte[] apply(@Nullable final ProfileRequestContext input) {
try {
final byte[] bytes = new byte[32];
SecureRandom.getInstanceStrong().nextBytes(bytes);
diff --git a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
index 521e29f..93e9170 100644
--- a/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
+++ b/webauthn-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/admin/webauthn-registration/webauthn-registration-beans.xml
@@ -39,12 +39,13 @@
<!-- Important that this gets the username from the subject context, not the initial context that is created -->
<bean id="PopulateWebAuthnRegistrationContext" scope="prototype"
- class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.PopulateWebAuthnRegistrationContext">
- <property name="usernameLookupStrategy">
- <bean id="usernameFromAuthnResult" scope="prototype"
- class="net.shibboleth.idp.plugin.authn.webauthn.context.navigate.UsernameLookupFromSubjectContext"/>
- </property>
+ class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.PopulateWebAuthnRegistrationContext"
+ p:usernameLookupStrategy="#{getObject('%{idp.authn.webauthn.registration.usernameLookupStrategy:shibboleth.authn.webauthn.RegistrationUsernameLookupStrategy}')}">
</bean>
+
+ <!-- Default username comes from previous c14n or session. -->
+ <bean id="shibboleth.authn.webauthn.RegistrationUsernameLookupStrategy"
+ class="net.shibboleth.idp.plugin.authn.webauthn.context.navigate.UsernameLookupFromSubjectContext" />
<bean id="AddAttestationConveyancePreference" scope="prototype" parent="AbstractWebAuthnRegistrationAction"
class="net.shibboleth.idp.plugin.authn.webauthn.admin.impl.AddAttestationConveyancePreference"
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list