elements icon indicating copy to clipboard operation
elements copied to clipboard

Add CORS handling for web-components

Open danpat opened this issue 4 years ago • 20 comments

  • [x] Read CONTRIBUTING.md

I found myself using elements to create an online schema viewer that lives behind an authenticated proxy. Things mostly worked, except for parts of our schema that were remote $ref references in the primary schema document. When elements attempted to resolve them in our environment, the browser console log exploded with CORS-related errors.

After a bunch of digging, it turned out that by default, elements was leaving @stoplight/json-schema-ref-parser use it's default HTTP resolver settings, which has has credentials: 'omit' in the options passed to fetch(). This fails when CORS headers are required.

This PR doesn't change the default, but exposes a new option to the elements-api tag called withCredentials that is passed through to @stoplight/json-schema-ref-parser, allowing user control over whether CORS handling is enabled or not.

I'm not a front-end person by default, so apologies if the style of this PR is out of line, or if I've missed something fundamental. These changes appear to be functioning as intendent in the environment where I have them deployed.

danpat avatar Jan 28 '22 17:01 danpat

✔️ Deploy Preview for stoplight-elements ready!

🔨 Explore the source changes: 2eead18ee0592a33b42e8f86ac60358b370b2c39

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoplight-elements/deploys/61f42191205c000008081f10

😎 Browse the preview: https://deploy-preview-2006--stoplight-elements.netlify.app

netlify[bot] avatar Jan 28 '22 17:01 netlify[bot]

✔️ Deploy Preview for stoplight-elements-demo ready!

🔨 Explore the source changes: 2eead18ee0592a33b42e8f86ac60358b370b2c39

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoplight-elements-demo/deploys/61f42191ec359b000839f5ba

😎 Browse the preview: https://deploy-preview-2006--stoplight-elements-demo.netlify.app/

netlify[bot] avatar Jan 28 '22 17:01 netlify[bot]

✔️ Deploy Preview for stoplight-elements-dev-portal-storybook ready!

🔨 Explore the source changes: 2eead18ee0592a33b42e8f86ac60358b370b2c39

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoplight-elements-dev-portal-storybook/deploys/61f42191a08c73000781cb0e

😎 Browse the preview: https://deploy-preview-2006--stoplight-elements-dev-portal-storybook.netlify.app

netlify[bot] avatar Jan 28 '22 17:01 netlify[bot]

✔️ Deploy Preview for stoplight-elements ready!

🔨 Explore the source changes: f76c4299eba0aa6b82502cfbb622fb625590d72a

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoplight-elements/deploys/622a339bfc4f18000be29d62

😎 Browse the preview: https://deploy-preview-2006--stoplight-elements.netlify.app

netlify[bot] avatar Jan 28 '22 17:01 netlify[bot]

✔️ Deploy Preview for stoplight-elements-demo ready!

🔨 Explore the source changes: f76c4299eba0aa6b82502cfbb622fb625590d72a

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoplight-elements-demo/deploys/622a339bdc55b00009ae93b9

😎 Browse the preview: https://deploy-preview-2006--stoplight-elements-demo.netlify.app/

netlify[bot] avatar Jan 28 '22 17:01 netlify[bot]

✔️ Deploy Preview for stoplight-elements-dev-portal-storybook ready!

🔨 Explore the source changes: f76c4299eba0aa6b82502cfbb622fb625590d72a

🔍 Inspect the deploy log: https://app.netlify.com/sites/stoplight-elements-dev-portal-storybook/deploys/622a339bdfaa3b0008f84f23

😎 Browse the preview: https://deploy-preview-2006--stoplight-elements-dev-portal-storybook.netlify.app

netlify[bot] avatar Jan 28 '22 17:01 netlify[bot]

We just ran into this today, amazing that this PR was already open. Thanks so much @danpat!

smoores-dev avatar Jan 28 '22 22:01 smoores-dev

Does @mmiask or @mnaumanali94 (or anyone else!) have any time to take a look at this? It would be really awesome to have this functionality!

smoores-dev avatar Feb 10 '22 15:02 smoores-dev

In the case @SMores and I are hitting it's actually not even a cross-origin request, but it looks like this project is using a fork of json-schema-ref-parser that a resolver that uses isomorphic-fetch and excludes credentials by default by passing 'omit'. If it just passed 'same-origin' as the default that would resolve our issue.

tilgovi avatar Feb 25 '22 22:02 tilgovi

I think withCredentials is a bit coarse for this use case because schemas might have references that span multiple domains. In practice, most folks are probably resolving from a single domain, but we can't assume that. I would bet the most common need for credentials is same-origin, though. If people want to set it to always send credentials, that's okay, but I can imagine wanting the 'same-origin' default, instead, so that I could have private schemas that reference public schemas and only send credentials for the private ones.

tilgovi avatar Feb 25 '22 22:02 tilgovi

@marbemac @lottamus How do y'all feel about this?

Nezteb avatar Mar 10 '22 17:03 Nezteb

Hi @danpat! Thanks for contributing to our OSS project! We'll work on getting this reviewed within the next couple weeks. I just wanted to keep you updated.

Nezteb avatar Mar 10 '22 17:03 Nezteb

@danpat is your need for actual cross-origin requests, or just same-origin requests with credentials?

When I dug into this, I determined that if the build were not using a fork of json-schema-ref-parser, but instead used webpack to provide Node.js polyfills for the browser via node-libs-browser, we'd be hitting code in stream-http that looks a lot like the resolver in the json-schema-ref-parser fork but defaults to same-origin rather than omit.

https://github.com/jhiesey/stream-http/blob/master/lib/request.js#L148

Since none of this withCredentials related stuff makes sense in a Node.js server environment, I imagine the webpack scenario is what json-schema-ref-parser might be expecting. But, I can't say for sure that someone else doesn't have a requirement to explicitly disable all credentials.

tilgovi avatar Mar 10 '22 22:03 tilgovi

@tilgovi Mine is a same-origin situation. Our setup is:

  1. I have elements being served from index.html at https://example.com/myschema/ <- all fetches to this require cookies to get through authentication logic
  2. I have mainschema.yaml at https://example.com/myschema/mainschema.yaml <- elements fetches this fine, cookies are included
  3. I have subschema.yaml at https://example.com/myschema/subschema.yaml <- this file is being referred to by mainschema.yaml like this:
components:
  schemas:
    SubSchema:
      $ref: "./subschema.yaml"

The core issue is that when the remote fetch for https://example.com/myschema/subschema.yaml happens, no cookies are included, so our authentication gateway intercepts the request and returns a 304 redirect to an authentication page.

danpat avatar Mar 10 '22 22:03 danpat

My team is having exactly the same problem as @danpat described -- we have an authentication proxy in a same-origin setup with an application that embeds Stoplight Elements. Requests issued by Stoplight lack the credentials: same-origin or credentials: include parameter, causing the authentication proxy to reject the request (redirecting to an authentication endpoint).

Any mechanism that adds pass a parameter to the Stoplight Elements React component letting us specify to use same-origin or include would resolve our problem.

By the principle of least privilege, we would prefer the ability to set same-origin (since we have a same-origin usecase) but include would work too.

As an aside, thanks for the fantastic PR @danpat -- this is exactly the kind of change I was looking for!

emanb29 avatar May 25 '22 19:05 emanb29

Looks like I am facing the same issue when serving apidocs via "Internal" github pages. With sub-schema reference it tries to hit github auth endpoint and fails with CORS access error.

aleskovets avatar Jul 05 '22 19:07 aleskovets

Is there any update on this?

marc-ste avatar Aug 30 '23 10:08 marc-ste

I still think this could be solved for most users by fixing the fork of json-schema-ref-parser. The fork being used here uses isomorphic-fetch and specifies omit as the default if credentials aren't requested.1. The default should be same-origin, which is what browsers do by default and what the webpack polyfill for the Node.js http module does.2

tilgovi avatar Sep 07 '23 17:09 tilgovi