Tôi sẽ đọc kỹ toàn bộ rồi rà soát từng lớp.

Bởi

Đánh giá tổng thể

Cấu trúc này rất tốt — rõ ràng, phân tách trách nhiệm tốt, kiến trúc Key Wrapping là quyết định kỹ thuật đúng đắn. Tuy nhiên tôi phát hiện một số vấn đề cần xử lý trước khi bắt đầu code.

Vấn đề 1 — Logic mâu thuẫn: Recovery Key dùng để làm gì?

Đây là vấn đề nghiêm trọng nhất trong toàn bộ tài liệu.

Tài liệu đặc tả gốc nói:

Recovery Key → mở khóa tạm thời khi sai mật khẩu quá 5 lần

RecoverPasswordWindow nói:

Tab 2 — Recovery Key → gọi ResetPasswordViaRecoveryKey() → reset mật khẩu mới

KeyManager nói:

masterKey = PBKDF2(recoveryKey, salt_recovery) → Recovery Key là nguồn gốc của masterKey

Vậy thực chất Recovery Key có 3 vai trò đang bị lẫn lộn:

Vai tròĐang nằm ở đâuVấn đề
Mở khóa tạm khi sai 5 lầnĐặc tả gốcMở khóa xong thì làm gì? Vẫn cần mật khẩu để unwrap masterKey
Reset mật khẩuRecoverPasswordWindowTrùng với OTP, cần làm rõ ưu tiên
Nguồn gốc masterKeyKeyManagerNếu mất Recovery Key thì mất masterKey → mất toàn bộ dữ liệu

Đề xuất làm rõ:

Recovery Key có 2 chức năng chính thức:
1. Dùng khi quên mật khẩu (thay thế OTP, hoặc khi mất điện thoại)
   → UnwrapMasterKeyViaRecovery() → cho phép đặt mật khẩu mới
2. Dùng khi sai mật khẩu quá 5 lần (mở khóa tạm + bắt buộc đổi mật khẩu ngay)
   → Cũng gọi UnwrapMasterKeyViaRecovery() → bắt buộc đặt mật khẩu mới

OTP Google Authenticator chỉ dùng khi quên mật khẩu (còn nhớ điện thoại)
→ VerifyTotp() → nếu đúng → cho phép đặt mật khẩu mới

Tóm lại: cả OTP và Recovery Key đều dẫn đến cùng 1 kết quả: đặt mật khẩu mới
Khác nhau ở cách xác minh danh tính trước đó

Sau khi làm rõ thì RecoverPasswordWindow cần cập nhật flow cho đúng.

Vấn đề 2 — AuthService bị mồ côi

Trong cấu trúc file có AuthService.cs ở Core (nhắc đến trong mô tả SetupWizard), nhưng không có file AuthService.cs trong cây thư mục. Chức năng của nó bị phân tán:

Chức năngĐang ở đâu
bcrypt hash/verifyCryptoService.cs
TOTP setup/verifyNhắc trong SetupWizard gọi AuthService.SetupTotp() nhưng file không tồn tại
Recovery Key hash/verifyRecoveryKeyGenerator.cs trong Helpers

Đề xuất: Thêm Core/AuthService.cs với chức năng rõ ràng:

  • SetupTotp(secret) — lưu TOTP secret vào header
  • VerifyTotp(code, secret) — xác minh mã 6 số
  • Không làm gì khác — bcrypt giữ ở CryptoService, Recovery Key giữ ở RecoveryKeyGenerator

Vấn đề 3 — DeviceWatcher bị xóa nhưng chức năng vẫn còn

Cấu trúc mới bỏ DeviceWatcher.cs, nhưng logic phát hiện rút USB đột ngột lại được chia ra 2 chỗ:

  • DriveHelper.cs → WatchForEject() + StopWatching()
  • App.xaml.cs → “Đăng ký handler cho USB eject: WMI event”

Vấn đề: DriveHelper đang làm 2 việc — cung cấp thông tin ổ đĩa (đúng chức năng) và theo dõi sự kiện rút USB (lẽ ra không phải việc của nó).

Đề xuất: Giữ DriveHelper chỉ lo thông tin tĩnh. Chuyển WatchForEject() và StopWatching() vào App.xaml.cs trực tiếp, hoặc tách thành Security/DeviceWatcher.cs như phiên bản trước. Dự án này nên chọn tách thành DeviceWatcher.cs vì logic xử lý rút USB đột ngột khá phức tạp (cần gọi CloseVault, ZeroAllKeys, LogService, thông báo UI).

Vấn đề 4 — ContainerHeader.cs ở Core nhưng thực chất là Model

ContainerHeader.cs hiện vừa định nghĩa cấu trúc dữ liệu vừa chứa logic đọc/ghi. Không sai, nhưng cần nhất quán với triết lý của dự án:

  • Nếu giữ nguyên ở Core: đổi tên thành VaultHeaderManager.cs để rõ nó có logic
  • Tên ContainerHeader dễ nhầm với Models/VaultHeader (nếu sau này thêm vào)

Đề xuất nhỏ: Giữ nguyên tên nhưng thêm ghi chú rõ đây là “header manager” không phải pure model.

Vấn đề 5 — Thiếu xử lý CancellationToken ở tầng ViewModel

VaultEngine.AddFiles() có nhận CancellationToken (token hủy tác vụ), nhưng không có ViewModel nào được mô tả là giữ CancellationTokenSource (nguồn phát token hủy) để người dùng có thể bấm nút hủy copy giữa chừng.

Đề xuất: Thêm vào mô tả MainViewModel:

  • CancellationTokenSource _copyCts — tạo mới mỗi lần bắt đầu copy
  • Nút hủy trong progress bar → gọi _copyCts.Cancel()

Vấn đề 6 — Nhỏ nhưng cần lưu ý

AppSettings.cs thiếu HeaderSize — kích thước cố định của header rất quan trọng (offset để biết data block bắt đầu từ đâu), nên khai báo ở đây:

const int HeaderSize = 314; // bytes — tính theo layout trong ContainerHeader

LogService dùng masterKey để mã hóa — điều này có nghĩa log chỉ đọc được khi vault đang mở. Đây là thiết kế đúng về bảo mật, nhưng cần ghi rõ trong mô tả để tránh nhầm sau này.

CopyProgress.IsCancelled — nên đổi thành CancellationReason enum (None / UserCancelled / InsufficientSpace / ThreatDetected) để UI hiển thị thông báo phù hợp từng trường hợp.

Tổng kết — Cần sửa gì trước khi code

Mức độVấn đềHành động
🔴 Nghiêm trọngRecovery Key có 3 vai trò mâu thuẫnLàm rõ flow trong tài liệu trước
🔴 Nghiêm trọngAuthService.cs được gọi nhưng không có trong cây thư mụcThêm vào Core
🟡 Quan trọngDeviceWatcher bị xóa nhưng logic vẫn còn, đang bị phân tánTách lại thành file riêng
🟡 Quan trọngCancellationTokenSource chưa có ở ViewModelBổ sung vào mô tả MainViewModel
🟢 NhỏAppSettings thiếu HeaderSizeThêm 1 dòng
🟢 NhỏCopyProgress.IsCancelled quá đơn giảnĐổi thành enum

Kết luận: Chưa sẵn sàng để bắt đầu code ngay. Cần giải quyết 2 vấn đề đỏ trước — đặc biệt là làm rõ flow của Recovery Key vì nó ảnh hưởng đến KeyManager, ContainerHeader, RecoverPasswordWindow và VaultEngine cùng lúc. Sau khi sửa xong 2 vấn đề đó thì cấu trúc này đủ tốt để bắt đầu Phase 1 (VaultEngine + Core).

Bạn muốn tôi giúp làm rõ flow Recovery Key trước không?

Viết một bình luận