Commit 1cb8cf534bc10445302b31fcf459e30cabe9c49f
1 parent
fe5a6531
fix(user): 修复权限保存的竞态/越权/软删兼容问题 (REQ-USR-002)
源自 Codex 对 wire-frontend-backend 分支的对抗性评审 (2× high / 1× medium),三条 finding 均已复核属实。 - [HIGH] 详情未加载就保存会把 permissionCategoryIds 当成 [] → 静默清空: - 前端 ids 三态化 (number[] | null),列表态 snapshot 不带 ids 时保持 null - 编辑模式下 ids 仍为 null 时 disable 修改/保存按钮,DTO 中省略该字段 - 后端 update() 把 delete+reinsert 包到 if (ids != null),null 视为"不动权限" - [HIGH] 过滤态下表头全选越权授权:allChecked 与 onChange 改为基于 visibleCategories - [MEDIUM] selectCategoryIdsByUserId 加 INNER JOIN tPermissionCategory 过滤 bDeleted=0, 避免软删分类导致 40023 阻塞无关字段编辑
Showing
3 changed files
with
59 additions
and
29 deletions
backend/src/main/java/com/xly/erp/module/usr/mapper/UserPermissionMapper.java
| @@ -13,6 +13,9 @@ public interface UserPermissionMapper extends BaseMapper<UserPermission> { | @@ -13,6 +13,9 @@ public interface UserPermissionMapper extends BaseMapper<UserPermission> { | ||
| 13 | @Delete("DELETE FROM tUserPermission WHERE iUserId = #{userId}") | 13 | @Delete("DELETE FROM tUserPermission WHERE iUserId = #{userId}") |
| 14 | int deleteByUserId(@Param("userId") Integer userId); | 14 | int deleteByUserId(@Param("userId") Integer userId); |
| 15 | 15 | ||
| 16 | - @Select("SELECT iCategoryId FROM tUserPermission WHERE iUserId = #{userId} ORDER BY iIncrement ASC") | 16 | + @Select("SELECT up.iCategoryId FROM tUserPermission up " |
| 17 | + + "INNER JOIN tPermissionCategory pc ON pc.iIncrement = up.iCategoryId " | ||
| 18 | + + "WHERE up.iUserId = #{userId} AND pc.bDeleted = 0 " | ||
| 19 | + + "ORDER BY up.iIncrement ASC") | ||
| 17 | List<Integer> selectCategoryIdsByUserId(@Param("userId") Integer userId); | 20 | List<Integer> selectCategoryIdsByUserId(@Param("userId") Integer userId); |
| 18 | } | 21 | } |
backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java
| @@ -191,19 +191,21 @@ public class UserServiceImpl implements UserService { | @@ -191,19 +191,21 @@ public class UserServiceImpl implements UserService { | ||
| 191 | throw new BizException(40020, "用户号或用户名已存在"); | 191 | throw new BizException(40020, "用户号或用户名已存在"); |
| 192 | } | 192 | } |
| 193 | 193 | ||
| 194 | - userPermissionMapper.deleteByUserId(id); | ||
| 195 | - if (ids != null && !ids.isEmpty()) { | ||
| 196 | - String createdBy = SecurityContextHelper.currentUserNo(); | ||
| 197 | - LocalDateTime now = LocalDateTime.now(); | ||
| 198 | - for (Integer cid : ids) { | ||
| 199 | - UserPermission rel = new UserPermission(); | ||
| 200 | - rel.setSBrandsId(tenant.getBrandsId()); | ||
| 201 | - rel.setSSubsidiaryId(tenant.getSubsidiaryId()); | ||
| 202 | - rel.setTCreateDate(now); | ||
| 203 | - rel.setIUserId(id); | ||
| 204 | - rel.setICategoryId(cid); | ||
| 205 | - rel.setSCreatedBy(createdBy); | ||
| 206 | - userPermissionMapper.insert(rel); | 194 | + if (ids != null) { |
| 195 | + userPermissionMapper.deleteByUserId(id); | ||
| 196 | + if (!ids.isEmpty()) { | ||
| 197 | + String createdBy = SecurityContextHelper.currentUserNo(); | ||
| 198 | + LocalDateTime now = LocalDateTime.now(); | ||
| 199 | + for (Integer cid : ids) { | ||
| 200 | + UserPermission rel = new UserPermission(); | ||
| 201 | + rel.setSBrandsId(tenant.getBrandsId()); | ||
| 202 | + rel.setSSubsidiaryId(tenant.getSubsidiaryId()); | ||
| 203 | + rel.setTCreateDate(now); | ||
| 204 | + rel.setIUserId(id); | ||
| 205 | + rel.setICategoryId(cid); | ||
| 206 | + rel.setSCreatedBy(createdBy); | ||
| 207 | + userPermissionMapper.insert(rel); | ||
| 208 | + } | ||
| 207 | } | 209 | } |
| 208 | } | 210 | } |
| 209 | return id; | 211 | return id; |
frontend/src/pages/usr/UserDetail.tsx
| @@ -76,7 +76,11 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -76,7 +76,11 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 76 | const [detail, setDetail] = useState<UserListVO | undefined>(snapshot); | 76 | const [detail, setDetail] = useState<UserListVO | undefined>(snapshot); |
| 77 | const currentSnapshot = detail ?? snapshot; | 77 | const currentSnapshot = detail ?? snapshot; |
| 78 | const [permissionCategories, setPermissionCategories] = useState<PermissionCategoryVO[]>([]); | 78 | const [permissionCategories, setPermissionCategories] = useState<PermissionCategoryVO[]>([]); |
| 79 | - const [permissionCategoryIds, setPermissionCategoryIds] = useState<number[]>([]); | 79 | + // null = 详情尚未加载(仅在新建模式下立即视为 [])。 |
| 80 | + // 编辑模式必须等到非 null 后才允许保存,否则会把"未知"误当成"清空"。 | ||
| 81 | + const [permissionCategoryIds, setPermissionCategoryIds] = useState<number[] | null>( | ||
| 82 | + initialMode === "new" ? [] : null | ||
| 83 | + ); | ||
| 80 | 84 | ||
| 81 | const buildForm = (snap?: UserListVO): FormState => | 85 | const buildForm = (snap?: UserListVO): FormState => |
| 82 | mode === "new" || !snap | 86 | mode === "new" || !snap |
| @@ -123,13 +127,18 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -123,13 +127,18 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 123 | setPermissionCategoryIds(u.permissionCategoryIds ?? []); | 127 | setPermissionCategoryIds(u.permissionCategoryIds ?? []); |
| 124 | }) | 128 | }) |
| 125 | .catch(() => { | 129 | .catch(() => { |
| 126 | - // interceptor already showed message | 130 | + // interceptor already showed message; 保持 null 以便"修改/保存"维持 disabled。 |
| 127 | }); | 131 | }); |
| 128 | }, [mode, userId]); | 132 | }, [mode, userId]); |
| 129 | 133 | ||
| 130 | useEffect(() => { | 134 | useEffect(() => { |
| 131 | setForm(buildForm(currentSnapshot)); | 135 | setForm(buildForm(currentSnapshot)); |
| 132 | - setPermissionCategoryIds(mode === "new" ? [] : currentSnapshot?.permissionCategoryIds ?? []); | 136 | + if (mode === "new") { |
| 137 | + setPermissionCategoryIds([]); | ||
| 138 | + } else if (currentSnapshot?.permissionCategoryIds !== undefined) { | ||
| 139 | + // detail() 返回过来的快照里带 ids 才采纳;列表态快照没有该字段时保持 null(仍未加载)。 | ||
| 140 | + setPermissionCategoryIds(currentSnapshot.permissionCategoryIds); | ||
| 141 | + } | ||
| 133 | // eslint-disable-next-line react-hooks/exhaustive-deps | 142 | // eslint-disable-next-line react-hooks/exhaustive-deps |
| 134 | }, [currentSnapshot, mode]); | 143 | }, [currentSnapshot, mode]); |
| 135 | 144 | ||
| @@ -137,7 +146,8 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -137,7 +146,8 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 137 | setForm((s) => ({ ...s, [k]: v })); | 146 | setForm((s) => ({ ...s, [k]: v })); |
| 138 | 147 | ||
| 139 | const disabled = mode === "view"; | 148 | const disabled = mode === "view"; |
| 140 | - const checkedCount = permissionCategoryIds.length; | 149 | + const checkedCount = permissionCategoryIds?.length ?? 0; |
| 150 | + const permsLoaded = permissionCategoryIds !== null; | ||
| 141 | 151 | ||
| 142 | const startEdit = () => setMode("edit"); | 152 | const startEdit = () => setMode("edit"); |
| 143 | const startNew = () => setMode("new"); | 153 | const startNew = () => setMode("new"); |
| @@ -147,7 +157,9 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -147,7 +157,9 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 147 | dispatch(setActiveTab("userlist")); | 157 | dispatch(setActiveTab("userlist")); |
| 148 | } else { | 158 | } else { |
| 149 | setForm(buildForm(currentSnapshot)); | 159 | setForm(buildForm(currentSnapshot)); |
| 150 | - setPermissionCategoryIds(currentSnapshot?.permissionCategoryIds ?? []); | 160 | + if (currentSnapshot?.permissionCategoryIds !== undefined) { |
| 161 | + setPermissionCategoryIds(currentSnapshot.permissionCategoryIds); | ||
| 162 | + } | ||
| 151 | setMode("view"); | 163 | setMode("view"); |
| 152 | } | 164 | } |
| 153 | }; | 165 | }; |
| @@ -164,8 +176,13 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -164,8 +176,13 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 164 | sUserType: form.sUserType, | 176 | sUserType: form.sUserType, |
| 165 | sLanguage: form.sLanguage, | 177 | sLanguage: form.sLanguage, |
| 166 | bCanModifyDocs: form.bCanModifyDocs, | 178 | bCanModifyDocs: form.bCanModifyDocs, |
| 167 | - permissionCategoryIds, | ||
| 168 | }; | 179 | }; |
| 180 | + if (mode === "new") { | ||
| 181 | + dto.permissionCategoryIds = permissionCategoryIds ?? []; | ||
| 182 | + } else if (permissionCategoryIds !== null) { | ||
| 183 | + // 编辑模式:未加载完前不下发该字段,后端按"不动权限"处理。 | ||
| 184 | + dto.permissionCategoryIds = permissionCategoryIds; | ||
| 185 | + } | ||
| 169 | setSubmitting(true); | 186 | setSubmitting(true); |
| 170 | try { | 187 | try { |
| 171 | if (mode === "new") { | 188 | if (mode === "new") { |
| @@ -217,14 +234,18 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -217,14 +234,18 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 217 | <ToolbarBtnDark | 234 | <ToolbarBtnDark |
| 218 | icon={<EditOutlined />} | 235 | icon={<EditOutlined />} |
| 219 | onClick={startEdit} | 236 | onClick={startEdit} |
| 220 | - disabled={mode !== "view" || !userId} | 237 | + disabled={mode !== "view" || !userId || !permsLoaded} |
| 221 | > | 238 | > |
| 222 | 修改 | 239 | 修改 |
| 223 | </ToolbarBtnDark> | 240 | </ToolbarBtnDark> |
| 224 | <ToolbarBtnDark icon={<DeleteOutlined />} disabled={mode !== "view"} danger> | 241 | <ToolbarBtnDark icon={<DeleteOutlined />} disabled={mode !== "view"} danger> |
| 225 | 删除 | 242 | 删除 |
| 226 | </ToolbarBtnDark> | 243 | </ToolbarBtnDark> |
| 227 | - <ToolbarBtnDark icon={<SaveOutlined />} onClick={save} disabled={mode === "view" || submitting}> | 244 | + <ToolbarBtnDark |
| 245 | + icon={<SaveOutlined />} | ||
| 246 | + onClick={save} | ||
| 247 | + disabled={mode === "view" || submitting || (mode === "edit" && !permsLoaded)} | ||
| 248 | + > | ||
| 228 | 保存 | 249 | 保存 |
| 229 | </ToolbarBtnDark> | 250 | </ToolbarBtnDark> |
| 230 | <ToolbarBtnDark icon={<CloseOutlined />} onClick={cancel} disabled={mode === "view"}> | 251 | <ToolbarBtnDark icon={<CloseOutlined />} onClick={cancel} disabled={mode === "view"}> |
| @@ -440,13 +461,14 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | @@ -440,13 +461,14 @@ export default function UserDetail({ userId, mode: initialMode }: Props) { | ||
| 440 | {permTab === "groups" ? ( | 461 | {permTab === "groups" ? ( |
| 441 | <PermissionGrid | 462 | <PermissionGrid |
| 442 | categories={permissionCategories} | 463 | categories={permissionCategories} |
| 443 | - selectedIds={permissionCategoryIds} | 464 | + selectedIds={permissionCategoryIds ?? []} |
| 444 | setPermission={(id, v) => | 465 | setPermission={(id, v) => |
| 445 | - setPermissionCategoryIds((s) => | ||
| 446 | - v ? Array.from(new Set([...s, id])) : s.filter((x) => x !== id) | ||
| 447 | - ) | 466 | + setPermissionCategoryIds((s) => { |
| 467 | + const base = s ?? []; | ||
| 468 | + return v ? Array.from(new Set([...base, id])) : base.filter((x) => x !== id); | ||
| 469 | + }) | ||
| 448 | } | 470 | } |
| 449 | - disabled={disabled} | 471 | + disabled={disabled || !permsLoaded} |
| 450 | /> | 472 | /> |
| 451 | ) : ( | 473 | ) : ( |
| 452 | <ScopeTab | 474 | <ScopeTab |
| @@ -494,7 +516,10 @@ function PermissionGrid({ categories, selectedIds, setPermission, disabled }: Pe | @@ -494,7 +516,10 @@ function PermissionGrid({ categories, selectedIds, setPermission, disabled }: Pe | ||
| 494 | const visibleCategories = categories.filter( | 516 | const visibleCategories = categories.filter( |
| 495 | (c) => !filter || c.sCategoryName.includes(filter) || c.sCategoryCode.includes(filter) | 517 | (c) => !filter || c.sCategoryName.includes(filter) || c.sCategoryCode.includes(filter) |
| 496 | ); | 518 | ); |
| 497 | - const allChecked = categories.length > 0 && categories.every((c) => selected.has(c.iIncrement)); | 519 | + // 表头全选只反映/操作"当前可见行",避免过滤态下越权授权或误清。 |
| 520 | + const allChecked = | ||
| 521 | + visibleCategories.length > 0 && | ||
| 522 | + visibleCategories.every((c) => selected.has(c.iIncrement)); | ||
| 498 | 523 | ||
| 499 | return ( | 524 | return ( |
| 500 | <table | 525 | <table |
| @@ -521,7 +546,7 @@ function PermissionGrid({ categories, selectedIds, setPermission, disabled }: Pe | @@ -521,7 +546,7 @@ function PermissionGrid({ categories, selectedIds, setPermission, disabled }: Pe | ||
| 521 | <PrimCheckbox | 546 | <PrimCheckbox |
| 522 | checked={allChecked} | 547 | checked={allChecked} |
| 523 | onChange={(v) => { | 548 | onChange={(v) => { |
| 524 | - categories.forEach((c) => setPermission(c.iIncrement, v)); | 549 | + visibleCategories.forEach((c) => setPermission(c.iIncrement, v)); |
| 525 | }} | 550 | }} |
| 526 | disabled={disabled} | 551 | disabled={disabled} |
| 527 | /> | 552 | /> |