Skip to content

Commit

Permalink
[8.x] Sets explicit access for public platform security endpoints (#1…
Browse files Browse the repository at this point in the history
…95099) (#196547)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Sets explicit access for public platform security endpoints
(#195099)](#195099)

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

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

<!--BACKPORT [{"author":{"name":"Jeramy
Soucy","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-16T13:11:25Z","message":"Sets
explicit access for public platform security endpoints
(#195099)\n\nRelated issue: #189833\r\n\r\n## Summary\r\n\r\nThis PR
explicitly sets the access level for platform security HTTP
API\r\nendpoints. This is to address restriction of internal endpoints
in v9.\r\nFor details, see
https://github.com/elastic/kibana/issues/189833.\r\n\r\nAdditionally,
this PR sets the `excludeFromOAS` option where applicable,\r\nin order
to refrain from generating documentation for endpoints which\r\nare
public but should either remain undocumented, or should be\r\ndocumented
as part of a specific topic (e.g. external
authentication\r\nflow).\r\n\r\nNote: the invalidate sessions API has
been changed to internal in\r\nserverless\r\n\r\nEndpoints excluded from
OAS:\r\n- GET /api/security/logout\r\n- GET /api/security/v1/logout\r\n-
/api/security/oidc/implicit\r\n- /api/security/v1/oidc/implicit\r\n-
/internal/security/oidc/implicit.js\r\n- GET
/api/security/oidc/callback\r\n- GET /api/security/v1/oidc\r\n- POST
/api/security/oidc/initiate_login\r\n- POST /api/security/v1/oidc\r\n-
GET /api/security/oidc/initiate_login\r\n- POST
/api/security/saml/callback\r\n-
/internal/security/reset_session_page.js\r\n-
/security/access_agreement\r\n- /security/account\r\n-
/internal/security/capture-url\r\n- /security/logged_out\r\n-
/login\r\n- /logout\r\n- /security/overwritten_session\r\n-
/spaces/space_selector","sha":"8d77cd49996281e746a0a7138c7624867c047053","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","v9.0.0","backport:prev-minor"],"title":"Sets
explicit access for public platform security
endpoints","number":195099,"url":"https://github.com/elastic/kibana/pull/195099","mergeCommit":{"message":"Sets
explicit access for public platform security endpoints
(#195099)\n\nRelated issue: #189833\r\n\r\n## Summary\r\n\r\nThis PR
explicitly sets the access level for platform security HTTP
API\r\nendpoints. This is to address restriction of internal endpoints
in v9.\r\nFor details, see
https://github.com/elastic/kibana/issues/189833.\r\n\r\nAdditionally,
this PR sets the `excludeFromOAS` option where applicable,\r\nin order
to refrain from generating documentation for endpoints which\r\nare
public but should either remain undocumented, or should be\r\ndocumented
as part of a specific topic (e.g. external
authentication\r\nflow).\r\n\r\nNote: the invalidate sessions API has
been changed to internal in\r\nserverless\r\n\r\nEndpoints excluded from
OAS:\r\n- GET /api/security/logout\r\n- GET /api/security/v1/logout\r\n-
/api/security/oidc/implicit\r\n- /api/security/v1/oidc/implicit\r\n-
/internal/security/oidc/implicit.js\r\n- GET
/api/security/oidc/callback\r\n- GET /api/security/v1/oidc\r\n- POST
/api/security/oidc/initiate_login\r\n- POST /api/security/v1/oidc\r\n-
GET /api/security/oidc/initiate_login\r\n- POST
/api/security/saml/callback\r\n-
/internal/security/reset_session_page.js\r\n-
/security/access_agreement\r\n- /security/account\r\n-
/internal/security/capture-url\r\n- /security/logged_out\r\n-
/login\r\n- /logout\r\n- /security/overwritten_session\r\n-
/spaces/space_selector","sha":"8d77cd49996281e746a0a7138c7624867c047053"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195099","number":195099,"mergeCommit":{"message":"Sets
explicit access for public platform security endpoints
(#195099)\n\nRelated issue: #189833\r\n\r\n## Summary\r\n\r\nThis PR
explicitly sets the access level for platform security HTTP
API\r\nendpoints. This is to address restriction of internal endpoints
in v9.\r\nFor details, see
https://github.com/elastic/kibana/issues/189833.\r\n\r\nAdditionally,
this PR sets the `excludeFromOAS` option where applicable,\r\nin order
to refrain from generating documentation for endpoints which\r\nare
public but should either remain undocumented, or should be\r\ndocumented
as part of a specific topic (e.g. external
authentication\r\nflow).\r\n\r\nNote: the invalidate sessions API has
been changed to internal in\r\nserverless\r\n\r\nEndpoints excluded from
OAS:\r\n- GET /api/security/logout\r\n- GET /api/security/v1/logout\r\n-
/api/security/oidc/implicit\r\n- /api/security/v1/oidc/implicit\r\n-
/internal/security/oidc/implicit.js\r\n- GET
/api/security/oidc/callback\r\n- GET /api/security/v1/oidc\r\n- POST
/api/security/oidc/initiate_login\r\n- POST /api/security/v1/oidc\r\n-
GET /api/security/oidc/initiate_login\r\n- POST
/api/security/saml/callback\r\n-
/internal/security/reset_session_page.js\r\n-
/security/access_agreement\r\n- /security/account\r\n-
/internal/security/capture-url\r\n- /security/logged_out\r\n-
/login\r\n- /logout\r\n- /security/overwritten_session\r\n-
/spaces/space_selector","sha":"8d77cd49996281e746a0a7138c7624867c047053"}}]}]
BACKPORT-->

Co-authored-by: Jeramy Soucy <[email protected]>
  • Loading branch information
kibanamachine and jeramysoucy authored Oct 16, 2024
1 parent b75aac6 commit 44eb0ca
Show file tree
Hide file tree
Showing 26 changed files with 112 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import { ConfigSchema } from '../config';
import { encryptionKeyRotationServiceMock } from '../crypto/index.mock';

export const routeDefinitionParamsMock = {
create: (config: Record<string, unknown> = {}) => ({
create: (config: Record<string, unknown> = {}, buildFlavor: BuildFlavor = 'traditional') => ({
router: httpServiceMock.createRouter(),
logger: loggingSystemMock.create().get(),
config: ConfigSchema.validate(config) as ConfigType,
encryptionKeyRotationService: encryptionKeyRotationServiceMock.create(),
buildFlavor: 'traditional' as BuildFlavor,
buildFlavor,
}),
};
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Key rotation routes', () => {

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({
access: 'public',
tags: ['access:rotateEncryptionKey', 'oas-tag:saved objects'],
summary: `Rotate a key for encrypted saved objects`,
description: `If a saved object cannot be decrypted using the primary encryption key, Kibana attempts to decrypt it using the specified decryption-only keys. In most of the cases this overhead is negligible, but if you're dealing with a large number of saved objects and experiencing performance issues, you may want to rotate the encryption key.
Expand Down Expand Up @@ -83,6 +84,25 @@ describe('Key rotation routes', () => {
);
});

it('defines route as internal when build flavor is serverless', () => {
const routeParamsMock = routeDefinitionParamsMock.create(
{ keyRotation: { decryptionOnlyKeys: ['b'.repeat(32)] } },
'serverless'
);
defineKeyRotationRoutes(routeParamsMock);
const [config] = routeParamsMock.router.post.mock.calls.find(
([{ path }]) => path === '/api/encrypted_saved_objects/_rotate_key'
)!;

expect(config.options).toEqual({
access: 'internal',
tags: ['access:rotateEncryptionKey', 'oas-tag:saved objects'],
summary: `Rotate a key for encrypted saved objects`,
description: `If a saved object cannot be decrypted using the primary encryption key, Kibana attempts to decrypt it using the specified decryption-only keys. In most of the cases this overhead is negligible, but if you're dealing with a large number of saved objects and experiencing performance issues, you may want to rotate the encryption key.
NOTE: Bulk key rotation can consume a considerable amount of resources and hence only user with a superuser role can trigger it.`,
});
});

it('returns 400 if decryption only keys are not specified.', async () => {
const routeParamsMock = routeDefinitionParamsMock.create();
defineKeyRotationRoutes(routeParamsMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function defineKeyRotationRoutes({
},
options: {
tags: ['access:rotateEncryptionKey', 'oas-tag:saved objects'],
access: buildFlavor === 'serverless' ? 'internal' : undefined,
access: buildFlavor === 'serverless' ? 'internal' : 'public',
summary: `Rotate a key for encrypted saved objects`,
description: `If a saved object cannot be decrypted using the primary encryption key, Kibana attempts to decrypt it using the specified decryption-only keys. In most of the cases this overhead is negligible, but if you're dealing with a large number of saved objects and experiencing performance issues, you may want to rotate the encryption key.
NOTE: Bulk key rotation can consume a considerable amount of resources and hence only user with a superuser role can trigger it.`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ describe('Common authentication routes', () => {
access: 'public',
authRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
excludeFromOAS: true,
});
expect(routeConfig.validate).toEqual({
body: undefined,
Expand Down Expand Up @@ -170,7 +171,7 @@ describe('Common authentication routes', () => {
});

it('correctly defines route.', async () => {
expect(routeConfig.options).toBeUndefined();
expect(routeConfig.options).toEqual({ access: 'internal' });
expect(routeConfig.validate).toBe(false);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export function defineCommonRoutes({
validate: { query: schema.object({}, { unknowns: 'allow' }) },
options: {
access: 'public',
excludeFromOAS: true,
authRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
},
Expand Down Expand Up @@ -89,10 +90,11 @@ export function defineCommonRoutes({
'/internal/security/me',
...(buildFlavor !== 'serverless' ? ['/api/security/v1/me'] : []),
]) {
const deprecated = path === '/api/security/v1/me';
router.get(
{ path, validate: false },
{ path, validate: false, options: { access: deprecated ? 'public' : 'internal' } },
createLicensedRouteHandler(async (context, request, response) => {
if (path === '/api/security/v1/me') {
if (deprecated) {
logger.warn(
`The "${basePath.serverBasePath}${path}" endpoint is deprecated and will be removed in the next major version.`,
{ tags: ['deprecation'] }
Expand Down
15 changes: 12 additions & 3 deletions x-pack/plugins/security/server/routes/authentication/oidc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function defineOIDCRoutes({
{
path,
validate: false,
options: { authRequired: false },
options: { authRequired: false, excludeFromOAS: true },
},
(context, request, response) => {
const serverBasePath = basePath.serverBasePath;
Expand Down Expand Up @@ -68,7 +68,7 @@ export function defineOIDCRoutes({
{
path: '/internal/security/oidc/implicit.js',
validate: false,
options: { authRequired: false },
options: { authRequired: false, excludeFromOAS: true },
},
(context, request, response) => {
const serverBasePath = basePath.serverBasePath;
Expand Down Expand Up @@ -106,7 +106,12 @@ export function defineOIDCRoutes({
{ unknowns: 'allow' }
),
},
options: { authRequired: false, tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW] },
options: {
access: 'public',
excludeFromOAS: true,
authRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
},
},
createLicensedRouteHandler(async (context, request, response) => {
const serverBasePath = basePath.serverBasePath;
Expand Down Expand Up @@ -184,6 +189,8 @@ export function defineOIDCRoutes({
),
},
options: {
access: 'public',
excludeFromOAS: true,
authRequired: false,
xsrfRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
Expand Down Expand Up @@ -227,6 +234,8 @@ export function defineOIDCRoutes({
),
},
options: {
access: 'public',
excludeFromOAS: true,
authRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('SAML authentication routes', () => {
expect(routeConfig.options).toEqual({
access: 'public',
authRequired: false,
excludeFromOAS: true,
xsrfRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export function defineSAMLRoutes({
},
options: {
access: 'public',
excludeFromOAS: true,
authRequired: false,
xsrfRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export function defineGetPrivilegesRoutes({ router, authz }: RouteDefinitionPara
),
}),
},
options: { access: 'public' },
},
createLicensedRouteHandler((context, request, response) => {
const respectLicenseLevel = request.query.respectLicenseLevel !== 'false'; // if undefined resolve to true by default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function resetSessionPageRoutes({ httpResources }: RouteDefinitionParams)
{
path: '/internal/security/reset_session_page.js',
validate: false,
options: { authRequired: false },
options: { authRequired: false, excludeFromOAS: true },
},
(context, request, response) => {
return response.renderJs({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,5 @@ import type { RouteDefinitionParams } from '..';
export function defineSessionManagementRoutes(params: RouteDefinitionParams) {
defineSessionInfoRoutes(params);
defineSessionExtendRoutes(params);

// The invalidate session API was introduced to address situations where the session index
// could grow rapidly - when session timeouts are disabled, or with anonymous access.
// In the serverless environment, sessions timeouts are always be enabled, and there is no
// anonymous access. This eliminates the need for an invalidate session HTTP API.
if (params.buildFlavor !== 'serverless') {
defineInvalidateSessionsRoutes(params);
}
defineInvalidateSessionsRoutes(params);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import type { RouteDefinitionParams } from '..';
/**
* Defines routes required for session invalidation.
*/
export function defineInvalidateSessionsRoutes({ router, getSession }: RouteDefinitionParams) {
export function defineInvalidateSessionsRoutes({
router,
getSession,
buildFlavor,
}: RouteDefinitionParams) {
router.post(
{
path: '/api/security/session/_invalidate',
Expand All @@ -34,7 +38,12 @@ export function defineInvalidateSessionsRoutes({ router, getSession }: RouteDefi
}),
},
options: {
access: 'public',
// The invalidate session API was introduced to address situations where the session index
// could grow rapidly - when session timeouts are disabled, or with anonymous access.
// In the serverless environment, sessions timeouts are always be enabled, and there is no
// anonymous access. However, keeping this endpoint available internally in serverless would
// be useful in situations where we need to batch-invalidate user sessions.
access: buildFlavor === 'serverless' ? 'internal' : 'public',
tags: ['access:sessionManagement'],
summary: `Invalidate user sessions`,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Access agreement view routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toBeUndefined();
expect(routeConfig.options).toEqual({ excludeFromOAS: true });
expect(routeConfig.validate).toBe(false);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function defineAccessAgreementRoutes({
const canHandleRequest = () => license.getFeatures().allowAccessAgreement;

httpResources.register(
{ path: '/security/access_agreement', validate: false },
{ path: '/security/access_agreement', validate: false, options: { excludeFromOAS: true } },
createLicensedRouteHandler(async (context, request, response) =>
canHandleRequest()
? response.renderCoreApp()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import type { RouteDefinitionParams } from '..';
* Defines routes required for the Account Management view.
*/
export function defineAccountManagementRoutes({ httpResources }: RouteDefinitionParams) {
httpResources.register({ path: '/security/account', validate: false }, (context, req, res) =>
res.renderCoreApp()
httpResources.register(
{ path: '/security/account', validate: false, options: { excludeFromOAS: true } },
(context, req, res) => res.renderCoreApp()
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('Capture URL view routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({ authRequired: false });
expect(routeConfig.options).toEqual({ authRequired: false, excludeFromOAS: true });

expect(routeConfig.validate).toEqual({
body: undefined,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/routes/views/capture_url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function defineCaptureURLRoutes({ httpResources }: RouteDefinitionParams)
validate: {
query: schema.object({ next: schema.maybe(schema.string()) }, { unknowns: 'ignore' }),
},
options: { authRequired: false },
options: { authRequired: false, excludeFromOAS: true },
},
(context, request, response) => response.renderAnonymousCoreApp()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('LoggedOut view routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({ authRequired: false });
expect(routeConfig.options).toEqual({ authRequired: false, excludeFromOAS: true });
expect(routeConfig.validate).toBe(false);
});

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/routes/views/logged_out.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function defineLoggedOutRoutes({
{
path: '/security/logged_out',
validate: false,
options: { authRequired: false },
options: { authRequired: false, excludeFromOAS: true },
},
async (context, request, response) => {
// Authentication flow isn't triggered automatically for this route, so we should explicitly
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/routes/views/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('Login view routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({ authRequired: 'optional' });
expect(routeConfig.options).toEqual({ authRequired: 'optional', excludeFromOAS: true });

expect(routeConfig.validate).toEqual({
body: undefined,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/routes/views/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function defineLoginRoutes({
{ unknowns: 'allow' }
),
},
options: { authRequired: 'optional' },
options: { authRequired: 'optional', excludeFromOAS: true },
},
async (context, request, response) => {
// Default to true if license isn't available or it can't be resolved for some reason.
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security/server/routes/views/logout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { RouteDefinitionParams } from '..';
*/
export function defineLogoutRoutes({ httpResources }: RouteDefinitionParams) {
httpResources.register(
{ path: '/logout', validate: false, options: { authRequired: false } },
{ path: '/logout', validate: false, options: { authRequired: false, excludeFromOAS: true } },
(context, request, response) => response.renderAnonymousCoreApp()
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { RouteDefinitionParams } from '..';
*/
export function defineOverwrittenSessionRoutes({ httpResources }: RouteDefinitionParams) {
httpResources.register(
{ path: '/security/overwritten_session', validate: false },
{ path: '/security/overwritten_session', validate: false, options: { excludeFromOAS: true } },
(context, req, res) => res.renderCoreApp()
);
}
2 changes: 1 addition & 1 deletion x-pack/plugins/spaces/server/routes/views/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Space Selector view routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toBeUndefined();
expect(routeConfig.options).toEqual({ excludeFromOAS: true });
expect(routeConfig.validate).toBe(false);
});

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/spaces/server/routes/views/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export interface ViewRouteDeps {

export function initSpacesViewsRoutes(deps: ViewRouteDeps) {
deps.httpResources.register(
{ path: '/spaces/space_selector', validate: false },
{ path: '/spaces/space_selector', validate: false, options: { excludeFromOAS: true } },
(context, request, response) => response.renderCoreApp()
);

Expand All @@ -32,6 +32,7 @@ export function initSpacesViewsRoutes(deps: ViewRouteDeps) {
schema.object({ next: schema.maybe(schema.string()) }, { unknowns: 'ignore' })
),
},
options: { excludeFromOAS: true },
},
async (context, request, response) => {
try {
Expand Down
Loading

0 comments on commit 44eb0ca

Please sign in to comment.