Skip to content

Commit

Permalink
Sets explicit access for public platform security endpoints (elastic#…
Browse files Browse the repository at this point in the history
…195099)

Related issue: elastic#189833

## Summary

This PR explicitly sets the access level for platform security HTTP API
endpoints. This is to address restriction of internal endpoints in v9.
For details, see elastic#189833.

Additionally, this PR sets the `excludeFromOAS` option where applicable,
in order to refrain from generating documentation for endpoints which
are public but should either remain undocumented, or should be
documented as part of a specific topic (e.g. external authentication
flow).

Note: the invalidate sessions API has been changed to internal in
serverless

Endpoints excluded from OAS:
- GET /api/security/logout
- GET /api/security/v1/logout
- /api/security/oidc/implicit
- /api/security/v1/oidc/implicit
- /internal/security/oidc/implicit.js
- GET /api/security/oidc/callback
- GET /api/security/v1/oidc
- POST /api/security/oidc/initiate_login
- POST /api/security/v1/oidc
- GET /api/security/oidc/initiate_login
- POST /api/security/saml/callback
- /internal/security/reset_session_page.js
- /security/access_agreement
- /security/account
- /internal/security/capture-url
- /security/logged_out
- /login
- /logout
- /security/overwritten_session
- /spaces/space_selector
  • Loading branch information
jeramysoucy authored Oct 16, 2024
1 parent 3d28d17 commit 8d77cd4
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 8d77cd4

Please sign in to comment.