Commit 3d2c0ad35aeb118714c4402f72eaaeea87adce87
1 parent
677b3e48
fix(usr): 修复 review round 1 must-fix REQ-USR-001
- 42301 响应携带 data.lockUntil(BizException 扩 data + handler 写入) - 失败计数改原子单 SQL UPDATE(去 noRollbackFor,行锁由 DB 保证) - SysUserMapper/SysCompanyMapper 去 SELECT *,改显式列名 - JwtUtil 短密钥静默补零 → 启动期硬抛 IllegalStateException - loginSuccess 去 entity 冗余路径,只保留 UpdateWrapper - 补 exp-iat==7200 / lockUntil 字段 / 并发原子累加 测试
Showing
10 changed files
with
109 additions
and
46 deletions
backend/src/main/java/com/xly/erp/common/exception/BizException.java
| ... | ... | @@ -9,14 +9,22 @@ import lombok.Getter; |
| 9 | 9 | @Getter |
| 10 | 10 | public class BizException extends RuntimeException { |
| 11 | 11 | private final int code; |
| 12 | + /** 可选附带的响应数据(例如 42301 锁定返 lockUntil)。null 表示无 data 字段。 */ | |
| 13 | + private final Object data; | |
| 12 | 14 | |
| 13 | 15 | public BizException(int code, String message) { |
| 16 | + this(code, message, (Object) null); | |
| 17 | + } | |
| 18 | + | |
| 19 | + public BizException(int code, String message, Object data) { | |
| 14 | 20 | super(message); |
| 15 | 21 | this.code = code; |
| 22 | + this.data = data; | |
| 16 | 23 | } |
| 17 | 24 | |
| 18 | 25 | public BizException(int code, String message, Throwable cause) { |
| 19 | 26 | super(message, cause); |
| 20 | 27 | this.code = code; |
| 28 | + this.data = null; | |
| 21 | 29 | } |
| 22 | 30 | } | ... | ... |
backend/src/main/java/com/xly/erp/common/exception/GlobalExceptionHandler.java
| ... | ... | @@ -19,11 +19,14 @@ import org.springframework.web.bind.annotation.RestControllerAdvice; |
| 19 | 19 | public class GlobalExceptionHandler { |
| 20 | 20 | |
| 21 | 21 | @ExceptionHandler(BizException.class) |
| 22 | - public ResponseEntity<Result<Void>> handleBiz(BizException e) { | |
| 23 | - log.warn("[BizException] code={} message={}", e.getCode(), e.getMessage()); | |
| 22 | + public ResponseEntity<Result<Object>> handleBiz(BizException e) { | |
| 23 | + log.warn("[BizException] code={} message={} hasData={}", e.getCode(), e.getMessage(), e.getData() != null); | |
| 24 | + Result<Object> body = e.getData() != null | |
| 25 | + ? Result.fail(e.getCode(), e.getMessage(), e.getData()) | |
| 26 | + : Result.fail(e.getCode(), e.getMessage()); | |
| 24 | 27 | return ResponseEntity |
| 25 | 28 | .status(ErrorCode.toHttpStatus(e.getCode())) |
| 26 | - .body(Result.fail(e.getCode(), e.getMessage())); | |
| 29 | + .body(body); | |
| 27 | 30 | } |
| 28 | 31 | |
| 29 | 32 | @ExceptionHandler(MethodArgumentNotValidException.class) | ... | ... |
backend/src/main/java/com/xly/erp/common/response/Result.java
| ... | ... | @@ -31,4 +31,9 @@ public class Result<T> { |
| 31 | 31 | public static <T> Result<T> fail(int code, String message) { |
| 32 | 32 | return new Result<>(code, message, null); |
| 33 | 33 | } |
| 34 | + | |
| 35 | + @SuppressWarnings("unchecked") | |
| 36 | + public static <T> Result<T> fail(int code, String message, T data) { | |
| 37 | + return new Result<>(code, message, data); | |
| 38 | + } | |
| 34 | 39 | } | ... | ... |
backend/src/main/java/com/xly/erp/common/security/JwtUtil.java
| ... | ... | @@ -33,9 +33,9 @@ public class JwtUtil { |
| 33 | 33 | void init() { |
| 34 | 34 | byte[] bytes = secret.getBytes(StandardCharsets.UTF_8); |
| 35 | 35 | if (bytes.length < 32) { |
| 36 | - byte[] padded = new byte[32]; | |
| 37 | - System.arraycopy(bytes, 0, padded, 0, bytes.length); | |
| 38 | - bytes = padded; | |
| 36 | + throw new IllegalStateException( | |
| 37 | + "JWT_SECRET 长度不足 32 字节(HS256 要求),实际 " + bytes.length | |
| 38 | + + " 字节。请在 .env.local 配置至少 256 位的随机字符串。"); | |
| 39 | 39 | } |
| 40 | 40 | this.key = Keys.hmacShaKeyFor(bytes); |
| 41 | 41 | } | ... | ... |
backend/src/main/java/com/xly/erp/module/usr/mapper/SysCompanyMapper.java
| ... | ... | @@ -8,6 +8,7 @@ import org.apache.ibatis.annotations.Select; |
| 8 | 8 | @Mapper |
| 9 | 9 | public interface SysCompanyMapper extends BaseMapper<SysCompany> { |
| 10 | 10 | |
| 11 | - @Select("SELECT * FROM sys_company WHERE sCompanyCode = #{code} LIMIT 1") | |
| 11 | + @Select("SELECT iIncrement, sCompanyCode, sCompanyName, iIsDeleted " + | |
| 12 | + "FROM sys_company WHERE sCompanyCode = #{code} LIMIT 1") | |
| 12 | 13 | SysCompany selectByCode(String code); |
| 13 | 14 | } | ... | ... |
backend/src/main/java/com/xly/erp/module/usr/mapper/SysUserMapper.java
| ... | ... | @@ -3,11 +3,43 @@ package com.xly.erp.module.usr.mapper; |
| 3 | 3 | import com.baomidou.mybatisplus.core.mapper.BaseMapper; |
| 4 | 4 | import com.xly.erp.module.usr.entity.SysUser; |
| 5 | 5 | import org.apache.ibatis.annotations.Mapper; |
| 6 | +import org.apache.ibatis.annotations.Param; | |
| 6 | 7 | import org.apache.ibatis.annotations.Select; |
| 8 | +import org.apache.ibatis.annotations.Update; | |
| 7 | 9 | |
| 8 | 10 | @Mapper |
| 9 | 11 | public interface SysUserMapper extends BaseMapper<SysUser> { |
| 10 | 12 | |
| 11 | - @Select("SELECT * FROM sys_user WHERE sUsername = #{username} LIMIT 1") | |
| 13 | + String LOGIN_COLUMNS = "iIncrement, sUsername, sUserCode, sPasswordHash, iEmployeeId, " + | |
| 14 | + "sUserType, sLanguage, iCanEditDocument, iIsDeleted, iFailedLoginCount, " + | |
| 15 | + "tLockUntil, tLastLoginDate"; | |
| 16 | + | |
| 17 | + @Select("SELECT " + LOGIN_COLUMNS + " FROM sys_user WHERE sUsername = #{username} LIMIT 1") | |
| 12 | 18 | SysUser selectByUsername(String username); |
| 19 | + | |
| 20 | + /** | |
| 21 | + * 原子累加失败登录次数;达到阈值 maxCount 时同步写 tLockUntil = NOW() + lockMinutes 分钟。 | |
| 22 | + * 单 SQL,DB 层保证并发安全。返回受影响行数(应为 1)。 | |
| 23 | + */ | |
| 24 | + /* | |
| 25 | + * MySQL 按 SET 子句从左到右求值,所以放在 +1 之后的引用看到的是新值。 | |
| 26 | + * 第二条 SET 用 `iFailedLoginCount >= maxCount` 即等价于"新计数 >= 阈值"判定。 | |
| 27 | + */ | |
| 28 | + @Update("UPDATE sys_user " + | |
| 29 | + "SET iFailedLoginCount = iFailedLoginCount + 1, " + | |
| 30 | + " tLockUntil = IF(iFailedLoginCount >= #{maxCount}, " + | |
| 31 | + " DATE_ADD(NOW(), INTERVAL #{lockMinutes} MINUTE), " + | |
| 32 | + " tLockUntil) " + | |
| 33 | + "WHERE iIncrement = #{userId}") | |
| 34 | + int incrementFailedLoginCountAtomic(@Param("userId") Integer userId, | |
| 35 | + @Param("maxCount") int maxCount, | |
| 36 | + @Param("lockMinutes") long lockMinutes); | |
| 37 | + | |
| 38 | + /** | |
| 39 | + * 成功登录写入:清零计数 + 清空锁定 + 更新登录时间。一次 UPDATE。 | |
| 40 | + */ | |
| 41 | + @Update("UPDATE sys_user " + | |
| 42 | + "SET iFailedLoginCount = 0, tLockUntil = NULL, tLastLoginDate = NOW() " + | |
| 43 | + "WHERE iIncrement = #{userId}") | |
| 44 | + int markLoginSuccess(@Param("userId") Integer userId); | |
| 13 | 45 | } | ... | ... |
backend/src/main/java/com/xly/erp/module/usr/service/impl/LoginServiceImpl.java
| ... | ... | @@ -16,9 +16,9 @@ import lombok.RequiredArgsConstructor; |
| 16 | 16 | import lombok.extern.slf4j.Slf4j; |
| 17 | 17 | import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; |
| 18 | 18 | import org.springframework.stereotype.Service; |
| 19 | -import org.springframework.transaction.annotation.Transactional; | |
| 20 | 19 | |
| 21 | 20 | import java.time.LocalDateTime; |
| 21 | +import java.time.format.DateTimeFormatter; | |
| 22 | 22 | import java.util.HashMap; |
| 23 | 23 | import java.util.Map; |
| 24 | 24 | |
| ... | ... | @@ -38,9 +38,8 @@ public class LoginServiceImpl implements LoginService { |
| 38 | 38 | private final JwtUtil jwtUtil; |
| 39 | 39 | |
| 40 | 40 | @Override |
| 41 | - @Transactional(noRollbackFor = BizException.class) | |
| 42 | 41 | public LoginVo login(String username, String password, String companyCode) { |
| 43 | - // 1. 公司校验 | |
| 42 | + // 1. 公司校验(只读,不需事务) | |
| 44 | 43 | SysCompany company = companyMapper.selectByCode(companyCode); |
| 45 | 44 | if (company == null || Integer.valueOf(1).equals(company.getIIsDeleted())) { |
| 46 | 45 | log.warn("[login] companyCode={} 不存在或已删除", companyCode); |
| ... | ... | @@ -60,51 +59,29 @@ public class LoginServiceImpl implements LoginService { |
| 60 | 59 | throw new BizException(ErrorCode.ACCOUNT_DELETED, "账号已被作废,禁止登录"); |
| 61 | 60 | } |
| 62 | 61 | |
| 63 | - // 4. 锁定校验(不计入失败次数) | |
| 62 | + // 4. 锁定校验(不计入失败次数;过期锁定视为已解锁) | |
| 64 | 63 | if (user.getTLockUntil() != null && user.getTLockUntil().isAfter(LocalDateTime.now())) { |
| 65 | 64 | log.warn("[login] username={} 锁定中,lockUntil={}", username, user.getTLockUntil()); |
| 66 | - throw new BizException(ErrorCode.ACCOUNT_LOCKED, "账号已锁定,请稍后再试"); | |
| 65 | + Map<String, Object> data = new HashMap<>(); | |
| 66 | + data.put("lockUntil", user.getTLockUntil() | |
| 67 | + .format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)); | |
| 68 | + throw new BizException(ErrorCode.ACCOUNT_LOCKED, "账号已锁定,请稍后再试", data); | |
| 67 | 69 | } |
| 68 | 70 | |
| 69 | 71 | // 5. 密码校验 |
| 70 | 72 | if (!passwordEncoder.matches(password, user.getSPasswordHash())) { |
| 71 | - handleFailedLogin(user); | |
| 72 | - log.warn("[login] username={} 密码错误,failedCount={}", username, | |
| 73 | - user.getIFailedLoginCount() + 1); | |
| 73 | + int rows = userMapper.incrementFailedLoginCountAtomic( | |
| 74 | + user.getIIncrement(), MAX_FAILED_LOGIN_COUNT, LOCK_DURATION_MINUTES); | |
| 75 | + log.warn("[login] username={} 密码错误,原子累加失败次数 rows={}", username, rows); | |
| 74 | 76 | throw new BizException(ErrorCode.BAD_CREDENTIALS, "用户名或密码错误"); |
| 75 | 77 | } |
| 76 | 78 | |
| 77 | 79 | // 6. 成功路径 |
| 78 | - return loginSuccess(user, company, companyCode); | |
| 80 | + return loginSuccess(user, companyCode); | |
| 79 | 81 | } |
| 80 | 82 | |
| 81 | - private void handleFailedLogin(SysUser user) { | |
| 82 | - int newCount = (user.getIFailedLoginCount() == null ? 0 : user.getIFailedLoginCount()) + 1; | |
| 83 | - SysUser update = new SysUser(); | |
| 84 | - update.setIIncrement(user.getIIncrement()); | |
| 85 | - update.setIFailedLoginCount(newCount); | |
| 86 | - if (newCount >= MAX_FAILED_LOGIN_COUNT) { | |
| 87 | - update.setTLockUntil(LocalDateTime.now().plusMinutes(LOCK_DURATION_MINUTES)); | |
| 88 | - log.warn("[login] username={} 失败累计达 {} 次,触发锁定 {} 分钟", | |
| 89 | - user.getSUsername(), newCount, LOCK_DURATION_MINUTES); | |
| 90 | - } | |
| 91 | - userMapper.updateById(update); | |
| 92 | - } | |
| 93 | - | |
| 94 | - private LoginVo loginSuccess(SysUser user, SysCompany company, String companyCode) { | |
| 95 | - SysUser update = new SysUser(); | |
| 96 | - update.setIIncrement(user.getIIncrement()); | |
| 97 | - update.setIFailedLoginCount(0); | |
| 98 | - update.setTLockUntil(null); | |
| 99 | - update.setTLastLoginDate(LocalDateTime.now()); | |
| 100 | - // tLockUntil 需 explicit null update via mapper — MyBatis-Plus 默认忽略 null, | |
| 101 | - // 这里用 UpdateWrapper 显式置 null | |
| 102 | - userMapper.update(update, | |
| 103 | - new com.baomidou.mybatisplus.core.conditions.update.UpdateWrapper<SysUser>() | |
| 104 | - .eq("iIncrement", user.getIIncrement()) | |
| 105 | - .set("tLockUntil", null) | |
| 106 | - .set("iFailedLoginCount", 0) | |
| 107 | - .set("tLastLoginDate", LocalDateTime.now())); | |
| 83 | + private LoginVo loginSuccess(SysUser user, String companyCode) { | |
| 84 | + userMapper.markLoginSuccess(user.getIIncrement()); | |
| 108 | 85 | |
| 109 | 86 | String employeeName = null; |
| 110 | 87 | if (user.getIEmployeeId() != null) { | ... | ... |
backend/src/test/java/com/xly/erp/common/security/JwtUtilTest.java
| ... | ... | @@ -44,6 +44,10 @@ class JwtUtilTest { |
| 44 | 44 | assertNotNull(parsed.get("jti")); |
| 45 | 45 | assertNotNull(parsed.get("iat")); |
| 46 | 46 | assertNotNull(parsed.get("exp")); |
| 47 | + | |
| 48 | + long iat = ((Number) parsed.get("iat")).longValue(); | |
| 49 | + long exp = ((Number) parsed.get("exp")).longValue(); | |
| 50 | + assertEquals(7200L, exp - iat, "exp - iat 必须严格等于 ttlSec(spec § 验收 § 2)"); | |
| 47 | 51 | } |
| 48 | 52 | |
| 49 | 53 | @Test | ... | ... |
backend/src/test/java/com/xly/erp/module/usr/controller/AuthControllerTest.java
| ... | ... | @@ -74,7 +74,7 @@ class AuthControllerTest { |
| 74 | 74 | } |
| 75 | 75 | |
| 76 | 76 | @Test |
| 77 | - void post_login_lockedAccount_returns423_code42301() throws Exception { | |
| 77 | + void post_login_lockedAccount_returns423_code42301_withLockUntil() throws Exception { | |
| 78 | 78 | jdbc.update("UPDATE sys_user SET iFailedLoginCount=5, tLockUntil=DATE_ADD(NOW(), INTERVAL 30 MINUTE) WHERE sUsername=?", |
| 79 | 79 | LoginTestSeeder.USER_OK); |
| 80 | 80 | mvc.perform(post("/api/v1/auth/login") |
| ... | ... | @@ -82,7 +82,8 @@ class AuthControllerTest { |
| 82 | 82 | .content(body(LoginTestSeeder.USER_OK, LoginTestSeeder.DEFAULT_PASSWORD, |
| 83 | 83 | LoginTestSeeder.COMPANY_OK))) |
| 84 | 84 | .andExpect(status().isLocked()) |
| 85 | - .andExpect(jsonPath("$.code").value(ErrorCode.ACCOUNT_LOCKED)); | |
| 85 | + .andExpect(jsonPath("$.code").value(ErrorCode.ACCOUNT_LOCKED)) | |
| 86 | + .andExpect(jsonPath("$.data.lockUntil").isNotEmpty()); | |
| 86 | 87 | } |
| 87 | 88 | |
| 88 | 89 | @Test | ... | ... |
backend/src/test/java/com/xly/erp/module/usr/service/LoginServiceImplTest.java
| ... | ... | @@ -168,6 +168,35 @@ class LoginServiceImplTest { |
| 168 | 168 | } |
| 169 | 169 | |
| 170 | 170 | @Test |
| 171 | + void login_concurrentBadPassword_atomicallyIncrementsCount() throws Exception { | |
| 172 | + // 2 线程并发各跑 2 次错误密码 → 计数累加必须 == 4(低于 5 不触发锁定, | |
| 173 | + // 专注验证原子性。锁定路径在 login_5thBadPassword_* 单线程测试中验证) | |
| 174 | + int perThread = 2; | |
| 175 | + int threadCount = 2; | |
| 176 | + Thread t1 = new Thread(() -> { | |
| 177 | + for (int i = 0; i < perThread; i++) { | |
| 178 | + try { | |
| 179 | + loginService.login(LoginTestSeeder.USER_OK, "WrongPass1!", LoginTestSeeder.COMPANY_OK); | |
| 180 | + } catch (BizException ignored) {} | |
| 181 | + } | |
| 182 | + }); | |
| 183 | + Thread t2 = new Thread(() -> { | |
| 184 | + for (int i = 0; i < perThread; i++) { | |
| 185 | + try { | |
| 186 | + loginService.login(LoginTestSeeder.USER_OK, "WrongPass1!", LoginTestSeeder.COMPANY_OK); | |
| 187 | + } catch (BizException ignored) {} | |
| 188 | + } | |
| 189 | + }); | |
| 190 | + t1.start(); t2.start(); | |
| 191 | + t1.join(); t2.join(); | |
| 192 | + | |
| 193 | + SysUser u = userMapper.selectByUsername(LoginTestSeeder.USER_OK); | |
| 194 | + assertEquals(perThread * threadCount, u.getIFailedLoginCount(), | |
| 195 | + "并发失败累加必须 == 总次数(原子 UPDATE 保证;非原子实现会丢失累加)"); | |
| 196 | + assertNull(u.getTLockUntil(), "总次数低于阈值不应触发锁定"); | |
| 197 | + } | |
| 198 | + | |
| 199 | + @Test | |
| 171 | 200 | void login_success_jwtParsesBack_with_sub_username_companyCode() { |
| 172 | 201 | LoginVo vo = loginService.login(LoginTestSeeder.USER_OK, LoginTestSeeder.DEFAULT_PASSWORD, |
| 173 | 202 | LoginTestSeeder.COMPANY_OK); |
| ... | ... | @@ -178,5 +207,8 @@ class LoginServiceImplTest { |
| 178 | 207 | assertEquals("NORMAL", claims.get("userType")); |
| 179 | 208 | assertEquals("zh-CN", claims.get("language")); |
| 180 | 209 | assertNotNull(claims.get("jti")); |
| 210 | + long iat = ((Number) claims.get("iat")).longValue(); | |
| 211 | + long exp = ((Number) claims.get("exp")).longValue(); | |
| 212 | + assertEquals(7200L, exp - iat, "exp - iat 必须 == TOKEN_TTL_SEC"); | |
| 181 | 213 | } |
| 182 | 214 | } | ... | ... |