From abe24df980735b5cddc0f7818834b75e5e3e2508 Mon Sep 17 00:00:00 2001 From: Jared Anderson Date: Fri, 20 Sep 2024 10:43:31 -0500 Subject: [PATCH] Fix CWWWC0002W warning message coming out when using mpGraphQL features - When using mpGraphQL features with appSecurity features and the application is not a GraphQL application, the CWWWC0002W message came out because the security logic for mpGraphQL unconditionally was adding a filter even when the application was not a GraphQL application. - Updated to only add the filter if the application is determined to be a GraphQL application. - Updated test cases that were ignoring the CWWWC0002W message on server stop to no longer have that ignore. --- .../component/GraphQLSecurityInitializer.java | 16 +++++ .../GraphQLServletContainerInitializer.java | 71 ++++++++++++------- .../graphql/fat/VoidQueryTest.java | 2 +- ...a => GraphQLAuthorizationInitializer.java} | 20 +++--- .../internal/tests/JakartaEE10Test.java | 2 - .../internal/tests/JakartaEE11Test.java | 2 - .../internal/tests/JakartaEE9Test.java | 2 - .../internal/test/suite/MPCompatibleTest.java | 14 ++-- .../test/suite/MP7EE10CompatibleTest.java | 4 +- .../test/suite/MP7EE11CompatibleTest.java | 4 +- 10 files changed, 78 insertions(+), 59 deletions(-) create mode 100644 dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLSecurityInitializer.java rename dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/{GraphQLAuthorizationServletContainerInitializer.java => GraphQLAuthorizationInitializer.java} (72%) diff --git a/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLSecurityInitializer.java b/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLSecurityInitializer.java new file mode 100644 index 00000000000..27a5dfb0a8d --- /dev/null +++ b/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLSecurityInitializer.java @@ -0,0 +1,16 @@ +/******************************************************************************* + * Copyright (c) 2024 IBM Corporation and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package com.ibm.ws.io.smallrye.graphql.component; + +import javax.servlet.ServletContext; + +public interface GraphQLSecurityInitializer { + public void onStartup(ServletContext ctx); +} diff --git a/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLServletContainerInitializer.java b/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLServletContainerInitializer.java index 8751bc92327..294af8edc7b 100644 --- a/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLServletContainerInitializer.java +++ b/dev/com.ibm.ws.io.smallrye.graphql/src/com/ibm/ws/io/smallrye/graphql/component/GraphQLServletContainerInitializer.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2020 IBM Corporation and others. + * Copyright (c) 2020, 2024 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at @@ -31,6 +31,14 @@ import javax.servlet.ServletException; import javax.servlet.ServletRegistration; +import org.eclipse.microprofile.graphql.ConfigKey; +import org.jboss.jandex.IndexView; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.osgi.service.component.annotations.ReferencePolicy; +import org.osgi.service.component.annotations.ReferencePolicyOption; + import com.ibm.websphere.ras.Tr; import com.ibm.websphere.ras.TraceComponent; import com.ibm.ws.container.service.app.deploy.ModuleInfo; @@ -41,7 +49,6 @@ import com.ibm.wsspi.logging.Introspector; import graphql.schema.GraphQLSchema; - import io.smallrye.graphql.bootstrap.Bootstrap; import io.smallrye.graphql.cdi.config.GraphQLConfig; import io.smallrye.graphql.execution.ExecutionService; @@ -52,19 +59,27 @@ import io.smallrye.graphql.servlet.IndexInitializer; import io.smallrye.graphql.servlet.SchemaServlet; -import org.eclipse.microprofile.graphql.ConfigKey; -import org.osgi.service.component.annotations.Component; -import org.jboss.jandex.IndexView; - @Component(property = { "service.vendor=IBM" }) public class GraphQLServletContainerInitializer implements ServletContainerInitializer, Introspector { private static final TraceComponent tc = Tr.register(GraphQLServletContainerInitializer.class); private static Map diagnostics = new WeakHashMap(); + private GraphQLSecurityInitializer securityInitializer; public static final String EXECUTION_SERVLET_NAME = "ExecutionServlet"; public static final String SCHEMA_SERVLET_NAME = "SchemaServlet"; public static final String UI_SERVLET_NAME = "UIServlet"; + @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC, policyOption = ReferencePolicyOption.GREEDY) + public void setSecurityInitializer(GraphQLSecurityInitializer secInitializer) { + securityInitializer = secInitializer; + } + + public void unsetSecurityInitializer(GraphQLSecurityInitializer secInitializer) { + if (securityInitializer == secInitializer) { + securityInitializer = null; + } + } + @FFDCIgnore({Throwable.class}) public void onStartup(Set> classes, ServletContext ctx) throws ServletException { if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) { @@ -118,6 +133,25 @@ public void onStartup(Set> classes, ServletContext ctx) throws ServletE } diagBag.webinfClassesUrl = webinfClassesUrl; + + GraphQLSchema graphQLSchema = null; + try { + IndexInitializer indexInitializer = new IndexInitializer(); + IndexView index = indexInitializer.createIndex(Collections.singleton(webinfClassesUrl)); + Schema schema = SchemaBuilder.build(index); + if (schema == null || (!schema.hasQueries() && !schema.hasMutations())) { + // not a GraphQL app as far as we can tell - trace and exit: + if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) { + Tr.debug(tc, "No GraphQL components found in app: " + ctx.getServletContextName()); + } + diagnostics.remove(ctx.getClassLoader()); + return; + } + diagBag.modelSchema = schema; + } catch (Throwable t) { + Tr.error(tc, "ERROR_GENERATING_SCHEMA_CWMGQ0001E", ctx.getServletContextName()); + throw new ServletException(t); + } GraphQLConfig config = new GraphQLConfig() { @Override public String getDefaultErrorMessage() { @@ -178,25 +212,8 @@ public Optional> getUnwrapExceptions() { }; diagBag.config = config; - GraphQLSchema graphQLSchema = null; - try { - IndexInitializer indexInitializer = new IndexInitializer(); - IndexView index = indexInitializer.createIndex(Collections.singleton(webinfClassesUrl)); - Schema schema = SchemaBuilder.build(index); - if (schema == null || (!schema.hasQueries() && !schema.hasMutations())) { - // not a GraphQL app as far as we can tell - trace and exit: - if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) { - Tr.debug(tc, "No GraphQL components found in app: " + ctx.getServletContextName()); - } - diagnostics.remove(ctx.getClassLoader()); - return; - } - diagBag.modelSchema = schema; - graphQLSchema = Bootstrap.bootstrap(schema, config); - } catch (Throwable t) { - Tr.error(tc, "ERROR_GENERATING_SCHEMA_CWMGQ0001E", ctx.getServletContextName()); - throw new ServletException(t); - } + graphQLSchema = Bootstrap.bootstrap(diagBag.modelSchema, config); + diagBag.graphQLSchema = graphQLSchema; if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) { @@ -218,6 +235,10 @@ public Optional> getUnwrapExceptions() { ServletRegistration.Dynamic schemaServletReg = ctx.addServlet(SCHEMA_SERVLET_NAME, new SchemaServlet(printer)); schemaServletReg.addMapping(path + "/schema.graphql"); + if (securityInitializer != null) { + securityInitializer.onStartup(ctx); + } + boolean enableGraphQLUIServlet = ConfigFacade.getOptionalValue("io.openliberty.enableGraphQLUI", boolean.class) .orElse(false); if (enableGraphQLUIServlet) { diff --git a/dev/com.ibm.ws.microprofile.graphql.1.0_fat/fat/src/com/ibm/ws/microprofile/graphql/fat/VoidQueryTest.java b/dev/com.ibm.ws.microprofile.graphql.1.0_fat/fat/src/com/ibm/ws/microprofile/graphql/fat/VoidQueryTest.java index fa260182597..90da7eda19d 100644 --- a/dev/com.ibm.ws.microprofile.graphql.1.0_fat/fat/src/com/ibm/ws/microprofile/graphql/fat/VoidQueryTest.java +++ b/dev/com.ibm.ws.microprofile.graphql.1.0_fat/fat/src/com/ibm/ws/microprofile/graphql/fat/VoidQueryTest.java @@ -59,6 +59,6 @@ public void checkForStartFailureMessage() throws Exception { @AfterClass public static void afterClass() throws Exception { - server.stopServer("CWWWC0001W", "SRVE0190E", "CWMGQ0001E", "CWWWC0002W"); + server.stopServer("CWWWC0001W", "SRVE0190E", "CWMGQ0001E"); } } diff --git a/dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/GraphQLAuthorizationServletContainerInitializer.java b/dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/GraphQLAuthorizationInitializer.java similarity index 72% rename from dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/GraphQLAuthorizationServletContainerInitializer.java rename to dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/GraphQLAuthorizationInitializer.java index a4982d22a99..9692439a475 100644 --- a/dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/GraphQLAuthorizationServletContainerInitializer.java +++ b/dev/com.ibm.ws.microprofile.graphql.authorization/src/com/ibm/ws/microprofile/graphql/authorization/component/GraphQLAuthorizationInitializer.java @@ -13,35 +13,33 @@ package com.ibm.ws.microprofile.graphql.authorization.component; import java.util.EnumSet; -import java.util.Set; import javax.servlet.DispatcherType; import javax.servlet.FilterRegistration; -import javax.servlet.ServletContainerInitializer; import javax.servlet.ServletContext; -import javax.servlet.ServletException; + +import org.osgi.service.component.annotations.Component; import com.ibm.websphere.ras.Tr; import com.ibm.websphere.ras.TraceComponent; +import com.ibm.ws.io.smallrye.graphql.component.GraphQLSecurityInitializer; import com.ibm.ws.io.smallrye.graphql.component.GraphQLServletContainerInitializer; -import org.osgi.service.component.annotations.Component; - @Component(property = { "service.vendor=IBM" }) -public class GraphQLAuthorizationServletContainerInitializer implements ServletContainerInitializer { - private static final TraceComponent tc = Tr.register(GraphQLAuthorizationServletContainerInitializer.class); +public class GraphQLAuthorizationInitializer implements GraphQLSecurityInitializer { + private static final TraceComponent tc = Tr.register(GraphQLAuthorizationInitializer.class); private static final String AUTH_FILTER_NAME = "AuthorizationFilter"; - public void onStartup(Set> classes, ServletContext ctx) throws ServletException { - if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) { - Tr.entry(tc, "onStartup", new Object[] {classes, ctx, ctx.getServletContextName(), ctx.getContextPath()}); + public void onStartup(ServletContext ctx) { + if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) { + Tr.entry(tc, "onStartup", new Object[] {ctx, ctx.getServletContextName(), ctx.getContextPath()}); } // add servlet filter for ExecutionServlet FilterRegistration.Dynamic filterReg = ctx.addFilter(AUTH_FILTER_NAME, AuthorizationFilter.getInstance()); filterReg.addMappingForServletNames(EnumSet.of(DispatcherType.REQUEST), true, GraphQLServletContainerInitializer.EXECUTION_SERVLET_NAME); - if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) { + if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) { Tr.exit(tc, "onStartup"); } } diff --git a/dev/io.openliberty.jakartaee10.internal_fat/fat/src/io/openliberty/jakartaee10/internal/tests/JakartaEE10Test.java b/dev/io.openliberty.jakartaee10.internal_fat/fat/src/io/openliberty/jakartaee10/internal/tests/JakartaEE10Test.java index 851b7821427..854106fb2b7 100644 --- a/dev/io.openliberty.jakartaee10.internal_fat/fat/src/io/openliberty/jakartaee10/internal/tests/JakartaEE10Test.java +++ b/dev/io.openliberty.jakartaee10.internal_fat/fat/src/io/openliberty/jakartaee10/internal/tests/JakartaEE10Test.java @@ -156,7 +156,6 @@ public static void tearDown() throws Exception { if (RepeatTestFilter.isRepeatActionActive(COMPAT_OL_FEATURES)) { toleratedWarnErrors = new String[] { "SRVE0280E", // TODO: SRVE0280E tracked by OpenLiberty issue #4857 "CWWKS5207W", // The remaining ones relate to config not done for the server / app - "CWWWC0002W", "CWMOT0010W", "TRAS4352W" // Only happens when running with WebSphere Liberty image due to an auto feature }; @@ -164,7 +163,6 @@ public static void tearDown() throws Exception { } else if (RepeatTestFilter.isRepeatActionActive(COMPAT_WL_FEATURES)) { toleratedWarnErrors = new String[] { "SRVE0280E", // TODO: SRVE0280E tracked by OpenLiberty issue #4857 "CWWKS5207W", // The remaining ones relate to config not done for the server / app - "CWWWC0002W", "CWMOT0010W", "CWWKG0033W", // related to missing config for collectives "CWSJY0035E", // wmqJmsClient.rar.location variable not in the server.xml diff --git a/dev/io.openliberty.jakartaee11.internal_fat/fat/src/io/openliberty/jakartaee11/internal/tests/JakartaEE11Test.java b/dev/io.openliberty.jakartaee11.internal_fat/fat/src/io/openliberty/jakartaee11/internal/tests/JakartaEE11Test.java index 7fcf97478ca..303e7bbd004 100644 --- a/dev/io.openliberty.jakartaee11.internal_fat/fat/src/io/openliberty/jakartaee11/internal/tests/JakartaEE11Test.java +++ b/dev/io.openliberty.jakartaee11.internal_fat/fat/src/io/openliberty/jakartaee11/internal/tests/JakartaEE11Test.java @@ -128,7 +128,6 @@ public static void tearDown() throws Exception { if (RepeatTestFilter.isRepeatActionActive(COMPAT_OL_FEATURES)) { toleratedWarnErrors = new String[] { "SRVE0280E", // TODO: SRVE0280E tracked by OpenLiberty issue #4857 "CWWKS5207W", // The remaining ones relate to config not done for the server / app - "CWWWC0002W", "CWMOT0010W", "CWWKE0701E", // TODO: Fix this or verify that it is expected "TRAS4352W" // Only happens when running with WebSphere Liberty image due to an auto feature @@ -137,7 +136,6 @@ public static void tearDown() throws Exception { } else if (RepeatTestFilter.isRepeatActionActive(COMPAT_WL_FEATURES)) { toleratedWarnErrors = new String[] { "SRVE0280E", // TODO: SRVE0280E tracked by OpenLiberty issue #4857 "CWWKS5207W", // The remaining ones relate to config not done for the server / app - "CWWWC0002W", "CWMOT0010W", "CWWKE0701E", // TODO: Fix this or verify that it is expected "CWWKG0033W", // related to missing config for collectives diff --git a/dev/io.openliberty.jakartaee9.internal_fat/fat/src/io/openliberty/jakartaee9/internal/tests/JakartaEE9Test.java b/dev/io.openliberty.jakartaee9.internal_fat/fat/src/io/openliberty/jakartaee9/internal/tests/JakartaEE9Test.java index c3a596bdbf8..c5233fed646 100644 --- a/dev/io.openliberty.jakartaee9.internal_fat/fat/src/io/openliberty/jakartaee9/internal/tests/JakartaEE9Test.java +++ b/dev/io.openliberty.jakartaee9.internal_fat/fat/src/io/openliberty/jakartaee9/internal/tests/JakartaEE9Test.java @@ -156,7 +156,6 @@ public static void tearDown() throws Exception { if (RepeatTestFilter.isRepeatActionActive(COMPAT_OL_FEATURES)) { toleratedWarnErrors = new String[] { "SRVE0280E", // TODO: SRVE0280E tracked by OpenLiberty issue #4857 "CWWKS5207W", // The remaining ones relate to config not done for the server / app - "CWWWC0002W", "CWMOT0010W", "TRAS4352W" // Only happens when running with WebSphere Liberty image due to an auto feature }; @@ -164,7 +163,6 @@ public static void tearDown() throws Exception { } else if (RepeatTestFilter.isRepeatActionActive(COMPAT_WL_FEATURES)) { toleratedWarnErrors = new String[] { "SRVE0280E", // TODO: SRVE0280E tracked by OpenLiberty issue #4857 "CWWKS5207W", // The remaining ones relate to config not done for the server / app - "CWWWC0002W", "CWMOT0010W", "CWWKG0033W", // related to missing config for collectives "CWSJY0035E", // wmqJmsClient.rar.location variable not in the server.xml diff --git a/dev/io.openliberty.microprofile.internal_fat/fat/src/io/openliberty/microprofile/internal/test/suite/MPCompatibleTest.java b/dev/io.openliberty.microprofile.internal_fat/fat/src/io/openliberty/microprofile/internal/test/suite/MPCompatibleTest.java index cb78c5903b8..86bf5e5fc05 100644 --- a/dev/io.openliberty.microprofile.internal_fat/fat/src/io/openliberty/microprofile/internal/test/suite/MPCompatibleTest.java +++ b/dev/io.openliberty.microprofile.internal_fat/fat/src/io/openliberty/microprofile/internal/test/suite/MPCompatibleTest.java @@ -1,10 +1,10 @@ /******************************************************************************* - * Copyright (c) 2020, 2021 IBM Corporation and others. + * Copyright (c) 2020, 2024 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-2.0/ - * + * * SPDX-License-Identifier: EPL-2.0 * * Contributors: @@ -272,10 +272,7 @@ public void testMP33plusStandalone() throws Exception { server.startServer(); runGetMethod(200, "/helloworld/helloworld", MESSAGE); } finally { - //I think CWWWC0002W is a configuration error and should not appear - //Should be removed by issue 15496 - server.stopServer("CWMOT0010W", //CWMOT0010W: OpenTracing cannot track JAX-RS requests because an OpentracingTracerFactory class was not provided or client libraries for tracing backend are not in the class path. - "CWWWC0002W");//CWWWC0002W: No servlet definition is found for the ExecutionServlet servlet name in the AuthorizationFilter filter mapping. + server.stopServer("CWMOT0010W"); //CWMOT0010W: OpenTracing cannot track JAX-RS requests because an OpentracingTracerFactory class was not provided or client libraries for tracing backend are not in the class path. } } @@ -299,10 +296,7 @@ public void testMP40plusStandalone() throws Exception { server.startServer(); runGetMethod(200, "/helloworld/helloworld", MESSAGE); } finally { - //I think CWWWC0002W is a configuration error and should not appear - //Should be removed by issue 15496 - server.stopServer("CWMOT0010W", //CWMOT0010W: OpenTracing cannot track JAX-RS requests because an OpentracingTracerFactory class was not provided or client libraries for tracing backend are not in the class path. - "CWWWC0002W");//CWWWC0002W: No servlet definition is found for the ExecutionServlet servlet name in the AuthorizationFilter filter mapping. + server.stopServer("CWMOT0010W"); //CWMOT0010W: OpenTracing cannot track JAX-RS requests because an OpentracingTracerFactory class was not provided or client libraries for tracing backend are not in the class path. } } diff --git a/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE10CompatibleTest.java b/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE10CompatibleTest.java index 626d97e3f01..814e5410f87 100644 --- a/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE10CompatibleTest.java +++ b/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE10CompatibleTest.java @@ -36,9 +36,7 @@ public static void setUp() throws Exception { @AfterClass public static void cleanUp() throws Exception { - //I think CWWWC0002W is a mpGraphQL-1.0 bug and should not appear - //See issue 15496 - MPCompatibilityTestUtils.cleanUp(server, "CWWWC0002W"); + MPCompatibilityTestUtils.cleanUp(server); } /** diff --git a/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE11CompatibleTest.java b/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE11CompatibleTest.java index efd64c0b89e..0aef4c63542 100644 --- a/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE11CompatibleTest.java +++ b/dev/io.openliberty.microprofile7.internal_fat/fat/src/io/openliberty/microprofile7/internal/test/suite/MP7EE11CompatibleTest.java @@ -38,9 +38,7 @@ public static void setUp() throws Exception { @AfterClass public static void cleanUp() throws Exception { - //I think CWWWC0002W is a mpGraphQL-1.0 bug and should not appear - //See issue 15496 - MPCompatibilityTestUtils.cleanUp(server, "CWWWC0002W"); + MPCompatibilityTestUtils.cleanUp(server); } /**