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

Add ability to specify CORS accepted origins #84316

Merged
merged 25 commits into from
Dec 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
cc0487a
add settings
mshustov Nov 24, 2020
8de37ff
Merge branch 'master' into add-cors
mshustov Nov 25, 2020
fa1c924
update abab package to version with types
mshustov Nov 25, 2020
29bfcae
add test case for CORS
mshustov Nov 25, 2020
e96ecea
add tests for cors config
mshustov Nov 25, 2020
f2ebd59
fix jest tests
mshustov Nov 25, 2020
e17fbdf
add deprecation message
mshustov Nov 25, 2020
eaf7782
tweak deprecation
mshustov Nov 25, 2020
f1659ec
make test runable on Cloud
mshustov Nov 25, 2020
0d7590c
add docs
mshustov Nov 25, 2020
fc7415b
Merge branch 'master' into add-cors
mshustov Nov 26, 2020
03903c5
fix type error
mshustov Nov 26, 2020
e11ef00
add test to throw on invalid URL
mshustov Nov 26, 2020
d5c8507
Merge branch 'master' into add-cors
mshustov Dec 1, 2020
466bd8b
address comments
mshustov Dec 1, 2020
569baf4
Merge branch 'master' into add-cors
mshustov Dec 1, 2020
8f57ec8
Update src/core/server/http/http_config.test.ts
mshustov Dec 2, 2020
bcebb53
Update docs/setup/settings.asciidoc
mshustov Dec 3, 2020
e9aa9ff
Merge branch 'master' into add-cors
mshustov Dec 3, 2020
9884a2c
Merge remote-tracking branch 'origin/add-cors' into add-cors
mshustov Dec 3, 2020
3b8a509
Merge branch 'master' into add-cors
mshustov Dec 4, 2020
8d6ea81
Merge branch 'master' into add-cors
mshustov Dec 7, 2020
d0956f8
Merge branch 'master' into add-cors
mshustov Dec 8, 2020
3e19081
allow kbn-xsrf headers to be set on CORS request
mshustov Dec 8, 2020
30b45aa
Merge branch 'master' into add-cors
mshustov Dec 10, 2020
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
9 changes: 9 additions & 0 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,15 @@ deprecation warning at startup. This setting cannot end in a slash (`/`).
| [[server-compression]] `server.compression.enabled:`
| Set to `false` to disable HTTP compression for all responses. *Default: `true`*

| `server.cors.enabled:`
| experimental[] Set to `true` to allow cross-origin API calls. *Default:* `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.

@kobelb We should start with marking it as experimental, I suppose

Copy link
Member

Choose a reason for hiding this comment

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

If we're marking the old server.cors setting as deprecated, then I'm not sure we should replace it with an experimental setting. Is there a reason to believe this won't be stable enough to mark as GA?

Copy link
Member

Choose a reason for hiding this comment

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

If we're marking the old server.cors setting as deprecated, then I'm not sure we should replace it with an experimental one. Is there a reason to believe this won't be stable enough to mark as GA?

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, the old server.cors setting could only be set when running Kibana from source, so we can just delete it if we want.

Copy link
Contributor Author

@mshustov mshustov Dec 2, 2020

Choose a reason for hiding this comment

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

that's correct. from #16714 (comment)

I apologize, it appears this feature was documented in #47701 when it should not have been. It is currently only available in dev mode which cannot be enabled in production builds.

We can mark it as GA, but it means that we won't be able to introduce any breaking changes if needed.
The experimental tag can be removed in v8.x after a trial.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha - I don't have any objections to marking as experimental or beta then


| `server.cors.credentials:`
| experimental[] Set to `true` to allow browser code to access response body whenever request performed with user credentials. *Default:* `false`

| `server.cors.origin:`
| experimental[] List of origins permitted to access resources. You must specify explicit hostnames and not use `*` for `server.cors.origin` when `server.cors.credentials: true`. *Default:* "*"

| `server.compression.referrerWhitelist:`
| Specifies an array of trusted hostnames, such as the {kib} host, or a reverse
proxy sitting in front of it. This determines whether HTTP compression may be used for responses, based on the request `Referer` header.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@
"@typescript-eslint/parser": "^4.8.1",
"@welldone-software/why-did-you-render": "^5.0.0",
"@yarnpkg/lockfile": "^1.1.0",
"abab": "^1.0.4",
"abab": "^2.0.4",
"angular-aria": "^1.8.0",
"angular-mocks": "^1.7.9",
"angular-recursion": "^1.0.5",
Expand Down
26 changes: 26 additions & 0 deletions src/core/server/config/deprecation/core_deprecations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,32 @@ describe('core deprecations', () => {
});
});

describe('server.cors', () => {
it('renames server.cors to server.cors.enabled', () => {
const { migrated } = applyCoreDeprecations({
server: { cors: true },
});
expect(migrated.server.cors).toEqual({ enabled: true });
});
mshustov marked this conversation as resolved.
Show resolved Hide resolved
it('logs a warning message about server.cors renaming', () => {
const { messages } = applyCoreDeprecations({
server: { cors: true },
});
expect(messages).toMatchInlineSnapshot(`
Array [
"\\"server.cors\\" is deprecated and has been replaced by \\"server.cors.enabled\\"",
]
`);
});
it('does not log deprecation message when server.cors.enabled set', () => {
const { migrated, messages } = applyCoreDeprecations({
server: { cors: { enabled: true } },
});
expect(migrated.server.cors).toEqual({ enabled: true });
expect(messages.length).toBe(0);
});
});

describe('rewriteBasePath', () => {
it('logs a warning is server.basePath is set and server.rewriteBasePath is not', () => {
const { messages } = applyCoreDeprecations({
Expand Down
12 changes: 12 additions & 0 deletions src/core/server/config/deprecation/core_deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ const rewriteBasePathDeprecation: ConfigDeprecation = (settings, fromPath, log)
return settings;
};

const rewriteCorsSettings: ConfigDeprecation = (settings, fromPath, log) => {
const corsSettings = get(settings, 'server.cors');
if (typeof get(settings, 'server.cors') === 'boolean') {
log('"server.cors" is deprecated and has been replaced by "server.cors.enabled"');
settings.server.cors = {
enabled: corsSettings,
};
}
return settings;
};

const cspRulesDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
const NONCE_STRING = `{nonce}`;
// Policies that should include the 'self' source
Expand Down Expand Up @@ -131,6 +142,7 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({ rename, unu
rename('cpu.cgroup.path.override', 'ops.cGroupOverrides.cpuPath'),
rename('cpuacct.cgroup.path.override', 'ops.cGroupOverrides.cpuAcctPath'),
rename('server.xsrf.whitelist', 'server.xsrf.allowlist'),
rewriteCorsSettings,
configPathDeprecation,
dataPathDeprecation,
rewriteBasePathDeprecation,
Expand Down
6 changes: 5 additions & 1 deletion 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.

3 changes: 3 additions & 0 deletions src/core/server/http/cookie_session_storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ configService.atPath.mockImplementation((path) => {
allowFromAnyIp: true,
ipAllowlist: [],
},
cors: {
enabled: false,
},
} as any);
}
if (path === 'externalUrl') {
Expand Down
53 changes: 53 additions & 0 deletions src/core/server/http/http_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,59 @@ describe('with compression', () => {
});
});

describe('cors', () => {
describe('origin', () => {
it('list cannot be empty', () => {
expect(() =>
config.schema.validate({
cors: {
origin: [],
},
})
).toThrowErrorMatchingInlineSnapshot(`
"[cors.origin]: types that failed validation:
- [cors.origin.0]: expected value to equal [*]
- [cors.origin.1]: array size is [0], but cannot be smaller than [1]"
`);
});

it('list of valid URLs', () => {
const origin = ['http://127.0.0.1:3000', 'https://elastic.co'];
expect(
config.schema.validate({
cors: { origin },
}).cors.origin
).toStrictEqual(origin);

expect(() =>
config.schema.validate({
cors: {
origin: ['*://elastic.co/*'],
},
})
).toThrow();
});

it('can be configured as "*" wildcard', () => {
expect(config.schema.validate({ cors: { origin: '*' } }).cors.origin).toBe('*');
});
});
describe('credentials', () => {
it('cannot use wildcard origin if "credentials: true"', () => {
expect(
() => config.schema.validate({ cors: { credentials: true, origin: '*' } }).cors.origin
).toThrowErrorMatchingInlineSnapshot(
`"[cors]: Cannot specify wildcard origin \\"*\\" with \\"credentials: true\\". Please provide a list of allowed origins."`
);
expect(
() => config.schema.validate({ cors: { credentials: true } }).cors.origin
).toThrowErrorMatchingInlineSnapshot(
`"[cors]: Cannot specify wildcard origin \\"*\\" with \\"credentials: true\\". Please provide a list of allowed origins."`
);
});
});
});

describe('HttpConfig', () => {
it('converts customResponseHeaders to strings or arrays of strings', () => {
const httpSchema = config.schema;
Expand Down
28 changes: 25 additions & 3 deletions src/core/server/http/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { SslConfig, sslSchema } from './ssl_config';

const validBasePathRegex = /^\/.*[^\/]$/;
const uuidRegexp = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$/i;

const hostURISchema = schema.uri({ scheme: ['http', 'https'] });
const match = (regex: RegExp, errorMsg: string) => (str: string) =>
regex.test(str) ? undefined : errorMsg;

Expand All @@ -45,7 +45,25 @@ export const config = {
validate: match(validBasePathRegex, "must start with a slash, don't end with one"),
})
),
cors: schema.boolean({ defaultValue: false }),
cors: schema.object(
{
enabled: schema.boolean({ defaultValue: false }),
credentials: schema.boolean({ defaultValue: false }),
origin: schema.oneOf(
[schema.literal('*'), schema.arrayOf(hostURISchema, { minSize: 1 })],
{
defaultValue: '*',
}
),
},
{
validate(value) {
if (value.credentials === true && value.origin === '*') {
return 'Cannot specify wildcard origin "*" with "credentials: true". Please provide a list of allowed origins.';
}
},
}
),
customResponseHeaders: schema.recordOf(schema.string(), schema.any(), {
defaultValue: {},
}),
Expand Down Expand Up @@ -148,7 +166,11 @@ export class HttpConfig {
public keepaliveTimeout: number;
public socketTimeout: number;
public port: number;
public cors: boolean | { origin: string[] };
public cors: {
enabled: boolean;
credentials: boolean;
origin: '*' | string[];
};
public customResponseHeaders: Record<string, string | string[]>;
public maxPayload: ByteSizeValue;
public basePath?: string;
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ beforeEach(() => {
allowFromAnyIp: true,
ipAllowlist: [],
},
cors: {
enabled: false,
},
} as any;

configWithSSL = {
Expand Down
23 changes: 23 additions & 0 deletions src/core/server/http/http_tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ describe('timeouts', () => {
host: '127.0.0.1',
maxPayload: new ByteSizeValue(1024),
ssl: {},
cors: {
enabled: false,
},
compression: { enabled: true },
requestId: {
allowFromAnyIp: true,
Expand Down Expand Up @@ -187,6 +190,26 @@ describe('getServerOptions', () => {
}
`);
});

it('properly configures CORS when cors enabled', () => {
const httpConfig = new HttpConfig(
config.schema.validate({
cors: {
enabled: true,
credentials: false,
origin: '*',
},
}),
{} as any,
{} as any
);

expect(getServerOptions(httpConfig).routes?.cors).toEqual({
credentials: false,
origin: '*',
headers: ['Accept', 'Authorization', 'Content-Type', 'If-None-Match', 'kbn-xsrf'],
});
});
});

describe('getRequestId', () => {
Expand Down
25 changes: 20 additions & 5 deletions src/core/server/http/http_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,34 @@
* specific language governing permissions and limitations
* under the License.
*/

import { Lifecycle, Request, ResponseToolkit, Server, ServerOptions, Util } from '@hapi/hapi';
import { Server } from '@hapi/hapi';
import type {
Lifecycle,
Request,
ResponseToolkit,
RouteOptionsCors,
ServerOptions,
Util,
} from '@hapi/hapi';
import Hoek from '@hapi/hoek';
import { ServerOptions as TLSOptions } from 'https';
import { ValidationError } from 'joi';
import type { ServerOptions as TLSOptions } from 'https';
import type { ValidationError } from 'joi';
import uuid from 'uuid';
import { HttpConfig } from './http_config';
import { validateObject } from './prototype_pollution';

const corsAllowedHeaders = ['Accept', 'Authorization', 'Content-Type', 'If-None-Match', 'kbn-xsrf'];
/**
* Converts Kibana `HttpConfig` into `ServerOptions` that are accepted by the Hapi server.
*/
export function getServerOptions(config: HttpConfig, { configureTLS = true } = {}) {
const cors: RouteOptionsCors | false = config.cors.enabled
? {
credentials: config.cors.credentials,
origin: config.cors.origin,
headers: corsAllowedHeaders,
}
: false;
// Note that all connection options configured here should be exactly the same
// as in the legacy platform server (see `src/legacy/server/http/index`). Any change
// SHOULD BE applied in both places. The only exception is TLS-specific options,
Expand All @@ -41,7 +56,7 @@ export function getServerOptions(config: HttpConfig, { configureTLS = true } = {
privacy: 'private',
otherwise: 'private, no-cache, no-store, must-revalidate',
},
cors: config.cors,
cors,
payload: {
maxBytes: config.maxPayload.getValueInBytes(),
},
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/http/https_redirect_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ beforeEach(() => {
enabled: true,
redirectHttpFromPort: chance.integer({ min: 20000, max: 30000 }),
},
cors: {
enabled: false,
},
} as HttpConfig;

server = new HttpsRedirectServer(loggingSystemMock.create().get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ describe('core lifecycle handlers', () => {
ssl: {
enabled: false,
},
cors: {
enabled: false,
},
compression: { enabled: true },
name: kibanaName,
customResponseHeaders: {
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/http/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ configService.atPath.mockImplementation((path) => {
ssl: {
enabled: false,
},
cors: {
enabled: false,
},
compression: { enabled: true },
xsrf: {
disableProtection: true,
Expand Down
1 change: 1 addition & 0 deletions x-pack/scripts/functional_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const alwaysImportedTests = [
require.resolve('../test/security_functional/oidc.config.ts'),
require.resolve('../test/security_functional/saml.config.ts'),
require.resolve('../test/functional_embedded/config.ts'),
require.resolve('../test/functional_cors/config.ts'),
require.resolve('../test/functional_enterprise_search/without_host_configured.config.ts'),
];
const onlyNotInCoverageTests = [
Expand Down
Loading