diff --git a/components/camel-http/src/main/java/org/apache/camel/component/http/DefaultAuthenticationHttpClientConfigurer.java b/components/camel-http/src/main/java/org/apache/camel/component/http/DefaultAuthenticationHttpClientConfigurer.java index 8bcad925eb642..13a1a9b4f1605 100644 --- a/components/camel-http/src/main/java/org/apache/camel/component/http/DefaultAuthenticationHttpClientConfigurer.java +++ b/components/camel-http/src/main/java/org/apache/camel/component/http/DefaultAuthenticationHttpClientConfigurer.java @@ -37,16 +37,26 @@ public class DefaultAuthenticationHttpClientConfigurer implements HttpClientConf private final String username; private final char[] password; private final String domain; + private final String scheme; private final String host; + private final Integer port; private final String bearerToken; private final HttpCredentialsHelper credentialsHelper; public DefaultAuthenticationHttpClientConfigurer(String user, String pwd, String domain, String host, String bearerToken, HttpCredentialsHelper credentialsHelper) { + this(user, pwd, domain, null, host, null, bearerToken, credentialsHelper); + } + + DefaultAuthenticationHttpClientConfigurer(String user, String pwd, String domain, String scheme, String host, + Integer port, String bearerToken, + HttpCredentialsHelper credentialsHelper) { this.username = user; this.password = pwd == null ? new char[0] : pwd.toCharArray(); this.domain = domain; + this.scheme = scheme; this.host = host; + this.port = port; this.bearerToken = bearerToken; this.credentialsHelper = credentialsHelper; } @@ -80,7 +90,7 @@ public void configureHttpClient(HttpClientBuilder clientBuilder) { defaultcreds = new UsernamePasswordCredentials(username, password); } clientBuilder.setDefaultCredentialsProvider(credentialsHelper - .getCredentialsProvider(host, null, defaultcreds)); + .getCredentialsProvider(scheme, host, port, defaultcreds)); } } diff --git a/components/camel-http/src/main/java/org/apache/camel/component/http/HttpComponent.java b/components/camel-http/src/main/java/org/apache/camel/component/http/HttpComponent.java index d7706108022e6..72f74b56e534e 100644 --- a/components/camel-http/src/main/java/org/apache/camel/component/http/HttpComponent.java +++ b/components/camel-http/src/main/java/org/apache/camel/component/http/HttpComponent.java @@ -84,6 +84,7 @@ public class HttpComponent extends HttpCommonComponent implements RestProducerFactory, SSLContextParametersAware { private static final Logger LOG = LoggerFactory.getLogger(HttpComponent.class); + private static final String TARGET_URI_PARAMETER = HttpComponent.class.getName() + ".targetUri"; @Metadata(label = "advanced", description = "To use the custom HttpClientConfigurer to perform configuration of the HttpClient that will be used.") @@ -247,6 +248,12 @@ public HttpComponent() { * @throws Exception is thrown if error creating configurer */ protected HttpClientConfigurer createHttpClientConfigurer(Map parameters, boolean secure) throws Exception { + URI targetUri = (URI) parameters.remove(TARGET_URI_PARAMETER); + return createHttpClientConfigurer(parameters, secure, targetUri); + } + + private HttpClientConfigurer createHttpClientConfigurer(Map parameters, boolean secure, URI targetUri) + throws Exception { // prefer to use endpoint configured over component configured HttpClientConfigurer configurer = resolveAndRemoveReferenceParameter(parameters, "httpClientConfigurer", HttpClientConfigurer.class); @@ -255,15 +262,15 @@ protected HttpClientConfigurer createHttpClientConfigurer(Map pa configurer = getHttpClientConfigurer(); } HttpCredentialsHelper credentialsProvider = new HttpCredentialsHelper(); - configurer = configureBasicAuthentication(parameters, configurer, credentialsProvider); + configurer = configureBasicAuthentication(parameters, configurer, credentialsProvider, targetUri); configurer = configureHttpProxy(parameters, configurer, secure, credentialsProvider); - configurer = configureOAuth2Authentication(parameters, configurer); + configurer = configureOAuth2Authentication(parameters, configurer, targetUri); return configurer; } private HttpClientConfigurer configureOAuth2Authentication( - Map parameters, HttpClientConfigurer configurer) { + Map parameters, HttpClientConfigurer configurer, URI targetUri) { String clientId = getParameter(parameters, "oauth2ClientId", String.class); String clientSecret = getParameter(parameters, "oauth2ClientSecret", String.class); @@ -302,14 +309,15 @@ private HttpClientConfigurer configureOAuth2Authentication( cacheTokens, cachedTokensDefaultExpirySeconds, cachedTokensExpirationMarginSeconds, - useBodyAuthentication)); + useBodyAuthentication, + targetUri)); } return configurer; } private HttpClientConfigurer configureBasicAuthentication( Map parameters, HttpClientConfigurer configurer, - HttpCredentialsHelper credentialsProvider) { + HttpCredentialsHelper credentialsProvider, URI targetUri) { String authUsername = getParameter(parameters, "authUsername", String.class); String authPassword = getParameter(parameters, "authPassword", String.class); @@ -319,7 +327,9 @@ private HttpClientConfigurer configureBasicAuthentication( return CompositeHttpConfigurer.combineConfigurers(configurer, new DefaultAuthenticationHttpClientConfigurer( - authUsername, authPassword, authDomain, authHost, null, credentialsProvider)); + authUsername, authPassword, authDomain, authScopeScheme(authHost, targetUri), + authScopeHost(authHost, targetUri), authScopePort(authHost, targetUri), null, + credentialsProvider)); } else if (this.httpConfiguration != null) { if ("basic".equalsIgnoreCase(this.httpConfiguration.getAuthMethod()) || "bearer".equalsIgnoreCase(this.httpConfiguration.getAuthMethod())) { @@ -327,7 +337,10 @@ private HttpClientConfigurer configureBasicAuthentication( new DefaultAuthenticationHttpClientConfigurer( this.httpConfiguration.getAuthUsername(), this.httpConfiguration.getAuthPassword(), this.httpConfiguration.getAuthDomain(), - this.httpConfiguration.getAuthHost(), this.httpConfiguration.getAuthBearerToken(), + authScopeScheme(this.httpConfiguration.getAuthHost(), targetUri), + authScopeHost(this.httpConfiguration.getAuthHost(), targetUri), + authScopePort(this.httpConfiguration.getAuthHost(), targetUri), + this.httpConfiguration.getAuthBearerToken(), credentialsProvider)); } } @@ -335,6 +348,35 @@ private HttpClientConfigurer configureBasicAuthentication( return configurer; } + /** + * The host the credentials are scoped to. + *

+ * {@code authHost} is optional and is unset in the common basic-auth configuration, which made the scope + * {@code new AuthScope(null, -1)} - matching any host, any port, any scheme. HttpClient then offers the credentials + * to whichever host issues a 401 challenge, so with {@code followRedirects=true} a redirect chosen by the remote + * server could collect them. Fall back to the authority the endpoint actually addresses. + */ + private static String authScopeHost(String authHost, URI targetUri) { + if (authHost != null) { + return authHost; + } + return targetUri != null ? targetUri.getHost() : null; + } + + private static String authScopeScheme(String authHost, URI targetUri) { + return authHost == null && targetUri != null ? targetUri.getScheme() : null; + } + + private static Integer authScopePort(String authHost, URI targetUri) { + if (authHost != null || targetUri == null) { + return null; + } + if (targetUri.getPort() >= 0) { + return targetUri.getPort(); + } + return "https".equalsIgnoreCase(targetUri.getScheme()) ? 443 : 80; + } + private HttpClientConfigurer configureHttpProxy( Map parameters, HttpClientConfigurer configurer, boolean secure, HttpCredentialsHelper credentialsProvider) { @@ -450,8 +492,14 @@ protected Endpoint createEndpoint(String uri, String remaining, Map tokenCache = new ConcurrentHashMap<>(); private final boolean useBodyAuthentication; private final String resourceIndicator; + private final URI targetUri; private HttpClient httpClient; public OAuth2ClientConfigurer(String clientId, String clientSecret, String tokenEndpoint, String resourceIndicator, String scope, boolean cacheTokens, long cachedTokensDefaultExpirySeconds, long cachedTokensExpirationMarginSeconds, boolean useBodyAuthentication) { + this(clientId, clientSecret, tokenEndpoint, resourceIndicator, scope, cacheTokens, + cachedTokensDefaultExpirySeconds, cachedTokensExpirationMarginSeconds, useBodyAuthentication, null); + } + + /** + * @param targetUri the URI the endpoint addresses. The bearer token is only attached to requests for the same + * authority, so that a redirect chosen by the remote server cannot collect it. Null keeps the + * previous behaviour of attaching it to whatever authority the request names. + */ + OAuth2ClientConfigurer(String clientId, String clientSecret, String tokenEndpoint, String resourceIndicator, + String scope, boolean cacheTokens, + long cachedTokensDefaultExpirySeconds, long cachedTokensExpirationMarginSeconds, + boolean useBodyAuthentication, URI targetUri) { + this.targetUri = targetUri; this.clientId = clientId; this.clientSecret = clientSecret; this.tokenEndpoint = tokenEndpoint; @@ -78,7 +97,16 @@ public void configureHttpClient(HttpClientBuilder clientBuilder) { clientBuilder.addRequestInterceptorFirst((HttpRequest request, EntityDetails entity, HttpContext context) -> { URI requestUri = getUriFromRequest(request); - OAuth2URIAndCredentials uriAndCredentials = new OAuth2URIAndCredentials(requestUri, clientId, clientSecret); + if (!isTargetAuthority(requestUri)) { + // HttpClient runs protocol-level request interceptors inside ProtocolExec, which sits below + // RedirectExec, so this runs again for every redirect hop. Without this check the bearer token is + // re-attached to whichever authority the Location header named. + LOG.debug("Not attaching the OAuth2 bearer token to {}, which is not the endpoint's authority {}", + requestUri, targetUri); + return; + } + OAuth2URIAndCredentials uriAndCredentials = new OAuth2URIAndCredentials( + requestUri, clientId, clientSecret, tokenEndpoint, scope, resourceIndicator); if (cacheTokens) { if (tokenCache.containsKey(uriAndCredentials) && !tokenCache.get(uriAndCredentials).isExpiredWithMargin(cachedTokensExpirationMarginSeconds)) { @@ -102,6 +130,32 @@ public void configureHttpClient(HttpClientBuilder clientBuilder) { }); } + private boolean isTargetAuthority(URI requestUri) { + if (targetUri == null) { + return true; + } + if (targetUri.getScheme() == null || targetUri.getHost() == null + || requestUri == null || requestUri.getScheme() == null || requestUri.getHost() == null) { + return false; + } + return targetUri.getScheme().equalsIgnoreCase(requestUri.getScheme()) + && targetUri.getHost().equalsIgnoreCase(requestUri.getHost()) + && effectivePort(targetUri) == effectivePort(requestUri); + } + + private static int effectivePort(URI uri) { + if (uri.getPort() >= 0) { + return uri.getPort(); + } + if ("http".equalsIgnoreCase(uri.getScheme())) { + return 80; + } + if ("https".equalsIgnoreCase(uri.getScheme())) { + return 443; + } + return -1; + } + private JsonObject getAccessTokenResponse(HttpClient httpClient) throws IOException { String bodyStr = "grant_type=client_credentials"; if (scope != null) { @@ -177,7 +231,16 @@ public String getToken() { } } - private record OAuth2URIAndCredentials(URI uri, String clientId, String clientSecret) { + /** + * Cache key for a minted token. + *

+ * Every field that shapes the token request has to be part of it. The map is static, so it is shared by every + * configurer instance and every CamelContext in the JVM; a key that left out the scope, the token endpoint or the + * resource indicator would let a route configured for a narrow scope be served a broad-scope token that another + * route cached first, which defeats the scoping the operator asked for and makes the audit trail misleading. + */ + private record OAuth2URIAndCredentials(URI uri, String clientId, String clientSecret, String tokenEndpoint, + String scope, String resourceIndicator) { } @Override diff --git a/components/camel-http/src/test/java/org/apache/camel/component/http/HttpClientConfigurerOverrideTest.java b/components/camel-http/src/test/java/org/apache/camel/component/http/HttpClientConfigurerOverrideTest.java new file mode 100644 index 0000000000000..074cd08cbba94 --- /dev/null +++ b/components/camel-http/src/test/java/org/apache/camel/component/http/HttpClientConfigurerOverrideTest.java @@ -0,0 +1,48 @@ +/* + * 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.http; + +import java.util.Map; + +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class HttpClientConfigurerOverrideTest extends CamelTestSupport { + + @Test + public void existingTwoArgumentOverrideIsStillInvoked() { + TrackingHttpComponent component = new TrackingHttpComponent(); + context.addComponent("http-tracking", component); + + assertThat(context.getEndpoint("http-tracking://localhost:8080")).isNotNull(); + assertThat(component.invoked).isTrue(); + } + + private static final class TrackingHttpComponent extends HttpComponent { + + private boolean invoked; + + @Override + protected HttpClientConfigurer createHttpClientConfigurer(Map parameters, boolean secure) + throws Exception { + invoked = true; + return super.createHttpClientConfigurer(parameters, secure); + } + } +} diff --git a/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2RedirectTokenLeakTest.java b/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2RedirectTokenLeakTest.java new file mode 100644 index 0000000000000..13b6b72c5a5e6 --- /dev/null +++ b/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2RedirectTokenLeakTest.java @@ -0,0 +1,155 @@ +/* + * 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.http; + +import java.util.concurrent.atomic.AtomicReference; + +import org.apache.camel.component.http.handler.OAuth2TokenRequestHandler; +import org.apache.camel.util.IOHelper; +import org.apache.hc.core5.http.impl.bootstrap.HttpServer; +import org.apache.hc.core5.http.impl.bootstrap.ServerBootstrap; +import org.apache.hc.core5.http.io.entity.StringEntity; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * HttpClient runs protocol-level request interceptors inside {@code ProtocolExec}, which sits below + * {@code RedirectExec} in the exec chain, so the OAuth2 interceptor runs once per redirect hop. Without a check it + * re-attaches the bearer token to whichever host the {@code Location} header named - a host chosen by the remote + * server, not by the route. + *

+ * The tests cover both a different host and a different port because credentials are scoped to an authority, not just a + * host name. + */ +public class HttpOAuth2RedirectTokenLeakTest extends BaseHttpTest { + + private static final String FAKE_TOKEN = "xxx.yyy.zzz"; + private static final String CLIENT_ID = "test-client"; + private static final String CLIENT_SECRET = "test-secret"; + + private final AtomicReference authorizationSeenAfterRedirect = new AtomicReference<>(); + + private HttpServer localServer; + private HttpServer differentHostRedirectTarget; + private HttpServer differentPortRedirectTarget; + + @Override + public void setupResources() throws Exception { + differentHostRedirectTarget = createRedirectTarget("127.0.0.1"); + differentHostRedirectTarget.start(); + differentPortRedirectTarget = createRedirectTarget("localhost"); + differentPortRedirectTarget.start(); + + localServer = ServerBootstrap.bootstrap() + .setCanonicalHostName("localhost").setHttpProcessor(getBasicHttpProcessor()) + .setConnectionReuseStrategy(getConnectionReuseStrategy()).setResponseFactory(getHttpResponseFactory()) + .setSslContext(getSSLContext()) + .register("/token", new OAuth2TokenRequestHandler(FAKE_TOKEN, CLIENT_ID, CLIENT_SECRET)) + .register("/redirect-to-different-host", (request, response, context) -> { + response.setHeader("Location", + "http://127.0.0.1:" + differentHostRedirectTarget.getLocalPort() + "/elsewhere"); + response.setCode(302); + }) + .register("/redirect-to-different-port", (request, response, context) -> { + response.setHeader("Location", + "http://localhost:" + differentPortRedirectTarget.getLocalPort() + "/elsewhere"); + response.setCode(302); + }) + .register("/challenge-on-different-host", (request, response, context) -> { + response.setHeader("Location", + "http://127.0.0.1:" + differentHostRedirectTarget.getLocalPort() + "/challenge"); + response.setCode(302); + }) + .register("/challenge-on-different-port", (request, response, context) -> { + response.setHeader("Location", + "http://localhost:" + differentPortRedirectTarget.getLocalPort() + "/challenge"); + response.setCode(302); + }) + .create(); + + localServer.start(); + } + + private HttpServer createRedirectTarget(String canonicalHostName) { + return ServerBootstrap.bootstrap() + .setCanonicalHostName(canonicalHostName).setHttpProcessor(getBasicHttpProcessor()) + .setConnectionReuseStrategy(getConnectionReuseStrategy()).setResponseFactory(getHttpResponseFactory()) + .register("/elsewhere", (request, response, context) -> { + authorizationSeenAfterRedirect.set( + request.containsHeader("Authorization") + ? request.getFirstHeader("Authorization").getValue() : null); + response.setCode(200); + response.setEntity(new StringEntity("Bye World")); + }) + .register("/challenge", (request, response, context) -> { + authorizationSeenAfterRedirect.set( + request.containsHeader("Authorization") + ? request.getFirstHeader("Authorization").getValue() : null); + response.setHeader("WWW-Authenticate", "Basic realm=\"elsewhere\""); + response.setCode(401); + }) + .create(); + } + + @Override + public void cleanupResources() throws Exception { + IOHelper.close(localServer, differentHostRedirectTarget, differentPortRedirectTarget); + } + + @ParameterizedTest + @ValueSource(booleans = { true, false }) + public void theBearerTokenIsNotSentToARedirectTarget(boolean differentHost) { + HttpComponent http = context.getComponent("http", HttpComponent.class); + http.setFollowRedirects(true); + + String tokenEndpoint = "http://localhost:" + localServer.getLocalPort() + "/token"; + String redirectPath = differentHost ? "/redirect-to-different-host" : "/redirect-to-different-port"; + String uri = "http://localhost:" + localServer.getLocalPort() + redirectPath + "?oauth2ClientId=" + CLIENT_ID + + "&oauth2ClientSecret=" + CLIENT_SECRET + "&oauth2TokenEndpoint=" + tokenEndpoint; + + String body = fluentTemplate.to(uri).request(String.class); + + assertThat(body).as("the redirect should still have been followed").isEqualTo("Bye World"); + assertThat(authorizationSeenAfterRedirect.get()) + .as("the bearer token must not be re-attached to the authority the Location header named").isNull(); + } + + /** + * The basic-auth half of the same problem: authHost is optional and unset in the common configuration, which made + * the credentials scope {@code new AuthScope(null, -1)} - any host, any port, any scheme. HttpClient then offers + * the credentials to whichever host issues a 401 challenge, including one reached by following a redirect the + * remote server chose. + */ + @ParameterizedTest + @ValueSource(booleans = { true, false }) + public void basicCredentialsAreNotOfferedToARedirectTarget(boolean differentHost) { + HttpComponent http = context.getComponent("http", HttpComponent.class); + http.setFollowRedirects(true); + + String challengePath = differentHost ? "/challenge-on-different-host" : "/challenge-on-different-port"; + String uri = "http://localhost:" + localServer.getLocalPort() + challengePath + + "?throwExceptionOnFailure=false" + + "&authUsername=scott&authPassword=tiger"; + + fluentTemplate.to(uri).request(String.class); + + assertThat(authorizationSeenAfterRedirect.get()) + .as("basic credentials must not be offered to the authority the Location header named").isNull(); + } +} diff --git a/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2TokenCachingTest.java b/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2TokenCachingTest.java index fb8cd4c95c2ab..540f6c37d1ab0 100644 --- a/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2TokenCachingTest.java +++ b/components/camel-http/src/test/java/org/apache/camel/component/http/HttpOAuth2TokenCachingTest.java @@ -63,6 +63,52 @@ public void tokenIsCached() throws Exception { } } + /** + * The cache is a static map shared by every configurer instance and every CamelContext in the JVM, so its key has + * to name every field that shapes the token request. When the scope was left out, a route asking for a narrow scope + * was served whatever token another route had cached for the same target and credentials. + *

+ * Uses the same trick as the tests around it: close the token endpoint, then make the second request. A cache hit + * succeeds; a miss has to mint a token and cannot. + */ + @Test + public void aDifferentScopeDoesNotReuseACachedToken() throws Exception { + try (var localServer = createLocalServer(); var localOAuth2Server = createLocalOAuth2Server()) { + String tokenEndpoint = "http://localhost:" + localOAuth2Server.getLocalPort() + "/token"; + String base = "http://localhost:" + localServer.getLocalPort() + "/post?httpMethod=POST&oauth2ClientId=" + + clientId + "&oauth2ClientSecret=" + clientSecret + "&oauth2TokenEndpoint=" + tokenEndpoint + + "&oauth2CacheTokens=true&oauth2Scope="; + + // caches a token for the narrow scope + template.request(base + "read", exchange -> { + }); + localOAuth2Server.close(); + + // same target and credentials, different scope: the narrow-scope token must not be handed out + Exchange exchange = template.request(base + "read+write", exchange1 -> { + }); + assertExceptionExchange(exchange); + } + } + + @Test + public void theSameScopeStillReusesTheCachedToken() throws Exception { + try (var localServer = createLocalServer(); var localOAuth2Server = createLocalOAuth2Server()) { + String tokenEndpoint = "http://localhost:" + localOAuth2Server.getLocalPort() + "/token"; + String requestUrl = "http://localhost:" + localServer.getLocalPort() + "/post?httpMethod=POST&oauth2ClientId=" + + clientId + "&oauth2ClientSecret=" + clientSecret + "&oauth2TokenEndpoint=" + tokenEndpoint + + "&oauth2CacheTokens=true&oauth2Scope=read"; + + template.request(requestUrl, exchange -> { + }); + localOAuth2Server.close(); + + Exchange exchange = template.request(requestUrl, exchange1 -> { + }); + assertExchange(exchange); + } + } + @Test public void tokenIsNotCachedWhenCacheTokensIsFalse() throws Exception { try (var localServer = createLocalServer(); var localOAuth2Server = createLocalOAuth2Server()) { diff --git a/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/JettyHttpComponent.java b/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/JettyHttpComponent.java index a82c5217795e2..d99f7883407db 100644 --- a/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/JettyHttpComponent.java +++ b/components/camel-jetty-common/src/main/java/org/apache/camel/component/jetty/JettyHttpComponent.java @@ -201,6 +201,10 @@ protected Endpoint createEndpoint(String uri, String remaining, Map + * {@code new CrossOriginFilter()} with no init parameters takes Jetty's own defaults, which are + * {@code allowedOrigins=*} together with {@code allowCredentials=true}. Since the filter reflects the request's + * origin rather than sending {@code *}, that is the credentialed any-origin configuration the fetch specification + * refuses to express - reflection being the usual way around that rule. An option named "enable CORS" should not + * mean "every origin, with credentials". + *

+ * Credentials therefore default to off. Reflection of the origin is left as it was, so enabling CORS keeps working + * for requests that carry no credentials; an operator who needs credentialed cross-origin requests sets + * {@code filterInit.allowCredentials=true} and is expected to name the origins in {@code filterInit.allowedOrigins} + * at the same time, which is warned about here if they do not. + */ + @SuppressWarnings("unchecked") + private void applyCorsDefaults(Map filterInitParameters) { + Object configuredCredentials = filterInitParameters.get(CrossOriginFilter.ALLOW_CREDENTIALS_PARAM); + if (configuredCredentials == null) { + filterInitParameters.put(CrossOriginFilter.ALLOW_CREDENTIALS_PARAM, "false"); + return; + } + Object configuredOrigins = filterInitParameters.get(CrossOriginFilter.ALLOWED_ORIGINS_PARAM); + if (Boolean.parseBoolean(configuredCredentials.toString()) + && (configuredOrigins == null || "*".equals(configuredOrigins.toString().trim()))) { + LOG.warn("enableCORS is configured with {}=true and no specific {}." + + " Every origin will be able to make credentialed cross-origin requests to this endpoint.", + CrossOriginFilter.ALLOW_CREDENTIALS_PARAM, CrossOriginFilter.ALLOWED_ORIGINS_PARAM); + } + } + private void setFilters(JettyHttpEndpoint endpoint, Server server) { ServletContextHandler context = server.getDescendant(ServletContextHandler.class); List filters = endpoint.getFilters(); diff --git a/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/EnableCORSTest.java b/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/EnableCORSTest.java index 5d067835054df..43786d86cad7c 100644 --- a/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/EnableCORSTest.java +++ b/components/camel-jetty/src/test/java/org/apache/camel/component/jetty/EnableCORSTest.java @@ -17,11 +17,13 @@ package org.apache.camel.component.jetty; import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.test.AvailablePortFinder; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.client5.http.impl.classic.HttpClients; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -29,6 +31,13 @@ public class EnableCORSTest extends BaseJettyTest { + @RegisterExtension + static AvailablePortFinder.Port port3 = AvailablePortFinder.find(); + + private static int getPort3() { + return port3.getPort(); + } + @Test public void testCORSdisabled() throws Exception { HttpGet httpMethod = new HttpGet("http://localhost:" + getPort() + "/test1"); @@ -44,19 +53,45 @@ public void testCORSdisabled() throws Exception { } } + /** + * enableCORS on its own reflects the request origin, which is what makes CORS work at all, but must not also grant + * credentials: reflecting the origin is the usual way around the fetch specification's refusal to pair "*" with + * credentials, so the two together are the credentialed any-origin configuration. + */ @Test - public void testCORSenabled() throws Exception { + public void testCORSenabledDoesNotGrantCredentials() throws Exception { HttpGet httpMethod = new HttpGet("http://localhost:" + getPort2() + "/test2"); httpMethod.addHeader("Origin", "http://localhost:9000"); httpMethod.addHeader("Referer", "http://localhost:9000"); + try (CloseableHttpClient client = HttpClients.createDefault(); + CloseableHttpResponse response = client.execute(httpMethod)) { + + assertEquals(200, response.getCode(), "Get a wrong response status"); + + // the origin is still reflected, so CORS itself keeps working + assertEquals("http://localhost:9000", response.getFirstHeader("Access-Control-Allow-Origin").getValue()); + + Object credentials = response.getFirstHeader("Access-Control-Allow-Credentials"); + assertTrue(credentials == null + || !Boolean.parseBoolean(response.getFirstHeader("Access-Control-Allow-Credentials").getValue()), + "credentials must not be granted to an origin the operator did not name"); + } + } + + @Test + public void testCORSCredentialsCanBeAskedFor() throws Exception { + HttpGet httpMethod = new HttpGet("http://localhost:" + getPort3() + "/test3"); + httpMethod.addHeader("Origin", "http://localhost:9000"); + httpMethod.addHeader("Referer", "http://localhost:9000"); + try (CloseableHttpClient client = HttpClients.createDefault(); CloseableHttpResponse response = client.execute(httpMethod)) { assertEquals(200, response.getCode(), "Get a wrong response status"); String responseHeader = response.getFirstHeader("Access-Control-Allow-Credentials").getValue(); - assertTrue(Boolean.parseBoolean(responseHeader), "CORS not enabled"); + assertTrue(Boolean.parseBoolean(responseHeader), "credentials should be granted when configured"); } } @@ -66,6 +101,9 @@ protected RouteBuilder createRouteBuilder() { public void configure() { from("jetty://http://localhost:{{port}}/test1?enableCORS=false").transform(simple("OK")); from("jetty://http://localhost:{{port2}}/test2?enableCORS=true").transform(simple("OK")); + from("jetty://http://localhost:" + getPort3() + "/test3?enableCORS=true" + + "&filterInit.allowedOrigins=http://localhost:9000" + + "&filterInit.allowCredentials=true").transform(simple("OK")); } }; } diff --git a/components/camel-platform-http-vertx/src/main/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpServerSupport.java b/components/camel-platform-http-vertx/src/main/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpServerSupport.java index aaca7dc7e62a0..ab5224550110f 100644 --- a/components/camel-platform-http-vertx/src/main/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpServerSupport.java +++ b/components/camel-platform-http-vertx/src/main/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpServerSupport.java @@ -114,14 +114,21 @@ static Handler createCorsHandler(VertxPlatformHttpServerConfigur corsConfig.getHeaders()); } - final boolean allowsOrigin - = ObjectHelper.isEmpty(corsConfig.getOrigins()) || corsConfig.getOrigins().contains(origin); + // With no origin list configured the request origin is simply reflected back. That is the + // standard way around the fetch spec's rule that "*" and credentials are mutually exclusive, + // so credentials are only allowed when the operator actually named the origins. + final boolean explicitOrigins = ObjectHelper.isNotEmpty(corsConfig.getOrigins()); + final boolean allowsOrigin = !explicitOrigins || corsConfig.getOrigins().contains(origin); if (allowsOrigin) { response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN, origin); + // The response body varies with the request origin, so it must not be cached against + // one origin and served to another. + response.headers().add(HttpHeaders.VARY, HttpHeaders.ORIGIN); + if (explicitOrigins) { + response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS, "true"); + } } - response.headers().set(HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS, "true"); - if (ObjectHelper.isNotEmpty(corsConfig.getExposedHeaders())) { response.headers().set(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS, String.join(",", corsConfig.getExposedHeaders())); diff --git a/components/camel-platform-http-vertx/src/test/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpEngineTest.java b/components/camel-platform-http-vertx/src/test/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpEngineTest.java index 2bfafb0db0fa3..d81479098864d 100644 --- a/components/camel-platform-http-vertx/src/test/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpEngineTest.java +++ b/components/camel-platform-http-vertx/src/test/java/org/apache/camel/component/platform/http/vertx/VertxPlatformHttpEngineTest.java @@ -73,8 +73,10 @@ import static org.hamcrest.Matchers.emptyOrNullString; import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -425,6 +427,82 @@ public void configure() { } } + @Test + public void testEngineCORSNoOriginListDoesNotAllowCredentials() throws Exception { + // With no origins configured the request origin is reflected back. Reflection plus + // Access-Control-Allow-Credentials is exactly what the fetch spec forbids expressing as "*", + // so credentials must not be granted to an origin the operator never named. + final CamelContext context = createCamelContextForTest(configuration -> { + configuration.getCors().setEnabled(true); + configuration.getCors().setMethods(Arrays.asList("GET", "POST")); + }); + + try { + context.addRoutes(new RouteBuilder() { + @Override + public void configure() { + from("platform-http:/").transform().constant("cors"); + } + }); + context.start(); + + final String origin = "http://attacker.example"; + + given() + .header("Origin", origin) + .when() + .get("/") + .then() + .statusCode(200) + .header("Access-Control-Allow-Origin", origin) + .header("Vary", equalToIgnoringCase("origin")) + .header("Access-Control-Allow-Credentials", nullValue()); + } finally { + context.stop(); + } + } + + @Test + public void testEngineCORSAllowsCredentialsOnlyForAConfiguredOrigin() throws Exception { + final String allowed = "https://app.example"; + final CamelContext context = createCamelContextForTest(configuration -> { + configuration.getCors().setEnabled(true); + configuration.getCors().setOrigins(Arrays.asList(allowed)); + configuration.getCors().setMethods(Arrays.asList("GET", "POST")); + }); + + try { + context.addRoutes(new RouteBuilder() { + @Override + public void configure() { + from("platform-http:/").transform().constant("cors"); + } + }); + context.start(); + + given() + .header("Origin", allowed) + .when() + .get("/") + .then() + .statusCode(200) + .header("Access-Control-Allow-Origin", allowed) + .header("Access-Control-Allow-Credentials", "true"); + + // An origin outside the configured list gets neither header + given() + .header("Origin", "https://other.example") + .when() + .get("/") + .then() + .statusCode(200) + .header("Access-Control-Allow-Origin", nullValue()) + .header("Access-Control-Allow-Credentials", nullValue()); + } finally { + context.stop(); + } + } + @Test public void testMatchOnUriPrefix() throws Exception { final CamelContext context = createCamelContextForTest();