From 3d2c0ad35aeb118714c4402f72eaaeea87adce87 Mon Sep 17 00:00:00 2001 From: zichun Date: Fri, 15 May 2026 00:41:10 +0800 Subject: [PATCH] fix(usr): 修复 review round 1 must-fix REQ-USR-001 --- backend/src/main/java/com/xly/erp/common/exception/BizException.java | 8 ++++++++ backend/src/main/java/com/xly/erp/common/exception/GlobalExceptionHandler.java | 9 ++++++--- backend/src/main/java/com/xly/erp/common/response/Result.java | 5 +++++ backend/src/main/java/com/xly/erp/common/security/JwtUtil.java | 6 +++--- backend/src/main/java/com/xly/erp/module/usr/mapper/SysCompanyMapper.java | 3 ++- backend/src/main/java/com/xly/erp/module/usr/mapper/SysUserMapper.java | 34 +++++++++++++++++++++++++++++++++- backend/src/main/java/com/xly/erp/module/usr/service/impl/LoginServiceImpl.java | 49 +++++++++++++------------------------------------ backend/src/test/java/com/xly/erp/common/security/JwtUtilTest.java | 4 ++++ backend/src/test/java/com/xly/erp/module/usr/controller/AuthControllerTest.java | 5 +++-- backend/src/test/java/com/xly/erp/module/usr/service/LoginServiceImplTest.java | 32 ++++++++++++++++++++++++++++++++ 10 files changed, 109 insertions(+), 46 deletions(-) diff --git a/backend/src/main/java/com/xly/erp/common/exception/BizException.java b/backend/src/main/java/com/xly/erp/common/exception/BizException.java index 3391bc0..cf7fa22 100644 --- a/backend/src/main/java/com/xly/erp/common/exception/BizException.java +++ b/backend/src/main/java/com/xly/erp/common/exception/BizException.java @@ -9,14 +9,22 @@ import lombok.Getter; @Getter public class BizException extends RuntimeException { private final int code; + /** 可选附带的响应数据（例如 42301 锁定返 lockUntil）。null 表示无 data 字段。 */ + private final Object data; public BizException(int code, String message) { + this(code, message, (Object) null); + } + + public BizException(int code, String message, Object data) { super(message); this.code = code; + this.data = data; } public BizException(int code, String message, Throwable cause) { super(message, cause); this.code = code; + this.data = null; } } diff --git a/backend/src/main/java/com/xly/erp/common/exception/GlobalExceptionHandler.java b/backend/src/main/java/com/xly/erp/common/exception/GlobalExceptionHandler.java index 8bc6a3b..39902a6 100644 --- a/backend/src/main/java/com/xly/erp/common/exception/GlobalExceptionHandler.java +++ b/backend/src/main/java/com/xly/erp/common/exception/GlobalExceptionHandler.java @@ -19,11 +19,14 @@ import org.springframework.web.bind.annotation.RestControllerAdvice; public class GlobalExceptionHandler { @ExceptionHandler(BizException.class) - public ResponseEntity> handleBiz(BizException e) { - log.warn("[BizException] code={} message={}", e.getCode(), e.getMessage()); + public ResponseEntity> handleBiz(BizException e) { + log.warn("[BizException] code={} message={} hasData={}", e.getCode(), e.getMessage(), e.getData() != null); + Result body = e.getData() != null + ? Result.fail(e.getCode(), e.getMessage(), e.getData()) + : Result.fail(e.getCode(), e.getMessage()); return ResponseEntity .status(ErrorCode.toHttpStatus(e.getCode())) - .body(Result.fail(e.getCode(), e.getMessage())); + .body(body); } @ExceptionHandler(MethodArgumentNotValidException.class) diff --git a/backend/src/main/java/com/xly/erp/common/response/Result.java b/backend/src/main/java/com/xly/erp/common/response/Result.java index e76b181..c7b2869 100644 --- a/backend/src/main/java/com/xly/erp/common/response/Result.java +++ b/backend/src/main/java/com/xly/erp/common/response/Result.java @@ -31,4 +31,9 @@ public class Result { public static Result fail(int code, String message) { return new Result<>(code, message, null); } + + @SuppressWarnings("unchecked") + public static Result fail(int code, String message, T data) { + return new Result<>(code, message, data); + } } diff --git a/backend/src/main/java/com/xly/erp/common/security/JwtUtil.java b/backend/src/main/java/com/xly/erp/common/security/JwtUtil.java index 5320200..266d843 100644 --- a/backend/src/main/java/com/xly/erp/common/security/JwtUtil.java +++ b/backend/src/main/java/com/xly/erp/common/security/JwtUtil.java @@ -33,9 +33,9 @@ public class JwtUtil { void init() { byte[] bytes = secret.getBytes(StandardCharsets.UTF_8); if (bytes.length < 32) { - byte[] padded = new byte[32]; - System.arraycopy(bytes, 0, padded, 0, bytes.length); - bytes = padded; + throw new IllegalStateException( + "JWT_SECRET 长度不足 32 字节（HS256 要求），实际 " + bytes.length + + " 字节。请在 .env.local 配置至少 256 位的随机字符串。"); } this.key = Keys.hmacShaKeyFor(bytes); } diff --git a/backend/src/main/java/com/xly/erp/module/usr/mapper/SysCompanyMapper.java b/backend/src/main/java/com/xly/erp/module/usr/mapper/SysCompanyMapper.java index 6705b51..45fa5c0 100644 --- a/backend/src/main/java/com/xly/erp/module/usr/mapper/SysCompanyMapper.java +++ b/backend/src/main/java/com/xly/erp/module/usr/mapper/SysCompanyMapper.java @@ -8,6 +8,7 @@ import org.apache.ibatis.annotations.Select; @Mapper public interface SysCompanyMapper extends BaseMapper { - @Select("SELECT * FROM sys_company WHERE sCompanyCode = #{code} LIMIT 1") + @Select("SELECT iIncrement, sCompanyCode, sCompanyName, iIsDeleted " + + "FROM sys_company WHERE sCompanyCode = #{code} LIMIT 1") SysCompany selectByCode(String code); } diff --git a/backend/src/main/java/com/xly/erp/module/usr/mapper/SysUserMapper.java b/backend/src/main/java/com/xly/erp/module/usr/mapper/SysUserMapper.java index daca0f9..60ed176 100644 --- a/backend/src/main/java/com/xly/erp/module/usr/mapper/SysUserMapper.java +++ b/backend/src/main/java/com/xly/erp/module/usr/mapper/SysUserMapper.java @@ -3,11 +3,43 @@ package com.xly.erp.module.usr.mapper; import com.baomidou.mybatisplus.core.mapper.BaseMapper; import com.xly.erp.module.usr.entity.SysUser; import org.apache.ibatis.annotations.Mapper; +import org.apache.ibatis.annotations.Param; import org.apache.ibatis.annotations.Select; +import org.apache.ibatis.annotations.Update; @Mapper public interface SysUserMapper extends BaseMapper { - @Select("SELECT * FROM sys_user WHERE sUsername = #{username} LIMIT 1") + String LOGIN_COLUMNS = "iIncrement, sUsername, sUserCode, sPasswordHash, iEmployeeId, " + + "sUserType, sLanguage, iCanEditDocument, iIsDeleted, iFailedLoginCount, " + + "tLockUntil, tLastLoginDate"; + + @Select("SELECT " + LOGIN_COLUMNS + " FROM sys_user WHERE sUsername = #{username} LIMIT 1") SysUser selectByUsername(String username); + + /** + * 原子累加失败登录次数；达到阈值 maxCount 时同步写 tLockUntil = NOW() + lockMinutes 分钟。 + * 单 SQL，DB 层保证并发安全。返回受影响行数（应为 1）。 + */ + /* + * MySQL 按 SET 子句从左到右求值，所以放在 +1 之后的引用看到的是新值。 + * 第二条 SET 用 `iFailedLoginCount >= maxCount` 即等价于"新计数 >= 阈值"判定。 + */ + @Update("UPDATE sys_user " + + "SET iFailedLoginCount = iFailedLoginCount + 1, " + + " tLockUntil = IF(iFailedLoginCount >= #{maxCount}, " + + " DATE_ADD(NOW(), INTERVAL #{lockMinutes} MINUTE), " + + " tLockUntil) " + + "WHERE iIncrement = #{userId}") + int incrementFailedLoginCountAtomic(@Param("userId") Integer userId, + @Param("maxCount") int maxCount, + @Param("lockMinutes") long lockMinutes); + + /** + * 成功登录写入：清零计数 + 清空锁定 + 更新登录时间。一次 UPDATE。 + */ + @Update("UPDATE sys_user " + + "SET iFailedLoginCount = 0, tLockUntil = NULL, tLastLoginDate = NOW() " + + "WHERE iIncrement = #{userId}") + int markLoginSuccess(@Param("userId") Integer userId); } diff --git a/backend/src/main/java/com/xly/erp/module/usr/service/impl/LoginServiceImpl.java b/backend/src/main/java/com/xly/erp/module/usr/service/impl/LoginServiceImpl.java index d1adf7d..dbdc8fe 100644 --- a/backend/src/main/java/com/xly/erp/module/usr/service/impl/LoginServiceImpl.java +++ b/backend/src/main/java/com/xly/erp/module/usr/service/impl/LoginServiceImpl.java @@ -16,9 +16,9 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; import java.util.HashMap; import java.util.Map; @@ -38,9 +38,8 @@ public class LoginServiceImpl implements LoginService { private final JwtUtil jwtUtil; @Override - @Transactional(noRollbackFor = BizException.class) public LoginVo login(String username, String password, String companyCode) { - // 1. 公司校验 + // 1. 公司校验（只读，不需事务） SysCompany company = companyMapper.selectByCode(companyCode); if (company == null || Integer.valueOf(1).equals(company.getIIsDeleted())) { log.warn("[login] companyCode={} 不存在或已删除", companyCode); @@ -60,51 +59,29 @@ public class LoginServiceImpl implements LoginService { throw new BizException(ErrorCode.ACCOUNT_DELETED, "账号已被作废，禁止登录"); } - // 4. 锁定校验（不计入失败次数） + // 4. 锁定校验（不计入失败次数；过期锁定视为已解锁） if (user.getTLockUntil() != null && user.getTLockUntil().isAfter(LocalDateTime.now())) { log.warn("[login] username={} 锁定中，lockUntil={}", username, user.getTLockUntil()); - throw new BizException(ErrorCode.ACCOUNT_LOCKED, "账号已锁定，请稍后再试"); + Map data = new HashMap<>(); + data.put("lockUntil", user.getTLockUntil() + .format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)); + throw new BizException(ErrorCode.ACCOUNT_LOCKED, "账号已锁定，请稍后再试", data); } // 5. 密码校验 if (!passwordEncoder.matches(password, user.getSPasswordHash())) { - handleFailedLogin(user); - log.warn("[login] username={} 密码错误，failedCount={}", username, - user.getIFailedLoginCount() + 1); + int rows = userMapper.incrementFailedLoginCountAtomic( + user.getIIncrement(), MAX_FAILED_LOGIN_COUNT, LOCK_DURATION_MINUTES); + log.warn("[login] username={} 密码错误，原子累加失败次数 rows={}", username, rows); throw new BizException(ErrorCode.BAD_CREDENTIALS, "用户名或密码错误"); } // 6. 成功路径 - return loginSuccess(user, company, companyCode); + return loginSuccess(user, companyCode); } - private void handleFailedLogin(SysUser user) { - int newCount = (user.getIFailedLoginCount() == null ? 0 : user.getIFailedLoginCount()) + 1; - SysUser update = new SysUser(); - update.setIIncrement(user.getIIncrement()); - update.setIFailedLoginCount(newCount); - if (newCount >= MAX_FAILED_LOGIN_COUNT) { - update.setTLockUntil(LocalDateTime.now().plusMinutes(LOCK_DURATION_MINUTES)); - log.warn("[login] username={} 失败累计达 {} 次，触发锁定 {} 分钟", - user.getSUsername(), newCount, LOCK_DURATION_MINUTES); - } - userMapper.updateById(update); - } - - private LoginVo loginSuccess(SysUser user, SysCompany company, String companyCode) { - SysUser update = new SysUser(); - update.setIIncrement(user.getIIncrement()); - update.setIFailedLoginCount(0); - update.setTLockUntil(null); - update.setTLastLoginDate(LocalDateTime.now()); - // tLockUntil 需 explicit null update via mapper — MyBatis-Plus 默认忽略 null， - // 这里用 UpdateWrapper 显式置 null - userMapper.update(update, - new com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper() - .eq("iIncrement", user.getIIncrement()) - .set("tLockUntil", null) - .set("iFailedLoginCount", 0) - .set("tLastLoginDate", LocalDateTime.now())); + private LoginVo loginSuccess(SysUser user, String companyCode) { + userMapper.markLoginSuccess(user.getIIncrement()); String employeeName = null; if (user.getIEmployeeId() != null) { diff --git a/backend/src/test/java/com/xly/erp/common/security/JwtUtilTest.java b/backend/src/test/java/com/xly/erp/common/security/JwtUtilTest.java index be17b94..6b2211b 100644 --- a/backend/src/test/java/com/xly/erp/common/security/JwtUtilTest.java +++ b/backend/src/test/java/com/xly/erp/common/security/JwtUtilTest.java @@ -44,6 +44,10 @@ class JwtUtilTest { assertNotNull(parsed.get("jti")); assertNotNull(parsed.get("iat")); assertNotNull(parsed.get("exp")); + + long iat = ((Number) parsed.get("iat")).longValue(); + long exp = ((Number) parsed.get("exp")).longValue(); + assertEquals(7200L, exp - iat, "exp - iat 必须严格等于 ttlSec（spec § 验收 § 2）"); } @Test diff --git a/backend/src/test/java/com/xly/erp/module/usr/controller/AuthControllerTest.java b/backend/src/test/java/com/xly/erp/module/usr/controller/AuthControllerTest.java index c864275..efecf83 100644 --- a/backend/src/test/java/com/xly/erp/module/usr/controller/AuthControllerTest.java +++ b/backend/src/test/java/com/xly/erp/module/usr/controller/AuthControllerTest.java @@ -74,7 +74,7 @@ class AuthControllerTest { } @Test - void post_login_lockedAccount_returns423_code42301() throws Exception { + void post_login_lockedAccount_returns423_code42301_withLockUntil() throws Exception { jdbc.update("UPDATE sys_user SET iFailedLoginCount=5, tLockUntil=DATE_ADD(NOW(), INTERVAL 30 MINUTE) WHERE sUsername=?", LoginTestSeeder.USER_OK); mvc.perform(post("/api/v1/auth/login") @@ -82,7 +82,8 @@ class AuthControllerTest { .content(body(LoginTestSeeder.USER_OK, LoginTestSeeder.DEFAULT_PASSWORD, LoginTestSeeder.COMPANY_OK))) .andExpect(status().isLocked()) - .andExpect(jsonPath("$.code").value(ErrorCode.ACCOUNT_LOCKED)); + .andExpect(jsonPath("$.code").value(ErrorCode.ACCOUNT_LOCKED)) + .andExpect(jsonPath("$.data.lockUntil").isNotEmpty()); } @Test diff --git a/backend/src/test/java/com/xly/erp/module/usr/service/LoginServiceImplTest.java b/backend/src/test/java/com/xly/erp/module/usr/service/LoginServiceImplTest.java index 35c2f81..a24490f 100644 --- a/backend/src/test/java/com/xly/erp/module/usr/service/LoginServiceImplTest.java +++ b/backend/src/test/java/com/xly/erp/module/usr/service/LoginServiceImplTest.java @@ -168,6 +168,35 @@ class LoginServiceImplTest { } @Test + void login_concurrentBadPassword_atomicallyIncrementsCount() throws Exception { + // 2 线程并发各跑 2 次错误密码 → 计数累加必须 == 4（低于 5 不触发锁定， + // 专注验证原子性。锁定路径在 login_5thBadPassword_* 单线程测试中验证） + int perThread = 2; + int threadCount = 2; + Thread t1 = new Thread(() -> { + for (int i = 0; i < perThread; i++) { + try { + loginService.login(LoginTestSeeder.USER_OK, "WrongPass1!", LoginTestSeeder.COMPANY_OK); + } catch (BizException ignored) {} + } + }); + Thread t2 = new Thread(() -> { + for (int i = 0; i < perThread; i++) { + try { + loginService.login(LoginTestSeeder.USER_OK, "WrongPass1!", LoginTestSeeder.COMPANY_OK); + } catch (BizException ignored) {} + } + }); + t1.start(); t2.start(); + t1.join(); t2.join(); + + SysUser u = userMapper.selectByUsername(LoginTestSeeder.USER_OK); + assertEquals(perThread * threadCount, u.getIFailedLoginCount(), + "并发失败累加必须 == 总次数（原子 UPDATE 保证；非原子实现会丢失累加）"); + assertNull(u.getTLockUntil(), "总次数低于阈值不应触发锁定"); + } + + @Test void login_success_jwtParsesBack_with_sub_username_companyCode() { LoginVo vo = loginService.login(LoginTestSeeder.USER_OK, LoginTestSeeder.DEFAULT_PASSWORD, LoginTestSeeder.COMPANY_OK); @@ -178,5 +207,8 @@ class LoginServiceImplTest { assertEquals("NORMAL", claims.get("userType")); assertEquals("zh-CN", claims.get("language")); assertNotNull(claims.get("jti")); + long iat = ((Number) claims.get("iat")).longValue(); + long exp = ((Number) claims.get("exp")).longValue(); + assertEquals(7200L, exp - iat, "exp - iat 必须 == TOKEN_TTL_SEC"); } } -- libgit2 0.22.2