Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Sets explicit access for public platform security endpoints (#195099) #196547

Merged
merged 1 commit into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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