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] [Authc] Security authentication config (#205367) #205625

Merged
merged 1 commit into from
Jan 6, 2025
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 @@ -397,35 +397,6 @@ describe('CoreKibanaRequest', () => {

expect(kibanaRequest.route.options.authRequired).toBe(true);
});
it('handles required authc: { enabled: false }', () => {
const request = hapiMocks.createRequest({
route: {
settings: {
app: {
security: { authc: { enabled: false } },
},
},
},
});
const kibanaRequest = CoreKibanaRequest.from(request);

expect(kibanaRequest.route.options.authRequired).toBe(false);
});

it(`handles required authc: { enabled: 'optional' }`, () => {
const request = hapiMocks.createRequest({
route: {
settings: {
app: {
security: { authc: { enabled: 'optional' } },
},
},
},
});
const kibanaRequest = CoreKibanaRequest.from(request);

expect(kibanaRequest.route.options.authRequired).toBe('optional');
});

it('handles required authz simple config', () => {
const security: RouteSecurity = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,6 @@ export class CoreKibanaRequest<
return true;
}

const security = this.getSecurity(request);

if (security?.authc !== undefined) {
return security.authc?.enabled ?? true;
}

const authOptions = request.route.settings.auth;
if (typeof authOptions === 'object') {
// 'try' is used in the legacy platform
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -542,22 +542,19 @@ describe('Versioned route', () => {
authz: {
requiredPrivileges: ['foo', 'bar', 'baz'],
},
authc: undefined,
};
const securityConfig1: RouteSecurity = {
authz: {
requiredPrivileges: ['foo'],
},
authc: {
enabled: 'optional',
},
authc: undefined,
};
const securityConfig2: RouteSecurity = {
authz: {
requiredPrivileges: ['foo', 'bar'],
},
authc: {
enabled: true,
},
authc: undefined,
};
const versionedRoute = versionedRouter
.get({ path: '/test/{id}', access: 'internal', security: securityConfigDefault })
Expand Down Expand Up @@ -669,4 +666,42 @@ describe('Versioned route', () => {
- [authz.requiredPrivileges.0.1]: expected value of type [string] but got [Object]"
`);
});

it('should correctly merge security configuration for versions', () => {
const versionedRouter = CoreVersionedRouter.from({ router });
const validSecurityConfig: RouteSecurity = {
authz: {
requiredPrivileges: ['foo'],
},
authc: {
enabled: 'optional',
},
};

const route = versionedRouter.get({
path: '/test/{id}',
access: 'internal',
security: validSecurityConfig,
});

route.addVersion(
{
version: '1',
validate: false,
security: {
authz: {
requiredPrivileges: ['foo', 'bar'],
},
},
},
handlerFn
);

// @ts-expect-error for test purpose
const security = route.getSecurity({ headers: { [ELASTIC_HTTP_VERSION_HEADER]: '1' } });

expect(security.authc).toEqual({ enabled: 'optional' });

expect(security.authz).toEqual({ requiredPrivileges: ['foo', 'bar'] });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,18 @@ export class CoreVersionedRoute implements VersionedRoute {
return [...this.handlers.values()];
}

public getSecurity: RouteSecurityGetter = (req: RequestLike) => {
public getSecurity: RouteSecurityGetter = (req?: RequestLike) => {
if (!req) {
return this.defaultSecurityConfig;
}

const version = this.getVersion(req)!;
const security = this.handlers.get(version)?.options.security ?? this.defaultSecurityConfig;

return this.handlers.get(version)?.options.security ?? this.defaultSecurityConfig;
// authc can be defined only on the top route level,
// so we need to merge it with the versioned one which can have different authz per version
return security
? { authz: security.authz, authc: this.defaultSecurityConfig?.authc }
: undefined;
};
}
69 changes: 69 additions & 0 deletions src/core/packages/http/server-internal/src/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1777,3 +1777,72 @@ describe('configuration change', () => {
);
});
});

test('exposes authentication details of incoming request to a route handler', async () => {
const { registerRouter, registerAuth, server: innerServer } = await server.setup({ config$ });

const router = new Router('', logger, enhanceWithContext, routerOptions);
router.get(
{
path: '/',
validate: false,
security: {
authc: { enabled: false, reason: 'test' },
authz: { enabled: false, reason: 'test' },
},
},
(context, req, res) => res.ok({ body: req.route })
);
router.get(
{
path: '/foo',
validate: false,
security: {
authc: { enabled: 'optional' },
authz: { enabled: false, reason: 'test' },
},
},
(context, req, res) => res.ok({ body: req.route })
);
// mocking to have `authRegistered` filed set to true
registerAuth((req, res) => res.unauthorized());
registerRouter(router);

await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200, {
method: 'get',
path: '/',
routePath: '/',
options: {
authRequired: false,
xsrfRequired: false,
access: 'internal',
tags: [],
timeout: {},
security: {
authc: { enabled: false, reason: 'test' },
authz: { enabled: false, reason: 'test' },
},
},
});
await supertest(innerServer.listener)
.get('/foo')
.expect(200, {
method: 'get',
path: '/foo',
routePath: '/foo',
options: {
authRequired: 'optional',
xsrfRequired: false,
access: 'internal',
tags: [],
timeout: {},
security: {
authc: { enabled: 'optional' },
authz: { enabled: false, reason: 'test' },
},
},
});
});
11 changes: 10 additions & 1 deletion src/core/packages/http/server-internal/src/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -738,14 +738,23 @@ export class HttpServer {
});
}

private getSecurity(route: RouterRoute) {
const securityConfig = route?.security;

// for versioned routes, we need to check if the security config is a function
return typeof securityConfig === 'function' ? securityConfig() : securityConfig;
}

private configureRoute(route: RouterRoute) {
const optionsLogger = this.log.get('options');
this.log.debug(`registering route handler for [${route.path}]`);
// Hapi does not allow payload validation to be specified for 'head' or 'get' requests
const validate = isSafeMethod(route.method) ? undefined : { payload: true };
const { authRequired, tags, body = {}, timeout, deprecated } = route.options;
const { tags, body = {}, timeout, deprecated } = route.options;
const { accepts: allow, override, maxBytes, output, parse } = body;

const authRequired = this.getSecurity(route)?.authc?.enabled ?? route.options.authRequired;

const kibanaRouteOptions: KibanaRouteOptions = {
xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method),
access: route.options.access ?? 'internal',
Expand Down
2 changes: 1 addition & 1 deletion src/core/packages/http/server/src/router/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { IKibanaSocket } from './socket';
import type { RouteMethod, RouteConfigOptions, RouteSecurity, RouteDeprecationInfo } from './route';
import type { Headers } from './headers';

export type RouteSecurityGetter = (request: {
export type RouteSecurityGetter = (request?: {
headers: KibanaRequest['headers'];
query?: KibanaRequest['query'];
}) => RouteSecurity | undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/core/packages/http/server/src/versioning/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ export interface AddVersionOpts<P, Q, B> {
*/
validate: false | VersionedRouteValidation<P, Q, B> | (() => VersionedRouteValidation<P, Q, B>); // Provide a way to lazily load validation schemas

security?: RouteSecurity;
security?: Pick<RouteSecurity, 'authz'>;

options?: {
deprecated?: RouteDeprecationInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
*/

import { Type } from '@kbn/config-schema';
import type { RequestHandler, RouteConfig } from '@kbn/core/server';
import { kibanaResponseFactory } from '@kbn/core/server';
import type { AuthzDisabled, RequestHandler, RouteConfig } from '@kbn/core/server';
import { coreMock, httpServerMock } from '@kbn/core/server/mocks';
import type { DeeplyMockedKeys } from '@kbn/utility-types-jest';

Expand Down Expand Up @@ -65,9 +65,11 @@ describe('Common authentication routes', () => {
});

it('correctly defines route.', async () => {
expect(routeConfig.security?.authc?.enabled).toEqual(false);
expect((routeConfig.security?.authz as AuthzDisabled).enabled).toEqual(false);

expect(routeConfig.options).toEqual({
access: 'public',
authRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
excludeFromOAS: true,
});
Expand Down Expand Up @@ -201,7 +203,9 @@ describe('Common authentication routes', () => {
});

it('correctly defines route.', () => {
expect(routeConfig.options).toEqual({ authRequired: false });
expect(routeConfig.security?.authc?.enabled).toEqual(false);
expect((routeConfig.security?.authz as AuthzDisabled).enabled).toEqual(false);

expect(routeConfig.validate).toEqual({
body: expect.any(Type),
query: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,18 @@ export function defineCommonRoutes({
enabled: false,
reason: 'This route must remain accessible to 3rd-party IdPs',
},
authc: {
enabled: false,
reason:
'This route is used for authentication - it does not require existing authentication',
},
},
// Allow unknown query parameters as this endpoint can be hit by the 3rd-party with any
// set of query string parameters (e.g. SAML/OIDC logout request/response parameters).
validate: { query: schema.object({}, { unknowns: 'allow' }) },
options: {
access: 'public',
excludeFromOAS: true,
authRequired: false,
tags: [ROUTE_TAG_CAN_REDIRECT, ROUTE_TAG_AUTH_FLOW],
...(isDeprecated && {
deprecated: {
Expand Down Expand Up @@ -191,7 +195,12 @@ export function defineCommonRoutes({
security: {
authz: {
enabled: false,
reason: `This route provides basic and token login capbility, which is delegated to the internal authentication service`,
reason: `This route provides basic and token login capability, which is delegated to the internal authentication service`,
},
authc: {
enabled: false,
reason:
'This route is used for authentication - it does not require existing authentication',
},
},
validate: {
Expand All @@ -210,7 +219,6 @@ export function defineCommonRoutes({
),
}),
},
options: { authRequired: false },
},
createLicensedRouteHandler(async (context, request, response) => {
const { providerType, providerName, currentURL, params } = request.body;
Expand Down
Loading