From dafc885563876dd65fdcd6af8012bf1b4a4864db Mon Sep 17 00:00:00 2001 From: Elena Shostak <165678770+elena-shostak@users.noreply.github.com> Date: Fri, 22 Nov 2024 17:13:08 +0100 Subject: [PATCH] [Authz] Removed security property from route.options types (#201352) ## Summary Removed `security` property from `route.options` types, `security` should exist only as a top level property. Fixed routes with incorrect config accordingly. ## Routes Impacted Routes with disabled authorization (impact can be considered negligible) ``` /internal/entities/managed/enablement /internal/entities/managed/enablement /internal/entities/managed/enablement /internal/entities/definition /internal/entities/definition/{id} /internal/entities/definition/{id?} /internal/entities/definition/{id}/_reset /internal/entities/definition/{id} /api/streams/_enable /api/streams/_resync /api/streams/{id}/_fork /api/streams/{id} /api/streams/{id} /api/streams/{id} /api/streams ``` Routes with authorization (will be backported to `8.17.0`) ``` /internal/product_doc_base/status /internal/product_doc_base/install /internal/product_doc_base/uninstall ``` __Fixes: https://github.com/elastic/kibana/issues/201347__ (cherry picked from commit ec7e1a808f9dc8012fb53a6a7bad0391afe265f2) --- .../src/router.test.ts | 17 ++++++++++++ .../src/router.ts | 5 ++++ .../core-http-server/src/router/request.ts | 6 +++-- .../http/core-http-server/src/router/route.ts | 7 ----- .../core-http-server/src/versioning/types.ts | 7 ++--- .../src/typings.ts | 3 ++- .../src/register_routes.ts | 8 ++++-- .../server/routes/installation.ts | 26 +++++++++---------- 8 files changed, 51 insertions(+), 28 deletions(-) diff --git a/packages/core/http/core-http-router-server-internal/src/router.test.ts b/packages/core/http/core-http-router-server-internal/src/router.test.ts index 2c702fb3ef702..f0eaa96879d42 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.test.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.test.ts @@ -305,6 +305,23 @@ describe('Router', () => { ); }); + it('throws if route has security declared wrong', () => { + const router = new Router('', logger, enhanceWithContext, routerOptions); + expect(() => + router.get( + // we use 'any' because validate requires valid Type or function usage + { + path: '/', + validate: false, + options: { security: { authz: { requiredPrivileges: [] } } } as any, + }, + (context, req, res) => res.ok({}) + ) + ).toThrowErrorMatchingInlineSnapshot( + `"\`options.security\` is not allowed in route config. Use \`security\` instead."` + ); + }); + it('throws if options.body.output is not a valid value', () => { const router = new Router('', logger, enhanceWithContext, routerOptions); expect(() => diff --git a/packages/core/http/core-http-router-server-internal/src/router.ts b/packages/core/http/core-http-router-server-internal/src/router.ts index f4d9b08888cc7..3b178a30529f7 100644 --- a/packages/core/http/core-http-router-server-internal/src/router.ts +++ b/packages/core/http/core-http-router-server-internal/src/router.ts @@ -115,6 +115,11 @@ function validOptions( ); } + // @ts-expect-error to eliminate problems with `security` in the options for route factories abstractions + if (options.security) { + throw new Error('`options.security` is not allowed in route config. Use `security` instead.'); + } + const body = shouldNotHavePayload ? undefined : { diff --git a/packages/core/http/core-http-server/src/router/request.ts b/packages/core/http/core-http-server/src/router/request.ts index 066372faca1e4..6fff29eb42b54 100644 --- a/packages/core/http/core-http-server/src/router/request.ts +++ b/packages/core/http/core-http-server/src/router/request.ts @@ -48,9 +48,11 @@ export interface KibanaRequestState extends RequestApplicationState { * Route options: If 'GET' or 'OPTIONS' method, body options won't be returned. * @public */ -export type KibanaRequestRouteOptions = Method extends 'get' | 'options' +export type KibanaRequestRouteOptions = (Method extends + | 'get' + | 'options' ? Required, 'body'>> - : Required>; + : Required>) & { security?: RouteSecurity }; /** * Request specific route information exposed to a handler. diff --git a/packages/core/http/core-http-server/src/router/route.ts b/packages/core/http/core-http-server/src/router/route.ts index da24adcb6802e..2efd405274113 100644 --- a/packages/core/http/core-http-server/src/router/route.ts +++ b/packages/core/http/core-http-server/src/router/route.ts @@ -398,13 +398,6 @@ export interface RouteConfigOptions { */ discontinued?: string; - /** - * Defines the security requirements for a route, including authorization and authentication. - * - * @remarks This will be surfaced in OAS documentation. - */ - security?: RouteSecurity; - /** * Whether this endpoint is being used to serve generated or static HTTP resources * like JS, CSS or HTML. _Do not set to `true` for HTTP APIs._ diff --git a/packages/core/http/core-http-server/src/versioning/types.ts b/packages/core/http/core-http-server/src/versioning/types.ts index 63e1e37754803..fa08810ce1fb7 100644 --- a/packages/core/http/core-http-server/src/versioning/types.ts +++ b/packages/core/http/core-http-server/src/versioning/types.ts @@ -19,6 +19,7 @@ import type { RequestHandlerContextBase, RouteValidationFunction, LazyValidator, + RouteSecurity, } from '../..'; import type { RouteDeprecationInfo } from '../router/route'; type RqCtx = RequestHandlerContextBase; @@ -35,12 +36,12 @@ export type VersionedRouteConfig = Omit< > & { options?: Omit< RouteConfigOptions, - 'access' | 'description' | 'summary' | 'deprecated' | 'discontinued' | 'security' + 'access' | 'description' | 'summary' | 'deprecated' | 'discontinued' >; /** See {@link RouteConfigOptions['access']} */ access: Exclude['access'], undefined>; /** See {@link RouteConfigOptions['security']} */ - security?: Exclude['security'], undefined>; + security?: RouteSecurity; /** * When enabled, the router will also check for the presence of an `apiVersion` * query parameter to determine the route version to resolve to: @@ -337,7 +338,7 @@ export interface AddVersionOpts { */ validate: false | VersionedRouteValidation | (() => VersionedRouteValidation); // Provide a way to lazily load validation schemas - security?: Exclude['security'], undefined>; + security?: RouteSecurity; options?: { deprecated?: RouteDeprecationInfo; diff --git a/packages/kbn-server-route-repository-utils/src/typings.ts b/packages/kbn-server-route-repository-utils/src/typings.ts index 35a2f41054c99..fc4fbe8ab6da6 100644 --- a/packages/kbn-server-route-repository-utils/src/typings.ts +++ b/packages/kbn-server-route-repository-utils/src/typings.ts @@ -15,6 +15,7 @@ import type { Logger, RequestHandlerContext, RouteConfigOptions, + RouteSecurity, RouteMethod, } from '@kbn/core/server'; import type { ServerSentEvent } from '@kbn/sse-utils'; @@ -277,5 +278,5 @@ export interface DefaultRouteHandlerResources extends CoreRouteHandlerResources } export interface DefaultRouteCreateOptions { - options?: RouteConfigOptions; + options?: RouteConfigOptions & { security?: RouteSecurity }; } diff --git a/packages/kbn-server-route-repository/src/register_routes.ts b/packages/kbn-server-route-repository/src/register_routes.ts index 5e0fa51a4544f..6e7b0d839b2f8 100644 --- a/packages/kbn-server-route-repository/src/register_routes.ts +++ b/packages/kbn-server-route-repository/src/register_routes.ts @@ -137,11 +137,14 @@ export function registerRoutes>({ validationObject = passThroughValidationObject; } + const { security, ...restOptions } = options ?? {}; + if (!version) { router[method]( { path: pathname, - options, + security, + options: restOptions, validate: validationObject, }, wrappedHandler @@ -150,7 +153,8 @@ export function registerRoutes>({ router.versioned[method]({ path: pathname, access: pathname.startsWith('/internal/') ? 'internal' : 'public', - options, + options: restOptions, + security, }).addVersion( { version, diff --git a/x-pack/plugins/ai_infra/product_doc_base/server/routes/installation.ts b/x-pack/plugins/ai_infra/product_doc_base/server/routes/installation.ts index dbede9f7d94d3..1e6b5545ebb4e 100644 --- a/x-pack/plugins/ai_infra/product_doc_base/server/routes/installation.ts +++ b/x-pack/plugins/ai_infra/product_doc_base/server/routes/installation.ts @@ -29,10 +29,10 @@ export const registerInstallationRoutes = ({ validate: false, options: { access: 'internal', - security: { - authz: { - requiredPrivileges: ['manage_llm_product_doc'], - }, + }, + security: { + authz: { + requiredPrivileges: ['manage_llm_product_doc'], }, }, }, @@ -56,13 +56,13 @@ export const registerInstallationRoutes = ({ validate: false, options: { access: 'internal', - security: { - authz: { - requiredPrivileges: ['manage_llm_product_doc'], - }, - }, timeout: { idleSocket: 20 * 60 * 1000 }, // install can take time. }, + security: { + authz: { + requiredPrivileges: ['manage_llm_product_doc'], + }, + }, }, async (ctx, req, res) => { const { documentationManager } = getServices(); @@ -90,10 +90,10 @@ export const registerInstallationRoutes = ({ validate: false, options: { access: 'internal', - security: { - authz: { - requiredPrivileges: ['manage_llm_product_doc'], - }, + }, + security: { + authz: { + requiredPrivileges: ['manage_llm_product_doc'], }, }, },