关于v2接口中间件强行要求验证sign字段问题

[dxkrs]

运行环境

php:7.4
wechatpay-php:1.4.2

描述你的问题现象

在使用此组件对接了大部分v3接口后，有些业务依然需要对接v2接口，比如“付款到零钱”接口。（https://pay.weixin.qq.com/wiki/doc/api/tools/mch_pay.php?chapter=14_2）

这类接口的响应数据中，并没有sign字段，但ClientXmlTrait.php中设置了一个guzzlehttp的中间件方法static::transformResponse()，该方法必须验证body中的sign字段，当验证不通过或者字段不存在，此时会抛出一个RejectionException的异常。这导致了，即使接口返回业务正常响应（例如已经真实付款到用户零钱），依然抛这个错。目前唯一可以解决的方法，是在RejectionException的抛错中通过getReason()方法来找到返回业务正常的响应。

提交了一个修改建议
#91

[TheNorthMemory]

添加这个 need_vefify 其实等于留了个后门，可能会被滥用到所有v2请求接口都被添加上，建议通过请求的 requestTarget 做内置白名单，符合白名单的URL自动忽略验签。

例如:

<?php
$securityUrls = [
    'mmpaymkttransfers/promotion/transfers',
];

if (in_array($request->getRequestTarget(), $securityUrls)) {
    return $response;
}

[TheNorthMemory]

另外，对于响应明确无须验签的，测试用例其实是给了例子，是可以通过请求时，传递特殊 handler 自动忽略验签，参考代码如下：

wechatpay-php/tests/OpenAPI/V2/Mmpaymkttransfers/Promotion/TransfersTest.php

Lines 42 to 45 in 3a64295

	/** @var HandlerStack $stack */
	$stack = $instance->getDriver()->select(ClientDecoratorInterface::XML_BASED)->getConfig('handler');
	$stack = clone $stack;
	$stack->remove('transform_response');

wechatpay-php/tests/OpenAPI/V2/Mmpaymkttransfers/Promotion/TransfersTest.php

Lines 94 to 96 in 3a64295

	// yes, start with `@` to prevent the internal `E_USER_DEPRECATED`
	$res = @$endpoint->post(['xml' => $data, 'handler' => $stack]);
	self::responseAssertion($res);

wechatpay-php/tests/OpenAPI/V2/Mmpaymkttransfers/Promotion/TransfersTest.php

Lines 135 to 140 in 3a64295

	// yes, start with `@` to prevent the internal `E_USER_DEPRECATED`
	@$endpoint->postAsync([
	'xml' => $data, 'handler' => $stack
	])->then(static function(ResponseInterface $res) {
	self::responseAssertion($res);
	})->wait();

[dxkrs]

“设置的白名单”应该是对同一个Cilent实例的全局管理，如果给使用组件者管理，在使用中维护不善依然有“后门”风险。
目前从最新的官网文档公示来看，依然有不少接口请求、查询的接口是不需要验证sign，何况可能存在一些旧项目依然使用已经无公示的v2接口（并且能正常调用），这么看的话把白名单写进组件里也不太现实。
通过传递handler来忽略验签确实是个好方法，也符合guzzlehttp的使用，但对组件使用者来说未免有点复杂了，而且传递特殊handler也是可以在工厂方法构造的时候传递，与“设置的白名单”有类似的“后门”风险。
既要安全作为首要考虑，又要简便、灵活使用，我新增了一个提交：把这个不验证标识改为只能在request的时候传递，才有效，在初始化Cilent实例（即工厂方法构造）中传递无效。

[TheNorthMemory]

试试这个diff，已知不需要验签的，无须请求(request)增加参数。

diff --git a/src/ClientXmlTrait.php b/src/ClientXmlTrait.php
index de02ef6..ed7c4c5 100644
--- a/src/ClientXmlTrait.php
+++ b/src/ClientXmlTrait.php
@@ -5,6 +5,7 @@ namespace WeChatPay;
 use function strlen;
 use function trigger_error;
 use function sprintf;
+use function in_array;
 
 use const E_USER_DEPRECATED;
 
@@ -30,6 +31,32 @@ trait ClientXmlTrait
         'Content-Type' => 'text/xml; charset=utf-8',
     ];
 
+    /**
+     * @var string[] - Special URLs whose were designed that none signature respond.
+     */
+    protected static $noneSignatureRespond = [
+        '/mchrisk/querymchrisk',
+        '/mchrisk/setmchriskcallback',
+        '/mchrisk/syncmchriskresult',
+        '/mmpaymkttransfers/gethbinfo',
+        '/mmpaymkttransfers/gettransferinfo',
+        '/mmpaymkttransfers/promotion/paywwsptrans2pocket',
+        '/mmpaymkttransfers/promotion/querywwsptrans2pocket',
+        '/mmpaymkttransfers/promotion/transfers',
+        '/mmpaymkttransfers/query_bank',
+        '/mmpaymkttransfers/sendgroupredpack',
+        '/mmpaymkttransfers/sendminiprogramhb',
+        '/mmpaymkttransfers/sendredpack',
+        '/pay/downloadbill',
+        '/pay/downloadfundflow',
+        '/payitil/report',
+        '/risk/getpublickey',
+        '/risk/getviolation',
+        '/sandboxnew/pay/getsignkey',
+        '/secapi/mch/submchmanage',
+        '/xdc/apiv2getsignkey/sign/getsignkey',
+    ];
+
     abstract protected static function body(MessageInterface $message): string;
 
     abstract protected static function withDefaults(array ...$config): array;
@@ -88,6 +115,10 @@ trait ClientXmlTrait
     {
         return static function (callable $handler) use ($secret): callable {
             return static function (RequestInterface $request, array $options = []) use ($secret, $handler): PromiseInterface {
+                if (in_array($request->getRequestTarget(), static::$noneSignatureRespond)) {
+                    return $handler($request, $options);
+                }
+
                 return $handler($request, $options)->then(static function(ResponseInterface $response) use ($secret) {
                     $result = Transformer::toArray(static::body($response));
 

[xy-peng]

同意 @TheNorthMemory 的观点，不应该提供给 SDK 使用方主动选择不验签的能力，从根本上避免安全隐患。例如，SDK 使用方因为验签不通过，干脆关闭了之。

目标应该是让 SDK 使用方不关注某个接口是否要验签，如何验签这些“琐碎”的事情，而集中精力放在业务逻辑上。所以 SDK 内置不验签接口清单的模式更合适。

[dxkrs]

我也赞同 @xy-peng 的说法，应该是要以安全为主，但如果真的要“从根本上避免安全隐患”，可能还需要禁止SDK使用方定制验签方式，例如在post(),postAsync()传递handler参数，或者是readme上面的定制节点介绍的方法，这些方法无疑是稍微复杂地“提供给 SDK 使用方主动选择不验签的能力”。

@TheNorthMemory 这个diff，由官方来维护v2接口不验签的接口，当然是最好，也很安全，期待能发布更新，但记得在工作备忘录中标记一下，以后有相关业务接口更新，也要及时发布此组件的一个新版本。

[TheNorthMemory]

无sign返回值，总感觉是不负责行为，即使是走https安全通道，也存在中间人攻击等行为，对v2接口做些维护工作，把这些“历史遗留缺sign问题”给补上，才是最好的方案。

[xy-peng]

无sign返回值，总感觉是不负责行为，即使是走https安全通道，也存在中间人攻击等行为，对v2接口做些维护工作，把这些“历史遗留缺sign问题”给补上，才是最好的方案。

最好的方案实际上可能困难重重。毕竟API是难以变更的，特别对于v2，增加参数对于一些实现比较极端的商户也是 BREAKING CHANGE。

比较现实的方案是：以白名单的方式不验签，并且不提供外部关闭验签的能力。