feat: Virtru as attribute store by KevLehman · Pull Request #40634 · RocketChat/Rocket.Chat

KevLehman · 2026-05-20T21:42:57Z

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

dionisio-bot · 2026-05-20T21:43:01Z

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

coderabbitai · 2026-05-20T21:43:05Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ee1ee9a7-dc08-4aae-b016-178a1e363589

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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.}

changeset-bot · 2026-05-20T21:43:05Z

⚠️ No Changeset found

Latest commit: d85927a

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

codecov · 2026-05-20T21:58:11Z

Codecov Report

❌ Patch coverage is 90.98837% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.77%. Comparing base (5d00ea5) to head (d85927a).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #40634      +/-   ##
===========================================
+ Coverage    69.65%   69.77%   +0.12%     
===========================================
  Files         3338     3343       +5     
  Lines       123246   123544     +298     
  Branches     21973    21985      +12     
===========================================
+ Hits         85841    86205     +364     
+ Misses       34052    33988      -64     
+ Partials      3353     3351       -2

Flag	Coverage Δ
e2e	`59.14% <ø> (-0.06%)`	⬇️
e2e-api	`45.99% <ø> (+0.01%)`	⬆️
unit	`70.66% <90.98%> (+0.19%)`	⬆️

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.

…in test

…t/config access

…no-op guard

…ty-keyed cache)

… typed entity-resolution error

…store selection

…tiveStore

…r tests

… (fail-closed)

Update test assertions to await the now-async method.

…seam + EE patch

…on patch

… to local

…e.store.switched

The service relied solely on the 'license.module' broker event to track the ABAC license flag, missing the initial state if the event fired before subscription. Seed hasAbacLicense in started() via License.hasModule('abac') so the effective attribute-store evaluation works on the first request.

In microservices the AbacService runs in the authorization-service process while the license lives in the monolith. reevaluateAttributeStore re-synced the store selection from a stale hasAbacLicense field, so the Virtru attribute store was never selected. Refresh the flag from the License service (broker call) before re-syncing; the monolith already invokes this on License.onToggledFeature('abac').

The attribute-store selection was gated by a cached hasAbacLicense flag seeded at startup and refreshed by a license-module event. In microservices the AbacService runs in the authorization-service while the license lives in the monolith, so both the startup seed and the event race the cross-process license validation, leaving the service stuck on the local store and the Virtru store unreachable. Query the license service live at the point of use instead: effectiveStore awaits License.hasModule('abac'), and the store instance is resolved on demand. This matches the request-time license gate already used by the ABAC API routes and removes the startup race entirely. Drops the hasAbacLicense field, the license-module listener and reevaluateAttributeStore.

The microservices never run the Log_Level settings watch that the monolith uses, so @rocket.chat/logger stayed pinned at 'warn' and debug output from service-side code was invisible in CI. Seed the logger's default level from a LOG_LEVEL env var and pass DEBUG_LOG_LEVEL into each microservice container so e2e debug runs surface service logs.

VirtruAttributeStore.decideRooms now translates a failed GetDecisionBulk call into PdpUnavailableError, so a write blocked by an unreachable decision endpoint surfaces error-pdp-unavailable instead of a raw error. e2e fixes: the "admin A can POST" test now targets a fresh room instead of the pre-seeded shared room (which already had the clearance key); the admin B DENY write tests assert error-abac-not-authorized-to-modify-room (the PDP is reachable and returns DENY); and the wipe-transition and local-mode-regression before hooks reset/seed the mock server before issuing PDP-gated writes.

rc-layne · 2026-05-22T20:41:41Z

⚠️ Layne — 3 warning(s)

Found 3 issue(s): 3 high.

julio-rocketchat · 2026-05-22T21:32:15Z

/layne exception-approve LAYNE-8598b68f3ce8435a LAYNE-e568b73325f960fd LAYNE-5929c5f8cb7138d7 reason: these are acceptable and valid ignoreSsrfValidation entries, not vulnerabilities

rc-layne · 2026-05-22T21:32:18Z

✅ Exception recorded for LAYNE-8598b68f3ce8435a, LAYNE-e568b73325f960fd, LAYNE-5929c5f8cb7138d7 by @julio-rocketchat: "these are acceptable and valid ignoreSsrfValidation entries, not vulnerabilities". Re-running scan...

KevLehman force-pushed the feat/abac-virtru-attribute-store branch from 7a0c73b to f3a549c Compare May 21, 2026 14:22

KevLehman added 25 commits May 22, 2026 10:17

refactor(abac): extract pure virtru identity/FQN helpers

4ef1aae

refactor(abac): add shared VirtruClient (transport/auth/reachability)

13ef798

refactor(abac): harden VirtruClient getConfig + assert Bearer header …

d003ffd

…in test

refactor(abac): VirtruPDP delegates transport/identity to VirtruClient

62f9077

refactor(abac): remove dead VirtruPDP.updateConfig + tidy VirtruClien…

15e93f6

…t/config access

chore(abac): add license dep + attribute-store-external error code

6053e0a

feat(abac): define IAttributeStore + GetEntitlements types

dc02c1c

feat(abac): LocalAttributeStore (identity/no-op, zero behavior change)

ba18421

refactor(abac): tidy LocalAttributeStore filter condition + document …

8d66103

…no-op guard

feat(abac): VirtruAttributeStore list/entitlements/validate (15s enti…

7f349d9

…ty-keyed cache)

fix(abac): evict VirtruAttributeStore entitlements cache on failure +…

813e093

… typed entity-resolution error

feat(abac): AbacService owns VirtruClient + effectiveStore/attribute-…

c88270b

…store selection

chore(abac): self-contained boot-ordering comment + default lastEffec…

8b5b0fe

…tiveStore

feat(abac): ABAC_Attribute_Store setting + listeners + license-up reeval

0a83070

refactor(abac): tidy attribute-store transition path + harden listene…

88f5ddb

…r tests

feat(abac): VirtruAttributeStore scopeRoomsPage + assertCanModifyRoom…

3bf815e

… (fail-closed)

test(abac): split assertCanModifyRoom into per-scenario cases

ea4287d

feat(abac): redact denied rooms in listAbacRooms via attribute store

69526a6

feat(abac): 4-site assertCanModifyRoom + validateAssignable store swap

cd7bb31

test(abac): match mocked validateAssignable arity to interface

09a11ac

feat(abac): block catalog CRUD in virtru attribute-store mode

ad97300

fix(abac): make isExternalAttributeStore async to match IAbacService

4c607bc

Update test assertions to await the now-async method.

feat(abac): serve attribute picker from store in virtru mode

c6d980d

feat(abac): redact abacAttributes on rooms.adminRooms.getRoom via CE …

99a5f46

…seam + EE patch

fix(abac): use settings-gated isABACManagedRoom in admin-room redacti…

72b120d

…on patch

KevLehman added 20 commits May 22, 2026 10:17

feat(abac): cascade ABAC_Attribute_Store=local when ABAC_PDP_Type set…

54f20bf

… to local

refactor(abac): rename audit event key to dot-separated abac.attribut…

6bcb736

…e.store.switched

move client to folder

4793671

types

62bc9d8

minor improvements

17db1f8

better error

31b6136

one audit less

58816fc

better caching

6331428

log

8e3c6f7

log malformed

35cf3ea

nits

5fef79f

license

4a446d0

add validation to remove action

f031bbc

simplify

54d159a

remove old license tests

2a00e01

KevLehman force-pushed the feat/abac-virtru-attribute-store branch from b2dc287 to 2a00e01 Compare May 22, 2026 16:20

KevLehman added 5 commits May 22, 2026 12:11

imp

78f45cb

store selection

7171f64

tests

26c0bbe

wrong test

2a4135e

log

d85927a

rc-layne Bot added the needs-security-review label May 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Virtru as attribute store#40634

feat: Virtru as attribute store#40634
KevLehman wants to merge 61 commits into
developfrom
feat/abac-virtru-attribute-store

KevLehman commented May 20, 2026

Uh oh!

dionisio-bot Bot commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026 •

edited

Loading

Review skipped

Uh oh!

changeset-bot Bot commented May 20, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 20, 2026 •

edited

Loading

Uh oh!

rc-layne Bot commented May 22, 2026 •

edited

Loading

Uh oh!

julio-rocketchat commented May 22, 2026

Uh oh!

rc-layne Bot commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

KevLehman commented May 20, 2026

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Uh oh!

dionisio-bot Bot commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

changeset-bot Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

codecov Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rc-layne Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Layne — 3 warning(s)

Uh oh!

julio-rocketchat commented May 22, 2026

Uh oh!

rc-layne Bot commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented May 20, 2026 •

edited

Loading

changeset-bot Bot commented May 20, 2026 •

edited

Loading

codecov Bot commented May 20, 2026 •

edited

Loading

rc-layne Bot commented May 22, 2026 •

edited

Loading