[java-idp-plugin-webauthn] branch main updated: Make base64 encoding consistent for logging
Phil Smart
philip.smart at jisc.ac.uk
Thu Nov 28 11:21:05 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=5db3c34c003c45a4122e76f9d3d51c6e55855267
The following commit(s) were added to refs/heads/main by this push:
new 5db3c34 Make base64 encoding consistent for logging
5db3c34 is described below
commit 5db3c34c003c45a4122e76f9d3d51c6e55855267
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Nov 28 11:21:01 2024 +0000
Make base64 encoding consistent for logging
- Some userIds were logged as base64URL encoded, and some just base64
encoded. These are now base64 encoded to make it consistent with how
Yubico represents them
---
.../authn/webauthn/admin/impl/AddUserId.java | 13 +---
.../admin/impl/AdminDeletePublicKeyCredential.java | 13 ++--
.../admin/impl/DeletePublicKeyCredential.java | 19 +-----
.../admin/impl/StorePublicKeyCredential.java | 13 +---
.../WebAuthnCredentialToRemoveAuditExtractor.java | 2 +-
.../WebAuthnUserCredentialAddedAuditExtractor.java | 2 +-
...bAuthnUserCredentialToRemoveAuditExtractor.java | 2 +-
.../audit/impl/WebAuthnUserIdAuditExtractor.java | 2 +-
.../impl/YubicoWebAuthnAuthenticationClient.java | 15 +++--
.../authn/webauthn/impl/WebAuthnSupport.java | 71 ++++++++++++++++++++++
.../impl/SecondFactorOnlyCredentialPolicyRule.java | 23 +------
11 files changed, 98 insertions(+), 77 deletions(-)
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 e4493bf..eb476dc 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
@@ -29,11 +29,10 @@ import com.yubico.webauthn.data.ByteArray;
import net.shibboleth.idp.plugin.authn.webauthn.admin.WebAuthnRegistrationEventIds;
import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
import net.shibboleth.idp.plugin.authn.webauthn.impl.AbstractWebAuthnAction;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.WebAuthnSupport;
import net.shibboleth.idp.plugin.authn.webauthn.storage.WebAuthnCredentialRepository;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NonnullBeforeExec;
-import net.shibboleth.shared.codec.Base64Support;
-import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -146,14 +145,8 @@ public class AddUserId extends AbstractWebAuthnAction<WebAuthnRegistrationContex
return;
}
if (log.isTraceEnabled()) {
- String userIdBase64;
- try {
- userIdBase64 = Base64Support.encodeURLSafe(userId);
- } catch (final EncodingException e) {
- // Do nothing, just 'null' userId;
- userIdBase64 = null;
- }
- log.trace("{} Generated user.id '{}' of size '{}'",getLogPrefix(),userIdBase64, userId.length);
+ log.trace("{} Generated user.id '{}' of size '{}'",getLogPrefix(),WebAuthnSupport.toBase64OrUnknown(userId)
+ , userId.length);
}
context.setUserId(userId);
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 ddf19b7..b136078 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
@@ -26,10 +26,9 @@ import com.yubico.webauthn.data.ByteArray;
import net.shibboleth.idp.plugin.authn.webauthn.admin.WebAuthnRegistrationEventIds;
import net.shibboleth.idp.plugin.authn.webauthn.audit.impl.AbstractWebAuthnAuditingAction;
import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnManagementContext;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.WebAuthnSupport;
import net.shibboleth.idp.plugin.authn.webauthn.storage.WebAuthnCredentialRepository;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
-import net.shibboleth.shared.codec.Base64Support;
-import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -87,13 +86,9 @@ public class AdminDeletePublicKeyCredential extends AbstractWebAuthnAuditingActi
final boolean removed = repository.removeRegistrationByUsernameAndCredentialId(
credentialUsername, new ByteArray(credentialId));
- if (log.isDebugEnabled()) {
- try {
- log.debug("{} Credential '{}' {} removed", getLogPrefix(), Base64Support.encodeURLSafe(credentialId),
- removed ? "was" : "was not");
- } catch (final EncodingException e) {
- // Do nothing if the encoding fails.
- }
+ if (log.isDebugEnabled()) {
+ log.debug("{} Credential '{}' {} removed", getLogPrefix(), WebAuthnSupport.toBase64OrUnknown(credentialId),
+ removed ? "was" : "was not");
}
if (removed) {
auditSuccess(profileRequestContext, "credential-removed");
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 269f42a..9f6ddec 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
@@ -17,7 +17,6 @@ package net.shibboleth.idp.plugin.authn.webauthn.admin.impl;
import java.util.Optional;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
import org.opensaml.messaging.context.navigate.ChildContextLookup;
import org.opensaml.profile.action.ActionSupport;
@@ -29,11 +28,10 @@ import com.yubico.webauthn.data.ByteArray;
import net.shibboleth.idp.plugin.authn.webauthn.admin.WebAuthnRegistrationEventIds;
import net.shibboleth.idp.plugin.authn.webauthn.audit.impl.AbstractWebAuthnAuditingAction;
import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.WebAuthnSupport;
import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRecord;
import net.shibboleth.idp.plugin.authn.webauthn.storage.WebAuthnCredentialRepository;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
-import net.shibboleth.shared.codec.Base64Support;
-import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -97,7 +95,7 @@ public class DeletePublicKeyCredential extends AbstractWebAuthnAuditingAction<We
// This is not an error
if (log.isDebugEnabled()) {
log.debug("{} Unable to find credential '{}' to remove for user '{}', nothing to "
- + "remove", getLogPrefix(), getCredentialIdBase64OrNull(credentialId), username);
+ + "remove", getLogPrefix(), WebAuthnSupport.toBase64OrUnknown(credentialId), username);
}
} else {
final boolean removed = repository.removeRegistrationByUsername(username, credential.get());
@@ -114,17 +112,4 @@ public class DeletePublicKeyCredential extends AbstractWebAuthnAuditingAction<We
context.setCredentialIdToRemove(null);
}
- /**
- * Convert the credential ID in bytes to a Base64URL encoded string.
- *
- * @param credentialId the credential identifier in bytes
- * @return the credential identifier as a base64URL encoded string.
- */
- @Nullable private String getCredentialIdBase64OrNull(@Nonnull final byte[] credentialId) {
- try {
- return Base64Support.encodeURLSafe(credentialId);
- } catch (final EncodingException e) {
- return null;
- }
- }
}
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 79bf7ae..3304b1d 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
@@ -34,11 +34,10 @@ import net.shibboleth.idp.plugin.authn.webauthn.admin.RegistrationResult;
import net.shibboleth.idp.plugin.authn.webauthn.admin.WebAuthnRegistrationEventIds;
import net.shibboleth.idp.plugin.authn.webauthn.audit.impl.AbstractWebAuthnAuditingAction;
import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnRegistrationContext;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.WebAuthnSupport;
import net.shibboleth.idp.plugin.authn.webauthn.storage.CredentialRecord;
import net.shibboleth.idp.plugin.authn.webauthn.storage.WebAuthnCredentialRepository;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
-import net.shibboleth.shared.codec.Base64Support;
-import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -177,14 +176,8 @@ public class StorePublicKeyCredential extends AbstractWebAuthnAuditingAction<Web
private void logRegistration(@Nonnull final WebAuthnRegistrationContext context, @Nullable final String username,
@Nonnull final RegistrationResult registrationResult) {
if (log.isInfoEnabled()) {
- String userIdBase64;
- try {
- final byte[] userId = context.getUserId();
- userIdBase64 = userId !=null ? Base64Support.encodeURLSafe(userId) : null;
- } catch (final EncodingException e) {
- // Do nothing, just set a 'null' userId;
- userIdBase64 = "null";
- }
+ final byte[] userId = context.getUserId();
+ final String userIdBase64 = userId !=null ? WebAuthnSupport.toBase64OrUnknown(userId) : null;
log.info("{} Added public key credential registration for user '{}' with user.id '{}' and key '{}'. "
+ "Using a discoverable credential '{}' and user verification '{}'",
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnCredentialToRemoveAuditExtractor.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnCredentialToRemoveAuditExtractor.java
index a685af3..ee8142d 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnCredentialToRemoveAuditExtractor.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnCredentialToRemoveAuditExtractor.java
@@ -49,7 +49,7 @@ public class WebAuthnCredentialToRemoveAuditExtractor extends AbstractWebAuthnM
}
try {
- return Base64Support.encodeURLSafe(credentialToRemove);
+ return Base64Support.encode(credentialToRemove, false);
} catch (final EncodingException e) {
return null;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialAddedAuditExtractor.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialAddedAuditExtractor.java
index c612c4e..bdd9531 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialAddedAuditExtractor.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialAddedAuditExtractor.java
@@ -53,7 +53,7 @@ public class WebAuthnUserCredentialAddedAuditExtractor extends AbstractWebAuthn
}
try {
- return Base64Support.encodeURLSafe(credentialToRemove);
+ return Base64Support.encode(credentialToRemove, false);
} catch (final EncodingException e) {
return null;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialToRemoveAuditExtractor.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialToRemoveAuditExtractor.java
index ba9e954..fcd178c 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialToRemoveAuditExtractor.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserCredentialToRemoveAuditExtractor.java
@@ -49,7 +49,7 @@ public class WebAuthnUserCredentialToRemoveAuditExtractor extends AbstractWebAu
}
try {
- return Base64Support.encodeURLSafe(credentialToRemove);
+ return Base64Support.encode(credentialToRemove, false);
} catch (final EncodingException e) {
return null;
}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserIdAuditExtractor.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserIdAuditExtractor.java
index 780e244..da81033 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserIdAuditExtractor.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/audit/impl/WebAuthnUserIdAuditExtractor.java
@@ -48,7 +48,7 @@ public class WebAuthnUserIdAuditExtractor extends AbstractWebAuthnAuditExtractor
return null;
}
try {
- return Base64Support.encodeURLSafe(userId);
+ return Base64Support.encode(userId, false);
} catch (final EncodingException e) {
return null;
}
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 2346617..6d8dcde 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
@@ -50,6 +50,7 @@ import net.shibboleth.idp.plugin.authn.webauthn.client.WebAuthnAuthenticationCli
import net.shibboleth.idp.plugin.authn.webauthn.exception.AssertionFailureException;
import net.shibboleth.idp.plugin.authn.webauthn.exception.RegistrationFailureException;
import net.shibboleth.idp.plugin.authn.webauthn.exception.WebAuthnAuthenticationClientException;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.WebAuthnSupport;
import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.annotation.constraint.NotLive;
import net.shibboleth.shared.logic.Constraint;
@@ -164,12 +165,14 @@ public class YubicoWebAuthnAuthenticationClient implements WebAuthnAuthenticatio
.username(Optional.ofNullable(username))
.build();
- if (username == null && userId == null) {
- log.debug("Attempting validation of assumed discoverable credential with userHandle from response '{}'",
- authenticatorAssertionResponse.getResponse().getUserHandle());
- } else {
- log.debug("Attempting validation of credential with username '{}' and userHandle '{}'",
- username, userId);
+ if (log.isDebugEnabled()) {
+ if (username == null && userId == null) {
+ log.debug("Attempting validation of assumed discoverable credential with userHandle from response "
+ + "'{}'", authenticatorAssertionResponse.getResponse().getUserHandle());
+ } else {
+ log.debug("Attempting validation of credential with username '{}' and userHandle '{}'",
+ username, WebAuthnSupport.toBase64OrUnknown(userId));
+ }
}
final com.yubico.webauthn.AssertionResult result = rp.finishAssertion(FinishAssertionOptions.builder()
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnSupport.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnSupport.java
new file mode 100644
index 0000000..fef3032
--- /dev/null
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/impl/WebAuthnSupport.java
@@ -0,0 +1,71 @@
+/*
+ * 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.impl;
+
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
+import net.shibboleth.shared.codec.Base64Support;
+import net.shibboleth.shared.codec.EncodingException;
+
+/**
+ * Helper methods for WebAuthn.
+ */
+ at ThreadSafe
+public final class WebAuthnSupport {
+
+ /**
+ * Private constructor.
+ */
+ private WebAuthnSupport() {
+ }
+
+ /**
+ * Convert the value to a Base64 URL encoded string. Return "unknown" if there is an error.
+ *
+ * @param value the value to Base64 URL encode
+ * @return the Bas64 URL encoded string or "unknown".
+ */
+ public static String toBase64UrlOrUnknown(@Nullable final byte[] value) {
+ if (value == null) {
+ return "unknown";
+ }
+ try {
+ return Base64Support.encodeURLSafe(value);
+ } catch (final EncodingException e) {
+ return "unknown";
+ }
+ }
+
+
+ /**
+ * Convert the value to a Base64 encoded string. Return "unknown" if there is an error.
+ *
+ * @param value the value to Base64 URL encode
+ * @return the Bas64 URL encoded string or "unknown".
+ */
+ public static String toBase64OrUnknown(@Nullable final byte[] value) {
+ if (value == null) {
+ return "unknown";
+ }
+ try {
+ return Base64Support.encode(value, false);
+ } catch (final EncodingException e) {
+ return "unknown";
+ }
+ }
+
+
+}
diff --git a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/policy/impl/SecondFactorOnlyCredentialPolicyRule.java b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/policy/impl/SecondFactorOnlyCredentialPolicyRule.java
index 9e3b822..226f7a3 100644
--- a/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/policy/impl/SecondFactorOnlyCredentialPolicyRule.java
+++ b/webauthn-impl/src/main/java/net/shibboleth/idp/plugin/authn/webauthn/policy/impl/SecondFactorOnlyCredentialPolicyRule.java
@@ -18,7 +18,6 @@ import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
@@ -28,12 +27,11 @@ import com.yubico.webauthn.data.ByteArray;
import net.shibboleth.idp.plugin.authn.webauthn.authn.AuthenticatorSupport;
import net.shibboleth.idp.plugin.authn.webauthn.context.WebAuthnAuthenticationContext;
+import net.shibboleth.idp.plugin.authn.webauthn.impl.WebAuthnSupport;
import net.shibboleth.idp.plugin.authn.webauthn.policy.CredentialPolicy;
import net.shibboleth.idp.plugin.authn.webauthn.storage.EnhancedCredentialRecord;
import net.shibboleth.shared.annotation.constraint.NotLive;
import net.shibboleth.shared.annotation.constraint.Unmodifiable;
-import net.shibboleth.shared.codec.Base64Support;
-import net.shibboleth.shared.codec.EncodingException;
import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.shared.primitive.StringSupport;
@@ -91,28 +89,11 @@ public class SecondFactorOnlyCredentialPolicyRule extends AbstractCredentialPoli
log.trace("Rejected credential '{}', authentication is sole-factor and authenticator '{}' that "
+ "created the credential should only be used as a second factor",
credential.getCredentialRecord().getCredentialIdBase64Url(),
- toBase64OrUnknown(credential.getCredentialRecord().getAaguid()));
+ WebAuthnSupport.toBase64UrlOrUnknown(credential.getCredentialRecord().getAaguid()));
}
return CredentialPolicyOutcome.REJECT;
}
return CredentialPolicyOutcome.ACCEPT;
}
-
- /**
- * Convert the value to Base64 URL encoding or return "unknown" if there is an error.
- *
- * @param value the value to Base64 URL encode
- * @return the Bas64 URL encoded string or "unknown".
- */
- private String toBase64OrUnknown(@Nullable final byte[] value) {
- if (value == null) {
- return "unknown";
- }
- try {
- return Base64Support.encodeURLSafe(value);
- } catch (final EncodingException e) {
- return "unknown";
- }
- }
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list