135 lines
7.1 KiB
Markdown
135 lines
7.1 KiB
Markdown
# RBAC getRecordset() Review
|
|
|
|
## Overview
|
|
Review of all `getRecordset()` calls in `interfaceDbChatObjects.py` and `interfaceDbComponentObjects.py` to determine which should be converted to `getRecordsetWithRBAC()`.
|
|
|
|
## Analysis Criteria
|
|
- **Convert to RBAC**: User-facing data that should respect access control
|
|
- **Keep as-is**: Internal/technical operations that don't need RBAC filtering
|
|
|
|
---
|
|
|
|
## interfaceDbChatObjects.py
|
|
|
|
### Summary: **14 calls found - ALL should be converted to `getRecordsetWithRBAC()`**
|
|
|
|
All calls access user-facing data (ChatMessage, ChatDocument, ChatStat, ChatLog) and should respect RBAC even when:
|
|
- Used in cascade delete operations (after parent access is verified)
|
|
- Used to fetch child records (after parent access is verified)
|
|
- Used for existence checks
|
|
|
|
**Rationale**: RBAC should be applied at every data access point to ensure consistent security and prevent potential bypass scenarios.
|
|
|
|
### Detailed List:
|
|
|
|
1. **Line 760** - `deleteWorkflow()` - Cascade delete ChatStat
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatStat, self.currentUser, recordFilter={"messageId": messageId})`
|
|
- **Reason**: Deleting related data should respect RBAC
|
|
|
|
2. **Line 765** - `deleteWorkflow()` - Cascade delete ChatDocument
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatDocument, self.currentUser, recordFilter={"messageId": messageId})`
|
|
- **Reason**: Deleting related data should respect RBAC
|
|
|
|
3. **Line 773** - `deleteWorkflow()` - Cascade delete ChatStat (workflow level)
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatStat, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Deleting related data should respect RBAC
|
|
|
|
4. **Line 778** - `deleteWorkflow()` - Cascade delete ChatLog
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatLog, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Deleting related data should respect RBAC
|
|
|
|
5. **Line 821** - `getMessages()` - Fetch messages for workflow
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatMessage, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Child records should still respect RBAC even if parent access is verified
|
|
|
|
6. **Line 1062** - `updateMessage()` - Check if message exists
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatMessage, self.currentUser, recordFilter={"id": messageId})`
|
|
- **Reason**: Existence checks should respect RBAC
|
|
|
|
7. **Line 1167** - `deleteMessage()` - Cascade delete ChatStat
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatStat, self.currentUser, recordFilter={"messageId": messageId})`
|
|
- **Reason**: Deleting related data should respect RBAC
|
|
|
|
8. **Line 1172** - `deleteMessage()` - Cascade delete ChatDocument
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatDocument, self.currentUser, recordFilter={"messageId": messageId})`
|
|
- **Reason**: Deleting related data should respect RBAC
|
|
|
|
9. **Line 1199** - `deleteFileFromMessage()` - Get documents for message
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatDocument, self.currentUser, recordFilter={"messageId": messageId})`
|
|
- **Reason**: Accessing related data should respect RBAC
|
|
|
|
10. **Line 1242** - `getDocuments()` - Get documents for message
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatDocument, self.currentUser, recordFilter={"messageId": messageId})`
|
|
- **Reason**: Public method accessing user data should respect RBAC
|
|
|
|
11. **Line 1291** - `getLogs()` - Fetch logs for workflow
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatLog, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Child records should still respect RBAC even if parent access is verified
|
|
|
|
12. **Line 1410** - `getStats()` - Fetch stats for workflow
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatStat, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Child records should still respect RBAC even if parent access is verified
|
|
|
|
13. **Line 1460** - `getUnifiedChatData()` - Fetch messages for workflow
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatMessage, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Child records should still respect RBAC even if parent access is verified
|
|
|
|
14. **Line 1501** - `getUnifiedChatData()` - Fetch logs for workflow
|
|
- **Action**: Convert to `getRecordsetWithRBAC(ChatLog, self.currentUser, recordFilter={"workflowId": workflowId})`
|
|
- **Reason**: Child records should still respect RBAC even if parent access is verified
|
|
|
|
---
|
|
|
|
## interfaceDbComponentObjects.py
|
|
|
|
### Summary: **3 calls found - 1 keep as-is, 2 should be converted**
|
|
|
|
### Detailed List:
|
|
|
|
1. **Line 149** - `_initializeStandardPrompts()` - Check if prompts exist
|
|
- **Action**: **KEEP AS-IS** ✅
|
|
- **Reason**: This is initialization code that runs during bootstrap. It checks if any prompts exist to avoid re-initialization. Since this runs with root user context and is a system-level check, RBAC is not needed here.
|
|
|
|
2. **Line 947** - `deleteFile()` - Get FileData for deletion
|
|
- **Action**: **CONVERT** to `getRecordsetWithRBAC(FileData, self.currentUser, recordFilter={"id": fileId})`
|
|
- **Reason**: FileData stores binary data associated with FileItem. While it's a technical table, we should still respect RBAC for consistency and security. The file access was already checked via `getFile()`, but FileData access should also be RBAC-filtered.
|
|
|
|
3. **Line 1032** - `getFileData()` - Get FileData for reading
|
|
- **Action**: **CONVERT** to `getRecordsetWithRBAC(FileData, self.currentUser, recordFilter={"id": fileId})`
|
|
- **Reason**: FileData access should respect RBAC. The file access was already checked via `getFile()`, but FileData access should also be RBAC-filtered for consistency.
|
|
|
|
**Note on FileData**: FileData is a technical table storing binary file content. However, for consistency and security, RBAC should still be applied. If FileData doesn't have RBAC rules defined, the RBAC filter will effectively be a no-op (allowing access), but the pattern is consistent.
|
|
|
|
---
|
|
|
|
## Implementation Priority
|
|
|
|
### High Priority (User-facing data access)
|
|
- All `interfaceDbChatObjects.py` calls (14 calls)
|
|
- `interfaceDbComponentObjects.py` FileData calls (2 calls)
|
|
|
|
### Low Priority (System initialization)
|
|
- `interfaceDbComponentObjects.py` Prompt initialization check (1 call) - Keep as-is
|
|
|
|
---
|
|
|
|
## Next Steps
|
|
|
|
1. Convert all 14 calls in `interfaceDbChatObjects.py` to `getRecordsetWithRBAC()`
|
|
2. Convert 2 FileData calls in `interfaceDbComponentObjects.py` to `getRecordsetWithRBAC()`
|
|
3. Keep 1 Prompt initialization check as-is
|
|
4. Test all changes to ensure RBAC filtering works correctly
|
|
5. Verify cascade delete operations still work correctly with RBAC
|
|
|
|
---
|
|
|
|
## Testing Checklist
|
|
|
|
After conversion, verify:
|
|
- [ ] Workflow deletion still works (cascade deletes)
|
|
- [ ] Message deletion still works (cascade deletes)
|
|
- [ ] File deletion still works (FileData cleanup)
|
|
- [ ] File reading still works (FileData access)
|
|
- [ ] Child record access (messages, logs, stats, documents) respects RBAC
|
|
- [ ] Users can only access data they have permission for
|
|
- [ ] No performance degradation from RBAC filtering
|