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

Configuration v2 fails due to CORS #446

Closed
jalmeroth opened this issue Dec 13, 2017 · 17 comments
Closed

Configuration v2 fails due to CORS #446

jalmeroth opened this issue Dec 13, 2017 · 17 comments
Assignees
Labels
Milestone

Comments

@jalmeroth
Copy link

jalmeroth commented Dec 13, 2017

I am trying to configure the latest dev-version of Homie 2 (f605adf).

When using the latest Configurator page v2 (03034c6) I am receiving an error in Chrome 63.0.3239.84 (Official Build) (64-bit) and other browsers:
Failed to load http://192.168.123.1/heart: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost:8000' is therefore not allowed access.

I could temporarily fix this issue by adding the following default headers after this line https://github.com/marvinroger/homie-esp8266/blob/develop/src/Homie/Boot/BootConfig.cpp#L18:

DefaultHeaders::Instance().addHeader(F("Access-Control-Allow-Origin"), F("*"));
DefaultHeaders::Instance().addHeader(F("Access-Control-Allow-Methods"), F("PUT, GET"));
DefaultHeaders::Instance().addHeader(F("Access-Control-Allow-Headers"), F("Content-Type, Origin, Referer, User-Agent"));

Please note: This is no the fix, it's just a proof to make it work temporarily.

Some of the following requests are also failing then as calls to __sendCORS() will send these headers again.

@timpur
Copy link
Contributor

timpur commented Dec 14, 2017

Ill look into a fix for CORS, but adding those lines wont help you here, because your origin is from 8000 (Self hosted webserver?) not the esp. Thus you need 8000 to allow CORS not the esp, because you cant even get to the esp in the first place? (in my understanding, might be wrong?)

Ill look into a solution for this. It would be nice to use the online config and not save to rely on the build in esp config, even though having it on the esp has other advantages, but thats up to you to decide and thats why ill try fix this.

@jalmeroth
Copy link
Author

Ill look into a fix for CORS, but adding those lines wont help you here, because your origin is from 8000 (Self hosted webserver?) not the esp. Thus you need 8000 to allow CORS not the esp, because you cant even get to the esp in the first place? (in my understanding, might be wrong?)

That's wrong, Access-Control-Allow-Origin: * allows access from everywhere, including my local webserver (not the esp itself).

Ill look into a solution for this. It would be nice to use the online config and not save to rely on the build in esp config, even though having it on the esp has other advantages, but thats up to you to decide and thats why ill try fix this.

Yeah, the online configurator is nice, but please keep the compatibility to checkout the gh-pages branch on your local maschine to configure homie devices, when you are offline or connected to the Homie Device AP. ;)

@marvinroger
Copy link
Member

Right, there’s a CORS issue. What I don’t get, though, is why it used to work.

The code was only handling preflight CORS request (OPTIONS), so the config endpoint was the only one that was supposed to work... anyway, the fix is pretty simple: it’s just a matter of wrapping each request into something that sends the CORS header, and it’lol work everywhere.

@timpur
Copy link
Contributor

timpur commented Dec 14, 2017

@jalmeroth

That's wrong, Access-Control-Allow-Origin: * allows access from everywhere, including my local webserver (not the esp itself).

but you want the webserver to tell the client (chrome) that its okay to go to the esp for requests ??? It wont work is you load the website from one server and then try access another webserver (ESP) with out the origin server giving CORS access??

@jalmeroth
Copy link
Author

@timpur well… as long as the AsyncWebserver, running on the ESP, is not sending these Access-Control-Allow-Headers, modern clients (e.g. Chrome) will not let you execute the JS XHR, which is in this case hosted on my local webserver, but could be anywhere else like the online configurator, and therefore the queries to the URLs necessary to configure your device will fail.

@timpur
Copy link
Contributor

timpur commented Dec 14, 2017

Seems my understanding of CORS was totally wrong.

@timpur
Copy link
Contributor

timpur commented Dec 14, 2017

But if you want @marvinroger i can fix this issue ?

@marvinroger
Copy link
Member

Yes @timpur, please, go ahead. 😉

@timpur
Copy link
Contributor

timpur commented Dec 14, 2017

@marvinroger assign this to me ? (how do you feel about making me a Collaborator ?)

@benzino77
Copy link
Contributor

benzino77 commented Dec 14, 2017

I think this is a great idea. @marvinroger you don't have so much time for the project as before and @timpur is one of the most active contributor which keep this project alive.

@marvinroger
Copy link
Member

marvinroger commented Dec 14, 2017 via email

@timpur timpur self-assigned this Dec 16, 2017
@timpur timpur added the bug label Dec 16, 2017
@tripflex
Copy link
Contributor

@timpur thank you for all your hard work on this, it's much appreciated! Hope to be able to start contributing myself very soon

@timpur timpur added this to the v2.1.0 milestone Dec 22, 2017
@timpur timpur mentioned this issue Jan 9, 2018
17 tasks
@tripflex
Copy link
Contributor

tripflex commented Jan 11, 2018

@jalmeroth thank you for this! I was having this issue while trying to call API endpoints using a Cordova mobile app, which does not work without the device handling CORS, and can confirm the update you provided fixed this issue for me

Yes I did look at potentially using a proxy to do this, but really CORS should be handled via the server, in the case being the device.

Being as though this is for the API endpoints, I don't see any reason to not be handling this on the device, @timpur can you please make sure to include this in the 2.1.0 release? I see it was added to the milestone, and think this is something we should def have in the codebase

@tripflex
Copy link
Contributor

tripflex commented Jan 11, 2018

Looking at the code itself, it appears that CORS handling already exists for /config and /wifi/connect ... so probably just need to set it for the other endpoints as well.

Adding those default headers makes it possible to get past CORS restrictions for GET requests without having to respond to OPTIONS

So looks like we would only need to add handling for HTTP_OPTIONS to call __sendCORS for the only other non-GET endpoint that does not already have it, /proxy/control

NOTE: You will still need to respond to the OPTIONS method for CORS pre-flight in most cases. (unless you are only using GET)

^ from ESPAsyncWebServer
Mozzila CORS Docs

@timpur
Copy link
Contributor

timpur commented Jan 12, 2018

@tripflex its fixed, did it last night (for me) all works, just haven't pushed to my repo, will do tonight.

All request have been opened to CORS, not really worried about security issues with that. Unless ive missed something?

@jalmeroth

timpur added a commit to timpur/homie-esp8266 that referenced this issue Jan 12, 2018
@tripflex
Copy link
Contributor

tripflex commented Jan 19, 2018

@timpur yup I tested myself using your 2.1 branch fork and no CORS issues at all! Thanks for fixing that so quick!

And no this is not an issue security wise, as CORS is really useful for preventing malicious scripts, browser extensions/plugins, or even other JS code from making requests to a server it should not be. Being as though this is for iot device, this should be open as the requests are almost always going to be coming from somewhere other than the device, not from the device itself.

Really this just helps prevent issues surrounding wanting to use something like Cordova to send API calls, or some custom javascript

I say this from my 10+ yrs experience as sys/net admin, and running a hosting company since 2009

@boneskull
Copy link

ha, I was just running into this. thanks @timpur

timpur added a commit to timpur/homie-esp8266 that referenced this issue Mar 18, 2018
@timpur timpur closed this as completed in 95d9566 Mar 18, 2018
euphi added a commit to euphi/homie-esp8266 that referenced this issue Mar 31, 2018
* 🔥 Remove hardcoded keepalive - fix homieiot#301 (homieiot#314)

Remove hardcoded KeepAlive for MQTT connection.
Default value of 15sec is already present in AsyncMQTT library

* 🐎 Improve uptime accuracy (homieiot#315)

improve uptime accuracy by storing milliseconds and only rounding when
publishing value
drawback: less time before rollover, but still long enough

* 🐛 Fix truncated IP (homieiot#318)

published IP is truncated.

* 🎨 Fix warning with parenthesis

* 🐛 Fix pio library.json bad dependency name

* ✨ Add support for static IP and BSSID/MAC, channel of AP (homieiot#327)

* Support for static IP and BSSID/MAC, Channel of AP

- added parameters to config.json which allow to define static ip, mask,
gateway, dns
- added parameters to config.json which allow to define BSSID and
channel of AP

To run device with defined static IP you have to define ip, mask and
gateway together.

To point device to connect to specific BSSID and channel you haveto
define bssid and channel together.

```
{

	"name": "The kitchen light",
	"device_id": "kitchen-light",
	"wifi": {
		"ssid": "Network_1",
		"password": "I'm a Wi-Fi password!",
		"bssid":
		"DE:AD:BE:EF:BA:BE",
		"channel": 1,
		"ip": "192.168.1.5",
		"mask": "255.255.255.0",
		"gw": "192.168.1.1"
	},
	"mqtt": {
		"host": "192.168.1.10",
		"port": 1883,
		"base_topic": "devices/",
		"auth": true,
		"username": "user",
		"password": "pass" i
	},
	"ota": {
		"enabled": true
	},
	"settings": {
		"param1": 55,
		"param2": "abcdefghijklm",
		"param3": true,
		"param4": false,
		"param5": 2147483647,
		"param6": -2147483647,
		"param7": 55,
		"param8": "abcdefghijklm",
		"param9": true,
		"param10": false
	}
}
```

* Addjustments for travis

* Another addjustments for travis

* Fix problem with prepareToSleep

* 🐎 Pass all callbacks by reference
Still store it by value

* 🐎 Make isActive const

* 🎨 Add custom settings value to initial log

* ✨ Abort if default setting value does not pass validator function
Fix homieiot#324

* 📝 Clarify ISSUE_TEMPLATE docs location for homieiot#331

* 📝 Implement new versioned Git docs (homieiot#341)

* ✨ Add versionned in-repo docs

* 🐛 Attempt to fix encrypted key

* 🐛 Fix permission issue on python exec

* 🐛 Add mission import

* 🐛 Fix relative path

* 🐛 Add missing commit

* ✨ Add index

* 🎨 Change index design

* 📝 Update all URLs to new docs

* 🎨 Fix broken doc link in README

* 📝 Add edit link to docs

* 📝 Move firmware_parser.py to scripts folder

* 🐛 Fix out of limits abort message not showing

* 👕 Be less strict on whitespace in comments

* 📝 Add dummy OTA updater script (TODO)

* 🎨 🐎 Use CircleCi instead of Travis CI (homieiot#348)

* ✨ Add Circle CI build

* 🐛 Add working_directory

* 🐛 Attempt to fix perm issue

* ✨ Generate docs from CircleCI

* 🐛 Fix path

* 🐛 Fix chmod permission

* 🐛 Try to run sudo

* 🐛 Attempt to use remote docker

* 🐛 Change bad directories

* 🐛 Remove dependency on Docker

* 🐛 Fix perm problem

* 🐛 Chamge tmp folder

* 🐎 Use CircleCI instead of Travis CI

* 🎨 Add SonoffDualShutters example

* 🐛 Install current lib to platformio

* 🐛 Actually ignore gh-pages in CI

* 🐛 Add missing SonoffDual dep

* 🐛 Fix ArduinoJSON 5.11.0 (homieiot#363)

* ⬆️ Update pio dependencies

* 🎨 Cleanup code a bit

* 🐛 Make sure every announcements packet are sent - closes homieiot#345

* 🎨 Refactor code and implement new OTA system
Closes homieiot#346

* 👕 Fix lint

* ✅ Use new workflow feature from Circle

* ✅ Ignore gh-pages at workflow level

* ✨ Add OTA_PROGRESS event

* 📝 Update docs for new OTA system

* ⬆️ Upgrade AsyncMqttClient dependency to 0.8.0

* 🐛 IHomieSetting::settings first, HomieNode-settings second! (homieiot#335)

*  IHomieSetting::settings first, HomieNode-settings second!

Set initialization priority of IHomieSetting::settings to value of highest allowed priority (101).

This allows other static variables to be of type HomieSetting<T>.

See https://github.com/euphi/HomieNodeCollection/blob/master/src/RGBWNode.cpp for example.

Note: As shown in the example it makes sense to have a static HomieSetting member, if you have a class that may be instantiated multiple times. (e.g. two LED Strips connected to the same ESP8266, as shown in https://github.com/euphi/ESP-LEDCtrl).

* Removed extra whitespaces

* Removed another whitespace

* 🐛 Fix not returning a value in setConfigurationApPassword (homieiot#378)

`HomieClass& HomieClass::setConfigurationApPassword(const char* password)` did not returned a reference to the Homie instance. This PR fixes this.

* 🐛 Fix crash when starting up without any defined node (homieiot#379)

* Fix crash when starting up without any defined node

Skip node publication if HomieNode::nodes.size() == 0

* Update BootNormal.cpp

* 🐛 Fix topic check for OTA upload (homieiot#375)

* Change topic check for OTA upload

* Change firmware topics to remove the 's'

* 🐛 Rename last OTA topic instance

* ✨ Add OTA updater script (homieiot#384)

* 🐍Add python ota updater script

* 💼 Update documentation of ota update script

* 😑 Add comments to ota updater script

* 🔮 Use 127.0.0.1:1883 as default broker setting

For the ota updater script

* 📝 Add details on how to interact with range property (homieiot#393)

Add hint to help people figure out how to interact with
range properties. Especially document the `_` separator.

* 📝 Add warning to input-handlers.md about concurrency (homieiot#400)

* 🐛 Interpret firmware file as an bytearray (homieiot#403)

Fix homieiot#397

* 📝 Update input-handlers.md (homieiot#401)

* Update input-handlers.md

Sorry, it seems that I used the `!!! warning` block in a wrong way.

* Update input-handlers.md

* Update input-handlers.md

* ✨ Use AsyncWebServer + Refactoring (homieiot#425)

* Initial AsyncWebServer

* Fixed Proccessing Body Requests (JSON)

* Doc Fixes + typo

* Added Missing Method in Timer.cpp

* Lots of Refactoring + Moved Reset Button to its own helper class for boots to use

* 🎨 Update BootNormal.cpp (homieiot#426)

Solve "else" errors from https://circleci.com/gh/marvinroger/homie-esp8266/136#tests/containers/0

* ⬆️ Update dependency to "ESP Async WebServer" (homieiot#434)

* 📝 Add instruction for @platformio (homieiot#435)

* Instruction for @platformio

* Explain how to used tagged version with @platformio

* Explain how to use tagged version with @platformio

* 🐛 Use v2.0.0-beta.2 as a working tagged example for @platformio (homieiot#437)

* 🎨 Simplify CI with @platformio (homieiot#438)

* 🐛 Install library via @platformio with all dependencies (homieiot#439)

* 💚 CI: Install staging version of Arduino Core for ESP8266 & @platformio (homieiot#440)

* 🐛 Pin Shutters dep version

* 🔥 Don't fail on CI docs step when testing a fork

* 🎨 More Refactoring + Deep Sleep + Prevent WiFi Reconnect when reboot (homieiot#432)

* Initial AsyncWebServer

* Fixed Proccessing Body Requests (JSON)

* Doc Fixes + typo

* Added Missing Method in Timer.cpp

* Lots of Refactoring + Moved Reset Button to its own helper class for boots to use

* Lots of Refactoring.

* Refactored ResetHandler + More Refactoring + Testing

* Added deep sleep function + Prevent Wifi reconnect before reboot (homieiot#380)[homieiot#380]

* Small typo fixes

* Small Rearange of code in BootConfig + Minor Refactor of function names

* Minior Commit to Triger a Github Action

* circleci build fix

* 🎨 First pass to fix linting

* 🎨 Second pass of lint

* 🎨 Final lint fix

* 📝 Update docs deps

* 📝 🎨 Update docs manifest

* 📝 🎨 Adjust HTTP JSON API doc

* 📝 Add configurators on website

* 📝 Update links to configurator

* Fix links under Features (homieiot#452)

* Set Device Stats Interval (homieiot#451) (homieiot#455)

* Add last step to uibundle README (homieiot#460)

* Add last step to uibundle README

* Added Arduino Support for doc

* Update updater script addressing quirks (homieiot#461)

* Update API for /wifi/connect from GET to PUT (homieiot#468)

Docs show `/wifi/connect` as `GET` when it should actually be `PUT`

* Proposal to optionally run HomieNode::loop() also in disconnected state

* Show Homie version

* Fix homieiot#446 homieiot#477 (homieiot#501)

* Fix homieiot#446 CORS Issue

* Fix for homieiot#477

* Fix Lint

* Fix Safari not displaying the config bundle HTML page (Fix homieiot#476) (homieiot#502)

* Fix Safari not displaying the config bundle HTML page

Safari cannot deal with gzip files that have a "*.gz" file extension.
Simply faking the filename solves the problem though.

* Update Readme Homie Version

* Fix Warnings (homieiot#503)

* Update Readme - Homie Convention

* Update to Homie Convention v2.0.1 (homieiot#507)
gorec2005 pushed a commit to gorec2005/homie-esp8266 that referenced this issue Jun 28, 2020
gorec2005 pushed a commit to gorec2005/homie-esp8266 that referenced this issue Jun 28, 2020
…s (live syntax checkers) to the ace editor - a standalone hack.

if you don't need worker(s), modify line homieiot#446 of edit.htm .setUseWorker(!0) to (!1) (true to false) repack by do.bat and modify update_ace.bat 
Changes of src/WebAuthentication.cpp and minor updates of my SmartSwitch example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants