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

Iodide should not hardcode server addresses #1943

Closed
wlach opened this issue Jun 14, 2019 · 3 comments
Closed

Iodide should not hardcode server addresses #1943

wlach opened this issue Jun 14, 2019 · 3 comments

Comments

@wlach
Copy link
Contributor

wlach commented Jun 14, 2019

Right now we hardcode server addresses in the iodide .js during the webpack phase of the build:

let { EVAL_FRAME_ORIGIN } = process.env;

To deploy internally at Mozilla (where we use a docker container), we set the circle environment for these two variables at build time. This has a number of disadvantages:

  1. If the origins ever change (as just happened with GCP), we need to remember to update the CircleCI configuration from their interface (very easy to forget)
  2. We can't use the same docker container for a production and stage version of the iodide server.

We should somehow fix the webpack builds to not require hardcoding this information. Not sure offhand what the right set of solutions here is, hopefully it's not too complicated.

/cc @bcolloran @robotblake

@bcolloran
Copy link
Contributor

The domains are needed to verify the that the communications between the eval-frame and the editor come from the correct hosts. since we're pretty much all-in on the server at this point, i guess we could have django inject the correct domains when it serves the pages (as we do with other NB info) -- they'd still need to be configured in .env for each deployment, but you wouln't have to re-build. so questions then:

  1. would that solve this use case?
  2. would serving the host info via django rather than hardcoding it at build time be less secure? (i assume not since we serve all kinds of secure token things this way using django?)
  3. how big of a deal is this? is it worth a day of someone's time to fix it?

one other thing: if we don't set the host domains at build time, we'd need to fully deprecate start-and-serve mode. i don't use that anymore ever, but it's worth noting.

also cc @hamilton and @mdboom re the start-and-serve bit

@wlach
Copy link
Contributor Author

wlach commented Jun 15, 2019

On some level, yeah, we need this information to come from Django. No, I don't think it's any less secure: If you can't trust what Django is sending you, well, you basically can't trust anything. There's almost certainly a way to keep start-and-serve working with this approach, I suspect it's something we can just add to the webpack config.

I would say this is pretty much something we have to fix before we can put this into production internally. Our internal configuration is already quite complex and has many moving pieces and assumptions, for sure this will come back to bite us again (perhaps at an inopportune time) if we don't fix it sooner.

@wlach
Copy link
Contributor Author

wlach commented Jul 2, 2019

wlach added a commit that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for #1943 ("Iodide should not hardcode server assets").
wlach added a commit that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for #1943 ("Iodide should not hardcode server assets").
wlach added a commit that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for #1943 ("Iodide should not hardcode server assets").
wlach added a commit that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for #1943 ("Iodide should not hardcode server assets").
wlach added a commit to wlach/iodide that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for iodide-project#1943 ("Iodide should not hardcode server assets").
wlach added a commit to wlach/iodide that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for iodide-project#1943 ("Iodide should not hardcode server assets").
wlach added a commit to wlach/iodide that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for iodide-project#1943 ("Iodide should not hardcode server assets").
wlach added a commit to wlach/iodide that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for iodide-project#1943 ("Iodide should not hardcode server assets").
wlach added a commit to wlach/iodide that referenced this issue Jul 15, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for iodide-project#1943 ("Iodide should not hardcode server assets").
wlach added a commit to wlach/iodide that referenced this issue Jul 18, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for iodide-project#1943 ("Iodide should not hardcode server assets").
wlach added a commit that referenced this issue Jul 18, 2019
This will allow us to swap in metadata used by the eval frame on the server
side, without recompiling the webpack assets. It is a necessary prerequisite
for #1943 ("Iodide should not hardcode server assets").
wlach added a commit that referenced this issue Jul 19, 2019
Instead we will have the iodide server inject these properties
wlach added a commit that referenced this issue Jul 19, 2019
Instead we will have the iodide server inject these properties
wlach added a commit to wlach/iodide that referenced this issue Jul 19, 2019
…e-project#1943)

Instead we will have the iodide server inject these properties
wlach added a commit to wlach/iodide that referenced this issue Jul 19, 2019
…e-project#1943)

Instead we will have the iodide server inject these properties
wlach added a commit to wlach/iodide that referenced this issue Jul 22, 2019
…e-project#1943)

Instead we will have the iodide server inject these properties
@wlach wlach closed this as completed in 2fe99ba Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants