Skip to content

Conversation

@KevLehman
Copy link
Member

@KevLehman KevLehman commented Nov 25, 2025

Proposed changes (including videos or screenshots)

Issue(s)

https://rocketchat.atlassian.net/browse/ABAC-58

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • New Features
    • System statistics now expose ABAC metrics when the ABAC module is licensed: ABAC enabled status, total attributes, total attribute values, and number of rooms enrolled with ABAC. These metrics appear in the public statistics surface only when the ABAC module is active.

✏️ Tip: You can customize this high-level summary in your review settings.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 25, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 25, 2025

⚠️ No Changeset found

Latest commit: 917b547

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Walkthrough

Adds ABAC usage metrics to the daily Statistics pipeline: exposes ABAC fields in IStats, adds model methods to count attributes/values/rooms, and enqueues ABAC-count operations when the ABAC license module is present.

Changes

Cohort / File(s) Summary
Statistics integration
apps/meteor/app/statistics/server/lib/statistics.ts
Import License and AbacAttributes; gate ABAC stats on License.hasModule('abac'); enqueue async operations to collect abacEnabled, abacTotalAttributes, abacTotalAttributeValues, abacRoomsEnrolled and resolve via existing Promise.all.
Type definitions
packages/core-typings/src/IStats.ts, packages/model-typings/src/models/IAbacAttributesModel.ts, packages/model-typings/src/models/IRoomsModel.ts
Add optional ABAC fields to IStats (abacEnabled, abacTotalAttributes, abacTotalAttributeValues, abacRoomsEnrolled); add countTotalValues(): Promise<number> to IAbacAttributesModel; add countAbacEnabled(): Promise<number> to IRoomsModel.
Model implementations
packages/models/src/models/AbacAttributes.ts, packages/models/src/models/Rooms.ts
Implement countTotalValues() on AbacAttributesRaw (aggregation summing sizes of values arrays); implement countAbacEnabled() on RoomsRaw (count rooms with abacAttributes and not archived).

Sequence Diagram(s)

sequenceDiagram
    participant Stats as Statistics Job
    participant License as License
    participant AbacAttrs as AbacAttributes Model
    participant Rooms as Rooms Model
    participant Promise as Promise.all

    Stats->>License: hasModule('abac')
    alt ABAC licensed
        Stats->>AbacAttrs: estimatedDocumentCount()
        Stats->>AbacAttrs: countTotalValues()
        Stats->>Rooms: countAbacEnabled()
        AbacAttrs-->>Stats: abacTotalAttributes / abacTotalAttributeValues
        Rooms-->>Stats: abacRoomsEnrolled
        Stats->>Promise: resolve([..., abacProms])
        Promise-->>Stats: resolved ABAC metrics
    else ABAC not licensed
        Note over Stats: ABAC metrics skipped (no calls)
    end
    Stats->>Stats: merge metrics into IStats payload
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas to review:
    • Aggregation in countTotalValues() — correctness of $ifNull and $size usage and returned shape.
    • Query/filter in countAbacEnabled() — ensure archived rooms are excluded and existence check matches schema.
    • Proper guarding by License.hasModule('abac') to avoid extra DB work when unlicensed.
    • Typings alignment between model interfaces and implementations.

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • aleksandernsilva
  • tassoevan
  • MartinSchoeler

Poem

🐇 I hopped through code at break of day,

Counting attributes along the way.
Rooms and values in tidy rows,
ABAC stats now tip their nose.
Hop on — the snapshot's good to go!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: ABAC statistics' directly and concisely describes the primary change—adding ABAC-related statistics to the system.
Linked Issues check ✅ Passed The PR implements all acceptance criteria from ABAC-58: adds abacEnabled, abacTotalAttributes, abacTotalAttributeValues, and abacRoomsEnrolled fields to statistics, with proper license gating and async counting methods, excluding archived rooms.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing ABAC statistics: interface updates (IStats, IAbacAttributesModel, IRoomsModel), implementation of counting methods (AbacAttributes, Rooms), and statistics collection logic—no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/abac-metrics-stats

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c3450e9 and 917b547.

📒 Files selected for processing (1)
  • packages/models/src/models/Rooms.ts (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:

  • packages/models/src/models/Rooms.ts
🧠 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-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/models/src/models/Rooms.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/models/src/models/Rooms.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:

  • packages/models/src/models/Rooms.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
🔇 Additional comments (1)
packages/models/src/models/Rooms.ts (1)

2316-2318: Archived exclusion is correct. No deleted field exclusion needed—deleted records are moved to trash collection.

The RoomsRaw class uses a trash collection parameter for RocketChatRecordDeleted<IRoom> records, implementing a soft-delete pattern. Deleted rooms are automatically stored separately and never appear in main collection queries. The countAbacEnabled() method correctly excludes archived rooms via the archived: { $ne: true } filter; explicit deleted field exclusion is unnecessary since deleted records are not in the active collection.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@KevLehman KevLehman changed the title add abac statistics chore: ABAC statistics Nov 25, 2025
@KevLehman KevLehman marked this pull request as ready for review November 25, 2025 17:14
@KevLehman KevLehman requested a review from a team as a code owner November 25, 2025 17:14
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 009716e and c3450e9.

📒 Files selected for processing (6)
  • apps/meteor/app/statistics/server/lib/statistics.ts (3 hunks)
  • packages/core-typings/src/IStats.ts (1 hunks)
  • packages/model-typings/src/models/IAbacAttributesModel.ts (1 hunks)
  • packages/model-typings/src/models/IRoomsModel.ts (1 hunks)
  • packages/models/src/models/AbacAttributes.ts (1 hunks)
  • packages/models/src/models/Rooms.ts (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:

  • packages/models/src/models/AbacAttributes.ts
  • packages/model-typings/src/models/IAbacAttributesModel.ts
  • packages/model-typings/src/models/IRoomsModel.ts
  • packages/models/src/models/Rooms.ts
  • apps/meteor/app/statistics/server/lib/statistics.ts
  • packages/core-typings/src/IStats.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-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/models/src/models/Rooms.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/app/statistics/server/lib/statistics.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/app/statistics/server/lib/statistics.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/app/statistics/server/lib/statistics.ts
🧬 Code graph analysis (1)
apps/meteor/app/statistics/server/lib/statistics.ts (2)
packages/core-services/src/index.ts (1)
  • License (164-164)
packages/models/src/index.ts (1)
  • AbacAttributes (231-231)
⏰ 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). (4)
  • GitHub Check: 🔨 Test Unit / Unit Tests
  • GitHub Check: 🔎 Code Check / Code Lint
  • GitHub Check: 🔎 Code Check / TypeScript
  • GitHub Check: 📦 Meteor Build (coverage)
🔇 Additional comments (6)
packages/model-typings/src/models/IRoomsModel.ts (1)

128-129: LGTM!

The interface addition is clean and follows the existing pattern.

packages/model-typings/src/models/IAbacAttributesModel.ts (1)

9-9: LGTM!

Clean interface addition for counting total attribute values.

packages/models/src/models/AbacAttributes.ts (1)

19-38: LGTM!

The aggregation correctly sums the size of all values arrays across documents, handling missing fields with $ifNull and returning 0 when no documents exist.

packages/core-typings/src/IStats.ts (1)

274-277: LGTM!

The ABAC statistics fields are properly typed and align with the PR objectives. The optional nature of these fields is appropriate since they're only populated when the ABAC module is licensed.

apps/meteor/app/statistics/server/lib/statistics.ts (2)

7-7: LGTM!

The License and AbacAttributes imports are correctly added to enable ABAC statistics collection.

Also applies to: 29-29


614-632: Approve ABAC statistics collection with one dependency.

The ABAC statistics implementation follows the existing patterns in this file correctly:

  • License gating is appropriate
  • Async operations are properly pushed to statsPms
  • Uses estimatedDocumentCount() for performance (acceptable for statistics)

The accuracy of abacRoomsEnrolled depends on fixing the Rooms.countAbacEnabled() method to exclude archived rooms as flagged in my other comment.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.2GiB 1.2GiB +13MiB
rocketchat 358MiB 346MiB +12MiB
omnichannel-transcript-service 132MiB 132MiB +76KiB
queue-worker-service 132MiB 132MiB +77KiB
ddp-streamer-service 127MiB 127MiB +75KiB
account-service 114MiB 114MiB +77KiB
authorization-service 111MiB 111MiB +124KiB
stream-hub-service 111MiB 111MiB +75KiB
presence-service 111MiB 111MiB +74KiB

📊 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/25 17:50 (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]
  line "authorization-service" [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]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 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]
  line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13]
  line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 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]
Loading

Statistics (last 7 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.2GiB
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-37606
  • Baseline: develop
  • Timestamp: 2025-11-25 17:50:06 UTC
  • Historical data points: 7

Updated: Tue, 25 Nov 2025 17:50:06 GMT

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat/abac@009716e). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             feat/abac   #37606   +/-   ##
============================================
  Coverage             ?   54.21%           
============================================
  Files                ?     2661           
  Lines                ?    50149           
  Branches             ?    11213           
============================================
  Hits                 ?    27188           
  Misses               ?    20813           
  Partials             ?     2148           
Flag Coverage Δ
e2e 57.28% <ø> (?)
e2e-api 43.26% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tassoevan tassoevan merged commit 65db004 into feat/abac Nov 26, 2025
51 checks passed
@tassoevan tassoevan deleted the chore/abac-metrics-stats branch November 26, 2025 14:03
KevLehman added a commit that referenced this pull request Nov 27, 2025
KevLehman added a commit that referenced this pull request Dec 1, 2025
KevLehman added a commit that referenced this pull request Dec 2, 2025
KevLehman added a commit that referenced this pull request Dec 8, 2025
KevLehman added a commit that referenced this pull request Dec 10, 2025
KevLehman added a commit that referenced this pull request Dec 15, 2025
MartinSchoeler pushed a commit that referenced this pull request Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants