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

【新增】 protection 模块新增 signature 实现 API 签名 #532

Conversation

craftsman4j
Copy link
Contributor

No description provided.

@YunaiV
Copy link
Owner

YunaiV commented May 20, 2024

收到,最迟周末会 review 代码

/**
* 同一个请求多长时间内有效 默认 10分钟
*/
long expireTime() default 600000L;
Copy link
Owner

Choose a reason for hiding this comment

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

/**
 * 幂等的超时时间,默认为 1 秒
 *
 * 注意,如果执行时间超过它,请求还是会进来
 */
int timeout() default 1;
/**
 * 时间单位,默认为 SECONDS 秒
 */
TimeUnit timeUnit() default TimeUnit.SECONDS;

建议改成这个哈。

/**
* url 客户端不需要传递,但是可以用来加签(如: /{id} 带有动态参数的 url ,如果没有动态参数可设置为 false 不进行加签)
*/
boolean urlEnable() default true;
Copy link
Owner

Choose a reason for hiding this comment

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

这个字段,可以考虑删除。
因为大多数用不到,还是想尽量保持简洁哈。

@Documented
@Target({ElementType.METHOD, ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface Signature {
Copy link
Owner

Choose a reason for hiding this comment

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

要不所有类,都加一个 Api前缀?
例如说,@ApiSignature

因为它只适合 HTTP API 场景,不像其它是全局方法

// 校验 appId 是否能获取到对应的 appSecret
String appId = request.getHeader(signature.appId());
String appSecret = signatureRedisDAO.getAppSecret(appId);
Assert.notNull(appSecret, "找不到对应的 appSecret");
Copy link
Owner

Choose a reason for hiding this comment

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

断言的时候,把 appId 带上哈,不然不好排查问题

return false;
}
String nonce = request.getHeader(signature.nonce());
if (StrUtil.isBlank(nonce) || nonce.length() < 10) {
Copy link
Owner

Choose a reason for hiding this comment

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

StrUtil.length(nonce) < 10 更简单

@Before("@annotation(signature)")
public void beforePointCut(JoinPoint joinPoint, Signature signature) {
if (!verifySignature(signature, Objects.requireNonNull(ServletUtils.getRequest()))) {
log.info("[beforePointCut][方法{} 参数({}) 签名失败]", joinPoint.getSignature().toString(),
Copy link
Owner

Choose a reason for hiding this comment

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

是不是把这里的 log.info 日志,拆解到失败的场景里,打印日志,这样更容易排查问题

private String getRequestBody(HttpServletRequest request) {
CacheRequestBodyWrapper requestWrapper = new CacheRequestBodyWrapper(request);
// 获取 body
return new String(requestWrapper.getBody(), StandardCharsets.UTF_8);
Copy link
Owner

Choose a reason for hiding this comment

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

StrUtl.utf8Str
更简洁一点

@@ -23,6 +23,10 @@ public class CacheRequestBodyWrapper extends HttpServletRequestWrapper {
*/
private final byte[] body;

public byte[] getBody() {
Copy link
Owner

Choose a reason for hiding this comment

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

建议这里不要返回 body;

单测通过反射去拿下。框架封装,尽量收敛(少点属性)

@YunaiV
Copy link
Owner

YunaiV commented May 25, 2024

蛮好的,基本没啥问题了。

加了点简化代码的建议,和项目的代码风格更一致

@craftsman4j craftsman4j deleted the dev-0519-framework-signature branch June 6, 2024 07:28
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.

2 participants