Skip to content

Commit

Permalink
[HTTP] Set explicit access for public HTTP APIs (elastic#192554)
Browse files Browse the repository at this point in the history
## Summary

We will be enforcing restricted access to internal HTTP APIs [from
9.0](elastic#186781). This PR is part 1
of audit checking that our public APIs have their access tag set
explicitly to ensure they are still available to end users after we
start enforcing HTTP API restrictions. APIs reviewed in this PR
([docs](https://www.elastic.co/guide/en/kibana/current/dashboard-import-api.html)):

<img width="260" alt="Screenshot 2024-09-11 at 11 25 55"
src="https://github.com/user-attachments/assets/499b1f1f-8e01-4463-9410-4500e438cd23">

## Note to reviewers

This audit is focussed on set `access: 'public'` where needed. Per the
screenshot our public-facing documentation is taken as the source of
truth for which APIs should be public. This may differ per offering so
please consider whether a given HTTP API should be public on both
serverless and stateful offerings.

## Risks

* If we miss an API that should be public, end users will encounter a
`400` response when they try to use the HTTP API on 9.0
* If we set an API's access to "public" it will not have the same
restrictions applied to it.

(cherry picked from commit 3fa5bdf)
  • Loading branch information
jloleysens committed Sep 23, 2024
1 parent a460623 commit e90578c
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 e90578c

Please sign in to comment.