Skip to content

Commit

Permalink
[8.x] [HTTP] Set explicit access for `public` HTTP APIs (#1…
Browse files Browse the repository at this point in the history
…92554) (#193735)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[HTTP] Set explicit access for `public` HTTP APIs
(#192554)](#192554)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jean-Louis
Leysens","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-23T14:53:31Z","message":"[HTTP]
Set explicit access for `public` HTTP APIs (#192554)\n\n##
Summary\r\n\r\nWe will be enforcing restricted access to internal HTTP
APIs [from\r\n9.0](#186781).
This PR is part 1\r\nof audit checking that our public APIs have their
access tag set\r\nexplicitly to ensure they are still available to end
users after we\r\nstart enforcing HTTP API restrictions. APIs reviewed
in this
PR\r\n([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):\r\n\r\n<img
width=\"260\" alt=\"Screenshot 2024-09-11 at 11 25
55\"\r\nsrc=\"https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23\">\r\n\r\n##
Note to reviewers\r\n\r\nThis audit is focussed on set `access:
'public'` where needed. Per the\r\nscreenshot our public-facing
documentation is taken as the source of\r\ntruth for which APIs should
be public. This may differ per offering so\r\nplease consider whether a
given HTTP API should be public on both\r\nserverless and stateful
offerings.\r\n\r\n## Risks\r\n\r\n* If we miss an API that should be
public, end users will encounter a\r\n`400` response when they try to
use the HTTP API on 9.0\r\n* If we set an API's access to \"public\" it
will not have the same\r\nrestrictions applied to
it.","sha":"3fa5bdf8732101812a656ec954e2a8d779838938","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:http","Team:Core","release_note:skip","v9.0.0","v8.16.0","backport:version"],"title":"[HTTP]
Set explicit access for `public` HTTP
APIs","number":192554,"url":"https://github.com/elastic/kibana/pull/192554","mergeCommit":{"message":"[HTTP]
Set explicit access for `public` HTTP APIs (#192554)\n\n##
Summary\r\n\r\nWe will be enforcing restricted access to internal HTTP
APIs [from\r\n9.0](#186781).
This PR is part 1\r\nof audit checking that our public APIs have their
access tag set\r\nexplicitly to ensure they are still available to end
users after we\r\nstart enforcing HTTP API restrictions. APIs reviewed
in this
PR\r\n([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):\r\n\r\n<img
width=\"260\" alt=\"Screenshot 2024-09-11 at 11 25
55\"\r\nsrc=\"https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23\">\r\n\r\n##
Note to reviewers\r\n\r\nThis audit is focussed on set `access:
'public'` where needed. Per the\r\nscreenshot our public-facing
documentation is taken as the source of\r\ntruth for which APIs should
be public. This may differ per offering so\r\nplease consider whether a
given HTTP API should be public on both\r\nserverless and stateful
offerings.\r\n\r\n## Risks\r\n\r\n* If we miss an API that should be
public, end users will encounter a\r\n`400` response when they try to
use the HTTP API on 9.0\r\n* If we set an API's access to \"public\" it
will not have the same\r\nrestrictions applied to
it.","sha":"3fa5bdf8732101812a656ec954e2a8d779838938"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192554","number":192554,"mergeCommit":{"message":"[HTTP]
Set explicit access for `public` HTTP APIs (#192554)\n\n##
Summary\r\n\r\nWe will be enforcing restricted access to internal HTTP
APIs [from\r\n9.0](#186781).
This PR is part 1\r\nof audit checking that our public APIs have their
access tag set\r\nexplicitly to ensure they are still available to end
users after we\r\nstart enforcing HTTP API restrictions. APIs reviewed
in this
PR\r\n([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):\r\n\r\n<img
width=\"260\" alt=\"Screenshot 2024-09-11 at 11 25
55\"\r\nsrc=\"https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23\">\r\n\r\n##
Note to reviewers\r\n\r\nThis audit is focussed on set `access:
'public'` where needed. Per the\r\nscreenshot our public-facing
documentation is taken as the source of\r\ntruth for which APIs should
be public. This may differ per offering so\r\nplease consider whether a
given HTTP API should be public on both\r\nserverless and stateful
offerings.\r\n\r\n## Risks\r\n\r\n* If we miss an API that should be
public, end users will encounter a\r\n`400` response when they try to
use the HTTP API on 9.0\r\n* If we set an API's access to \"public\" it
will not have the same\r\nrestrictions applied to
it.","sha":"3fa5bdf8732101812a656ec954e2a8d779838938"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Jean-Louis Leysens <[email protected]>
  • Loading branch information
kibanamachine and jloleysens authored Sep 23, 2024
1 parent 15751ec commit 3cab9c2
Show file tree
Hide file tree
Showing 21 changed files with 58 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,14 @@ export function registerRoutes({
maxImportPayloadBytes: config.maxImportPayloadBytes,
coreUsageData,
logger,
access: internalOnServerless,
});
registerLegacyExportRoute(legacyRouter, {
kibanaVersion,
coreUsageData,
logger,
access: internalOnServerless,
});
registerLegacyExportRoute(legacyRouter, { kibanaVersion, coreUsageData, logger });

const internalRouter = http.createRouter<InternalSavedObjectsRequestHandlerContext>(
'/internal/saved_objects/'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import moment from 'moment';
import { schema } from '@kbn/config-schema';
import type { Logger } from '@kbn/logging';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { RouteAccess } from '@kbn/core-http-server';
import type { InternalSavedObjectRouter } from '../../internal_types';
import { exportDashboards } from './lib';

Expand All @@ -20,7 +21,13 @@ export const registerLegacyExportRoute = (
kibanaVersion,
coreUsageData,
logger,
}: { kibanaVersion: string; coreUsageData: InternalCoreUsageDataSetup; logger: Logger }
access,
}: {
kibanaVersion: string;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}
) => {
router.get(
{
Expand All @@ -31,6 +38,7 @@ export const registerLegacyExportRoute = (
}),
},
options: {
access,
tags: ['api'],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { schema } from '@kbn/config-schema';
import type { Logger } from '@kbn/logging';
import type { SavedObject } from '@kbn/core-saved-objects-server';
import type { InternalCoreUsageDataSetup } from '@kbn/core-usage-data-base-server-internal';
import type { RouteAccess } from '@kbn/core-http-server';
import type { InternalSavedObjectRouter } from '../../internal_types';
import { importDashboards } from './lib';

Expand All @@ -20,7 +21,13 @@ export const registerLegacyImportRoute = (
maxImportPayloadBytes,
coreUsageData,
logger,
}: { maxImportPayloadBytes: number; coreUsageData: InternalCoreUsageDataSetup; logger: Logger }
access,
}: {
maxImportPayloadBytes: number;
coreUsageData: InternalCoreUsageDataSetup;
logger: Logger;
access: RouteAccess;
}
) => {
router.post(
{
Expand All @@ -38,6 +45,7 @@ export const registerLegacyImportRoute = (
}),
},
options: {
access,
tags: ['api'],
body: {
maxBytes: maxImportPayloadBytes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('POST /api/dashboards/export', () => {
kibanaVersion: 'mockversion',
coreUsageData,
logger: loggerMock.create(),
access: 'public',
});

handlerContext.savedObjects.client.bulkGet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('POST /api/dashboards/import', () => {
maxImportPayloadBytes: 26214400,
coreUsageData,
logger: loggerMock.create(),
access: 'public',
});

handlerContext.savedObjects.client.bulkCreate.mockResolvedValueOnce({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export function defineDeleteRolesRoutes({ router }: RouteDefinitionParams) {
{
path: '/api/security/role/{name}',
options: {
access: 'public',
summary: `Delete a role`,
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export function defineGetRolesRoutes({
{
path: '/api/security/role/{name}',
options: {
access: 'public',
summary: `Get a role`,
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export function defineGetAllRolesRoutes({
{
path: '/api/security/role',
options: {
access: 'public',
summary: `Get all roles`,
},
validate: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export function defineBulkCreateOrUpdateRolesRoutes({
{
path: '/api/security/roles',
options: {
access: 'public',
summary: 'Create or update roles',
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function definePutRolesRoutes({
{
path: '/api/security/role/{name}',
options: {
access: 'public',
summary: `Create or update a role`,
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Invalidate sessions routes', () => {

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({
access: 'public',
summary: 'Invalidate user sessions',
tags: ['access:sessionManagement'],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function defineInvalidateSessionsRoutes({ router, getSession }: RouteDefi
}),
},
options: {
access: 'public',
tags: ['access:sessionManagement'],
summary: `Invalidate user sessions`,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,21 @@ const areObjectsUnique = (objects: SavedObjectIdentifier[]) =>
_.uniqBy(objects, (o: SavedObjectIdentifier) => `${o.type}:${o.id}`).length === objects.length;

export function initCopyToSpacesApi(deps: ExternalRouteDeps) {
const { router, getSpacesService, usageStatsServicePromise, getStartServices, log } = deps;
const {
router,
getSpacesService,
usageStatsServicePromise,
getStartServices,
log,
isServerless,
} = deps;
const usageStatsClientPromise = usageStatsServicePromise.then(({ getClient }) => getClient());

router.post(
{
path: '/api/spaces/_copy_saved_objects',
options: {
access: isServerless ? 'internal' : 'public',
tags: ['access:copySavedObjectsToSpaces'],
description: `Copy saved objects to spaces`,
},
Expand Down Expand Up @@ -148,6 +156,7 @@ export function initCopyToSpacesApi(deps: ExternalRouteDeps) {
{
path: '/api/spaces/_resolve_copy_saved_objects_errors',
options: {
access: isServerless ? 'internal' : 'public',
tags: ['access:copySavedObjectsToSpaces'],
description: `Resolve conflicts copying saved objects`,
},
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/spaces/server/routes/api/external/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import { wrapError } from '../../../lib/errors';
import { createLicensedRouteHandler } from '../../lib';

export function initDeleteSpacesApi(deps: ExternalRouteDeps) {
const { router, log, getSpacesService } = deps;
const { router, log, getSpacesService, isServerless } = deps;

router.delete(
{
path: '/api/spaces/space/{id}',
options: {
access: isServerless ? 'internal' : 'public',
description: `Delete a space`,
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import { wrapError } from '../../../lib/errors';
import { createLicensedRouteHandler } from '../../lib';

export function initDisableLegacyUrlAliasesApi(deps: ExternalRouteDeps) {
const { router, getSpacesService, usageStatsServicePromise, log } = deps;
const { router, getSpacesService, usageStatsServicePromise, log, isServerless } = deps;
const usageStatsClientPromise = usageStatsServicePromise.then(({ getClient }) => getClient());

router.post(
{
path: '/api/spaces/_disable_legacy_url_aliases',
options: {
access: isServerless ? 'internal' : 'public',
description: `Disable legacy URL aliases`,
},
validate: {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/spaces/server/routes/api/external/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import { wrapError } from '../../../lib/errors';
import { createLicensedRouteHandler } from '../../lib';

export function initGetSpaceApi(deps: ExternalRouteDeps) {
const { router, getSpacesService } = deps;
const { router, getSpacesService, isServerless } = deps;

router.get(
{
path: '/api/spaces/space/{id}',
options: {
access: isServerless ? 'internal' : 'public',
description: `Get a space`,
},
validate: {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/spaces/server/routes/api/external/get_all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import { wrapError } from '../../../lib/errors';
import { createLicensedRouteHandler } from '../../lib';

export function initGetAllSpacesApi(deps: ExternalRouteDeps) {
const { router, log, getSpacesService } = deps;
const { router, log, getSpacesService, isServerless } = deps;

router.get(
{
path: '/api/spaces/space',
options: {
access: isServerless ? 'internal' : 'public',
description: `Get all spaces`,
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ import { wrapError } from '../../../lib/errors';
import { createLicensedRouteHandler } from '../../lib';

export function initGetShareableReferencesApi(deps: ExternalRouteDeps) {
const { router, getStartServices } = deps;
const { router, getStartServices, isServerless } = deps;

router.post(
{
path: '/api/spaces/_get_shareable_references',
options: {
access: isServerless ? 'internal' : 'public',
description: `Get shareable references`,
},
validate: {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/spaces/server/routes/api/external/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export function initPostSpacesApi(deps: ExternalRouteDeps) {
{
path: '/api/spaces/space',
options: {
access: isServerless ? 'internal' : 'public',
description: `Create a space`,
},
validate: {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/spaces/server/routes/api/external/put.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export function initPutSpacesApi(deps: ExternalRouteDeps) {
{
path: '/api/spaces/space/{id}',
options: {
access: isServerless ? 'internal' : 'public',
description: `Update a space`,
},
validate: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { SPACE_ID_REGEX } from '../../../lib/space_schema';
import { createLicensedRouteHandler } from '../../lib';

export function initUpdateObjectsSpacesApi(deps: ExternalRouteDeps) {
const { router, getStartServices } = deps;
const { router, getStartServices, isServerless } = deps;

const spacesSchema = schema.arrayOf(
schema.string({
Expand All @@ -37,6 +37,7 @@ export function initUpdateObjectsSpacesApi(deps: ExternalRouteDeps) {
{
path: '/api/spaces/_update_objects_spaces',
options: {
access: isServerless ? 'internal' : 'public',
description: `Update saved objects in spaces`,
},
validate: {
Expand Down

0 comments on commit 3cab9c2

Please sign in to comment.