Repository navigation
Enforce realm and asset scoping for rule alarms - #3459
Conversation
wborn
left a comment
There was a problem hiding this comment.
This is an initial AI-assisted review.
The scope validation and expanded integration tests provide good coverage of the intended behavior.
One concern remains: synchronous Keycloak role lookups during automatic alarm notification processing introduce an external dependency that can cause alarm creation to fail after the alarm has already been persisted. This should be addressed before merging.
wborn
left a comment
There was a problem hiding this comment.
Thanks for addressing the previous review feedback. The database-backed notification recipient queries are a good improvement, avoiding repeated Keycloak Admin API calls.
The realm and asset scoping validation looks sound, including restricted assignee handling, validation of existing asset links, and global ruleset support. The additional tests also provide good coverage.
The composite-role limitation of UserQuery is now tracked in #3524. I've left one non-blocking suggestion for extending that issue to cover a future improvement to assignee validation.
No further blocking issues found.
This review was AI-assisted.
|



Alarm rules could create or extend alarms outside their asset scope and assign users without validating access. Global rules also failed because creation and linking required an engine realm.
This PR adds validation before alarms or links are persisted:
Part of the rules facade scoping review tracked in #2675.