mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-24 16:05:00 +08:00
Deduplicate System Console config isDisabled dependencies (#38124)
* Add ESLint rule to dedupe System Console config dependencies Introduce @mattermost/no-redundant-admin-config-deps so admin_definition settings that depend on a bool parent via stateIsFalse do not repeat that parent's config/state isDisabled checks, matching the dependency-tree approach discussed on #38024. Co-authored-by: Jesse Hallam <lieut-data@users.noreply.github.com> * Remove redundant System Console isDisabled config checks Drop duplicated parent config/state conditions from SAML and OAuth DCR settings now covered by bool parent dependencies, as enforced by @mattermost/no-redundant-admin-config-deps. Co-authored-by: Jesse Hallam <lieut-data@users.noreply.github.com> * Attribute redundant admin config deps to closest parent Build the full bool-parent dependency tree before reporting, then pick the most specific immediate parent that implies a condition so grandparent-first declaration order cannot mislabel the fix target. Co-authored-by: Jesse Hallam <lieut-data@users.noreply.github.com> * Fix eslint-plugin lint violations in admin config deps rule Resolve no-nested-ternary, wrap-regex, no-unused-vars, no-underscore-dangle, and lines-around-comment issues so Web App CI check-lint passes on the new rule and its tests. Co-authored-by: Jesse Hallam <lieut-data@users.noreply.github.com> * Harden admin config deps rule for regex keys and parent parse Keep distinct regex patterns from collapsing to the same canonicalize key, and skip non-JSON identifier parent args instead of throwing. Co-authored-by: mattermost-code <matty-code@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Jesse Hallam <lieut-data@users.noreply.github.com> Co-authored-by: mattermost-code <matty-code@mattermost.com>
This commit is contained in:
co-authored by
mattermost-code
Cursor Agent
Jesse Hallam
parent
e86491adac
commit
f8a5beb175
@@ -177,6 +177,8 @@ const SAML_SETTINGS_CANONICAL_ALGORITHM_C14N11 = 'Canonical1.1';
|
||||
// - type: which define the widget type.
|
||||
// - label (and label_default): which define the main text of the setting.
|
||||
// - isDisabled: a function which receive current config, the state of the page and the license.
|
||||
// Prefer depending on a parent bool setting (it.stateIsFalse('Parent.Key')) over repeating that
|
||||
// parent's config/state checks; @mattermost/no-redundant-admin-config-deps enforces this.
|
||||
// - isHidden: a function which receive current config, the state of the page and the license.
|
||||
//
|
||||
// Custom Widget (extends from Widget):
|
||||
@@ -4619,7 +4621,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)),
|
||||
it.configIsFalse('GuestAccountsSettings', 'Enable'),
|
||||
it.stateIsFalse('SamlSettings.EnableSyncWithLdap'),
|
||||
it.stateIsFalse('SamlSettings.Enable'),
|
||||
),
|
||||
},
|
||||
{
|
||||
@@ -4641,7 +4642,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
help_text_markdown: false,
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)),
|
||||
it.stateIsFalse('SamlSettings.Enable'),
|
||||
it.stateIsFalse('SamlSettings.EnableSyncWithLdap'),
|
||||
),
|
||||
},
|
||||
@@ -4795,7 +4795,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
remove_action: removePrivateSamlCertificate,
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)),
|
||||
it.stateIsFalse('SamlSettings.Enable'),
|
||||
it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
),
|
||||
},
|
||||
@@ -4813,7 +4812,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
remove_action: removePublicSamlCertificate,
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)),
|
||||
it.stateIsFalse('SamlSettings.Enable'),
|
||||
it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
),
|
||||
},
|
||||
@@ -4835,7 +4833,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
label: defineMessage({id: 'admin.saml.signatureAlgorithmTitle', defaultMessage: 'Signature Algorithm'}),
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)),
|
||||
it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
it.stateIsFalse('SamlSettings.SignRequest'),
|
||||
),
|
||||
options: [
|
||||
@@ -4879,7 +4876,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
],
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)),
|
||||
it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
it.stateIsFalse('SamlSettings.SignRequest'),
|
||||
),
|
||||
},
|
||||
@@ -4948,7 +4944,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
isDisabled: it.any(
|
||||
it.not(it.isSystemAdmin),
|
||||
it.stateIsFalse('SamlSettings.EnableAdminAttribute'),
|
||||
it.stateIsFalse('SamlSettings.Enable'),
|
||||
),
|
||||
},
|
||||
{
|
||||
@@ -6060,7 +6055,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
placeholder: defineMessage({id: 'admin.oauth.dcrRedirectURIAllowlistPlaceholder', defaultMessage: 'E.g.: https://*.example.com/**, https://app.example.com/callback'}),
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.INTEGRATIONS.INTEGRATION_MANAGEMENT)),
|
||||
it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'),
|
||||
it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'),
|
||||
),
|
||||
isHidden: it.licensedForFeature('Cloud'),
|
||||
|
||||
@@ -28,6 +28,54 @@ export function someAction() {
|
||||
}
|
||||
```
|
||||
|
||||
### no-redundant-admin-config-deps
|
||||
|
||||
Enforces that System Console settings in `admin_definition` files do not repeat `isDisabled` config/state checks already implied by a parent setting they depend on.
|
||||
|
||||
When setting B disables itself with `it.stateIsFalse('A')`, A is a bool setting, and A already includes condition C (for example `it.configIsTrue('ClusterSettings', 'Enable')`), repeating C on B is redundant and tends to drift. Keep the dependency on A and omit the duplicated checks. Inheritance only follows bool parents (disabled bools are forced false on save). Permission and license helpers are not treated as config dependencies.
|
||||
|
||||
The rule builds the full bool-parent dependency tree before reporting. If a setting lists both a parent and a grandparent, redundant conditions are attributed to the closest (most specific) parent — not whichever ancestor appears first in `isDisabled`.
|
||||
|
||||
Examples of **incorrect** code for this rule:
|
||||
```javascript
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'EmailSettings.EnableEmailBatching',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'number',
|
||||
key: 'EmailSettings.EmailBatchingBufferSize',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.stateIsFalse('EmailSettings.EnableEmailBatching'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
),
|
||||
},
|
||||
```
|
||||
|
||||
Examples of **correct** code for this rule:
|
||||
```javascript
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'EmailSettings.EnableEmailBatching',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'number',
|
||||
key: 'EmailSettings.EmailBatchingBufferSize',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.EnableEmailBatching'),
|
||||
),
|
||||
},
|
||||
```
|
||||
|
||||
### use-external-link
|
||||
|
||||
Ensures that any link which opens a URL outside of Mattermost using `target="_blank"` uses the `ExternalLink` component.
|
||||
|
||||
@@ -52,6 +52,7 @@ const base = {
|
||||
},
|
||||
rules: {
|
||||
'@mattermost/no-dispatch-getstate': 2,
|
||||
'@mattermost/no-redundant-admin-config-deps': 2,
|
||||
'@mattermost/use-external-link': 2,
|
||||
'@stylistic/array-bracket-spacing': [
|
||||
2,
|
||||
|
||||
@@ -41,6 +41,7 @@
|
||||
"scripts": {
|
||||
"check": "eslint . --quiet",
|
||||
"fix": "eslint . --quiet --fix",
|
||||
"test": "node --test rules/*.test.js",
|
||||
"clean": "rm -rf node_modules"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,9 +2,11 @@
|
||||
// See LICENSE.txt for license information.
|
||||
|
||||
import noDispatchGetState from './no-dispatch-getstate.js';
|
||||
import noRedundantAdminConfigDeps from './no-redundant-admin-config-deps.js';
|
||||
import useExternalLink from './use-external-link.js';
|
||||
|
||||
export default {
|
||||
'no-dispatch-getstate': noDispatchGetState,
|
||||
'no-redundant-admin-config-deps': noRedundantAdminConfigDeps,
|
||||
'use-external-link': useExternalLink,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,439 @@
|
||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||
// See LICENSE.txt for license information.
|
||||
|
||||
/**
|
||||
* Detects redundant System Console isDisabled conditions that are already
|
||||
* implied by a parent config setting dependency.
|
||||
*
|
||||
* When setting B declares `it.stateIsFalse('A')` (or equivalent) and A is a bool
|
||||
* setting that already disables itself under condition C, repeating C on B is
|
||||
* redundant and can drift when A's dependencies change.
|
||||
*
|
||||
* Inheritance only follows bool parents: disabled bools are forced false on
|
||||
* save (see getSettingValue), so stateIsFalse(parent) covers the parent's
|
||||
* config checks. File/text parents can remain truthy while disabled, so their
|
||||
* checks are not treated as transitive.
|
||||
*
|
||||
* The dependency tree is built fully before reporting. When a setting lists
|
||||
* both a parent and a grandparent, redundant conditions are attributed to the
|
||||
* closest (most specific) bool parent — not whichever ancestor appears first
|
||||
* in the isDisabled list.
|
||||
*
|
||||
* Only config/state helpers are considered (stateIsFalse/True/Equals/...,
|
||||
* configIsFalse/True, …). Permission and license checks are left alone —
|
||||
* non-bool children still need their own permission gates.
|
||||
*/
|
||||
|
||||
const CONFIG_HELPERS = new Set([
|
||||
'stateIsFalse',
|
||||
'stateIsTrue',
|
||||
'stateEquals',
|
||||
'stateEqualsOrDefault',
|
||||
'stateMatches',
|
||||
'configIsFalse',
|
||||
'configIsTrue',
|
||||
'configContains',
|
||||
'clientConfigIsTrue',
|
||||
'clientConfigIsFalse',
|
||||
]);
|
||||
|
||||
function isItMemberCall(node, name) {
|
||||
return (
|
||||
node?.type === 'CallExpression' &&
|
||||
node.callee?.type === 'MemberExpression' &&
|
||||
!node.callee.computed &&
|
||||
node.callee.object?.type === 'Identifier' &&
|
||||
node.callee.object.name === 'it' &&
|
||||
node.callee.property?.type === 'Identifier' &&
|
||||
(name === undefined || node.callee.property.name === name)
|
||||
);
|
||||
}
|
||||
|
||||
function literalValue(node) {
|
||||
if (!node) {
|
||||
return undefined;
|
||||
}
|
||||
if (node.type === 'Literal') {
|
||||
return node.value;
|
||||
}
|
||||
if (node.type === 'TemplateLiteral' && node.expressions.length === 0) {
|
||||
return node.quasis[0]?.value?.cooked;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function serializeArg(node) {
|
||||
// Regex literals must be keyed by pattern+flags. literalValue returns a
|
||||
// RegExp whose JSON.stringify is "{}", which would collapse distinct patterns.
|
||||
if (node?.type === 'Literal' && node.regex) {
|
||||
return `re:${JSON.stringify(node.regex.pattern)}:${JSON.stringify(node.regex.flags)}`;
|
||||
}
|
||||
|
||||
const value = literalValue(node);
|
||||
if (value === undefined) {
|
||||
if (node?.type === 'Identifier') {
|
||||
return `id:${node.name}`;
|
||||
}
|
||||
if (node?.type === 'MemberExpression') {
|
||||
return `mem:${serializeArg(node.object)}.${node.property?.name ?? '?'}`;
|
||||
}
|
||||
if (node?.type === 'UnaryExpression' && node.operator === '!' && node.argument?.type === 'Literal') {
|
||||
return `!${JSON.stringify(node.argument.value)}`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
return JSON.stringify(value);
|
||||
}
|
||||
|
||||
function canonicalize(node) {
|
||||
if (!node) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (isItMemberCall(node, 'not') && node.arguments.length === 1) {
|
||||
const inner = canonicalize(node.arguments[0]);
|
||||
return inner ? `not:${inner}` : null;
|
||||
}
|
||||
|
||||
if (!isItMemberCall(node) || node.arguments.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const name = node.callee.property.name;
|
||||
if (!CONFIG_HELPERS.has(name)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const args = node.arguments.map(serializeArg);
|
||||
if (args.some((a) => a === null)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return `${name}:${args.join(',')}`;
|
||||
}
|
||||
|
||||
function collectTopLevelConditions(node, out) {
|
||||
if (!node) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (isItMemberCall(node, 'any')) {
|
||||
for (const arg of node.arguments) {
|
||||
collectTopLevelConditions(arg, out);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const key = canonicalize(node);
|
||||
if (key) {
|
||||
out.push({key, node});
|
||||
}
|
||||
}
|
||||
|
||||
function getProperty(objectExpression, name) {
|
||||
if (objectExpression?.type !== 'ObjectExpression') {
|
||||
return null;
|
||||
}
|
||||
for (const prop of objectExpression.properties) {
|
||||
if (prop.type !== 'Property' || prop.computed) {
|
||||
continue;
|
||||
}
|
||||
const keyName = getPropertyKeyName(prop.key);
|
||||
if (keyName === name) {
|
||||
return prop;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function getPropertyKeyName(keyNode) {
|
||||
if (keyNode.type === 'Identifier') {
|
||||
return keyNode.name;
|
||||
}
|
||||
if (keyNode.type === 'Literal') {
|
||||
return keyNode.value;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function getSettingKey(objectExpression) {
|
||||
const keyProp = getProperty(objectExpression, 'key');
|
||||
if (!keyProp) {
|
||||
return null;
|
||||
}
|
||||
return literalValue(keyProp.value);
|
||||
}
|
||||
|
||||
function getSettingType(objectExpression) {
|
||||
const typeProp = getProperty(objectExpression, 'type');
|
||||
if (!typeProp) {
|
||||
return null;
|
||||
}
|
||||
return literalValue(typeProp.value);
|
||||
}
|
||||
|
||||
function parseSerializedStringArg(serialized) {
|
||||
try {
|
||||
const value = JSON.parse(serialized);
|
||||
return typeof value === 'string' ? value : undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function dependencyParentKeys(conditions) {
|
||||
const parents = [];
|
||||
for (const {key} of conditions) {
|
||||
// stateIsFalse:"Section.Setting" or not:stateIsTrue:"Section.Setting"
|
||||
let match = (/^stateIsFalse:(.*)$/).exec(key);
|
||||
if (match) {
|
||||
const value = parseSerializedStringArg(match[1]);
|
||||
if (value !== undefined) {
|
||||
parents.push(value);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
match = (/^not:stateIsTrue:(.*)$/).exec(key);
|
||||
if (match) {
|
||||
const value = parseSerializedStringArg(match[1]);
|
||||
if (value !== undefined) {
|
||||
parents.push(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
return parents;
|
||||
}
|
||||
|
||||
function isBoolSetting(key, settingsByKey) {
|
||||
const defs = settingsByKey.get(key) || [];
|
||||
return defs.some((def) => def.settingType === 'bool');
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the full bool-parent dependency tree once, then derive inherited
|
||||
* condition sets. Reporting uses this snapshot so parent attribution does not
|
||||
* depend on isDisabled declaration order.
|
||||
*
|
||||
* @returns {Map<string, {
|
||||
* isBool: boolean,
|
||||
* directConditions: Set<string>,
|
||||
* boolParents: string[],
|
||||
* inheritedConditions: Set<string>,
|
||||
* ancestors: Set<string>,
|
||||
* }>}
|
||||
*/
|
||||
function buildDependencyTree(settingsByKey) {
|
||||
const tree = new Map();
|
||||
|
||||
for (const [key, defs] of settingsByKey) {
|
||||
const directConditions = new Set();
|
||||
const parentKeys = [];
|
||||
let isBool = false;
|
||||
|
||||
for (const def of defs) {
|
||||
if (def.settingType === 'bool') {
|
||||
isBool = true;
|
||||
}
|
||||
for (const {key: conditionKey} of def.conditions) {
|
||||
directConditions.add(conditionKey);
|
||||
}
|
||||
for (const parentKey of def.parentKeys) {
|
||||
if (!parentKeys.includes(parentKey)) {
|
||||
parentKeys.push(parentKey);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
tree.set(key, {
|
||||
isBool,
|
||||
directConditions,
|
||||
boolParents: parentKeys.filter((parentKey) => isBoolSetting(parentKey, settingsByKey)),
|
||||
inheritedConditions: new Set(),
|
||||
ancestors: new Set(),
|
||||
});
|
||||
}
|
||||
|
||||
const walked = new Set();
|
||||
|
||||
function walk(settingKey, visiting) {
|
||||
const node = tree.get(settingKey);
|
||||
if (!node || walked.has(settingKey)) {
|
||||
return node;
|
||||
}
|
||||
if (visiting.has(settingKey)) {
|
||||
return node;
|
||||
}
|
||||
visiting.add(settingKey);
|
||||
|
||||
const inheritedConditions = new Set();
|
||||
const ancestors = new Set();
|
||||
|
||||
for (const parentKey of node.boolParents) {
|
||||
ancestors.add(parentKey);
|
||||
const parentNode = walk(parentKey, visiting);
|
||||
if (!parentNode) {
|
||||
continue;
|
||||
}
|
||||
for (const conditionKey of parentNode.directConditions) {
|
||||
inheritedConditions.add(conditionKey);
|
||||
}
|
||||
for (const conditionKey of parentNode.inheritedConditions) {
|
||||
inheritedConditions.add(conditionKey);
|
||||
}
|
||||
for (const ancestorKey of parentNode.ancestors) {
|
||||
ancestors.add(ancestorKey);
|
||||
}
|
||||
}
|
||||
|
||||
node.inheritedConditions = inheritedConditions;
|
||||
node.ancestors = ancestors;
|
||||
walked.add(settingKey);
|
||||
visiting.delete(settingKey);
|
||||
return node;
|
||||
}
|
||||
|
||||
for (const key of tree.keys()) {
|
||||
walk(key, new Set());
|
||||
}
|
||||
|
||||
return tree;
|
||||
}
|
||||
|
||||
function nodeImpliesCondition(node, conditionKey) {
|
||||
return node.directConditions.has(conditionKey) || node.inheritedConditions.has(conditionKey);
|
||||
}
|
||||
|
||||
/**
|
||||
* Among immediate bool parents that imply the condition, pick the closest /
|
||||
* most specific one: a parent that is a descendant of another candidate wins
|
||||
* over that ancestor. Declaration order is ignored.
|
||||
*/
|
||||
function closestImplyingParent(settingKey, conditionKey, tree) {
|
||||
const node = tree.get(settingKey);
|
||||
if (!node) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const candidates = node.boolParents.filter((parentKey) => {
|
||||
const parentNode = tree.get(parentKey);
|
||||
return parentNode && nodeImpliesCondition(parentNode, conditionKey);
|
||||
});
|
||||
|
||||
if (candidates.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return candidates.reduce((best, candidate) => {
|
||||
const bestNode = tree.get(best);
|
||||
const candidateNode = tree.get(candidate);
|
||||
|
||||
// Prefer the candidate that is under the current best (more specific).
|
||||
if (candidateNode.ancestors.has(best)) {
|
||||
return candidate;
|
||||
}
|
||||
|
||||
// Keep best when it is under the candidate.
|
||||
if (bestNode.ancestors.has(candidate)) {
|
||||
return best;
|
||||
}
|
||||
|
||||
// Disjoint parents that both imply the condition: stable tie-break by key.
|
||||
return candidate < best ? candidate : best;
|
||||
});
|
||||
}
|
||||
|
||||
export default {
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description: 'Disallow System Console isDisabled conditions already implied by a parent config dependency',
|
||||
},
|
||||
schema: [],
|
||||
messages: {
|
||||
redundant:
|
||||
"Redundant isDisabled condition '{{condition}}' on '{{setting}}' — already implied by dependency on '{{parent}}'. Depend on the parent setting and omit duplicated config checks.",
|
||||
},
|
||||
},
|
||||
create(context) {
|
||||
const filename = context.filename || context.getFilename();
|
||||
if (!(/admin_definition/).test(filename)) {
|
||||
return {};
|
||||
}
|
||||
|
||||
const settings = [];
|
||||
|
||||
return {
|
||||
ObjectExpression(node) {
|
||||
const settingKey = getSettingKey(node);
|
||||
if (typeof settingKey !== 'string' || !settingKey.includes('.')) {
|
||||
return;
|
||||
}
|
||||
|
||||
const settingType = getSettingType(node);
|
||||
if (typeof settingType !== 'string') {
|
||||
return;
|
||||
}
|
||||
|
||||
const isDisabledProp = getProperty(node, 'isDisabled');
|
||||
if (!isDisabledProp) {
|
||||
return;
|
||||
}
|
||||
|
||||
const conditions = [];
|
||||
collectTopLevelConditions(isDisabledProp.value, conditions);
|
||||
if (conditions.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
settings.push({
|
||||
key: settingKey,
|
||||
settingType,
|
||||
conditions,
|
||||
parentKeys: dependencyParentKeys(conditions),
|
||||
node,
|
||||
});
|
||||
},
|
||||
|
||||
'Program:exit'() {
|
||||
const settingsByKey = new Map();
|
||||
for (const setting of settings) {
|
||||
if (!settingsByKey.has(setting.key)) {
|
||||
settingsByKey.set(setting.key, []);
|
||||
}
|
||||
settingsByKey.get(setting.key).push(setting);
|
||||
}
|
||||
|
||||
// Build the complete tree before attributing any parent.
|
||||
const tree = buildDependencyTree(settingsByKey);
|
||||
|
||||
for (const setting of settings) {
|
||||
const node = tree.get(setting.key);
|
||||
if (!node || node.boolParents.length === 0 || node.inheritedConditions.size === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const {key, node: conditionNode} of setting.conditions) {
|
||||
if (!node.inheritedConditions.has(key)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const implyingParent = closestImplyingParent(setting.key, key, tree);
|
||||
if (!implyingParent) {
|
||||
continue;
|
||||
}
|
||||
|
||||
context.report({
|
||||
node: conditionNode,
|
||||
messageId: 'redundant',
|
||||
data: {
|
||||
condition: key,
|
||||
setting: setting.key,
|
||||
parent: implyingParent,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
@@ -0,0 +1,339 @@
|
||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||
// See LICENSE.txt for license information.
|
||||
|
||||
import {RuleTester} from 'eslint';
|
||||
|
||||
import rule from './no-redundant-admin-config-deps.js';
|
||||
|
||||
const ruleTester = new RuleTester({
|
||||
languageOptions: {
|
||||
ecmaVersion: 2022,
|
||||
sourceType: 'module',
|
||||
},
|
||||
});
|
||||
|
||||
const filename = 'admin_definition.tsx';
|
||||
|
||||
ruleTester.run('no-redundant-admin-config-deps', rule, {
|
||||
valid: [
|
||||
{
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'EmailSettings.EnableEmailBatching',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'number',
|
||||
key: 'EmailSettings.EmailBatchingBufferSize',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.EnableEmailBatching'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
},
|
||||
{
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'ServiceSettings.EnableOAuthServiceProvider',
|
||||
isDisabled: it.not(it.userHasWritePermissionOnResource('integrations')),
|
||||
},
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'ServiceSettings.EnableDynamicClientRegistration',
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource('integrations')),
|
||||
it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'text',
|
||||
key: 'ServiceSettings.DCRRedirectURIAllowlist',
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource('integrations')),
|
||||
it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
},
|
||||
{
|
||||
|
||||
// Non-bool parents (fileupload) do not imply their isDisabled conditions,
|
||||
// so repeating Encrypt alongside PrivateKeyFile is intentional.
|
||||
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'SamlSettings.Encrypt',
|
||||
isDisabled: it.stateIsFalse('SamlSettings.Enable'),
|
||||
},
|
||||
{
|
||||
type: 'fileupload',
|
||||
key: 'SamlSettings.PrivateKeyFile',
|
||||
isDisabled: it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
},
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'SamlSettings.SignRequest',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
it.stateIsFalse('SamlSettings.PrivateKeyFile'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
},
|
||||
{
|
||||
|
||||
// Distinct regex patterns on the same helper+key are not the same condition.
|
||||
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'SupportSettings.Enable',
|
||||
isDisabled: it.stateMatches('SupportSettings.ReportAProblemType', /link/),
|
||||
},
|
||||
{
|
||||
type: 'text',
|
||||
key: 'SupportSettings.Mail',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('SupportSettings.Enable'),
|
||||
it.stateMatches('SupportSettings.ReportAProblemType', /email/),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
},
|
||||
{
|
||||
|
||||
// Identifier parent keys must not throw during canonicalize/JSON.parse.
|
||||
|
||||
filename,
|
||||
code: `
|
||||
const PARENT_KEY = 'ServiceSettings.EnableOAuthServiceProvider';
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'ServiceSettings.EnableDynamicClientRegistration',
|
||||
isDisabled: it.stateIsFalse(PARENT_KEY),
|
||||
},
|
||||
{
|
||||
type: 'text',
|
||||
key: 'ServiceSettings.DCRRedirectURIAllowlist',
|
||||
isDisabled: it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
},
|
||||
{
|
||||
|
||||
// Non-admin_definition files are ignored
|
||||
|
||||
filename: 'other_file.tsx',
|
||||
code: `
|
||||
const x = {
|
||||
type: 'number',
|
||||
key: 'EmailSettings.EmailBatchingBufferSize',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.stateIsFalse('EmailSettings.EnableEmailBatching'),
|
||||
),
|
||||
};
|
||||
const y = {
|
||||
type: 'bool',
|
||||
key: 'EmailSettings.EnableEmailBatching',
|
||||
isDisabled: it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
};
|
||||
`,
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'EmailSettings.EnableEmailBatching',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
it.configIsFalse('ServiceSettings', 'SiteURL'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'number',
|
||||
key: 'EmailSettings.EmailBatchingBufferSize',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('EmailSettings.SendEmailNotifications'),
|
||||
it.stateIsFalse('EmailSettings.EnableEmailBatching'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
it.configIsFalse('ServiceSettings', 'SiteURL'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
errors: [
|
||||
{messageId: 'redundant', data: {condition: 'stateIsFalse:"EmailSettings.SendEmailNotifications"', setting: 'EmailSettings.EmailBatchingBufferSize', parent: 'EmailSettings.EnableEmailBatching'}},
|
||||
{messageId: 'redundant', data: {condition: 'configIsTrue:"ClusterSettings","Enable"', setting: 'EmailSettings.EmailBatchingBufferSize', parent: 'EmailSettings.EnableEmailBatching'}},
|
||||
{messageId: 'redundant', data: {condition: 'configIsFalse:"ServiceSettings","SiteURL"', setting: 'EmailSettings.EmailBatchingBufferSize', parent: 'EmailSettings.EnableEmailBatching'}},
|
||||
],
|
||||
},
|
||||
{
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'ServiceSettings.EnableOAuthServiceProvider',
|
||||
isDisabled: it.not(it.userHasWritePermissionOnResource('integrations')),
|
||||
},
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'ServiceSettings.EnableDynamicClientRegistration',
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource('integrations')),
|
||||
it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'text',
|
||||
key: 'ServiceSettings.DCRRedirectURIAllowlist',
|
||||
isDisabled: it.any(
|
||||
it.not(it.userHasWritePermissionOnResource('integrations')),
|
||||
it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'),
|
||||
it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
errors: [
|
||||
{messageId: 'redundant', data: {condition: 'stateIsFalse:"ServiceSettings.EnableOAuthServiceProvider"', setting: 'ServiceSettings.DCRRedirectURIAllowlist', parent: 'ServiceSettings.EnableDynamicClientRegistration'}},
|
||||
],
|
||||
},
|
||||
{
|
||||
|
||||
// Bool parent Encrypt implies Enable; PrivateKeyFile should not repeat Enable
|
||||
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'SamlSettings.Encrypt',
|
||||
isDisabled: it.stateIsFalse('SamlSettings.Enable'),
|
||||
},
|
||||
{
|
||||
type: 'fileupload',
|
||||
key: 'SamlSettings.PrivateKeyFile',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('SamlSettings.Enable'),
|
||||
it.stateIsFalse('SamlSettings.Encrypt'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
errors: [
|
||||
{messageId: 'redundant', data: {condition: 'stateIsFalse:"SamlSettings.Enable"', setting: 'SamlSettings.PrivateKeyFile', parent: 'SamlSettings.Encrypt'}},
|
||||
],
|
||||
},
|
||||
{
|
||||
|
||||
// Grandparent listed before parent: still attribute to the closest parent (Mid),
|
||||
// not Root, after the full tree is built.
|
||||
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'FeatureSettings.Root',
|
||||
isDisabled: it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
},
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'FeatureSettings.Mid',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('FeatureSettings.Root'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
),
|
||||
},
|
||||
{
|
||||
type: 'number',
|
||||
key: 'FeatureSettings.Leaf',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('FeatureSettings.Root'),
|
||||
it.stateIsFalse('FeatureSettings.Mid'),
|
||||
it.configIsTrue('ClusterSettings', 'Enable'),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
errors: [
|
||||
{messageId: 'redundant', data: {condition: 'configIsTrue:"ClusterSettings","Enable"', setting: 'FeatureSettings.Mid', parent: 'FeatureSettings.Root'}},
|
||||
{messageId: 'redundant', data: {condition: 'stateIsFalse:"FeatureSettings.Root"', setting: 'FeatureSettings.Leaf', parent: 'FeatureSettings.Mid'}},
|
||||
{messageId: 'redundant', data: {condition: 'configIsTrue:"ClusterSettings","Enable"', setting: 'FeatureSettings.Leaf', parent: 'FeatureSettings.Mid'}},
|
||||
],
|
||||
},
|
||||
{
|
||||
|
||||
// Same regex on parent and child is redundant; distinct patterns are not.
|
||||
|
||||
filename,
|
||||
code: `
|
||||
const AdminDefinition = {
|
||||
settings: [
|
||||
{
|
||||
type: 'bool',
|
||||
key: 'SupportSettings.Enable',
|
||||
isDisabled: it.stateMatches('SupportSettings.ReportAProblemType', /link/),
|
||||
},
|
||||
{
|
||||
type: 'text',
|
||||
key: 'SupportSettings.Mail',
|
||||
isDisabled: it.any(
|
||||
it.stateIsFalse('SupportSettings.Enable'),
|
||||
it.stateMatches('SupportSettings.ReportAProblemType', /link/),
|
||||
),
|
||||
},
|
||||
],
|
||||
};
|
||||
`,
|
||||
errors: [
|
||||
{messageId: 'redundant', data: {condition: 'stateMatches:"SupportSettings.ReportAProblemType",re:"link":""', setting: 'SupportSettings.Mail', parent: 'SupportSettings.Enable'}},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
Reference in New Issue
Block a user