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

migrate xsrf / version-check / custom-headers handlers to NP #53684

Merged
Merged
Show file tree
Hide file tree
Changes from 6 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
6 changes: 6 additions & 0 deletions src/core/server/http/__snapshots__/http_config.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions src/core/server/http/cookie_session_storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ configService.atPath.mockReturnValue(
verificationMode: 'none',
},
compression: { enabled: true },
xsrf: {
disableProtection: true,
whitelist: [],
},
} as any)
);

Expand Down
23 changes: 23 additions & 0 deletions src/core/server/http/http_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ import { getEnvOptions } from '../config/__mocks__/env';
const validHostnames = ['www.example.com', '8.8.8.8', '::1', 'localhost'];
const invalidHostname = 'asdf$%^';

jest.mock('os', () => ({
...jest.requireActual('os'),
hostname: () => 'kibana-hostname',
}));

test('has defaults for config', () => {
const httpSchema = config.schema;
const obj = {};
Expand Down Expand Up @@ -86,6 +91,24 @@ test('accepts only valid uuids for server.uuid', () => {
);
});

test('uses os.hostname() as default for server.name', () => {
const httpSchema = config.schema;
const validated = httpSchema.validate({});
expect(validated.name).toEqual('kibana-hostname');
});

test('throws if xsrf.whitelist element does not start with a slash', () => {
const httpSchema = config.schema;
const obj = {
xsrf: {
whitelist: ['/valid-path', 'invalid-path'],
},
};
expect(() => httpSchema.validate(obj)).toThrowErrorMatchingInlineSnapshot(
`"[xsrf.whitelist.1]: must start with a slash"`
);
});

describe('with TLS', () => {
test('throws if TLS is enabled but `key` is not specified', () => {
const httpSchema = config.schema;
Expand Down
19 changes: 19 additions & 0 deletions src/core/server/http/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/

import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema';
import { hostname } from 'os';

import { Env } from '../config';
import { CspConfigType, CspConfig, ICspConfig } from '../csp';
import { SslConfig, sslSchema } from './ssl_config';
Expand All @@ -34,6 +36,7 @@ export const config = {
path: 'server',
schema: schema.object(
{
name: schema.string({ defaultValue: () => hostname() }),
autoListen: schema.boolean({ defaultValue: true }),
basePath: schema.maybe(
schema.string({
Expand Down Expand Up @@ -64,6 +67,9 @@ export const config = {
),
schema.boolean({ defaultValue: false })
),
customResponseHeaders: schema.recordOf(schema.string(), schema.string(), {
defaultValue: {},
}),
Comment on lines +60 to +62
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the same name. It's quite long but I don't really have a better idea. If someone comes with a better name, we could deprecates this one now that deprecations can be handled in NP configs

host: schema.string({
defaultValue: 'localhost',
hostname: true,
Expand Down Expand Up @@ -98,6 +104,13 @@ export const config = {
validate: match(uuidRegexp, 'must be a valid uuid'),
})
),
xsrf: schema.object({
disableProtection: schema.boolean({ defaultValue: false }),
whitelist: schema.arrayOf(
schema.string({ validate: match(/^\//, 'must start with a slash') }),
{ defaultValue: [] }
),
}),
},
{
validate: rawConfig => {
Expand Down Expand Up @@ -126,12 +139,14 @@ export const config = {
export type HttpConfigType = TypeOf<typeof config.schema>;

export class HttpConfig {
public name: string;
public autoListen: boolean;
public host: string;
public keepaliveTimeout: number;
public socketTimeout: number;
public port: number;
public cors: boolean | { origin: string[] };
public customResponseHeaders: Record<string, string>;
public maxPayload: ByteSizeValue;
public basePath?: string;
public rewriteBasePath: boolean;
Expand All @@ -140,15 +155,18 @@ export class HttpConfig {
public ssl: SslConfig;
public compression: { enabled: boolean; referrerWhitelist?: string[] };
public csp: ICspConfig;
public xsrf: { disableProtection: boolean; whitelist: string[] };

/**
* @internal
*/
constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType, env: Env) {
this.name = rawHttpConfig.name;
this.autoListen = rawHttpConfig.autoListen;
this.host = rawHttpConfig.host;
this.port = rawHttpConfig.port;
this.cors = rawHttpConfig.cors;
this.customResponseHeaders = rawHttpConfig.customResponseHeaders;
this.maxPayload = rawHttpConfig.maxPayload;
this.basePath = rawHttpConfig.basePath;
this.keepaliveTimeout = rawHttpConfig.keepaliveTimeout;
Expand All @@ -159,5 +177,6 @@ export class HttpConfig {
this.defaultRoute = rawHttpConfig.defaultRoute;
this.compression = rawHttpConfig.compression;
this.csp = new CspConfig(rawCspConfig);
this.xsrf = rawHttpConfig.xsrf;
}
}
13 changes: 13 additions & 0 deletions src/core/server/http/http_server.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import {
RouteMethod,
KibanaResponseFactory,
} from './router';
import { OnPreResponseToolkit } from './lifecycle/on_pre_response';
import { OnPostAuthToolkit } from './lifecycle/on_post_auth';
import { OnPreAuthToolkit } from './lifecycle/on_pre_auth';

interface RequestFixtureOptions {
headers?: Record<string, string>;
Expand Down Expand Up @@ -137,9 +140,19 @@ const createLifecycleResponseFactoryMock = (): jest.Mocked<LifecycleResponseFact
customError: jest.fn(),
});

type ToolkitMock = jest.Mocked<OnPreResponseToolkit & OnPostAuthToolkit & OnPreAuthToolkit>;

const createToolkitMock = (): ToolkitMock => {
return {
next: jest.fn(),
rewriteUrl: jest.fn(),
};
};

export const httpServerMock = {
createKibanaRequest: createKibanaRequestMock,
createRawRequest: createRawRequestMock,
createResponseFactory: createResponseFactoryMock,
createLifecycleResponseFactory: createLifecycleResponseFactoryMock,
createToolkit: createToolkitMock,
};
6 changes: 6 additions & 0 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export interface HttpServerSetup {
};
}

/** @internal */
export type LifecycleRegistrar = Pick<
HttpServerSetup,
'registerAuth' | 'registerOnPreAuth' | 'registerOnPostAuth' | 'registerOnPreResponse'
>;

export class HttpServer {
private server?: Server;
private config?: HttpConfig;
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/http/http_service.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,7 @@ jest.mock('./http_server', () => {
HttpServer: mockHttpServer,
};
});

jest.mock('./lifecycle_handlers', () => ({
registerCoreHandlers: jest.fn(),
}));
17 changes: 11 additions & 6 deletions src/core/server/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ import { Observable, Subscription, combineLatest } from 'rxjs';
import { first, map } from 'rxjs/operators';
import { Server } from 'hapi';

import { LoggerFactory } from '../logging';
import { CoreService } from '../../types';

import { Logger } from '../logging';
import { Logger, LoggerFactory } from '../logging';
import { ContextSetup } from '../context';
import { Env } from '../config';
import { CoreContext } from '../core_context';
import { PluginOpaqueId } from '../plugins';
import { CspConfigType, config as cspConfig } from '../csp';
Expand All @@ -43,6 +42,7 @@ import {
} from './types';

import { RequestHandlerContext } from '../../server';
import { registerCoreHandlers } from './lifecycle_handlers';

interface SetupDeps {
context: ContextSetup;
Expand All @@ -57,18 +57,20 @@ export class HttpService implements CoreService<InternalHttpServiceSetup, HttpSe

private readonly logger: LoggerFactory;
private readonly log: Logger;
private readonly env: Env;
private notReadyServer?: Server;
private requestHandlerContext?: RequestHandlerContextContainer;

constructor(private readonly coreContext: CoreContext) {
const { logger, configService, env } = coreContext;

this.logger = logger;
this.env = env;
this.log = logger.get('http');
this.config$ = combineLatest(
this.config$ = combineLatest([
configService.atPath<HttpConfigType>(httpConfig.path),
configService.atPath<CspConfigType>(cspConfig.path)
).pipe(map(([http, csp]) => new HttpConfig(http, csp, env)));
configService.atPath<CspConfigType>(cspConfig.path),
]).pipe(map(([http, csp]) => new HttpConfig(http, csp, env)));
this.httpServer = new HttpServer(logger, 'Kibana');
this.httpsRedirectServer = new HttpsRedirectServer(logger.get('http', 'redirect', 'server'));
}
Expand All @@ -92,6 +94,9 @@ export class HttpService implements CoreService<InternalHttpServiceSetup, HttpSe
}

const { registerRouter, ...serverContract } = await this.httpServer.setup(config);

registerCoreHandlers(serverContract, config, this.env);

Comment on lines +97 to +99
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not add a test that ensure this is properly invoked during the service setup phase as the feature is heavily integration-tested. Tell we if we want to still add a test.

const contract: InternalHttpServiceSetup = {
...serverContract,

Expand Down
Loading