Support enable/disable rules on request configuration rules - #12702
Conversation
|
Converted to draft waiting for 2026.02.00 to be out |
| */ | ||
| const isRuleEnabled = (rule) => { | ||
| const enabled = rule?.enabled ?? true; | ||
| const monitoredState = getMonitoredState(getState()); |
There was a problem hiding this comment.
Other getMonitoredState are including also the ConfigUtils.getConfigProp('monitorState') as second argument to include additional monitorState configured in a custom localConfig.json
const monitoredState = getMonitoredState(getState(), ConfigUtils.getConfigProp('monitorState'));
| const isRuleEnabled = (rule) => { | ||
| const enabled = rule?.enabled ?? true; | ||
| const monitoredState = getMonitoredState(getState()); | ||
| return !!handleExpression((path) => get(monitoredState, path), undefined, enabled); | ||
| }; |
There was a problem hiding this comment.
This is just a proposal, if we think isRuleEnabled is enough we can keep it for the moment.
At the moment we run isRuleEnabled for each rule, which means we call getMonitoredState and its internal memoize resolver (filterState) once per rule on every request. This could be costly because the resolver builds the cache key with JSON.stringify over every monitored entry, and the entry list is configurable via monitorState, so we cannot predict what ends up in there (it could technically be a large object).
The proposal is a function that takes rules and returns filtered rules:
/**
* Filters out rules whose `enabled` property resolves to falsy.
* `enabled` is a boolean or a plugin expression string (same syntax as `cfg.disablePluginIf`).
* Rules without an `enabled` property are always applied.
* @param {object[]} rules the request configuration rules
* @returns {object[]} the enabled rules
*/
const filterEnabledRules = (rules = []) => {
const needsMonitoredState = rules.some(rule => isString(rule?.enabled));
const monitoredState = needsMonitoredState ? getMonitoredState(getState(), ConfigUtils.getConfigProp('monitorState')) : {};
const getMonitored = (path) => get(monitoredState, path);
return rules.filter(rule => !!handleExpression(getMonitored, undefined, rule?.enabled ?? true));
};applied like this:
return filterEnabledRules(stateRules);return filterEnabledRules(configRules);return filterEnabledRules(convertAuthenticationRulesToRequestConfiguration(authRules));Each entry in requestsConfigurationRules (and legacy authenticationRules)
can now set enabled to a boolean or a plugin expression string, evaluated
the same way as cfg.disablePluginIf (e.g. via state('usergroups')). Lets
an auth/header rule apply only to matching user groups.
Include the monitorState configuration in the monitored state, as every other getMonitoredState caller does, so expressions can use custom entries from localConfig.json. Resolve the monitored state once per filtering and only when a rule actually declares an expression: building it serializes every monitored entry, and getRequestConfigurationRules runs on each request.
407ccc6 to
c556aac
Compare
|
Successfully created backport PR for |
|
@ElenaGallo please test this fix on dev, thanks |
3 participants
Fixes #12704
Summary
requestsConfigurationRules(and legacyauthenticationRules) can now declare anenabledproperty: a plain boolean or a plugin expression string (same syntax ascfg.disablePluginIf), e.g."{includes(state('usergroups'), 'editor')}".enabledkeep the current behaviour (always applied).PluginsUtils.handleExpression/getMonitoredStatemachinery, no new expression syntax introduced.local-config.md.Test plan
SecurityUtils-test.js: noenabledproperty,enabled: false, expression resolving false, expression resolving true againststate('usergroups').SecurityUtils-test.jssuite locally via karma (chrome headless) — 59/59 passing, no regressions.