Skip to content

Commit

Permalink
[8.x] [Authc] Security authentication config (elastic#205367) (elasti…
Browse files Browse the repository at this point in the history
…c#205625)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Authc] Security authentication config
(elastic#205367)](elastic#205367)

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

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

<!--BACKPORT [{"author":{"name":"Elena
Shostak","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-06T16:02:21Z","message":"[Authc]
Security authentication config (elastic#205367)\n\n## Summary\r\n\r\nWe cannot
support `security.authc` evolvement for versioned routes,\r\nsince
authentication is passed down to hapi during route registration\r\nand
it is tight up with the authentication strategy defined. Adjusted\r\nthe
code to pass `auth` option
correctly.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n__Fixes:
https://github.com/elastic/kibana/issues/205360__","sha":"26cc597b368d21df305fc3a3c84e0bb94e8e8881","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","Feature:Security/Authentication","release_note:skip","v9.0.0","backport:prev-minor"],"title":"[Authc]
Security authentication
config","number":205367,"url":"https://github.com/elastic/kibana/pull/205367","mergeCommit":{"message":"[Authc]
Security authentication config (elastic#205367)\n\n## Summary\r\n\r\nWe cannot
support `security.authc` evolvement for versioned routes,\r\nsince
authentication is passed down to hapi during route registration\r\nand
it is tight up with the authentication strategy defined. Adjusted\r\nthe
code to pass `auth` option
correctly.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n__Fixes:
https://github.com/elastic/kibana/issues/205360__","sha":"26cc597b368d21df305fc3a3c84e0bb94e8e8881"}},"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/205367","number":205367,"mergeCommit":{"message":"[Authc]
Security authentication config (elastic#205367)\n\n## Summary\r\n\r\nWe cannot
support `security.authc` evolvement for versioned routes,\r\nsince
authentication is passed down to hapi during route registration\r\nand
it is tight up with the authentication strategy defined. Adjusted\r\nthe
code to pass `auth` option
correctly.\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/e5cf28bc27b6ca80c92c44a4fc805adce857b518/packages/core/http/core-http-server-internal/src/http_server.ts#L378-L393\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n__Fixes:
https://github.com/elastic/kibana/issues/205360__","sha":"26cc597b368d21df305fc3a3c84e0bb94e8e8881"}}]}]
BACKPORT-->

Co-authored-by: Elena Shostak <[email protected]>
  • Loading branch information
kibanamachine and elena-shostak authored Jan 6, 2025
1 parent 8b42d98 commit b8ea6b4
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 52 deletions.
29 changes: 0 additions & 29 deletions src/core/packages/http/router-server-internal/src/request.test.ts
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
6 changes: 0 additions & 6 deletions src/core/packages/http/router-server-internal/src/request.ts
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

0 comments on commit b8ea6b4

Please sign in to comment.