Open tangsky001 opened 3 months ago
1、权限验证逻辑错误 在add方法中,只有角色ID为1的用户可以添加角色,而在delete方法中,只有角色ID为0的用户可以删除角色。这里存在逻辑不一致的问题。 2、SQL注入风险 虽然代码中没有直接的SQL语句,但通过roleService进行数据库操作时,如果roleService的实现没有正确处理输入参数,可能会存在SQL注入的风险。 3、重复调用 在all方法中,roleService.getAll()被调用了两次,这是没必要的,应该调用一次并存储结果以供后续使用。 4、重复代码 在add和delete方法中,获取当前登录用户ID的代码是重复的 5、异常处理 在add和delete方法中,当操作失败时抛出的是ControllerException,但没有提供具体的错误信息。
(建议)
public class RoleController {
private static final int ADMIN_ROLE_ID = 1; // 定义管理员角色ID常量 @Autowired private IRoleService roleService; private int getCurrentRoleId() { return SecurityUtils.getUserInfo().getRole().getId(); } @PostMapping("/add") @Operation(summary = "添加角色", security = @SecurityRequirement(name = JwtUtils.HEADER)) @Parameter(name = "name", description = "角色名", required = true) @Parameter(name = "authority", description = "权限(自行定义内容,目前未使用)", required = true) public void add(@RequestParam String name, @RequestParam(required = false) String authority) { if (getCurrentRoleId() != ADMIN_ROLE_ID) { throw new ControllerException("只有管理员可以添加角色", ErrorCode.ERROR403); } Role role = new Role(); role.setName(name); role.setAuthority(authority); role.setCreateTime(DateUtil.getNow()); role.setUpdateTime(DateUtil.getNow()); int addResult = roleService.add(role); if (addResult0) { throw new ControllerException("添加角色失败", ErrorCode.ERROR100); } } @DeleteMapping("/delete") @Operation(summary = "删除角色", security = @SecurityRequirement(name = JwtUtils.HEADER)) @Parameter(name = "id", description = "用户Id", required = true) public void delete(@RequestParam Integer id) { // 获取当前登录用户id if (getCurrentRoleId() != ADMIN_ROLE_ID) { throw new ControllerException("只有管理员可以删除角色", ErrorCode.ERROR403); }
int deleteResult = roleService.delete(id); if (deleteResult <= 0) { throw new ControllerException("删除角色失败", ErrorCode.ERROR100); } }
@GetMapping("/all") @Operation(summary = "查询角色", security = @SecurityRequirement(name = JwtUtils.HEADER)) public List<Role> all() { // 获取当前登录用户id return roleService.getAll(); }
}
1、权限验证逻辑错误 在add方法中,只有角色ID为1的用户可以添加角色,而在delete方法中,只有角色ID为0的用户可以删除角色。这里存在逻辑不一致的问题。 2、SQL注入风险 虽然代码中没有直接的SQL语句,但通过roleService进行数据库操作时,如果roleService的实现没有正确处理输入参数,可能会存在SQL注入的风险。 3、重复调用 在all方法中,roleService.getAll()被调用了两次,这是没必要的,应该调用一次并存储结果以供后续使用。 4、重复代码 在add和delete方法中,获取当前登录用户ID的代码是重复的 5、异常处理 在add和delete方法中,当操作失败时抛出的是ControllerException,但没有提供具体的错误信息。
(建议)
public class RoleController {
int deleteResult = roleService.delete(id); if (deleteResult <= 0) { throw new ControllerException("删除角色失败", ErrorCode.ERROR100); } }
}