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

Adding HTTPS on Prod version #214

Merged
merged 20 commits into from
Oct 30, 2020
Merged

Adding HTTPS on Prod version #214

merged 20 commits into from
Oct 30, 2020

Conversation

RichtXO
Copy link
Member

@RichtXO RichtXO commented Oct 4, 2020

Features Added

I added certs and open both port 80 and 443. Port 80 will redirect to port 443. Also made it auto-generate the OpenSSL certificate every time it reruns.

Additional Info

https://tech.osteel.me/posts/docker-for-local-web-development-part-5-https-all-the-things#generating-the-certificate
https://www.digitalocean.com/community/tutorials/how-to-create-a-self-signed-ssl-certificate-for-nginx-in-ubuntu-16-04

Added certs and open both port 80 and 443. Port 80 will redirect to port 443.
@RichtXO RichtXO linked an issue Oct 4, 2020 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Oct 4, 2020

Deploy Link: http://45.33.71.174/
Branch and Build Info: http://45.33.71.174:3000/info.txt
Expected Time Ready: Sat Oct 3 20:32:27 EDT 2020

@123joshuawu
Copy link
Member

The autogenerating openssl is cool

Seems like the redirect is redirecting to localhost:443 which isn't working on the pr-deploy

Also, just wondering, how would we specify custom ssl certificates?

@RichtXO
Copy link
Member Author

RichtXO commented Oct 4, 2020

Oh, that's weird... I'll double-check with my end on this.
Just wondering, did you open 443 for the thing that generates the deploy link?

@123joshuawu
Copy link
Member

Ah I did not, only the :80 to :443 redirect

@RichtXO
Copy link
Member Author

RichtXO commented Oct 4, 2020

Wait, what does that mean?

@123joshuawu
Copy link
Member

So the issue is the pr-deploy is not overriding the $HOST env var for the web container so it defaults to localhost. Thus when you go to port 80, it returns a 301 redirect to https://localhost:443 which fails

@RichtXO
Copy link
Member Author

RichtXO commented Oct 4, 2020

So now said that's a deployment issue? Nothing wrong with the code. It's just that this deployment thing isn't (like you said) not overriding.

@123joshuawu
Copy link
Member

Yes, unfortunately we'll need to fix that before we merge or its going to break on subsequent pull requests. tagging @jshom re: the pr-cd workflow

@github-actions
Copy link

github-actions bot commented Oct 6, 2020

Deploy Link: http://173.255.231.178/
Branch and Build Info: http://173.255.231.178:3000/info.txt
Expected Time Ready: Tue Oct 6 17:26:54 EDT 2020

Added certs and open both port 80 and 443. Port 80 will redirect to port 443.
@github-actions
Copy link

github-actions bot commented Oct 6, 2020

Deploy Link: /
Branch and Build Info: :3000/info.txt
Expected Time Ready: Tue Oct 6 17:33:39 EDT 2020

@jshom
Copy link
Contributor

jshom commented Oct 6, 2020

Niiiiiiiiiice! Good stuff man! 🎊

Two small tweaks to get this to work with our temp and other environments:

  1. PR-CD can be fixed by going around redirects through providing link in github as HTTPS instead of HTTP in https://github.com/YACS-RCOS/yacs.n/blob/master/ops/provision.js#L73
  2. Link certificates through docker volume for prod deploy and other environments, if these exist, use existing instead of self-signed certificate

@github-actions
Copy link

github-actions bot commented Oct 6, 2020

Deploy Link: https://23.239.14.246/
Branch and Build Info: https://23.239.14.246:3000/info.txt
Expected Time Ready: Tue Oct 6 17:52:31 EDT 2020

Added certs and open both port 80 and 443. Port 80 will redirect to port 443.
@jshom
Copy link
Contributor

jshom commented Oct 6, 2020

oi, the build info link... this is fine for now, will make another issue for this, no need to fix it in this PR (this would need to stick to http)

@RichtXO
Copy link
Member Author

RichtXO commented Oct 6, 2020

Yeah. But this is weird. I think 3 days ago, I couldn't get into port 443 on the old URL: 45.33.71.174. I typed the URL with the HTTPS and didn't go through. It may have been my cookies.

@jshom
Copy link
Contributor

jshom commented Oct 6, 2020

Yeah. But this is weird. I think 3 days ago, I couldn't get into port 443 on the old URL: 45.33.71.174. I typed the URL with the HTTPS and didn't go through. It may have been my cookies.

Ye maybe, I think that could have been a flaky build too.

@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Oct 9, 2020
@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Oct 9, 2020
@github-actions
Copy link

github-actions bot commented Oct 9, 2020

Deploy Link: http://45.79.183.91/
Branch and Build Info: http://45.79.183.91:3000/info.txt
Expected Time Ready: Fri Oct 9 17:31:27 EDT 2020

@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Oct 14, 2020
@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Oct 14, 2020
@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Oct 14, 2020
@github-actions
Copy link

Deploy Link: https://45.79.171.4/
Branch and Build Info: https://45.79.171.4:3000/info.txt
Expected Time Ready: Wed Oct 14 11:20:03 EDT 2020

@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Oct 14, 2020
@RichtXO RichtXO requested a review from 123joshuawu October 14, 2020 15:12
Copy link
Member

@123joshuawu 123joshuawu left a comment

Choose a reason for hiding this comment

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

I'm bad with bash and certificates so ill defer to @jshom but looks good to me

@jshom
Copy link
Contributor

jshom commented Oct 14, 2020

Small thing from a glance, seeing file src/web/certificate/READMD.md changed :) did u mean src/web/certificate/README.md. Gonna test with custom cert in a bit, sorry bout the delay on the review here. Looks great!

@RichtXO
Copy link
Member Author

RichtXO commented Oct 14, 2020

Small thing from a glance, seeing file src/web/certificate/READMD.md changed :)

What changed on the README? I was planning to add more stuff on how to obtain a cert and issue it to a CA for free.

did u mean src/web/certificate/README.md. Gonna test with custom cert in a bit, sorry bout the delay on the review here.

It's alright! I didn't know if what I implemented was what you guys had in mind!

Added certs and open both port 80 and 443. Port 80 will redirect to port 443.
@jshom
Copy link
Contributor

jshom commented Oct 30, 2020

@jshom
Copy link
Contributor

jshom commented Oct 30, 2020

WOOOHOOO, thing works. Gonna open new issue to address nginx tweaks for protocols & ciphers to address the test.

Copy link
Contributor

@jshom jshom left a comment

Choose a reason for hiding this comment

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

After removal of the csr check line for provided key/cert, and small whitespace fixes; good for merge!

src/web/scripts/entrypoint.sh Outdated Show resolved Hide resolved
src/web/nginx.conf Show resolved Hide resolved
ops/provision.js Show resolved Hide resolved
@github-actions
Copy link

Deploy Link: https://45.33.88.96/
Branch and Build Info: https://45.33.88.96:3000/info.txt
Expected Time Ready: Fri Oct 30 16:47:11 EDT 2020

@jshom jshom mentioned this pull request Oct 30, 2020
@github-actions
Copy link

Deploy Link: https://97.107.142.48/
Branch and Build Info: https://97.107.142.48:3000/info.txt
Expected Time Ready: Fri Oct 30 17:28:35 EDT 2020

@123joshuawu 123joshuawu self-requested a review October 30, 2020 21:23
@RichtXO RichtXO merged commit dc82c3d into master Oct 30, 2020
@RichtXO RichtXO deleted the HTTPS branch October 30, 2020 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prod - Secure with https
3 participants