diff --git a/pom.xml b/pom.xml index 8e9384c742..1178f64af1 100644 --- a/pom.xml +++ b/pom.xml @@ -144,6 +144,8 @@ 9.4.57.v20241219 1.85 2.3.3.RELEASE + + 1.2.13 diff --git a/weixin-java-common/src/main/java/me/chanjar/weixin/common/util/http/SensitiveRequestUtils.java b/weixin-java-common/src/main/java/me/chanjar/weixin/common/util/http/SensitiveRequestUtils.java new file mode 100644 index 0000000000..d23f046dbb --- /dev/null +++ b/weixin-java-common/src/main/java/me/chanjar/weixin/common/util/http/SensitiveRequestUtils.java @@ -0,0 +1,69 @@ +package me.chanjar.weixin.common.util.http; + +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import me.chanjar.weixin.common.error.WxError; +import me.chanjar.weixin.common.error.WxErrorException; +import me.chanjar.weixin.common.error.WxRuntimeException; + +/** + * Safe parameter and exception handling for requests containing credentials. + */ +public final class SensitiveRequestUtils { + private SensitiveRequestUtils() { + } + + /** + * Encodes one raw query parameter value, without interpreting existing percent escapes. + * + * @param value raw, non-null parameter value + * @return UTF-8 form-encoded value + * @throws NullPointerException if the value is null + */ + public static String encodeQueryValue(String value) { + try { + return URLEncoder.encode(value, StandardCharsets.UTF_8.name()); + } catch (UnsupportedEncodingException e) { + throw new IllegalStateException("UTF-8 is not available"); + } + } + + /** + * Retains the WeChat error code and stack frames without exposing response data or causes. + * This is intended for credential-bearing entry points, not general exception conversion. + * + * @param failure original failure + * @return safe exception without the original message, JSON, cause or suppressed exceptions + */ + public static WxErrorException sanitize(WxErrorException failure) { + WxErrorException safe = new WxErrorException(new WxError(failure.getError().getErrorCode(), + "Sensitive request failed")); + safe.setStackTrace(failure.getStackTrace()); + return safe; + } + + /** + * Removes request data from a runtime failure. Common argument, state and null failures + * keep their categories; other runtime failures become {@link WxRuntimeException}. + * Original exception class names and stack frames remain available for diagnosis. + * + * @param failure original failure + * @return safe exception without the original message, cause or suppressed exceptions + */ + public static RuntimeException sanitize(RuntimeException failure) { + String message = "Sensitive request failed (" + failure.getClass().getName() + ")"; + RuntimeException safe; + if (failure instanceof IllegalArgumentException) { + safe = new IllegalArgumentException(message); + } else if (failure instanceof IllegalStateException) { + safe = new IllegalStateException(message); + } else if (failure instanceof NullPointerException) { + safe = new NullPointerException(message); + } else { + safe = new WxRuntimeException(message); + } + safe.setStackTrace(failure.getStackTrace()); + return safe; + } +} diff --git a/weixin-java-common/src/test/java/me/chanjar/weixin/common/util/http/SensitiveRequestUtilsTest.java b/weixin-java-common/src/test/java/me/chanjar/weixin/common/util/http/SensitiveRequestUtilsTest.java new file mode 100644 index 0000000000..5c0822edc4 --- /dev/null +++ b/weixin-java-common/src/test/java/me/chanjar/weixin/common/util/http/SensitiveRequestUtilsTest.java @@ -0,0 +1,67 @@ +package me.chanjar.weixin.common.util.http; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.URLDecoder; +import me.chanjar.weixin.common.error.WxError; +import me.chanjar.weixin.common.error.WxErrorException; +import me.chanjar.weixin.common.error.WxRuntimeException; +import org.testng.annotations.Test; + +import static org.testng.Assert.*; + +public class SensitiveRequestUtilsTest { + @Test + public void encodesRawValuesExactlyOnce() throws Exception { + for (String value : new String[]{"NORMAL_CODE", "", "a b\n", "+&=#?%2F", "中文\uD83D\uDE00"}) { + String encoded = SensitiveRequestUtils.encodeQueryValue(value); + assertEquals(URLDecoder.decode(encoded, "UTF-8"), value); + assertFalse(encoded.contains("&")); + assertFalse(encoded.contains("#")); + } + assertEquals(SensitiveRequestUtils.encodeQueryValue("%2F"), "%252F"); + } + + @Test(expectedExceptions = NullPointerException.class) + public void doesNotConvertNullToAValue() { + SensitiveRequestUtils.encodeQueryValue(null); + } + + @Test + public void removesAllOriginalErrorRepresentations() { + WxError error = WxError.builder().errorCode(40029).errorMsg("FAKE_SECRET") + .errorMsgEn("FAKE_SECRET").json("FAKE_SECRET").build(); + WxErrorException original = new WxErrorException(error, new IllegalArgumentException("FAKE_SECRET")); + original.addSuppressed(new IllegalStateException("FAKE_SECRET")); + WxErrorException safe = SensitiveRequestUtils.sanitize(original); + assertEquals(safe.getError().getErrorCode(), 40029); + assertNull(safe.getError().getJson()); + assertNull(safe.getError().getErrorMsgEn()); + assertSafe(safe, original); + assertEquals(original.getError().getJson(), "FAKE_SECRET"); + } + + @Test + public void retainsCommonRuntimeCategoriesWithoutCauses() { + for (RuntimeException original : new RuntimeException[]{new IllegalArgumentException("FAKE_SECRET"), + new IllegalStateException("FAKE_SECRET"), new NullPointerException("FAKE_SECRET"), + new WxRuntimeException("FAKE_SECRET")}) { + original.initCause(new RuntimeException("FAKE_SECRET")); + original.addSuppressed(new RuntimeException("FAKE_SECRET")); + RuntimeException safe = SensitiveRequestUtils.sanitize(original); + assertEquals(safe.getClass(), original.getClass()); + assertSafe(safe, original); + } + assertTrue(SensitiveRequestUtils.sanitize(new UnsupportedOperationException("FAKE_SECRET")) + instanceof WxRuntimeException); + } + + private void assertSafe(Throwable safe, Throwable original) { + StringWriter trace = new StringWriter(); + safe.printStackTrace(new PrintWriter(trace)); + assertFalse(trace.toString().contains("FAKE_SECRET")); + assertNull(safe.getCause()); + assertEquals(safe.getSuppressed().length, 0); + assertEquals(safe.getStackTrace(), original.getStackTrace()); + } +} diff --git a/weixin-java-common/src/test/resources/testng.xml b/weixin-java-common/src/test/resources/testng.xml index a5c082f03b..a9064b8c7a 100644 --- a/weixin-java-common/src/test/resources/testng.xml +++ b/weixin-java-common/src/test/resources/testng.xml @@ -3,6 +3,7 @@ + diff --git a/weixin-java-miniapp/pom.xml b/weixin-java-miniapp/pom.xml index 2a4daa7469..3c6d0fd49d 100644 --- a/weixin-java-miniapp/pom.xml +++ b/weixin-java-miniapp/pom.xml @@ -53,6 +53,7 @@ ch.qos.logback logback-classic + ${logback-slf4j1-test.version} test diff --git a/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/WxMaService.java b/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/WxMaService.java index 4e001c6409..e2ae6b4236 100644 --- a/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/WxMaService.java +++ b/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/WxMaService.java @@ -38,6 +38,8 @@ public interface WxMaService extends WxService { /** * 获取登录后的 session 信息。 + * 登录参数按原始值传入，无需 URL 编码。为防止泄露凭证，失败异常保留错误码和栈帧， + * 不包含原始请求、响应、cause 或 suppressed 异常；其他运行时异常可能转换为 WxRuntimeException。 * * @param jsCode 登录时获取的 code * @return 登录 session 结果对象 diff --git a/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/impl/BaseWxMaServiceImpl.java b/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/impl/BaseWxMaServiceImpl.java index bf69439a65..49014318c4 100644 --- a/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/impl/BaseWxMaServiceImpl.java +++ b/weixin-java-miniapp/src/main/java/cn/binarywang/wx/miniapp/api/impl/BaseWxMaServiceImpl.java @@ -211,16 +211,22 @@ public String getPaidUnionId(String openid, String transactionId, String mchId, @Override public WxMaJscode2SessionResult jsCode2SessionInfo(String jsCode) throws WxErrorException { - final WxMaConfig config = getWxMaConfig(); - Map params = new HashMap<>(8); - params.put("appid", config.getAppid()); - params.put("secret", config.getSecret()); - params.put("js_code", jsCode); - params.put("grant_type", "authorization_code"); - - String result = - get(JSCODE_TO_SESSION_URL, Joiner.on("&").withKeyValueSeparator("=").join(params)); - return WxMaJscode2SessionResult.fromJson(result); + try { + final WxMaConfig config = getWxMaConfig(); + Map params = new HashMap<>(8); + params.put("appid", SensitiveRequestUtils.encodeQueryValue(config.getAppid())); + params.put("secret", SensitiveRequestUtils.encodeQueryValue(config.getSecret())); + params.put("js_code", SensitiveRequestUtils.encodeQueryValue(jsCode)); + params.put("grant_type", "authorization_code"); + + String result = + get(JSCODE_TO_SESSION_URL, Joiner.on("&").withKeyValueSeparator("=").join(params)); + return WxMaJscode2SessionResult.fromJson(result); + } catch (WxErrorException e) { + throw SensitiveRequestUtils.sanitize(e); + } catch (RuntimeException e) { + throw SensitiveRequestUtils.sanitize(e); + } } @Override @@ -390,7 +396,7 @@ private R executeWithRetry(ExecutorAction executor, String uri, String int retryTimes = 0; do { try { - return this.executeInternal(executor, uri, dataForLog, false); + return this.executeInternal(executor, uri, dataForLog, false, JSCODE_TO_SESSION_URL.equals(uri)); } catch (WxErrorException e) { if (retryTimes + 1 > this.maxRetryTimes) { log.warn("重试达到最大次数【{}】", maxRetryTimes); @@ -423,7 +429,8 @@ private R executeWithRetry(ExecutorAction executor, String uri, String } private R executeInternal( - ExecutorAction executor, String uri, String dataForLog, boolean doNotAutoRefreshToken) + ExecutorAction executor, String uri, String dataForLog, boolean doNotAutoRefreshToken, + boolean code2Session) throws WxErrorException { if (uri.contains("access_token=")) { @@ -440,7 +447,11 @@ private R executeInternal( uri + (uri.contains("?") ? "&" : "?") + "access_token=" + accessToken; try { R result = executor.execute(uriWithAccessToken); - log.debug("\n【请求地址】: {}\n【请求参数】：{}\n【响应数据】：{}", uriWithAccessToken, dataForLog, result); + if (code2Session) { + log.debug("code2Session request completed"); + } else { + log.debug("\n【请求地址】: {}\n【请求参数】：{}\n【响应数据】：{}", uriWithAccessToken, dataForLog, result); + } return result; } catch (WxErrorException e) { WxError error = e.getError(); @@ -459,15 +470,18 @@ private R executeInternal( } if (this.getWxMaConfig().autoRefreshToken() && !doNotAutoRefreshToken) { log.warn( - "即将重新获取新的access_token，错误代码：{}，错误信息：{}", error.getErrorCode(), error.getErrorMsg()); + "即将重新获取新的access_token，错误代码：{}，错误信息：{}", error.getErrorCode(), + code2Session ? "[redacted]" : error.getErrorMsg()); // 下一次不再自动重试 // 当小程序误调用第三方平台专属接口时,第三方无法使用小程序的access token,如果可以继续自动获取token会导致无限循环重试,直到栈溢出 - return this.executeInternal(executor, uri, dataForLog, true); + return this.executeInternal(executor, uri, dataForLog, true, code2Session); } } if (error.getErrorCode() != 0) { - if (error.getErrorCode() == WxMaErrorMsgEnum.CODE_43101.getCode()) { + if (code2Session) { + log.warn("code2Session request failed, error code: {}", error.getErrorCode()); + } else if (error.getErrorCode() == WxMaErrorMsgEnum.CODE_43101.getCode()) { // 43101 日志太多, 打印为debug, 其他情况打印为warn log.debug("\n【请求地址】: {}\n【请求参数】：{}\n【错误信息】：{}", uriWithAccessToken, dataForLog, error); } else { @@ -477,8 +491,12 @@ private R executeInternal( } return null; } catch (IOException e) { - log.warn( - "\n【请求地址】: {}\n【请求参数】：{}\n【异常信息】：{}", uriWithAccessToken, dataForLog, e.getMessage()); + if (code2Session) { + log.warn("code2Session request failed, exception type: {}", e.getClass().getName()); + } else { + log.warn( + "\n【请求地址】: {}\n【请求参数】：{}\n【异常信息】：{}", uriWithAccessToken, dataForLog, e.getMessage()); + } throw new WxRuntimeException(e); } } @@ -491,7 +509,7 @@ private R executeInternal( * @throws WxErrorException 异常 */ protected String extractAccessToken(String resultContent) throws WxErrorException { - log.debug("access-token response: {}", resultContent); + log.debug("access-token response received"); WxMaConfig config = this.getWxMaConfig(); WxError error = WxError.fromJson(resultContent, WxType.MiniApp); if (error.getErrorCode() != 0) { diff --git a/weixin-java-miniapp/src/test/java/cn/binarywang/wx/miniapp/api/impl/WxMaCode2SessionSecurityTest.java b/weixin-java-miniapp/src/test/java/cn/binarywang/wx/miniapp/api/impl/WxMaCode2SessionSecurityTest.java new file mode 100644 index 0000000000..90b3b30bb2 --- /dev/null +++ b/weixin-java-miniapp/src/test/java/cn/binarywang/wx/miniapp/api/impl/WxMaCode2SessionSecurityTest.java @@ -0,0 +1,297 @@ +package cn.binarywang.wx.miniapp.api.impl; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import cn.binarywang.wx.miniapp.config.impl.WxMaDefaultConfigImpl; +import com.sun.net.httpserver.HttpServer; +import java.io.Closeable; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.InetSocketAddress; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import me.chanjar.weixin.common.enums.WxType; +import me.chanjar.weixin.common.error.WxError; +import me.chanjar.weixin.common.error.WxErrorException; +import me.chanjar.weixin.common.error.WxRuntimeException; +import me.chanjar.weixin.common.util.http.RequestExecutor; +import org.slf4j.LoggerFactory; +import org.testng.annotations.Test; + +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; +import static org.testng.Assert.*; + +public class WxMaCode2SessionSecurityTest { + private static final String SECRET = "FAKE_APP_SECRET"; + private static final String TOKEN = "FAKE_ACCESS_TOKEN"; + private static final String SESSION = "FAKE_SESSION_KEY"; + private static final String RESULT = "{\"openid\":\"FAKE_OPENID\",\"session_key\":\"" + SESSION + "\"}"; + + @Test + public void allHttpClientsSendEncodedValuesAndPreserveSession() throws Exception { + AtomicReference query = new AtomicReference<>(); + AtomicReference response = new AtomicReference<>(RESULT); + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/sns/jscode2session", exchange -> { + query.set(exchange.getRequestURI().getRawQuery()); + byte[] body = response.get().getBytes(StandardCharsets.UTF_8); + exchange.sendResponseHeaders(200, body.length); + try (java.io.OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + }); + server.start(); + try (LogCapture logs = new LogCapture()) { + BaseWxMaServiceImpl[] services = {new WxMaServiceImpl(), new WxMaServiceHttpComponentsImpl(), + new WxMaServiceOkHttpImpl(), new WxMaServiceJoddHttpImpl()}; + for (BaseWxMaServiceImpl service : services) { + WxMaDefaultConfigImpl config = config(); + config.setAppid("FAKE_APP_ID+&=中文"); + config.setSecret(SECRET + "+&=中文\uD83D\uDE00"); + config.setApiHostUrl("http://127.0.0.1:" + server.getAddress().getPort()); + service.setWxMaConfig(config); + try { + response.set(RESULT); + for (String code : new String[]{"NORMAL_CODE", "a b\n", "a+&=#?%2F", "中文\uD83D\uDE00", ""}) { + assertEquals(service.jsCode2SessionInfo(code).getSessionKey(), SESSION); + Map params = decode(query.get()); + assertEquals(params.size(), 5); + assertEquals(params.get("js_code"), code); + assertEquals(params.get("secret"), config.getSecret()); + assertEquals(params.get("appid"), config.getAppid()); + assertEquals(params.get("access_token"), TOKEN); + } + response.set("{\"errcode\":40029,\"errmsg\":\"" + SECRET + TOKEN + "\"}"); + try { + service.jsCode2SessionInfo("FAKE_CODE"); + fail("Expected WeChat error"); + } catch (WxErrorException e) { + assertEquals(e.getError().getErrorCode(), 40029); + assertSafe(e); + } + } finally { + Object client = service.getRequestHttpClient(); + if (client instanceof Closeable) { + ((Closeable) client).close(); + } else if (client instanceof okhttp3.OkHttpClient) { + ((okhttp3.OkHttpClient) client).connectionPool().evictAll(); + ((okhttp3.OkHttpClient) client).dispatcher().executorService().shutdown(); + } + } + } + logs.assertSafe(); + } finally { + server.stop(0); + } + } + + @Test + public void errorsAndRetryLogsDoNotContainCredentials() throws Exception { + try (LogCapture logs = new LogCapture()) { + TestService service = service(); + service.getWxMaConfig().setApiHostUrl("http://proxy.invalid"); + when(service.executor.execute(anyString(), anyString(), eq(WxType.MiniApp))) + .thenThrow(error(40001)).thenThrow(error(-1)).thenReturn(RESULT); + assertEquals(service.jsCode2SessionInfo("FAKE_CODE").getSessionKey(), SESSION); + assertEquals(service.tokenRequests, 1); + verify(service.executor, times(3)).execute(startsWith("http://proxy.invalid/"), anyString(), eq(WxType.MiniApp)); + + for (Exception failure : new Exception[]{error(40029), new IOException(SECRET + TOKEN), + new IllegalArgumentException(SECRET + TOKEN), new IllegalStateException(SECRET + TOKEN)}) { + reset(service.executor); + when(service.executor.execute(anyString(), anyString(), eq(WxType.MiniApp))).thenThrow(failure); + try { + service.jsCode2SessionInfo("FAKE_CODE"); + fail("Expected failure"); + } catch (WxErrorException e) { + assertEquals(e.getError().getErrorCode(), 40029); + assertNull(e.getError().getJson()); + assertSafe(e); + } catch (RuntimeException e) { + assertSafe(e); + } + } + logs.assertSafe(); + } + } + + @Test + public void tokenAndParsingFailuresAreProtectedAndNullRemainsNullFailure() throws Exception { + try (LogCapture logs = new LogCapture()) { + TestService service = service(); + service.getWxMaConfig().expireAccessToken(); + when(service.executor.execute(anyString(), anyString(), eq(WxType.MiniApp))).thenReturn(RESULT); + assertEquals(service.jsCode2SessionInfo("FAKE_CODE").getSessionKey(), SESSION); + assertEquals(service.tokenRequests, 1); + when(service.executor.execute(anyString(), anyString(), eq(WxType.MiniApp))) + .thenReturn("{\"session_key\":\"" + SESSION + "\",\"expires_in\":"); + try { + service.jsCode2SessionInfo("FAKE_CODE"); + fail("Expected malformed response failure"); + } catch (WxRuntimeException e) { + assertSafe(e); + } + try { + service.jsCode2SessionInfo(null); + fail("Expected null input failure"); + } catch (NullPointerException e) { + assertSafe(e); + } + service.getWxMaConfig().expireAccessToken(); + service.failToken = true; + try { + service.jsCode2SessionInfo("FAKE_CODE"); + fail("Expected token failure"); + } catch (WxRuntimeException e) { + assertSafe(e); + } + logs.assertSafe(); + } + } + + @Test + public void ordinaryGetKeepsSerializedQueryAndExceptionContract() throws Exception { + TestService service = service(); + IllegalArgumentException original = new IllegalArgumentException("ordinary failure"); + when(service.executor.execute(anyString(), eq("value=a%2Fb"), eq(WxType.MiniApp))).thenThrow(original); + try { + service.get("https://api.weixin.qq.com/ordinary", "value=a%2Fb"); + fail("Expected failure"); + } catch (IllegalArgumentException e) { + assertSame(e, original); + } + } + + @Test + public void accountSwitchKeepsCredentialsWithTheirAccount() throws Exception { + TestService service = service(); + WxMaDefaultConfigImpl first = config(); + WxMaDefaultConfigImpl second = config(); + second.setAppid("SECOND_APP_ID"); + second.setSecret("SECOND_SECRET"); + second.updateAccessToken("SECOND_TOKEN", 7200); + Map configs = new HashMap<>(); + configs.put(first.getAppid(), first); + configs.put(second.getAppid(), second); + service.setMultiConfigs(configs, first.getAppid()); + when(service.executor.execute(anyString(), anyString(), eq(WxType.MiniApp))).thenAnswer(invocation -> { + Map params = decode(invocation.getArgument(1)); + String uri = invocation.getArgument(0); + if ("SECOND_APP_ID".equals(params.get("appid"))) { + assertEquals(params.get("secret"), "SECOND_SECRET"); + assertTrue(uri.endsWith("access_token=SECOND_TOKEN")); + } else { + assertEquals(params.get("secret"), SECRET); + assertTrue(uri.endsWith("access_token=" + TOKEN)); + } + return RESULT; + }); + assertEquals(service.jsCode2SessionInfo("FAKE_CODE").getSessionKey(), SESSION); + assertTrue(service.switchover(second.getAppid())); + assertEquals(service.jsCode2SessionInfo("FAKE_CODE").getSessionKey(), SESSION); + assertTrue(service.switchover(first.getAppid())); + assertEquals(service.jsCode2SessionInfo("FAKE_CODE").getSessionKey(), SESSION); + } + + private static WxMaDefaultConfigImpl config() { + WxMaDefaultConfigImpl config = new WxMaDefaultConfigImpl(); + config.setAppid("FAKE_APP_ID"); + config.setSecret(SECRET); + config.updateAccessToken(TOKEN, 7200); + return config; + } + + private TestService service() { + TestService service = new TestService(); + service.setWxMaConfig(config()); + service.setRetrySleepMillis(0); + return service; + } + + private WxErrorException error(int code) { + return new WxErrorException(WxError.builder().errorCode(code).errorMsg(SECRET + TOKEN) + .errorMsgEn(SECRET).json(SESSION).build(), new RuntimeException(SECRET)); + } + + private static Map decode(String query) throws Exception { + Map result = new HashMap<>(); + for (String pair : query.split("&")) { + String[] parts = pair.split("=", 2); + result.put(parts[0], URLDecoder.decode(parts[1], "UTF-8")); + } + return result; + } + + private static void assertSafe(Throwable e) { + StringWriter trace = new StringWriter(); + e.printStackTrace(new PrintWriter(trace)); + assertNoSecrets(trace.toString()); + assertNull(e.getCause()); + assertEquals(e.getSuppressed().length, 0); + } + + private static void assertNoSecrets(String text) { + for (String secret : new String[]{SECRET, TOKEN, SESSION, "FAKE_CODE", "FAKE_OPENID"}) { + assertFalse(text.contains(secret), "Credentials must not appear in diagnostics"); + } + } + + private static class LogCapture implements AutoCloseable { + private final Logger logger = (Logger) LoggerFactory.getLogger(BaseWxMaServiceImpl.class); + private final Level previous = logger.getLevel(); + private final ListAppender appender = new ListAppender<>(); + + LogCapture() { + logger.setLevel(Level.DEBUG); + appender.start(); + logger.addAppender(appender); + } + + void assertSafe() { + assertFalse(appender.list.isEmpty()); + for (ILoggingEvent event : appender.list) { + assertNoSecrets(event.getFormattedMessage()); + } + } + + @Override + public void close() { + logger.detachAppender(appender); + logger.setLevel(previous); + appender.stop(); + } + } + + private static class TestService extends WxMaServiceImpl { + @SuppressWarnings("unchecked") + private final RequestExecutor executor = mock(RequestExecutor.class); + private int tokenRequests; + private boolean failToken; + + @Override + public void initHttp() { + // The inherited execution/retry pipeline is exercised with a controlled transport. + } + + @Override + public String get(String url, String query) throws WxErrorException { + return (String) super.execute(executor, url, query); + } + + @Override + protected String doGetAccessTokenRequest() throws IOException { + tokenRequests++; + if (failToken) { + throw new IOException(SECRET + TOKEN); + } + return "{\"access_token\":\"" + TOKEN + "\",\"expires_in\":7200}"; + } + } +} diff --git a/weixin-java-miniapp/src/test/resources/testng.xml b/weixin-java-miniapp/src/test/resources/testng.xml index 9733604a1c..8d38decfe7 100644 --- a/weixin-java-miniapp/src/test/resources/testng.xml +++ b/weixin-java-miniapp/src/test/resources/testng.xml @@ -3,6 +3,7 @@ + diff --git a/weixin-java-open/pom.xml b/weixin-java-open/pom.xml index 509733caa4..2e3884bf9b 100644 --- a/weixin-java-open/pom.xml +++ b/weixin-java-open/pom.xml @@ -67,6 +67,11 @@ testng test + + org.mockito + mockito-core + test + com.google.inject guice @@ -94,6 +99,7 @@ ch.qos.logback logback-classic + ${logback-slf4j1-test.version} test diff --git a/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/WxOpenComponentService.java b/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/WxOpenComponentService.java index 9bb648b6c6..82a51d4919 100644 --- a/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/WxOpenComponentService.java +++ b/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/WxOpenComponentService.java @@ -496,6 +496,9 @@ public interface WxOpenComponentService { /** * Miniapp jscode 2 session wx ma jscode 2 session result. + * Pass raw parameter values, without URL encoding. Failures retain error codes and stack + * frames but omit raw responses, causes and suppressed exceptions to protect credentials. + * Other runtime exception subclasses may be converted to WxRuntimeException. * * @param appId the app id * @param jsCode the js code diff --git a/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenComponentServiceImpl.java b/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenComponentServiceImpl.java index 84f9190bb9..938356cf0e 100644 --- a/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenComponentServiceImpl.java +++ b/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenComponentServiceImpl.java @@ -15,6 +15,7 @@ import me.chanjar.weixin.common.error.WxRuntimeException; import me.chanjar.weixin.common.util.crypto.SHA1; import me.chanjar.weixin.common.util.http.URIUtil; +import me.chanjar.weixin.common.util.http.SensitiveRequestUtils; import me.chanjar.weixin.common.util.json.GsonParser; import me.chanjar.weixin.common.util.json.WxGsonBuilder; import me.chanjar.weixin.mp.api.WxMpService; @@ -272,7 +273,8 @@ public String get(String uri, String accessTokenKey) throws WxErrorException { lock.unlock(); } if (this.getWxOpenConfigStorage().autoRefreshToken()) { - log.warn("即将重新获取新的access_token，错误代码：{}，错误信息：{}", error.getErrorCode(), error.getErrorMsg()); + log.warn("即将重新获取新的access_token，错误代码：{}，错误信息：{}", error.getErrorCode(), + WxOpenServiceAbstractImpl.isSensitiveRequest(uri) ? "[redacted]" : error.getErrorMsg()); return this.get(uri, accessTokenKey); } } @@ -494,9 +496,18 @@ public String oauth2buildAuthorizationUrl(String appId, String redirectURI, Stri @Override public WxMaJscode2SessionResult miniappJscode2Session(String appId, String jsCode) throws WxErrorException { - String url = String.format(MINIAPP_JSCODE_2_SESSION, appId, jsCode, getWxOpenConfigStorage().getComponentAppId()); - String responseContent = get(url); - return WxMaJscode2SessionResult.fromJson(responseContent); + try { + String url = String.format(MINIAPP_JSCODE_2_SESSION, + SensitiveRequestUtils.encodeQueryValue(String.valueOf(appId)), + SensitiveRequestUtils.encodeQueryValue(String.valueOf(jsCode)), + SensitiveRequestUtils.encodeQueryValue(String.valueOf(getWxOpenConfigStorage().getComponentAppId()))); + String responseContent = get(url); + return WxMaJscode2SessionResult.fromJson(responseContent); + } catch (WxErrorException e) { + throw SensitiveRequestUtils.sanitize(e); + } catch (RuntimeException e) { + throw SensitiveRequestUtils.sanitize(e); + } } @Override diff --git a/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenServiceAbstractImpl.java b/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenServiceAbstractImpl.java index bad2241aa5..2ca81aca04 100644 --- a/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenServiceAbstractImpl.java +++ b/weixin-java-open/src/main/java/me/chanjar/weixin/open/api/impl/WxOpenServiceAbstractImpl.java @@ -43,20 +43,40 @@ public void setWxOpenConfigStorage(WxOpenConfigStorage wxOpenConfigStorage) { public abstract void initHttp(); protected T execute(RequestExecutor executor, String uri, E data) throws WxErrorException { + boolean sensitive = isSensitiveRequest(uri); try { T result = executor.execute(uri, data, WxType.Open); - log.debug("\n【请求地址】: {}\n【请求参数】：{}\n【响应数据】：{}", uri, data, result); + if (sensitive) { + log.debug("Sensitive request completed"); + } else { + log.debug("\n【请求地址】: {}\n【请求参数】：{}\n【响应数据】：{}", uri, data, result); + } return result; } catch (WxErrorException e) { WxError error = e.getError(); if (error.getErrorCode() != 0) { - log.warn("\n【请求地址】: {}\n【请求参数】：{}\n【错误信息】：{}", uri, data, error); + if (sensitive) { + log.warn("Sensitive request failed, error code: {}", error.getErrorCode()); + } else { + log.warn("\n【请求地址】: {}\n【请求参数】：{}\n【错误信息】：{}", uri, data, error); + } throw new WxErrorException(error, e); } return null; } catch (IOException e) { - log.warn("\n【请求地址】: {}\n【请求参数】：{}\n【异常信息】：{}", uri, data, e.getMessage()); + if (sensitive) { + log.warn("Sensitive request failed, exception type: {}", e.getClass().getName()); + } else { + log.warn("\n【请求地址】: {}\n【请求参数】：{}\n【异常信息】：{}", uri, data, e.getMessage()); + } throw new WxRuntimeException(e); } } + + static boolean isSensitiveRequest(String uri) { + String loginUrl = WxOpenComponentService.MINIAPP_JSCODE_2_SESSION.split("\\?", 2)[0]; + String tokenUrl = WxOpenComponentService.API_COMPONENT_TOKEN_URL; + return uri.equals(loginUrl) || uri.startsWith(loginUrl + "?") + || uri.equals(tokenUrl) || uri.startsWith(tokenUrl + "?"); + } } diff --git a/weixin-java-open/src/test/java/me/chanjar/weixin/open/api/impl/WxOpenCode2SessionSecurityTest.java b/weixin-java-open/src/test/java/me/chanjar/weixin/open/api/impl/WxOpenCode2SessionSecurityTest.java new file mode 100644 index 0000000000..8d468121a1 --- /dev/null +++ b/weixin-java-open/src/test/java/me/chanjar/weixin/open/api/impl/WxOpenCode2SessionSecurityTest.java @@ -0,0 +1,230 @@ +package me.chanjar.weixin.open.api.impl; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.URI; +import java.net.URLDecoder; +import java.util.HashMap; +import java.util.Map; +import me.chanjar.weixin.common.enums.WxType; +import me.chanjar.weixin.common.error.WxError; +import me.chanjar.weixin.common.error.WxErrorException; +import me.chanjar.weixin.common.error.WxRuntimeException; +import me.chanjar.weixin.common.util.http.RequestExecutor; +import me.chanjar.weixin.open.api.WxOpenComponentService; +import org.mockito.ArgumentCaptor; +import org.slf4j.LoggerFactory; +import org.testng.annotations.Test; + +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; +import static org.testng.Assert.*; + +public class WxOpenCode2SessionSecurityTest { + private static final String SECRET = "FAKE_COMPONENT_SECRET"; + private static final String TOKEN = "FAKE_COMPONENT_TOKEN"; + private static final String SESSION = "FAKE_SESSION_KEY"; + private static final String RESULT = "{\"openid\":\"FAKE_OPENID\",\"session_key\":\"" + SESSION + "\"}"; + + @Test + public void encodesParametersAndKeepsGetExtensionAndNullBehavior() throws Exception { + try (LogCapture logs = new LogCapture()) { + TestService service = service(); + WxOpenComponentService component = service.getWxOpenComponentService(); + for (String code : new String[]{"NORMAL_CODE", "a b\n", "a+&=#?%2F", "中文\uD83D\uDE00", "", null}) { + reset(service.executor); + when(service.executor.execute(anyString(), isNull(), eq(WxType.Open))).thenReturn(RESULT); + assertEquals(component.miniappJscode2Session("appid+&=", code).getSessionKey(), SESSION); + ArgumentCaptor uri = ArgumentCaptor.forClass(String.class); + verify(service.executor).execute(uri.capture(), isNull(), eq(WxType.Open)); + Map query = decode(URI.create(uri.getValue()).getRawQuery()); + assertEquals(query.size(), 5); + assertEquals(query.get("appid"), "appid+&="); + assertEquals(query.get("js_code"), String.valueOf(code)); + assertEquals(query.get("component_appid"), "component+&="); + assertEquals(query.get("component_access_token"), TOKEN); + } + logs.assertSafe(); + } + } + + @Test + public void refreshesTokenWithoutLoggingLoginOrCredentialPayloads() throws Exception { + try (LogCapture logs = new LogCapture()) { + TestService service = service(); + when(service.executor.execute(anyString(), nullable(String.class), eq(WxType.Open))) + .thenThrow(error(40001)) + .thenReturn("{\"component_access_token\":\"" + TOKEN + "\",\"expires_in\":7200}") + .thenReturn(RESULT); + assertEquals(service.getWxOpenComponentService().miniappJscode2Session("appid", "FAKE_CODE") + .getSessionKey(), SESSION); + verify(service.executor).execute(eq(WxOpenComponentService.API_COMPONENT_TOKEN_URL), + contains(SECRET), eq(WxType.Open)); + logs.assertSafe(); + } + } + + @Test + public void protectsWechatTransportAndResponseParsingFailures() throws Exception { + try (LogCapture logs = new LogCapture()) { + TestService service = service(); + for (Exception failure : new Exception[]{error(40029), new IOException(SECRET + TOKEN), + new IllegalArgumentException(SECRET + TOKEN), new IllegalStateException(SECRET + TOKEN)}) { + reset(service.executor); + when(service.executor.execute(anyString(), isNull(), eq(WxType.Open))).thenThrow(failure); + try { + service.getWxOpenComponentService().miniappJscode2Session("appid", "FAKE_CODE"); + fail("Expected failure"); + } catch (WxErrorException e) { + assertEquals(e.getError().getErrorCode(), 40029); + assertNull(e.getError().getJson()); + assertNull(e.getError().getErrorMsgEn()); + assertSafe(e); + } catch (RuntimeException e) { + assertSafe(e); + } + } + reset(service.executor); + when(service.executor.execute(anyString(), isNull(), eq(WxType.Open))) + .thenReturn("{\"session_key\":\"" + SESSION + "\",\"expires_in\":"); + try { + service.getWxOpenComponentService().miniappJscode2Session("appid", "FAKE_CODE"); + fail("Expected malformed response failure"); + } catch (WxRuntimeException e) { + assertSafe(e); + } + logs.assertSafe(); + } + } + + @Test + public void firstTokenRequestFailureIsProtected() throws Exception { + try (LogCapture logs = new LogCapture()) { + TestService service = service(); + service.getWxOpenConfigStorage().expireComponentAccessToken(); + when(service.executor.execute(anyString(), anyString(), eq(WxType.Open))) + .thenThrow(new IOException(SECRET + TOKEN)); + try { + service.getWxOpenComponentService().miniappJscode2Session("appid", "FAKE_CODE"); + fail("Expected token failure"); + } catch (WxRuntimeException e) { + assertSafe(e); + } + logs.assertSafe(); + } + } + + @Test + public void sensitiveEndpointMatchingDoesNotChangeOtherRequests() throws Exception { + assertTrue(WxOpenServiceAbstractImpl.isSensitiveRequest(WxOpenComponentService.API_COMPONENT_TOKEN_URL)); + assertTrue(WxOpenServiceAbstractImpl.isSensitiveRequest( + "https://api.weixin.qq.com/sns/component/jscode2session?appid=x")); + assertFalse(WxOpenServiceAbstractImpl.isSensitiveRequest( + "https://api.weixin.qq.com/sns/component/jscode2session-other?appid=x")); + TestService service = service(); + IllegalArgumentException original = new IllegalArgumentException("ordinary failure"); + when(service.executor.execute(anyString(), eq("value=a%2Fb"), eq(WxType.Open))).thenThrow(original); + try { + service.get("https://api.weixin.qq.com/ordinary", "value=a%2Fb"); + fail("Expected failure"); + } catch (IllegalArgumentException e) { + assertSame(e, original); + } + } + + private TestService service() { + TestService service = new TestService(); + WxOpenInMemoryConfigStorage config = new WxOpenInMemoryConfigStorage(); + config.setComponentAppId("component+&="); + config.setComponentAppSecret(SECRET); + config.setComponentVerifyTicket("FAKE_TICKET"); + config.updateComponentAccessToken(TOKEN, 7200); + service.setWxOpenConfigStorage(config); + return service; + } + + private WxErrorException error(int code) { + return new WxErrorException(WxError.builder().errorCode(code).errorMsg(SECRET + TOKEN) + .errorMsgEn(SECRET).json(SESSION).build(), new RuntimeException(SECRET)); + } + + private Map decode(String query) throws Exception { + Map result = new HashMap<>(); + for (String pair : query.split("&")) { + String[] parts = pair.split("=", 2); + result.put(parts[0], URLDecoder.decode(parts[1], "UTF-8")); + } + return result; + } + + private void assertSafe(Throwable e) { + StringWriter trace = new StringWriter(); + e.printStackTrace(new PrintWriter(trace)); + assertNoSecrets(trace.toString()); + assertNull(e.getCause()); + assertEquals(e.getSuppressed().length, 0); + } + + private static void assertNoSecrets(String text) { + for (String secret : new String[]{SECRET, TOKEN, SESSION, "FAKE_CODE", "FAKE_OPENID", "FAKE_TICKET"}) { + assertFalse(text.contains(secret), "Credentials must not appear in diagnostics"); + } + } + + private static class LogCapture implements AutoCloseable { + private final Logger[] loggers = {(Logger) LoggerFactory.getLogger(WxOpenServiceAbstractImpl.class), + (Logger) LoggerFactory.getLogger(WxOpenComponentServiceImpl.class)}; + private final Level[] previous = new Level[loggers.length]; + private final ListAppender appender = new ListAppender<>(); + + LogCapture() { + appender.start(); + for (int i = 0; i < loggers.length; i++) { + previous[i] = loggers[i].getLevel(); + loggers[i].setLevel(Level.DEBUG); + loggers[i].addAppender(appender); + } + } + + void assertSafe() { + assertFalse(appender.list.isEmpty()); + for (ILoggingEvent event : appender.list) { + assertNoSecrets(event.getFormattedMessage()); + } + } + + @Override + public void close() { + for (int i = 0; i < loggers.length; i++) { + loggers[i].detachAppender(appender); + loggers[i].setLevel(previous[i]); + } + appender.stop(); + } + } + + private static class TestService extends WxOpenServiceImpl { + @SuppressWarnings("unchecked") + private final RequestExecutor executor = mock(RequestExecutor.class); + + @Override + public void initHttp() { + // Keep the inherited component/token flow with a controlled transport. + } + + @Override + public String get(String uri, String data) throws WxErrorException { + return super.execute(executor, uri, data); + } + + @Override + public String post(String uri, String data) throws WxErrorException { + return super.execute(executor, uri, data); + } + } +} diff --git a/weixin-java-open/src/test/resources/testng.xml b/weixin-java-open/src/test/resources/testng.xml index 4fc5e6b52e..4599e3798c 100644 --- a/weixin-java-open/src/test/resources/testng.xml +++ b/weixin-java-open/src/test/resources/testng.xml @@ -10,6 +10,7 @@ +