search

CapnBry / HeaterMeter

HeaterMeter and LinkMeter Arduino BBQ Controller
https://tvwbb.com/forums/heatermeter-diy-bbq-controller.85/
MIT License
503 stars 83 forks source link

Access for historic data currently not possible because of CORS errors #44

Closed iGod42 closed 5 years ago

iGod42 commented 6 years ago

It is currently not possible to access the /lm/hist endpoint because of CORS errors. While the same issue is fixed with PR #43 for endpoints under /lm/api/, /lm doesn't have a setting to enable CORS. I'm not sure if adding CORS would be correct there, it seems like the endpoint should be moved (or aliased) in /lm/api.

CapnBry commented 6 years ago

You're right about this and #43, good catch on these. However, I noticed a couple of weeks ago that API write access doesn't work at all, which is a major blocker to fixing anything else. Are you not seeing that as well?

The problem is that luci/OpenWrt/LEDE doesn't have a spec for how the authentication scheme is supposed to work, with the only clue being in understanding how the source works. About 6 months ago (maybe more) they reorganized all the code in the dispatcher auth which prevents controllers from rolling their own session management which is how the apikey works. IIRC it (the dispatcher) is in a very weird state right now, which seemed to almost make creating your own auth impossible. I'm in the middle of updating everything back to OpenWrt since LEDE re-merged though so I can't do anything until that is done. They've included my squashfs stuff for our target but at the same time made some changes to it to make it incompatible with our current build. There's also a ton of hand testing for regressions because there literally a thousand changes between our last LEDE pull and OpenWrt HEAD.

I will take a look at this as soon as I'm done with that. (oof so much to do). I didn't initially move /lm/hist over due to the formatting, all the /lm/api stuff is JSON format only so I was planning on adding a new endpoint with the data in the right format so it won't have to be parsed on the client side any more. The API hasn't been a big priority though because there's almost nobody using it. I appreciate you finding these things though and I might have a chance to get some code time this week.

iGod42 commented 6 years ago

I haven't tried writing yet to be honest. I think I successfully placed one call in Insomnia but I might be mistaken since I initially set out to create an API that works on plain old Raspian. It could have been that too.

What I have done is have a look at the authentication logic and I'm not sure if the API key is really a great option. Here's why. I started to create a responsive page (that might evolve to a progressive web app at some point) for two reasons. 1. I'm not the biggest fan of the Luci user experience, especially on mobile. 2. I've been supplying HeaterMeters to my friends and family, not all of which are computer-literate. For them it would be great to have a zero-config way of just opening an app/web page that checks the local network for their unit and gives them the possibility to change the set point or other settings. For this to work though, they'd have to get the API key and supply it to the app. I haven't checked if they are randomly generated, but I assume they are, so each of them would have to do that.

Since there is no real sensitive data that we'd have to protect from being accessed, I think that basic auth, or even no auth at all, would be fine.

It sounds like you have a lot of work to go through, so for the issue at hand there's no rush at all, as I've got local workarounds. As for the authentication: If you really have to re-work stuff there maybe keep that in mind. Or if there's an easy option I'm missing, please let me know

minorOffense commented 5 years ago

Any chance the CORS patches code can be rolled into a release? I’m building a VueJS web app to replace the stock cook UI (just reading data for now, eventually to write data and replace those config pages too). I worked around it by breaking the cors security settings in Firefox. But it would help a lot if the app had the headers in place.

CapnBry commented 5 years ago

Ah son of a... I thought I had integrated this already. I was wondering why I had left the ticket open and I thought it was just to remind me that the luci dispatcher was a bit weird and needed a new look.

Building a new snapshot right now with CORS enabled on /lm/hist and it will be up in a few minutes at https://heatermeter.com/dl/

minorOffense commented 5 years ago

Excellent thank you very much.