oscerd commented on code in PR #27435:
URL: https://github.com/apache/camel/pull/27435#discussion_r4194600877
##########
components/camel-undertow/src/main/java/org/apache/camel/component/undertow/handlers/CamelWebSocketHandler.java:
##########
@@ -142,37 +169,127 @@ private static void send(
*/
@Override
public void handleRequest(HttpServerExchange exchange) throws Exception {
- UndertowConsumer currentConsumer = getConsumer();
- if (currentConsumer != null &&
currentConsumer.getEndpoint().getOauthHttpSecurity() != null) {
- if (exchange.isInIoThread()) {
- exchange.dispatch(this);
- return;
+ entryHandler.handleRequest(exchange);
+ }
+
+ /**
+ * The handler that applies the security settings of the path to an
upgrade request and then performs the WebSocket
+ * handshake. A consumer can run its own handlers, such as its access log,
before it.
+ */
+ public HttpHandler getUpgradeHandler() {
+ return upgradeHandler;
+ }
+
+ /**
+ * Lets the given security provider wrap this handler, as it wraps the
handlers of HTTP endpoints. The handler stays
+ * registered as is, so that it remains shared by the consumer and the
producers of the path.
+ */
+ public void wrapWith(UndertowSecurityProvider securityProvider) throws
Exception {
+ HttpHandler wrapped =
securityProvider.wrapHttpHandler(consumerRequestHandler);
+ // a provider that returns no handler disables the path, as it does
for HTTP endpoints
+ this.entryHandler = wrapped != null ? wrapped :
ResponseCodeHandler.HANDLE_405;
+ }
+
+ private void handleConsumerRequest(HttpServerExchange exchange) throws
Exception {
+ consumerHandler.handleRequest(exchange);
+ }
+
+ private void upgrade(HttpServerExchange exchange) throws Exception {
+ List<UndertowEndpoint> endpoints = securityEndpoints();
+ if
(endpoints.stream().noneMatch(CamelWebSocketHandler::hasSecurityChecks)) {
+ this.delegate.handleRequest(exchange);
+ return;
+ }
+ if (exchange.isInIoThread()) {
+ exchange.dispatch(upgradeHandler);
+ return;
+ }
+ Set<String> authenticatedEndpoints = new HashSet<>();
+ Map<String, Object> headers = new HashMap<>();
+ for (UndertowEndpoint endpoint : endpoints) {
+ OAuthHttpSecuritySupport oauthHttpSecurity =
endpoint.getOauthHttpSecurity();
+ if (oauthHttpSecurity != null) {
+ Validation validation =
oauthHttpSecurity.validate(endpoint.getCamelContext(),
authorizationHeaders(exchange));
+ exchange.getRequestHeaders().remove(Headers.AUTHORIZATION);
+ if (!validation.isAuthenticated()) {
+ reject(exchange, validation);
+ return;
+ }
+
exchange.putAttachment(OAUTH_TOKEN_VALIDATION_RESULT_ATTACHMENT,
validation.getValidationResult());
}
- OAuthHttpSecuritySupport oauthHttpSecurity =
currentConsumer.getEndpoint().getOauthHttpSecurity();
- Validation validation =
oauthHttpSecurity.validate(currentConsumer.getEndpoint().getCamelContext(),
- authorizationHeaders(exchange));
- exchange.getRequestHeaders().remove(Headers.AUTHORIZATION);
- if (!validation.isAuthenticated()) {
- reject(exchange, validation);
- return;
+ if (endpoint.requiresAuthentication()) {
+ int statusCode = endpoint.authenticate(exchange);
+ if (statusCode != StatusCodes.OK) {
+ exchange.setStatusCode(statusCode);
+ exchange.endExchange();
+ return;
+ }
+ authenticatedEndpoints.add(endpoint.getEndpointUri());
+ if (endpoint.getSecurityProvider() != null) {
+ endpoint.getSecurityProvider().addHeader(headers::put,
exchange);
+ }
}
- exchange.putAttachment(OAUTH_TOKEN_VALIDATION_RESULT_ATTACHMENT,
validation.getValidationResult());
+ }
+ if (!authenticatedEndpoints.isEmpty()) {
+ exchange.putAttachment(SECURITY_PROVIDER_RESULT_ATTACHMENT,
+ new SecurityProviderResult(authenticatedEndpoints,
headers));
}
this.delegate.handleRequest(exchange);
}
- private UndertowConsumer getConsumer() {
+ /**
+ * The endpoints whose security settings apply to the path: the
consumer's, also while it is stopped, otherwise the
+ * producers'. All the Camel endpoints of a path share its WebSocket
connections.
+ */
+ private List<UndertowEndpoint> securityEndpoints() {
Review Comment:
Added. When a consumer is on the path, a producer that configures its own
`allowedRoles`, `securityConfiguration` or `securityProvider`, and whose
settings differ from the consumer's, now logs a WARN once per producer
endpoint, for example:
```
The security settings of
ws://localhost:8080/mixed?allowedRoles=admin&sendToAll=true are not used: the
settings of the consumer ws://localhost:8080/mixed apply to its WebSocket path
```
Producers that only inherit the settings of the component log nothing, so
the usual `from("undertow:ws:...").to("undertow:ws:...")` route stays quiet.
The decision is covered by `CamelWebSocketHandlerSecuritySettingsTest`.
_Claude Code on behalf of Andrea Cosentino (oscerd)_
##########
components/camel-undertow/src/main/java/org/apache/camel/component/undertow/UndertowComponent.java:
##########
@@ -372,7 +373,12 @@ public HttpHandler registerEndpoint(
handlers.add(registrationInfo);
HttpHandler handlerWrapped = handler;
- if (this.securityProvider != null) {
+ if (handler instanceof CamelWebSocketHandler webSocketHandler) {
+ // the WebSocket handler of a path is shared by its consumer and
producers, so it must stay registered as is
+ if (this.securityProvider != null) {
+ webSocketHandler.wrapWith(this.securityProvider);
Review Comment:
This is done before `registerHandler` on purpose. Which instance is kept is
only known once `registerHandler` returns, and by then the path is already
routable: wrapping the returned instance afterwards would leave a short window
in which the first upgrade requests on a new path do not go through the
provider's wrapper. Wrapping an instance that ends up unused costs one extra
`wrapHttpHandler` call and has no effect. I added a comment there to say so;
happy to restructure it if you prefer another shape.
_Claude Code on behalf of Andrea Cosentino (oscerd)_
##########
components/camel-undertow/src/main/java/org/apache/camel/component/undertow/handlers/CamelWebSocketHandler.java:
##########
@@ -142,37 +169,127 @@ private static void send(
*/
@Override
public void handleRequest(HttpServerExchange exchange) throws Exception {
- UndertowConsumer currentConsumer = getConsumer();
- if (currentConsumer != null &&
currentConsumer.getEndpoint().getOauthHttpSecurity() != null) {
- if (exchange.isInIoThread()) {
- exchange.dispatch(this);
- return;
+ entryHandler.handleRequest(exchange);
+ }
+
+ /**
+ * The handler that applies the security settings of the path to an
upgrade request and then performs the WebSocket
+ * handshake. A consumer can run its own handlers, such as its access log,
before it.
+ */
+ public HttpHandler getUpgradeHandler() {
+ return upgradeHandler;
+ }
+
+ /**
+ * Lets the given security provider wrap this handler, as it wraps the
handlers of HTTP endpoints. The handler stays
+ * registered as is, so that it remains shared by the consumer and the
producers of the path.
+ */
+ public void wrapWith(UndertowSecurityProvider securityProvider) throws
Exception {
+ HttpHandler wrapped =
securityProvider.wrapHttpHandler(consumerRequestHandler);
+ // a provider that returns no handler disables the path, as it does
for HTTP endpoints
+ this.entryHandler = wrapped != null ? wrapped :
ResponseCodeHandler.HANDLE_405;
+ }
+
+ private void handleConsumerRequest(HttpServerExchange exchange) throws
Exception {
+ consumerHandler.handleRequest(exchange);
+ }
+
+ private void upgrade(HttpServerExchange exchange) throws Exception {
+ List<UndertowEndpoint> endpoints = securityEndpoints();
+ if
(endpoints.stream().noneMatch(CamelWebSocketHandler::hasSecurityChecks)) {
+ this.delegate.handleRequest(exchange);
+ return;
+ }
+ if (exchange.isInIoThread()) {
+ exchange.dispatch(upgradeHandler);
+ return;
+ }
+ Set<String> authenticatedEndpoints = new HashSet<>();
+ Map<String, Object> headers = new HashMap<>();
+ for (UndertowEndpoint endpoint : endpoints) {
+ OAuthHttpSecuritySupport oauthHttpSecurity =
endpoint.getOauthHttpSecurity();
+ if (oauthHttpSecurity != null) {
+ Validation validation =
oauthHttpSecurity.validate(endpoint.getCamelContext(),
authorizationHeaders(exchange));
+ exchange.getRequestHeaders().remove(Headers.AUTHORIZATION);
+ if (!validation.isAuthenticated()) {
+ reject(exchange, validation);
+ return;
+ }
+
exchange.putAttachment(OAUTH_TOKEN_VALIDATION_RESULT_ATTACHMENT,
validation.getValidationResult());
}
- OAuthHttpSecuritySupport oauthHttpSecurity =
currentConsumer.getEndpoint().getOauthHttpSecurity();
- Validation validation =
oauthHttpSecurity.validate(currentConsumer.getEndpoint().getCamelContext(),
- authorizationHeaders(exchange));
- exchange.getRequestHeaders().remove(Headers.AUTHORIZATION);
- if (!validation.isAuthenticated()) {
- reject(exchange, validation);
- return;
+ if (endpoint.requiresAuthentication()) {
+ int statusCode = endpoint.authenticate(exchange);
+ if (statusCode != StatusCodes.OK) {
+ exchange.setStatusCode(statusCode);
+ exchange.endExchange();
+ return;
+ }
+ authenticatedEndpoints.add(endpoint.getEndpointUri());
+ if (endpoint.getSecurityProvider() != null) {
+ endpoint.getSecurityProvider().addHeader(headers::put,
exchange);
+ }
}
- exchange.putAttachment(OAUTH_TOKEN_VALIDATION_RESULT_ATTACHMENT,
validation.getValidationResult());
+ }
+ if (!authenticatedEndpoints.isEmpty()) {
+ exchange.putAttachment(SECURITY_PROVIDER_RESULT_ATTACHMENT,
+ new SecurityProviderResult(authenticatedEndpoints,
headers));
}
this.delegate.handleRequest(exchange);
}
- private UndertowConsumer getConsumer() {
+ /**
+ * The endpoints whose security settings apply to the path: the
consumer's, also while it is stopped, otherwise the
+ * producers'. All the Camel endpoints of a path share its WebSocket
connections.
+ */
+ private List<UndertowEndpoint> securityEndpoints() {
consumerLock.lock();
try {
- return consumer;
+ if (consumerEndpoint != null) {
+ return List.of(consumerEndpoint);
+ }
} finally {
consumerLock.unlock();
}
+ return producerEndpoints.stream().distinct().toList();
}
- private boolean requiresOAuth() {
- UndertowConsumer currentConsumer = getConsumer();
- return currentConsumer != null &&
currentConsumer.getEndpoint().getOauthHttpSecurity() != null;
+ private static boolean hasSecurityChecks(UndertowEndpoint endpoint) {
+ return endpoint.getOauthHttpSecurity() != null ||
endpoint.requiresAuthentication();
+ }
+
+ private static boolean isAuthenticated(WebSocketChannel channel,
List<UndertowEndpoint> endpoints) {
+ for (UndertowEndpoint endpoint : endpoints) {
+ if (!isAuthenticated(channel, endpoint)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Whether the handshake of the given channel passed the security checks
of the given endpoint: its OAuth
+ * validation, its security provider and its allowed roles. Always {@code
true} for an endpoint without such checks.
+ */
+ public static boolean isAuthenticated(WebSocketChannel channel,
UndertowEndpoint endpoint) {
+ if (endpoint.getOauthHttpSecurity() != null && (channel == null
+ || !(channel.getAttribute(
+
OAuthHttpSecuritySupport.OAUTH_TOKEN_VALIDATION_RESULT) instanceof
OAuthTokenValidationResult))) {
+ return false;
+ }
+ if (endpoint.requiresAuthentication()) {
+ return channel != null
+ && channel.getAttribute(SECURITY_PROVIDER_RESULT)
instanceof SecurityProviderResult result
+ && result.endpointUris.contains(endpoint.getEndpointUri());
+ }
+ return true;
Review Comment:
Fixed. An upgrade request now records the consumer whose handler chain it
went through, since a request only reaches the upgrade if the `handlers` of
that consumer let it through. An endpoint with `handlers` now counts as a
security check of its path. A channel whose handshake did not pass that chain,
such as one opened while only a producer without security settings used the
path, is closed when it sends a message and is skipped when producers send.
The regression test is
`UndertowWsSecurityWithoutProviderTest#connectionOpenedBeforeAConsumerWithHandlersStartedIsNotServed`,
which uses `handlers=#lateBasicAuth` next to the late-consumer test. It also
checks that a client that goes through the handlers afterwards is served. The
test fails on the previous head.
_Claude Code on behalf of Andrea Cosentino (oscerd)_
##########
components/camel-undertow/src/main/java/org/apache/camel/component/undertow/DefaultUndertowHost.java:
##########
@@ -165,7 +175,7 @@ public HttpHandler registerHandler(
private Undertow registerHandler(UndertowConsumer consumer,
Undertow.Builder builder, HttpHandler handler) {
UndertowSecurityProvider securityProvider = consumer == null
- ? null
+ ? this.securityProvider
: consumer.getEndpoint().getComponent().getSecurityProvider()
!= null
?
consumer.getEndpoint().getComponent().getSecurityProvider()
: consumer.getEndpoint().getSecurityProvider();
Review Comment:
Fixed, and a bit more generally.
- The registering endpoint is now passed through the registration
(`UndertowComponent.registerEndpoint(endpoint, consumer, ...)` and a default
`UndertowHost.registerHandler(endpoint, consumer, ...)`), so the producer
endpoint is known.
- The servlet context is no longer decided only by the first registration on
a port. The server delegates to an entry handler, and the servlet deployment is
started around it as soon as a registering endpoint, or its component, has a
provider whose `requireServletContext()` is true.
- This also covers a provider configured on a consumer endpoint whose
route's producer registered first on the port, which had the same problem.
`ProviderWithServletWebSocketEndpointTest` covers both endpoint-scoped cases:
- a producer-only path with `securityProvider=#servletProvider`;
- a consumer with it, on a port where a producer without a provider
registered first.
Both fail on the previous head.
_Claude Code on behalf of Andrea Cosentino (oscerd)_
--
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]
