Files
alla-tms/docs/role-permission-review.md
2026-07-16 09:53:14 +07:00

258 lines
15 KiB
Markdown

# Role Permission Review
วันที่ตรวจสอบ: 2026-06-30
## Scope
เอกสารนี้เป็นการตรวจสอบระบบสิทธิ์การเข้าถึงของ 3 กลุ่มหลัก:
- Employee
- HRD Admin
- IT Admin
ขอบเขตที่ตรวจ:
- การมองเห็นเมนูใน sidebar
- การกัน direct URL ที่ระดับ page
- การกันสิทธิ์ที่ระดับ API / server-side
- การจำกัดขอบเขตข้อมูล
- ความสอดคล้องระหว่าง requirement กับ implementation ปัจจุบัน
ไฟล์หลักที่ใช้ตรวจ:
- `src/config/nav-config.ts`
- `src/hooks/use-nav.ts`
- `src/lib/auth/roles.ts`
- `src/lib/auth/session.ts`
- `src/lib/auth/page-guards.ts`
- `src/proxy.ts`
- `src/app/dashboard/**`
- `src/app/api/**`
## Requirement Summary
จากแผนงานที่ให้ตรวจ ระบบนี้ควรแยกสิทธิ์อย่างน้อยเป็น 3 ระดับ:
- Employee เห็นเฉพาะส่วนที่ใช้กับตัวเอง และข้อมูลต้องถูก scope เป็นของตัวเอง
- HRD Admin จัดการข้อมูลฝั่งงานอบรมและข้อมูลบุคลากรในองค์กรได้
- IT Admin จัดการสิทธิ์ระดับสูงกว่า HRD และเข้าถึงส่วน technical / audit ได้
ประเด็นสำคัญที่แผนเน้นเป็นพิเศษ:
- เมนูและหน้าจอต้องสอดคล้องกับสิทธิ์จริง
- ห้ามพึ่งแค่ซ่อนเมนู ต้องกันที่ server ด้วย
- ข้อมูลของ Employee ต้องไม่ทะลุไปเห็นของคนอื่น
- ฟังก์ชัน `Import พนักงาน` ควรถูกทบทวน/ถอดออกจาก flow ถ้าไม่ได้ต้องการใช้งานแล้ว
## Findings
### 1. High: หน้าและ API `Import พนักงาน` ยังเปิดใช้งานอยู่
สถานะ: `Fail`
หลักฐาน:
- [src/app/dashboard/employee-directory/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/employee-directory/page.tsx:54) ยังมีปุ่มลิงก์ไป `/dashboard/import-employees`
- [src/app/dashboard/import-employees/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/import-employees/page.tsx:5) ยังเปิดหน้า import
- [src/app/api/import-employees/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/import-employees/route.ts:401) ยังเปิด API import โดยใช้ `requireHRD()`
ผลกระทบ:
- ถ้า requirement ล่าสุดต้องการยกเลิก flow นี้ ปัจจุบันถือว่ายังไม่ปิดจริง
- ผู้ใช้ HRD / IT ยังเข้าใช้งานได้ผ่านปุ่ม, direct URL และ API
คำแนะนำ:
- ถ้าต้องการถอดฟังก์ชันนี้จริง ควรลบปุ่ม, ปิดหน้า route, ปิด API และตรวจ flow ที่เกี่ยวข้องกับ `master-review` ต่อด้วย
### 2. Medium: สิทธิ์ฝั่ง Employee สำหรับเมนูบางหน้าไม่สอดคล้องกับสิทธิ์จริงของหน้า
สถานะ: `Need Review`
หลักฐาน:
- เมนู `Reports` ใน [src/config/nav-config.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/config/nav-config.ts:103) จำกัดเป็น `businessRole: 'HRD'`
- แต่หน้า reports ใช้ [src/app/dashboard/reports/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/reports/page.tsx:20) ผ่าน `requireEmployeeDashboardAccess()`
- data scope ของ reports ก็รองรับ employee แบบ self-scope ใน [src/features/reports/server/report-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/reports/server/report-data.ts:126)
ข้อสังเกตเพิ่ม:
- หน้า `Announcements` ใช้ `requireEmployeeDashboardAccess()` ใน [src/app/dashboard/announcements/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/announcements/page.tsx:18)
- หน้า `Notifications` ใช้ `requireEmployeeDashboardAccess()` ใน [src/app/dashboard/notifications/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/notifications/page.tsx:16)
- แต่ใน `nav-config.ts` เมนู `Announcements` ถูกวางไว้ในกลุ่ม HRD/IT เท่านั้น และ `Notifications` ไม่มีใน sidebar
ผลกระทบ:
- สิทธิ์จริงของระบบกับสิ่งที่ผู้ใช้เห็นในเมนูไม่ตรงกัน
- ผู้ใช้ employee อาจ “เข้าได้แต่ไม่เห็นเมนู”
คำแนะนำ:
- ตกลงให้ชัดว่าหน้าใดควรเป็น employee-facing
- จากนั้น sync ระหว่าง `nav-config.ts` กับ page/API permission ให้ตรงกัน
### 3. Medium: โมเดลสิทธิ์ IT กับ HRD ยังพึ่งการตีความทางอ้อมมากกว่าตารางสิทธิ์ที่ชัด
สถานะ: `Need Review`
หลักฐาน:
- `super_admin` ถูก map เป็น `IT` ใน [src/lib/auth/roles.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/lib/auth/roles.ts:48)
- `isHRD()` คืนค่า `true` ให้ทั้ง HRD และ super admin ใน [src/lib/auth/roles.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/lib/auth/roles.ts:63)
- `requireHRD()` จึงเปิดให้ IT ผ่านได้ด้วยใน [src/lib/auth/session.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/lib/auth/session.ts:143)
ผลกระทบ:
- ตอนนี้ระบบทำงานแบบ “IT เข้าส่วน HRD ได้ทั้งหมด” ซึ่งอาจถูกต้องก็ได้
- แต่ถ้าอนาคตอยากแยกสิทธิ์ HRD กับ IT ให้ต่างกันมากขึ้น จะตามแก้ยาก เพราะ logic ผูกกันอยู่หลายชั้น
คำแนะนำ:
- ถ้าต้องการให้ IT = สิทธิ์มากกว่า HRD แบบตั้งใจ ถือว่าใช้งานได้
- แต่ถ้าต้องการ matrix สิทธิ์ที่ตรวจสอบง่าย ควรมี role-permission matrix กลางที่อ่านแล้วตอบได้ทันทีว่าแต่ละ role ทำอะไรได้บ้าง
### 4. Low: เอกสาร RBAC ปัจจุบันไม่ตรงกับ implementation บางจุด
สถานะ: `Need Review`
หลักฐาน:
- [docs/nav-rbac.md](/d:/WY-2569/HRD/training-system-minimal-refactor/docs/nav-rbac.md:85) ระบุว่า HRD เข้าถึง `Audit Logs` ได้
- แต่เมนู `Audit Logs` ใน [src/config/nav-config.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/config/nav-config.ts:94) จำกัด `systemRole: 'super_admin'`
- และหน้า audit log ใช้ [src/app/dashboard/audit-logs/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/audit-logs/page.tsx:16) ผ่าน `requireITDashboardAccess()`
ผลกระทบ:
- คนดูเอกสารอาจเข้าใจสิทธิ์ผิด
- เสี่ยงตัดสินใจต่อยอด feature จากเอกสารที่ไม่ตรงโค้ดจริง
คำแนะนำ:
- อัปเดตเอกสารให้ตรงกับ implementation ปัจจุบัน
## Menu Visibility Review
### Employee
ผลที่พบ:
- เห็น `Overview`, `Training Records`, `Online Lessons`
- ไม่เห็น `Reports` จาก sidebar แม้ว่าหน้า reports จะเข้าได้จริง
- ไม่เห็น `Announcements` และ `Notifications` จาก sidebar แม้ว่าหน้าเหล่านี้เปิดสิทธิ์ employee
สรุป: `Partial`
### HRD Admin
ผลที่พบ:
- เห็น `Pending Review`
- เห็นกลุ่ม `ข้อมูลหลัก` และเข้าถึง `Employee Directory`, `Courses`, `Training Policy`, `Announcements`
- เข้า `Import Employees` ได้ผ่าน direct URL และจากปุ่มในหน้า employee directory
- ไม่เห็น `Users` เพราะเมนูนั้นเปิดเฉพาะ IT
สรุป: `Mostly Pass`
### IT Admin
ผลที่พบ:
- เห็นเมนู `Users`
- เห็นส่วนของ HRD ได้ทั้งหมดผ่าน role mapping
- เห็น `Audit Logs` เฉพาะ `super_admin`
สรุป: `Pass`
## Route Protection Review
### ผ่าน
- หน้า employee-facing ใช้ `requireEmployeeDashboardAccess()`
- หน้า HRD ใช้ `requireHRDDashboardAccess()`
- หน้า IT ใช้ `requireITDashboardAccess()` หรือ `requireUserManagementDashboardAccess()`
หลักฐานสำคัญ:
- [src/lib/auth/page-guards.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/lib/auth/page-guards.ts:23)
- [src/app/dashboard/users/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/users/page.tsx:18)
- [src/app/dashboard/employee-directory/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/employee-directory/page.tsx:20)
- [src/app/dashboard/pending-review/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/pending-review/page.tsx:21)
- [src/app/dashboard/audit-logs/page.tsx](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/dashboard/audit-logs/page.tsx:16)
### ข้อสังเกต
- [src/proxy.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/proxy.ts:27) กันได้แค่ “ล็อกอินหรือยัง”
- การกัน “สิทธิ์ role ไหนเข้าได้” ยังต้องพึ่ง page guard และ API handler ต่อ ซึ่งตอนนี้ถือว่าทำถูกทิศทางแล้ว
สรุป: `Pass`
## API / Server Action Permission Review
### ผ่าน
- `Users API` เปิดเฉพาะ IT ผ่าน `requireUserManagementAccess()`
- [src/app/api/users/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/users/route.ts:23)
- [src/app/api/users/[id]/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/users/[id]/route.ts:25)
- `Audit Logs API` เปิดเฉพาะ IT ผ่าน `requireIT()`
- [src/app/api/audit-logs/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/audit-logs/route.ts:8)
- `Employee Directory`, `Courses`, `Training Policies`, `Import Employees`, `Master Review` เปิดเฉพาะ HRD/IT ผ่าน `requireHRD()`
### ผ่านแบบมีเงื่อนไข
- `Training Records` ใช้ `requireOrganizationAccess()` แล้วคุมต่อด้วย scope และ review permission ภายใน service
- `Reports Export` กัน employee ไม่ให้ export รายงานรวม โดยอนุญาตเฉพาะ `employeeTranscript`
- [src/app/api/reports/export/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/reports/export/route.ts:42)
สรุป: `Pass`
## Data Scope Review
### Employee
ผ่าน:
- training records ถูก scope ตาม employee mapping ของ user
- [src/features/training-records/server/training-record-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/training-records/server/training-record-data.ts:224)
- reports ถูก scope ตาม employee ของตัวเอง ถ้า role ไม่ใช่ owner/admin
- [src/features/reports/server/report-data.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/features/reports/server/report-data.ts:126)
### HRD / IT
ผ่าน:
- reports ระดับองค์กรเปิดให้ membership role `owner/admin`
- training review เปิดเฉพาะ role ที่ review ได้
- [src/app/api/training-records/[id]/review/route.ts](/d:/WY-2569/HRD/training-system-minimal-refactor/src/app/api/training-records/[id]/review/route.ts:31)
สรุป: `Pass`
## Hardcoded Roles Review
สิ่งที่พบ:
- role string หลักกระจุกอยู่ใน `src/lib/auth/roles.ts`
- แต่ยังมีการอ้าง role ตรง ๆ ซ้ำใน `nav-config.ts`, `session.ts`, เอกสาร, และบาง feature
ประเมิน:
- ยังไม่ถึงขั้นกระจายมั่ว
- แต่ยังไม่มี permission matrix กลางที่เป็น single source of truth
สรุป: `Acceptable but should improve later`
## Summary
ภาพรวมปัจจุบัน:
- ระบบกันสิทธิ์ที่ระดับ page และ API ถือว่าใช้ได้
- data scope ของ employee ฝั่ง training records และ reports ถือว่าปลอดภัยในระดับหนึ่ง
- จุดที่ไม่ตรง requirement ชัดที่สุดคือ `Import พนักงาน` ยังไม่ถูกถอดออกจริง
- อีกจุดที่ควรรีบจัดระเบียบคือความไม่ตรงกันระหว่าง “เมนูที่เห็น” กับ “สิทธิ์จริงของหน้า” โดยเฉพาะ `Reports`, `Announcements`, `Notifications`
## Recommended Next Actions
1. ตัดสินใจให้ชัดว่า `Import พนักงาน` จะเก็บหรือจะถอด แล้วปิดทั้ง UI, route, API ให้ครบ
2. ทำ matrix สิทธิ์ของ `Employee / HRD / IT` แบบสั้น ๆ แล้วใช้เป็นตัวเทียบกับ `nav-config.ts`
3. sync `nav-config.ts` กับ page/API permission ให้ผู้ใช้เห็นเมนูตรงกับสิทธิ์จริง
4. อัปเดต `docs/nav-rbac.md` ให้ตรงกับโค้ดปัจจุบัน