[java-identity-provider] branch main updated: IDP-2069 Null handling
Rod Widdowson
rdw at steadingsoftware.com
Wed Feb 22 16:23:56 UTC 2023
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=a01c5bac07b11ca06dc1c8902d5226ba9b4a891f
The following commit(s) were added to refs/heads/main by this push:
new a01c5bac0 IDP-2069 Null handling
a01c5bac0 is described below
commit a01c5bac07b11ca06dc1c8902d5226ba9b4a891f
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed Feb 22 14:53:30 2023 +0000
IDP-2069 Null handling
https://shibboleth.atlassian.net/browse/IDP-2069
Start to clean idp-auth-impl. Work still ongoing.
---
.../AttemptedAuthenticationFlowAuditExtractor.java | 12 +++-
.../impl/AttemptedUsernameAuditExtractor.java | 1 +
.../impl/AuthenticationErrorAuditExtractor.java | 1 +
.../impl/AuthenticationFlowAuditExtractor.java | 10 +++-
.../impl/CertificateIssuerAuditExtractor.java | 1 +
.../impl/CertificateSubjectAuditExtractor.java | 1 +
.../impl/TransformedUsernameAuditExtractor.java | 1 +
.../authn/duo/impl/AbstractDuoAuthenticator.java | 7 ++-
.../idp/authn/duo/impl/DuoAuthAPIResponse.java | 6 +-
.../idp/authn/duo/impl/DuoAuthAuthenticator.java | 5 +-
.../idp/authn/duo/impl/DuoAuthResponse.java | 6 +-
.../shibboleth/idp/authn/duo/impl/DuoDevice.java | 13 +++--
.../idp/authn/duo/impl/DuoPreauthResponse.java | 3 +-
.../idp/authn/duo/impl/DuoResponseWrapper.java | 6 +-
.../shibboleth/idp/authn/duo/impl/DuoSupport.java | 6 +-
.../idp/authn/duo/impl/ValidateDuoAuthAPI.java | 67 +++++++++++++---------
.../idp/authn/duo/impl/ValidateDuoWebResponse.java | 22 ++++---
.../ExtractDuoAuthenticationFromHeadersTest.java | 40 +++++++++----
18 files changed, 144 insertions(+), 64 deletions(-)
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedAuthenticationFlowAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedAuthenticationFlowAuditExtractor.java
index be982c274..8b37d0562 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedAuthenticationFlowAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedAuthenticationFlowAuditExtractor.java
@@ -23,6 +23,7 @@ import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
+import net.shibboleth.idp.authn.AuthenticationFlowDescriptor;
import net.shibboleth.idp.authn.context.AuthenticationContext;
/**
@@ -33,11 +34,16 @@ import net.shibboleth.idp.authn.context.AuthenticationContext;
public class AttemptedAuthenticationFlowAuditExtractor implements Function<ProfileRequestContext,String> {
/** {@inheritDoc} */
- @Nullable public String apply(@Nullable final ProfileRequestContext input) {
+ @Nullable public String apply(final @Nullable ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
- if (authnCtx != null && authnCtx.getAttemptedFlow() != null) {
- return authnCtx.getAttemptedFlow().getId();
+ AuthenticationFlowDescriptor flow = null;
+ if (authnCtx != null) {
+ flow = authnCtx.getAttemptedFlow();
+ }
+ if (flow != null) {
+ return flow.getId();
}
return null;
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedUsernameAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedUsernameAuditExtractor.java
index fcc98440b..1caa1559a 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedUsernameAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AttemptedUsernameAuditExtractor.java
@@ -38,6 +38,7 @@ public class AttemptedUsernameAuditExtractor implements Function<ProfileRequestC
/** {@inheritDoc} */
@Nullable public String apply(@Nullable final ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
if (authnCtx != null) {
final UsernamePasswordContext upContext = authnCtx.getSubcontext(UsernamePasswordContext.class);
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationErrorAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationErrorAuditExtractor.java
index 9901ccb05..f6b09870e 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationErrorAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationErrorAuditExtractor.java
@@ -39,6 +39,7 @@ public class AuthenticationErrorAuditExtractor implements Function<ProfileReques
/** {@inheritDoc} */
@Nullable public Collection<String> apply(@Nullable final ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
if (authnCtx != null) {
final AuthenticationErrorContext errorCtx = authnCtx.getSubcontext(AuthenticationErrorContext.class);
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationFlowAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationFlowAuditExtractor.java
index c01fb2758..5660fe288 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationFlowAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/AuthenticationFlowAuditExtractor.java
@@ -23,6 +23,7 @@ import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
+import net.shibboleth.idp.authn.AuthenticationResult;
import net.shibboleth.idp.authn.context.AuthenticationContext;
/** {@link Function} that returns the authentication flow ID used to satisfy a request. */
@@ -31,9 +32,14 @@ public class AuthenticationFlowAuditExtractor implements Function<ProfileRequest
/** {@inheritDoc} */
@Nullable public String apply(@Nullable final ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
- if (authnCtx != null && authnCtx.getAuthenticationResult() != null) {
- return authnCtx.getAuthenticationResult().getAuthenticationFlowId();
+ AuthenticationResult result = null;
+ if (authnCtx != null) {
+ result = authnCtx.getAuthenticationResult();
+ }
+ if (result != null) {
+ return result.getAuthenticationFlowId();
}
return null;
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateIssuerAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateIssuerAuditExtractor.java
index 40f68925a..5b591ff11 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateIssuerAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateIssuerAuditExtractor.java
@@ -34,6 +34,7 @@ public class CertificateIssuerAuditExtractor implements Function<ProfileRequestC
/** {@inheritDoc} */
@Nullable public String apply(@Nullable final ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
if (authnCtx != null) {
final CertificateContext cc = authnCtx.getSubcontext(CertificateContext.class);
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateSubjectAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateSubjectAuditExtractor.java
index 9bd7e1f67..84fe1aed8 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateSubjectAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/CertificateSubjectAuditExtractor.java
@@ -34,6 +34,7 @@ public class CertificateSubjectAuditExtractor implements Function<ProfileRequest
/** {@inheritDoc} */
@Nullable public String apply(@Nullable final ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
if (authnCtx != null) {
final CertificateContext cc = authnCtx.getSubcontext(CertificateContext.class);
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/TransformedUsernameAuditExtractor.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/TransformedUsernameAuditExtractor.java
index 8263f1b70..08b8140a1 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/TransformedUsernameAuditExtractor.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/audit/impl/TransformedUsernameAuditExtractor.java
@@ -37,6 +37,7 @@ public class TransformedUsernameAuditExtractor implements Function<ProfileReques
/** {@inheritDoc} */
@Nullable public String apply(@Nullable final ProfileRequestContext input) {
+ assert input != null;
final AuthenticationContext authnCtx = input.getSubcontext(AuthenticationContext.class);
if (authnCtx != null) {
final UsernamePasswordContext upContext = authnCtx.getSubcontext(UsernamePasswordContext.class);
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/AbstractDuoAuthenticator.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/AbstractDuoAuthenticator.java
index 5564ce2e5..b64af49f8 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/AbstractDuoAuthenticator.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/AbstractDuoAuthenticator.java
@@ -119,10 +119,13 @@ public abstract class AbstractDuoAuthenticator extends AbstractInitializableComp
// Make the request.
final HttpClientContext clientContext = HttpClientContext.create();
+ assert clientContext != null;
HttpClientSecuritySupport.marshalSecurityParameters(clientContext, httpClientSecurityParameters, true);
HttpClientSecuritySupport.addDefaultTLSTrustEngineCriteria(clientContext, request);
final ClassicHttpResponse httpResponse = httpClient.executeOpen(null, request, clientContext);
- HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, request.getScheme());
+ final String scheme = request.getScheme();
+ assert scheme != null;
+ HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, scheme);
// Check the HTTP response code.
final int httpStatusCode = httpResponse.getCode();
@@ -155,4 +158,4 @@ public abstract class AbstractDuoAuthenticator extends AbstractInitializableComp
return duoResponse;
}
-}
\ No newline at end of file
+}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAPIResponse.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAPIResponse.java
index 840aeb5c2..123cac4df 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAPIResponse.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAPIResponse.java
@@ -33,11 +33,11 @@ public abstract class DuoAuthAPIResponse {
/** the result. */
@JsonProperty("result")
- @Nonnull private String result;
+ private String result;
/** the status message. */
@JsonProperty("status_msg")
- @Nonnull private String statusMessage;
+ private String statusMessage;
/**
* Get the Duo result string.
@@ -45,6 +45,7 @@ public abstract class DuoAuthAPIResponse {
* @return the result string
*/
@Nonnull public String getResult() {
+ assert result != null;
return result;
}
@@ -54,6 +55,7 @@ public abstract class DuoAuthAPIResponse {
* @return the Duo status message
*/
@Nonnull public String getStatusMessage() {
+ assert statusMessage != null;
return statusMessage;
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAuthenticator.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAuthenticator.java
index 2a463a68e..039492886 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAuthenticator.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthAuthenticator.java
@@ -53,6 +53,7 @@ public class DuoAuthAuthenticator extends AbstractDuoAuthenticator {
@Nonnull private final TypeReference<DuoResponseWrapper<DuoAuthResponse>> wrapperTypeRef;
/** Constructor. */
+ @SuppressWarnings("null")
public DuoAuthAuthenticator() {
wrapperTypeRef = new TypeReference<>() {};
paramEscaper = UrlEscapers.urlFormParameterEscaper();
@@ -77,6 +78,7 @@ public class DuoAuthAuthenticator extends AbstractDuoAuthenticator {
.setPath("/auth/v2/auth").build();
final ClassicRequestBuilder rb =
ClassicRequestBuilder.post().setUri(uri).addParameter(DuoAuthAPI.DUO_USERNAME, duoContext.getUsername());
+ assert rb != null;
if (duoContext.getClientAddress() != null) {
rb.addParameter(DuoAuthAPI.DUO_IPADDR, duoContext.getClientAddress());
}
@@ -98,6 +100,7 @@ public class DuoAuthAuthenticator extends AbstractDuoAuthenticator {
}
DuoSupport.signRequest(rb, duoIntegration);
final ClassicHttpRequest request = rb.build();
+ assert request != null;
// do it
return doAPIRequest(request, wrapperTypeRef).getResponse();
@@ -106,4 +109,4 @@ public class DuoAuthAuthenticator extends AbstractDuoAuthenticator {
}
}
-}
\ No newline at end of file
+}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthResponse.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthResponse.java
index eeee7a3d6..a94062fa2 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthResponse.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoAuthResponse.java
@@ -31,10 +31,10 @@ import com.fasterxml.jackson.annotation.JsonProperty;
public class DuoAuthResponse extends DuoAuthAPIResponse {
/** the status string. */
- @JsonProperty("status") @Nonnull private String status;
+ @JsonProperty("status") private String status;
/** the trusted device token string. */
- @JsonProperty("trusted_device_token") @Nullable private String trustedDeviceToken;
+ @JsonProperty("trusted_device_token") private String trustedDeviceToken;
/**
* Get the Duo status string.
@@ -42,6 +42,7 @@ public class DuoAuthResponse extends DuoAuthAPIResponse {
* @return Duo status string
*/
@Nonnull public String getStatus() {
+ assert status != null;
return status;
}
@@ -51,6 +52,7 @@ public class DuoAuthResponse extends DuoAuthAPIResponse {
* @return Duo trusted device token string
*/
@Nullable public String getTrustedDeviceToken() {
+ assert trustedDeviceToken != null;
return trustedDeviceToken;
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoDevice.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoDevice.java
index e3e23c1e9..ca6181a5f 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoDevice.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoDevice.java
@@ -59,7 +59,8 @@ public class DuoDevice {
*
* @return the device identifier
*/
- @Nullable public String getDevice() {
+ @Nonnull public String getDevice() {
+ assert device != null;
return device;
}
@@ -68,7 +69,8 @@ public class DuoDevice {
*
* @return the device type
*/
- @Nullable public String getType() {
+ @Nonnull public String getType() {
+ assert type != null;
return type;
}
@@ -77,7 +79,8 @@ public class DuoDevice {
*
* @return the device number
*/
- @Nullable public String getNumber() {
+ @Nonnull public String getNumber() {
+ assert number != null;
return number;
}
@@ -86,7 +89,8 @@ public class DuoDevice {
*
* @return the device name
*/
- @Nullable public String getName() {
+ @Nonnull public String getName() {
+ assert name != null;
return name;
}
@@ -96,6 +100,7 @@ public class DuoDevice {
* @return the device capabilities
*/
@Nonnull public Collection<String> getCapabilities() {
+ assert capabilities != null;
return capabilities;
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoPreauthResponse.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoPreauthResponse.java
index ce2943460..53cc918bb 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoPreauthResponse.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoPreauthResponse.java
@@ -18,6 +18,7 @@
package net.shibboleth.idp.authn.duo.impl;
import java.net.URL;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -34,7 +35,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
public class DuoPreauthResponse extends DuoAuthAPIResponse {
/** the {@link List} of {@link DuoDevice}s registered. */
- @JsonProperty("devices") @Nonnull private List<DuoDevice> devices;
+ @JsonProperty("devices") @Nonnull private List<DuoDevice> devices = new ArrayList<>();
/** the {@link URL} for the self-enrollment portal. */
@JsonProperty("enroll_portal_url") @Nullable private URL enrollPortalURL;
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoResponseWrapper.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoResponseWrapper.java
index 40de6c7f4..4aa0d9422 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoResponseWrapper.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoResponseWrapper.java
@@ -33,11 +33,11 @@ public class DuoResponseWrapper<T extends DuoAuthAPIResponse> {
/** the inner response. */
@JsonProperty("response")
- @Nonnull private T response;
+ private T response;
/** the response status. */
@JsonProperty("stat")
- @Nonnull private String stat;
+ private String stat;
/**
* Get the inner response.
@@ -45,6 +45,7 @@ public class DuoResponseWrapper<T extends DuoAuthAPIResponse> {
* @return inner response
*/
@Nonnull public T getResponse() {
+ assert response != null;
return response;
}
@@ -54,6 +55,7 @@ public class DuoResponseWrapper<T extends DuoAuthAPIResponse> {
* @return response status
*/
@Nonnull public String getStat() {
+ assert stat != null;
return stat;
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoSupport.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoSupport.java
index e08de4dc8..880768951 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoSupport.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/DuoSupport.java
@@ -109,6 +109,7 @@ public final class DuoSupport {
}
final String username = DuoWeb.verifyResponse(duo.getIntegrationKey(), duo.getSecretKey(),
duo.getApplicationKey(), signedResponseToken);
+ assert username != null;
return username;
} catch (final ArrayIndexOutOfBoundsException e) {
// This guard is to prevent an unusual issue being encountered by at least one deployer.
@@ -136,6 +137,7 @@ public final class DuoSupport {
final String skey = duo.getSecretKey();
final int sigVersion = 2;
final String date = RFC_2822_DATE_FORMAT.format(ZonedDateTime.now());
+ assert date != null;
final String canon = canonRequest(request, date, sigVersion);
final String sig = Util.hmacSign(skey, canon);
@@ -166,7 +168,9 @@ public final class DuoSupport {
canon += request.getMethod().toUpperCase() + "\n";
canon += uri.getHost().toLowerCase() + "\n";
canon += uri.getPath() + "\n";
- canon += createQueryString(request.getParameters());
+ final List<NameValuePair> parms = request.getParameters();
+ assert parms != null;
+ canon += createQueryString(parms);
return canon;
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoAuthAPI.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoAuthAPI.java
index c26b5391b..106716f32 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoAuthAPI.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoAuthAPI.java
@@ -21,6 +21,7 @@ package net.shibboleth.idp.authn.duo.impl;
import java.security.Principal;
import java.util.HashMap;
import java.util.Map;
+import java.util.Set;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -46,6 +47,7 @@ import net.shibboleth.idp.authn.duo.context.DuoAuthenticationContext;
import net.shibboleth.idp.authn.impl.AbstractAuditingValidationAction;
import net.shibboleth.idp.profile.IdPAuditFields;
import net.shibboleth.idp.session.context.navigate.CanonicalUsernameLookupStrategy;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
@@ -91,13 +93,13 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
@Nonnull private Function<ProfileRequestContext,String> usernameLookupStrategy;
/** Implementation of Duo AuthApi /auth endpoint. */
- @Nonnull private DuoAuthAuthenticator authAuthenticator;
+ @NonnullAfterInit private DuoAuthAuthenticator authAuthenticator;
/** Implementation of Duo AuthApi /preauth enpoint. */
- @Nonnull private DuoPreauthAuthenticator preauthAuthenticator;
+ @NonnullAfterInit private DuoPreauthAuthenticator preauthAuthenticator;
- /** DuoApi context for tokens. **/
- @Nonnull @NotEmpty private DuoAuthenticationContext duoContext;
+ /** DuoApi context for tokens. Non-Null after preExecute **/
+ @Nullable @NotEmpty private DuoAuthenticationContext duoContext;
/** Duo integration to use. */
@Nullable private DuoIntegration duoIntegration;
@@ -175,6 +177,7 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
if (preauthAuthenticator == null) {
throw new ComponentInitializationException("DuoPreauthAuthenticator cannot be null");
}
+
}
/** {@inheritDoc} */
@@ -199,14 +202,14 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
return false;
}
- duoContext = authenticationContext.getSubcontext(DuoAuthenticationContext.class);
- if (duoContext == null) {
+ final DuoAuthenticationContext context = duoContext = authenticationContext.getSubcontext(DuoAuthenticationContext.class);
+ if (context == null) {
log.info("{} No DuoAuthenticationContext available", getLogPrefix());
handleError(profileRequestContext, authenticationContext, "No DuoAuthenticationContext context available",
AuthnEventIds.INVALID_AUTHN_CTX);
recordFailure(profileRequestContext);
return false;
- } else if (duoContext.getFactor() == null) {
+ } else if (context.getFactor() == null) {
log.info("{} No factor set in DuoAuthenticationContext", getLogPrefix());
handleError(profileRequestContext, authenticationContext, "No Duo factor set in DuoAuthenticationContext",
AuthnEventIds.REQUEST_UNSUPPORTED);
@@ -214,7 +217,7 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
return false;
}
- duoContext.setUsername(username);
+ context.setUsername(username);
return true;
}
@@ -225,10 +228,13 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
@Nonnull final AuthenticationContext authenticationContext) {
log.trace("{} Attempting Duo AuthAPI authentication", getLogPrefix());
-
+ final DuoAuthenticationContext duoCtx = duoContext;
+ assert duoCtx != null;
+
try {
// Duo AuthAPI pre-authentication
- final DuoPreauthResponse preAuthResponse = preauthAuthenticator.authenticate(duoContext, duoIntegration);
+ assert duoIntegration != null;
+ final DuoPreauthResponse preAuthResponse = preauthAuthenticator.authenticate(duoCtx, duoIntegration);
if (preAuthResponse == null) {
log.info("{} No Duo AuthAPI preauthentication response", getLogPrefix());
throw new DuoWebException("No preauthentication response");
@@ -256,23 +262,25 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
}
// Validate device ID specified against the enrolled set.
- if (duoContext.getDeviceID() != null && !DuoAuthAPI.DUO_DEVICE_AUTO.equals(duoContext.getDeviceID())) {
+ if (duoCtx.getDeviceID() != null && !DuoAuthAPI.DUO_DEVICE_AUTO.equals(duoCtx.getDeviceID())) {
boolean found = false;
for (final DuoDevice device : preAuthResponse.getDevices()) {
- if (duoContext.getDeviceID().equals(device.getDevice())) {
+ final String deviceId = duoCtx.getDeviceID();
+ assert deviceId != null;
+ if (deviceId.equals(device.getDevice())) {
found = true;
break;
- } else if (duoContext.getDeviceID().equals(device.getName())) {
+ } else if (deviceId.equals(device.getName())) {
log.debug("{} Remapped device ID based on device name ({}) for '{}'", getLogPrefix(),
device.getName(), username);
- duoContext.setDeviceID(device.getDevice());
+ duoCtx.setDeviceID(device.getDevice());
found = true;
break;
}
}
if (!found) {
log.info("{} Duo authentication failed for '{}': non-existent device ID ({})", getLogPrefix(),
- username, duoContext.getDeviceID());
+ username, duoCtx.getDeviceID());
handleError(profileRequestContext, authenticationContext, AuthnEventIds.INVALID_CREDENTIALS,
AuthnEventIds.INVALID_CREDENTIALS);
recordFailure(profileRequestContext);
@@ -281,7 +289,8 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
}
// Duo AuthAPI authentication
- final DuoAuthResponse authenticationResponse = authAuthenticator.authenticate(duoContext, duoIntegration);
+ assert duoIntegration != null;
+ final DuoAuthResponse authenticationResponse = authAuthenticator.authenticate(duoCtx, duoIntegration);
if (authenticationResponse == null) {
log.info("{} No Duo AuthAPI authentication response", getLogPrefix());
throw new DuoWebException("No authentication response");
@@ -290,7 +299,7 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
final String authResult = authenticationResponse.getResult();
if (DuoAuthAPI.DUO_AUTH_RESULT_ALLOW.equals(authResult)) {
log.info("{} Duo authentication succeeded for '{}' (Factor: {}, Device: {})", getLogPrefix(), username,
- duoContext.getFactor(), duoContext.getDeviceID());
+ duoCtx.getFactor(), duoCtx.getDeviceID());
recordSuccess(profileRequestContext);
buildAuthenticationResult(profileRequestContext, authenticationContext);
} else if (DuoAuthAPI.DUO_AUTH_RESULT_DENY.equals(authResult)) {
@@ -310,9 +319,13 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
// CheckStyle: CyclomaticComplexity|MethodLength|ReturnCount OFF
/** {@inheritDoc} */
- @Override protected Subject populateSubject(@Nonnull final Subject subject) {
- subject.getPrincipals().add(new DuoPrincipal(username));
- subject.getPrincipals().addAll(duoIntegration.getSupportedPrincipals(Principal.class));
+ @Override protected @Nonnull Subject populateSubject(@Nonnull final Subject subject) {
+ assert username != null;
+ final DuoPrincipal princ = new DuoPrincipal(username);
+ subject.getPrincipals().add(princ);
+ assert duoIntegration != null;
+ final Set<Principal> princs = duoIntegration.getSupportedPrincipals(Principal.class);
+ subject.getPrincipals().addAll(princs);
return subject;
}
@@ -322,7 +335,7 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
super.buildAuthenticationResult(profileRequestContext, authenticationContext);
// Bypass c14n. We already operate on a canonical name, so just re-confirm it.
- profileRequestContext.getSubcontext(SubjectCanonicalizationContext.class, true).setPrincipalName(username);
+ profileRequestContext.getOrCreateSubcontext(SubjectCanonicalizationContext.class).setPrincipalName(username);
}
/** {@inheritDoc} */
@@ -337,13 +350,13 @@ public class ValidateDuoAuthAPI extends AbstractAuditingValidationAction {
if (duoIntegration != null) {
fields.put(AuthnAuditFields.DUO_CLIENT_ID, duoIntegration.getIntegrationKey());
}
-
- if (duoContext != null) {
- if (duoContext.getDeviceID() != null) {
- fields.put(AuthnAuditFields.DUO_DEVICE_ID, duoContext.getDeviceID());
+ final DuoAuthenticationContext duoCtx = duoContext;
+ if (duoCtx != null) {
+ if (duoCtx.getDeviceID() != null) {
+ fields.put(AuthnAuditFields.DUO_DEVICE_ID, duoCtx.getDeviceID());
}
- if (duoContext.getFactor() != null) {
- fields.put(AuthnAuditFields.DUO_FACTOR, duoContext.getFactor());
+ if (duoCtx.getFactor() != null) {
+ fields.put(AuthnAuditFields.DUO_FACTOR, duoCtx.getFactor());
}
}
diff --git a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoWebResponse.java b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoWebResponse.java
index 3fc055df9..81de852af 100644
--- a/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoWebResponse.java
+++ b/idp-authn-impl/src/main/java/net/shibboleth/idp/authn/duo/impl/ValidateDuoWebResponse.java
@@ -23,6 +23,7 @@ import java.security.NoSuchAlgorithmException;
import java.security.Principal;
import java.util.Collections;
import java.util.Map;
+import java.util.Set;
import java.util.function.Function;
import javax.annotation.Nonnull;
@@ -165,8 +166,8 @@ public class ValidateDuoWebResponse extends AbstractAuditingValidationAction {
return false;
}
- signedResponse = servletRequest.getParameter(RESPONSE_PARAM);
- if (signedResponse == null || signedResponse.isEmpty()) {
+ final String response = signedResponse = servletRequest.getParameter(RESPONSE_PARAM);
+ if (response == null || response.isEmpty()) {
log.warn("{} No signed Duo response in the request", getLogPrefix());
handleError(profileRequestContext, authenticationContext, AuthnEventIds.NO_CREDENTIALS,
AuthnEventIds.NO_CREDENTIALS);
@@ -186,6 +187,7 @@ public class ValidateDuoWebResponse extends AbstractAuditingValidationAction {
final String usernameFromDuo;
try {
+ assert duoIntegration != null && signedResponse != null;
usernameFromDuo = DuoSupport.validateSignedResponseToken(duoIntegration, signedResponse);
} catch (final InvalidKeyException | NoSuchAlgorithmException | DuoWebException | IOException e) {
log.warn("{} Error validating signed Duo response for username '{}'", getLogPrefix(), username, e);
@@ -193,7 +195,7 @@ public class ValidateDuoWebResponse extends AbstractAuditingValidationAction {
recordFailure(profileRequestContext);
return;
}
-
+ assert username != null;
if (!username.equals(usernameFromDuo)) {
log.warn("{} Username '{}' from Duo response does not match previously established username '{}'",
getLogPrefix(), usernameFromDuo, username);
@@ -209,9 +211,15 @@ public class ValidateDuoWebResponse extends AbstractAuditingValidationAction {
/** {@inheritDoc} */
@Override
- protected Subject populateSubject(@Nonnull final Subject subject) {
- subject.getPrincipals().add(new DuoPrincipal(username));
- subject.getPrincipals().addAll(duoIntegration.getSupportedPrincipals(Principal.class));
+ protected @Nonnull Subject populateSubject(@Nonnull final Subject subject) {
+
+ assert username != null;
+ final DuoPrincipal princ = new DuoPrincipal(username);
+ subject.getPrincipals().add(princ);
+ assert duoIntegration != null;
+ final Set<Principal> princs = duoIntegration.getSupportedPrincipals(Principal.class);
+ subject.getPrincipals().addAll(princs);
+
return subject;
}
@@ -222,7 +230,7 @@ public class ValidateDuoWebResponse extends AbstractAuditingValidationAction {
super.buildAuthenticationResult(profileRequestContext, authenticationContext);
// Bypass c14n. We already operate on a canonical name, so just re-confirm it.
- profileRequestContext.getSubcontext(SubjectCanonicalizationContext.class, true).setPrincipalName(username);
+ profileRequestContext.getOrCreateSubcontext(SubjectCanonicalizationContext.class).setPrincipalName(username);
}
/** {@inheritDoc} */
diff --git a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/duo/impl/ExtractDuoAuthenticationFromHeadersTest.java b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/duo/impl/ExtractDuoAuthenticationFromHeadersTest.java
index a04928a3e..6b7dfc2b9 100644
--- a/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/duo/impl/ExtractDuoAuthenticationFromHeadersTest.java
+++ b/idp-authn-impl/src/test/java/net/shibboleth/idp/authn/duo/impl/ExtractDuoAuthenticationFromHeadersTest.java
@@ -24,6 +24,7 @@ import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import jakarta.servlet.http.HttpServletRequest;
import net.shibboleth.idp.authn.AuthnEventIds;
import net.shibboleth.idp.authn.context.AuthenticationContext;
import net.shibboleth.idp.authn.duo.DuoAuthAPI;
@@ -56,6 +57,7 @@ public class ExtractDuoAuthenticationFromHeadersTest extends BaseAuthenticationC
ActionTestingSupport.assertEvent(event, AuthnEventIds.NO_CREDENTIALS);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
Assert.assertNull(duoCtx);
}
@@ -71,6 +73,7 @@ public class ExtractDuoAuthenticationFromHeadersTest extends BaseAuthenticationC
ActionTestingSupport.assertEvent(event, AuthnEventIds.NO_CREDENTIALS);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
Assert.assertNull(duoCtx);
}
@@ -83,62 +86,79 @@ public class ExtractDuoAuthenticationFromHeadersTest extends BaseAuthenticationC
ActionTestingSupport.assertProceedEvent(event);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
- Assert.assertNotNull(duoCtx);
+ assert duoCtx != null;
Assert.assertEquals(duoCtx.getFactor(), DuoAuthAPI.DUO_FACTOR_AUTO);
Assert.assertEquals(duoCtx.getDeviceID(), DuoAuthAPI.DUO_DEVICE_AUTO);
Assert.assertNull(duoCtx.getPasscode());
}
@Test public void testFactorAutoDevice() {
- ((MockHttpServletRequest) action.getHttpServletRequest()).addHeader(DuoAuthAPI.DUO_FACTOR_HEADER_NAME, DuoAuthAPI.DUO_FACTOR_PUSH);
+ assert action != null;
+ final MockHttpServletRequest request = ((MockHttpServletRequest) action.getHttpServletRequest());
+ assert request != null;
+ request.addHeader(DuoAuthAPI.DUO_FACTOR_HEADER_NAME, DuoAuthAPI.DUO_FACTOR_PUSH);
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
- Assert.assertNotNull(duoCtx);
+ assert duoCtx != null;
Assert.assertEquals(duoCtx.getFactor(), DuoAuthAPI.DUO_FACTOR_PUSH);
Assert.assertEquals(duoCtx.getDeviceID(), DuoAuthAPI.DUO_DEVICE_AUTO);
Assert.assertNull(duoCtx.getPasscode());
}
@Test public void testDeviceAutoFactor() {
- ((MockHttpServletRequest) action.getHttpServletRequest()).addHeader(DuoAuthAPI.DUO_DEVICE_HEADER_NAME, "foo");
+ assert action != null;
+ final MockHttpServletRequest request = ((MockHttpServletRequest) action.getHttpServletRequest());
+ assert request != null;
+ request.addHeader(DuoAuthAPI.DUO_DEVICE_HEADER_NAME, "foo");
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
- Assert.assertNotNull(duoCtx);
+ assert duoCtx != null;
Assert.assertEquals(duoCtx.getFactor(), DuoAuthAPI.DUO_FACTOR_AUTO);
Assert.assertEquals(duoCtx.getDeviceID(), "foo");
Assert.assertNull(duoCtx.getPasscode());
}
@Test public void testNoPasscode() {
- ((MockHttpServletRequest) action.getHttpServletRequest()).addHeader(DuoAuthAPI.DUO_FACTOR_HEADER_NAME, DuoAuthAPI.DUO_FACTOR_PASSCODE);
+ assert action != null;
+ final MockHttpServletRequest request = ((MockHttpServletRequest) action.getHttpServletRequest());
+ assert request != null;
+ request.addHeader(DuoAuthAPI.DUO_FACTOR_HEADER_NAME, DuoAuthAPI.DUO_FACTOR_PASSCODE);
final Event event = action.execute(src);
ActionTestingSupport.assertEvent(event, AuthnEventIds.NO_CREDENTIALS);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
- Assert.assertNull(duoCtx);
+ assert duoCtx == null;
}
@Test public void testPasscode() {
- ((MockHttpServletRequest) action.getHttpServletRequest()).addHeader(DuoAuthAPI.DUO_FACTOR_HEADER_NAME, DuoAuthAPI.DUO_FACTOR_PASSCODE);
- ((MockHttpServletRequest) action.getHttpServletRequest()).addHeader(DuoAuthAPI.DUO_PASSCODE_HEADER_NAME, "foo");
+ assert action != null;
+ final MockHttpServletRequest request = ((MockHttpServletRequest) action.getHttpServletRequest());
+ assert request != null;
+ request.addHeader(DuoAuthAPI.DUO_FACTOR_HEADER_NAME, DuoAuthAPI.DUO_FACTOR_PASSCODE);
+ request.addHeader(DuoAuthAPI.DUO_PASSCODE_HEADER_NAME, "foo");
final Event event = action.execute(src);
ActionTestingSupport.assertProceedEvent(event);
final AuthenticationContext authCtx = prc.getSubcontext(AuthenticationContext.class, false);
+ assert authCtx != null;
final DuoAuthenticationContext duoCtx = authCtx.getSubcontext(DuoAuthenticationContext.class, false);
- Assert.assertNotNull(duoCtx);
+ assert duoCtx != null;
Assert.assertEquals(duoCtx.getFactor(), DuoAuthAPI.DUO_FACTOR_PASSCODE);
Assert.assertNull(duoCtx.getDeviceID());
Assert.assertEquals(duoCtx.getPasscode(), "foo");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list