feat: ABAC Logs Tab #37633
feat: ABAC Logs Tab #37633
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughAdds an ABAC "Logs" admin tab and UI component that lists ABAC audit events (GET /v1/abac/audit) with date-range filtering and pagination; extends client ABACQueryKeys with a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant AdminUI as Admin ABAC UI
participant Client as Browser (React)
participant API as Server API (/v1/abac/audit)
participant DB as Audit Store
AdminUI ->> Client: open "Logs" tab or change filters/pagination
Client ->> API: GET /v1/abac/audit?start=&end=&offset=&count=
API ->> DB: query audit events (date filters, offset, count)
DB -->> API: return events + total/count
API -->> Client: respond with typed ABAC event union (events, total, offset, count)
Client ->> AdminUI: render table, pagination, avatars, or no-results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-11-27T17:56:26.050ZApplied to files:
📚 Learning: 2025-10-30T19:30:46.541ZApplied to files:
📚 Learning: 2025-10-27T14:38:46.994ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
MartinSchoeler
force-pushed
the
feat/ABAC-Logs-page
branch
from
afd977f to
52c486e
Compare
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 18:29", "12/02 19:40 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 11 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 02 Dec 2025 19:40:43 GMT |
MartinSchoeler
force-pushed
the
feat/ABAC-Logs-page
branch
4 times, most recently
from
17372c9 to
c167891
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/i18n/src/locales/en.i18n.json (2)
43-44: Copy tweak for clarity.Hyphenation reads awkwardly. Prefer simpler phrasing.
- "ABAC_No_logs_description": "ABAC-management related activity will appear here.", + "ABAC_No_logs_description": "ABAC management activity will appear here.",
49-58: Align sentence casing with nearby “No …” strings.“ABAC_No_attributes” uses title case while “ABAC_No_rooms” is sentence case. Make them consistent (sentence case).
- "ABAC_No_attributes": "No Attributes", + "ABAC_No_attributes": "No attributes",apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx (1)
13-13: Unused import detected.
ContextualbarSkeletonBodyis imported but doesn't appear to be used in this file.-import { ContextualbarDialog, ContextualbarSkeletonBody } from '../../../components/Contextualbar'; +import { ContextualbarDialog } from '../../../components/Contextualbar';apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (4)
42-45: Remove code comment per coding guidelines.As per coding guidelines, avoid code comments in TypeScript/JavaScript implementations.
- // Whenever the user changes the filter or the text, reset the pagination to the first page useEffect(() => { setCurrent(0); }, [startDate, endDate, setCurrent]);
94-95: Track the TODO for endpoint improvement.The
@ts-expect-errorindicates the endpoint doesn't send the room name. Consider creating an issue to track this improvement so it doesn't get forgotten.Would you like me to open a new issue to track adding the room name to the
/v1/abac/auditendpoint response?
130-134: Consider adding a loading state indicator.When
isLoadingis true, the table renders empty sincedata?.events?.mapreturns nothing. Consider showing a loading skeleton or spinner to improve user experience.- {(!data || data.events?.length === 0) && !isLoading ? ( + {isLoading ? ( + <Box display='flex' justifyContent='center' alignItems='center' height='full'> + {/* Add loading spinner or skeleton here */} + </Box> + ) : !data || data.events?.length === 0 ? ( <Box display='flex' justifyContent='center' height='full'> <GenericNoResults icon='extended-view' title={t('ABAC_No_logs')} description={t('ABAC_No_logs_description')} /> </Box>
52-75: Consider extracting pure helper function.
getActionLabelis a pure function that doesn't depend on component state. Extracting it outside the component avoids recreation on each render.// Move outside component const getActionLabel = (t: TFunction, action?: AbacAttributeDefinitionChangeType | null) => { switch (action) { case 'created': return t('Created'); // ... rest of cases } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/meteor/client/lib/queryKeys.ts(1 hunks)apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx(1 hunks)apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx(2 hunks)apps/meteor/client/views/admin/ABAC/AdminABACTabs.tsx(1 hunks)packages/i18n/src/locales/en.i18n.json(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/admin/ABAC/AdminABACPage.tsxapps/meteor/client/views/admin/ABAC/AdminABACLogs.tsxapps/meteor/client/views/admin/ABAC/AdminABACTabs.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
📚 Learning: 2025-11-27T17:56:26.027Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.027Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/lib/queryKeys.tsapps/meteor/client/views/admin/ABAC/AdminABACPage.tsxapps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
📚 Learning: 2025-10-27T14:38:46.994Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-10-24T17:32:05.348Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
Applied to files:
packages/i18n/src/locales/en.i18n.json
🧬 Code graph analysis (1)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (5)
apps/meteor/client/components/GenericTable/hooks/usePagination.ts (1)
usePagination(11-40)apps/meteor/client/lib/queryKeys.ts (1)
ABACQueryKeys(121-132)packages/core-typings/src/ServerAudit/IAuditServerAbacAction.ts (1)
AbacAttributeDefinitionChangeType(8-17)packages/rest-typings/src/index.ts (1)
OperationResult(195-197)apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts (1)
isLoading(279-282)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
apps/meteor/client/lib/queryKeys.ts (1)
123-126: ABAC logs query keys are consistent and correctly typedThe new
ABACQueryKeys.logsnamespace mirrors the existingroomAttributesstructure, composes fromABACQueryKeys.all, and usesPaginatedRequestwithas consttuples consistently with the rest of this file. I don’t see any correctness or typing issues here.packages/i18n/src/locales/en.i18n.json (2)
128-128: LGTM onABAC_Logs.Label reads well and matches the tab intent.
5498-5498: No duplicate or collision issues found with theUpdatedkey.The search confirms that
Updatedis a distinct key separate fromUpdated_at(which translates to "Updated at"). There is only one definition ofUpdatedat line 5498, and it's legitimately used in the codebase (e.g., in AdminABACLogs.tsx for a status label). The generic key is appropriate for its intended purpose.apps/meteor/client/views/admin/ABAC/AdminABACTabs.tsx (1)
23-25: LGTM!The new "logs" tab follows the established pattern for existing tabs, with consistent use of the translation hook and navigation handler.
apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx (1)
69-69: LGTM!The logs tab integration follows the established conditional rendering pattern used for other ABAC tabs.
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (1)
27-50: Data fetching implementation looks good.The query structure follows best practices with proper cache key management via
ABACQueryKeys.logs.logsList(query)and appropriate date defaults.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (1)
1-178: Great work! Previous review feedback has been addressed.The timestamp handling (lines 90, 100) and date formatting (line 160) have been properly implemented using
new Date(event.ts)andformatDate(). The double margin issue has also been resolved.A few optional UX improvements to consider:
Loading state: While the component handles the
isLoadingflag in the conditional (line 132), users see an empty table during initial load. Consider showing a loading skeleton or spinner.Default date range (lines 28-29): Defaulting both start and end dates to today may not surface historical logs. Consider defaulting to a wider range (e.g., last 7 or 30 days) for better initial UX.
Room name display (line 97-98): The TODO is noted. The component currently displays room IDs instead of names, which may confuse users.
Do you want me to help implement any of these improvements, such as generating a loading state component or adjusting the default date range?
packages/i18n/src/locales/en.i18n.json (2)
43-44: Grammar tweak: “ABAC‑related activity …”Polish the description for readability.
- "ABAC_No_logs_description": "ABAC-management related activity will appear here.", + "ABAC_No_logs_description": "ABAC-related activity will appear here.",
49-58: Normalize empty‑state casing for consistency.Match sentence case used elsewhere (e.g., “No rooms”).
- "ABAC_No_attributes": "No Attributes", + "ABAC_No_attributes": "No attributes",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/meteor/client/lib/queryKeys.ts(1 hunks)apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx(1 hunks)packages/i18n/src/locales/en.i18n.json(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsxapps/meteor/client/lib/queryKeys.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
📚 Learning: 2025-11-07T14:50:33.544Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37423
File: packages/i18n/src/locales/en.i18n.json:18-18
Timestamp: 2025-11-07T14:50:33.544Z
Learning: Rocket.Chat settings: in apps/meteor/ee/server/settings/abac.ts, the Abac_Cache_Decision_Time_Seconds setting uses invalidValue: 0 as the fallback when ABAC is unlicensed. With a valid license, admins can still set the value to 0 to intentionally disable the ABAC decision cache.
Applied to files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
📚 Learning: 2025-10-30T19:30:46.541Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
Applied to files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsxapps/meteor/client/lib/queryKeys.ts
📚 Learning: 2025-10-27T14:38:46.994Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-10-24T17:32:05.348Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
Applied to files:
packages/i18n/src/locales/en.i18n.json
🧬 Code graph analysis (1)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (5)
apps/meteor/client/components/GenericTable/hooks/usePagination.ts (1)
usePagination(11-40)apps/meteor/client/lib/queryKeys.ts (1)
ABACQueryKeys(121-132)packages/core-typings/src/ServerAudit/IAuditServerAbacAction.ts (1)
AbacAttributeDefinitionChangeType(8-17)packages/rest-typings/src/index.ts (1)
OperationResult(195-197)apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts (1)
isLoading(279-282)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/client/lib/queryKeys.ts (1)
123-126: LGTM! Query key structure follows established patterns.The new
logsnamespace is well-structured and consistent with other query keys in the file. Theall()andlist(query)pattern matches the existingroomAttributesstructure above.packages/i18n/src/locales/en.i18n.json (3)
28-31: ABAC labels added — looks good; confirm migration from plural key.New singular keys are clear. Please confirm no remaining UI references to the older “ABAC_Room_Attributes” (Line 127) to avoid mixed labels. If kept for compatibility, consider marking usages for deprecation.
128-128: ABAC tab label OK.“ABAC_Logs”: “Logs” aligns with the new tab. No issues.
5498-5498: No action needed. The "Updated" key exists only once in the locales directory at line 5498 and is not duplicated.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/abac #37633 +/- ##
============================================
Coverage ? 54.28%
============================================
Files ? 2637
Lines ? 50052
Branches ? 11202
============================================
Hits ? 27171
Misses ? 20711
Partials ? 2170
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
MartinSchoeler
force-pushed
the
feat/ABAC-Logs-page
branch
from
4c72f4e to
a1e6c4f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/i18n/src/locales/en.i18n.json (1)
43-44: Minor copy tweak: hyphenation.Suggest “ABAC management–related activity will appear here.” for readability.
- "ABAC_No_logs_description": "ABAC-management related activity will appear here.", + "ABAC_No_logs_description": "ABAC management–related activity will appear here.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/meteor/client/lib/queryKeys.ts(1 hunks)apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx(1 hunks)apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx(2 hunks)apps/meteor/client/views/admin/ABAC/AdminABACTabs.tsx(1 hunks)packages/i18n/src/locales/en.i18n.json(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/client/views/admin/ABAC/AdminABACTabs.tsx
- apps/meteor/client/views/admin/ABAC/AdminABACPage.tsx
- apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/lib/queryKeys.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/lib/queryKeys.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-10-27T14:38:46.994Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 Learning: 2025-10-24T17:32:05.348Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
Applied to files:
packages/i18n/src/locales/en.i18n.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
apps/meteor/client/lib/queryKeys.ts (1)
123-126: LGTM! Past review feedback has been addressed.The implementation follows the existing ABAC query key pattern and matches the suggestion from the previous review. The
logs.listnaming is clean and consistent with the nested structure.packages/i18n/src/locales/en.i18n.json (3)
128-128: ABAC_Logs added — OK.Matches the new tab label context.
5498-5498: Generic “Updated” label — OK.No issues spotted.
28-31: ABAC element labels and casing fix are correct.ABAC_Element_Name normalization is confirmed in use in AdminABACLogs.tsx, and no stale references to the old casing remain. New audit action labels (Key_removed, Key_renamed, Value_removed, Key_added, Key_updated) and empty-state descriptions (ABAC_No_attributes, ABAC_No_rooms) are properly added with consistent PascalCase naming convention.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (2)
28-43: Initial date computation may be off-by-one in non‑UTC timezonesUsing
new Date().toISOString().split('T')[0]derives “today” in UTC, not in the user’s local timezone. In many timezones this can result in the date inputs defaulting to tomorrow or yesterday relative to the user’s local day, and thestart/endfilters (...T00:00:00.000Z/...T23:59:59.999Z) then query the wrong 24‑hour window.Consider deriving the local calendar date instead, e.g.:
-const [startDate, setStartDate] = useState<string>(new Date().toISOString().split('T')[0]); -const [endDate, setEndDate] = useState<string>(new Date().toISOString().split('T')[0]); +const getTodayLocalDate = () => { + const today = new Date(); + const year = today.getFullYear(); + const month = String(today.getMonth() + 1).padStart(2, '0'); + const day = String(today.getDate()).padStart(2, '0'); + return `${year}-${month}-${day}`; +}; + +const [startDate, setStartDate] = useState<string>(getTodayLocalDate); +const [endDate, setEndDate] = useState<string>(getTodayLocalDate);and keep the UTC boundaries in the query if that’s the server convention.
45-48: Stale comment wording (mentions “text” filter that doesn’t exist)The comment mentions “filter or the text”, but this component only has date filters. To avoid confusion for future readers, either update it to reference date filters only or remove it (the effect is self‑describing from its dependency array):
- // Whenever the user changes the filter or the text, reset the pagination to the first page useEffect(() => { setCurrent(0); }, [startDate, endDate, setCurrent]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
📚 Learning: 2025-11-07T14:50:33.544Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37423
File: packages/i18n/src/locales/en.i18n.json:18-18
Timestamp: 2025-11-07T14:50:33.544Z
Learning: Rocket.Chat settings: in apps/meteor/ee/server/settings/abac.ts, the Abac_Cache_Decision_Time_Seconds setting uses invalidValue: 0 as the fallback when ABAC is unlicensed. With a valid license, admins can still set the value to 0 to intentionally disable the ABAC decision cache.
Applied to files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
📚 Learning: 2025-10-30T19:30:46.541Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
Applied to files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx (2)
1-54: Overall data-fetching and pagination flow looks solidThe integration of
usePagination, the memoizedqueryobject, anduseQuerywithABACQueryKeys.logs.list(query)is consistent and type-safe, and the endpoint wiring viauseEndpoint('GET', '/v1/abac/audit')matches the intended contract. No blocking issues in this segment.
80-102: Event mapping looks correct; ts-expect-error is acceptable as a temporary workaroundThe
getEventInfohelper correctly normalizes both attribute-change and room-level events, and returning aDatefortimestampintegrates cleanly withuseFormatDateAndTime. UsingSerialized<OperationResult<...>>['events'][number]is a nice way to stay aligned with the REST typings.The
@ts-expect-erroraroundevent.data?.find((item) => item.key === 'room')?.value?.nameis understandable given the current REST typings; just ensure ABAC-84 eventually tightens the type and lets you remove this suppression.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/ee/server/api/abac/schemas.ts (1)
1-1: Narrowing events to ABAC-specific IServerEvents looks correctImporting
IServerEventsand typingeventsas a union of the four ABAC audit event shapes aligns with the/abac/auditquery filter and will give the client a tighter, more accurate type surface. No functional issues spotted.If you want to reduce future drift, consider factoring the ABAC audit event keys into a shared
const(e.g.ABAC_AUDIT_EVENT_TYPES) and using it both in this union and in the server query, so adding a new ABAC event only needs a single update.Also applies to: 228-235
apps/meteor/ee/server/api/abac/index.ts (1)
3-3: Typed cast of ABAC audit events matches query filterUsing
IServerEventsand castingeventsto the union of the four ABAC audit event types is consistent with the$infilter ontand with the updated response schema. This tightens the API typing without changing runtime behavior.To keep things maintainable, consider extracting the ABAC audit event type keys into a shared
constand deriving both the$inarray and the response type from it, e.g.:const ABAC_AUDIT_EVENT_TYPES = [ 'abac.action.performed', 'abac.attribute.changed', 'abac.object.attribute.changed', 'abac.object.attributes.removed', ] as const; type AbacAuditEvent = IServerEvents[(typeof ABAC_AUDIT_EVENT_TYPES)[number]];Then reuse
ABAC_AUDIT_EVENT_TYPESin the Mongo query andAbacAuditEvent[]in the response, avoiding future mismatches.Also applies to: 381-408
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx(1 hunks)apps/meteor/ee/server/api/abac/index.ts(2 hunks)apps/meteor/ee/server/api/abac/schemas.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/admin/ABAC/AdminABACLogs.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/ee/server/api/abac/schemas.tsapps/meteor/ee/server/api/abac/index.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/ee/server/api/abac/schemas.tsapps/meteor/ee/server/api/abac/index.ts
📚 Learning: 2025-11-07T14:50:33.544Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37423
File: packages/i18n/src/locales/en.i18n.json:18-18
Timestamp: 2025-11-07T14:50:33.544Z
Learning: Rocket.Chat settings: in apps/meteor/ee/server/settings/abac.ts, the Abac_Cache_Decision_Time_Seconds setting uses invalidValue: 0 as the fallback when ABAC is unlicensed. With a valid license, admins can still set the value to 0 to intentionally disable the ABAC decision cache.
Applied to files:
apps/meteor/ee/server/api/abac/schemas.tsapps/meteor/ee/server/api/abac/index.ts
📚 Learning: 2025-10-24T17:32:05.348Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
Applied to files:
apps/meteor/ee/server/api/abac/schemas.ts
📚 Learning: 2025-10-27T14:38:46.994Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Applied to files:
apps/meteor/ee/server/api/abac/schemas.tsapps/meteor/ee/server/api/abac/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
MartinSchoeler
force-pushed
the
feat/ABAC-Logs-page
branch
from
f911b20 to
ad80378
Compare
Proposed changes (including videos or screenshots)
Issue(s)
ABAC-51
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Localization
API
✏️ Tip: You can customize this high-level summary in your review settings.