From 3e9f4411250611d5bfaf5c1a469e1433755d18e1 Mon Sep 17 00:00:00 2001 From: achmelo Date: Mon, 24 May 2021 18:15:22 +0200 Subject: [PATCH 1/3] unauthorized response message, websocket tests Signed-off-by: achmelo --- discoverable-client/build.gradle | 1 + .../configuration/SecurityConfiguration.java | 37 ++++ .../gateway/ws/WebSocketRoutedSession.java | 22 ++ .../integration/proxy/WebSocketProxyTest.java | 189 ++++++++++++------ 4 files changed, 185 insertions(+), 64 deletions(-) create mode 100644 discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java diff --git a/discoverable-client/build.gradle b/discoverable-client/build.gradle index 62d4c35606..cc767f3b3f 100644 --- a/discoverable-client/build.gradle +++ b/discoverable-client/build.gradle @@ -46,6 +46,7 @@ dependencies { implementation libraries.spring_boot_starter_web implementation libraries.spring_boot_starter_websocket implementation libraries.spring_boot_starter_validation + implementation libraries.spring_boot_starter_security implementation libraries.bootstrap implementation libraries.jquery diff --git a/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java b/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java new file mode 100644 index 0000000000..585204c335 --- /dev/null +++ b/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java @@ -0,0 +1,37 @@ +/* + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright Contributors to the Zowe Project. + */ +package org.zowe.apiml.client.configuration; + +import org.springframework.context.annotation.Configuration; +import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; + +@Configuration +@EnableWebSecurity +public class SecurityConfiguration extends WebSecurityConfigurerAdapter { + + @Override + protected void configure(HttpSecurity http) throws Exception { + http.csrf().disable() + .authorizeRequests() + .antMatchers("/ws/**").authenticated() + .antMatchers("/**").permitAll() + .and().httpBasic(); + } + + @Override + protected void configure(AuthenticationManagerBuilder auth) throws Exception { + auth.inMemoryAuthentication() + .withUser("user") + .password("{noop}pass").roles("ADMIN"); + } +} diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/ws/WebSocketRoutedSession.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/ws/WebSocketRoutedSession.java index 43219a6f61..471dd86d86 100644 --- a/gateway-service/src/main/java/org/zowe/apiml/gateway/ws/WebSocketRoutedSession.java +++ b/gateway-service/src/main/java/org/zowe/apiml/gateway/ws/WebSocketRoutedSession.java @@ -12,8 +12,10 @@ import lombok.extern.slf4j.Slf4j; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.websocket.api.UpgradeException; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.util.concurrent.ListenableFuture; import org.springframework.web.socket.CloseStatus; import org.springframework.web.socket.WebSocketHttpHeaders; @@ -24,6 +26,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.URI; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; /** @@ -86,11 +89,30 @@ private WebSocketSession createWebSocketClientSession(WebSocketSession webSocket } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw webSocketProxyException(targetUrl, e, webSocketServerSession, false); + } catch (ExecutionException e) { + throw handleExecutionException(targetUrl, e, webSocketServerSession, false); } catch (Exception e) { throw webSocketProxyException(targetUrl, e, webSocketServerSession, false); } } + private WebSocketProxyError handleExecutionException(String targetUrl, ExecutionException cause, WebSocketSession webSocketServerSession, boolean logError) { + if (cause.getCause() != null && cause.getCause().getCause() instanceof UpgradeException) { + UpgradeException upgradeException = (UpgradeException) cause.getCause().getCause(); + if (upgradeException.getResponseStatusCode() == HttpStatus.UNAUTHORIZED.value()) { + String message = "Invalid login credentials"; + if (logError) { + log.debug(message); + } + return new WebSocketProxyError(message, cause, webSocketServerSession); + } else { + return webSocketProxyException(targetUrl, cause, webSocketServerSession, logError); + } + } else { + return webSocketProxyException(targetUrl, cause, webSocketServerSession, logError); + } + } + private WebSocketProxyError webSocketProxyException(String targetUrl, Exception cause, WebSocketSession webSocketServerSession, boolean logError) { String message = String.format("Error opening session to WebSocket service at %s: %s", targetUrl, cause.getMessage()); if (logError) { diff --git a/integration-tests/src/test/java/org/zowe/apiml/integration/proxy/WebSocketProxyTest.java b/integration-tests/src/test/java/org/zowe/apiml/integration/proxy/WebSocketProxyTest.java index 8c2773289b..19fa69f511 100644 --- a/integration-tests/src/test/java/org/zowe/apiml/integration/proxy/WebSocketProxyTest.java +++ b/integration-tests/src/test/java/org/zowe/apiml/integration/proxy/WebSocketProxyTest.java @@ -10,6 +10,8 @@ package org.zowe.apiml.integration.proxy; import org.apache.http.client.utils.URIBuilder; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.web.socket.CloseStatus; import org.springframework.web.socket.TextMessage; @@ -27,6 +29,7 @@ import java.net.URI; import java.net.URISyntaxException; +import java.util.Base64; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -39,9 +42,26 @@ class WebSocketProxyTest implements TestWithStartedInstances { private final GatewayServiceConfiguration serviceConfiguration = ConfigReader.environmentConfiguration().getGatewayServiceConfiguration(); - private final static int WAIT_TIMEOUT_MS = 10000; - private final static String UPPERCASE_URL = "/ws/v1/discoverableclient/uppercase"; - private final static String HEADER_URL = "/ws/v1/discoverableclient/header"; + private static final int WAIT_TIMEOUT_MS = 10000; + private static final String UPPERCASE_URL = "/ws/v1/discoverableclient/uppercase"; + private static final String HEADER_URL = "/ws/v1/discoverableclient/header"; + + private static final WebSocketHttpHeaders VALID_AUTH_HEADERS = new WebSocketHttpHeaders(); + private static final WebSocketHttpHeaders INVALID_AUTH_HEADERS = new WebSocketHttpHeaders(); + private static final String validToken = "apimlAuthenticationToken=tokenValue"; + + + @BeforeAll + static void setup() { + String plainCred = "user:pass"; + String base64cred = Base64.getEncoder().encodeToString(plainCred.getBytes()); + VALID_AUTH_HEADERS.add("Authorization", "Basic " + base64cred); + + String invalidPlainCred = "user:invalidPass"; + String invalidBase64cred = Base64.getEncoder().encodeToString(invalidPlainCred.getBytes()); + INVALID_AUTH_HEADERS.add("Authorization", "Basic " + invalidBase64cred); + + } private TextWebSocketHandler appendResponseHandler(StringBuilder target, int countToNotify) { final AtomicInteger counter = new AtomicInteger(countToNotify); @@ -89,86 +109,127 @@ private WebSocketSession appendingWebSocketSession(String url, StringBuilder res return appendingWebSocketSession(url, null, response, countToNotify); } - @Test - void shouldRouteWebSocketSession() throws Exception { - final StringBuilder response = new StringBuilder(); - WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL), response, 1); - session.sendMessage(new TextMessage("hello world!")); - synchronized (response) { - response.wait(WAIT_TIMEOUT_MS); - } + @Nested + class WhenRoutingSession { + @Nested + class Authentication { + @Nested + class WhenValid { + @Nested + class ReturnSuccess { + @Test + void message() throws Exception { + final StringBuilder response = new StringBuilder(); - assertEquals("HELLO WORLD!", response.toString()); - session.close(); - } + WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL), VALID_AUTH_HEADERS, response, 1); - @Test - void shouldRouteHeaders() throws Exception { - final StringBuilder response = new StringBuilder(); - WebSocketHttpHeaders headers = new WebSocketHttpHeaders(); - headers.add("X-Test", "value"); - WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(HEADER_URL), headers, response, 1); + session.sendMessage(new TextMessage("hello world!")); + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } - session.sendMessage(new TextMessage("gimme those headers")); - synchronized (response) { - response.wait(WAIT_TIMEOUT_MS); - } + assertEquals("HELLO WORLD!", response.toString()); + session.close(); + } - assertTrue(response.toString().contains("x-test:\"value\"")); - session.sendMessage(new TextMessage("bye")); - session.close(); - } + @Test + void headers() throws Exception { + final StringBuilder response = new StringBuilder(); + VALID_AUTH_HEADERS.add("X-Test", "value"); + VALID_AUTH_HEADERS.add("Cookie", validToken); + WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(HEADER_URL), VALID_AUTH_HEADERS, response, 1); + + session.sendMessage(new TextMessage("gimme those headers")); + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } + + assertTrue(response.toString().contains("x-test:\"value\"")); + assertTrue(response.toString().contains(validToken)); + session.sendMessage(new TextMessage("bye")); + session.close(); + } + } - @Test - void shouldCloseSessionAfterClientServerCloses() throws Exception { - final StringBuilder response = new StringBuilder(); - WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL), response, 2); + @Nested + class ReturnError { + @Test + void whenPathIsNotCorrect() throws Exception { + final StringBuilder response = new StringBuilder(); + appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL + "bad"), VALID_AUTH_HEADERS, response, 1); - session.sendMessage(new TextMessage("bye")); - synchronized (response) { - response.wait(WAIT_TIMEOUT_MS); - } + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } - assertEquals("BYECloseStatus[code=1000, reason=null]", response.toString()); - } + System.out.println("Response: " + response.toString()); + assertEquals(0, response.toString().indexOf("CloseStatus[code=1003,")); + } - @Test - void shouldFailIfPathIsNotCorrect() throws Exception { - final StringBuilder response = new StringBuilder(); - appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL + "bad"), response, 1); + @Test + void whenServiceIsNotCorrect() throws Exception { + final StringBuilder response = new StringBuilder(); + appendingWebSocketSession(discoverableClientGatewayUrl("/ws/v1/wrong-service/uppercase"), VALID_AUTH_HEADERS, response, 1); - synchronized (response) { - response.wait(WAIT_TIMEOUT_MS); - } + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } - System.out.println("Response: " + response.toString()); - assertEquals(0, response.toString().indexOf("CloseStatus[code=1003,")); - } + assertEquals("CloseStatus[code=1003, reason=Requested service wrong-service is not known by the gateway]", + response.toString()); + } + + @Test + void whenUrlFormatIsNotCorrect() throws Exception { + final StringBuilder response = new StringBuilder(); + appendingWebSocketSession(discoverableClientGatewayUrl("/ws/wrong"), response, 1); + + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } + + assertEquals("CloseStatus[code=1003, reason=Invalid URL format]", response.toString()); + } + } + } + + @Nested + class WhenInvalid { + @Test + void returnError() throws Exception { + final StringBuilder response = new StringBuilder(); - @Test - void shouldFailIfServiceIsNotCorrect() throws Exception { - final StringBuilder response = new StringBuilder(); - appendingWebSocketSession(discoverableClientGatewayUrl("/ws/v1/wrong-service/uppercase"), response, 1); + WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL), INVALID_AUTH_HEADERS, response, 1); - synchronized (response) { - response.wait(WAIT_TIMEOUT_MS); + session.sendMessage(new TextMessage("hello world!")); + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } + + assertEquals("CloseStatus[code=1003, reason=Invalid login credentials]", response.toString()); + session.close(); + } + } } - assertEquals("CloseStatus[code=1003, reason=Requested service wrong-service is not known by the gateway]", - response.toString()); } - @Test - void shouldFailIfUrlFormatIsNotCorrect() throws Exception { - final StringBuilder response = new StringBuilder(); - appendingWebSocketSession(discoverableClientGatewayUrl("/ws/wrong"), response, 1); + @Nested + class WhenClosingSession { + @Test + void getCorrectResponse() throws Exception { + final StringBuilder response = new StringBuilder(); + WebSocketSession session = appendingWebSocketSession(discoverableClientGatewayUrl(UPPERCASE_URL), VALID_AUTH_HEADERS, response, 2); - synchronized (response) { - response.wait(WAIT_TIMEOUT_MS); - } + session.sendMessage(new TextMessage("bye")); + synchronized (response) { + response.wait(WAIT_TIMEOUT_MS); + } - assertEquals("CloseStatus[code=1003, reason=Invalid URL format]", response.toString()); + assertEquals("BYECloseStatus[code=1000, reason=null]", response.toString()); + } } + } From 05c9db8605b17e341c92a0db2534c5e87ce1fec7 Mon Sep 17 00:00:00 2001 From: achmelo Date: Mon, 24 May 2021 18:53:42 +0200 Subject: [PATCH 2/3] ignore auth header for /api/** Signed-off-by: achmelo --- .../apiml/client/configuration/SecurityConfiguration.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java b/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java index 585204c335..a6d6e1b32d 100644 --- a/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java +++ b/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java @@ -12,6 +12,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.builders.WebSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; @@ -22,6 +23,7 @@ public class SecurityConfiguration extends WebSecurityConfigurerAdapter { @Override protected void configure(HttpSecurity http) throws Exception { http.csrf().disable() + .authorizeRequests() .antMatchers("/ws/**").authenticated() .antMatchers("/**").permitAll() @@ -34,4 +36,10 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception { .withUser("user") .password("{noop}pass").roles("ADMIN"); } + + @Override + public void configure(WebSecurity web) { + web.ignoring() + .antMatchers( "/api/**"); + } } From 19f8f617a4b7d198e1f490ba85f31017d6ec41a9 Mon Sep 17 00:00:00 2001 From: achmelo Date: Mon, 24 May 2021 18:53:56 +0200 Subject: [PATCH 3/3] format Signed-off-by: achmelo --- .../zowe/apiml/client/configuration/SecurityConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java b/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java index a6d6e1b32d..1471d0c704 100644 --- a/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java +++ b/discoverable-client/src/main/java/org/zowe/apiml/client/configuration/SecurityConfiguration.java @@ -40,6 +40,6 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception { @Override public void configure(WebSecurity web) { web.ignoring() - .antMatchers( "/api/**"); + .antMatchers("/api/**"); } }