From cec19ceade7011f44416a64b4169bdc84d8964d2 Mon Sep 17 00:00:00 2001 From: Phil Adams Date: Mon, 27 Nov 2023 11:44:14 -0600 Subject: [PATCH] feat: improve behavior of HTTP redirects This commit introduces a new okhttp interceptor that will implement customized behavior for HTTP redirects. Specifically, "safe" headers will be included in a redirected request if the original and redirected hosts are the same, or are both within IBM's "cloud.ibm.com" domain. Signed-off-by: Phil Adams --- .travis.yml | 7 + .../sdk/core/http/HttpClientSingleton.java | 84 ++-- .../sdk/core/http/HttpConfigOptions.java | 24 +- .../sdk/core/http/RedirectInterceptor.java | 232 +++++++++ .../sdk/core/test/http/HttpConfigTest.java | 1 + .../core/test/http/redirect/RedirectTest.java | 441 ++++++++++++++++++ .../test/service/ConfigureServiceTest.java | 6 +- 7 files changed, 761 insertions(+), 34 deletions(-) create mode 100644 src/main/java/com/ibm/cloud/sdk/core/http/RedirectInterceptor.java create mode 100644 src/test/java/com/ibm/cloud/sdk/core/test/http/redirect/RedirectTest.java diff --git a/.travis.yml b/.travis.yml index b411eb2c2..e65905e61 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,6 +16,13 @@ cache: directories: - "$HOME/.m2" +addons: + hosts: + - region1.cloud.ibm.com + - region1.notcloud.ibm.com + - region2.cloud.ibm.com + - region2.notcloud.ibm.com + env: global: - MVN_ARGS="--settings build/.travis.settings.xml" diff --git a/src/main/java/com/ibm/cloud/sdk/core/http/HttpClientSingleton.java b/src/main/java/com/ibm/cloud/sdk/core/http/HttpClientSingleton.java index 778d8a86a..f40ad5db4 100644 --- a/src/main/java/com/ibm/cloud/sdk/core/http/HttpClientSingleton.java +++ b/src/main/java/com/ibm/cloud/sdk/core/http/HttpClientSingleton.java @@ -1,5 +1,5 @@ /** - * (C) Copyright IBM Corp. 2015, 2022. + * (C) Copyright IBM Corp. 2015, 2023. * * Licensed 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 @@ -16,6 +16,8 @@ import com.ibm.cloud.sdk.core.http.HttpConfigOptions.LoggingLevel; import com.ibm.cloud.sdk.core.http.gzip.GzipRequestInterceptor; import com.ibm.cloud.sdk.core.service.security.DelegatingSSLSocketFactory; +import com.ibm.cloud.sdk.core.util.EnvironmentUtils; + import okhttp3.Authenticator; import okhttp3.ConnectionSpec; import okhttp3.Interceptor; @@ -179,6 +181,7 @@ private OkHttpClient configureHttpClient() { final OkHttpClient.Builder builder = new OkHttpClient.Builder(); addCookieJar(builder); + addRedirectInterceptor(builder); builder.connectTimeout(60, TimeUnit.SECONDS); builder.writeTimeout(60, TimeUnit.SECONDS); @@ -197,11 +200,34 @@ private OkHttpClient configureHttpClient() { * * @param builder the builder */ - private void addCookieJar(final OkHttpClient.Builder builder) { + private OkHttpClient.Builder addCookieJar(final OkHttpClient.Builder builder) { final CookieManager cookieManager = new CookieManager(); cookieManager.setCookiePolicy(CookiePolicy.ACCEPT_ALL); builder.cookieJar(new ServiceCookieJar(cookieManager)); + return builder; + } + + /** + * Adds the RedirectInterceptor to the client builder. + * @param builder + */ + private OkHttpClient.Builder addRedirectInterceptor(final OkHttpClient.Builder builder) { + String envvar = EnvironmentUtils.getenv("IBMCLOUD_BYPASS_CUSTOM_REDIRECTS"); + if (!Boolean.valueOf(envvar)) { + // Disable the built-in "followRedirects" so that okhttp will ignore redirects. + builder.followRedirects(false).followSslRedirects(false); + + // Add our RedirectInterceptor to the client. + builder.addInterceptor(new RedirectInterceptor()); + LOG.log(Level.FINE, "Registered the redirect interceptor."); + } else { + LOG.log(Level.FINE, "Bypassed redirect interceptor via options."); + + // Set the defaults for these two options. + builder.followRedirects(true).followSslRedirects(true); + } + return builder; } /** @@ -337,30 +363,6 @@ public static void setupTLSProtocol(final OkHttpClient.Builder builder) { } } - /** - * Sets a new list of interceptors for the specified {@link OkHttpClient} instance by removing the specified - * interceptor and returns a new instance with the interceptors configured as requested. - * - * @param client the {@link OkHttpClient} instance to remove the interceptors from - * @param interceptorToRemove the class name of the interceptor to remove - * @return the new {@link OkHttpClient} instance with the new list of interceptors - */ - private OkHttpClient reconfigureClientInterceptors(OkHttpClient client, String interceptorToRemove) { - OkHttpClient.Builder builder = client.newBuilder(); - - if (!builder.interceptors().isEmpty()) { - for (Iterator iter = builder.interceptors().iterator(); iter.hasNext(); ) { - Interceptor element = iter.next(); - if (interceptorToRemove.equals(element.getClass().getSimpleName())) { - LOG.log(Level.FINE, "Removing interceptor " + element.getClass().getName() + " from http client instance."); - iter.remove(); - } - } - } - - return builder.build(); - } - /** * Sets a new list of interceptors for the specified {@link OkHttpClient} instance by removing any interceptors * that implement "interfaceToRemove". @@ -370,7 +372,7 @@ private OkHttpClient reconfigureClientInterceptors(OkHttpClient client, String i * @return the new {@link OkHttpClient} instance with the updated list of interceptors */ private OkHttpClient reconfigureClientInterceptors(OkHttpClient client, - Class interfaceToRemove) { + Class interfaceToRemove) { OkHttpClient.Builder builder = client.newBuilder(); if (!builder.interceptors().isEmpty()) { @@ -395,6 +397,7 @@ private OkHttpClient reconfigureClientInterceptors(OkHttpClient client, public OkHttpClient createHttpClient() { Builder builder = okHttpClient.newBuilder(); addCookieJar(builder); + addRedirectInterceptor(builder); return builder.build(); } @@ -419,7 +422,10 @@ public OkHttpClient configureClient(HttpConfigOptions options) { * @return a new {@link OkHttpClient} instance with the specified options applied */ public OkHttpClient configureClient(OkHttpClient client, HttpConfigOptions options) { + Boolean customRedirects = null; if (options != null) { + customRedirects = options.getCustomRedirects(); + if (options.shouldDisableSslVerification()) { client = disableSslVerification(client); } @@ -443,6 +449,7 @@ public OkHttpClient configureClient(OkHttpClient client, HttpConfigOptions optio options.getAuthenticator()); if (retryInterceptor != null) { client = client.newBuilder().addInterceptor(retryInterceptor).build(); + LOG.log(Level.FINE, "Registered the retry interceptor."); } else { LOG.log(Level.WARNING, "The retry interceptor factory returned a null IRetryInterceptor instance. Retries are disabled."); @@ -453,14 +460,35 @@ public OkHttpClient configureClient(OkHttpClient client, HttpConfigOptions optio // Configure the GZIP interceptor. Boolean enableGzip = options.getGzipCompression(); if (enableGzip != null) { - client = reconfigureClientInterceptors(client, "GzipRequestInterceptor"); + client = reconfigureClientInterceptors(client, GzipRequestInterceptor.class); if (enableGzip.booleanValue()) { client = client.newBuilder() .addInterceptor(new GzipRequestInterceptor()) .build(); + LOG.log(Level.FINE, "Registered the gzip interceptor."); } } } + + // Configure the redirect interceptor. + // We want "custom redirects" to be enabled by default (including a scenario + // where "options" is null), hence the need to handle this feature outside the "if" above. + // We want to register the interceptor if: + // 1. options == null + // 2. options != null and "custom redirects" is explicitly set to Boolean.TRUE + // + // We also want to allow "custom redirects" to be bypassed by setting an environment variable. + // This may be only temporary until we are comfortable with this new function, but I'm + // adding support for this environment variable as a quick fix in case there are "unforeseen + // consequences" after this feature hits the street. + // Example: + // export IBMCLOUD_BYPASS_CUSTOM_REDIRECTS=true + // Setting this environment variable will revert back to using the okhttp builtin support for redirects. + client = reconfigureClientInterceptors(client, RedirectInterceptor.class); + if (customRedirects == null || customRedirects.booleanValue()) { + client = addRedirectInterceptor(client.newBuilder()).build(); + } + return client; } } diff --git a/src/main/java/com/ibm/cloud/sdk/core/http/HttpConfigOptions.java b/src/main/java/com/ibm/cloud/sdk/core/http/HttpConfigOptions.java index d1c626b90..64cc02ab0 100644 --- a/src/main/java/com/ibm/cloud/sdk/core/http/HttpConfigOptions.java +++ b/src/main/java/com/ibm/cloud/sdk/core/http/HttpConfigOptions.java @@ -1,5 +1,5 @@ /** - * (C) Copyright IBM Corp. 2015, 2019. + * (C) Copyright IBM Corp. 2015, 2023. * * Licensed 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 @@ -39,6 +39,7 @@ public enum LoggingLevel { private Boolean enableRetries; private int maxRetries; private int maxRetryInterval; + private Boolean enableCustomRedirects = Boolean.TRUE; private Proxy proxy; private Authenticator proxyAuthenticator; private LoggingLevel loggingLevel; @@ -66,6 +67,10 @@ public int getMaxRetryInterval() { return this.maxRetryInterval; } + public Boolean getCustomRedirects() { + return this.enableCustomRedirects; + } + public Proxy getProxy() { return this.proxy; } @@ -98,6 +103,7 @@ public static class Builder { private Boolean enableRetries; private int maxRetries; private int maxRetryInterval; + private Boolean enableCustomRedirects = Boolean.TRUE; private Proxy proxy; private Authenticator proxyAuthenticator; private LoggingLevel loggingLevel; @@ -122,10 +128,10 @@ public Builder disableSslVerification(boolean disableSslVerification) { } /** - * Sets flag to enable gzip compression of request bodies during HTTP requests. This should ONLY be used if truly + * Sets flag to enable compression of request bodies during HTTP requests. This should ONLY be used if truly * intended, as many webservers can't handle this. * - * @param enableGzipCompression whether to disable SSL verification or not + * @param enableGzipCompression whether to perform gzip-compression of request bodies or not * @return the builder */ public Builder enableGzipCompression(Boolean enableGzipCompression) { @@ -178,6 +184,17 @@ public Builder disableRetries() { return this; } + /** + * Sets flag to enable our custom redirect behavior. + * + * @param enableCustomRedirects whether to enable our custom redirect behavior or not + * @return the builder + */ + public Builder enableCustomRedirects(Boolean enableCustomRedirects) { + this.enableCustomRedirects = enableCustomRedirects; + return this; + } + /** * Sets HTTP proxy to be used by connections with the current client. * @@ -218,6 +235,7 @@ private HttpConfigOptions(Builder builder) { this.enableRetries = builder.enableRetries; this.maxRetries = builder.maxRetries; this.maxRetryInterval = builder.maxRetryInterval; + this.enableCustomRedirects = builder.enableCustomRedirects; this.proxy = builder.proxy; this.proxyAuthenticator = builder.proxyAuthenticator; this.loggingLevel = builder.loggingLevel; diff --git a/src/main/java/com/ibm/cloud/sdk/core/http/RedirectInterceptor.java b/src/main/java/com/ibm/cloud/sdk/core/http/RedirectInterceptor.java new file mode 100644 index 000000000..2b34b921d --- /dev/null +++ b/src/main/java/com/ibm/cloud/sdk/core/http/RedirectInterceptor.java @@ -0,0 +1,232 @@ +/** + * (C) Copyright IBM Corp. 2023. + * + * Licensed 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 com.ibm.cloud.sdk.core.http; + +import java.io.IOException; +import java.net.ProtocolException; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.logging.Logger; + +import org.apache.commons.lang3.StringUtils; + +import okhttp3.HttpUrl; +import okhttp3.Interceptor; +import okhttp3.Request; +import okhttp3.Response; +import okhttp3.internal.http.HttpMethod; + +/** + * This class is an okhttp Interceptor implementation that will automatically + * handle redirects, while also allowing "safe" headers to be included in + * cross-site redirect requests. + */ +public class RedirectInterceptor implements Interceptor { + private static final Logger LOG = Logger.getLogger(RedirectInterceptor.class.getName()); + + // Max # of redirects supported by this interceptor. + // Note that okhttp supports up to 20 "follow-ups" (redirects + retries). + // We'll go with a max of 10 redirects to align with the other cores. + private static final int MAX_REDIRECTS = 10; + + // This is considered a "safe domain" (i.e. if both hosts are located + // within the IBM Cloud domain, then it's considered a "safe" redirect). + private static final String SAFE_DOMAIN = ".cloud.ibm.com"; + + // These are the HTTP headers that are stripped out of a request considered + // to be an unsafe redirect. + private static List safeHeaders = Arrays.asList("Authorization", "WWW-Authenticate", "Cookie", "Cookie2"); + + /** + * Default ctor. + */ + public RedirectInterceptor() { + } + + /** + * This is the primary method invoked by OkHttp when processing interceptors. + * This interceptor will handle automatic redirects up to a maximum of MAX_REDIRECTS. + */ + @Override + public Response intercept(Interceptor.Chain chain) throws IOException { + // Grab our original request from the chain. + Request request = chain.request(); + + // Invoke the first request, then react to the response. + Response response = chain.proceed(request); + + // Keep processing responses while they indicate a redirect. + int redirectCount = 0; + while (isRedirectStatusCode(response.code())) { + LOG.fine(String.format("Received redirect response code (%d) for request %s %s", + response.code(), request.method(), request.url().toString())); + + // Check to see if we've exhausted the max redirects for this request. + redirectCount++; + if (redirectCount > MAX_REDIRECTS) { + LOG.fine(String.format( + "Exhausted max redirects limit (%d), throwing ProtocolException.", redirectCount)); + throw new ProtocolException(String.format("Too many redirects: %d", redirectCount)); + } + + // Build the new request using the redirect info contained in "response". + request = buildRedirect(response); + + // If we couldn't build the new request, then bail out now and just return our current response; + if (request == null) { + break; + } + + // Send the request to the redirected location and receive the new response. + response = chain.proceed(request); + } + + // Return the last response that we received. + return response; + } + + /** + * Builds a new request from a response that indicates that an HTTP redirect + * should be performed. + * @param response the response containing the HTTP "redirect" + * @return a new Request instance or null if an error occurred + */ + protected Request buildRedirect(Response response) throws ProtocolException { + // Retrieve the redirected location and validate. + String location = response.header("Location"); + if (StringUtils.isEmpty(location)) { + throw new ProtocolException("Location header missing or empty in redirect response"); + } + + LOG.fine(String.format("Redirect location: %s", location)); + + // Resolve "location" relative to the original request URL. + HttpUrl url = resolveUrl(response.request().url(), location); + if (url == null) { + throw new ProtocolException( + String.format("A redirect response contains an invalid Location header: %s", location)); + } + + // Make sure the original and redirected request URLs have the same scheme. + boolean sameScheme = url.scheme() == response.request().url().scheme(); + if (!sameScheme) { + LOG.fine("Redirects to a different URL scheme (http -> https, https -> http) are not supported."); + return null; + } + + // Create a request builder from the original request, and start building the redirected request. + Request.Builder builder = response.request().newBuilder(); + String method = response.request().method(); + if (HttpMethod.permitsRequestBody(method)) { + // Map the redirected request to a GET if needed. + int responseCode = response.code(); + boolean maintainBody = responseCode == 307 || responseCode == 308; + if (maintainBody) { + // Use the existing method and body. + builder.method(method, response.request().body()); + LOG.fine(String.format("Using redirect method %s", method)); + } else { + // Map request to a GET with no body. + builder.method("GET", null); + LOG.fine("Redirect method changed to GET."); + } + if (!maintainBody) { + builder.removeHeader("Transfer-Encoding"); + builder.removeHeader("Content-Length"); + builder.removeHeader("Content-Type"); + LOG.fine("Removed body-related headers from redirected request"); + } + } + + // Finally, if this is not a "safe" redirection request, then we need to + // strip out any "safe" headers. + if (!isSafeRedirect(response.request().url(), url)) { + LOG.fine("This is an unsafe redirect."); + Set requestHeaders = response.request().headers().names(); + for (String header : safeHeaders) { + builder.removeHeader(header); + + // If "header" is present in the original request, then log a debug message indicating + // that we're removing it. + if (requestHeaders.contains(header)) { + LOG.fine(String.format("Removed header '%s' from redirected request.", header)); + } + } + } else { + LOG.fine("This is a safe redirect. No headers will be removed."); + } + + // Return the new redirected request. + return builder.url(url).build(); + } + + /** + * Determine whether or not "statusCode" indicates a redirect. + * + * @param statusCode the statusCode to check + * @return true if the specified status code indicates a redirect, false otherwise + */ + protected boolean isRedirectStatusCode(int statusCode) { + boolean isRedirect = false; + switch (statusCode) { + case 300: // Multiple Choice + case 301: // Moved Permanently + case 302: // Moved Temporarily/Found + case 303: // See Other + case 307: // Temporary Redirect + case 308: // Permanent Redirect + isRedirect = true; + break; + } + + return isRedirect; + } + + /** + * Returns true iff the redirection request from "originalUrl" to "redirectedUrl" is + * considered safe. In this context, "safe" means: + * 1. The two URLs share the same host + * OR + * 2. The hosts associated with both URLs are located within the IBM Cloud domain (".cloud.ibm.com"). + * + * @param originalUrl the HttpUrl instance associated with the original request + * @param redirectedUrl the HttpUrl instance associated with a redirected request + * @return + */ + protected boolean isSafeRedirect(HttpUrl originalUrl, HttpUrl redirectedUrl) { + String origHost = originalUrl.host(); + String redirectHost = redirectedUrl.host(); + boolean sameHost = origHost.equals(redirectHost); + boolean safeDomain = origHost.endsWith(SAFE_DOMAIN) && redirectHost.endsWith(SAFE_DOMAIN); + return sameHost || safeDomain; + } + + /** + * Returns a new HttpUrl instance by resolving "newUrl" relative to "baseUrl". + * @param baseUrl a "baseline" HttpUrl instance (e.g. "https://myhost.com/v1") + * @param newUrl a (possibly relative) URL string (e.g. "../v2/api") + * @return a new HttpUrl instance obtained by applying "newUrl" to "baseUrl" + * (e.g. "https://myhost.com/v2/api") + */ + protected HttpUrl resolveUrl(HttpUrl baseUrl, String newUrl) { + HttpUrl result = null; + try { + result = baseUrl.newBuilder(newUrl).build(); + } catch (Throwable t) { + // absorb any exceptions here + } + return result; + } +} diff --git a/src/test/java/com/ibm/cloud/sdk/core/test/http/HttpConfigTest.java b/src/test/java/com/ibm/cloud/sdk/core/test/http/HttpConfigTest.java index 30e547437..ac500d8e5 100644 --- a/src/test/java/com/ibm/cloud/sdk/core/test/http/HttpConfigTest.java +++ b/src/test/java/com/ibm/cloud/sdk/core/test/http/HttpConfigTest.java @@ -61,6 +61,7 @@ public Request authenticate(Route route, Response response) throws IOException { assertTrue(configOptions.shouldDisableSslVerification()); assertEquals(authenticator, configOptions.getProxyAuthenticator()); assertNull(configOptions.getGzipCompression()); + assertEquals(Boolean.TRUE, configOptions.getCustomRedirects()); assertEquals(proxy, configOptions.getProxy()); assertEquals(HttpConfigOptions.LoggingLevel.HEADERS, configOptions.getLoggingLevel()); diff --git a/src/test/java/com/ibm/cloud/sdk/core/test/http/redirect/RedirectTest.java b/src/test/java/com/ibm/cloud/sdk/core/test/http/redirect/RedirectTest.java new file mode 100644 index 000000000..42ad84307 --- /dev/null +++ b/src/test/java/com/ibm/cloud/sdk/core/test/http/redirect/RedirectTest.java @@ -0,0 +1,441 @@ +/** + * (C) Copyright IBM Corp. 2023. + * + * Licensed 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 com.ibm.cloud.sdk.core.test.http.redirect; + +import static com.ibm.cloud.sdk.core.http.HttpHeaders.CONTENT_TYPE; +import static com.ibm.cloud.sdk.core.http.HttpHeaders.LOCATION; +import static org.junit.Assert.assertFalse; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; +import static java.util.Map.entry; + +import java.io.IOException; +import java.net.ProtocolException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.logging.ConsoleHandler; +import java.util.logging.Level; +import java.util.logging.Logger; + +import org.apache.commons.lang3.StringUtils; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.ibm.cloud.sdk.core.http.HttpClientSingleton; +import com.ibm.cloud.sdk.core.http.HttpConfigOptions; +import com.ibm.cloud.sdk.core.http.HttpMediaType; +import com.ibm.cloud.sdk.core.http.RedirectInterceptor; +import com.ibm.cloud.sdk.core.http.RequestBuilder; +import com.ibm.cloud.sdk.core.http.Response; +import com.ibm.cloud.sdk.core.http.ServiceCall; +import com.ibm.cloud.sdk.core.security.Authenticator; +import com.ibm.cloud.sdk.core.security.BearerTokenAuthenticator; +import com.ibm.cloud.sdk.core.service.BaseService; +import com.ibm.cloud.sdk.core.service.exception.UnauthorizedException; +import com.ibm.cloud.sdk.core.service.model.GenericModel; +import com.ibm.cloud.sdk.core.test.BaseServiceUnitTest; +import com.ibm.cloud.sdk.core.util.EnvironmentUtils; +import com.ibm.cloud.sdk.core.util.ResponseConverterUtils; + +import okhttp3.HttpUrl; +import okhttp3.Interceptor; +import okhttp3.OkHttpClient; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; + +public class RedirectTest extends BaseServiceUnitTest { + private static final Logger LOG = Logger.getLogger(RedirectTest.class.getName()); + private static final String OPERATION_PATH = "/v1/path"; + + // Logging level used by this test. + // For debugging, set this to Level.FINE or Level.ALL, etc. + private static Level logLevel = Level.SEVERE; + + // This will be our mocked version of the EnvironmentUtils class. + private static MockedStatic envMock = null; + + public void createEnvMock() { + envMock = Mockito.mockStatic(EnvironmentUtils.class); + } + + public void clearEnvMock() { + if (envMock != null) { + envMock.close(); + envMock = null; + } + } + + public class TestModel extends GenericModel { + String name; + + public String getName() { + return name; + } + } + + public class TestService extends BaseService { + + private static final String SERVICE_NAME = "test"; + + TestService(Authenticator auth) { + super(SERVICE_NAME, auth); + } + + ServiceCall testGet() { + RequestBuilder builder = RequestBuilder.get(HttpUrl.parse(getServiceUrl() + OPERATION_PATH)); + return createServiceCall(builder.build(), ResponseConverterUtils.getObject(TestModel.class)); + } + + ServiceCall testPost() { + RequestBuilder builder = RequestBuilder.post(HttpUrl.parse(getServiceUrl() + OPERATION_PATH)); + builder.bodyContent("this is the request body", "text/plain"); + return createServiceCall(builder.build(), ResponseConverterUtils.getObject(TestModel.class)); + } + } + + private RedirectTest.TestService service; + protected MockWebServer server2; + + protected Map defaultHeaders = Map.ofEntries( + entry("WWW-Authenticate", "authentication instructions"), + entry("Cookie", "chocolate chip"), + entry("Cookie2", "snickerdoodle"), + entry("Foo", "bar")); + + protected List allHeaders = Arrays.asList("Authorization", "WWW-Authenticate", "Cookie", "Cookie2", "Foo"); + protected List unsafeHeaders = Arrays.asList("Foo"); + + /** + * Sets up the two mock servers and the mock service instance prior to the + * invocation of each test method. + */ + @Override + @BeforeMethod + public void setUp() throws Exception { + super.setUp(); + server2 = new MockWebServer(); + server2.start(); + + service = new RedirectTest.TestService(new BearerTokenAuthenticator("this is not a secret")); + service.setServiceUrl(getServerUrl1()); + service.setDefaultHeaders(defaultHeaders); + + createEnvMock(); + + // Set up java.util.logging to display messages on the console. + ConsoleHandler handler = new ConsoleHandler(); + handler.setLevel(logLevel); + Logger logger; + logger = Logger.getLogger(RedirectInterceptor.class.getName()); + logger.setLevel(logLevel); + logger.addHandler(handler); + + logger = Logger.getLogger(HttpClientSingleton.class.getName()); + logger.setLevel(logLevel); + logger.addHandler(handler); + + LOG.setLevel(logLevel); + LOG.addHandler(handler); + } + + @Override + @AfterMethod + public void tearDown() throws IOException { + super.tearDown(); + server2.shutdown(); + clearEnvMock(); + } + + protected String getServerUrl1() { + return super.getMockWebServerUrl(); + } + + protected String getServerUrl2() { + return StringUtils.chop(server2.url("/").toString()); + } + + /** + * Generic test method that simulates a redirected request scenario for an + * initial GET request. + * + * @param host1 the "first" host to which the original request is to be sent + * @param host2 the "second" host to which the redirected request is to be sent + * @param headersPresent the set of headers that should be present in the redirected request + * @throws Exception + */ + protected void runRedirectTestGet(String host1, String host2, List headersPresent, int expectedStatusCode, + int redirectStatusCode) throws Exception { + String server1Url = getServerUrl1().replace("localhost", host1); + String server2Url = getServerUrl2().replace("localhost", host2); + String location = server2Url + OPERATION_PATH; + server.enqueue(new MockResponse().setResponseCode(redirectStatusCode).addHeader(LOCATION, location)); + if (expectedStatusCode == 200) { + server2.enqueue(new MockResponse().setResponseCode(200) + .addHeader(CONTENT_TYPE, HttpMediaType.APPLICATION_JSON).setBody("{\"name\": \"Jason Bourne\"}")); + } else { + server2.enqueue(new MockResponse().setResponseCode(401)); + } + + service.setServiceUrl(server1Url); + Response r = null; + try { + r = service.testGet().execute(); + assertEquals(expectedStatusCode, r.getStatusCode()); + } catch (UnauthorizedException e) { + assertEquals(expectedStatusCode, 401); + } catch (Throwable t) { + fail("Caught unexpected exception: " + t.toString()); + } + + if (expectedStatusCode == 200) { + assertNotNull(r); + assertEquals(r.getResult().getName(), "Jason Bourne"); + } + + // Make sure the first request has all the headers. + assertEquals(server.getRequestCount(), 1); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getPath(), OPERATION_PATH); + verifyHeadersPresent(request, allHeaders); + + // Make sure the second request has only the headers contained in + // "headersPresent". + assertEquals(server2.getRequestCount(), 1); + request = server2.takeRequest(); + assertEquals(request.getPath(), OPERATION_PATH); + assertEquals(request.getMethod(), "GET"); + verifyHeadersPresent(request, headersPresent); + List headersAbsent = new ArrayList<>(allHeaders); + headersAbsent.removeAll(headersPresent); + verifyHeadersAbsent(request, headersAbsent); + } + + /** + * Generic test method that simulates a redirected request scenario for an + * initial POST request with a body. + * + * @param host1 the "first" host to which the original request is to be sent + * @param host2 the "second" host to which the redirected request is to be sent + * @param headersPresent the set of headers that should be present in the redirected request + * @throws Exception + */ + protected void runRedirectTestPost(String host1, String host2, List headersPresent, int expectedStatusCode, + int redirectStatusCode) throws Exception { + String server1Url = getServerUrl1().replace("localhost", host1); + String server2Url = getServerUrl2().replace("localhost", host2); + String location = server2Url + OPERATION_PATH; + server.enqueue(new MockResponse().setResponseCode(redirectStatusCode).addHeader(LOCATION, location)); + if (expectedStatusCode == 200) { + server2.enqueue(new MockResponse().setResponseCode(200) + .addHeader(CONTENT_TYPE, HttpMediaType.APPLICATION_JSON).setBody("{\"name\": \"Jason Bourne\"}")); + } else { + server2.enqueue(new MockResponse().setResponseCode(401)); + } + + service.setServiceUrl(server1Url); + Response r = null; + try { + r = service.testPost().execute(); + assertEquals(expectedStatusCode, r.getStatusCode()); + } catch (UnauthorizedException e) { + assertEquals(expectedStatusCode, 401); + } catch (Throwable t) { + fail("Caught unexpected exception: " + t.toString()); + } + + if (expectedStatusCode == 200) { + assertNotNull(r); + assertEquals(r.getResult().getName(), "Jason Bourne"); + } + + // Make sure the first request has all the headers. + assertEquals(server.getRequestCount(), 1); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getPath(), OPERATION_PATH); + verifyHeadersPresent(request, allHeaders); + + // Make sure the second request has only the headers contained in + // "headersPresent". + assertEquals(server2.getRequestCount(), 1); + request = server2.takeRequest(); + assertEquals(request.getPath(), OPERATION_PATH); + // Also make sure the redirected request was mapped to a GET, depending on the + // redirect status code used. + if (redirectStatusCode == 307 || redirectStatusCode == 308) { + assertEquals(request.getMethod(), "POST"); + assertTrue(request.getBody().size() > 0); + } else { + assertEquals(request.getMethod(), "GET"); + assertEquals(request.getBody().size(), 0); + } + verifyHeadersPresent(request, headersPresent); + List headersAbsent = new ArrayList<>(allHeaders); + headersAbsent.removeAll(headersPresent); + verifyHeadersAbsent(request, headersAbsent); + } + + @Test + public void testRedirectAuthSuccessGet1() throws Exception { + // Hosts are the same. + runRedirectTestGet("region1.cloud.ibm.com", "region1.cloud.ibm.com", allHeaders, 200, 301); + } + + @Test + public void testRedirectAuthSuccessGet2() throws Exception { + // Hosts are different, but within the safe zone. + runRedirectTestGet("region1.cloud.ibm.com", "region2.cloud.ibm.com", allHeaders, 200, 300); + } + + @Test + public void testRedirectAuthSuccessGet3() throws Exception { + // Hosts are the same (outside the safe zone, but this is irrelevant). + runRedirectTestGet("region1.notcloud.ibm.com", "region1.notcloud.ibm.com", allHeaders, 200, 307); + } + + @Test + public void testRedirectAuthSuccessPost1() throws Exception { + // Hosts are the same. + runRedirectTestPost("region1.cloud.ibm.com", "region1.cloud.ibm.com", allHeaders, 200, 302); + } + + @Test + public void testRedirectAuthSuccessPost2() throws Exception { + // Hosts are different, but within the safe zone. + runRedirectTestPost("region1.cloud.ibm.com", "region2.cloud.ibm.com", allHeaders, 200, 303); + } + + @Test + public void testRedirectAuthSuccessPost3() throws Exception { + // Hosts are the same (outside of safe zone, but this is irrelevant). + runRedirectTestPost("region1.notcloud.ibm.com", "region1.notcloud.ibm.com", allHeaders, 200, 308); + } + + @Test + public void testRedirectAuthFail1() throws Exception { + // Hosts different and one is not in safe zone. + runRedirectTestGet("region1.notcloud.ibm.com", "region2.cloud.ibm.com", unsafeHeaders, 401, 301); + } + + @Test + public void testRedirectAuthFail2() throws Exception { + // Hosts different and one is not in safe zone. + runRedirectTestGet("region1.cloud.ibm.com", "region2.notcloud.ibm.com", unsafeHeaders, 401, 302); + } + + @Test + public void testRedirectAuthFail3() throws Exception { + // Hosts different and neither are in safe zone. + runRedirectTestPost("region1.notcloud.ibm.com", "region2.notcloud.ibm.com", unsafeHeaders, 401, 308); + } + + @Test + public void testRedirectAuthFail4() throws Exception { + // Hosts different and neither are in safe zone. + runRedirectTestPost("region1.notcloud.ibm.com", "region2.notcloud.ibm.com", unsafeHeaders, 401, 303); + } + + @Test + public void testRedirectLimit() throws Exception { + // Tests the limit of 10 redirects. + // In this scenario, we should throw a RuntimeException with a + // java.net.ProtocolException as the causedBy. + + String server1Url = getServerUrl1().replace("localhost", "region1.cloud.ibm.com"); + String location = server1Url + OPERATION_PATH; + + // Queue up enough redirect responses to put us over the limit. + for (int i = 0; i <= 20; i++) { + server.enqueue(new MockResponse().setResponseCode(301).addHeader(LOCATION, location)); + } + service.setServiceUrl(server1Url); + try { + service.testGet().execute(); + fail("Expected an exception!"); + } catch (RuntimeException e) { + Throwable causedBy = e.getCause(); + assertNotNull(causedBy); + assertTrue(causedBy instanceof ProtocolException); + } catch (Throwable t) { + fail("Caught incorrect exception: " + t.toString()); + } + } + + @Test + public void testRedirectNoCustomRedirects() throws Exception { + service.configureClient(new HttpConfigOptions.Builder().enableCustomRedirects(false).build()); + assertFalse(checkForInterceptor(service.getClient(), RedirectInterceptor.class)); + } + + @Test + public void testRedirectBypassInterceptor() throws Exception { + // Mock the env var to bypass the interceptor. + envMock.when(() -> EnvironmentUtils.getenv("IBMCLOUD_BYPASS_CUSTOM_REDIRECTS")).thenReturn("true"); + + // Trigger the re-config of the service's client instance. + service.configureClient(null); + + assertFalse(checkForInterceptor(service.getClient(), RedirectInterceptor.class)); + } + + + /** + * Verifies that the headers contained in "headerNames" are present in request + * "r". + * + * @param r the request to check + * @param headerNames the names of the headers to verify + */ + protected void verifyHeadersPresent(RecordedRequest r, List headerNames) { + for (String h : headerNames) { + assertNotNull(r.getHeader(h), String.format("Missing header %s from request", h)); + } + } + + /** + * Verifies that the headers contained in "headerNames" are not present in + * request "r". + * + * @param r the request to check + * @param headerNames the names of the headers that should not be present + */ + protected void verifyHeadersAbsent(RecordedRequest r, List headerNames) { + for (String h : headerNames) { + assertNull(r.getHeader(h), String.format("Header %s should not be present!", h)); + } + } + + /** + * Checks to see if "client" contains a registered interceptor with the class "interceptorClass". + * @param client the client to check + * @param interceptorClass the interceptor class to look for + */ + protected boolean checkForInterceptor(OkHttpClient client, Class interceptorClass) { + assertNotNull(client); + for (Interceptor i : client.newBuilder().interceptors()) { + if (interceptorClass.isAssignableFrom(i.getClass())) { + return true; + } + } + return false; + } +} diff --git a/src/test/java/com/ibm/cloud/sdk/core/test/service/ConfigureServiceTest.java b/src/test/java/com/ibm/cloud/sdk/core/test/service/ConfigureServiceTest.java index b656fd9f7..3865a27fd 100644 --- a/src/test/java/com/ibm/cloud/sdk/core/test/service/ConfigureServiceTest.java +++ b/src/test/java/com/ibm/cloud/sdk/core/test/service/ConfigureServiceTest.java @@ -1,5 +1,5 @@ /** - * (C) Copyright IBM Corp. 2015, 2022. + * (C) Copyright IBM Corp. 2015, 2023. * * Licensed 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 @@ -97,7 +97,7 @@ public void testConfigureServiceOnInitiationCreds() { OkHttpClient client = svc.getClient(); assertNotNull(client); List interceptors = client.interceptors(); - assertTrue(interceptors.size() == 0); + assertEquals(interceptors.size(), 1); } @Test @@ -155,7 +155,7 @@ public void testConfigureServiceOnInitiationCredsGzipDisabled() { OkHttpClient client = svc.getClient(); assertNotNull(client); List interceptors = client.interceptors(); - assertFalse(interceptors.size() > 0); + assertEquals(interceptors.size(), 1); boolean containsGzipInterceptor = false; GzipRequestInterceptor gzip = new GzipRequestInterceptor();