Skip to content

Conversation

@mumme74
Copy link
Contributor

@mumme74 mumme74 commented Apr 3, 2021

Add https://
This implements an option to use https and a way to simply create self signed certificate and private key.
You can of course use a custem key-cert pair if you want.

Many things today need https to be activated, including sensor suite for mobiles.

refs: #113, #8

My rationale for wanting this pull req, even though chrome apps discontinue:
I would very much like this to be merged even though apps is not supported after 2022-06
I plan to use this webapp when I'am teaching. Students all have chromebooks and can't find much simpler way to serve https content. Like a webpage that responds to a mobile's accelerometer.
At least we have a year before its closed down.

Having https in the app in chromestore makes it much easier when students must work on there own at home (due to the pandemic)

@kzahel
Copy link
Owner

kzahel commented Apr 3, 2021

Wow! This is really exciting.
I tried your branch but ran into this problem when trying to load the page.

This site can’t provide a secure connection
127.0.0.1 didn’t accept your login certificate, or one may not have been provided.
Try contacting the system admin.
ERR_BAD_SSL_CLIENT_AUTH_CERT

@kzahel
Copy link
Owner

kzahel commented Apr 3, 2021

Ah, duh. I didn't click on the button to generate the key! A small suggestion, I think it would be an improvement if clicking the "https" checkbox would show/hide the certificate info. That way I would know when clicking it that I need to do something there.

This is awesome! Thanks so much for the PR. I'll test it a little bit more, merge and release soon.

@kzahel
Copy link
Owner

kzahel commented Apr 3, 2021

One issue I see (not sure if you see it) is when I click generate crypto, the placeholder label on the key input fields overlays the key, until you type something into the field. Not a blocker, but might be good to see if it's possible to fix somehow

out.mp4

@mumme74
Copy link
Contributor Author

mumme74 commented Apr 4, 2021

Thank you for the quick response and all the suggested inprovements!

I have now done the suggested changes, and implement hide/show https fields.
I have never used react ui before so I hope I didn't do something obviously wrong.

Regarding the textbox hint issue in your movie, I can't see that one here. But I think I found a workaround for a possible cause.

Very glad to hear that you are willing to merge this and update appstore! 👍 😀

I hope that the code now is in a mergable state with my last push! #3e20c13

Copy link
Owner

@kzahel kzahel left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! looks great.

@kzahel kzahel merged commit 7ee08df into kzahel:master Apr 6, 2021
@kzahel
Copy link
Owner

kzahel commented Apr 6, 2021

Just submitted the new version for review! If you see version 0.5.2 in the store (sometimes review happens really quick) it should have https working 🤞

@mumme74
Copy link
Contributor Author

mumme74 commented Apr 6, 2021

Perfect!

Thank you!

@kzahel
Copy link
Owner

kzahel commented Apr 7, 2021

It's in the store and I just tested it and it works on my chromebook.
Interestingly on a Mac device it doesn't have the "Proceed ... (unsafe)" button. But after looking on stack overflow it looks like there's a "secret code" if you type

"thisisunsafe" into the error page, it allows you to see the content.

Haha. I wonder if it's worthwhile mentioning that somewhere near the "enable https" button (if they're on a mac at least) 🤷

@mumme74
Copy link
Contributor Author

mumme74 commented Apr 12, 2021

Nice ! 😄

Regarding MAC, yes perhaps you are right, I would have to try when I get hold of a MAC.
Is it safari or chrome or both on MAC? Whole thing seems strange 🤨

It might be related to node-forge not supporting TLS 1.2 ⁉️ 🤔
I am working on a patch for node-forge to support TLS1.2 although not yet finished. 🙄
https://github.com/mumme74/forge/tree/tls12

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.

2 participants