Commit f53689c3439f1d4bf2b937a4c3610112a8373d2c
1 parent
df28ae66
fix(usr): 修复 review round 1 must-fix REQ-USR-003
- HIGH 修注入:UserQueryDTO 移除 column 字段,
改成 service 局部变量 + UserMapper @Param("column") 单独传入,
防止 GET query-string 通过 setter 绑定绕过白名单。
- HIGH 修 spec § 6:service 在 queryField=='deleted' 时
把 queryValue 标准化为 '0' / '1';UserMapper.xml 加 deleted
专用 CAST(#{queryValue} AS UNSIGNED) 分支处理 MySQL bit(1)
与字符串隐式比较的不一致;恢复 get_filterByDeletedTrue IT。
- MEDIUM 修 XML deleted 边界:仅当 queryField=='deleted' 且
queryValue 非空时让用户控制 bDeleted 取值,否则保留默认过滤。
Showing
7 changed files
with
86 additions
and
39 deletions
backend/src/main/java/com/xly/erp/module/usr/dto/UserQueryDTO.java
| 1 | 1 | package com.xly.erp.module.usr.dto; |
| 2 | 2 | |
| 3 | -import com.fasterxml.jackson.annotation.JsonIgnore; | |
| 4 | 3 | import jakarta.validation.constraints.Max; |
| 5 | 4 | import jakarta.validation.constraints.Min; |
| 6 | 5 | import jakarta.validation.constraints.Pattern; |
| ... | ... | @@ -18,7 +17,7 @@ public class UserQueryDTO { |
| 18 | 17 | @Max(100) |
| 19 | 18 | private Integer pageSize = 20; |
| 20 | 19 | |
| 21 | - /** 可空:缺省视为不过滤;服务层白名单校验后映射到 column 字段 */ | |
| 20 | + /** 可空:缺省视为不过滤;服务层白名单映射为 SQL 列名后通过 mapper @Param 单独传入 */ | |
| 22 | 21 | @Pattern(regexp = "^(username|staffname|userno|department|usertype|language|deleted|lastLoginDate|createdBy)?$", |
| 23 | 22 | message = "queryField 非法") |
| 24 | 23 | private String queryField; |
| ... | ... | @@ -30,9 +29,4 @@ public class UserQueryDTO { |
| 30 | 29 | /** 可空:缺省视为不过滤 */ |
| 31 | 30 | @Size(max = 100) |
| 32 | 31 | private String queryValue; |
| 33 | - | |
| 34 | - /** 服务层白名单映射后的实际 SQL 列名(如 "u.sUserName")。 | |
| 35 | - * Jackson 忽略——前端不能也无法直接传入;XML mapper 用 ${query.column} 渲染。 */ | |
| 36 | - @JsonIgnore | |
| 37 | - private String column; | |
| 38 | 32 | } | ... | ... |
backend/src/main/java/com/xly/erp/module/usr/mapper/UserMapper.java
| ... | ... | @@ -9,6 +9,11 @@ import org.apache.ibatis.annotations.Param; |
| 9 | 9 | |
| 10 | 10 | public interface UserMapper extends BaseMapper<UserEntity> { |
| 11 | 11 | |
| 12 | - /** REQ-USR-003 用户列表查询:跨表 JOIN tStaff,按 query 过滤 + 分页。XML 实现。 */ | |
| 13 | - IPage<UserListItemVO> searchUsers(IPage<UserListItemVO> page, @Param("query") UserQueryDTO query); | |
| 12 | + /** | |
| 13 | + * REQ-USR-003 用户列表查询:跨表 JOIN tStaff,按 query 过滤 + 分页。XML 实现。 | |
| 14 | + * @param column service 层白名单映射后的 SQL 列字符串(如 "u.sUserName");外部输入绝不直接走这里。 | |
| 15 | + */ | |
| 16 | + IPage<UserListItemVO> searchUsers(IPage<UserListItemVO> page, | |
| 17 | + @Param("query") UserQueryDTO query, | |
| 18 | + @Param("column") String column); | |
| 14 | 19 | } | ... | ... |
backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java
| ... | ... | @@ -187,13 +187,15 @@ public class UserServiceImpl implements UserService { |
| 187 | 187 | @Override |
| 188 | 188 | @Transactional(readOnly = true) |
| 189 | 189 | public PageResult<UserListItemVO> search(UserQueryDTO query) { |
| 190 | - // 1. queryField 白名单 + 列映射(防 SQL 注入) | |
| 190 | + // 1. queryField 白名单 + 列映射(防 SQL 注入)。 | |
| 191 | + // column 是 service 内部局部变量,通过 mapper @Param("column") 单独传入;不写回 DTO, | |
| 192 | + // 避免 GET query-string 绑定(@JsonIgnore 仅对 Jackson 生效,无法防 setter 注入)。 | |
| 193 | + String column = null; | |
| 191 | 194 | if (query.getQueryField() != null && !query.getQueryField().isEmpty()) { |
| 192 | - String mapped = QUERY_COLUMN_MAP.get(query.getQueryField()); | |
| 193 | - if (mapped == null) { | |
| 195 | + column = QUERY_COLUMN_MAP.get(query.getQueryField()); | |
| 196 | + if (column == null) { | |
| 194 | 197 | throw new BizException(ErrorCode.PARAM_INVALID, "queryField 非法: " + query.getQueryField()); |
| 195 | 198 | } |
| 196 | - query.setColumn(mapped); | |
| 197 | 199 | } |
| 198 | 200 | |
| 199 | 201 | // 2. matchType 白名单 |
| ... | ... | @@ -202,13 +204,26 @@ public class UserServiceImpl implements UserService { |
| 202 | 204 | throw new BizException(ErrorCode.PARAM_INVALID, "matchType 非法: " + query.getMatchType()); |
| 203 | 205 | } |
| 204 | 206 | |
| 205 | - // 3. 默认值兜底 | |
| 207 | + // 3. spec § 业务规则 6:deleted 字段值标准化('true'/'1' → '1';'false'/'0' → '0';其它非法) | |
| 208 | + if ("deleted".equals(query.getQueryField()) | |
| 209 | + && query.getQueryValue() != null && !query.getQueryValue().isEmpty()) { | |
| 210 | + String v = query.getQueryValue().trim().toLowerCase(); | |
| 211 | + if ("true".equals(v) || "1".equals(v)) { | |
| 212 | + query.setQueryValue("1"); | |
| 213 | + } else if ("false".equals(v) || "0".equals(v)) { | |
| 214 | + query.setQueryValue("0"); | |
| 215 | + } else { | |
| 216 | + throw new BizException(ErrorCode.PARAM_INVALID, "deleted queryValue 仅支持 true/false/1/0"); | |
| 217 | + } | |
| 218 | + } | |
| 219 | + | |
| 220 | + // 4. 默认值兜底 | |
| 206 | 221 | int pageNum = query.getPageNum() == null ? 1 : query.getPageNum(); |
| 207 | 222 | int pageSize = query.getPageSize() == null ? 20 : query.getPageSize(); |
| 208 | 223 | |
| 209 | - // 4. MP 分页查询 | |
| 224 | + // 5. MP 分页查询 | |
| 210 | 225 | IPage<UserListItemVO> page = new Page<>(pageNum, pageSize); |
| 211 | - IPage<UserListItemVO> result = userMapper.searchUsers(page, query); | |
| 226 | + IPage<UserListItemVO> result = userMapper.searchUsers(page, query, column); | |
| 212 | 227 | |
| 213 | 228 | return PageResult.of(result); |
| 214 | 229 | } | ... | ... |
backend/src/main/resources/mapper/usr/UserMapper.xml
| ... | ... | @@ -3,7 +3,8 @@ |
| 3 | 3 | <mapper namespace="com.xly.erp.module.usr.mapper.UserMapper"> |
| 4 | 4 | |
| 5 | 5 | <!-- REQ-USR-003 用户列表查询:LEFT JOIN tStaff + 动态 WHERE。 |
| 6 | - query.column 由 service 层白名单映射,绝不接受用户原始输入。 --> | |
| 6 | + column 由 service 层白名单映射后通过 @Param("column") 单独传入, | |
| 7 | + 绝不接受 DTO 中可被 GET query-string 绑定的字段。 --> | |
| 7 | 8 | <select id="searchUsers" resultType="com.xly.erp.module.usr.vo.UserListItemVO"> |
| 8 | 9 | SELECT |
| 9 | 10 | u.iIncrement, u.sUserName, s.sStaffName, u.sUserNo, |
| ... | ... | @@ -12,20 +13,26 @@ |
| 12 | 13 | FROM tUser u |
| 13 | 14 | LEFT JOIN tStaff s ON u.iStaffId = s.iIncrement AND s.bDeleted = 0 |
| 14 | 15 | <where> |
| 15 | - <if test="query.queryField != 'deleted'"> | |
| 16 | + <!-- 默认过滤已软删除:仅当 queryField=='deleted' 且 queryValue 非空时让用户控制 bDeleted 取值 --> | |
| 17 | + <if test="query.queryField != 'deleted' or query.queryValue == null or query.queryValue == ''"> | |
| 16 | 18 | u.bDeleted = 0 |
| 17 | 19 | </if> |
| 18 | - <if test="query.column != null and query.column != '' and query.queryValue != null and query.queryValue != ''"> | |
| 20 | + <if test="column != null and column != '' and query.queryValue != null and query.queryValue != ''"> | |
| 19 | 21 | AND |
| 20 | 22 | <choose> |
| 23 | + <!-- deleted 是 bit(1) 列,MySQL 与字符串 '1'/'0' 隐式比较不可靠; | |
| 24 | + service 已把 queryValue 标准化为 '0' / '1',此处显式 CAST 成整数。 --> | |
| 25 | + <when test="query.queryField == 'deleted'"> | |
| 26 | + ${column} = CAST(#{query.queryValue} AS UNSIGNED) | |
| 27 | + </when> | |
| 21 | 28 | <when test="query.matchType == 'equals'"> |
| 22 | - ${query.column} = #{query.queryValue} | |
| 29 | + ${column} = #{query.queryValue} | |
| 23 | 30 | </when> |
| 24 | 31 | <when test="query.matchType == 'notContains'"> |
| 25 | - ${query.column} NOT LIKE CONCAT('%', #{query.queryValue}, '%') | |
| 32 | + ${column} NOT LIKE CONCAT('%', #{query.queryValue}, '%') | |
| 26 | 33 | </when> |
| 27 | 34 | <otherwise> |
| 28 | - ${query.column} LIKE CONCAT('%', #{query.queryValue}, '%') | |
| 35 | + ${column} LIKE CONCAT('%', #{query.queryValue}, '%') | |
| 29 | 36 | </otherwise> |
| 30 | 37 | </choose> |
| 31 | 38 | </if> | ... | ... |
backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java
| ... | ... | @@ -422,13 +422,23 @@ class UserControllerIT { |
| 422 | 422 | .andExpect(jsonPath("$.data.list[0].sStaffName").value(staffName)); |
| 423 | 423 | } |
| 424 | 424 | |
| 425 | - // get_filterByDeletedTrue_returnsOnlyDeleted 暂时移除 | |
| 426 | - // 原因:MyBatis-Plus PaginationInnerInterceptor 与 MySQL bit(1) 列在 String "1" 隐式比较时 | |
| 427 | - // count 与 main SELECT 行为不一致(count=1 但 list=[]);spec § 业务规则 6 的 | |
| 428 | - // 值标准化('true'/'false' → '1'/'0')也未实现。 | |
| 429 | - // 计划:留作 REQ-USR-004 时统一处理 + 后续 docs sweep 重做。 | |
| 430 | - // service 单测 search_invalidQueryField / search_passesMappedColumnToMapper 已覆盖白名单 + 列映射核心; | |
| 431 | - // mapper IT searchUsers_emptyFilter / searchUsers_filterByUserName 已覆盖 SQL 路径骨架。 | |
| 425 | + @Test | |
| 426 | + void get_filterByDeletedTrue_returnsOnlyDeleted() throws Exception { | |
| 427 | + // 插一个用户后软删 | |
| 428 | + Integer userId = insertUser("del_" + System.nanoTime(), null, List.of()); | |
| 429 | + UserEntity patch = new UserEntity(); | |
| 430 | + patch.setIIncrement(userId); | |
| 431 | + patch.setBDeleted(true); | |
| 432 | + userMapper.updateById(patch); | |
| 433 | + | |
| 434 | + mockMvc.perform(get("/api/users") | |
| 435 | + .param("queryField", "deleted") | |
| 436 | + .param("matchType", "equals") | |
| 437 | + .param("queryValue", "true")) | |
| 438 | + .andExpect(status().isOk()) | |
| 439 | + .andExpect(jsonPath("$.code").value(200)) | |
| 440 | + .andExpect(jsonPath("$.data.list[?(@.iIncrement==" + userId + ")]").exists()); | |
| 441 | + } | |
| 432 | 442 | |
| 433 | 443 | @Test |
| 434 | 444 | void get_pagination_returnsCorrectSlice() throws Exception { | ... | ... |
backend/src/test/java/com/xly/erp/module/usr/mapper/UserMapperSearchIT.java
| ... | ... | @@ -59,7 +59,7 @@ class UserMapperSearchIT { |
| 59 | 59 | insertUser("bob_" + System.nanoTime(), null); |
| 60 | 60 | |
| 61 | 61 | UserQueryDTO query = new UserQueryDTO(); |
| 62 | - IPage<UserListItemVO> result = userMapper.searchUsers(new Page<>(1, 50), query); | |
| 62 | + IPage<UserListItemVO> result = userMapper.searchUsers(new Page<>(1, 50), query, null); | |
| 63 | 63 | |
| 64 | 64 | assertThat(result.getTotal()).isGreaterThanOrEqualTo(2L); |
| 65 | 65 | assertThat(result.getRecords()).extracting(UserListItemVO::getSUserName) |
| ... | ... | @@ -74,11 +74,11 @@ class UserMapperSearchIT { |
| 74 | 74 | |
| 75 | 75 | UserQueryDTO query = new UserQueryDTO(); |
| 76 | 76 | query.setQueryField("username"); |
| 77 | - query.setColumn("u.sUserName"); | |
| 78 | 77 | query.setMatchType("contains"); |
| 79 | 78 | query.setQueryValue(alicePrefix); |
| 80 | 79 | |
| 81 | - IPage<UserListItemVO> result = userMapper.searchUsers(new Page<>(1, 50), query); | |
| 80 | + // column 由 service 层映射;mapper IT 直接传 "u.sUserName" | |
| 81 | + IPage<UserListItemVO> result = userMapper.searchUsers(new Page<>(1, 50), query, "u.sUserName"); | |
| 82 | 82 | |
| 83 | 83 | assertThat(result.getRecords()).hasSize(1); |
| 84 | 84 | assertThat(result.getRecords().get(0).getSUserName()).startsWith(alicePrefix); | ... | ... |
backend/src/test/java/com/xly/erp/module/usr/service/UserServiceImplTest.java
| ... | ... | @@ -425,7 +425,7 @@ class UserServiceImplTest { |
| 425 | 425 | UserQueryDTO query = new UserQueryDTO(); |
| 426 | 426 | IPage<UserListItemVO> emptyPage = new Page<>(1, 20); |
| 427 | 427 | emptyPage.setTotal(0L); |
| 428 | - when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class))).thenReturn(emptyPage); | |
| 428 | + when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class), any())).thenReturn(emptyPage); | |
| 429 | 429 | |
| 430 | 430 | PageResult<UserListItemVO> result = service.search(query); |
| 431 | 431 | assertThat(result.getTotal()).isZero(); |
| ... | ... | @@ -460,13 +460,13 @@ class UserServiceImplTest { |
| 460 | 460 | query.setQueryField("username"); |
| 461 | 461 | query.setQueryValue("alice"); |
| 462 | 462 | IPage<UserListItemVO> emptyPage = new Page<>(1, 20); |
| 463 | - when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class))).thenReturn(emptyPage); | |
| 463 | + when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class), any())).thenReturn(emptyPage); | |
| 464 | 464 | |
| 465 | 465 | service.search(query); |
| 466 | 466 | |
| 467 | - ArgumentCaptor<UserQueryDTO> cap = ArgumentCaptor.forClass(UserQueryDTO.class); | |
| 468 | - verify(userMapper).searchUsers(any(IPage.class), cap.capture()); | |
| 469 | - assertThat(cap.getValue().getColumn()).isEqualTo("u.sUserName"); | |
| 467 | + ArgumentCaptor<String> colCap = ArgumentCaptor.forClass(String.class); | |
| 468 | + verify(userMapper).searchUsers(any(IPage.class), any(UserQueryDTO.class), colCap.capture()); | |
| 469 | + assertThat(colCap.getValue()).isEqualTo("u.sUserName"); | |
| 470 | 470 | } |
| 471 | 471 | |
| 472 | 472 | @Test |
| ... | ... | @@ -475,14 +475,30 @@ class UserServiceImplTest { |
| 475 | 475 | query.setPageNum(null); |
| 476 | 476 | query.setPageSize(null); |
| 477 | 477 | IPage<UserListItemVO> emptyPage = new Page<>(1, 20); |
| 478 | - when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class))).thenReturn(emptyPage); | |
| 478 | + when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class), any())).thenReturn(emptyPage); | |
| 479 | 479 | |
| 480 | 480 | service.search(query); |
| 481 | 481 | |
| 482 | 482 | ArgumentCaptor<IPage> pageCap = ArgumentCaptor.forClass(IPage.class); |
| 483 | - verify(userMapper).searchUsers(pageCap.capture(), any(UserQueryDTO.class)); | |
| 483 | + verify(userMapper).searchUsers(pageCap.capture(), any(UserQueryDTO.class), any()); | |
| 484 | 484 | IPage<?> page = pageCap.getValue(); |
| 485 | 485 | assertThat(page.getCurrent()).isEqualTo(1L); |
| 486 | 486 | assertThat(page.getSize()).isEqualTo(20L); |
| 487 | 487 | } |
| 488 | + | |
| 489 | + @Test | |
| 490 | + void search_deletedQueryValueTrue_normalizedToOne() { | |
| 491 | + UserQueryDTO query = new UserQueryDTO(); | |
| 492 | + query.setQueryField("deleted"); | |
| 493 | + query.setQueryValue("true"); | |
| 494 | + query.setMatchType("equals"); | |
| 495 | + IPage<UserListItemVO> emptyPage = new Page<>(1, 20); | |
| 496 | + when(userMapper.searchUsers(any(IPage.class), any(UserQueryDTO.class), any())).thenReturn(emptyPage); | |
| 497 | + | |
| 498 | + service.search(query); | |
| 499 | + | |
| 500 | + ArgumentCaptor<UserQueryDTO> cap = ArgumentCaptor.forClass(UserQueryDTO.class); | |
| 501 | + verify(userMapper).searchUsers(any(IPage.class), cap.capture(), any()); | |
| 502 | + assertThat(cap.getValue().getQueryValue()).isEqualTo("1"); | |
| 503 | + } | |
| 488 | 504 | } | ... | ... |