Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 添加平台公钥加密功能及补充文档 #2872

Merged
merged 11 commits into from
Jan 7, 2025
Merged

Conversation

lyt8384
Copy link
Contributor

@lyt8384 lyt8384 commented Jan 3, 2025

No description provided.

Copy link

vercel bot commented Jan 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
easywechat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 7, 2025 5:55am

@overtrue
Copy link
Collaborator

overtrue commented Jan 3, 2025

Ras 是啥意思?

@lyt8384
Copy link
Contributor Author

lyt8384 commented Jan 3, 2025

Ras 是啥意思?

Rsa,拼错了。手动尴尬

@@ -3,8 +3,8 @@
请仔细阅读并理解:[微信官方文档 - 微信支付](https://pay.weixin.qq.com/wiki/doc/apiv3/wxpay/pages/index.shtml)

> [!NOTE]
> 2024年Q3,微信支付官方开启了「平台公钥」平替「平台证书」方案,初始化所需的参数仅需配置上 **平台公钥ID** 及 **平台公钥** 即完全兼容支持,CLI/API下载 **平台证书** 已不是一个必要步骤,可略过。
> **平台公钥ID** 及 **平台公钥** 均可在 [微信支付商户平台](https://pay.weixin.qq.com/) -> 账户中心 -> API安全 查看及/或下载。
> 2024 年 Q3,微信支付官方开启了「平台公钥」平替「平台证书」方案,初始化所需的参数仅需配置上 **平台公钥 ID** 及 **平台公钥** 即完全兼容支持,CLI/API 下载 **平台证书** 已不是一个必要步骤,可略过。
Copy link
Contributor

@TheNorthMemory TheNorthMemory Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

参考 wechatpay-apiv3/wechatpay-php@f666a5d, 建议把 平台公钥 ID平台公钥 名词统一改成 微信支付公钥ID微信支付公钥。(作为客户端,其实微信支付官方他就是平台,他持有的公钥就是平台公钥,他们可能很忌讳用「平台」这个词儿吧)

> 2024年Q3,微信支付官方开启了「平台公钥」平替「平台证书」方案,初始化所需的参数仅需配置上 **平台公钥ID** 及 **平台公钥** 即完全兼容支持,CLI/API下载 **平台证书** 已不是一个必要步骤,可略过。
> **平台公钥ID** 及 **平台公钥** 均可在 [微信支付商户平台](https://pay.weixin.qq.com/) -> 账户中心 -> API安全 查看及/或下载。
> 2024 年 Q3,微信支付官方开启了「平台公钥」平替「平台证书」方案,初始化所需的参数仅需配置上 **平台公钥 ID** 及 **平台公钥** 即完全兼容支持,CLI/API 下载 **平台证书** 已不是一个必要步骤,可略过。
> **平台公钥 ID** 及 **平台公钥** 均可在 [微信支付商户平台](https://pay.weixin.qq.com/) -> 账户中心 -> API 安全 查看及/或下载。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

平台公钥 ID平台公钥 建议改成 微信支付公钥ID微信支付公钥

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

平台公钥 ID平台公钥 建议改成 微信支付公钥ID微信支付公钥

ide的问题把md文件格式化了,这个提交的内容还是你上次提交的

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗯,我昨天本来想PR这个来着,既然你提交了就一并改成一致完事。

ide的问题把md文件格式化了,这个提交的内容还是你上次提交的

Comment on lines 169 to 172
public function createRsaEncrypt(string $plaintext, ?string $serial = null): string
{
$platformCerts = $this->merchant->getPlatformCerts();
$platformCert = $serial ? $this->merchant->getPlatformCert($serial) : reset($platformCerts);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里可能有一点点小问题,客户端在加密敏感信息的时候,平台证书序列号/微信支付公钥ID是要随着请求头一并回送给服务端,自动选择第一个,没返回标识,请求时可能会造成不配对而产生异常,这种异常debug起来会非常困难。建议把第二型参用引用形式传递,这样就可以把自动选择的 标识 一并返回给上层。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最初我是想着通过传递类似 ->withEncodeValus([...]) 这种形式在发出请求前去处理加密字段和请求头的,后来发现搞复杂了,而且对多层map的形式兼容不太友善,就回归了这种简单的形式。
最初公钥ID是想着从配置里面直接取,后面发现没有单独配置公钥ID的key。然后一般情况下只会配置一个微信支付公钥,所以就考虑了取第一个。
第二个改成引用好像easywechat这边比较少这种调用模式,看看正超的意见?

ps。我倾向于增加多一个配置项 微信支付公钥ID

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

引用传递例如:

/**
 * @param string|null $serial The serial number of the platform certificate 
 *                           OR the plateform publicKeyId to use for encryption. 
 *                            If null, the first available certificate/plateform publicKey will be used.
 *                            Additionally, while passed it as of a reference, 
 *                            the serial number OR the plateform publicKeyId is filled to.
 */
public function createRsaEncrypt(string $plaintext, ?string &$serial = null): string
{
    $platformCerts = $this->merchant->getPlatformCerts();
    $platformCert = $serial ? $this->merchant->getPlatformCert($serial) : reset($platformCerts);
    is_null($serial) && $serial = key(platformCerts);
}

这是让函数返回两个值的一个方法,能解决从配置项去取pubKeyId/serailNo的困惑

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个我知道,但是这样下来整体风格不一致了。尤其在使用过程中,对多个参数加密的时候总感觉很怪。
我看了下好像整个框架之前没有引用传递这样出去的。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的 reset 改成 array_key_first?

$platformCerts = $this->merchant->getPlatformCerts();
$platformCert = $serial ? $this->merchant->getPlatformCert($serial) : reset($platformCerts);
if (empty($platformCert)) {
throw new InvalidConfigException('Missing platform certficate.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

单词拼写错了,是certificate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

惨,看来我还是把拼写纠错打开吧。哈哈

@lyt8384
Copy link
Contributor Author

lyt8384 commented Jan 6, 2025

@TheNorthMemory 我重新调整了下把微信支付公钥ID塞到了Merchant里,这样就不用把$serial引用回去了。但是有新的问题,单元测试不知道怎么写。

if (empty($platformCert)) {
throw new InvalidConfigException('Missing platform certificate.');
}

if (!openssl_public_encrypt($plaintext, $encrypted, $platformCert, OPENSSL_PKCS1_OAEP_PADDING)) {
throw new EncryptionFailureException('Encrypt failed.');
}

$this->merchant->setPlatformCertSerial($serial ?? key($platformCerts));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createRsaEncrypt 不应该有副作用去修改其它值,它应该是一个无污染的干净的加密函数

@overtrue
Copy link
Collaborator

overtrue commented Jan 6, 2025

感谢你的贡献,以下是我跟 @TheNorthMemory 讨论后的想法,供你参考:

  1. 设置请求头,这里可以考虑在 client 增加一个快捷方法:
     withSerialHeader(string $thing) // $thing 可以是序列号,也可以是公钥ID
  2. 加密的那个方法, 我认为应该是无状态的,它不需要依赖 merchant, 我们觉得它应该是这样:
    encryptWithRsaPublicKey(string $plaintext, string $serial)

总结:

这样改的好处是,不需要增加“默认公钥”这个东西在 Merchant 上,因为它是一个数组,设置它的作用仅用于加密,而不能用于解密,有点没必要了。 encryptWithRsaPublicKey 里,根据传入的 $serial$merchant->getPlatformCert($serial) 即可。逻辑直观,没有各种默认上下文导致误解。

@TheNorthMemory
Copy link
Contributor

不建议把 平台的公钥/证书 揉进商户配置里,可以参考这个 wechatpay-apiv3/wechatpay-guzzle-middleware#25 (comment) ,这Rsa加/解密上,我是犯过「过渡设计」的问题的,平台证书/公钥 加密的密文,商户私钥 是无法解密的(不配对),只有 平台私钥才能解密,配对信息其实就是 证书序列号 或者是 公钥ID

@lyt8384
Copy link
Contributor Author

lyt8384 commented Jan 6, 2025

感谢你的贡献,以下是我跟 @TheNorthMemory 讨论后的想法,供你参考:

  1. 设置请求头,这里可以考虑在 client 增加一个快捷方法:
     withSerialHeader(string $thing) // $thing 可以是序列号,也可以是公钥ID
    
  2. 加密的那个方法, 我认为应该是无状态的,它不需要依赖 merchant, 我们觉得它应该是这样:
    encryptWithRsaPublicKey(string $plaintext, string $serial)
    

总结:

这样改的好处是,不需要增加“默认公钥”这个东西在 Merchant 上,因为它是一个数组,设置它的作用仅用于加密,而不能用于解密,有点没必要了。 encryptWithRsaPublicKey 里,根据传入的 $serial$merchant->getPlatformCert($serial) 即可。逻辑直观,没有各种默认上下文导致误解。

最初我的设想其实就是这样的,但是发现一个使用过程中的问题,用户需要另外自行去维护这个公钥ID/证书ID,稍有不便。

@overtrue
Copy link
Collaborator

overtrue commented Jan 6, 2025

不需要维护啊,不是配置到数组了吗?

@overtrue
Copy link
Collaborator

overtrue commented Jan 6, 2025

你可以考虑 encryptWithRsaPublicKey 第二个参数默认取第一个,我觉得问题不大

@lyt8384
Copy link
Contributor Author

lyt8384 commented Jan 6, 2025

这样调整是否可行?
在 client 增加了withSerialHeader(?string $serial = null),不传$serial默认取第一个。
撤回了上一个提交的内容,createRsaEncrypt 确实不应该污染merchant

@overtrue overtrue merged commit 5152681 into w7corp:6.x Jan 7, 2025
5 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants