[Authz] Enabled no_deprecated_authz_config for migration only (#196852)

## Summary To ensure that `no_deprecated_authz_config` rule is only applied during an intentional migration, added check for env vars presence. If neither `MIGRATE_ENABLED_AUTHZ` nor `MIGRATE_DISABLED_AUTHZ` is set, the rule will be skipped, avoiding unnecessary or unforeseen code changes both locally and in the CI. Added fix and test case for `access:${APP.TEST_ID}` tags that have property access in the template literal. __Closes: https://github.com/elastic/kibana/issues/196846__ ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Elastic Machine <[email protected]>
elastic · Oct 22, 2024 · 5ffe226 · 5ffe226
1 parent c5067fd
commit 5ffe226
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 35 deletions.
diff --git a/packages/kbn-eslint-config/.eslintrc.js b/packages/kbn-eslint-config/.eslintrc.js
@@ -326,6 +326,7 @@ module.exports = {
     '@kbn/imports/uniform_imports': 'error',
     '@kbn/imports/no_unused_imports': 'error',
     '@kbn/imports/no_boundary_crossing': 'error',
+    '@kbn/eslint/no_deprecated_authz_config': 'error',
 
     'no-new-func': 'error',
     'no-implied-eval': 'error',

diff --git a/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.js b/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.js
@@ -39,6 +39,33 @@ const maybeReportDisabledSecurityConfig = (node, context, isVersionedRoute = fal
     return;
   }
 
+  const hasSecurityInRoot = (config) => {
+    const securityInRoot = config.properties.find(
+      (property) => property.key && property.key.name === 'security'
+    );
+
+    if (securityInRoot) {
+      return true;
+    }
+
+    const optionsProperty = config.properties.find(
+      (prop) => prop.key && prop.key.name === 'options'
+    );
+
+    if (optionsProperty?.value?.properties) {
+      const tagsProperty = optionsProperty.value.properties.find(
+        (prop) => prop.key.name === 'tags'
+      );
+
+      const accessTagsFilter = (el) => isLiteralAccessTag(el) || isTemplateLiteralAccessTag(el);
+      const accessTags = tagsProperty?.value?.elements?.filter(accessTagsFilter) ?? [];
+
+      return accessTags.length > 0;
+    }
+
+    return false;
+  };
+
   if (isVersionedRoute) {
     const [versionConfig] = node.arguments;
 
@@ -53,33 +80,6 @@ const maybeReportDisabledSecurityConfig = (node, context, isVersionedRoute = fal
 
       let currentNode = node;
 
-      const hasSecurityInRoot = (config) => {
-        const securityInRoot = config.properties.find(
-          (property) => property.key && property.key.name === 'security'
-        );
-
-        if (securityInRoot) {
-          return true;
-        }
-
-        const optionsProperty = config.properties.find(
-          (prop) => prop.key && prop.key.name === 'options'
-        );
-
-        if (optionsProperty?.value?.properties) {
-          const tagsProperty = optionsProperty.value.properties.find(
-            (prop) => prop.key.name === 'tags'
-          );
-
-          const accessTagsFilter = (el) => isLiteralAccessTag(el) || isTemplateLiteralAccessTag(el);
-          const accessTags = tagsProperty.value.elements.filter(accessTagsFilter);
-
-          return accessTags.length > 0;
-        }
-
-        return false;
-      };
-
       while (
         currentNode &&
         currentNode.type === 'CallExpression' &&
@@ -126,11 +126,14 @@ const maybeReportDisabledSecurityConfig = (node, context, isVersionedRoute = fal
     }
   } else {
     const [routeConfig] = node.arguments;
-    const securityProperty = routeConfig.properties.find(
-      (property) => property.key && property.key.name === 'security'
-    );
 
-    if (!securityProperty) {
+    const pathProperty = routeConfig.properties?.find((prop) => prop?.key?.name === 'path');
+
+    if (!pathProperty) {
+      return;
+    }
+
+    if (!hasSecurityInRoot(routeConfig)) {
       const pathProperty = routeConfig.properties.find((prop) => prop.key.name === 'path');
       context.report({
         node: routeConfig,
@@ -181,7 +184,14 @@ const handleRouteConfig = (node, context, isVersionedRoute = false) => {
             const staticPart = firstQuasi.split(ACCESS_TAG_PREFIX)[1] || '';
 
             const dynamicParts = el.expressions.map((expression, index) => {
-              const dynamicPlaceholder = `\${${expression.name}}`;
+              let dynamicPlaceholder;
+              if (expression.property) {
+                // Case: object.property
+                dynamicPlaceholder = `\${${expression.object.name}.${expression.property.name}}`;
+              } else {
+                // Case: simple variable
+                dynamicPlaceholder = `\${${expression.name}}`;
+              }
               const nextQuasi = el.quasis[index + 1].value.raw || '';
               return `${dynamicPlaceholder}${nextQuasi}`;
             });
@@ -290,13 +300,25 @@ module.exports = {
       CallExpression(node) {
         const callee = node.callee;
 
+        // Skipping by default if any of env vars is not set
+        const shouldSkipMigration =
+          !process.env.MIGRATE_ENABLED_AUTHZ && !process.env.MIGRATE_DISABLED_AUTHZ;
+
+        if (shouldSkipMigration) {
+          return;
+        }
+
         if (
           callee.type === 'MemberExpression' &&
           callee.object &&
           callee.object.name === 'router' &&
           routeMethods.includes(callee.property.name)
         ) {
-          handleRouteConfig(node, context, false);
+          if (process.env.MIGRATE_ENABLED_AUTHZ === 'false') {
+            maybeReportDisabledSecurityConfig(node, context, false);
+          } else {
+            handleRouteConfig(node, context, false);
+          }
         }
 
         if (
@@ -310,7 +332,11 @@ module.exports = {
           const versionConfig = node.arguments[0];
 
           if (versionConfig && versionConfig.type === 'ObjectExpression') {
-            handleRouteConfig(node, context, true);
+            if (process.env.MIGRATE_ENABLED_AUTHZ === 'false') {
+              maybeReportDisabledSecurityConfig(node, context, true);
+            } else {
+              handleRouteConfig(node, context, true);
+            }
           }
         }
       },

diff --git a/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.test.js b/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.test.js
@@ -11,8 +11,17 @@ const { RuleTester } = require('eslint');
 const rule = require('./no_deprecated_authz_config');
 const dedent = require('dedent');
 
-// Indentation is a big problem in the test cases, dedent library does not work as expected.
+beforeAll(() => {
+  process.env.MIGRATE_ENABLED_AUTHZ = 'true';
+  process.env.MIGRATE_DISABLED_AUTHZ = 'true';
+});
+
+afterAll(() => {
+  delete process.env.MIGRATE_ENABLED_AUTHZ;
+  delete process.env.MIGRATE_DISABLED_AUTHZ;
+});
 
+// Indentation is a big problem in the test cases, dedent library does not work as expected.
 const ruleTester = new RuleTester({
   parser: require.resolve('@typescript-eslint/parser'),
   parserOptions: {
@@ -202,6 +211,28 @@ ruleTester.run('no_deprecated_authz_config', rule, {
       `,
       name: 'invalid: access tags are template literals, move to security.authz.requiredPrivileges',
     },
+    {
+      code: `
+        router.get({
+          path: '/some/path',
+          options: {
+            tags: [\`access:\${APP.TEST_ID}\`],
+          },
+        });
+      `,
+      errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }],
+      output: `
+        router.get({
+          path: '/some/path',
+          security: {
+                authz: {
+                  requiredPrivileges: [\`\${APP.TEST_ID}\`],
+                },
+              },
+        });
+      `,
+      name: 'invalid: access tags are template literals, move to security.authz.requiredPrivileges',
+    },
     {
       code: `
         router.get({