From f7d1f5b13af7b0d9ff85f55aeb44723d4c69d7f0 Mon Sep 17 00:00:00 2001 From: Udara Pathum <46132469+hwupathum@users.noreply.github.com> Date: Tue, 7 May 2024 12:28:52 +0530 Subject: [PATCH 1/4] Add httpclient5 cache --- .../org.wso2.carbon.identity.captcha/pom.xml | 10 +++-- .../captcha/internal/CaptchaComponent.java | 17 +++++++++ .../captcha/internal/CaptchaDataHolder.java | 13 +++++++ .../identity/captcha/util/CaptchaUtil.java | 27 +++++++------- pom.xml | 37 +++++++++++++++---- 5 files changed, 80 insertions(+), 24 deletions(-) diff --git a/components/org.wso2.carbon.identity.captcha/pom.xml b/components/org.wso2.carbon.identity.captcha/pom.xml index 42c20cf05a..a68babcb09 100644 --- a/components/org.wso2.carbon.identity.captcha/pom.xml +++ b/components/org.wso2.carbon.identity.captcha/pom.xml @@ -47,11 +47,11 @@ org.wso2.orbit.org.apache.httpcomponents - httpclient + httpclient5 - org.apache.httpcomponents.wso2 - httpcore + org.wso2.orbit.org.apache.httpcomponents + httpcore5 com.google.code.findbugs @@ -81,6 +81,10 @@ org.wso2.carbon org.wso2.carbon.core + + org.wso2.carbon + org.wso2.carbon.http.client + org.wso2.carbon.identity.framework org.wso2.carbon.identity.core diff --git a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaComponent.java b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaComponent.java index 121bf38c29..8234f793c2 100644 --- a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaComponent.java +++ b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaComponent.java @@ -201,4 +201,21 @@ protected void unsetAccountLockService(AccountLockService accountLockService) { CaptchaDataHolder.getInstance().setAccountLockService(null); } + + @Reference( + name = "HttpClientService", + service = org.wso2.carbon.http.client.services.HttpClientService.class, + cardinality = ReferenceCardinality.MANDATORY, + policy = ReferencePolicy.DYNAMIC, + unbind = "unsetHttpClientService" + ) + protected void setHttpClientService(org.wso2.carbon.http.client.services.HttpClientService httpClientService) { + + CaptchaDataHolder.getInstance().setHttpClientService(httpClientService); + } + + protected void unsetHttpClientService(org.wso2.carbon.http.client.services.HttpClientService httpClientService) { + + CaptchaDataHolder.getInstance().setHttpClientService(null); + } } diff --git a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaDataHolder.java b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaDataHolder.java index 5b704e6a56..9cccab0d3a 100644 --- a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaDataHolder.java +++ b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/internal/CaptchaDataHolder.java @@ -18,6 +18,7 @@ package org.wso2.carbon.identity.captcha.internal; +import org.wso2.carbon.http.client.services.HttpClientService; import org.wso2.carbon.identity.captcha.connector.CaptchaConnector; import org.wso2.carbon.identity.governance.IdentityGovernanceService; import org.wso2.carbon.identity.handler.event.account.lock.service.AccountLockService; @@ -77,6 +78,8 @@ public class CaptchaDataHolder { private boolean forcefullyEnabledRecaptchaForAllTenants; + private HttpClientService httpClientService; + private CaptchaDataHolder() { } @@ -268,4 +271,14 @@ public void setForcefullyEnabledRecaptchaForAllTenants(boolean forcefullyEnabled this.forcefullyEnabledRecaptchaForAllTenants = forcefullyEnabledRecaptchaForAllTenants; } + + public HttpClientService getHttpClientService() { + + return httpClientService; + } + + public void setHttpClientService(HttpClientService httpClientService) { + + this.httpClientService = httpClientService; + } } diff --git a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java index 9ed2db14cf..89e67c400c 100644 --- a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java +++ b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java @@ -26,17 +26,17 @@ import org.apache.commons.lang.math.NumberUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.http.HttpEntity; -import org.apache.http.HttpHeaders; -import org.apache.http.HttpResponse; -import org.apache.http.NameValuePair; -import org.apache.http.entity.StringEntity; -import org.apache.http.client.entity.UrlEncodedFormEntity; -import org.apache.http.client.methods.HttpPost; -import org.apache.http.client.utils.URIBuilder; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.message.BasicNameValuePair; +import org.apache.hc.client5.http.classic.methods.HttpPost; +import org.apache.hc.client5.http.entity.UrlEncodedFormEntity; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; +import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.NameValuePair; +import org.apache.hc.core5.http.io.entity.StringEntity; +import org.apache.hc.core5.http.message.BasicNameValuePair; +import org.apache.hc.core5.net.URIBuilder; import org.wso2.carbon.context.PrivilegedCarbonContext; import org.wso2.carbon.identity.application.authentication.framework.ApplicationAuthenticator; import org.wso2.carbon.identity.application.authentication.framework.config.ConfigurationFacade; @@ -274,7 +274,8 @@ public static Map getClaimValues(User user, int tenantId, public static boolean isValidCaptcha(String reCaptchaResponse) throws CaptchaException { - CloseableHttpClient httpclient = HttpClientBuilder.create().useSystemProperties().build(); + CloseableHttpClient httpclient = + CaptchaDataHolder.getInstance().getHttpClientService().getClosableHttpClient(CaptchaUtil.class.getName()); String reCaptchaType = CaptchaDataHolder.getInstance().getReCaptchaType(); HttpPost httpPost; @@ -289,7 +290,7 @@ public static boolean isValidCaptcha(String reCaptchaResponse) throws CaptchaExc httpPost = createReCaptchaVerificationHttpPost(reCaptchaResponse); } - HttpResponse response; + CloseableHttpResponse response; try { response = httpclient.execute(httpPost); } catch (IOException e) { diff --git a/pom.xml b/pom.xml index 2fa1ee5429..55cf9b7cb4 100644 --- a/pom.xml +++ b/pom.xml @@ -146,12 +146,12 @@ org.wso2.orbit.org.apache.httpcomponents - httpclient + httpclient5 ${httpcomponents-httpclient.wso2.version} - org.apache.httpcomponents.wso2 - httpcore + org.wso2.orbit.org.apache.httpcomponents + httpcore5 ${httpcore.version} @@ -195,6 +195,21 @@ org.wso2.carbon org.wso2.carbon.core ${carbon.kernel.version} + + + org.apache.httpcomponents.wso2 + httpclient + + + org.apache.httpcomponents.wso2 + httpcore + + + + + org.wso2.carbon + org.wso2.carbon.http.client + ${carbon.kernel.version} org.wso2.carbon @@ -223,6 +238,12 @@ org.wso2.carbon.identity.framework org.wso2.carbon.identity.application.authentication.framework ${carbon.identity.framework.version} + + + org.wso2.orbit.org.apache.httpcomponents + httpclient + + org.wso2.carbon.identity.framework @@ -663,14 +684,14 @@ 3.0.0.wso2v1 [3.0.0.wso2v1, 4.0.0) 1.2.0.wso2v1 - 4.3.3.wso2v1 + 5.2.1.wso2v1 [4.3.3, 5.0.0) 2.6.0.wso2v1 [2.6.0,3.0.0) 2.5 1.2.5 - 4.3.6.wso2v2 - [4.3.1.wso2v2,5.0.0) + 5.2.1.wso2v1 + [5.2.1.wso2v1,6.0.0) 2.9.0 [2.3.1,3.0.0) @@ -688,8 +709,8 @@ 5.3.27 - 4.9.0 - 4.9.0 + 4.10.12-SNAPSHOT + 4.10.12-SNAPSHOT [4.5.0, 5.0.0) [1.0.1, 2.0.0) [1.0.1, 2.0.0) From cac3dda75c953cab5d148b9e079859c9664c5ef6 Mon Sep 17 00:00:00 2001 From: Udara Pathum <46132469+hwupathum@users.noreply.github.com> Date: Tue, 7 May 2024 18:20:06 +0530 Subject: [PATCH 2/4] Refactor CaptchaUtil --- .../org.wso2.carbon.identity.captcha/pom.xml | 7 +- .../identity/captcha/util/CaptchaUtil.java | 167 +++++++----------- .../captcha/util/CaptchaUtilTest.java | 48 ++--- 3 files changed, 83 insertions(+), 139 deletions(-) diff --git a/components/org.wso2.carbon.identity.captcha/pom.xml b/components/org.wso2.carbon.identity.captcha/pom.xml index a68babcb09..7c0e1200ac 100644 --- a/components/org.wso2.carbon.identity.captcha/pom.xml +++ b/components/org.wso2.carbon.identity.captcha/pom.xml @@ -165,10 +165,8 @@ org.apache.commons.lang; version="${commons-lang.wso2.osgi.version.range}", org.apache.commons.collections; version="${commons-collections.wso2.osgi.version.range}", org.apache.commons.logging.*; version="${commons-logging.osgi.version.range}", - org.apache.http.*, - org.apache.http.client.*;version="${httpcomponents-httpclient.imp.pkg.version.range}", - org.apache.http.impl.client.*;version="${httpcomponents-httpclient.imp.pkg.version.range}", - org.apache.http.message.*;version="${httpcore.osgi.version.range}", + org.apache.hc.client5.http.*;version="${httpcomponents-httpclient.imp.pkg.version.range}", + org.apache.hc.core5.*;version="${httpcore.osgi.version.range}", javax.servlet.*; version="${imp.pkg.version.javax.servlet}", @@ -203,6 +201,7 @@ org.wso2.carbon.context; version="${carbon.kernel.package.import.version.range}", org.wso2.carbon.identity.multi.attribute.login.mgt.*; version="${carbon.identity.framework.imp.pkg.version.range}", + org.wso2.carbon.http.client.*; version="${carbon.kernel.package.import.version.range}", diff --git a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java index 89e67c400c..23ed756ac9 100644 --- a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java +++ b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java @@ -18,26 +18,20 @@ package org.wso2.carbon.identity.captcha.util; -import com.google.gson.JsonElement; import com.google.gson.JsonObject; -import com.google.gson.JsonParser; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.math.NumberUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hc.client5.http.classic.methods.HttpPost; -import org.apache.hc.client5.http.entity.UrlEncodedFormEntity; -import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; -import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; -import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.NameValuePair; -import org.apache.hc.core5.http.io.entity.StringEntity; -import org.apache.hc.core5.http.message.BasicNameValuePair; import org.apache.hc.core5.net.URIBuilder; import org.wso2.carbon.context.PrivilegedCarbonContext; +import org.wso2.carbon.http.client.HttpClientConstants; +import org.wso2.carbon.http.client.exception.HttpClientException; +import org.wso2.carbon.http.client.handler.JsonResponseHandler; +import org.wso2.carbon.http.client.request.HttpPostRequest; import org.wso2.carbon.identity.application.authentication.framework.ApplicationAuthenticator; import org.wso2.carbon.identity.application.authentication.framework.config.ConfigurationFacade; import org.wso2.carbon.identity.application.authentication.framework.config.model.AuthenticatorConfig; @@ -65,7 +59,6 @@ import org.wso2.securevault.commons.MiscellaneousUtil; import java.io.IOException; -import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; import java.net.URISyntaxException; @@ -79,6 +72,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; + import javax.servlet.ServletRequest; import static org.wso2.carbon.identity.captcha.util.CaptchaConstants.BASIC_AUTH_MECHANISM; @@ -274,8 +268,6 @@ public static Map getClaimValues(User user, int tenantId, public static boolean isValidCaptcha(String reCaptchaResponse) throws CaptchaException { - CloseableHttpClient httpclient = - CaptchaDataHolder.getInstance().getHttpClientService().getClosableHttpClient(CaptchaUtil.class.getName()); String reCaptchaType = CaptchaDataHolder.getInstance().getReCaptchaType(); HttpPost httpPost; @@ -290,24 +282,27 @@ public static boolean isValidCaptcha(String reCaptchaResponse) throws CaptchaExc httpPost = createReCaptchaVerificationHttpPost(reCaptchaResponse); } - CloseableHttpResponse response; + JsonObject verificationResponse; try { - response = httpclient.execute(httpPost); + verificationResponse = CaptchaDataHolder.getInstance().getHttpClientService() + .getClosableHttpClient(CaptchaUtil.class.getName()).execute(httpPost, new JsonResponseHandler()); + } catch (HttpClientException e) { + if (HttpClientConstants.Error.RESPONSE_ENTITY_EMPTY.getCode().equals(e.getErrorCode())) { + throw new CaptchaServerException("reCaptcha verification response is not received."); + } + throw new CaptchaServerException("Unable to read the verification response.", e.getCause()); } catch (IOException e) { throw new CaptchaServerException("Unable to get the verification response.", e); } - - HttpEntity entity = response.getEntity(); - if (entity == null) { - throw new CaptchaServerException("reCaptcha verification response is not received."); + if (verificationResponse == null) { + throw new CaptchaClientException("Error receiving reCaptcha response from the server"); } - if (CaptchaConstants.RE_CAPTCHA_TYPE_ENTERPRISE.equals(reCaptchaType)) { // For ReCaptcha Enterprise. - verifyReCaptchaEnterpriseResponse(entity); + verifyReCaptchaEnterpriseResponse(verificationResponse); } else { // For Recaptcha v2 and v3. - verifyReCaptchaResponse(entity); + verifyReCaptchaResponse(verificationResponse); } return true; @@ -315,127 +310,97 @@ public static boolean isValidCaptcha(String reCaptchaResponse) throws CaptchaExc private static HttpPost createReCaptchaEnterpriseVerificationHttpPost(String reCaptchaResponse) { - HttpPost httpPost; String recaptchaUrl = CaptchaDataHolder.getInstance().getReCaptchaVerifyUrl(); String projectID = CaptchaDataHolder.getInstance().getReCaptchaProjectID(); String siteKey = CaptchaDataHolder.getInstance().getReCaptchaSiteKey(); String apiKey = CaptchaDataHolder.getInstance().getReCaptchaAPIKey(); String verifyUrl = recaptchaUrl + "/v1/projects/" + projectID + "/assessments?key=" + apiKey; - httpPost = new HttpPost(verifyUrl); - - httpPost.setHeader(HttpHeaders.CONTENT_TYPE, "application/json"); String json = String.format("{ \"event\": { \"token\": \"%s\", \"siteKey\": \"%s\" } }", reCaptchaResponse, siteKey); - StringEntity entity = new StringEntity(json, StandardCharsets.UTF_8); - - httpPost.setEntity(entity); - - return httpPost; + return HttpPostRequest.createJsonRequest(verifyUrl, json); } private static HttpPost createReCaptchaVerificationHttpPost(String reCaptchaResponse) { - HttpPost httpPost; - httpPost = new HttpPost(CaptchaDataHolder.getInstance().getReCaptchaVerifyUrl()); - List params = Arrays.asList(new BasicNameValuePair("secret", CaptchaDataHolder - .getInstance().getReCaptchaSecretKey()), - new BasicNameValuePair("response", reCaptchaResponse)); - httpPost.setEntity(new UrlEncodedFormEntity(params, StandardCharsets.UTF_8)); - - return httpPost; + Map params = new HashMap<>(); + params.put("secret", CaptchaDataHolder.getInstance().getReCaptchaSecretKey()); + params.put("response", reCaptchaResponse); + return HttpPostRequest.createUrlEncodedRequest(CaptchaDataHolder.getInstance().getReCaptchaVerifyUrl(), params); } - private static void verifyReCaptchaEnterpriseResponse(HttpEntity entity) + private static void verifyReCaptchaEnterpriseResponse(JsonObject verificationResponse) throws CaptchaServerException, CaptchaClientException { final double scoreThreshold = CaptchaDataHolder.getInstance().getReCaptchaScoreThreshold(); final double warnScoreThreshold = CaptchaDataHolder.getInstance().getReCaptchaWarnScoreThreshold(); try { - try (InputStream in = entity.getContent()) { - JsonElement jsonElement = JsonParser.parseReader(new InputStreamReader(in, StandardCharsets.UTF_8)); - JsonObject verificationResponse = jsonElement.getAsJsonObject(); - if (verificationResponse == null) { - throw new CaptchaClientException("Error receiving reCaptcha response from the server"); - } - JsonObject tokenProperties = verificationResponse.get(CaptchaConstants.CAPTCHA_TOKEN_PROPERTIES). - getAsJsonObject(); - boolean success = tokenProperties.get(CaptchaConstants.CAPTCHA_VALID).getAsBoolean(); + JsonObject tokenProperties = verificationResponse.get(CaptchaConstants.CAPTCHA_TOKEN_PROPERTIES). + getAsJsonObject(); + boolean success = tokenProperties.get(CaptchaConstants.CAPTCHA_VALID).getAsBoolean(); - JsonObject riskAnalysis = verificationResponse.get(CaptchaConstants.CAPTCHA_RISK_ANALYSIS). - getAsJsonObject(); + JsonObject riskAnalysis = verificationResponse.get(CaptchaConstants.CAPTCHA_RISK_ANALYSIS). + getAsJsonObject(); - // Whether this request was a valid reCAPTCHA token. - if (!success) { - throw new CaptchaClientException("reCaptcha token is invalid. Error:" + - verificationResponse.get("error-codes")); + // Whether this request was a valid reCAPTCHA token. + if (!success) { + throw new CaptchaClientException("reCaptcha token is invalid. Error:" + + verificationResponse.get("error-codes")); + } + if (riskAnalysis.get(CaptchaConstants.CAPTCHA_SCORE) != null) { + double score = riskAnalysis.get(CaptchaConstants.CAPTCHA_SCORE).getAsDouble(); + // reCAPTCHA enterprise response contains score. + if (log.isDebugEnabled()) { + log.debug("reCAPTCHA Enterprise response { timestamp:" + + tokenProperties.get("createTime") + ", action: " + + tokenProperties.get("action") + ", score: " + score + " }"); } - if (riskAnalysis.get(CaptchaConstants.CAPTCHA_SCORE) != null) { - double score = riskAnalysis.get(CaptchaConstants.CAPTCHA_SCORE).getAsDouble(); - // reCAPTCHA enterprise response contains score. - if (log.isDebugEnabled()) { - log.debug("reCAPTCHA Enterprise response { timestamp:" + - tokenProperties.get("createTime") + ", action: " + - tokenProperties.get("action") + ", score: " + score + " }"); - } - if (score < scoreThreshold) { - throw new CaptchaClientException("reCaptcha score is less than the threshold."); - } else if (score < warnScoreThreshold) { - log.warn("User access with low reCaptcha score."); - } + if (score < scoreThreshold) { + throw new CaptchaClientException("reCaptcha score is less than the threshold."); + } else if (score < warnScoreThreshold) { + log.warn("User access with low reCaptcha score."); } } - } catch (IOException e) { - throw new CaptchaServerException("Unable to read the verification response.", e); } catch (ClassCastException e) { throw new CaptchaServerException("Unable to cast the response value.", e); } } - private static void verifyReCaptchaResponse(HttpEntity entity) + private static void verifyReCaptchaResponse(JsonObject verificationResponse) throws CaptchaServerException, CaptchaClientException { final double scoreThreshold = CaptchaDataHolder.getInstance().getReCaptchaScoreThreshold(); final double warnScoreThreshold = CaptchaDataHolder.getInstance().getReCaptchaWarnScoreThreshold(); try { - try (InputStream in = entity.getContent()) { - JsonElement jsonElement = JsonParser.parseReader(new InputStreamReader(in, StandardCharsets.UTF_8)); - JsonObject verificationResponse = jsonElement.getAsJsonObject(); - if (verificationResponse == null) { - throw new CaptchaClientException("Error receiving reCaptcha response from the server"); + boolean success = verificationResponse.get(CaptchaConstants.CAPTCHA_SUCCESS).getAsBoolean(); + // Whether this request was a valid reCAPTCHA token. + if (!success) { + throw new CaptchaClientException("reCaptcha token is invalid. Error:" + + verificationResponse.get("error-codes")); + } + if (verificationResponse.get(CaptchaConstants.CAPTCHA_SCORE) != null) { + double score = verificationResponse.get(CaptchaConstants.CAPTCHA_SCORE).getAsDouble(); + // reCAPTCHA v3 response contains score. + if (log.isDebugEnabled()) { + log.debug("reCAPTCHA v3 response { timestamp:" + + verificationResponse.get("challenge_ts") + ", action: " + + verificationResponse.get("action") + ", score: " + score + " }"); } - boolean success = verificationResponse.get(CaptchaConstants.CAPTCHA_SUCCESS).getAsBoolean(); - // Whether this request was a valid reCAPTCHA token. - if (!success) { - throw new CaptchaClientException("reCaptcha token is invalid. Error:" + - verificationResponse.get("error-codes")); + if (score < scoreThreshold) { + throw new CaptchaClientException("reCaptcha score is less than the threshold."); + } else if (score < warnScoreThreshold) { + log.warn("reCaptcha score is below warn threshold."); } - if (verificationResponse.get(CaptchaConstants.CAPTCHA_SCORE) != null) { - double score = verificationResponse.get(CaptchaConstants.CAPTCHA_SCORE).getAsDouble(); - // reCAPTCHA v3 response contains score. - if (log.isDebugEnabled()) { - log.debug("reCAPTCHA v3 response { timestamp:" + - verificationResponse.get("challenge_ts") + ", action: " + - verificationResponse.get("action") + ", score: " + score + " }"); - } - if (score < scoreThreshold) { - throw new CaptchaClientException("reCaptcha score is less than the threshold."); - } else if (score < warnScoreThreshold) { - log.warn("reCaptcha score is below warn threshold."); - } - } else { - if (log.isDebugEnabled()) { - log.debug("reCAPTCHA v2 response { timestamp:" + - verificationResponse.get("challenge_ts") + " }"); - } + } else { + if (log.isDebugEnabled()) { + log.debug("reCAPTCHA v2 response { timestamp:" + + verificationResponse.get("challenge_ts") + " }"); } } - } catch (IOException e) { - throw new CaptchaServerException("Unable to read the verification response.", e); } catch (ClassCastException e) { throw new CaptchaServerException("Unable to cast the response value.", e); } diff --git a/components/org.wso2.carbon.identity.captcha/src/test/java/org/wso2/carbon/identity/captcha/util/CaptchaUtilTest.java b/components/org.wso2.carbon.identity.captcha/src/test/java/org/wso2/carbon/identity/captcha/util/CaptchaUtilTest.java index 0a8083d3c4..0585a19e32 100644 --- a/components/org.wso2.carbon.identity.captcha/src/test/java/org/wso2/carbon/identity/captcha/util/CaptchaUtilTest.java +++ b/components/org.wso2.carbon.identity.captcha/src/test/java/org/wso2/carbon/identity/captcha/util/CaptchaUtilTest.java @@ -17,20 +17,18 @@ package org.wso2.carbon.identity.captcha.util; import com.google.gson.JsonObject; -import org.apache.http.HttpEntity; -import org.apache.http.client.methods.HttpPost; -import org.mockito.Mockito; +import org.apache.hc.client5.http.classic.methods.HttpPost; import org.mockito.MockitoAnnotations; import org.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import org.wso2.carbon.identity.captcha.internal.CaptchaDataHolder; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.net.URISyntaxException; import static org.testng.Assert.assertThrows; @@ -67,7 +65,7 @@ private Method getCreateReCaptchaVerificationHttpPostMethod() throws NoSuchMetho private Method getVerifyReCaptchaEnterpriseResponseMethod() throws NoSuchMethodException { Method method = CaptchaUtil.class.getDeclaredMethod("verifyReCaptchaEnterpriseResponse", - HttpEntity.class); + JsonObject.class); method.setAccessible(true); return method; } @@ -75,7 +73,7 @@ private Method getVerifyReCaptchaEnterpriseResponseMethod() throws NoSuchMethodE private Method getVerifyReCaptchaResponseMethod() throws NoSuchMethodException { Method method = CaptchaUtil.class.getDeclaredMethod("verifyReCaptchaResponse", - HttpEntity.class); + JsonObject.class); method.setAccessible(true); return method; } @@ -102,7 +100,7 @@ private JsonObject getReCaptchaJsonObject(boolean valid, double score) { @Test (description = "This method is used to test the createReCaptchaEnterpriseVerificationHttpPost method") public void testCreateReCaptchaEnterpriseVerificationHttpPost() throws NoSuchMethodException, - InvocationTargetException, IllegalAccessException { + InvocationTargetException, IllegalAccessException, URISyntaxException { CaptchaDataHolder.getInstance().setReCaptchaVerifyUrl(RECAPTCHA_ENTERPRISE_API_URL); CaptchaDataHolder.getInstance().setReCaptchaAPIKey("dummyKey"); @@ -112,13 +110,13 @@ public void testCreateReCaptchaEnterpriseVerificationHttpPost() throws NoSuchMet Method method = getCreateReCaptchaEnterpriseVerificationHttpPostMethod(); HttpPost httpPost = (HttpPost) method.invoke(null, "reCaptchaEnterpriseResponse"); String expectedURI = RECAPTCHA_ENTERPRISE_API_URL+ "/v1/projects/dummyProjectId/assessments?key=dummyKey"; - Assert.assertEquals(httpPost.getURI().toString(), expectedURI); + Assert.assertEquals(httpPost.getUri().toString(), expectedURI); } @Test (description = "This method is used to test the createReCaptchaEnterpriseVerificationHttpPost method") public void testCreateReCaptchaVerificationHttpPost() throws NoSuchMethodException, - InvocationTargetException, IllegalAccessException { + InvocationTargetException, IllegalAccessException, URISyntaxException { CaptchaDataHolder.getInstance().setReCaptchaVerifyUrl(RECAPTCHA_API_URL); CaptchaDataHolder.getInstance().setReCaptchaSecretKey("dummyKey"); @@ -127,7 +125,7 @@ public void testCreateReCaptchaVerificationHttpPost() throws NoSuchMethodExcepti Method method = getCreateReCaptchaVerificationHttpPostMethod(); HttpPost httpPost = (HttpPost) method.invoke(null, "reCaptchaEnterpriseResponse"); - Assert.assertEquals(httpPost.getURI().toString(), RECAPTCHA_API_URL); + Assert.assertEquals(httpPost.getUri().toString(), RECAPTCHA_API_URL); } @@ -140,11 +138,8 @@ public void testVerifyReCaptchaEnterpriseResponseWithHighScore() throws IOExcept JsonObject verificationResponse = getReCaptchaEnterpriseJsonObject(true, 0.7); Method method = getVerifyReCaptchaEnterpriseResponseMethod(); - HttpEntity httpEntity = Mockito.mock(HttpEntity.class); - Mockito.when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream( - verificationResponse.toString().getBytes())); // Verify no exception is thrown for high score. - method.invoke(null, httpEntity); + method.invoke(null, verificationResponse); } @Test (description = "This method is used to test the verifyReCaptchaEnterpriseResponse method, " + @@ -155,11 +150,8 @@ public void testVerifyReCaptchaEnterpriseResponseWithLowScore() throws IOExcepti JsonObject verificationResponse = getReCaptchaEnterpriseJsonObject(true, 0.4); Method method = getVerifyReCaptchaEnterpriseResponseMethod(); - HttpEntity httpEntity = Mockito.mock(HttpEntity.class); - Mockito.when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream(verificationResponse.toString(). - getBytes())); // Verify an exception is thrown for low score. - assertThrows(InvocationTargetException.class, () -> method.invoke(null, httpEntity)); + assertThrows(InvocationTargetException.class, () -> method.invoke(null, verificationResponse)); } @Test (description = "This method is used to test the verifyReCaptchaEnterpriseResponse method, " + @@ -170,11 +162,8 @@ public void testVerifyReCaptchaEnterpriseResponseWithInvalidResponse() throws IO JsonObject verificationResponse = getReCaptchaEnterpriseJsonObject(false, 0.7); Method method = getVerifyReCaptchaEnterpriseResponseMethod(); - HttpEntity httpEntity = Mockito.mock(HttpEntity.class); - Mockito.when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream(verificationResponse. - toString().getBytes())); // Verify an exception is thrown for invalid response. - assertThrows(InvocationTargetException.class, () -> method.invoke(null, httpEntity)); + assertThrows(InvocationTargetException.class, () -> method.invoke(null, verificationResponse)); } @Test (description = "This method is used to test the verifyReCaptchaResponse method, " + @@ -186,11 +175,8 @@ public void testVerifyReCaptchaResponseWithHighScore() throws IOException, NoSuc JsonObject verificationResponse = getReCaptchaJsonObject(true, 0.7); Method method = getVerifyReCaptchaResponseMethod(); - HttpEntity httpEntity = Mockito.mock(HttpEntity.class); - Mockito.when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream(verificationResponse.toString(). - getBytes())); // Verify no exception is thrown for high score. - method.invoke(null, httpEntity); + method.invoke(null, verificationResponse); } @Test (description = "This method is used to test the verifyReCaptchaResponse method, " + @@ -201,11 +187,8 @@ public void testVerifyReCaptchaResponseWithLowScore() throws IOException, NoSuch JsonObject verificationResponse = getReCaptchaJsonObject(true, 0.4); Method method = getVerifyReCaptchaResponseMethod(); - HttpEntity httpEntity = Mockito.mock(HttpEntity.class); - Mockito.when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream(verificationResponse.toString(). - getBytes())); // Verify no exception is thrown for low score. - assertThrows(InvocationTargetException.class, () -> method.invoke(null, httpEntity)); + assertThrows(InvocationTargetException.class, () -> method.invoke(null, verificationResponse)); } @Test (description = "This method is used to test the verifyReCaptchaResponse method, " + @@ -216,10 +199,7 @@ public void testVerifyReCaptchaResponseWithInvalidResponse() throws IOException, JsonObject verificationResponse = getReCaptchaJsonObject(false, 0.7); Method method = getVerifyReCaptchaResponseMethod(); - HttpEntity httpEntity = Mockito.mock(HttpEntity.class); - Mockito.when(httpEntity.getContent()).thenReturn(new ByteArrayInputStream(verificationResponse.toString(). - getBytes())); // Verify no exception is thrown for invalid response. - assertThrows(InvocationTargetException.class, () -> method.invoke(null, httpEntity)); + assertThrows(InvocationTargetException.class, () -> method.invoke(null, verificationResponse)); } } From 165960278ac5a95ccb7432b6ebf62c5fe3d7f98e Mon Sep 17 00:00:00 2001 From: Udara Pathum <46132469+hwupathum@users.noreply.github.com> Date: Wed, 8 May 2024 14:35:37 +0530 Subject: [PATCH 3/4] Fix version ranges --- .../org.wso2.carbon.identity.captcha/pom.xml | 4 +- pom.xml | 38 +++++++++---------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/components/org.wso2.carbon.identity.captcha/pom.xml b/components/org.wso2.carbon.identity.captcha/pom.xml index 7c0e1200ac..b7efc95372 100644 --- a/components/org.wso2.carbon.identity.captcha/pom.xml +++ b/components/org.wso2.carbon.identity.captcha/pom.xml @@ -165,8 +165,8 @@ org.apache.commons.lang; version="${commons-lang.wso2.osgi.version.range}", org.apache.commons.collections; version="${commons-collections.wso2.osgi.version.range}", org.apache.commons.logging.*; version="${commons-logging.osgi.version.range}", - org.apache.hc.client5.http.*;version="${httpcomponents-httpclient.imp.pkg.version.range}", - org.apache.hc.core5.*;version="${httpcore.osgi.version.range}", + org.apache.hc.client5.http.*;version="${httpclient5.osgi.version.range}", + org.apache.hc.core5.*;version="${httpclient5.osgi.version.range}", javax.servlet.*; version="${imp.pkg.version.javax.servlet}", diff --git a/pom.xml b/pom.xml index 55cf9b7cb4..5882dea97b 100644 --- a/pom.xml +++ b/pom.xml @@ -146,13 +146,23 @@ org.wso2.orbit.org.apache.httpcomponents - httpclient5 + httpclient ${httpcomponents-httpclient.wso2.version} + + org.apache.httpcomponents.wso2 + httpcore + ${httpcore.version} + + + org.wso2.orbit.org.apache.httpcomponents + httpclient5 + ${httpclient5.version} + org.wso2.orbit.org.apache.httpcomponents httpcore5 - ${httpcore.version} + ${httpclient5.version} com.google.code.findbugs @@ -195,16 +205,6 @@ org.wso2.carbon org.wso2.carbon.core ${carbon.kernel.version} - - - org.apache.httpcomponents.wso2 - httpclient - - - org.apache.httpcomponents.wso2 - httpcore - - org.wso2.carbon @@ -238,12 +238,6 @@ org.wso2.carbon.identity.framework org.wso2.carbon.identity.application.authentication.framework ${carbon.identity.framework.version} - - - org.wso2.orbit.org.apache.httpcomponents - httpclient - - org.wso2.carbon.identity.framework @@ -684,14 +678,16 @@ 3.0.0.wso2v1 [3.0.0.wso2v1, 4.0.0) 1.2.0.wso2v1 - 5.2.1.wso2v1 + 4.3.3.wso2v1 [4.3.3, 5.0.0) + 5.2.1.wso2v1 + [5.2.1, 6.0.0) 2.6.0.wso2v1 [2.6.0,3.0.0) 2.5 1.2.5 - 5.2.1.wso2v1 - [5.2.1.wso2v1,6.0.0) + 4.3.6.wso2v2 + [4.3.1.wso2v2,5.0.0) 2.9.0 [2.3.1,3.0.0) From 3015e169118ce0d29fb93a8095adb0a26f12e144 Mon Sep 17 00:00:00 2001 From: Udara Pathum <46132469+hwupathum@users.noreply.github.com> Date: Mon, 27 May 2024 13:39:11 +0530 Subject: [PATCH 4/4] Refactor --- .../recovery/endpoint/Utils/RecoveryUtil.java | 24 ++++++++++++ .../endpoint/impl/CaptchaApiServiceImpl.java | 37 +++++++++++-------- .../identity/captcha/util/CaptchaUtil.java | 2 +- pom.xml | 4 +- 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/Utils/RecoveryUtil.java b/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/Utils/RecoveryUtil.java index 9aef837e70..96ea569202 100644 --- a/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/Utils/RecoveryUtil.java +++ b/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/Utils/RecoveryUtil.java @@ -12,6 +12,7 @@ import org.slf4j.MDC; import org.wso2.carbon.base.MultitenantConstants; import org.wso2.carbon.context.PrivilegedCarbonContext; +import org.wso2.carbon.http.client.request.HttpPostRequest; import org.wso2.carbon.identity.application.common.model.User; import org.wso2.carbon.identity.captcha.util.CaptchaConstants; import org.wso2.carbon.identity.core.util.IdentityTenantUtil; @@ -50,6 +51,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; @@ -403,7 +405,11 @@ private static Properties validateCaptchaConfigs(Properties properties) { * @param reCaptchaResponse ReCaptcha response token * @param properties ReCaptcha properties * @return httpResponse + * + * @deprecated Please use {@link #makeCaptchaVerificationHttpRequest(String reCaptchaResponseToken, + * Properties properties)} for httpclient 5.x version */ + @Deprecated public static HttpResponse makeCaptchaVerificationHttpRequest(ReCaptchaResponseTokenDTO reCaptchaResponse, Properties properties) { @@ -425,6 +431,24 @@ public static HttpResponse makeCaptchaVerificationHttpRequest(ReCaptchaResponseT return response; } + /** + * Make HTTP call for ReCaptcha Verification with the provided ReCaptcha response token. + * + * @param reCaptchaResponseToken ReCaptcha response token. + * @param properties ReCaptcha properties. + * @return HTTP Response + */ + public static org.apache.hc.client5.http.classic.methods.HttpPost makeCaptchaVerificationHttpRequest( + String reCaptchaResponseToken, Properties properties) { + + String reCaptchaSecretKey = properties.getProperty(CaptchaConstants.RE_CAPTCHA_SECRET_KEY); + String reCaptchaVerifyUrl = properties.getProperty(CaptchaConstants.RE_CAPTCHA_VERIFY_URL); + Map params = new HashMap<>(); + params.put("secret", reCaptchaSecretKey); + params.put("response", reCaptchaResponseToken); + return HttpPostRequest.createUrlEncodedRequest(reCaptchaVerifyUrl, params); + } + /** * Resolves site-key, secret-key and any other property if they are configured using secure vault. * diff --git a/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/impl/CaptchaApiServiceImpl.java b/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/impl/CaptchaApiServiceImpl.java index fd96ce7afa..f59fa9fe4f 100644 --- a/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/impl/CaptchaApiServiceImpl.java +++ b/components/org.wso2.carbon.identity.api.user.recovery/src/main/java/org/wso2/carbon/identity/recovery/endpoint/impl/CaptchaApiServiceImpl.java @@ -18,12 +18,14 @@ package org.wso2.carbon.identity.recovery.endpoint.impl; import com.google.gson.JsonObject; -import com.google.gson.JsonParser; -import org.apache.commons.io.IOUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.http.HttpEntity; -import org.apache.http.HttpResponse; +import org.apache.hc.client5.http.classic.methods.HttpPost; +import org.wso2.carbon.http.client.HttpClientConstants; +import org.wso2.carbon.http.client.exception.HttpClientException; +import org.wso2.carbon.http.client.handler.JsonResponseHandler; +import org.wso2.carbon.http.client.request.HttpPostRequest; +import org.wso2.carbon.identity.captcha.internal.CaptchaDataHolder; import org.wso2.carbon.identity.captcha.util.CaptchaConstants; import org.wso2.carbon.identity.recovery.endpoint.CaptchaApiService; import org.wso2.carbon.identity.recovery.endpoint.Constants; @@ -33,7 +35,8 @@ import org.wso2.carbon.identity.recovery.endpoint.dto.ReCaptchaVerificationResponseDTO; import java.io.IOException; -import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; import java.util.Properties; import javax.ws.rs.core.Response; @@ -78,24 +81,19 @@ public Response verifyCaptcha(ReCaptchaResponseTokenDTO reCaptchaResponse, Strin } Properties properties = RecoveryUtil.getValidatedCaptchaConfigs(); - boolean reCaptchaEnabled = Boolean.valueOf(properties.getProperty(CaptchaConstants.RE_CAPTCHA_ENABLED)); + boolean reCaptchaEnabled = Boolean.parseBoolean(properties.getProperty(CaptchaConstants.RE_CAPTCHA_ENABLED)); String reCaptchaType = properties.getProperty(CaptchaConstants.RE_CAPTCHA_TYPE); if (!reCaptchaEnabled) { RecoveryUtil.handleBadRequest("ReCaptcha is disabled", Constants.INVALID); } - HttpResponse response = RecoveryUtil.makeCaptchaVerificationHttpRequest(reCaptchaResponse, properties); - HttpEntity entity = response.getEntity(); + HttpPost httpPost = RecoveryUtil.makeCaptchaVerificationHttpRequest(reCaptchaResponse.getToken(), properties); ReCaptchaVerificationResponseDTO reCaptchaVerificationResponseDTO = new ReCaptchaVerificationResponseDTO(); - if (entity == null) { - RecoveryUtil.handleBadRequest("ReCaptcha verification response is not received.", - Constants.STATUS_INTERNAL_SERVER_ERROR_MESSAGE_DEFAULT); - } - try (InputStream in = entity.getContent()) { - JsonObject verificationResponse = new JsonParser().parse(IOUtils.toString(in)).getAsJsonObject(); - + try { + JsonObject verificationResponse = CaptchaDataHolder.getInstance().getHttpClientService() + .getClosableHttpClient(CaptchaApiServiceImpl.class.getName()).execute(httpPost, new JsonResponseHandler()); if (CaptchaConstants.RE_CAPTCHA_TYPE_ENTERPRISE.equals(reCaptchaType)) { // For Recaptcha Enterprise. JsonObject tokenProperties = verificationResponse.get(CaptchaConstants.CAPTCHA_TOKEN_PROPERTIES) @@ -107,10 +105,17 @@ public Response verifyCaptcha(ReCaptchaResponseTokenDTO reCaptchaResponse, Strin reCaptchaVerificationResponseDTO.setSuccess(verificationResponse.get( CaptchaConstants.CAPTCHA_SUCCESS).getAsBoolean()); } - } catch (IOException e) { + } catch (HttpClientException e) { + if (HttpClientConstants.Error.RESPONSE_ENTITY_EMPTY.getCode().equals(e.getErrorCode())) { + RecoveryUtil.handleBadRequest("ReCaptcha verification response is not received.", + Constants.STATUS_INTERNAL_SERVER_ERROR_MESSAGE_DEFAULT); + } log.error("Unable to read the verification response.", e); RecoveryUtil.handleBadRequest("Unable to read the verification response.", Constants.STATUS_INTERNAL_SERVER_ERROR_MESSAGE_DEFAULT); + } catch (IOException e) { + RecoveryUtil.handleBadRequest(String.format("Unable to get the verification response : %s", e.getMessage()), + Constants.STATUS_INTERNAL_SERVER_ERROR_MESSAGE_DEFAULT); } return Response.ok(reCaptchaVerificationResponseDTO).build(); diff --git a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java index 23ed756ac9..c593560bfd 100644 --- a/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java +++ b/components/org.wso2.carbon.identity.captcha/src/main/java/org/wso2/carbon/identity/captcha/util/CaptchaUtil.java @@ -290,7 +290,7 @@ public static boolean isValidCaptcha(String reCaptchaResponse) throws CaptchaExc if (HttpClientConstants.Error.RESPONSE_ENTITY_EMPTY.getCode().equals(e.getErrorCode())) { throw new CaptchaServerException("reCaptcha verification response is not received."); } - throw new CaptchaServerException("Unable to read the verification response.", e.getCause()); + throw new CaptchaServerException("Unable to read the verification response.", e); } catch (IOException e) { throw new CaptchaServerException("Unable to get the verification response.", e); } diff --git a/pom.xml b/pom.xml index 5882dea97b..5dbc35eeee 100644 --- a/pom.xml +++ b/pom.xml @@ -705,8 +705,8 @@ 5.3.27 - 4.10.12-SNAPSHOT - 4.10.12-SNAPSHOT + 4.10.17-SNAPSHOT + 4.10.17-SNAPSHOT [4.5.0, 5.0.0) [1.0.1, 2.0.0) [1.0.1, 2.0.0)