feat: Access Checks for ABAC managed rooms #37423
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
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 |
|
Contributor
WalkthroughAdds ABAC read-access evaluation with decision caching and TTL, integrates ABAC checks into room-access validators and service types, propagates room Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as Client
participant S as ServerAuth
participant AB as AbacService
participant Sub as Subscriptions
participant Set as Settings
participant DB as UsersDB
U->>C: Request room access
C->>S: canAccessRoom(roomId, userId)
S->>S: Fetch room (includes abacAttributes)
alt ABAC enabled & room has abacAttributes
S->>AB: canAccessObject(room, user, READ, ROOM)
AB->>Set: read Abac_Cache_Decision_Time_Seconds
AB->>Sub: get subscription (abacLastTimeChecked)
alt cached and within TTL
AB-->>S: allow (cached)
else
AB->>DB: query user matching room abacAttributes
alt user matches
AB->>Sub: setAbacLastTimeCheckedByUserIdAndRoomId(now)
AB-->>S: allow
else
AB-->>S: deny (and trigger removal)
end
end
alt ABAC allow AND permission checks pass
S-->>C: Access granted
else
S-->>C: Access denied
end
else
S->>S: Legacy joined/type permission checks
S-->>C: Access allowed/denied
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/abac #37423 +/- ##
=============================================
- Coverage 70.62% 68.25% -2.37%
=============================================
Files 3064 3368 +304
Lines 106340 116277 +9937
Branches 18717 20979 +2262
=============================================
+ Hits 75098 79364 +4266
- Misses 29278 34218 +4940
- Partials 1964 2695 +731
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
packages/core-typings/src/ISubscription.ts (1)
75-76: ABAC cache timestamp: clarify semantics, ensure safe exposure, plan invalidation
- Looks good as an optional server-side timestamp.
- Clarify scope: is this for read only or any ABAC operation? If decisions differ per operation, consider per-op fields or a small struct later to avoid conflating results.
- If exposed to clients, confirm it’s read-only and not accepted by any update endpoints to prevent spoofing.
- Ensure invalidation on ABAC-relevant changes (room ABAC attributes, user attributes, membership/role changes) rather than relying solely on TTL. When ABAC is globally disabled, avoid updating this field to prevent misleading freshness checks. Based on learnings.
packages/models/src/models/Subscriptions.ts (1)
2088-2102: Add session support to the setter and handle missing subscriptions
- Extend
setAbacLastTimeCheckedByUserIdAndRoomIdto acceptoptions: { session?: ClientSession }and pass it toupdateOne.- In callers, inspect the returned
UpdateResult.matchedCountto detect when no document was updated and handle accordingly.- setAbacLastTimeCheckedByUserIdAndRoomId(userId: string, roomId: string, time: Date): Promise<UpdateResult> { + setAbacLastTimeCheckedByUserIdAndRoomId( + userId: string, + roomId: string, + time: Date, + options: { session?: ClientSession } = {}, + ): Promise<UpdateResult> { const query = { rid: roomId, 'u._id': userId }; const update: UpdateFilter<ISubscription> = { $set: { abacLastTimeChecked: time } }; - return this.updateOne(query, update); + return this.updateOne(query, update, { session: options.session }); }packages/i18n/src/locales/en.i18n.json (2)
17-18: Unify ABAC i18n key prefix with existing convention (ABAC_ vs Abac_).Other ABAC keys use the all‑caps ABAC_ prefix (e.g., ABAC_Enabled). Rename these keys for consistency.
- "Abac_Cache_Decision_Time_Seconds": "ABAC Cache Decision Time (seconds)", - "Abac_Cache_Decision_Time_Seconds_Description": "Time in seconds to cache access control decisions. Setting this value to 0 will disable caching.", + "ABAC_Cache_Decision_Time_Seconds": "ABAC Cache Decision Time (seconds)", + "ABAC_Cache_Decision_Time_Seconds_Description": "Time in seconds to cache access control decisions. Setting this value to 0 will disable caching."
17-18: Tighten copy and align with TTL terminology used elsewhere.Keep the meaning but use concise TTL phrasing consistent with other settings.
- "ABAC_Cache_Decision_Time_Seconds": "ABAC Cache Decision Time (seconds)", - "ABAC_Cache_Decision_Time_Seconds_Description": "Time in seconds to cache access control decisions. Setting this value to 0 will disable caching.", + "ABAC_Cache_Decision_Time_Seconds": "ABAC Decision Cache TTL (seconds)", + "ABAC_Cache_Decision_Time_Seconds_Description": "Seconds to cache ABAC access decisions. Set to 0 to disable caching."Optionally append: “Effective only when ABAC is enabled.” Based on learnings
apps/meteor/server/services/authorization/service.ts (1)
89-96: Projection updated to include ABAC attributes — looks correct. Optional optimization.Fetching abacAttributes here enables ABAC read checks downstream. Optionally, to reduce payloads, fetch this field only when ABAC is enabled and/or for private rooms (t === 'p'), since ABAC attributes apply only to private rooms/teams. Based on learnings.
📜 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 (15)
apps/meteor/client/lib/utils/mapSubscriptionFromApi.ts(1 hunks)apps/meteor/ee/server/settings/abac.ts(1 hunks)apps/meteor/server/services/authorization/canAccessRoom.ts(2 hunks)apps/meteor/server/services/authorization/service.ts(1 hunks)apps/meteor/tests/end-to-end/api/abac.ts(1 hunks)ee/packages/abac/src/can-access-object.spec.ts(1 hunks)ee/packages/abac/src/index.ts(3 hunks)packages/core-services/src/types/IAbacService.ts(2 hunks)packages/core-services/src/types/IAuthorization.ts(1 hunks)packages/core-typings/src/Abac.ts(1 hunks)packages/core-typings/src/ISubscription.ts(1 hunks)packages/core-typings/src/index.ts(1 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/model-typings/src/models/ISubscriptionsModel.ts(1 hunks)packages/models/src/models/Subscriptions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 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.
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`.
📚 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/core-typings/src/Abac.tspackages/core-services/src/types/IAbacService.tspackages/core-services/src/types/IAuthorization.tsee/packages/abac/src/can-access-object.spec.tsapps/meteor/server/services/authorization/canAccessRoom.tspackages/i18n/src/locales/en.i18n.jsonapps/meteor/server/services/authorization/service.tsee/packages/abac/src/index.tsapps/meteor/ee/server/settings/abac.tsapps/meteor/tests/end-to-end/api/abac.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:
packages/core-services/src/types/IAbacService.tspackages/core-services/src/types/IAuthorization.tsee/packages/abac/src/can-access-object.spec.tsapps/meteor/server/services/authorization/canAccessRoom.tsapps/meteor/server/services/authorization/service.tsee/packages/abac/src/index.tsapps/meteor/tests/end-to-end/api/abac.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/core-typings/src/ISubscription.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/client/lib/utils/mapSubscriptionFromApi.tspackages/model-typings/src/models/ISubscriptionsModel.tsapps/meteor/server/services/authorization/service.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
ee/packages/abac/src/can-access-object.spec.tsapps/meteor/tests/end-to-end/api/abac.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/server/services/authorization/canAccessRoom.tsapps/meteor/server/services/authorization/service.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/server/services/authorization/canAccessRoom.tsee/packages/abac/src/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
packages/model-typings/src/models/ISubscriptionsModel.tsapps/meteor/server/services/authorization/service.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/server/services/authorization/service.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/end-to-end/api/abac.ts
🧬 Code graph analysis (9)
packages/core-services/src/types/IAbacService.ts (2)
packages/core-typings/src/IRoom.ts (1)
IRoom(22-98)packages/core-typings/src/IUser.ts (1)
IUser(186-255)
packages/core-services/src/types/IAuthorization.ts (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(22-98)
packages/models/src/models/Subscriptions.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-77)
apps/meteor/client/lib/utils/mapSubscriptionFromApi.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(10-77)
ee/packages/abac/src/can-access-object.spec.ts (1)
ee/packages/abac/src/index.ts (1)
AbacService(14-608)
apps/meteor/server/services/authorization/canAccessRoom.ts (2)
apps/meteor/server/services/authorization/service.ts (1)
Authorization(14-205)packages/core-services/src/index.ts (2)
License(164-164)Abac(202-202)
apps/meteor/server/services/authorization/service.ts (1)
packages/core-typings/src/IRoom.ts (1)
IRoom(22-98)
ee/packages/abac/src/index.ts (4)
packages/core-typings/src/IAbacAttribute.ts (1)
IAbacAttributeDefinition(3-14)packages/core-typings/src/IRoom.ts (1)
IRoom(22-98)packages/core-typings/src/IUser.ts (1)
IUser(186-255)packages/core-services/src/index.ts (1)
Settings(187-187)
apps/meteor/tests/end-to-end/api/abac.ts (3)
apps/meteor/tests/data/api-data.ts (2)
request(10-10)credentials(39-42)apps/meteor/tests/data/users.helper.ts (1)
createUser(10-39)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
⏰ 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 (5)
packages/core-typings/src/index.ts (1)
151-152: Expose ABAC typings via index — LGTM.Re-export is appropriate and keeps external imports consistent.
packages/core-typings/src/Abac.ts (1)
1-9: New ABAC enums — LGTM.Enums are straightforward and align with usage across the PR.
packages/core-services/src/types/IAuthorization.ts (1)
4-4: RoomAccessValidator widened to include ABAC attributes — LGTM.Keeps types aligned with service and ABAC checks.
packages/core-services/src/types/IAbacService.ts (1)
28-33: ABAC service surface verified — all call sites correctly use typed enums.Checked all
canAccessObjectinvocations: test suite and production code both consistently passAbacAccessOperation.READandAbacObjectType.ROOMvia proper enum references, with negative test cases validating rejection of unsupported values. Signature and implementation align; room Pick type includes requiredabacAttributesfield.apps/meteor/client/lib/utils/mapSubscriptionFromApi.ts (1)
10-11: Map abacLastTimeChecked — LGTM. ConfirmedabacLastTimeChecked?: DateinISubscription.ts; no further action.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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/tests/end-to-end/api/abac.ts(1 hunks)ee/packages/abac/src/can-access-object.spec.ts(1 hunks)ee/packages/abac/src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/tests/end-to-end/api/abac.ts
🧰 Additional context used
🧠 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.
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`.
📚 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:
ee/packages/abac/src/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:
ee/packages/abac/src/index.tsee/packages/abac/src/can-access-object.spec.ts
📚 Learning: 2025-11-07T14:50:33.528Z
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.528Z
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:
ee/packages/abac/src/index.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
ee/packages/abac/src/index.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
ee/packages/abac/src/can-access-object.spec.ts
🧬 Code graph analysis (2)
ee/packages/abac/src/index.ts (4)
packages/core-typings/src/IAbacAttribute.ts (1)
IAbacAttributeDefinition(3-14)packages/core-typings/src/IRoom.ts (1)
IRoom(22-98)packages/core-typings/src/IUser.ts (1)
IUser(186-255)packages/core-services/src/index.ts (2)
Settings(187-187)Room(171-171)
ee/packages/abac/src/can-access-object.spec.ts (1)
ee/packages/abac/src/index.ts (1)
AbacService(14-618)
tassoevan
requested changes
Nov 10, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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/server/services/authorization/canAccessRoom.ts(2 hunks)
🧰 Additional context used
🧠 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.
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`.
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.
📚 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/server/services/authorization/canAccessRoom.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/server/services/authorization/canAccessRoom.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/server/services/authorization/canAccessRoom.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/server/services/authorization/canAccessRoom.ts
🧬 Code graph analysis (1)
apps/meteor/server/services/authorization/canAccessRoom.ts (2)
apps/meteor/server/services/authorization/service.ts (1)
Authorization(14-205)packages/core-services/src/index.ts (2)
License(164-164)Abac(202-202)
tassoevan
approved these changes
Nov 11, 2025
This was referenced Nov 11, 2025
KevLehman
added a commit
that referenced
this pull request
Nov 12, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
This was referenced Nov 13, 2025
KevLehman
added a commit
that referenced
this pull request
Nov 18, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
KevLehman
added a commit
that referenced
this pull request
Nov 24, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
KevLehman
added a commit
that referenced
this pull request
Nov 27, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
KevLehman
added a commit
that referenced
this pull request
Dec 1, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
KevLehman
added a commit
that referenced
this pull request
Dec 2, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
Merged
KevLehman
added a commit
that referenced
this pull request
Dec 8, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
KevLehman
added a commit
that referenced
this pull request
Dec 10, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
KevLehman
added a commit
that referenced
this pull request
Dec 15, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
MartinSchoeler
pushed a commit
that referenced
this pull request
Dec 17, 2025
Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
This was referenced Dec 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/ABAC-27
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Improvements
Tests