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

fix(CORS): only allow connections from the designated host #4985

Merged
merged 6 commits into from
Feb 3, 2020

Conversation

Akryum
Copy link
Member

@Akryum Akryum commented Dec 20, 2019

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:
For security reasons, only allow connection from localhost.

@Akryum Akryum self-assigned this Dec 20, 2019
packages/@vue/cli/lib/ui.js Outdated Show resolved Hide resolved
@haoqunjiang
Copy link
Member

I'm not sure if I was testing it incorrectly, but as far as I tried out, this change seems does not prevent other domains from accessing the GraphQL interface.

@haoqunjiang
Copy link
Member

I tested this by running another instance of the cli-ui frontend and point the VUE_APP_CLI_UI_URL variable to the modified server instance.

@haoqunjiang
Copy link
Member

Got it.

The origin check should be placed in the WebSocket server, that is, the apolloServerOptions.subscriptions.onConnect method.

It's because

a CORS policy cannot be inserted into an HTTP Upgrade response

@Akryum
Copy link
Member Author

Akryum commented Dec 30, 2019

Good catch!

@haoqunjiang
Copy link
Member

I got it fixed locally, by adding the following lines in vue-cli-plugin-apollo/graphql-server/index.js:

httpServer.on('upgrade', (req, socket) => {
  const { origin } = req.headers
  if (!origin || !/https?:\/\/localhost/.test(origin)) {
    socket.destroy()
  }
})

But since the httpServer instance is not exported from the plugin, I can't do the fix on the Vue CLI side.
Could you please add it to the plugin API?

@haoqunjiang haoqunjiang changed the title fix(cors): only allow localhost fix(CORS): only allow connections from the designated host Feb 3, 2020
@haoqunjiang
Copy link
Member

I've migrated to a custom version of the graphql-server to work around this issue for now.

Should check it again after the apollo plugin adds the needed API.

@haoqunjiang haoqunjiang merged commit da43343 into dev Feb 3, 2020
@haoqunjiang haoqunjiang deleted the vue-ui-cors branch February 3, 2020 11:35
haoqunjiang added a commit that referenced this pull request Feb 4, 2020
…5142)

* fix: followup of #4985, allow same-site ws requests of any domain

* fix: match whole string
mactanxin pushed a commit to mactanxin/vue-cli that referenced this pull request Feb 11, 2020
* fix(cors): only allow localhost

* fix: use host so it's configurable

* fix: use cors options object

* feat: use a custom graphql-server instead of the one from apollo plugin

exports the httpServer instance

* fix: add CORS validation in the http upgrade request

Co-authored-by: Haoqun Jiang <haoqunjiang@gmail.com>
mactanxin pushed a commit to mactanxin/vue-cli that referenced this pull request Feb 11, 2020
…ain (vuejs#5142)

* fix: followup of vuejs#4985, allow same-site ws requests of any domain

* fix: match whole string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants