130 lines
4.0 KiB
Markdown
130 lines
4.0 KiB
Markdown
|
# 未修复问题记录
|
|||
|
|
|
|||
|
|
本文档记录 20260329 审查中发现的需要架构重构才能修复的问题。
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 1. TOTP 恢复码删除非原子
|
|||
|
|
|
|||
|
|
**严重程度**: 中危
|
|||
|
|
**文件**: `internal/service/auth_service.go`
|
|||
|
|
**问题描述**: 删除恢复码时使用循环逐个删除，存在并发安全问题。如果在删除过程中发生错误，已删除的恢复码无法恢复。
|
|||
|
|
|
|||
|
|
**当前代码**:
|
|||
|
|
```go
|
|||
|
|
// 遍历删除每个恢复码
|
|||
|
|
for _, code := range codes {
|
|||
|
|
if err := s.totpRepo.DeleteRecoveryCode(ctx, userID, code); err != nil {
|
|||
|
|
return err
|
|||
|
|
}
|
|||
|
|
}
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
**修复方案**: 需要使用数据库事务支持，将所有恢复码删除操作放在一个事务中完成。
|
|||
|
|
|
|||
|
|
**是否可快速修复**: 否，需要：
|
|||
|
|
- 确认 TotpRepository 接口是否支持事务
|
|||
|
|
- 可能需要重构 repository 层以支持事务
|
|||
|
|
- 需要全面的回归测试
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 2. social_account_repo.go 使用原生 SQL 而非 GORM
|
|||
|
|
|
|||
|
|
**严重程度**: 中危
|
|||
|
|
**文件**: `internal/repository/social_account_repo.go`
|
|||
|
|
**问题描述**: 该仓库实现使用原生 SQL 而非 GORM ORM，与其他仓库实现不一致。
|
|||
|
|
|
|||
|
|
**影响**:
|
|||
|
|
- 代码风格不统一
|
|||
|
|
- 无法利用 GORM 的高级特性（如自动迁移、软删除、钩子等）
|
|||
|
|
- 增加 SQL 注入风险（虽然当前代码使用了参数化查询，风险较低）
|
|||
|
|
|
|||
|
|
**修复方案**: 重写为使用 GORM 的方式：
|
|||
|
|
```go
|
|||
|
|
func (r *SocialAccountRepositoryImpl) Create(ctx context.Context, account *domain.SocialAccount) error {
|
|||
|
|
return r.db.WithContext(ctx).Create(account).Error
|
|||
|
|
}
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
**是否可快速修复**: 否，需要：
|
|||
|
|
- 大规模重构仓库实现
|
|||
|
|
- 确保所有查询逻辑与现有 SQL 语义一致
|
|||
|
|
- 更新相关测试
|
|||
|
|
- 回归测试验证
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 3. React 双重状态管理
|
|||
|
|
|
|||
|
|
**严重程度**: 中危
|
|||
|
|
**文件**: `frontend/admin/src/app/providers/AuthProvider.tsx`
|
|||
|
|
**问题描述**: AuthProvider 同时使用 React useState 和模块级别的存储（getCurrentUser、setCurrentUser 等）来管理用户状态。这种双重管理增加了复杂性。
|
|||
|
|
|
|||
|
|
**当前设计**:
|
|||
|
|
```typescript
|
|||
|
|
// React 状态
|
|||
|
|
const [user, setUser] = useState<SessionUser | null>(getCurrentUser())
|
|||
|
|
// 模块级别存储（用于页面刷新后恢复状态）
|
|||
|
|
const effectiveUser = user ?? getCurrentUser()
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
**修复方案**: 两种选择
|
|||
|
|
1. **移除模块存储**: 仅使用 React Context，完全依赖会话恢复 API
|
|||
|
|
2. **移除 React 状态**: 完全依赖模块存储，移除 setUser 调用
|
|||
|
|
|
|||
|
|
**是否可快速修复**: 否，需要：
|
|||
|
|
- 评估对用户体验的影响（页面刷新后的状态恢复）
|
|||
|
|
- 确保所有状态更新路径正确
|
|||
|
|
- 全面回归测试
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 4. ProfileSecurityPage 状态管理
|
|||
|
|
|
|||
|
|
**严重程度**: 中危
|
|||
|
|
**文件**: `frontend/admin/src/pages/admin/ProfileSecurityPage/ProfileSecurityPage.tsx`
|
|||
|
|
**问题描述**: 该页面管理约 20+ 个状态变量，导致代码复杂度和维护成本增加。
|
|||
|
|
|
|||
|
|
**当前状态变量**:
|
|||
|
|
- passwordLoading, avatarLoading, totpLoading
|
|||
|
|
- setupVisible, disableVisible, bindVisible, unbindVisible
|
|||
|
|
- totpCode, disableCode, totpEnabled, totpSetup
|
|||
|
|
- devicesLoading, devices, loginLogs, operationLogs, logsLoading
|
|||
|
|
- socialLoading, socialAccounts, oauthProviders
|
|||
|
|
- emailBindingEnabled, phoneBindingEnabled
|
|||
|
|
- socialSubmitting, activeProvider
|
|||
|
|
- 以及表单实例等
|
|||
|
|
|
|||
|
|
**修复方案**: 使用复合组件模式拆分：
|
|||
|
|
- `PasswordSection`
|
|||
|
|
- `AvatarSection`
|
|||
|
|
- `TOTPSection`
|
|||
|
|
- `ContactBindingsSection`（已存在）
|
|||
|
|
- `SocialAccountsSection`
|
|||
|
|
- `DevicesSection`
|
|||
|
|
- `LoginLogsSection`
|
|||
|
|
- `OperationLogsSection`
|
|||
|
|
|
|||
|
|
**是否可快速修复**: 否，需要：
|
|||
|
|
- 重构页面组件结构
|
|||
|
|
- 提取子组件
|
|||
|
|
- 状态提升或使用状态管理库
|
|||
|
|
- 全面回归测试
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 修复优先级建议
|
|||
|
|
|
|||
|
|
| 问题 | 优先级 | 建议 |
|
|||
|
|
|------|--------|------|
|
|||
|
|
| TOTP 恢复码非原子 | 高 | 后续 sprint 修复 |
|
|||
|
|
| social_account_repo GORM 重构 | 中 | 技术债务，跟踪 |
|
|||
|
|
| React 双重状态管理 | 低 | 评估后决定 |
|
|||
|
|
| ProfileSecurityPage 重构 | 低 | 如需维护该页面则修复 |
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
*文档创建日期: 2026-03-29*
|
|||
|
|
*来源: PROJECT_REVIEW_REPORT.md 审查报告*
|