luigidemasi commented on code in PR #28237: URL: https://github.com/apache/camel/pull/28237#discussion_r4339083914
########## components/camel-undertow/src/test/java/org/apache/camel/component/undertow/spi/SecurityProviderWebSocketConsumerHandlersWindowTest.java: ########## @@ -0,0 +1,142 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.camel.component.undertow.spi; + +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.WebSocket; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import io.undertow.websockets.core.CloseMessage; +import org.apache.camel.CamelContext; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.component.undertow.UndertowBasicAuthHandler; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * A WebSocket path whose producer applies the allowed roles while its consumer, started later, adds its own handlers on + * the same endpoint URI: a connection accepted while only the producer served the path runs the allowed-roles check but + * not the consumer handlers, so once that consumer is up the connection is neither served by the route nor sent + * messages, while a connection that goes through the consumer handlers is served. + */ +class SecurityProviderWebSocketConsumerHandlersWindowTest extends AbstractSecurityProviderTest { + + private final AtomicInteger routeInvocations = new AtomicInteger(); + + @Override + protected CamelContext createCamelContext() throws Exception { + CamelContext context = super.createCamelContext(); + context.getRegistry().bind("wsHandlers", new UndertowBasicAuthHandler()); + return context; + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + // producer and consumer share the same endpoint URI: the producer keeps the shared handler + // registered and applies allowedRoles, the consumer adds the handlers on top + from("direct:feed") + .to("undertow:ws://localhost:{{port}}/guarded?allowedRoles=user&handlers=#wsHandlers&sendToAll=true"); + from("undertow:ws://localhost:{{port}}/guarded?allowedRoles=user&handlers=#wsHandlers&sendToAll=true") + .routeId("guarded").autoStartup(false) + .process(exchange -> routeInvocations.incrementAndGet()) + .to("mock:guarded"); + } + }; + } + + @Test + void connectionAcceptedBeforeAConsumerWithHandlersStartedIsNotServed() throws Exception { + securityConfiguration.setRoleToAssign("user"); + + // only the producer serves the path, so the handshake runs the allowed-roles check but not the consumer handlers + RecordingListener listener = new RecordingListener(); + WebSocket webSocket = connect("/guarded", null, listener); + + context.getRouteController().startRoute("guarded"); + + // the connection does not get what the producer sends + template.sendBody("direct:feed", "broadcast"); + // and its messages do not reach the route: it is closed instead + webSocket.sendText("hello", true).join(); + + assertEquals(CloseMessage.MSG_VIOLATES_POLICY, listener.closeCode.orTimeout(10, TimeUnit.SECONDS).join()); Review Comment: **P2 — Close existing peers when the consumer starts.** Sending `hello` before awaiting the close hides a missing lifecycle step. `setConsumer(...)` installs the new security settings without invalidating existing peers, and `send(...)` only filters those peers out. An already-established connection is closed by the consumer's rejection path only when it sends an application message. I reproduced this with a real WebSocket client: connect while only the producer serves the path, confirm receipt of a broadcast to ensure server-side registration has completed, start the consumer, then broadcast again without sending client data. Awaiting close code `1008` times out, consistently across three attempts. A receive-only client therefore remains connected but silently stops receiving messages, so reconnect-on-close logic never runs. This does not meet CAMEL-25917's requirement to close these connections when the consumer is registered, or the upgrade note's stated behavior. Please close existing peers that fail the updated path security check when registering the consumer, and add a regression test that awaits `1008` after startup without first sending a client application message. Keep the current inbound rejection test as separate coverage. _AI-generated review by Codex on behalf of @luigidemasi._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
