From c3a26c32e57ae16c9e3b299cb094e692f87d0aec Mon Sep 17 00:00:00 2001 From: Elena Shostak <165678770+elena-shostak@users.noreply.github.com> Date: Thu, 22 May 2025 08:45:17 +0200 Subject: [PATCH] [Authz] Cleanup of access tags functionality and documentation (#220231) ## Summary Mandatory security config has been added in https://github.com/elastic/kibana/pull/215180. This PR cleans up access tags functionality, documentation and migration eslint rule `no_deprecated_authz_config` that is no longer needed. ### Checklist - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 --- .../next_docs/build_and_validate_docs.sh | 2 +- dev_docs/key_concepts/api_authorization.mdx | 88 +--- docs/extend/development-security.md | 6 +- packages/kbn-eslint-config/.eslintrc.js | 1 - packages/kbn-eslint-plugin-eslint/index.js | 1 - .../rules/no_deprecated_authz_config.js | 345 ------------- .../rules/no_deprecated_authz_config.test.js | 462 ------------------ .../core_versioned_route.test.ts | 4 +- .../server-internal/src/http_server.test.ts | 10 +- .../http/server-internal/src/http_server.ts | 10 +- .../authorization/api_authorization.test.ts | 80 ++- .../server/authorization/api_authorization.ts | 324 ++++++------ .../product_features_service.test.ts | 38 -- .../product_features_service.ts | 6 - 14 files changed, 252 insertions(+), 1125 deletions(-) delete mode 100644 packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.js delete mode 100644 packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.test.js diff --git a/.buildkite/scripts/steps/next_docs/build_and_validate_docs.sh b/.buildkite/scripts/steps/next_docs/build_and_validate_docs.sh index 28f7bba3c46a..4abb05e9d261 100755 --- a/.buildkite/scripts/steps/next_docs/build_and_validate_docs.sh +++ b/.buildkite/scripts/steps/next_docs/build_and_validate_docs.sh @@ -7,7 +7,7 @@ source .buildkite/scripts/bootstrap.sh echo "--- Build docs" echo "⚠️ run 'node scripts/validate_next_docs --debug' locally to debug ⚠️" -node scripts/validate_next_docs +node scripts/validate_next_docs --debug diff --git a/dev_docs/key_concepts/api_authorization.mdx b/dev_docs/key_concepts/api_authorization.mdx index d5bf4f3f17b8..2bfc0ee06023 100644 --- a/dev_docs/key_concepts/api_authorization.mdx +++ b/dev_docs/key_concepts/api_authorization.mdx @@ -11,18 +11,15 @@ Authorization is an important aspect of API design. It must be considered for al Table of contents: 1. [API authorization](#api-authorization) -2. [[Deprecated] Adding API authorization with `access` tags](#deprecated-adding-api-authorization-with-access-tags) - - [Why not add `access` tags to all routes by default?](#why-not-add-access-tags-to-all-routes-by-default) -3. [Adding API authorization with `security` configuration](#adding-api-authorization-with-security-configuration) +2. [Adding API authorization with `security` configuration](#adding-api-authorization-with-security-configuration) - [Key features](#key-features) - [Configuring authorization on routes](#configuring-authorization-on-routes) - [Opting out of authorization for specific routes](#opting-out-of-authorization-for-specific-routes) - [Classic router security configuration examples](#classic-router-security-configuration-examples) - [Versioned router security configuration examples](#versioned-router-security-configuration-examples) -4. [Authorization response available in route handlers](#authorization-response-available-in-route-handlers) -5. [OpenAPI specification (OAS) documentation](#openapi-specification-oas-documentation) -6. [Migrating from `access` tags to `security` configuration](#migrating-from-access-tags-to-security-configuration) -7. [Questions?](#questions) +3. [Authorization response available in route handlers](#authorization-response-available-in-route-handlers) +4. [OpenAPI specification (OAS) documentation](#openapi-specification-oas-documentation) +5. [Questions?](#questions) ## API authorization Kibana API routes do not have any authorization checks applied by default. This means that your APIs are accessible to anyone with valid credentials, regardless of their permissions. This includes users with no roles assigned. @@ -38,29 +35,6 @@ This is **especially** important if your route does any of the following: 3. Calls a third-party service. 4. Exposes any non-public information to the caller, such as system configuration or state, as part of the successful or even error response. -## [Deprecated] Adding API authorization with `access` tags -**Note**: `access` tags were deprecated in favour of `security` configuration. - -`access` tags are used to restrict access to API routes. They are used to ensure that only users with the required privileges can access the route. - -Example configuration: -```ts -router.get({ - path: '/api/path', - options: { - tags: ['access:', 'access:'], - }, - ... -}, handler); -``` - -More information on adding `access` tags to your routes can be found temporarily in the [legacy documentation](https://www.elastic.co/guide/en/kibana/current/development-security.html#development-plugin-feature-registration) - -### Why not add `access` tags to all routes by default? -Each authorization check that we perform involves a round-trip to Elasticsearch, so they are not as cheap as we'd like. Therefore, we want to keep the number of authorization checks we perform within a single route to a minimum. -Adding an `access` tag to routes that leverage the Saved Objects Service would be redundant in most cases, since the Saved Objects Service will be performing authorization anyway. - - ## Adding API authorization with `security` configuration `KibanaRouteOptions` provides a security configuration at the route definition level, offering robust security configurations for both **Classic** and **Versioned** routes. @@ -78,18 +52,6 @@ Adding an `access` tag to routes that leverage the Saved Objects Service would b ### Configuring authorization on routes -**Before migration:** -```ts -router.get({ - path: '/api/path', - options: { - tags: ['access:', 'access:'], - }, - ... -}, handler); -``` - -**After migration:** ```ts router.get({ path: '/api/path', @@ -379,48 +341,6 @@ To check the OAS documentation for a specific API route and see its security det GET /api/oas?pathStartsWith=/your/api/path ``` -## Migrating from `access` tags to `security` configuration -We aim to use the same privileges that are currently defined with tags `access:`. -To assist with this migration, we have created eslint rule `no_deprecated_authz_config`, that will automatically convert your `access` tags to the new `security` configuration. -It scans route definitions and converts `access` tags to the new `requiredPrivileges` configuration. - -Note: The rule is disabled by default to avoid automatic migrations without an oversight. You can perform migrations by running: - -**Migrate routes with defined authorization** -```sh -MIGRATE_DISABLED_AUTHZ=false MIGRATE_ENABLED_AUTHZ=true npx eslint --ext .ts --fix path/to/your/folder -``` - -**Migrate routes opted out from authorization** -```sh -MIGRATE_DISABLED_AUTHZ=true MIGRATE_ENABLED_AUTHZ=false npx eslint --ext .ts --fix path/to/your/folder -``` -We encourage you to migrate routes that are opted out from authorization to new config and provide legitimate reason for disabled authorization. -It is better to migrate routes opted out from authorization iteratively and elaborate on the reasoning. -Routes without a compelling reason to opt-out of authorization should plan to introduce them as soon as possible. - -**Migrate all routes** -```sh -MIGRATE_DISABLED_AUTHZ=true MIGRATE_ENABLED_AUTHZ=true npx eslint --ext .ts --fix path/to/your/folder -``` - -**How to migrate if you have an utility function for route creation?** -If you have utility function that creates routes, i.e `createApmServerRoute` or `createObservabilityOnboardingServerRoute`, you can easily modify the eslint rule to handle your case. -For example, you register the route with `access` tags in your utility function: -```ts -createApmServerRoute({ - endpoint: 'GET /your/route/path', - options: { tags: ['access:apm'] }, - handler: async (resources): => { - // your handler logic - }, -}) -``` -You can modify [the rule](https://github.com/elastic/kibana/blob/6a50066e00ae38a64c5365fd66b4dc32857ba1fc/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.js#L312-#L315) to handle your case by adding the following code: -```ts -callee.type === 'Identifier' && callee.name === 'createApmServerRoute' -``` - ## Questions? If you have any questions or need help with API authorization, please reach out to the `@elastic/kibana-security` team. diff --git a/docs/extend/development-security.md b/docs/extend/development-security.md index 6169f0aaf285..219b9d45cc21 100644 --- a/docs/extend/development-security.md +++ b/docs/extend/development-security.md @@ -268,8 +268,12 @@ Unlike the Canvas example, Dev Tools does not require access to any saved object server.route({ path: '/api/console/proxy', method: 'POST', + security: { + authz: { + requiredPrivileges: ['console'], + }, + }, config: { - tags: ['access:console'], handler: async (req, h) => { // ... } diff --git a/packages/kbn-eslint-config/.eslintrc.js b/packages/kbn-eslint-config/.eslintrc.js index 2d215f794abf..20f4b34cf02d 100644 --- a/packages/kbn-eslint-config/.eslintrc.js +++ b/packages/kbn-eslint-config/.eslintrc.js @@ -318,7 +318,6 @@ module.exports = { '@kbn/disable/no_naked_eslint_disable': 'error', '@kbn/eslint/no_async_promise_body': 'error', '@kbn/eslint/no_async_foreach': 'error', - '@kbn/eslint/no_deprecated_authz_config': 'error', '@kbn/eslint/require_kibana_feature_privileges_naming': 'warn', '@kbn/eslint/no_trailing_import_slash': 'error', '@kbn/eslint/no_constructor_args_in_property_initializers': 'error', diff --git a/packages/kbn-eslint-plugin-eslint/index.js b/packages/kbn-eslint-plugin-eslint/index.js index 2eeb718ad160..e022fee797c0 100644 --- a/packages/kbn-eslint-plugin-eslint/index.js +++ b/packages/kbn-eslint-plugin-eslint/index.js @@ -20,7 +20,6 @@ module.exports = { no_this_in_property_initializers: require('./rules/no_this_in_property_initializers'), no_unsafe_console: require('./rules/no_unsafe_console'), no_unsafe_hash: require('./rules/no_unsafe_hash'), - no_deprecated_authz_config: require('./rules/no_deprecated_authz_config'), require_kibana_feature_privileges_naming: require('./rules/require_kibana_feature_privileges_naming'), }, }; 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 deleted file mode 100644 index 8661c5e1c52d..000000000000 --- a/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.js +++ /dev/null @@ -1,345 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -const routeMethods = ['get', 'put', 'delete', 'post', 'patch']; -const ACCESS_TAG_PREFIX = 'access:'; - -const isStringLiteral = (el) => el.type === 'Literal' && typeof el.value === 'string'; -const isLiteralAccessTag = (el) => isStringLiteral(el) && el.value.startsWith(ACCESS_TAG_PREFIX); - -const isTemplateLiteralAccessTag = (el) => - el.type === 'TemplateLiteral' && el.quasis[0].value.raw.startsWith(ACCESS_TAG_PREFIX); - -const maybeReportDisabledSecurityConfig = (node, context, isVersionedRoute = false) => { - // Allow disabling migration for routes that are opted out from authorization - if (process.env.MIGRATE_DISABLED_AUTHZ === 'false') { - return; - } - - const callee = node.callee; - const isAddVersionCall = - callee.type === 'MemberExpression' && callee.property.name === 'addVersion'; - - const disabledSecurityConfig = ` - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - },`; - - // Skipping root route call intentionally, we will check root route security config in addVersion node traversal - if (isVersionedRoute && !isAddVersionCall) { - 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; - - if (versionConfig && versionConfig.type === 'ObjectExpression') { - const securityInVersion = versionConfig.properties.find( - (property) => property.key && property.key.name === 'security' - ); - - if (securityInVersion) { - return; - } - - let currentNode = node; - - while ( - currentNode && - currentNode.type === 'CallExpression' && - currentNode.callee.type === 'MemberExpression' - ) { - const callee = currentNode.callee; - - if ( - callee.object && - callee.object.property && - callee.object.property.name === 'versioned' && - routeMethods.includes(callee.property.name) - ) { - const [routeConfig] = currentNode.arguments; - - if (routeConfig && routeConfig.type === 'ObjectExpression') { - const securityInRoot = hasSecurityInRoot(routeConfig); - - // If security is missing in both the root and the version - if (!securityInRoot) { - context.report({ - node: versionConfig, - message: 'Security config is missing in addVersion call', - fix(fixer) { - const versionProperty = versionConfig.properties.find( - (property) => property.key && property.key.name === 'version' - ); - const insertPosition = versionProperty.range[1]; - - return fixer.insertTextAfterRange( - [insertPosition, insertPosition + 1], - `${disabledSecurityConfig}` - ); - }, - }); - } - } - - break; - } - - currentNode = callee.object; - } - } - } else { - const [routeConfig] = node.arguments; - - 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, - message: 'Security config is missing', - fix(fixer) { - const insertPosition = pathProperty.range[1]; - - return fixer.insertTextAfterRange( - [insertPosition, insertPosition + 1], - `${disabledSecurityConfig}` - ); - }, - }); - } - } -}; - -const handleRouteConfig = (node, context, isVersionedRoute = false) => { - const [routeConfig] = node.arguments; - - if (routeConfig && routeConfig.type === 'ObjectExpression') { - const optionsProperty = routeConfig.properties.find( - (prop) => prop.key && prop.key.name === 'options' - ); - - if (!optionsProperty) { - return maybeReportDisabledSecurityConfig(node, context, isVersionedRoute); - } - - if (optionsProperty?.value?.properties) { - const tagsProperty = optionsProperty.value.properties.find( - (prop) => prop.key.name === 'tags' - ); - - const accessTagsFilter = (el) => isLiteralAccessTag(el) || isTemplateLiteralAccessTag(el); - const nonAccessTagsFilter = (el) => - !isLiteralAccessTag(el) && !isTemplateLiteralAccessTag(el); - - const getAccessPrivilege = (el) => { - if (el.type === 'Literal') { - return `'${el.value.split(ACCESS_TAG_PREFIX)[1]}'`; - } - - if (el.type === 'TemplateLiteral') { - const firstQuasi = el.quasis[0].value.raw; - - if (firstQuasi.startsWith(ACCESS_TAG_PREFIX)) { - const staticPart = firstQuasi.split(ACCESS_TAG_PREFIX)[1] || ''; - - const dynamicParts = el.expressions.map((expression, index) => { - 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}`; - }); - - return `\`${staticPart}${dynamicParts.join('')}\``; - } - } - }; - - if (!tagsProperty) { - return maybeReportDisabledSecurityConfig(node, context, isVersionedRoute); - } - - if (tagsProperty && tagsProperty.value.type === 'ArrayExpression') { - const accessTags = tagsProperty.value.elements.filter(accessTagsFilter); - const nonAccessTags = tagsProperty.value.elements.filter(nonAccessTagsFilter); - - if (!accessTags.length) { - return maybeReportDisabledSecurityConfig(node, context, isVersionedRoute); - } - - context.report({ - node: tagsProperty, - message: `Move 'access' tags to security.authz.requiredPrivileges.`, - fix(fixer) { - const accessPrivileges = accessTags.map(getAccessPrivilege); - - const securityConfig = `security: { - authz: { - requiredPrivileges: [${accessPrivileges.map((priv) => priv).join(', ')}], - }, - }`; - - const sourceCode = context.getSourceCode(); - - const fixes = []; - let remainingOptions = []; - - // If there are non-access tags, keep the 'tags' property with those - if (nonAccessTags.length > 0) { - const nonAccessTagsText = `[${nonAccessTags - .map((tag) => sourceCode.getText(tag)) - .join(', ')}]`; - fixes.push(fixer.replaceText(tagsProperty.value, nonAccessTagsText)); - } else { - // Check if 'options' will be empty after removing 'tags' - remainingOptions = optionsProperty.value.properties.filter( - (prop) => prop.key.name !== 'tags' - ); - - // If options are empty, replace the entire 'options' with 'security' config - if (remainingOptions.length === 0) { - fixes.push(fixer.replaceText(optionsProperty, securityConfig)); - } - } - - // If 'options' was replaced or has other properties, insert security separately - if (remainingOptions.length > 0) { - // If no non-access tags, remove 'tags' - const nextToken = sourceCode.getTokenAfter(tagsProperty); - - if (nextToken && nextToken.value === ',') { - // Remove the 'tags' property and the trailing comma - fixes.push(fixer.removeRange([tagsProperty.range[0], nextToken.range[1]])); - } else { - fixes.push(fixer.remove(tagsProperty)); - } - fixes.push(fixer.insertTextBefore(optionsProperty, `${securityConfig},`)); - } - - if (nonAccessTags.length && !remainingOptions.length) { - fixes.push(fixer.insertTextBefore(optionsProperty, `${securityConfig},`)); - } - - return fixes; - }, - }); - } - } - } -}; - -/** - * ESLint Rule: Migrate `access` tags in route configurations to `security.authz.requiredPrivileges`. - * - * This rule checks for the following in route configurations: - * 1. If a route (e.g., `router.get()`, `router.post()`) contains an `options` property with `tags`. - * 2. If `tags` contains any `access:` tags, these are moved to `security.authz.requiredPrivileges`. - * 3. If no `security` configuration exists, it reports an error and suggests adding a default `security` config. - * 4. It handles both standard routes and versioned routes (e.g., `router.versioned.post()`, `router.addVersion()`). - * 5. If other non-access tags exist, they remain in `tags`. - */ -module.exports = { - meta: { - type: 'suggestion', - docs: { - description: 'Migrate routes with and without access tags to security config', - category: 'Best Practices', - recommended: false, - }, - fixable: 'code', - }, - - create(context) { - return { - 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) - ) { - if (process.env.MIGRATE_ENABLED_AUTHZ === 'false') { - maybeReportDisabledSecurityConfig(node, context, false); - } else { - handleRouteConfig(node, context, false); - } - } - - if ( - (callee.type === 'MemberExpression' && callee.property.name === 'addVersion') || - (callee.object && - callee.object.type === 'MemberExpression' && - callee.object.object.name === 'router' && - callee.object.property.name === 'versioned' && - routeMethods.includes(callee.property.name)) - ) { - const versionConfig = node.arguments[0]; - - if (versionConfig && versionConfig.type === 'ObjectExpression') { - 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 deleted file mode 100644 index 4f0fc375ad55..000000000000 --- a/packages/kbn-eslint-plugin-eslint/rules/no_deprecated_authz_config.test.js +++ /dev/null @@ -1,462 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -const { RuleTester } = require('eslint'); -const rule = require('./no_deprecated_authz_config'); -const dedent = require('dedent'); - -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: { - sourceType: 'module', - ecmaVersion: 2018, - }, -}); - -ruleTester.run('no_deprecated_authz_config', rule, { - valid: [ - { - code: ` - router.get( - { - path: '/api/security/authz_poc/simple_privileges_example_1', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization ', - }, - }, - validate: false, - }, - () => {} - ); - `, - name: 'valid: security config is present and authz is disabled', - }, - { - code: ` - router.get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - security: { - authz: { - requiredPrivileges: ['somePrivilege'], - }, - }, - }); - `, - name: 'valid: security config is present and authz is enabled', - }, - { - code: ` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - }) - .addVersion( - { - version: '1', - validate: false, - security: { - authz: { - requiredPrivileges: ['managePrivileges'], - }, - }, - }, - () => {} - ); - `, - name: 'valid: security config is present for versioned route', - }, - { - code: ` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - security: { - authz: { - requiredPrivileges: ['managePrivileges'], - }, - }, - }) - .addVersion( - { - version: '1', - validate: false, - }, - () => {} - ); - `, - name: 'valid: security config is present for versioned route provided in root route definition', - }, - ], - - invalid: [ - { - code: dedent(` - router.get( - { - path: '/test/path', - validate: false, - }, - () => {} - ); - `), - errors: [{ message: 'Security config is missing' }], - output: dedent(` - router.get( - { - path: '/test/path', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, - validate: false, - }, - () => {} - ); - `), - name: 'invalid: security config is missing', - }, - { - code: ` - router.get({ - path: '/some/path', - options: { - tags: ['access:securitySolution'], - }, - }); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: ['securitySolution'], - }, - }, - }); - `, - name: 'invalid: access tags are string literals, move to security.authz.requiredPrivileges', - }, - { - code: ` - router.get({ - path: '/some/path', - options: { - tags: ['access:ml:someTag', 'access:prefix:someTag'], - }, - }); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: ['ml:someTag', 'prefix:someTag'], - }, - }, - }); - `, - name: 'invalid: access tags have multiple prefixes, move to security.authz.requiredPrivileges', - }, - { - code: ` - router.get({ - path: '/some/path', - options: { - tags: [\`access:\${APP_ID}-entity-analytics\`], - }, - }); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: [\`\${APP_ID}-entity-analytics\`], - }, - }, - }); - `, - 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({ - path: '/some/path', - options: { - tags: ['access:securitySolution', routeTagHelper('someTag')], - }, - }); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: ['securitySolution'], - }, - },options: { - tags: [routeTagHelper('someTag')], - }, - }); - `, - name: 'invalid: access tags and tags made with helper function, only access tags are moved to security.authz.requiredPrivileges', - }, - { - code: ` - router.get({ - path: '/some/path', - options: { - tags: ['access:securitySolution', 'otherTag'], - }, - }); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: ['securitySolution'], - }, - },options: { - tags: ['otherTag'], - }, - }); - `, - name: 'invalid: both access tags and non access tags, move only access tags to security.authz.requiredPrivileges', - }, - { - code: ` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['access:securitySolution'], - }, - }) - .addVersion( - { - version: '1', - validate: false, - security: { - authz: { - requiredPrivileges: [ApiActionPermission.ManageSpaces], - }, - }, - }, - () => {} - ); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.versioned - .get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: ['securitySolution'], - }, - }, - }) - .addVersion( - { - version: '1', - validate: false, - security: { - authz: { - requiredPrivileges: [ApiActionPermission.ManageSpaces], - }, - }, - }, - () => {} - ); - `, - name: 'invalid: versioned route root access tags, move access tags to security.authz.requiredPrivileges', - }, - { - code: ` - router.get({ - path: '/some/path', - options: { - tags: ['access:securitySolution', \`access:\${APP_ID}-entity-analytics\`], - }, - }); - `, - errors: [{ message: "Move 'access' tags to security.authz.requiredPrivileges." }], - output: ` - router.get({ - path: '/some/path', - security: { - authz: { - requiredPrivileges: ['securitySolution', \`\${APP_ID}-entity-analytics\`], - }, - }, - }); - `, - name: 'invalid: string and template literal access tags, move both to security.authz.requiredPrivileges', - }, - { - code: dedent(` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - }) - .addVersion( - { - version: '1', - validate: false, - }, - () => {} - ); - `), - errors: [{ message: 'Security config is missing in addVersion call' }], - output: dedent(` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - }) - .addVersion( - { - version: '1', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, - validate: false, - }, - () => {} - ); - `), - name: 'invalid: security config is missing in addVersion call', - }, - { - code: dedent(` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - }) - .addVersion( - { - version: '1', - validate: false, - }, - () => {} - ) - .addVersion( - { - version: '2', - validate: false, - }, - () => {} - ); - `), - errors: [ - { message: 'Security config is missing in addVersion call' }, - { message: 'Security config is missing in addVersion call' }, - ], - output: dedent(` - router.versioned - .get({ - path: '/some/path', - options: { - tags: ['otherTag'], - }, - }) - .addVersion( - { - version: '1', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, - validate: false, - }, - () => {} - ) - .addVersion( - { - version: '2', - security: { - authz: { - enabled: false, - reason: 'This route is opted out from authorization', - }, - }, - validate: false, - }, - () => {} - ); - `), - name: 'invalid: security config is missing in multiple addVersion call', - }, - ], -}); diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts index 8e2a73acfda4..4f44c248dbbe 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.test.ts @@ -259,7 +259,7 @@ describe('Versioned route', () => { description: 'test', options: { authRequired: true, - tags: ['access:test'], + tags: ['oas:test'], timeout: { payload: 60_000, idleSocket: 10_000 }, xsrfRequired: false, excludeFromOAS: true, @@ -284,7 +284,7 @@ describe('Versioned route', () => { authRequired: true, excludeFromOAS: true, httpResource: true, - tags: ['access:test'], + tags: ['oas:test'], timeout: { idleSocket: 10_000, payload: 60_000 }, xsrfRequired: false, }, diff --git a/src/core/packages/http/server-internal/src/http_server.test.ts b/src/core/packages/http/server-internal/src/http_server.test.ts index 9abb61919a31..6085b3307665 100644 --- a/src/core/packages/http/server-internal/src/http_server.test.ts +++ b/src/core/packages/http/server-internal/src/http_server.test.ts @@ -1884,7 +1884,15 @@ describe('setup contract', () => { 1, expect.objectContaining({ options: { - app: { access: 'public' }, + app: { + access: 'public', + security: { + authz: { + enabled: false, + reason: 'Route serves static assets', + }, + }, + }, auth: false, cache: { privacy: 'public', diff --git a/src/core/packages/http/server-internal/src/http_server.ts b/src/core/packages/http/server-internal/src/http_server.ts index a7bfad83c3d6..c626aa0fbd57 100644 --- a/src/core/packages/http/server-internal/src/http_server.ts +++ b/src/core/packages/http/server-internal/src/http_server.ts @@ -737,7 +737,15 @@ export class HttpServer { }, }, options: { - app: { access: 'public' }, + app: { + access: 'public', + security: { + authz: { + enabled: false, + reason: 'Route serves static assets', + }, + }, + }, auth: false, cache: { privacy: 'public', diff --git a/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.test.ts b/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.test.ts index 93e8aa603ad9..1f5e307fdccb 100644 --- a/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.test.ts +++ b/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.test.ts @@ -32,7 +32,15 @@ describe('initAPIAuthorization', () => { const mockRequest = httpServerMock.createKibanaRequest({ method: 'get', path: '/foo/bar', - routeTags: ['access:foo'], + kibanaRouteOptions: { + xsrfRequired: true, + access: 'internal', + security: { + authz: { + requiredPrivileges: ['foo'], + }, + }, + }, }); const mockResponse = httpServerMock.createResponseFactory(); const mockPostAuthToolkit = httpServiceMock.createOnPostAuthToolkit(); @@ -61,6 +69,16 @@ describe('initAPIAuthorization', () => { method: 'get', path: '/foo/bar', routeTags: ['not-access:foo'], + kibanaRouteOptions: { + xsrfRequired: true, + access: 'internal', + security: { + authz: { + enabled: false, + reason: 'Used in test suite', + }, + }, + }, }); const mockResponse = httpServerMock.createResponseFactory(); const mockPostAuthToolkit = httpServiceMock.createOnPostAuthToolkit(); @@ -90,12 +108,25 @@ describe('initAPIAuthorization', () => { method: 'get', path: '/foo/bar', headers, - routeTags: ['access:foo'], + kibanaRouteOptions: { + xsrfRequired: true, + access: 'internal', + security: { + authz: { + requiredPrivileges: ['foo'], + }, + }, + }, }); const mockResponse = httpServerMock.createResponseFactory(); const mockPostAuthToolkit = httpServiceMock.createOnPostAuthToolkit(); - const mockCheckPrivileges = jest.fn().mockReturnValue({ hasAllRequested: true }); + const mockCheckPrivileges = jest.fn().mockReturnValue({ + privileges: { + kibana: [{ privilege: 'api:foo', authorized: true }], + }, + hasAllRequested: true, + }); mockAuthz.mode.useRbacForRequest.mockReturnValue(true); mockAuthz.checkPrivilegesDynamicallyWithRequest.mockImplementation((request) => { // hapi conceals the actual "request" from us, so we make sure that the headers are passed to @@ -105,10 +136,22 @@ describe('initAPIAuthorization', () => { return mockCheckPrivileges; }); + mockAuthz.checkPrivilegesWithRequest.mockImplementation((request) => { + expect(request.headers).toMatchObject(headers); + + return { + globally: () => ({ + privileges: { + kibana: [{ privilege: 'api:foo', authorized: true }], + }, + }), + }; + }); + await postAuthHandler(mockRequest, mockResponse, mockPostAuthToolkit); expect(mockResponse.notFound).not.toHaveBeenCalled(); - expect(mockPostAuthToolkit.next).toHaveBeenCalledTimes(1); + expect(mockPostAuthToolkit.authzResultNext).toHaveBeenCalledTimes(1); expect(mockCheckPrivileges).toHaveBeenCalledWith({ kibana: [mockAuthz.actions.api.get('foo')], }); @@ -131,12 +174,25 @@ describe('initAPIAuthorization', () => { method: 'get', path: '/foo/bar', headers, - routeTags: ['access:foo'], + kibanaRouteOptions: { + xsrfRequired: true, + access: 'internal', + security: { + authz: { + requiredPrivileges: ['foo'], + }, + }, + }, }); const mockResponse = httpServerMock.createResponseFactory(); const mockPostAuthToolkit = httpServiceMock.createOnPostAuthToolkit(); - const mockCheckPrivileges = jest.fn().mockReturnValue({ hasAllRequested: false }); + const mockCheckPrivileges = jest.fn().mockReturnValue({ + privileges: { + kibana: [{ privilege: 'api:foo', authorized: false }], + }, + hasAllRequested: false, + }); mockAuthz.mode.useRbacForRequest.mockReturnValue(true); mockAuthz.checkPrivilegesDynamicallyWithRequest.mockImplementation((request) => { // hapi conceals the actual "request" from us, so we make sure that the headers are passed to @@ -146,6 +202,18 @@ describe('initAPIAuthorization', () => { return mockCheckPrivileges; }); + mockAuthz.checkPrivilegesWithRequest.mockImplementation((request) => { + expect(request.headers).toMatchObject(headers); + + return { + globally: () => ({ + privileges: { + kibana: [{ privilege: 'api:foo', authorized: false }], + }, + }), + }; + }); + await postAuthHandler(mockRequest, mockResponse, mockPostAuthToolkit); expect(mockResponse.forbidden).toHaveBeenCalledTimes(1); diff --git a/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.ts b/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.ts index 70d5f10a6776..f996dd3b9943 100644 --- a/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.ts +++ b/x-pack/platform/plugins/shared/security/server/authorization/api_authorization.ts @@ -59,199 +59,171 @@ export function initAPIAuthorization( return toolkit.next(); } - const security = request.route.options.security; + const security = request.route.options.security!; - if (security) { - if (isAuthzDisabled(security.authz)) { - return toolkit.next(); - } + if (isAuthzDisabled(security.authz)) { + return toolkit.next(); + } - const authz = security.authz as AuthzEnabled; - const normalizeRequiredPrivileges = async ( - privileges: AuthzEnabled['requiredPrivileges'] - ) => { - const hasOperatorPrivileges = privileges.some( - (privilege) => - privilege === ReservedPrivilegesSet.operator || - (typeof privilege === 'object' && - privilege.allRequired?.includes(ReservedPrivilegesSet.operator)) - ); - - // nothing to normalize - if (!hasOperatorPrivileges) { - return privileges; - } - - const securityConfig = await getSecurityConfig(); - - // nothing to normalize - if (securityConfig.operator_privileges.enabled) { - return privileges; - } - - return privileges.reduce((acc, privilege) => { - if (typeof privilege === 'object') { - const operatorPrivilegeIndex = - privilege.allRequired?.findIndex((p) => p === ReservedPrivilegesSet.operator) ?? -1; - - acc.push( - operatorPrivilegeIndex !== -1 - ? { - ...privilege, - // @ts-ignore wrong types for `toSpliced` - allRequired: privilege.allRequired?.toSpliced(operatorPrivilegeIndex, 1), - } - : privilege - ); - } else if (privilege !== ReservedPrivilegesSet.operator) { - acc.push(privilege); - } - - return acc; - }, []); - }; - - // We need to normalize privileges to drop unintended privilege checks. - // Operator privileges check should be only performed if the `operator_privileges` are enabled in config. - const requiredPrivileges = await normalizeRequiredPrivileges(authz.requiredPrivileges); - - const { requestedPrivileges, requestedReservedPrivileges } = requiredPrivileges.reduce( - (acc, privilegeEntry) => { - const privileges = - typeof privilegeEntry === 'object' - ? [ - ...unwindNestedSecurityPrivileges( - privilegeEntry.allRequired ?? [] - ), - ...unwindNestedSecurityPrivileges( - privilegeEntry.anyRequired ?? [] - ), - ] - : [privilegeEntry]; - - for (const privilege of privileges) { - if (isReservedPrivilegeSet(privilege)) { - acc.requestedReservedPrivileges.push(privilege); - } else { - acc.requestedPrivileges.push(privilege); - } - } - - return acc; - }, - { - requestedPrivileges: [] as string[], - requestedReservedPrivileges: [] as string[], - } + const authz = security.authz as AuthzEnabled; + const normalizeRequiredPrivileges = async (privileges: AuthzEnabled['requiredPrivileges']) => { + const hasOperatorPrivileges = privileges.some( + (privilege) => + privilege === ReservedPrivilegesSet.operator || + (typeof privilege === 'object' && + privilege.allRequired?.includes(ReservedPrivilegesSet.operator)) ); - const checkPrivileges = checkPrivilegesDynamicallyWithRequest(request); - const privilegeToApiOperation = (privilege: string) => - privilege.replace(API_OPERATION_PREFIX, ''); - - const kibanaPrivileges: Record = {}; - - if (requestedPrivileges.length > 0) { - const checkPrivilegesResponse = await checkPrivileges({ - kibana: requestedPrivileges.map((permission) => actions.api.get(permission)), - }); - - for (const kbPrivilege of checkPrivilegesResponse.privileges.kibana) { - kibanaPrivileges[privilegeToApiOperation(kbPrivilege.privilege)] = kbPrivilege.authorized; - } + // nothing to normalize + if (!hasOperatorPrivileges) { + return privileges; } - for (const reservedPrivilege of requestedReservedPrivileges) { - if (reservedPrivilege === ReservedPrivilegesSet.superuser) { - const checkSuperuserPrivilegesResponse = await checkPrivilegesWithRequest( - request - ).globally(SUPERUSER_PRIVILEGES); - kibanaPrivileges[ReservedPrivilegesSet.superuser] = - checkSuperuserPrivilegesResponse.hasAllRequested; - } + const securityConfig = await getSecurityConfig(); - if (reservedPrivilege === ReservedPrivilegesSet.operator) { - const currentUser = getCurrentUser(request); - - kibanaPrivileges[ReservedPrivilegesSet.operator] = currentUser?.operator ?? false; - } + // nothing to normalize + if (securityConfig.operator_privileges.enabled) { + return privileges; } - const hasRequestedPrivilege = (kbPrivilege: Privilege | PrivilegeSet) => { - if (typeof kbPrivilege === 'object') { - const allRequired = kbPrivilege.allRequired ?? []; - const anyRequired = kbPrivilege.anyRequired ?? []; + return privileges.reduce((acc, privilege) => { + if (typeof privilege === 'object') { + const operatorPrivilegeIndex = + privilege.allRequired?.findIndex((p) => p === ReservedPrivilegesSet.operator) ?? -1; - return ( - allRequired.every((privilege) => + acc.push( + operatorPrivilegeIndex !== -1 + ? { + ...privilege, + // @ts-ignore wrong types for `toSpliced` + allRequired: privilege.allRequired?.toSpliced(operatorPrivilegeIndex, 1), + } + : privilege + ); + } else if (privilege !== ReservedPrivilegesSet.operator) { + acc.push(privilege); + } + + return acc; + }, []); + }; + + // We need to normalize privileges to drop unintended privilege checks. + // Operator privileges check should be only performed if the `operator_privileges` are enabled in config. + const requiredPrivileges = await normalizeRequiredPrivileges(authz.requiredPrivileges); + + const { requestedPrivileges, requestedReservedPrivileges } = requiredPrivileges.reduce( + (acc, privilegeEntry) => { + const privileges = + typeof privilegeEntry === 'object' + ? [ + ...unwindNestedSecurityPrivileges( + privilegeEntry.allRequired ?? [] + ), + ...unwindNestedSecurityPrivileges( + privilegeEntry.anyRequired ?? [] + ), + ] + : [privilegeEntry]; + + for (const privilege of privileges) { + if (isReservedPrivilegeSet(privilege)) { + acc.requestedReservedPrivileges.push(privilege); + } else { + acc.requestedPrivileges.push(privilege); + } + } + + return acc; + }, + { + requestedPrivileges: [] as string[], + requestedReservedPrivileges: [] as string[], + } + ); + + const checkPrivileges = checkPrivilegesDynamicallyWithRequest(request); + const privilegeToApiOperation = (privilege: string) => + privilege.replace(API_OPERATION_PREFIX, ''); + + const kibanaPrivileges: Record = {}; + + if (requestedPrivileges.length > 0) { + const checkPrivilegesResponse = await checkPrivileges({ + kibana: requestedPrivileges.map((permission) => actions.api.get(permission)), + }); + + for (const kbPrivilege of checkPrivilegesResponse.privileges.kibana) { + kibanaPrivileges[privilegeToApiOperation(kbPrivilege.privilege)] = kbPrivilege.authorized; + } + } + + for (const reservedPrivilege of requestedReservedPrivileges) { + if (reservedPrivilege === ReservedPrivilegesSet.superuser) { + const checkSuperuserPrivilegesResponse = await checkPrivilegesWithRequest(request).globally( + SUPERUSER_PRIVILEGES + ); + kibanaPrivileges[ReservedPrivilegesSet.superuser] = + checkSuperuserPrivilegesResponse.hasAllRequested; + } + + if (reservedPrivilege === ReservedPrivilegesSet.operator) { + const currentUser = getCurrentUser(request); + + kibanaPrivileges[ReservedPrivilegesSet.operator] = currentUser?.operator ?? false; + } + } + + const hasRequestedPrivilege = (kbPrivilege: Privilege | PrivilegeSet) => { + if (typeof kbPrivilege === 'object') { + const allRequired = kbPrivilege.allRequired ?? []; + const anyRequired = kbPrivilege.anyRequired ?? []; + + return ( + allRequired.every((privilege) => + typeof privilege === 'string' + ? kibanaPrivileges[privilege] + : // checking composite privileges + privilege.anyOf.some( + (anyPrivilegeEntry: Privilege) => kibanaPrivileges[anyPrivilegeEntry] + ) + ) && + (!anyRequired.length || + anyRequired.some((privilege) => typeof privilege === 'string' ? kibanaPrivileges[privilege] : // checking composite privileges - privilege.anyOf.some( - (anyPrivilegeEntry: Privilege) => kibanaPrivileges[anyPrivilegeEntry] + privilege.allOf.every( + (allPrivilegeEntry: Privilege) => kibanaPrivileges[allPrivilegeEntry] ) - ) && - (!anyRequired.length || - anyRequired.some((privilege) => - typeof privilege === 'string' - ? kibanaPrivileges[privilege] - : // checking composite privileges - privilege.allOf.every( - (allPrivilegeEntry: Privilege) => kibanaPrivileges[allPrivilegeEntry] - ) - )) - ); - } - - return kibanaPrivileges[kbPrivilege]; - }; - - for (const privilege of requiredPrivileges) { - if (!hasRequestedPrivilege(privilege)) { - const missingPrivileges = Object.keys(kibanaPrivileges).filter( - (key) => !kibanaPrivileges[key] - ); - const forbiddenMessage = `API [${request.route.method.toUpperCase()} ${ - request.url.pathname - }${ - request.url.search - }] is unauthorized for user, this action is granted by the Kibana privileges [${missingPrivileges}]`; - - logger.warn(forbiddenMessage); - - return response.forbidden({ - body: { - message: forbiddenMessage, - }, - }); - } + )) + ); } - return toolkit.authzResultNext(kibanaPrivileges); + return kibanaPrivileges[kbPrivilege]; + }; + + for (const privilege of requiredPrivileges) { + if (!hasRequestedPrivilege(privilege)) { + const missingPrivileges = Object.keys(kibanaPrivileges).filter( + (key) => !kibanaPrivileges[key] + ); + const forbiddenMessage = `API [${request.route.method.toUpperCase()} ${ + request.url.pathname + }${ + request.url.search + }] is unauthorized for user, this action is granted by the Kibana privileges [${missingPrivileges}]`; + + logger.warn(forbiddenMessage); + + return response.forbidden({ + body: { + message: forbiddenMessage, + }, + }); + } } - const tags = request.route.options.tags; - const tagPrefix = 'access:'; - const actionTags = tags.filter((tag) => tag.startsWith(tagPrefix)); - - // if there are no tags starting with "access:", just continue - if (actionTags.length === 0) { - return toolkit.next(); - } - - const apiActions = actionTags.map((tag) => actions.api.get(tag.substring(tagPrefix.length))); - const checkPrivileges = checkPrivilegesDynamicallyWithRequest(request); - const checkPrivilegesResponse = await checkPrivileges({ kibana: apiActions }); - - // we've actually authorized the request - if (checkPrivilegesResponse.hasAllRequested) { - logger.debug(`User authorized for "${request.url.pathname}${request.url.search}"`); - return toolkit.next(); - } - - logger.warn( - `User not authorized for "${request.url.pathname}${request.url.search}": responding with 403` - ); - return response.forbidden(); + return toolkit.authzResultNext(kibanaPrivileges); }); } diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts index 32efcb32fd70..68c328e76de6 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.test.ts @@ -283,44 +283,6 @@ describe('ProductFeaturesService', () => { .isActionRegistered as jest.Mock; }); - describe('when using access tag', () => { - const getReq = (tags: string[] = []) => - ({ - route: { options: { tags } }, - url: { pathname: '', search: '' }, - } as unknown as KibanaRequest); - - it('should authorize when no tag matches', async () => { - await lastRegisteredFn( - getReq(['access:something', 'access:securitySolution']), - res, - toolkit - ); - - expect(mockIsActionRegistered).not.toHaveBeenCalled(); - expect(res.notFound).not.toHaveBeenCalled(); - expect(toolkit.next).toHaveBeenCalledTimes(1); - }); - - it('should check when tag matches and return not found when not action registered', async () => { - mockIsActionRegistered.mockReturnValueOnce(false); - await lastRegisteredFn(getReq(['access:securitySolution-foo']), res, toolkit); - - expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-foo'); - expect(res.notFound).toHaveBeenCalledTimes(1); - expect(toolkit.next).not.toHaveBeenCalled(); - }); - - it('should check when tag matches and continue when action registered', async () => { - mockIsActionRegistered.mockReturnValueOnce(true); - await lastRegisteredFn(getReq(['access:securitySolution-foo']), res, toolkit); - - expect(mockIsActionRegistered).toHaveBeenCalledWith('api:securitySolution-foo'); - expect(res.notFound).not.toHaveBeenCalled(); - expect(toolkit.next).toHaveBeenCalledTimes(1); - }); - }); - describe('when using security authz', () => { beforeEach(() => { mockIsActionRegistered.mockImplementation((action: string) => action.includes('enabled')); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.ts index 819581273c64..ef81c27fe99e 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/product_features_service/product_features_service.ts @@ -27,7 +27,6 @@ import { import { API_ACTION_PREFIX } from '@kbn/security-solution-features/actions'; import type { RecursiveReadonly } from '@kbn/utility-types'; import type { ExperimentalFeatures } from '../../../common'; -import { APP_ID } from '../../../common'; import { ProductFeatures } from './product_features'; import { securityDefaultSavedObjects, @@ -249,9 +248,6 @@ export class ProductFeaturesService { // Should be used only by routes that do not need RBAC, only direct productFeature control. const APP_FEATURE_TAG_PREFIX = 'securitySolutionProductFeature:'; - /** @deprecated Use security.authz.requiredPrivileges instead */ - const API_ACTION_TAG_PREFIX = `access:${APP_ID}-`; - const isAuthzEnabled = (authz?: RecursiveReadonly): authz is AuthzEnabled => { return Boolean((authz as AuthzEnabled)?.requiredPrivileges); }; @@ -271,8 +267,6 @@ export class ProductFeaturesService { isEnabled = this.isEnabled( tag.substring(APP_FEATURE_TAG_PREFIX.length) as ProductFeatureKeyType ); - } else if (tag.startsWith(API_ACTION_TAG_PREFIX)) { - isEnabled = this.isApiPrivilegeEnabled(tag.substring(API_ACTION_TAG_PREFIX.length)); } if (!isEnabled) {