farmOS / field-kit

A modular, offline-first companion app to farmOS.
https://farmOS.org
GNU General Public License v3.0
60 stars 39 forks source link

Remove procedure for authorizing with http #354

Closed jgaehring closed 4 years ago

jgaehring commented 4 years ago

Right now we have a catch block that runs if any requrest to authorize via https fails. It basically repeats the same request except via http, instead of https:

https://github.com/farmOS/farmOS-client/blob/93595be0345e1771805ff9b239866922a4f8a4cd/src/core/store/authModule.js#L68-L91

This was kind of a brute force way of ensuring the user's server wasn't using http, since we don't ask for that information from the user. However, since the time we implemented this, we've made it a requirement to have an SSL cert and use https on your server, as stated in the docs.

I suggest then that we eliminate this catch block and just error out if https fails, since the call via http seems futile at this point. It's a good-sized chunk of code we're maintaining, and unnecessary duplication, imo. We may be able to authorize the user, but we can't guarantee the rest of the app's functionality at that point, so I think it's fair to just let it fail outright.

@mstenta, what are your thoughts?

mstenta commented 4 years ago

I'm in favor of removing the logic.

Question: how are you testing locally? Do you have a self-signed SSL certificate?

jgaehring commented 4 years ago

I'm in favor of removing the logic.

Cool, I'm just going to lump this in with the OAuth PR then, just to get this taken care of right away.

Question: how are you testing locally? Do you have a self-signed SSL certificate?

No, I just run on http. Browsers make an exception for http://localhost in cases where they would normally require https://.

mstenta commented 4 years ago

Browsers make an exception for http://localhost in cases where they would normally require https://.

Oh didn't know that!

But won't the request be made to "https://localhost" in that case? When I put that into Firefox (without a self-signed cert set up) it gives me "Unable to connect". So if Field Kit is making requests to "https://localhost" I would expect the same to happen, because there is no server configured to listen on port 443 locally.

jgaehring commented 4 years ago

But won't the request be made to "https://localhost" in that case?

Nope, everything stays on http://localhost. It just makes an exception for those requests, and for access to certain browser API's like geolocation, which usually requires https.

mstenta commented 4 years ago

But in the Field Kit Login screen we only type in "localhost", with "https://" assumed. So I would assume that means it will make requests to "https://localhost", no? Am I misunderstanding?

jgaehring commented 4 years ago

The special sauce:

https://github.com/farmOS/farmOS-client/blob/38b642d1b5b820c9fc31c544725e16c945e11553/src/core/store/authModule.js#L14-L16

So in development, the base url is just an empty string, so all requests are essentially relative addresses. Then the proxy server picks up any relative address:

https://github.com/farmOS/farmOS-client/blob/38b642d1b5b820c9fc31c544725e16c945e11553/config/index.js#L14-L22

mstenta commented 4 years ago

Ah ha! That's what I was looking for. Makes sense!

jgaehring commented 4 years ago

Resolved by #350