Skip to content

Commit

Permalink
[7.x] Rename server.xsrf.whitelist to server.xsrf.allowlist (#84791) (#…
Browse files Browse the repository at this point in the history
…84861)

* Rename server.xsrf.whitelist to server.xsrf.allowlist (#84791)

* rename xsrd.whitelist to xsrf.allowlist

* update docs

* update telemetry schema

* update kbn-config tests
# Conflicts:
#	src/core/server/config/deprecation/core_deprecations.ts

* Update core_deprecations.ts

* miss import
  • Loading branch information
mshustov authored Dec 3, 2020
1 parent 36a62c1 commit 645eae8
Show file tree
Hide file tree
Showing 23 changed files with 43 additions and 51 deletions.
2 changes: 1 addition & 1 deletion docs/api/using-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ For all APIs, you must use a request header. The {kib} APIs support the `kbn-xsr
By default, you must use `kbn-xsrf` for all API calls, except in the following scenarios:

* The API endpoint uses the `GET` or `HEAD` operations
* The path is whitelisted using the <<settings-xsrf-whitelist, `server.xsrf.whitelist`>> setting
* The path is allowed using the <<settings-xsrf-allowlist, `server.xsrf.allowlist`>> setting
* XSRF protections are disabled using the <<settings-xsrf-disableProtection, `server.xsrf.disableProtection`>> setting

`Content-Type: application/json`::
Expand Down
2 changes: 1 addition & 1 deletion docs/apm/api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ users interacting with APM APIs must have <<apm-app-api-user,sufficient privileg
By default, you must use `kbn-xsrf` for all API calls, except in the following scenarios:

* The API endpoint uses the `GET` or `HEAD` operations
* The path is whitelisted using the <<settings-xsrf-whitelist, `server.xsrf.whitelist`>> setting
* The path is allowed using the <<settings-xsrf-allowlist, `server.xsrf.allowlist`>> setting
* XSRF protections are disabled using the <<settings-xsrf-disableProtection, `server.xsrf.disableProtection`>> setting

`Content-Type: application/json`::
Expand Down
4 changes: 2 additions & 2 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -575,10 +575,10 @@ all http requests to https over the port configured as <<server-port, `server.po
| An array of supported protocols with versions.
Valid protocols: `TLSv1`, `TLSv1.1`, `TLSv1.2`, `TLSv1.3`. *Default: TLSv1.1, TLSv1.2, TLSv1.3*

| [[settings-xsrf-whitelist]] `server.xsrf.whitelist:`
| [[settings-xsrf-allowlist]] `server.xsrf.allowlist:`
| It is not recommended to disable protections for
arbitrary API endpoints. Instead, supply the `kbn-xsrf` header.
The <<settings-xsrf-whitelist, `server.xsrf.whitelist`>> setting requires the following format:
The <<settings-xsrf-allowlist, `server.xsrf.allowlist`>> setting requires the following format:

|===

Expand Down

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 @@ -96,7 +96,7 @@ describe('#get', () => {
someNotSupportedValue: 'val',
xsrf: {
disableProtection: false,
whitelist: [],
allowlist: [],
},
},
});
Expand All @@ -119,7 +119,7 @@ describe('#get', () => {
someNotSupportedValue: 'val',
xsrf: {
disableProtection: false,
whitelist: [],
allowlist: [],
},
},
});
Expand Down
5 changes: 3 additions & 2 deletions src/core/server/config/deprecation/core_deprecations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,13 @@ describe('core deprecations', () => {

describe('xsrfDeprecation', () => {
it('logs a warning if server.xsrf.whitelist is set', () => {
const { messages } = applyCoreDeprecations({
const { migrated, messages } = applyCoreDeprecations({
server: { xsrf: { whitelist: ['/path'] } },
});
expect(migrated.server.xsrf.allowlist).toEqual(['/path']);
expect(messages).toMatchInlineSnapshot(`
Array [
"It is not recommended to disable xsrf protections for API endpoints via [server.xsrf.whitelist]. It will be removed in 8.0 release. Instead, supply the \\"kbn-xsrf\\" header.",
"\\"server.xsrf.whitelist\\" is deprecated and has been replaced by \\"server.xsrf.allowlist\\"",
]
`);
});
Expand Down
13 changes: 2 additions & 11 deletions src/core/server/config/deprecation/core_deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,6 @@ const dataPathDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
return settings;
};

const xsrfDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
if ((settings.server?.xsrf?.whitelist ?? []).length > 0) {
log(
'It is not recommended to disable xsrf protections for API endpoints via [server.xsrf.whitelist]. ' +
'It will be removed in 8.0 release. Instead, supply the "kbn-xsrf" header.'
);
}
return settings;
};

const rewriteBasePathDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
if (has(settings, 'server.basePath') && !has(settings, 'server.rewriteBasePath')) {
log(
Expand Down Expand Up @@ -116,6 +106,7 @@ const mapManifestServiceUrlDeprecation: ConfigDeprecation = (settings, fromPath,
export const coreDeprecationProvider: ConfigDeprecationProvider = ({
unusedFromRoot,
renameFromRoot,
rename,
}) => [
unusedFromRoot('savedObjects.indexCheckTimeout'),
unusedFromRoot('server.xsrf.token'),
Expand Down Expand Up @@ -148,12 +139,12 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({
renameFromRoot('xpack.telemetry.url', 'telemetry.url'),
renameFromRoot('cpu.cgroup.path.override', 'ops.cGroupOverrides.cpuPath'),
renameFromRoot('cpuacct.cgroup.path.override', 'ops.cGroupOverrides.cpuAcctPath'),
renameFromRoot('server.xsrf.whitelist', 'server.xsrf.allowlist'),
unusedFromRoot('elasticsearch.preserveHost'),
unusedFromRoot('elasticsearch.startupTimeout'),
configPathDeprecation,
dataPathDeprecation,
rewriteBasePathDeprecation,
cspRulesDeprecation,
mapManifestServiceUrlDeprecation,
xsrfDeprecation,
];
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const createStartContractMock = () => {
},
xsrf: {
disableProtection: false,
whitelistConfigured: false,
allowlistConfigured: false,
},
},
logging: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ describe('CoreUsageDataService', () => {
"truststoreConfigured": false,
},
"xsrf": Object {
"allowlistConfigured": false,
"disableProtection": false,
"whitelistConfigured": false,
},
},
"logging": Object {
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/core_usage_data_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export class CoreUsageDataService implements CoreService<void, CoreUsageDataStar
},
xsrf: {
disableProtection: http.xsrf.disableProtection,
whitelistConfigured: isConfigured.array(http.xsrf.whitelist),
allowlistConfigured: isConfigured.array(http.xsrf.allowlist),
},
requestId: {
allowFromAnyIp: http.requestId.allowFromAnyIp,
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/core_usage_data/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export interface CoreConfigUsageData {
};
xsrf: {
disableProtection: boolean;
whitelistConfigured: boolean;
allowlistConfigured: boolean;
};
requestId: {
allowFromAnyIp: boolean;
Expand Down

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

2 changes: 1 addition & 1 deletion src/core/server/http/cookie_session_storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ configService.atPath.mockReturnValue(
compression: { enabled: true },
xsrf: {
disableProtection: true,
whitelist: [],
allowlist: [],
},
customResponseHeaders: {},
requestId: {
Expand Down
6 changes: 3 additions & 3 deletions src/core/server/http/http_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,15 @@ test('uses os.hostname() as default for server.name', () => {
expect(validated.name).toEqual('kibana-hostname');
});

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

Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export const config = {
),
xsrf: schema.object({
disableProtection: schema.boolean({ defaultValue: false }),
whitelist: schema.arrayOf(
allowlist: schema.arrayOf(
schema.string({ validate: match(/^\//, 'must start with a slash') }),
{ defaultValue: [] }
),
Expand Down Expand Up @@ -142,7 +142,7 @@ export class HttpConfig {
public ssl: SslConfig;
public compression: { enabled: boolean; referrerWhitelist?: string[] };
public csp: ICspConfig;
public xsrf: { disableProtection: boolean; whitelist: string[] };
public xsrf: { disableProtection: boolean; allowlist: string[] };
public requestId: { allowFromAnyIp: boolean; ipAllowlist: string[] };

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const actualVersion = pkg.version;
const versionHeader = 'kbn-version';
const xsrfHeader = 'kbn-xsrf';
const nameHeader = 'kbn-name';
const whitelistedTestPath = '/xsrf/test/route/whitelisted';
const allowlistedTestPath = '/xsrf/test/route/whitelisted';
const xsrfDisabledTestPath = '/xsrf/test/route/disabled';
const kibanaName = 'my-kibana-name';
const setupDeps = {
Expand All @@ -63,7 +63,7 @@ describe('core lifecycle handlers', () => {
customResponseHeaders: {
'some-header': 'some-value',
},
xsrf: { disableProtection: false, whitelist: [whitelistedTestPath] },
xsrf: { disableProtection: false, allowlist: [allowlistedTestPath] },
requestId: {
allowFromAnyIp: true,
ipAllowlist: [],
Expand Down Expand Up @@ -179,7 +179,7 @@ describe('core lifecycle handlers', () => {
}
);
((router as any)[method.toLowerCase()] as RouteRegistrar<any>)<any, any, any>(
{ path: whitelistedTestPath, validate: false },
{ path: allowlistedTestPath, validate: false },
(context, req, res) => {
return res.ok({ body: 'ok' });
}
Expand Down Expand Up @@ -235,7 +235,7 @@ describe('core lifecycle handlers', () => {
});

it('accepts whitelisted requests without either an xsrf or version header', async () => {
await getSupertest(method.toLowerCase(), whitelistedTestPath).expect(200, 'ok');
await getSupertest(method.toLowerCase(), allowlistedTestPath).expect(200, 'ok');
});

it('accepts requests on a route with disabled xsrf protection', async () => {
Expand Down
16 changes: 8 additions & 8 deletions src/core/server/http/lifecycle_handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('xsrf post-auth handler', () => {

describe('non destructive methods', () => {
it('accepts requests without version or xsrf header', () => {
const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } });
const config = createConfig({ xsrf: { allowlist: [], disableProtection: false } });
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({ method: 'get', headers: {} });

Expand All @@ -74,7 +74,7 @@ describe('xsrf post-auth handler', () => {

describe('destructive methods', () => {
it('accepts requests with xsrf header', () => {
const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } });
const config = createConfig({ xsrf: { allowlist: [], disableProtection: false } });
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({ method: 'post', headers: { 'kbn-xsrf': 'xsrf' } });

Expand All @@ -88,7 +88,7 @@ describe('xsrf post-auth handler', () => {
});

it('accepts requests with version header', () => {
const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } });
const config = createConfig({ xsrf: { allowlist: [], disableProtection: false } });
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({ method: 'post', headers: { 'kbn-version': 'some-version' } });

Expand All @@ -102,7 +102,7 @@ describe('xsrf post-auth handler', () => {
});

it('returns a bad request if called without xsrf or version header', () => {
const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } });
const config = createConfig({ xsrf: { allowlist: [], disableProtection: false } });
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({ method: 'post' });

Expand All @@ -121,7 +121,7 @@ describe('xsrf post-auth handler', () => {
});

it('accepts requests if protection is disabled', () => {
const config = createConfig({ xsrf: { whitelist: [], disableProtection: true } });
const config = createConfig({ xsrf: { allowlist: [], disableProtection: true } });
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({ method: 'post', headers: {} });

Expand All @@ -134,9 +134,9 @@ describe('xsrf post-auth handler', () => {
expect(result).toEqual('next');
});

it('accepts requests if path is whitelisted', () => {
it('accepts requests if path is allowlisted', () => {
const config = createConfig({
xsrf: { whitelist: ['/some-path'], disableProtection: false },
xsrf: { allowlist: ['/some-path'], disableProtection: false },
});
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({ method: 'post', headers: {}, path: '/some-path' });
Expand All @@ -152,7 +152,7 @@ describe('xsrf post-auth handler', () => {

it('accepts requests if xsrf protection on a route is disabled', () => {
const config = createConfig({
xsrf: { whitelist: [], disableProtection: false },
xsrf: { allowlist: [], disableProtection: false },
});
const handler = createXsrfPostAuthHandler(config);
const request = forgeRequest({
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/http/lifecycle_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ const XSRF_HEADER = 'kbn-xsrf';
const KIBANA_NAME_HEADER = 'kbn-name';

export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler => {
const { whitelist, disableProtection } = config.xsrf;
const { allowlist, disableProtection } = config.xsrf;

return (request, response, toolkit) => {
if (
disableProtection ||
whitelist.includes(request.route.path) ||
allowlist.includes(request.route.path) ||
request.route.options.xsrfRequired === false
) {
return toolkit.next();
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/http/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ configService.atPath.mockReturnValue(
compression: { enabled: true },
xsrf: {
disableProtection: true,
whitelist: [],
allowlist: [],
},
customResponseHeaders: {},
requestId: {
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ export interface CoreConfigUsageData {
};
xsrf: {
disableProtection: boolean;
whitelistConfigured: boolean;
allowlistConfigured: boolean;
};
requestId: {
allowFromAnyIp: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function getCoreUsageCollector(
},
xsrf: {
disableProtection: { type: 'boolean' },
whitelistConfigured: { type: 'boolean' },
allowlistConfigured: { type: 'boolean' },
},
requestId: {
allowFromAnyIp: { type: 'boolean' },
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/telemetry/schema/oss_plugins.json
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,7 @@
"disableProtection": {
"type": "boolean"
},
"whitelistConfigured": {
"allowlistConfigured": {
"type": "boolean"
}
}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/alerting_api_integration/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions)
(pluginDir) =>
`--plugin-path=${path.resolve(__dirname, 'fixtures', 'plugins', pluginDir)}`
),
`--server.xsrf.whitelist=${JSON.stringify(getAllExternalServiceSimulatorPaths())}`,
`--server.xsrf.allowlist=${JSON.stringify(getAllExternalServiceSimulatorPaths())}`,
...(ssl
? [
`--elasticsearch.hosts=${servers.elasticsearch.protocol}://${servers.elasticsearch.hostname}:${servers.elasticsearch.port}`,
Expand Down

0 comments on commit 645eae8

Please sign in to comment.