From 70730b2240173cba5a6ae37950049463ef5c661a Mon Sep 17 00:00:00 2001 From: byronantak Date: Thu, 28 Nov 2024 14:21:08 +0000 Subject: [PATCH] Use interceptor approach instead of direct proxy --- .../AuthenticatedRestTemplate.java | 50 ++--------- ...late.java => CookieStoreRestTemplate.java} | 31 +------ ...ginAuthenticationCsrfTokenInterceptor.java | 33 +++----- .../rest/resttemplate/ProxyCheckService.java | 28 +++++-- .../ProxyOutboundRequestInterceptor.java | 83 +++++++++++++++++++ .../rest/resttemplate/ResttemplateConfig.java | 13 ++- .../AuthenticatedRestTemplateTest.java | 2 +- .../resttemplate/ResttemplateConfigTest.java | 2 +- 8 files changed, 139 insertions(+), 103 deletions(-) rename restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/{ProxiedCookieStoreRestTemplate.java => CookieStoreRestTemplate.java} (50%) create mode 100644 restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyOutboundRequestInterceptor.java diff --git a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplate.java b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplate.java index c764de00cb..5c3ecbf2b1 100644 --- a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplate.java +++ b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplate.java @@ -17,7 +17,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpEntity; -import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -52,29 +51,28 @@ public class AuthenticatedRestTemplate { @Autowired ResttemplateConfig restTemplateConfig; - @Autowired ProxyCheckService proxyCheckService; - /** Will delegate calls to the {@link RestTemplate} instance that was configured */ - @Autowired ProxiedCookieStoreRestTemplate restTemplate; + @Autowired CookieStoreRestTemplate restTemplate; /** Used to intercept requests and inject CSRF token */ @Autowired LoginAuthenticationCsrfTokenInterceptor loginAuthenticationCsrfTokenInterceptor; @Autowired RestTemplateUtil restTemplateUtil; + @Autowired ProxyOutboundRequestInterceptor proxyOutboundRequestInterceptor; + /** Initialize the internal restTemplate instance */ @PostConstruct protected void init() { logger.debug("Create the RestTemplate instance that will be wrapped"); - configureEnvoyProxy(restTemplate); makeRestTemplateWithCustomObjectMapper(restTemplate); setErrorHandlerWithLogging(restTemplate); logger.debug("Set interceptor for authentication"); List interceptors = - Collections.singletonList( - loginAuthenticationCsrfTokenInterceptor); + Collections.unmodifiableList( + List.of(proxyOutboundRequestInterceptor, loginAuthenticationCsrfTokenInterceptor)); restTemplate.setRequestFactory( new InterceptingClientHttpRequestFactory(restTemplate.getRequestFactory(), interceptors)); @@ -147,42 +145,6 @@ protected void makeRestTemplateWithCustomObjectMapper(RestTemplate restTemplate) } } - /*** - * Send a dummy request to the proxy. If we get a success, it's configured - */ - protected boolean isProxyConfigured() { - String testUrl = - UriComponentsBuilder.newInstance() - .scheme("http") - .host(restTemplateConfig.getProxyHost()) - .port(restTemplateConfig.getProxyPort()) - .path("login") - .build() - .toUriString(); - HttpHeaders headers = new HttpHeaders(); - headers.set("Host", restTemplateConfig.getHost()); - HttpEntity httpEntity = new HttpEntity<>(null, headers); - ResponseEntity response = - restTemplate.exchange(testUrl, HttpMethod.GET, httpEntity, Void.class); - return response.getStatusCode().is2xxSuccessful(); - } - - /*** - * Update RestTemplate to use Proxy for all requests going forward; - * adds headers to indicate the host - */ - protected void configureEnvoyProxy(ProxiedCookieStoreRestTemplate restTemplate) { - boolean isEnvoyConfigured = proxyCheckService.hasProxy(); - if (isEnvoyConfigured) { - restTemplate.configureProxy( - true, - restTemplateConfig.getProxyHost(), - restTemplateConfig.getProxyPort(), - (httpRequest, entityDetails, httpContext) -> - httpRequest.addHeader("Host", restTemplateConfig.getHost())); - } - } - /** * Gets the full URI (scheme + host + port + path) to access the REST WS for a given resource * path. @@ -395,7 +357,7 @@ public void put(String resourcePath, Object request) { } @VisibleForTesting - public ProxiedCookieStoreRestTemplate getRestTemplate() { + public CookieStoreRestTemplate getRestTemplate() { return restTemplate; } } diff --git a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxiedCookieStoreRestTemplate.java b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/CookieStoreRestTemplate.java similarity index 50% rename from restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxiedCookieStoreRestTemplate.java rename to restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/CookieStoreRestTemplate.java index 1f3cc6c306..307bbe58db 100644 --- a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxiedCookieStoreRestTemplate.java +++ b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/CookieStoreRestTemplate.java @@ -1,14 +1,9 @@ package com.box.l10n.mojito.rest.resttemplate; import org.apache.hc.client5.http.classic.HttpClient; -import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.cookie.BasicCookieStore; import org.apache.hc.client5.http.cookie.CookieStore; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; -import org.apache.hc.client5.http.impl.classic.HttpClients; -import org.apache.hc.client5.http.impl.routing.DefaultProxyRoutePlanner; -import org.apache.hc.core5.http.HttpHost; -import org.apache.hc.core5.http.HttpRequestInterceptor; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; import org.springframework.stereotype.Component; import org.springframework.web.client.RestTemplate; @@ -20,11 +15,11 @@ * @author wyau */ @Component -public class ProxiedCookieStoreRestTemplate extends RestTemplate { +public class CookieStoreRestTemplate extends RestTemplate { CookieStore cookieStore; - public ProxiedCookieStoreRestTemplate() { + public CookieStoreRestTemplate() { super(); cookieStore = new BasicCookieStore(); setCookieStoreAndUpdateRequestFactory(cookieStore); @@ -45,28 +40,6 @@ public void setCookieStoreAndUpdateRequestFactory(CookieStore cookieStore) { setRequestFactory(new HttpComponentsClientHttpRequestFactory(hc)); } - public void configureProxy( - boolean useProxy, String proxyHost, int proxyPort, HttpRequestInterceptor proxyInterceptor) { - logger.debug("Configuring proxy for Rest Template. Enabled = {}", useProxy); - HttpClientBuilder clientBuilder = - HttpClients.custom().setDefaultCookieStore(cookieStore).disableRedirectHandling(); - - if (useProxy) { - logger.debug("Configuring proxy for Rest Template. Proxy settings {}:{}", proxyHost, proxyPort); - if (proxyInterceptor != null) { - HttpHost proxy = new HttpHost(proxyHost, proxyPort); - DefaultProxyRoutePlanner routePlanner = new DefaultProxyRoutePlanner(proxy); - RequestConfig config = RequestConfig.custom().build(); - clientBuilder.setDefaultRequestConfig(config); - clientBuilder.addRequestInterceptorFirst(proxyInterceptor); - clientBuilder.setRoutePlanner(routePlanner); - } - } - - // Rebuild the request factory with or without proxy settings - setRequestFactory(new HttpComponentsClientHttpRequestFactory(clientBuilder.build())); - } - public CookieStore getCookieStore() { return cookieStore; } diff --git a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/LoginAuthenticationCsrfTokenInterceptor.java b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/LoginAuthenticationCsrfTokenInterceptor.java index edd40942f8..464005bb1b 100644 --- a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/LoginAuthenticationCsrfTokenInterceptor.java +++ b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/LoginAuthenticationCsrfTokenInterceptor.java @@ -4,7 +4,6 @@ import jakarta.annotation.PostConstruct; import java.io.IOException; import java.net.URI; -import java.util.Collections; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -57,7 +56,7 @@ public class LoginAuthenticationCsrfTokenInterceptor implements ClientHttpReques * This is used for the authentication flow to keep things separate from the restTemplate that * this interceptor is intercepting */ - ProxiedCookieStoreRestTemplate restTemplateForAuthenticationFlow; + CookieStoreRestTemplate restTemplateForAuthenticationFlow; /** * This is so that we obtain access to the cookie store used inside HttpClient to check to see if @@ -77,31 +76,24 @@ public class LoginAuthenticationCsrfTokenInterceptor implements ClientHttpReques @Autowired CredentialProvider credentialProvider; /** Will delegate calls to the {@link RestTemplate} instance that was configured */ - @Autowired ProxiedCookieStoreRestTemplate restTemplate; + @Autowired CookieStoreRestTemplate restTemplate; + + @Autowired ProxyOutboundRequestInterceptor proxyOutboundRequestInterceptor; /** Init */ @PostConstruct protected void init() { - restTemplateForAuthenticationFlow = new ProxiedCookieStoreRestTemplate(); + restTemplateForAuthenticationFlow = new CookieStoreRestTemplate(); cookieStore = restTemplateForAuthenticationFlow.getCookieStore(); - boolean hasProxy = proxyCheckService.hasProxy(); - if (hasProxy) { - logger.debug("Configuring Proxy for authenticationRestTemplate"); - restTemplateForAuthenticationFlow.configureProxy( - true, - resttemplateConfig.getProxyHost(), - resttemplateConfig.getProxyPort(), - (httpRequest, entityDetails, httpContext) -> - httpRequest.addHeader("Host", resttemplateConfig.getHost())); - } logger.debug( "Inject cookie store used in the rest template for authentication flow into the restTemplate so that they will match"); restTemplate.setCookieStoreAndUpdateRequestFactory(cookieStore); List interceptors = - Collections.singletonList( + List.of( + proxyOutboundRequestInterceptor, new ClientHttpRequestInterceptor() { @Override public ClientHttpResponse intercept( @@ -152,8 +144,8 @@ public ClientHttpResponse intercept( /** * Handle http response from the intercept. It will check to see if the initial response was - * successful (ie. error status such as 301, 403). If so, it'll try the authentication flow again. - * If it further encounters an unsuccessful response, then it'll throw a {@link + * successful (i.e. error status such as 301, 403). If so, it'll try the authentication flow + * again. If it further encounters an unsuccessful response, then it'll throw a {@link * RestClientException} * * @param request @@ -261,7 +253,7 @@ protected void injectCsrfTokenIntoHeader(HttpRequest request, CsrfToken csrfToke } logger.debug( - "Injecting CSRF token into request {} header: {}", request.getURI(), csrfToken.getToken()); + "Injecting CSRF token into request {} token: {}", request.getURI(), csrfToken.getToken()); request.getHeaders().add(csrfToken.getHeaderName(), csrfToken.getToken()); } @@ -286,7 +278,7 @@ protected synchronized void startAuthenticationFlow(boolean usesPreAuthAuthentic restTemplateForAuthenticationFlow.getForEntity( restTemplateUtil.getURIForResource(formLoginConfig.getLoginFormPath()), String.class); - logger.error("login Resonse status code {}", loginResponseEntity.getStatusCode()); + logger.debug("login Resonse status code {}", loginResponseEntity.getStatusCode()); if (!loginResponseEntity.hasBody()) { throw new SessionAuthenticationException( "Authentication failed: no CSRF token could be found. GET login status code = " @@ -301,6 +293,7 @@ protected synchronized void startAuthenticationFlow(boolean usesPreAuthAuthentic MultiValueMap loginPostParams = new LinkedMultiValueMap<>(); loginPostParams.add("username", credentialProvider.getUsername()); loginPostParams.add("password", credentialProvider.getPassword()); + loginPostParams.add("_csrf", latestCsrfToken.getToken()); logger.debug( "Post to login url to startAuthenticationFlow with user={}, pwd={}", @@ -313,7 +306,7 @@ protected synchronized void startAuthenticationFlow(boolean usesPreAuthAuthentic String.class); // TODO(P1) This current way of checking if authentication is successful is somewhat - // hacky. Bascailly it says that authentication is successful if a 302 is returned + // hacky. Basically it says that authentication is successful if a 302 is returned // and the redirect (from location header) maps to the login redirect path from the config. URI locationURI = URI.create(postLoginResponseEntity.getHeaders().get("Location").get(0)); String expectedLocation = diff --git a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyCheckService.java b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyCheckService.java index 851c5495de..50bc506971 100644 --- a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyCheckService.java +++ b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyCheckService.java @@ -1,5 +1,8 @@ package com.box.l10n.mojito.rest.resttemplate; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.InitializingBean; @@ -8,6 +11,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.ResponseEntity; +import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; import org.springframework.stereotype.Component; import org.springframework.web.client.RestTemplate; import org.springframework.web.util.UriComponentsBuilder; @@ -20,8 +24,6 @@ public class ProxyCheckService implements InitializingBean { @Autowired ResttemplateConfig restTemplateConfig; - @Autowired RestTemplate restTemplate; - @Override public void afterPropertiesSet() { hasProxy = isProxyConfigured(); @@ -31,11 +33,11 @@ public boolean hasProxy() { return hasProxy; } - protected boolean isProxyConfigured() { + private boolean isProxyConfigured() { String testUrl = UriComponentsBuilder.newInstance() - .scheme("http") + .scheme(restTemplateConfig.getProxyScheme()) .host(restTemplateConfig.getProxyHost()) .port(restTemplateConfig.getProxyPort()) .path("login") @@ -44,15 +46,29 @@ protected boolean isProxyConfigured() { logger.debug("Checking if proxy is configured with URL '{}'", testUrl); HttpHeaders headers = new HttpHeaders(); headers.set("Host", restTemplateConfig.getHost()); + logger.debug("With headers {}", headers); HttpEntity httpEntity = new HttpEntity<>(null, headers); try { + RestTemplate tempRestTemplate = buildRestTemplate(); ResponseEntity response = - restTemplate.exchange(testUrl, HttpMethod.GET, httpEntity, Void.class); + tempRestTemplate.exchange(testUrl, HttpMethod.GET, httpEntity, Void.class); logger.debug("Proxy login request response code {}", response.getStatusCode()); return response.getStatusCode().is2xxSuccessful(); } catch (Exception e) { - logger.warn("Unable to reach service via proxy. Defaulting to direct access", e.getMessage()); + logger.warn( + "Proxy does not allow access to specified host. Falling back to directly accessing it", + e); return false; } } + + /*** + * This is needed because the default RestTemplate does not allow the caller + * to set the Host header + */ + private RestTemplate buildRestTemplate() { + CloseableHttpClient httpClient = + HttpClients.custom().setDefaultRequestConfig(RequestConfig.custom().build()).build(); + return new RestTemplate(new HttpComponentsClientHttpRequestFactory(httpClient)); + } } diff --git a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyOutboundRequestInterceptor.java b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyOutboundRequestInterceptor.java new file mode 100644 index 0000000000..3d0fce199a --- /dev/null +++ b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ProxyOutboundRequestInterceptor.java @@ -0,0 +1,83 @@ +package com.box.l10n.mojito.rest.resttemplate; + +import java.io.IOException; +import java.net.URI; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; +import org.springframework.http.client.ClientHttpRequestExecution; +import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.stereotype.Component; +import org.springframework.web.util.UriComponentsBuilder; + +@Component +public class ProxyOutboundRequestInterceptor implements ClientHttpRequestInterceptor { + + Logger logger = LoggerFactory.getLogger(ProxyOutboundRequestInterceptor.class); + + @Autowired ResttemplateConfig restTemplateConfig; + @Autowired ProxyCheckService proxyCheckService; + + @Override + public ClientHttpResponse intercept( + HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException { + + if (proxyCheckService.hasProxy()) { + // To prevent adding extra layers, when the proxied request fails and is retried + if (request.getURI().getHost().equals(restTemplateConfig.getProxyHost())) { + logger.debug("Proxy has already been configured for request"); + return execution.execute(request, body); + } + + logger.debug("Configuring request via proxy"); + String rawPath = request.getURI().getRawPath(); + String rawQuery = request.getURI().getRawQuery(); + URI proxyUri = + UriComponentsBuilder.newInstance() + .scheme(restTemplateConfig.getProxyScheme()) + .host(restTemplateConfig.getProxyHost()) + .port(restTemplateConfig.getProxyPort()) + .path(rawPath) + .query(rawQuery) + .build() + .toUri(); + + HttpRequest modifiedRequest = buildProxiedRequest(request, proxyUri); + logger.debug( + "Modified Proxy Request. Method: {}. Uri: {}. Headers: {}", + modifiedRequest.getMethod(), + modifiedRequest.getURI(), + modifiedRequest.getHeaders()); + return execution.execute(modifiedRequest, body); + } + + logger.debug("Proxy is not configured for request"); + return execution.execute(request, body); + } + + private HttpRequest buildProxiedRequest(HttpRequest request, URI proxyUri) { + return new HttpRequest() { + @Override + public HttpMethod getMethod() { + return request.getMethod(); + } + + @Override + public URI getURI() { + return proxyUri; + } + + @Override + public HttpHeaders getHeaders() { + HttpHeaders headers = new HttpHeaders(); + headers.putAll(request.getHeaders()); + headers.set("Host", restTemplateConfig.getHost()); + return headers; + } + }; + } +} diff --git a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfig.java b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfig.java index 563ffdcd6b..51170a168b 100644 --- a/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfig.java +++ b/restclient/src/main/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfig.java @@ -17,7 +17,8 @@ public class ResttemplateConfig { String scheme = "http"; String contextPath = ""; String proxyHost = "localhost"; - int proxyPort = 19193; + String proxyScheme = "http"; + Integer proxyPort = 19193; Authentication authentication = new Authentication(); @@ -106,11 +107,19 @@ public void setProxyHost(String proxyHost) { this.proxyHost = proxyHost; } - public int getProxyPort() { + public Integer getProxyPort() { return proxyPort; } public void setProxyPort(int proxyPort) { this.proxyPort = proxyPort; } + + public String getProxyScheme() { + return proxyScheme; + } + + public void setProxyScheme(String proxyScheme) { + this.proxyScheme = proxyScheme; + } } diff --git a/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplateTest.java b/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplateTest.java index ecaaec92d8..b6f0da5825 100644 --- a/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplateTest.java +++ b/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/AuthenticatedRestTemplateTest.java @@ -55,7 +55,7 @@ public void before() { wireMockServer.start(); // resets the mock server that was set inside the rest template - authenticatedRestTemplate.restTemplate = new ProxiedCookieStoreRestTemplate(); + authenticatedRestTemplate.restTemplate = new CookieStoreRestTemplate(); authenticatedRestTemplate.init(); // if port was 0, then the server will randomize it on start up, and now we diff --git a/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfigTest.java b/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfigTest.java index 186704a53d..cf981be4b2 100644 --- a/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfigTest.java +++ b/restclient/src/test/java/com/box/l10n/mojito/rest/resttemplate/ResttemplateConfigTest.java @@ -35,6 +35,6 @@ public void testConfig() { assertEquals("ChangeMe", resttemplateConfig.getAuthentication().getPassword()); assertEquals("", resttemplateConfig.getContextPath()); assertEquals("localhost", resttemplateConfig.getProxyHost()); - assertEquals(19193, resttemplateConfig.getProxyPort()); + assertEquals(Integer.valueOf(19193), resttemplateConfig.getProxyPort()); } }