From a97bb094d1fde7d2bbf18dc63270969a9d97cc00 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Thu, 1 Aug 2024 22:30:38 +0530 Subject: [PATCH 01/10] Add changes related to extracting API key from enforcer --- .../connect/enforcer/constants/Constants.java | 3 ++ .../enforcer/constants/HttpConstants.java | 1 + .../connect/enforcer/grpc/ExtAuthService.java | 12 ++++++- .../jwt/InternalAPIKeyAuthenticator.java | 36 +++++++++++++++++-- 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/Constants.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/Constants.java index fea7b10fc9..bf022b5274 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/Constants.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/Constants.java @@ -71,4 +71,7 @@ public class Constants { public static final String PROP_CON_FACTORY = "connectionfactory.TopicConnectionFactory"; public static final String DEFAULT_DESTINATION_TYPE = "Topic"; public static final String DEFAULT_CON_FACTORY_JNDI_NAME = "TopicConnectionFactory"; + + // keyword to identify API-Key sent in sec-websocket-protocol header + public static final String WS_API_KEY_IDENTIFIER = "choreo-internal-API-Key"; } diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/HttpConstants.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/HttpConstants.java index 764f5dc427..74a9e5a30f 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/HttpConstants.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/constants/HttpConstants.java @@ -28,4 +28,5 @@ public class HttpConstants { public static final String X_REQUEST_ID_HEADER = "x-request-id"; public static final String APPLICATION_JSON = "application/json"; public static final String BASIC_LOWER = "basic"; + public static final String WEBSOCKET_PROTOCOL_HEADER = "sec-websocket-protocol"; } diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java index 599b6b8000..c9427d4693 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java @@ -37,6 +37,7 @@ import org.json.JSONObject; import org.wso2.choreo.connect.enforcer.api.ResponseObject; import org.wso2.choreo.connect.enforcer.constants.APIConstants; +import org.wso2.choreo.connect.enforcer.constants.Constants; import org.wso2.choreo.connect.enforcer.constants.HttpConstants; import org.wso2.choreo.connect.enforcer.constants.MetadataConstants; import org.wso2.choreo.connect.enforcer.constants.RouterAccessLogConstants; @@ -101,6 +102,8 @@ private CheckResponse buildResponse(CheckRequest request, ResponseObject respons DeniedHttpResponse.Builder responseBuilder = DeniedHttpResponse.newBuilder(); HttpStatus status = HttpStatus.newBuilder().setCodeValue(responseObject.getStatusCode()).build(); String traceKey = request.getAttributes().getRequest().getHttp().getId(); + String[] secProtocolHeaderForWS = request.getAttributes().getRequest().getHttp().getHeadersOrDefault( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER, "").split(","); Struct.Builder structBuilder = Struct.newBuilder(); // Used to identify that the choreo-connect-enforcer handled the request. It is used to // provide local reply for authentication failures. @@ -151,7 +154,14 @@ private CheckResponse buildResponse(CheckRequest request, ResponseObject respons .build(); } else { OkHttpResponse.Builder okResponseBuilder = OkHttpResponse.newBuilder(); - + if (secProtocolHeaderForWS[0].equals(Constants.WS_API_KEY_IDENTIFIER) && + secProtocolHeaderForWS.length == 2) { + okResponseBuilder.addResponseHeadersToAdd( + HeaderValueOption.newBuilder() + .setHeader(HeaderValue.newBuilder().setKey( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER).setValue(Constants.WS_API_KEY_IDENTIFIER).build()) + .build()); + } // If the user is sending the APIKey credentials within query parameters, those query parameters should // not be sent to the backend. Hence, the :path header needs to be constructed again removing the apiKey // query parameter. In this scenario, apiKey query parameter is sent within the property called diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java index 0f3db01e49..57c28a9420 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java @@ -36,6 +36,7 @@ import org.wso2.choreo.connect.enforcer.config.EnforcerConfig; import org.wso2.choreo.connect.enforcer.constants.APIConstants; import org.wso2.choreo.connect.enforcer.constants.APISecurityConstants; +import org.wso2.choreo.connect.enforcer.constants.HttpConstants; import org.wso2.choreo.connect.enforcer.dto.APIKeyValidationInfoDTO; import org.wso2.choreo.connect.enforcer.dto.JWTTokenPayloadInfo; import org.wso2.choreo.connect.enforcer.exception.APISecurityException; @@ -47,6 +48,8 @@ import org.wso2.choreo.connect.enforcer.util.FilterUtils; import java.text.ParseException; +import java.util.Arrays; +import java.util.stream.Collectors; /** * Implements the authenticator interface to authenticate request using an Internal Key. @@ -55,6 +58,7 @@ public class InternalAPIKeyAuthenticator extends APIKeyHandler { private static final Log log = LogFactory.getLog(InternalAPIKeyAuthenticator.class); private String securityParam; + private String secProtocolHeader = "sec-webSocket-protocol"; private AbstractAPIMgtGatewayJWTGenerator jwtGenerator; private final boolean isGatewayTokenCacheEnabled; @@ -68,9 +72,20 @@ public InternalAPIKeyAuthenticator(String securityParam) { } @Override + // check if sec-websocket-protocol header is there public boolean canAuthenticate(RequestContext requestContext) { - String internalKey = requestContext.getHeaders().get( - ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); + String apiType = requestContext.getMatchedAPI().getApiType(); + String internalKey; + if (apiType.equalsIgnoreCase("WS")) { + internalKey = requestContext.getHeaders().get( + ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); + if (internalKey == null || internalKey.isBlank()) { + internalKey = requestContext.getHeaders().get(HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(",") [1]; + } + } else { + internalKey = requestContext.getHeaders().get( + ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); + } return isAPIKey(internalKey); } @@ -281,10 +296,25 @@ public String getName() { } private String extractInternalKey(RequestContext requestContext) { - String internalKey = requestContext.getHeaders().get(securityParam); + String internalKey; + internalKey = requestContext.getHeaders().get(securityParam); if (internalKey != null) { return internalKey.trim(); } + if (requestContext.getMatchedAPI().getApiType().equalsIgnoreCase("WS")) { + String secProtocolHeaderValue = requestContext.getHeaders().get(HttpConstants.WEBSOCKET_PROTOCOL_HEADER); + internalKey = secProtocolHeaderValue.split(",")[1]; + if (internalKey != null) { + if (secProtocolHeaderValue.split(",").length > 2) { + String protocols = Arrays.stream( + secProtocolHeaderValue.split(","), 2, + secProtocolHeaderValue.split(",").length) + .collect(Collectors.joining(",")); + requestContext.addOrModifyHeaders(HttpConstants.WEBSOCKET_PROTOCOL_HEADER, protocols); + } + return internalKey.trim(); + } + } return null; } From 82688158ef5b60209f30c5789b0f2ba899df7981 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Thu, 1 Aug 2024 22:59:12 +0530 Subject: [PATCH 02/10] Fix checkstyle errors --- .../choreo/connect/enforcer/grpc/ExtAuthService.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java index c9427d4693..b3f5183618 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java @@ -157,10 +157,11 @@ private CheckResponse buildResponse(CheckRequest request, ResponseObject respons if (secProtocolHeaderForWS[0].equals(Constants.WS_API_KEY_IDENTIFIER) && secProtocolHeaderForWS.length == 2) { okResponseBuilder.addResponseHeadersToAdd( - HeaderValueOption.newBuilder() - .setHeader(HeaderValue.newBuilder().setKey( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER).setValue(Constants.WS_API_KEY_IDENTIFIER).build()) - .build()); + HeaderValueOption.newBuilder() + .setHeader(HeaderValue.newBuilder() + .setKey(HttpConstants.WEBSOCKET_PROTOCOL_HEADER) + .setValue(Constants.WS_API_KEY_IDENTIFIER).build()) + .build()); } // If the user is sending the APIKey credentials within query parameters, those query parameters should // not be sent to the backend. Hence, the :path header needs to be constructed again removing the apiKey From 335c63a54b587cf6e27c4b1d077aa710e55b37c9 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Fri, 2 Aug 2024 10:07:27 +0530 Subject: [PATCH 03/10] Add review changes --- .../jwt/InternalAPIKeyAuthenticator.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java index 57c28a9420..24fddb0e53 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java @@ -36,6 +36,7 @@ import org.wso2.choreo.connect.enforcer.config.EnforcerConfig; import org.wso2.choreo.connect.enforcer.constants.APIConstants; import org.wso2.choreo.connect.enforcer.constants.APISecurityConstants; +import org.wso2.choreo.connect.enforcer.constants.Constants; import org.wso2.choreo.connect.enforcer.constants.HttpConstants; import org.wso2.choreo.connect.enforcer.dto.APIKeyValidationInfoDTO; import org.wso2.choreo.connect.enforcer.dto.JWTTokenPayloadInfo; @@ -58,7 +59,6 @@ public class InternalAPIKeyAuthenticator extends APIKeyHandler { private static final Log log = LogFactory.getLog(InternalAPIKeyAuthenticator.class); private String securityParam; - private String secProtocolHeader = "sec-webSocket-protocol"; private AbstractAPIMgtGatewayJWTGenerator jwtGenerator; private final boolean isGatewayTokenCacheEnabled; @@ -72,20 +72,21 @@ public InternalAPIKeyAuthenticator(String securityParam) { } @Override - // check if sec-websocket-protocol header is there public boolean canAuthenticate(RequestContext requestContext) { String apiType = requestContext.getMatchedAPI().getApiType(); - String internalKey; + String internalKey = requestContext.getHeaders().get( + ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); if (apiType.equalsIgnoreCase("WS")) { - internalKey = requestContext.getHeaders().get( - ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); if (internalKey == null || internalKey.isBlank()) { - internalKey = requestContext.getHeaders().get(HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(",") [1]; + String[] secProtocolHeaderValues = requestContext.getHeaders().get( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); + if (secProtocolHeaderValues.length > 1 && secProtocolHeaderValues[0].equals( + Constants.WS_API_KEY_IDENTIFIER)) { + internalKey = secProtocolHeaderValues[1]; + } } - } else { - internalKey = requestContext.getHeaders().get( - ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); } + return isAPIKey(internalKey); } @@ -302,13 +303,17 @@ private String extractInternalKey(RequestContext requestContext) { return internalKey.trim(); } if (requestContext.getMatchedAPI().getApiType().equalsIgnoreCase("WS")) { - String secProtocolHeaderValue = requestContext.getHeaders().get(HttpConstants.WEBSOCKET_PROTOCOL_HEADER); - internalKey = secProtocolHeaderValue.split(",")[1]; + String[] secProtocolHeaderValues = requestContext.getHeaders().get( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); + if (secProtocolHeaderValues.length > 1 && secProtocolHeaderValues[0].equals( + Constants.WS_API_KEY_IDENTIFIER)) { + internalKey = secProtocolHeaderValues[1]; + } if (internalKey != null) { - if (secProtocolHeaderValue.split(",").length > 2) { + if (secProtocolHeaderValues.length > 2) { String protocols = Arrays.stream( - secProtocolHeaderValue.split(","), 2, - secProtocolHeaderValue.split(",").length) + secProtocolHeaderValues, 2, + secProtocolHeaderValues.length) .collect(Collectors.joining(",")); requestContext.addOrModifyHeaders(HttpConstants.WEBSOCKET_PROTOCOL_HEADER, protocols); } From f9b6ebc46eda50babfa7b383e3a1f9e53ffa373d Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Sun, 4 Aug 2024 16:50:45 +0530 Subject: [PATCH 04/10] Address review comments --- .../commons/model/RequestContext.java | 16 +++ .../connect/enforcer/api/ResponseObject.java | 9 ++ .../connect/enforcer/api/WebSocketAPI.java | 4 + .../connect/enforcer/grpc/ExtAuthService.java | 21 ++-- .../jwt/InternalAPIKeyAuthenticator.java | 17 ++- .../jwt/InternalAPIKeyAuthenticatiorTest.java | 115 ++++++++++++++++++ 6 files changed, 168 insertions(+), 14 deletions(-) create mode 100644 enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java diff --git a/enforcer-parent/commons/src/main/java/org/wso2/choreo/connect/enforcer/commons/model/RequestContext.java b/enforcer-parent/commons/src/main/java/org/wso2/choreo/connect/enforcer/commons/model/RequestContext.java index a5fb6b9347..b8a61bce04 100644 --- a/enforcer-parent/commons/src/main/java/org/wso2/choreo/connect/enforcer/commons/model/RequestContext.java +++ b/enforcer-parent/commons/src/main/java/org/wso2/choreo/connect/enforcer/commons/model/RequestContext.java @@ -74,6 +74,8 @@ public class RequestContext { // For example, reason for denying a request private String extAuthDetails; + private Map responseHeadersToAddMap; + /** * The dynamic metadata sent from enforcer are stored in this metadata map. * @return dynamic metadata map @@ -358,6 +360,20 @@ public void setExtAuthDetails(String extAuthDetails) { this.extAuthDetails = extAuthDetails; } + /** + * Specifies if headers needs to be added for the response based on request + * + * @return response headers to add map + */ + public Map getResponseHeadersToAddMap() { + return responseHeadersToAddMap; + } + + public void setResponseHeadersToAddMap(Map responseHeadersToAddMap) { + this.responseHeadersToAddMap = responseHeadersToAddMap; + } + + /** * Implements builder pattern to build an {@link RequestContext} object. */ diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/ResponseObject.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/ResponseObject.java index bd9349ca96..223d038995 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/ResponseObject.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/ResponseObject.java @@ -39,6 +39,7 @@ public class ResponseObject { private String requestPath; private String apiUuid; private String extAuthDetails; + private Map responseHeadersToAddMap; public ArrayList getRemoveHeaderMap() { return removeHeaderMap; @@ -48,6 +49,14 @@ public void setRemoveHeaderMap(ArrayList removeHeaderMap) { this.removeHeaderMap = removeHeaderMap; } + public Map getResponseHeadersToAddMap() { + return responseHeadersToAddMap; + } + + public void setResponseHeadersToAddMap(Map responseHeadersToAddMap) { + this.responseHeadersToAddMap = responseHeadersToAddMap; + } + public ResponseObject(String correlationID) { this.correlationID = correlationID; } diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java index f46f4f8ab1..42e45c2dc3 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java @@ -174,6 +174,10 @@ public ResponseObject process(RequestContext requestContext) { if (requestContext.getAddHeaders() != null && requestContext.getAddHeaders().size() > 0) { responseObject.setHeaderMap(requestContext.getAddHeaders()); } + if (requestContext.getResponseHeadersToAddMap() != null + && requestContext.getResponseHeadersToAddMap().size() > 0) { + responseObject.setResponseHeadersToAddMap(requestContext.getResponseHeadersToAddMap()); + } logger.debug("ext_authz metadata: {}", requestContext.getMetadataMap()); responseObject.setMetaDataMap(requestContext.getMetadataMap()); } else { diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java index b3f5183618..fa757f3c6d 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java @@ -37,7 +37,6 @@ import org.json.JSONObject; import org.wso2.choreo.connect.enforcer.api.ResponseObject; import org.wso2.choreo.connect.enforcer.constants.APIConstants; -import org.wso2.choreo.connect.enforcer.constants.Constants; import org.wso2.choreo.connect.enforcer.constants.HttpConstants; import org.wso2.choreo.connect.enforcer.constants.MetadataConstants; import org.wso2.choreo.connect.enforcer.constants.RouterAccessLogConstants; @@ -154,15 +153,7 @@ private CheckResponse buildResponse(CheckRequest request, ResponseObject respons .build(); } else { OkHttpResponse.Builder okResponseBuilder = OkHttpResponse.newBuilder(); - if (secProtocolHeaderForWS[0].equals(Constants.WS_API_KEY_IDENTIFIER) && - secProtocolHeaderForWS.length == 2) { - okResponseBuilder.addResponseHeadersToAdd( - HeaderValueOption.newBuilder() - .setHeader(HeaderValue.newBuilder() - .setKey(HttpConstants.WEBSOCKET_PROTOCOL_HEADER) - .setValue(Constants.WS_API_KEY_IDENTIFIER).build()) - .build()); - } + // If the user is sending the APIKey credentials within query parameters, those query parameters should // not be sent to the backend. Hence, the :path header needs to be constructed again removing the apiKey // query parameter. In this scenario, apiKey query parameter is sent within the property called @@ -186,6 +177,16 @@ private CheckResponse buildResponse(CheckRequest request, ResponseObject respons } ); } + + if (responseObject.getResponseHeadersToAddMap() != null) { + responseObject.getResponseHeadersToAddMap().forEach((key, value) -> { + HeaderValueOption headerValueOption = HeaderValueOption.newBuilder() + .setHeader(HeaderValue.newBuilder().setKey(key).setValue(value).build()) + .build(); + okResponseBuilder.addResponseHeadersToAdd(headerValueOption); + } + ); + } okResponseBuilder.addAllHeadersToRemove(responseObject.getRemoveHeaderMap()); if (responseObject.getMetaDataMap() != null) { responseObject.getMetaDataMap().forEach((key, value) -> diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java index 24fddb0e53..3bb5e934ca 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java @@ -50,6 +50,7 @@ import java.text.ParseException; import java.util.Arrays; +import java.util.HashMap; import java.util.stream.Collectors; /** @@ -75,16 +76,24 @@ public InternalAPIKeyAuthenticator(String securityParam) { public boolean canAuthenticate(RequestContext requestContext) { String apiType = requestContext.getMatchedAPI().getApiType(); String internalKey = requestContext.getHeaders().get( - ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); - if (apiType.equalsIgnoreCase("WS")) { - if (internalKey == null || internalKey.isBlank()) { - String[] secProtocolHeaderValues = requestContext.getHeaders().get( + ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); + if (apiType.equalsIgnoreCase("WS") && internalKey == null) { + String[] secProtocolHeaderValues = requestContext.getHeaders().get( HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); + if (internalKey == null || internalKey.isBlank()) { if (secProtocolHeaderValues.length > 1 && secProtocolHeaderValues[0].equals( Constants.WS_API_KEY_IDENTIFIER)) { internalKey = secProtocolHeaderValues[1]; } } + + if (secProtocolHeaderValues[0].equals(Constants.WS_API_KEY_IDENTIFIER) && + secProtocolHeaderValues.length == 2) { + HashMap responseHeadersToAddMap = new HashMap<>(); + responseHeadersToAddMap.put( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER, secProtocolHeaderValues[0]); + requestContext.setResponseHeadersToAddMap(responseHeadersToAddMap); + } } return isAPIKey(internalKey); diff --git a/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java b/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java new file mode 100644 index 0000000000..a7e6dffe52 --- /dev/null +++ b/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. + * + * WSO2 Inc. 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.wso2.choreo.connect.enforcer.security.jwt; + +import java.lang.reflect.InvocationTargetException; +import java.util.HashMap; +import java.util.Map; +import java.lang.reflect.Method; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.wso2.carbon.apimgt.common.gateway.dto.JWTConfigurationDto; +import org.wso2.choreo.connect.enforcer.commons.model.APIConfig; +import org.wso2.choreo.connect.enforcer.commons.model.RequestContext; +import org.wso2.choreo.connect.enforcer.config.ConfigHolder; +import org.wso2.choreo.connect.enforcer.config.EnforcerConfig; +import org.wso2.choreo.connect.enforcer.config.dto.CacheDto; +@RunWith(PowerMockRunner.class) +@PrepareForTest({ConfigHolder.class}) +@PowerMockIgnore("javax.management.*") +public class InternalAPIKeyAuthenticatiorTest { + + @Test + public void extractInternalKeyTest() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + PowerMockito.mockStatic(ConfigHolder.class); + ConfigHolder configHolder = Mockito.mock(ConfigHolder.class); + EnforcerConfig enforcerConfig = Mockito.mock(EnforcerConfig.class); + CacheDto cacheDto = Mockito.mock(CacheDto.class); + Mockito.when(cacheDto.isEnabled()).thenReturn(true); + Mockito.when(enforcerConfig.getCacheDto()).thenReturn(cacheDto); + JWTConfigurationDto jwtConfigurationDto = Mockito.mock(JWTConfigurationDto.class); + Mockito.when(jwtConfigurationDto.isEnabled()).thenReturn(false); + Mockito.when(enforcerConfig.getJwtConfigurationDto()).thenReturn(jwtConfigurationDto); + Mockito.when(configHolder.getConfig()).thenReturn(enforcerConfig); + Mockito.when(ConfigHolder.getInstance()).thenReturn(configHolder); + + String securityParam = "API-Key"; + + String mockToken = "eyJraWQiOiJnYXRld2F5XUlMyNTYifQlzaGVyXC92MlwvYXBpc1wvaW50ZXJuYlzaGVyXC92XBpc1wvaW50ZXJuY." + + "eyJzdWIiOiJhMzllYGV2OjQ0M1wvYXBpXC9hbVwvcHVibGlzaGVyXC92MlwvYXBpc1wvaW50ZXJuYWwta2V5Iiwia2V5dHlwZcl." + + "cnZpY2VcL3YxLjAiLCJwdWJsaXNoZXIiOiJjaG9yZW9fZGV2X2FwaW1fYWRtaW4iLCJ2ZXJzaW9uIjoidj7MIXRnS-2UWHdrmd7"; + + String secWebsocketProtocolHeader = "sec-websocket-protocol"; + + // Test case to test for an Upgrade request sent from the choreo console + // The token will be set to the sec-websocket-protocol header with c keyword + // the value after choreo-internal-API-Key will be the token + RequestContext.Builder builder = new RequestContext.Builder("/pets"); + builder.matchedAPI(new APIConfig.Builder("Petstore") + .basePath("/choreo") + .apiType("WS") + .build()); + Map headersMap = new HashMap<>(); + headersMap.put( + secWebsocketProtocolHeader, + "choreo-internal-API-Key, " + mockToken); + builder.headers(headersMap); + RequestContext requestContext = builder.build(); + InternalAPIKeyAuthenticator internalAPIKeyAuthenticator = new InternalAPIKeyAuthenticator(securityParam); + Method extractInternalKeyMethod = InternalAPIKeyAuthenticator.class.getDeclaredMethod( + "extractInternalKey",RequestContext.class); + extractInternalKeyMethod.setAccessible(true); + String internalKey = (String) extractInternalKeyMethod.invoke(internalAPIKeyAuthenticator, requestContext); + Assert.assertEquals(internalKey, mockToken); + + // test case to test for Upgrade request sent with the API-Key header set + RequestContext.Builder builder2 = new RequestContext.Builder("/pets"); + builder2.matchedAPI(new APIConfig.Builder("Petstore") + .basePath("/choreo") + .apiType("WS") + .build()); + Map headersMap2 = new HashMap<>(); + headersMap2.put(securityParam, mockToken); + headersMap2.put(secWebsocketProtocolHeader, "chat"); + builder2.headers(headersMap2); + RequestContext requestContext2 = builder.build(); + String internalKey2 = (String) extractInternalKeyMethod.invoke(internalAPIKeyAuthenticator, requestContext2); + Assert.assertEquals(internalKey2, mockToken); + + // Test case to test for normal REST API HTTP Request + RequestContext.Builder builder3 = new RequestContext.Builder("/pets"); + builder3.matchedAPI(new APIConfig.Builder("Petstore") + .basePath("/choreo") + .apiType("HTTP") + .build()); + Map headersMap3 = new HashMap<>(); + headersMap3.put(securityParam, mockToken); + builder3.headers(headersMap3); + RequestContext requestContext3 = builder.build(); + String internalKey3 = (String) extractInternalKeyMethod.invoke(internalAPIKeyAuthenticator, requestContext3); + Assert.assertEquals(internalKey3, mockToken); + } +} From e74c1c1ee7c5bd34cc2a3e8d075fd47f4dd75b0f Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Tue, 6 Aug 2024 17:19:05 +0530 Subject: [PATCH 05/10] Add changes with tests --- .azure/product-microgateway-sca-scan.yaml | 3 - .../jwt/InternalAPIKeyAuthenticator.java | 58 ++++++++----- ...a => InternalAPIKeyAuthenticatorTest.java} | 82 +++++++++++-------- 3 files changed, 86 insertions(+), 57 deletions(-) rename enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/{InternalAPIKeyAuthenticatiorTest.java => InternalAPIKeyAuthenticatorTest.java} (58%) diff --git a/.azure/product-microgateway-sca-scan.yaml b/.azure/product-microgateway-sca-scan.yaml index cf0b89fe42..7831511b6b 100644 --- a/.azure/product-microgateway-sca-scan.yaml +++ b/.azure/product-microgateway-sca-scan.yaml @@ -26,9 +26,6 @@ pr: - 'choreo' - 'main' -pool: - vmImage: 'ubuntu-latest' - resources: repositories: - repository: templates diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java index 3bb5e934ca..d2a4ea0d4b 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java @@ -78,20 +78,12 @@ public boolean canAuthenticate(RequestContext requestContext) { String internalKey = requestContext.getHeaders().get( ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); if (apiType.equalsIgnoreCase("WS") && internalKey == null) { - String[] secProtocolHeaderValues = requestContext.getHeaders().get( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); - if (internalKey == null || internalKey.isBlank()) { - if (secProtocolHeaderValues.length > 1 && secProtocolHeaderValues[0].equals( - Constants.WS_API_KEY_IDENTIFIER)) { - internalKey = secProtocolHeaderValues[1]; - } - } + internalKey = extractInternalKeyInWSProtocolHeader(requestContext); - if (secProtocolHeaderValues[0].equals(Constants.WS_API_KEY_IDENTIFIER) && - secProtocolHeaderValues.length == 2) { + if (isAdditionalResponseHeadersRequired(requestContext)) { HashMap responseHeadersToAddMap = new HashMap<>(); responseHeadersToAddMap.put( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER, secProtocolHeaderValues[0]); + HttpConstants.WEBSOCKET_PROTOCOL_HEADER, Constants.WS_API_KEY_IDENTIFIER); requestContext.setResponseHeadersToAddMap(responseHeadersToAddMap); } } @@ -312,18 +304,10 @@ private String extractInternalKey(RequestContext requestContext) { return internalKey.trim(); } if (requestContext.getMatchedAPI().getApiType().equalsIgnoreCase("WS")) { - String[] secProtocolHeaderValues = requestContext.getHeaders().get( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); - if (secProtocolHeaderValues.length > 1 && secProtocolHeaderValues[0].equals( - Constants.WS_API_KEY_IDENTIFIER)) { - internalKey = secProtocolHeaderValues[1]; - } + internalKey = extractInternalKeyInWSProtocolHeader(requestContext); if (internalKey != null) { - if (secProtocolHeaderValues.length > 2) { - String protocols = Arrays.stream( - secProtocolHeaderValues, 2, - secProtocolHeaderValues.length) - .collect(Collectors.joining(",")); + String protocols = getProtocolsToSetInRequestHeaders(requestContext); + if (protocols != null) { requestContext.addOrModifyHeaders(HttpConstants.WEBSOCKET_PROTOCOL_HEADER, protocols); } return internalKey.trim(); @@ -332,6 +316,36 @@ private String extractInternalKey(RequestContext requestContext) { return null; } + public String extractInternalKeyInWSProtocolHeader(RequestContext requestContext) { + String protocolHeader = requestContext.getHeaders().get( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER); + if (protocolHeader != null) { + String[] secProtocolHeaderValues = protocolHeader.split(","); + if (secProtocolHeaderValues.length > 1 && secProtocolHeaderValues[0].equals( + Constants.WS_API_KEY_IDENTIFIER)) { + return secProtocolHeaderValues[1].trim(); + } + } + return null; + } + + public String getProtocolsToSetInRequestHeaders(RequestContext requestContext) { + String[] secProtocolHeaderValues = requestContext.getHeaders().get( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); + if (secProtocolHeaderValues.length > 2) { + return Arrays.stream(secProtocolHeaderValues, 2, secProtocolHeaderValues.length) + .collect(Collectors.joining(",")).trim(); + } + return null; + } + + public boolean isAdditionalResponseHeadersRequired(RequestContext requestContext) { + String[] secProtocolHeaderValues = requestContext.getHeaders().get( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); + return secProtocolHeaderValues[0].equals(Constants.WS_API_KEY_IDENTIFIER) && + secProtocolHeaderValues.length == 2; + } + @Override public int getPriority() { return -10; diff --git a/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java b/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java similarity index 58% rename from enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java rename to enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java index a7e6dffe52..bfbfafe76f 100644 --- a/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatiorTest.java +++ b/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java @@ -18,10 +18,8 @@ package org.wso2.choreo.connect.enforcer.security.jwt; -import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.Map; -import java.lang.reflect.Method; import org.junit.Assert; import org.junit.Test; @@ -40,10 +38,10 @@ @RunWith(PowerMockRunner.class) @PrepareForTest({ConfigHolder.class}) @PowerMockIgnore("javax.management.*") -public class InternalAPIKeyAuthenticatiorTest { +public class InternalAPIKeyAuthenticatorTest { @Test - public void extractInternalKeyTest() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + public void extractInternalKeyInWSProtocolHeaderTest() { PowerMockito.mockStatic(ConfigHolder.class); ConfigHolder configHolder = Mockito.mock(ConfigHolder.class); EnforcerConfig enforcerConfig = Mockito.mock(EnforcerConfig.class); @@ -65,7 +63,7 @@ public void extractInternalKeyTest() throws NoSuchMethodException, InvocationTar String secWebsocketProtocolHeader = "sec-websocket-protocol"; // Test case to test for an Upgrade request sent from the choreo console - // The token will be set to the sec-websocket-protocol header with c keyword + // The token will be set to the sec-websocket-protocol header with choreo-internal-API-Key keyword // the value after choreo-internal-API-Key will be the token RequestContext.Builder builder = new RequestContext.Builder("/pets"); builder.matchedAPI(new APIConfig.Builder("Petstore") @@ -75,41 +73,61 @@ public void extractInternalKeyTest() throws NoSuchMethodException, InvocationTar Map headersMap = new HashMap<>(); headersMap.put( secWebsocketProtocolHeader, - "choreo-internal-API-Key, " + mockToken); + "choreo-internal-API-Key," + mockToken); builder.headers(headersMap); RequestContext requestContext = builder.build(); InternalAPIKeyAuthenticator internalAPIKeyAuthenticator = new InternalAPIKeyAuthenticator(securityParam); - Method extractInternalKeyMethod = InternalAPIKeyAuthenticator.class.getDeclaredMethod( - "extractInternalKey",RequestContext.class); - extractInternalKeyMethod.setAccessible(true); - String internalKey = (String) extractInternalKeyMethod.invoke(internalAPIKeyAuthenticator, requestContext); - Assert.assertEquals(internalKey, mockToken); + Assert.assertEquals(internalAPIKeyAuthenticator.extractInternalKeyInWSProtocolHeader(requestContext), mockToken); - // test case to test for Upgrade request sent with the API-Key header set + // Test case to test for an Upgrade request sent from a client with api-key RequestContext.Builder builder2 = new RequestContext.Builder("/pets"); builder2.matchedAPI(new APIConfig.Builder("Petstore") - .basePath("/choreo") - .apiType("WS") - .build()); + .basePath("/choreo") + .apiType("WS") + .build()); Map headersMap2 = new HashMap<>(); headersMap2.put(securityParam, mockToken); - headersMap2.put(secWebsocketProtocolHeader, "chat"); builder2.headers(headersMap2); - RequestContext requestContext2 = builder.build(); - String internalKey2 = (String) extractInternalKeyMethod.invoke(internalAPIKeyAuthenticator, requestContext2); - Assert.assertEquals(internalKey2, mockToken); - - // Test case to test for normal REST API HTTP Request - RequestContext.Builder builder3 = new RequestContext.Builder("/pets"); - builder3.matchedAPI(new APIConfig.Builder("Petstore") - .basePath("/choreo") - .apiType("HTTP") - .build()); - Map headersMap3 = new HashMap<>(); - headersMap3.put(securityParam, mockToken); - builder3.headers(headersMap3); - RequestContext requestContext3 = builder.build(); - String internalKey3 = (String) extractInternalKeyMethod.invoke(internalAPIKeyAuthenticator, requestContext3); - Assert.assertEquals(internalKey3, mockToken); + RequestContext requestContext2 = builder2.build(); + Assert.assertEquals(internalAPIKeyAuthenticator.extractInternalKeyInWSProtocolHeader(requestContext2), null); + + } + + @Test + public void getProtocolsToSetInRequestHeadersTest() { + PowerMockito.mockStatic(ConfigHolder.class); + ConfigHolder configHolder = Mockito.mock(ConfigHolder.class); + EnforcerConfig enforcerConfig = Mockito.mock(EnforcerConfig.class); + CacheDto cacheDto = Mockito.mock(CacheDto.class); + Mockito.when(cacheDto.isEnabled()).thenReturn(true); + Mockito.when(enforcerConfig.getCacheDto()).thenReturn(cacheDto); + JWTConfigurationDto jwtConfigurationDto = Mockito.mock(JWTConfigurationDto.class); + Mockito.when(jwtConfigurationDto.isEnabled()).thenReturn(false); + Mockito.when(enforcerConfig.getJwtConfigurationDto()).thenReturn(jwtConfigurationDto); + Mockito.when(configHolder.getConfig()).thenReturn(enforcerConfig); + Mockito.when(ConfigHolder.getInstance()).thenReturn(configHolder); + + String securityParam = "API-Key"; + + String secWebsocketProtocolHeader = "sec-websocket-protocol"; + + String mockToken = "eyJraWQiOiJnYXRld2F5XUlMyNTYifQlzaGVyXC92MlwvYXBpc1wvaW50ZXJuYlzaGVyXC92XBpc1wvaW50ZXJuY." + + "eyJzdWIiOiJhMzllYGV2OjQ0M1wvYXBpXC9hbVwvcHVibGlzaGVyXC92MlwvYXBpc1wvaW50ZXJuYWwta2V5Iiwia2V5dHlwZcl." + + "cnZpY2VcL3YxLjAiLCJwdWJsaXNoZXIiOiJjaG9yZW9fZGV2X2FwaW1fYWRtaW4iLCJ2ZXJzaW9uIjoidj7MIXRnS-2UWHdrmd7"; + + RequestContext.Builder builder = new RequestContext.Builder("/pets"); + builder.matchedAPI(new APIConfig.Builder("Petstore") + .basePath("/choreo") + .apiType("WS") + .build()); + Map headersMap = new HashMap<>(); + headersMap.put( + secWebsocketProtocolHeader, + "choreo-internal-API-Key, " + mockToken + ", " + "chat, bar"); + builder.headers(headersMap); + RequestContext requestContext = builder.build(); + InternalAPIKeyAuthenticator internalAPIKeyAuthenticator = new InternalAPIKeyAuthenticator(securityParam); + Assert.assertEquals(internalAPIKeyAuthenticator.getProtocolsToSetInRequestHeaders(requestContext), "chat, bar"); + } } From 7f491ade9c7f8102f3c14bbabcdd3ec73cd56ec1 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Wed, 7 Aug 2024 11:42:36 +0530 Subject: [PATCH 06/10] add review comments --- .../connect/enforcer/grpc/ExtAuthService.java | 2 -- .../jwt/InternalAPIKeyAuthenticator.java | 36 +++++++++++-------- .../jwt/InternalAPIKeyAuthenticatorTest.java | 2 +- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java index fa757f3c6d..1a030fbb77 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/grpc/ExtAuthService.java @@ -101,8 +101,6 @@ private CheckResponse buildResponse(CheckRequest request, ResponseObject respons DeniedHttpResponse.Builder responseBuilder = DeniedHttpResponse.newBuilder(); HttpStatus status = HttpStatus.newBuilder().setCodeValue(responseObject.getStatusCode()).build(); String traceKey = request.getAttributes().getRequest().getHttp().getId(); - String[] secProtocolHeaderForWS = request.getAttributes().getRequest().getHttp().getHeadersOrDefault( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER, "").split(","); Struct.Builder structBuilder = Struct.newBuilder(); // Used to identify that the choreo-connect-enforcer handled the request. It is used to // provide local reply for authentication failures. diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java index d2a4ea0d4b..0b4de10664 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java @@ -51,6 +51,7 @@ import java.text.ParseException; import java.util.Arrays; import java.util.HashMap; +import java.util.Map; import java.util.stream.Collectors; /** @@ -79,13 +80,7 @@ public boolean canAuthenticate(RequestContext requestContext) { ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); if (apiType.equalsIgnoreCase("WS") && internalKey == null) { internalKey = extractInternalKeyInWSProtocolHeader(requestContext); - - if (isAdditionalResponseHeadersRequired(requestContext)) { - HashMap responseHeadersToAddMap = new HashMap<>(); - responseHeadersToAddMap.put( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER, Constants.WS_API_KEY_IDENTIFIER); - requestContext.setResponseHeadersToAddMap(responseHeadersToAddMap); - } + addWSProtocolResponseHeaderIfRequired(requestContext); } return isAPIKey(internalKey); @@ -305,13 +300,13 @@ private String extractInternalKey(RequestContext requestContext) { } if (requestContext.getMatchedAPI().getApiType().equalsIgnoreCase("WS")) { internalKey = extractInternalKeyInWSProtocolHeader(requestContext); - if (internalKey != null) { + if (internalKey != null && !internalKey.isEmpty()) { String protocols = getProtocolsToSetInRequestHeaders(requestContext); if (protocols != null) { requestContext.addOrModifyHeaders(HttpConstants.WEBSOCKET_PROTOCOL_HEADER, protocols); } return internalKey.trim(); - } + } } return null; } @@ -326,7 +321,7 @@ public String extractInternalKeyInWSProtocolHeader(RequestContext requestContext return secProtocolHeaderValues[1].trim(); } } - return null; + return ""; } public String getProtocolsToSetInRequestHeaders(RequestContext requestContext) { @@ -339,11 +334,22 @@ public String getProtocolsToSetInRequestHeaders(RequestContext requestContext) { return null; } - public boolean isAdditionalResponseHeadersRequired(RequestContext requestContext) { - String[] secProtocolHeaderValues = requestContext.getHeaders().get( - HttpConstants.WEBSOCKET_PROTOCOL_HEADER).split(","); - return secProtocolHeaderValues[0].equals(Constants.WS_API_KEY_IDENTIFIER) && - secProtocolHeaderValues.length == 2; + public void addWSProtocolResponseHeaderIfRequired(RequestContext requestContext) { + String secProtocolHeader = requestContext.getHeaders().get(HttpConstants.WEBSOCKET_PROTOCOL_HEADER); + if (secProtocolHeader != null) { + String[] secProtocolHeaderValues = secProtocolHeader.split(","); + if (secProtocolHeaderValues[0].equals(Constants.WS_API_KEY_IDENTIFIER) && + secProtocolHeaderValues.length == 2) { + Map responseHeadersToAddMap = requestContext.getResponseHeadersToAddMap(); + + if (responseHeadersToAddMap == null) { + responseHeadersToAddMap = new HashMap<>(); + } + responseHeadersToAddMap.put( + HttpConstants.WEBSOCKET_PROTOCOL_HEADER, Constants.WS_API_KEY_IDENTIFIER); + requestContext.setResponseHeadersToAddMap(responseHeadersToAddMap); + } + } } @Override diff --git a/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java b/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java index bfbfafe76f..9bae7cd042 100644 --- a/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java +++ b/enforcer-parent/enforcer/src/test/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticatorTest.java @@ -89,7 +89,7 @@ public void extractInternalKeyInWSProtocolHeaderTest() { headersMap2.put(securityParam, mockToken); builder2.headers(headersMap2); RequestContext requestContext2 = builder2.build(); - Assert.assertEquals(internalAPIKeyAuthenticator.extractInternalKeyInWSProtocolHeader(requestContext2), null); + Assert.assertEquals(internalAPIKeyAuthenticator.extractInternalKeyInWSProtocolHeader(requestContext2), ""); } From c3f83d3ba4764d24bd35aa93907a67cbf75fa006 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Wed, 7 Aug 2024 14:24:33 +0530 Subject: [PATCH 07/10] revert wf file change --- .azure/product-microgateway-sca-scan.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.azure/product-microgateway-sca-scan.yaml b/.azure/product-microgateway-sca-scan.yaml index 7831511b6b..232e38d9d8 100644 --- a/.azure/product-microgateway-sca-scan.yaml +++ b/.azure/product-microgateway-sca-scan.yaml @@ -26,6 +26,10 @@ pr: - 'choreo' - 'main' +pool: + vmImage: 'ubuntu-latest' + + resources: repositories: - repository: templates From ddfac9f595aee0a0fe806a0f1a1d3004625c40ed Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Wed, 7 Aug 2024 14:25:01 +0530 Subject: [PATCH 08/10] remove line --- .azure/product-microgateway-sca-scan.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.azure/product-microgateway-sca-scan.yaml b/.azure/product-microgateway-sca-scan.yaml index 232e38d9d8..cf0b89fe42 100644 --- a/.azure/product-microgateway-sca-scan.yaml +++ b/.azure/product-microgateway-sca-scan.yaml @@ -29,7 +29,6 @@ pr: pool: vmImage: 'ubuntu-latest' - resources: repositories: - repository: templates From 83771d8616deb186897478d41d4b62bf71e013c2 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Fri, 9 Aug 2024 12:09:47 +0530 Subject: [PATCH 09/10] Fix passing apikey header to backend when security enabled --- .../org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java index 42e45c2dc3..2463686f63 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/api/WebSocketAPI.java @@ -170,6 +170,9 @@ public ResponseObject process(RequestContext requestContext) { Utils.populateRemoveAndProtectedHeaders(requestContext); if (executeFilterChain(requestContext)) { + responseObject.setRemoveHeaderMap(requestContext.getRemoveHeaders()); + responseObject.setQueryParamsToRemove(requestContext.getQueryParamsToRemove()); + responseObject.setQueryParamMap(requestContext.getQueryParameters()); responseObject.setStatusCode(APIConstants.StatusCodes.OK.getCode()); if (requestContext.getAddHeaders() != null && requestContext.getAddHeaders().size() > 0) { responseObject.setHeaderMap(requestContext.getAddHeaders()); From 6bb2e736e30ac54c6d9dda91098418eab774d763 Mon Sep 17 00:00:00 2001 From: Thushani Jayasekera Date: Fri, 9 Aug 2024 12:23:49 +0530 Subject: [PATCH 10/10] refactor for cases where both api key and ws protocol header is passed --- .../enforcer/security/jwt/InternalAPIKeyAuthenticator.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java index 0b4de10664..a7f128fd8d 100644 --- a/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java +++ b/enforcer-parent/enforcer/src/main/java/org/wso2/choreo/connect/enforcer/security/jwt/InternalAPIKeyAuthenticator.java @@ -78,8 +78,10 @@ public boolean canAuthenticate(RequestContext requestContext) { String apiType = requestContext.getMatchedAPI().getApiType(); String internalKey = requestContext.getHeaders().get( ConfigHolder.getInstance().getConfig().getAuthHeader().getTestConsoleHeaderName().toLowerCase()); - if (apiType.equalsIgnoreCase("WS") && internalKey == null) { - internalKey = extractInternalKeyInWSProtocolHeader(requestContext); + if (apiType.equalsIgnoreCase("WS")) { + if (internalKey == null) { + internalKey = extractInternalKeyInWSProtocolHeader(requestContext); + } addWSProtocolResponseHeaderIfRequired(requestContext); }