[java-identity-provider] 02/06: IDP-2069 Null handling
Rod Widdowson
rdw at steadingsoftware.com
Fri Feb 10 13:50:19 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=dc76584cea598b8f23c83cc254502bd9e14a9199
commit dc76584cea598b8f23c83cc254502bd9e14a9199
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Thu Feb 9 15:21:14 2023 +0000
IDP-2069 Null handling
https://shibboleth.atlassian.net/browse/IDP-2069
Clean up idp-admin-impl
---
...InitializeAdministrativeProfileContextTree.java | 38 +++++++++++++++-------
.../shibboleth/idp/admin/impl/OutputMetrics.java | 20 ++++++++----
.../idp/admin/impl/DoStorageOperationTest.java | 5 +++
...ializeAdministrativeProfileContextTreeTest.java | 16 ++++-----
4 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTree.java b/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTree.java
index 6ebc74705..f369621ff 100644
--- a/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTree.java
+++ b/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTree.java
@@ -33,8 +33,11 @@ import net.shibboleth.idp.profile.context.RelyingPartyContext;
import net.shibboleth.idp.ui.context.RelyingPartyUIContext;
import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.NonnullSupplier;
import net.shibboleth.shared.spring.util.SpringSupport;
+import jakarta.servlet.http.HttpServletRequest;
+
/**
* An action that processes settings from a supplied {@link AdministrativeFlowDescriptor} to prepare
* the profile context tree for subsequent use by an administrative profile flow.
@@ -91,14 +94,23 @@ public class InitializeAdministrativeProfileContextTree extends AbstractProfileA
return false;
}
- if (flowDescriptor == null) {
+ final AdministrativeFlowDescriptor descriptor = flowDescriptor;
+ if (descriptor == null) {
log.warn("{} Administrative profile '{}' not enabled", getLogPrefix(),
profileRequestContext.getProfileId());
ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_PROFILE_CONFIG);
return false;
- } else if (!flowDescriptor.getId().equals(profileRequestContext.getProfileId())) {
+ }
+ final String id = descriptor.getId();
+ if (id == null ||!id.equals(profileRequestContext.getProfileId())) {
log.warn("{} Profile ID '{}' doesn't match descriptor ID '{}", getLogPrefix(),
- profileRequestContext.getProfileId(), flowDescriptor.getId());
+ profileRequestContext.getProfileId(), id);
+ ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_PROFILE_CONFIG);
+ return false;
+ }
+ if (getHttpServletRequest() == null) {
+ log.warn("{} Profile ID '{}' No HttpRequestSupplier available", getLogPrefix(),
+ profileRequestContext.getProfileId());
ActionSupport.buildEvent(profileRequestContext, IdPEventIds.INVALID_PROFILE_CONFIG);
return false;
}
@@ -110,18 +122,22 @@ public class InitializeAdministrativeProfileContextTree extends AbstractProfileA
@Override
protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext) {
- profileRequestContext.setLoggingId(flowDescriptor.getLoggingId());
- profileRequestContext.setBrowserProfile(!flowDescriptor.isNonBrowserSupported(profileRequestContext));
+ final AdministrativeFlowDescriptor descriptor = flowDescriptor;
+ assert descriptor != null;
+ profileRequestContext.setLoggingId(descriptor.getLoggingId());
+ profileRequestContext.setBrowserProfile(!descriptor.isNonBrowserSupported(profileRequestContext));
final RelyingPartyContext rpCtx = new RelyingPartyContext();
profileRequestContext.addSubcontext(rpCtx, true);
- rpCtx.setRelyingPartyId(flowDescriptor.getId());
- rpCtx.setProfileConfig(flowDescriptor);
+ rpCtx.setRelyingPartyId(descriptor.getId());
+ rpCtx.setProfileConfig(descriptor);
- final RelyingPartyUIContext uiCtx = rpCtx.getSubcontext(RelyingPartyUIContext.class, true);
- uiCtx.setRPUInfo(flowDescriptor.getUIInfo());
- uiCtx.setBrowserLanguageRanges(SpringSupport.getLanguageRange(getHttpServletRequest()));
- uiCtx.setRequestSupplier(getHttpServletRequestSupplier());
+ final RelyingPartyUIContext uiCtx = rpCtx.getOrCreateSubcontext(RelyingPartyUIContext.class);
+ uiCtx.setRPUInfo(descriptor.getUIInfo());
+ final NonnullSupplier<HttpServletRequest> supplier = getHttpServletRequestSupplier();
+ assert supplier != null;
+ uiCtx.setBrowserLanguageRanges(SpringSupport.getLanguageRange(supplier.get()));
+ uiCtx.setRequestSupplier(supplier);
if (null != fallbackLanguages) {
uiCtx.setFallbackLanguages(fallbackLanguages);
diff --git a/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/OutputMetrics.java b/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/OutputMetrics.java
index 779b8f565..becf58fb3 100644
--- a/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/OutputMetrics.java
+++ b/idp-admin-impl/src/main/java/net/shibboleth/idp/admin/impl/OutputMetrics.java
@@ -22,7 +22,6 @@ import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
-import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -53,6 +52,7 @@ import net.shibboleth.idp.profile.context.SpringRequestContext;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NonnullElements;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -106,7 +106,7 @@ public class OutputMetrics extends AbstractProfileAction {
/** Constructor. */
public OutputMetrics() {
- metricFilterMap = Collections.emptyMap();
+ metricFilterMap = CollectionSupport.emptyMap();
dateTimeFormatter = DateTimeFormatter.ISO_INSTANT;
}
@@ -216,7 +216,7 @@ public class OutputMetrics extends AbstractProfileAction {
/** {@inheritDoc} */
@Override
- protected boolean doPreExecute(final ProfileRequestContext profileRequestContext) {
+ protected boolean doPreExecute(final @Nonnull ProfileRequestContext profileRequestContext) {
if (!super.doPreExecute(profileRequestContext)) {
return false;
@@ -240,12 +240,18 @@ public class OutputMetrics extends AbstractProfileAction {
ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
return false;
}
+ final HttpServletResponse response = getHttpServletResponse();
+ if (response == null) {
+ log.warn("{} No HttpServletResponse available", getLogPrefix());
+ ActionSupport.buildEvent(profileRequestContext, EventIds.INVALID_PROFILE_CTX);
+ return false;
+ }
metricId = (String) requestContext.getFlowScope().get(METRIC_ID);
if (metricId == null) {
log.warn("{} No {} flow variable found in request", getLogPrefix(), METRIC_ID);
try {
- getHttpServletResponse().sendError(HttpServletResponse.SC_NOT_FOUND);
+ response.sendError(HttpServletResponse.SC_NOT_FOUND);
} catch (final IOException e) {
ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
}
@@ -256,7 +262,7 @@ public class OutputMetrics extends AbstractProfileAction {
}
/** {@inheritDoc} */
- @Override protected void doExecute(final ProfileRequestContext profileRequestContext) {
+ @Override protected void doExecute(final @Nonnull ProfileRequestContext profileRequestContext) {
MetricFilter filter = ALL_METRICS.equals(metricId) ? MetricFilter.ALL : metricFilterMap.get(metricId);
if (filter == null) {
@@ -273,7 +279,7 @@ public class OutputMetrics extends AbstractProfileAction {
try {
final HttpServletResponse response = getHttpServletResponse();
-
+ assert response != null;
response.setHeader("Cache-Control", "must-revalidate,no-cache,no-store");
response.setStatus(HttpServletResponse.SC_OK);
if (allowedOrigin != null) {
@@ -331,7 +337,7 @@ public class OutputMetrics extends AbstractProfileAction {
/** {@inheritDoc} */
public boolean matches(final String name, final Metric metric) {
- return parentFilter.matches(name, metric) && metricFilter.matches(name, metric);
+ return parentFilter.matches(name, metric) && metricFilter != null && metricFilter.matches(name, metric);
}
}
diff --git a/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/DoStorageOperationTest.java b/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/DoStorageOperationTest.java
index 69b7d7f73..1cdd3e945 100644
--- a/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/DoStorageOperationTest.java
+++ b/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/DoStorageOperationTest.java
@@ -248,6 +248,7 @@ public class DoStorageOperationTest {
Assert.assertEquals(response.getStatus(), HttpServletResponse.SC_CREATED);
final StorageRecord<?> record = storageService.read(CONTEXT, KEY);
+ assert record != null;
Assert.assertEquals(record.getVersion(), 1);
Assert.assertEquals(record.getValue(), VALUE);
}
@@ -296,6 +297,7 @@ public class DoStorageOperationTest {
Assert.assertEquals(response.getStatus(), HttpServletResponse.SC_OK);
final StorageRecord<?> record = storageService.read(CONTEXT, KEY);
+ assert record != null;
Assert.assertEquals(record.getVersion(), 2);
Assert.assertEquals(record.getValue(), "changed");
}
@@ -320,6 +322,7 @@ public class DoStorageOperationTest {
Assert.assertEquals(response.getStatus(), HttpServletResponse.SC_CREATED);
final StorageRecord<?> record = storageService.read(CONTEXT, KEY);
+ assert record != null;
Assert.assertEquals(record.getVersion(), 1);
Assert.assertEquals(record.getValue(), VALUE);
}
@@ -346,6 +349,7 @@ public class DoStorageOperationTest {
Assert.assertEquals(response.getStatus(), HttpServletResponse.SC_OK);
final StorageRecord<?> record = storageService.read(CONTEXT, KEY);
+ assert record != null;
Assert.assertEquals(record.getVersion(), 2);
Assert.assertEquals(record.getValue(), "changed");
}
@@ -372,6 +376,7 @@ public class DoStorageOperationTest {
Assert.assertEquals(response.getStatus(), HttpServletResponse.SC_CONFLICT);
final StorageRecord<?> record = storageService.read(CONTEXT, KEY);
+ assert record != null;
Assert.assertEquals(record.getVersion(), 1);
Assert.assertEquals(record.getValue(), VALUE);
}
diff --git a/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTreeTest.java b/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTreeTest.java
index 085e88380..fbe19d229 100644
--- a/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTreeTest.java
+++ b/idp-admin-impl/src/test/java/net/shibboleth/idp/admin/impl/InitializeAdministrativeProfileContextTreeTest.java
@@ -17,8 +17,6 @@
package net.shibboleth.idp.admin.impl;
-import java.util.Collections;
-
import javax.annotation.Nonnull;
import org.opensaml.core.testing.OpenSAMLInitBaseTestCase;
@@ -29,7 +27,6 @@ import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import jakarta.servlet.http.HttpServletRequest;
import net.shibboleth.idp.admin.BasicAdministrativeFlowDescriptor;
import net.shibboleth.idp.profile.IdPEventIds;
import net.shibboleth.idp.profile.context.RelyingPartyContext;
@@ -37,10 +34,13 @@ import net.shibboleth.idp.profile.context.navigate.WebflowRequestContextProfileR
import net.shibboleth.idp.profile.testing.ActionTestingSupport;
import net.shibboleth.idp.profile.testing.RequestContextBuilder;
import net.shibboleth.idp.ui.context.RelyingPartyUIContext;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.primitive.LangBearingString;
import net.shibboleth.shared.primitive.NonnullSupplier;
+import jakarta.servlet.http.HttpServletRequest;
+
/** {@link InitializeAdministrativeProfileContextTree} unit test. */
@SuppressWarnings("javadoc")
public class InitializeAdministrativeProfileContextTreeTest extends OpenSAMLInitBaseTestCase {
@@ -62,9 +62,9 @@ public class InitializeAdministrativeProfileContextTreeTest extends OpenSAMLInit
descriptor = new BasicAdministrativeFlowDescriptor("foo");
descriptor.setLoggingId("log");
descriptor.setNonBrowserSupported(true);
- descriptor.setDisplayNames(Collections.singletonList(new LangBearingString("name", "en")));
- descriptor.setDescriptions(Collections.singletonList(new LangBearingString("description", "en")));
- descriptor.setLogos(Collections.singletonList(new BasicAdministrativeFlowDescriptor.Logo("http://logo", null, 10, 10)));
+ descriptor.setDisplayNames(CollectionSupport.singletonList(new LangBearingString("name", "en")));
+ descriptor.setDescriptions(CollectionSupport.singletonList(new LangBearingString("description", "en")));
+ descriptor.setLogos(CollectionSupport.singletonList(new BasicAdministrativeFlowDescriptor.Logo("http://logo", null, 10, 10)));
action = new InitializeAdministrativeProfileContextTree();
action.setAdministrativeFlowDescriptor(descriptor);
@@ -100,14 +100,14 @@ public class InitializeAdministrativeProfileContextTreeTest extends OpenSAMLInit
Assert.assertEquals(prc.getLoggingId(), "log");
final RelyingPartyContext rpc = prc.getSubcontext(RelyingPartyContext.class);
- Assert.assertNotNull(rpc);
+ assert rpc != null;
Assert.assertEquals(rpc.getRelyingPartyId(), "foo");
Assert.assertNotNull(rpc.getProfileConfig());
Assert.assertSame(rpc.getProfileConfig(), descriptor);
final RelyingPartyUIContext ui = rpc.getSubcontext(RelyingPartyUIContext.class);
- Assert.assertNotNull(ui);
+ assert ui != null;
Assert.assertEquals(ui.getServiceName(), "name");
Assert.assertEquals(ui.getServiceDescription(), "description");
Assert.assertEquals(ui.getLogo(), "http://logo");
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list