Add ability to specify CORS accepted origins by mshustov · Pull Request #84316 · elastic/kibana

mshustov · 2020-11-25T11:18:40Z

Resolves #16714.

Summary

The current implementation is based on a discussion in the parent issue.
Kibana supports only 3 CORS options at the moment:

server.cors.enabled Set to true to allow cross-origin API calls. Default: false
server.cors.credentials Set to true to allow browser code to access response body whenever request performed with user credentials. Default: false
server.cors.origin List of origins permitted to access resources. You must specify server.cors.origin when server.cors.credentials: true. Default: "*"

Kibana extend Access-Control-Allow-Headers list with the next headers: ['Accept', 'Authorization', 'Content-Type', 'If-None-Match', 'kbn-xsrf']

Checklist

Delete any items that are not applicable to this PR.

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

Release Notes

Added experimental support for configuring CORS policy:

server.cors.enabled Set to true to allow cross-origin API calls. Default: false
server.cors.allowCredentials Set to true to allow browser code to access response body whenever request performed with user credentials. Default: false
server.cors.allowOrigin List of origins permitted to access resources. You must specify server.cors.allowOrigin when server.cors.allowCredentials: true. Default: ["*"]

mshustov · 2020-11-26T09:36:10Z

docs/setup/settings.asciidoc

 | 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`


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

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?

elasticmachine · 2020-11-26T10:27:48Z

Pinging @elastic/kibana-core (Team:Core)

src/core/server/config/deprecation/core_deprecations.test.ts

x-pack/test/functional_cors/plugins/kibana_cors_test/server/plugin.ts

pgayvallet · 2020-11-30T14:49:04Z

x-pack/test/functional_cors/tests/cors.ts

+    it('Communicates to Kibana with configured CORS', async () => {
+      const args: string[] = config.get('kbnTestServer.serverArgs');
+      const originSetting = args.find((str) => str.includes('server.cors.origin'));
+      if (!originSetting) {
+        throw new Error('Cannot find "server.cors.origin" argument');
+      }
+      const [, value] = originSetting.split('=');
+      const url = JSON.parse(value);
+
+      await browser.navigateTo(url[0]);
+      const element = await find.byCssSelector('p');
+      expect(await element.getVisibleText()).to.be('content from kibana');
+    });


++ Nice test!

legrego · 2020-12-01T17:53:29Z

ACK: Will review later today or tomorrow

legrego · 2020-12-01T17:55:34Z

docs/setup/settings.asciidoc

 | 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`


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?

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

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.

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

legrego · 2020-12-01T18:01:01Z

docs/setup/settings.asciidoc

 | 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`


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?

src/core/server/http/http_config.test.ts

legrego · 2020-12-02T18:46:39Z

x-pack/test/functional_cors/tests/cors.ts

+    it('Communicates to Kibana with configured CORS', async () => {
+      const args: string[] = config.get('kbnTestServer.serverArgs');
+      const originSetting = args.find((str) => str.includes('server.cors.origin'));
+      if (!originSetting) {
+        throw new Error('Cannot find "server.cors.origin" argument');
+      }
+      const [, value] = originSetting.split('=');
+      const url = JSON.parse(value);
+
+      await browser.navigateTo(url[0]);
+      const element = await find.byCssSelector('p');
+      expect(await element.getVisibleText()).to.be('content from kibana');
+    });


++ Nice test!

Co-authored-by: Larry Gregory <lgregorydev@gmail.com>

kobelb

Thanks for adding this, @restrry.

Without the ability to customize the access-control-allow-headers response header and Kibana not responding to the OPTIONS pre-flights, the user will be limited to the APIs that they can call using CORS. Do we intend to add those in a separate PR, or is there a reason why we shouldn't do so?

docs/setup/settings.asciidoc

Co-authored-by: Brandon Kobel <brandon.kobel@gmail.com>

mshustov · 2020-12-03T06:53:00Z

Without the ability to customize the access-control-allow-headers response header and Kibana not responding to the OPTIONS pre-flights, the user will be limited to the APIs that they can call using CORS. Do we intend to add those in a separate PR, or is there a reason why we shouldn't do so?

@kobelb What kind of API is cannot be used with the current implementation? I can think of xsrf protection only, but that might be configured via server.xsrf.allowlist.

kobelb · 2020-12-03T15:50:25Z

@kobelb What kind of API is cannot be used with the current implementation? I can think of xsrf protection only, but that might be configured via server.xsrf.allowlist.

We shouldn't be adding APIs to the server.xsrf.allowlist for them to work with CORS. CORS allows us to relax the same-origin restrictions in a selective manner, based on the origin, headers, etc.

Currently, all non-GET HTTP APIs in Kibana don't work with CORS. Kibana requires that all non-GET HTTP requests have the kbn-xsrf header specified for our XSRF protection. This custom header causes the user's browser to send a pre-flight OPTIONS request, that includes the origin, to Kibana. This allows Kibana to selectively determine which origins we're comfortable relaxing the same-origin restrictions and accepting requests from. However, these OPTIONS requests are failing right now:

FWIW, my prior statement about Kibana not responding to OPTIONS requests was incorrect. In prior versions of Kibana, I recall Kibana not responding to any HTTP requests using the OPTIONS verb, but it appears that Kibana is now.

mshustov · 2020-12-08T17:51:46Z

@kobelb I adopted the code to allow kbn-xsrf header to be set on CORS request 3e19081, PTAL

kobelb · 2020-12-09T22:09:08Z

Thanks for making these changes, @restrry. This is looking great!

* add settings * update abab package to version with types * add test case for CORS * add tests for cors config * fix jest tests * add deprecation message * tweak deprecation * make test runable on Cloud * add docs * fix type error * add test to throw on invalid URL * address comments * Update src/core/server/http/http_config.test.ts Co-authored-by: Larry Gregory <lgregorydev@gmail.com> * Update docs/setup/settings.asciidoc Co-authored-by: Brandon Kobel <brandon.kobel@gmail.com> * allow kbn-xsrf headers to be set on CORS request Co-authored-by: Larry Gregory <lgregorydev@gmail.com> Co-authored-by: Brandon Kobel <brandon.kobel@gmail.com> # Conflicts: # src/core/server/config/deprecation/core_deprecations.ts # x-pack/scripts/functional_tests.js

kibanamachine · 2020-12-14T12:37:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 30b45aa

Metrics [docs]

Distributable file count

id	before	after	diff
`default`	46990	47750	+760

History

💚 Build #93389 succeeded 30b45aa
💚 Build #92823 succeeded 3e19081
💚 Build #91622 succeeded 9884a2c
💚 Build #91619 succeeded bcebb53
💚 Build #91495 succeeded 8f57ec8

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

mshustov added 4 commits November 24, 2020 12:49

add settings

cc0487a

Merge branch 'master' into add-cors

8de37ff

update abab package to version with types

fa1c924

add test case for CORS

29bfcae

mshustov added release_note:enhancement Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// enhancement New value added to drive a business result v8.0.0 v7.11.0 labels Nov 25, 2020

mshustov added 8 commits November 25, 2020 13:53

add tests for cors config

e96ecea

fix jest tests

f2ebd59

add deprecation message

e17fbdf

tweak deprecation

eaf7782

make test runable on Cloud

f1659ec

add docs

0d7590c

Merge branch 'master' into add-cors

fc7415b

fix type error

03903c5

mshustov commented Nov 26, 2020

View reviewed changes

mshustov added the docs label Nov 26, 2020

mshustov marked this pull request as ready for review November 26, 2020 10:27

mshustov requested a review from a team as a code owner November 26, 2020 10:27

mshustov changed the title ~~[WIP] Add ability to specify CORS accepted origins~~ Add ability to specify CORS accepted origins Nov 26, 2020

add test to throw on invalid URL

e11ef00

pgayvallet approved these changes Nov 30, 2020

View reviewed changes

mshustov requested a review from legrego December 1, 2020 13:48

mshustov added 3 commits December 1, 2020 14:48

Merge branch 'master' into add-cors

d5c8507

address comments

466bd8b

Merge branch 'master' into add-cors

569baf4

legrego reviewed Dec 1, 2020

View reviewed changes

mshustov requested review from kobelb and legrego December 2, 2020 16:03

legrego approved these changes Dec 2, 2020

View reviewed changes

Update src/core/server/http/http_config.test.ts

8f57ec8

Co-authored-by: Larry Gregory <lgregorydev@gmail.com>

kobelb approved these changes Dec 2, 2020

View reviewed changes

docs/setup/settings.asciidoc Outdated Show resolved Hide resolved

mshustov and others added 3 commits December 3, 2020 09:32

Update docs/setup/settings.asciidoc

bcebb53

Co-authored-by: Brandon Kobel <brandon.kobel@gmail.com>

Merge branch 'master' into add-cors

e9aa9ff

Merge remote-tracking branch 'origin/add-cors' into add-cors

9884a2c

Merge branch 'master' into add-cors

3b8a509

kobelb mentioned this pull request Dec 4, 2020

Ability to specify CORS accepted origins #16714

Closed

mshustov added 3 commits December 7, 2020 19:22

Merge branch 'master' into add-cors

8d6ea81

Merge branch 'master' into add-cors

d0956f8

allow kbn-xsrf headers to be set on CORS request

3e19081

mshustov requested a review from kobelb December 8, 2020 17:51

kobelb approved these changes Dec 9, 2020

View reviewed changes

Merge branch 'master' into add-cors

30b45aa

mshustov merged commit 44688d9 into elastic:master Dec 10, 2020

mshustov deleted the add-cors branch December 10, 2020 14:14

mshustov mentioned this pull request Dec 10, 2020

[7.x] Add ability to specify CORS accepted origins (#84316) #85570

Merged

mshustov mentioned this pull request Dec 14, 2020

align cors settings names with elasticsearch #85738

Merged

jportner mentioned this pull request Mar 8, 2021

Improve CORS response headers #94024

Open

jeramysoucy mentioned this pull request Oct 29, 2024

[FTR][Ownership] Assign guided_onboarding, etc #198044

Merged

Conversation

mshustov commented Nov 25, 2020 • edited by jportner Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Release Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 26, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

legrego commented Dec 1, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshustov Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kobelb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mshustov commented Dec 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kobelb commented Dec 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mshustov commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kobelb commented Dec 9, 2020

Uh oh!

kibanamachine commented Dec 14, 2020

💚 Build Succeeded

Metrics [docs]

Distributable file count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mshustov commented Nov 25, 2020 •

edited by jportner

Loading

mshustov Dec 2, 2020 •

edited

Loading

kobelb left a comment •

edited

Loading

mshustov commented Dec 3, 2020 •

edited

Loading

kobelb commented Dec 3, 2020 •

edited

Loading

mshustov commented Dec 8, 2020 •

edited

Loading