Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Wrangler dev https server #1485

Merged
merged 9 commits into from
Aug 12, 2020
Merged

Wrangler dev https server #1485

merged 9 commits into from
Aug 12, 2020

Conversation

jspspike
Copy link
Contributor

@jspspike jspspike commented Aug 6, 2020

This will make the wrangler dev use https for the local server on gcs and edge. Edge can take in --http option to use http instead. Borrows some stuff from #1445

Closes #1204

@jspspike jspspike requested a review from a team as a code owner August 6, 2020 23:59
@jspspike jspspike changed the title Josh/dev https Wrangler dev https server Aug 7, 2020
src/commands/dev/edge/server/https.rs Outdated Show resolved Hide resolved
src/commands/dev/edge/server/https.rs Outdated Show resolved Hide resolved
src/commands/dev/gcs/server.rs Outdated Show resolved Hide resolved
src/commands/dev/gcs/server.rs Outdated Show resolved Hide resolved
src/commands/dev/mod.rs Outdated Show resolved Hide resolved
src/commands/dev/tls/certs.rs Show resolved Hide resolved
src/commands/dev/tls/certs.rs Outdated Show resolved Hide resolved
src/commands/dev/tls/certs.rs Show resolved Hide resolved
src/commands/dev/tls/certs.rs Outdated Show resolved Hide resolved
@jspspike
Copy link
Contributor Author

jspspike commented Aug 11, 2020

So after talking about it with @EverlastingBugstopper we thought about how to structure https with dev. With dev there's the local server which accepts your requests and there's what you upstream to. We were thinking that making https default for both local and upstream would add a lot of friction to users when they run into browser warnings and curl failing without --insecure. Plus it seems most use cases won't use this. Instead making http default for both would have less friction and for other use cases we can add "local-https" and "upstream-https" cli argument (and wrangler.toml argument). Https will opt where those users can deal with the friction. This covers use cases where users rely on https behavior or if someone has a host that uses https but wants their local to be http. We just need to make sure we have good docs about the new arguments because it's not really apparent what they mean at first glance.

@kentonv
Copy link
Member

kentonv commented Aug 11, 2020

Instead making http default for both

To clarify... Are you saying that wrangler dev would by default connect to Cloudflare's edge via HTTP, not HTTPS?

I think that would be a dangerous default. We should be encrypting all network traffic unless the user very explicitly tells us not to.

Moreover, most sites these days enable "always use HTTPS" which causes all HTTP requests to return a redirect to HTTPS, which would likely lead to confusion when using wrangler dev.

I do agree that traffic to localhost should not use HTTPS since there's no such thing as a valid certificate for localhost, and there's also no possibility of MITM.

@jspspike
Copy link
Contributor Author

To clarify... Are you saying that wrangler dev would by default connect to Cloudflare's edge via HTTP, not HTTPS?

I think that would be a dangerous default. We should be encrypting all network traffic unless the user very explicitly tells us not to.

That's what I was intending but this makes sense. Local http and upstream https by default sound good? @EverlastingBugstopper @ashleymichal

@EverlastingBugstopper
Copy link
Contributor

EverlastingBugstopper commented Aug 11, 2020

the following configurations should be possible. when i say upstream i mean cloudflare's edge OR the --host specified for gcs dev, and when i say local i mean the protocol on the developers machine

upstream https + local http (default)
upstream https + local https
upstream http + local http


upstream http + local https should not be possible i think

src/commands/dev/edge/mod.rs Outdated Show resolved Hide resolved
src/commands/dev/mod.rs Outdated Show resolved Hide resolved
src/commands/dev/mod.rs Outdated Show resolved Hide resolved
src/commands/dev/server_config/mod.rs Outdated Show resolved Hide resolved
src/commands/dev/tls/certs.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super close! just that one thing - i have a meeting coming up after that ill check it out and do some QA and then im ready to merge this up.

Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fantastic work on this

@EverlastingBugstopper EverlastingBugstopper added the dev `wrangler dev` label Aug 12, 2020
@jspspike jspspike deleted the josh/dev-https branch August 12, 2020 21:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dev `wrangler dev`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[dev] HTTPS support
4 participants