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

Globally enforce internal API restriction #193792

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

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

Original file line number Diff line number Diff line change
Expand Up @@ -539,13 +539,13 @@ describe('restrictInternalApis', () => {
restrictInternalApis: true,
});
});
it('defaults to false', () => {
it('defaults to true', () => {
expect(
config.schema.validate({ restrictInternalApis: undefined }, { serverless: true })
).toMatchObject({ restrictInternalApis: false });
).toMatchObject({ restrictInternalApis: true });
expect(
config.schema.validate({ restrictInternalApis: undefined }, { traditional: true })
).toMatchObject({ restrictInternalApis: false });
).toMatchObject({ restrictInternalApis: true });
});
});

Expand Down Expand Up @@ -677,7 +677,7 @@ describe('HttpConfig', () => {
});
});

it('defaults restrictInternalApis to false', () => {
it('defaults restrictInternalApis to true', () => {
const rawConfig = config.schema.validate({}, {});
const rawCspConfig = cspConfig.schema.validate({});
const rawPermissionsPolicyConfig = permissionsPolicyConfig.schema.validate({});
Expand All @@ -687,6 +687,6 @@ describe('HttpConfig', () => {
ExternalUrlConfig.DEFAULT,
rawPermissionsPolicyConfig
);
expect(httpConfig.restrictInternalApis).toBe(false);
expect(httpConfig.restrictInternalApis).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,8 @@ const configSchema = schema.object(
},
}
),
// allow access to internal routes by default to prevent breaking changes in current offerings
restrictInternalApis: offeringBasedSchema({
serverless: schema.boolean({ defaultValue: false }),
traditional: schema.boolean({ defaultValue: false }),
}),
// disable access to internal routes by default
restrictInternalApis: schema.boolean({ defaultValue: true }),

versioned: schema.object({
/**
Expand Down Expand Up @@ -385,8 +382,8 @@ export class HttpConfig implements IHttpConfig {
this.requestId = rawHttpConfig.requestId;
this.shutdownTimeout = rawHttpConfig.shutdownTimeout;

// default to `false` to prevent breaking changes in current offerings
this.restrictInternalApis = rawHttpConfig.restrictInternalApis ?? false;
// defaults to `true` if not set through config.
this.restrictInternalApis = rawHttpConfig.restrictInternalApis;
this.eluMonitor = rawHttpConfig.eluMonitor;
this.versioned = rawHttpConfig.versioned;
this.oas = rawHttpConfig.oas;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const createConfigService = ({
shutdownTimeout: moment.duration(30, 'seconds'),
keepaliveTimeout: 120_000,
socketTimeout: 120_000,
restrictInternalApis: false,
restrictInternalApis: false, // disable restriction for Kibana tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leaves the restriction disabled for tests consuming createConfigService.

versioned: {
versionResolution: 'oldest',
strictClientVersionCheck: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const DEFAULTS_SETTINGS = {
// port and aren't affected by the timing issues in test environment.
port: 0,
xsrf: { disableProtection: true },
restrictInternalApis: true,
},
logging: {
root: {
Expand Down Expand Up @@ -149,6 +150,7 @@ export function createRootWithCorePlugins(
console: { type: 'console', layout: { type: 'pattern' } },
},
},
server: { restrictInternalApis: true },
// createRootWithSettings sets default value to "true", so undefined should be threatened as "true".
...(cliArgs.oss === false
? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const getServerlessESClient = ({ port }: { port: number }) => {
const getServerlessDefault = () => {
return {
server: {
restrictInternalApis: true,
restrictInternalApis: true, // has no effect, defaults to true
versioned: {
versionResolution: 'newest',
strictClientVersionCheck: false,
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-config/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface CliArgs {
cache: boolean;
dist: boolean;
serverless?: boolean;
retrictInternalApis?: boolean;
}

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('server config', () => {
},
"payloadTimeout": 20000,
"port": 3000,
"restrictInternalApis": false,
"restrictInternalApis": true,
"shutdownTimeout": "PT30S",
"socketTimeout": 120000,
"ssl": Object {
Expand Down Expand Up @@ -193,10 +193,10 @@ describe('server config', () => {
});

describe('restrictInternalApis', () => {
test('is false by default', () => {
test('is true by default', () => {
const configSchema = config.schema;
const obj = {};
expect(new ServerConfig(configSchema.validate(obj)).restrictInternalApis).toBe(false);
expect(new ServerConfig(configSchema.validate(obj)).restrictInternalApis).toBe(true);
});

test('can specify retriction on access to internal APIs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const configSchema = schema.object(
defaultValue: 120000,
}),
ssl: sslSchema,
restrictInternalApis: schema.boolean({ defaultValue: false }),
restrictInternalApis: schema.boolean({ defaultValue: true }),
},
{
validate: (rawConfig) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-server-http-tools/src/get_listener.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
restrictInternalApis: true,
});

describe('getServerListener', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
restrictInternalApis: true,
});

describe('getServerOptions', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
restrictInternalApis: true,
});

describe('getServerTLSOptions', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('PUT /internal/core/_settings', () => {
logging: {
loggers: [{ name: loggerName, level: 'error', appenders: ['console'] }],
},
server: { restrictInternalApis: false },
};
const { startES, startKibana } = createTestServers({
adjustTimeout: (t: number) => jest.setTimeout(t),
Expand Down Expand Up @@ -82,6 +83,9 @@ describe('checking all opted-in dynamic config settings', () => {
logging: {
loggers: [{ name: 'root', level: 'info', appenders: ['console'] }],
},
server: {
restrictInternalApis: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's less work to disable the restriction in createTestServers than adding the header/query param to tests.
We disable the restriction by default for ftr tests too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should/do we have a task for updating these requests in the future or is the plan to leave them unrestricted in tests?

Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Sep 30, 2024

Choose a reason for hiding this comment

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

Should/do we have a task for updating these requests in the future or is the plan to leave them unrestricted in tests

The plan is to update requests if and when we need to, which is consistent with the base config for our FTR tests.

As for this specific test, createRootWithSettings explicitly enables the restriction. Here, we change that in setup, and is an example of how consumers can override the default should they choose to.

},
};

set(settings, PLUGIN_SYSTEM_ENABLE_ALL_PLUGINS_CONFIG_PATH, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('default route provider', () => {
root = createRootWithCorePlugins({
server: {
basePath: '/hello',
restrictInternalApis: false,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('trace', () => {
requestId: {
allowFromAnyIp: true,
},
restrictInternalApis: false,
},
execution_context: {
enabled: true,
Expand Down Expand Up @@ -191,6 +192,7 @@ describe('trace', () => {
requestId: {
allowFromAnyIp: true,
},
restrictInternalApis: false,
},
});
await rootExecutionContextDisabled.preboot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const configService = createConfigService({
allowFromAnyIp: true,
ipAllowlist: [],
},
restrictInternalApis: false,
} as any,
});
const contextSetup = contextServiceMock.createSetupContract();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('http service', () => {
root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down Expand Up @@ -185,6 +186,7 @@ describe('http service', () => {
root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('Http2 - Smoke tests', () => {
redirectHttpFromPort: 10003,
},
shutdownTimeout: '5s',
restrictInternalApis: false,
});
config = new HttpConfig(rawConfig, CSP_CONFIG, EXTERNAL_URL_CONFIG, PERMISSIONS_POLICY_CONFIG);
server = new HttpServer(coreContext, 'tests', of(config.shutdownTimeout));
Expand Down
1 change: 1 addition & 0 deletions src/core/server/integration_tests/http/http_auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('http auth', () => {
root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('Http server', () => {
enabled: false,
},
shutdownTimeout: moment.duration(5, 's'),
restrictInternalApis: false,
} as any;

server = new HttpServer(coreContext, 'tests', of(config.shutdownTimeout));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const testConfig: Parameters<typeof createConfigService>[0] = {
'referrer-policy': 'strict-origin', // overrides a header that is defined by securityResponseHeaders
},
xsrf: { disableProtection: false, allowlist: [allowlistedTestPath] },
restrictInternalApis: false,
},
};

Expand Down
5 changes: 5 additions & 0 deletions src/core/server/integration_tests/http/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('request logging', () => {
const root = createRoot({
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down Expand Up @@ -74,6 +75,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down Expand Up @@ -121,6 +123,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
};

beforeEach(() => {
Expand Down Expand Up @@ -332,6 +335,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down Expand Up @@ -431,6 +435,7 @@ describe('request logging', () => {
initialize: false,
},
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
const { http } = await root.setup();
Expand Down
6 changes: 4 additions & 2 deletions src/core/server/integration_tests/http/oas.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ it('is disabled by default', async () => {
});

it('handles requests when enabled', async () => {
const server = await startService({ config: { server: { oas: { enabled: true } } } });
const server = await startService({
config: { server: { oas: { enabled: true }, restrictInternalApis: false } },
});
const result = await supertest(server.listener).get('/api/oas');
expect(result.status).toBe(200);
});
Expand Down Expand Up @@ -157,7 +159,7 @@ it.each([
'can filter paths based on query params $queryParam',
async ({ queryParam, includes, excludes }) => {
const server = await startService({
config: { server: { oas: { enabled: true } } },
config: { server: { oas: { enabled: true }, restrictInternalApis: false } },
createRoutes: (getRouter) => {
const router1 = getRouter(Symbol('myPlugin'));
router1.get(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function applyTestsWithDisableUnsafeEvalSetTo(disableUnsafeEval: boolean) {
csp: { disableUnsafeEval },
plugins: { initialize: false },
elasticsearch: { skipStartupConnectionCheck: true },
server: { restrictInternalApis: false },
});
await root.preboot();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ function createRoot() {
},
],
},
server: { restrictInternalApis: false },
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function createRoot(appenderConfig: any) {
},
],
},
server: { restrictInternalApis: false },
});
}

Expand Down
1 change: 1 addition & 0 deletions src/core/server/integration_tests/node/logging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ function createRootWithRoles(roles: string[]) {
level: 'info',
},
},
server: { restrictInternalApis: false },
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {

const result = await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send([
{
id: 'abc123',
Expand Down Expand Up @@ -127,6 +128,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {

await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send(docs)
.expect(200);

Expand Down Expand Up @@ -158,6 +160,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {
it('returns with status 400 when a type is hidden from the HTTP APIs', async () => {
const result = await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send([
{
id: 'hiddenID',
Expand All @@ -177,6 +180,7 @@ describe('POST /api/saved_objects/_bulk_create', () => {
it('logs a warning message when called', async () => {
await supertest(httpSetup.server.listener)
.post('/api/saved_objects/_bulk_create')
.set('x-elastic-internal-origin', 'kibana')
.send([
{
id: 'abc1234',
Expand Down
Loading