nextstrain / nextstrain.org

The Nextstrain website
https://nextstrain.org
GNU Affero General Public License v3.0
88 stars 49 forks source link

Allow read-only CORS requests for public resources #658

Closed tsibley closed 1 year ago

tsibley commented 1 year ago

This makes it possible, for example, to do client-side analyses on the dataset JSONs using standard nextstrain.org URLs. Requested by @corneliusroemer¹, as it makes it much easier to load arbitrary trees into Nextclade web.

It's also nice in principle to allow cross-origin requests for public resources since it enables unforeseen integrations without having to ask permission.

I didn't use the "cors" library here since it does a lot more than what we need, is more permissive by default, and setting our own headers is less of a blackbox and thus clearer as to what we're sending and when.

¹ https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1666027479770509 https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1676473418174269

Testing

corneliusroemer commented 1 year ago

It looks good to me - though I don't have the knowledge to do a security audit. It sounds like you checked the headers - if there are no other changes at least things shouldn't blow up. I'll test Nextclade as soon as this is live.

tsibley commented 1 year ago

@corneliusroemer Thanks! You should be able to test this PR with Nextclade using URLs at https://nextstrain-s-trs-cors-ppot6sgp.herokuapp.com/.

tsibley commented 1 year ago

I verified it worked by:

  1. Visiting https://nextclade-git-feat-composite-fitness-nextstrain.vercel.app/?dataset-name=sars-cov-2-21L&input-tree=https://nextstrain-s-trs-cors-ppot6sgp.herokuapp.com/staging/nextclade/sars-cov-2/21L
  2. Clicking "Load example" (next to the "Run" button) in the "Add more sequence data" section
  3. Clicking "Run"
  4. Observing successful analysis without error modal from Nextclade, and a network request in the browser dev tools.
tsibley commented 1 year ago

(Note that you don't need to use the /charon/getDataset?prefix= URLs.)