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

alexa: fix device discovery / state callback #2388

Merged
merged 4 commits into from Oct 28, 2020
Merged

Conversation

aL1aL7
Copy link
Contributor

@aL1aL7 aL1aL7 commented Oct 26, 2020

alexa device discovery/state calls use URL of type
/api/hash_id_key_whatever/lights

This is blocked when API is disabled and fails
also when http API is enabled because no apikey is
sent by amazon calls

This commit grabs the "whatever Amazon Hash" during
device recognition and save it. All requets to URLS
starting with /api/hash_id_key_whatever are now
passing the auth check

alexa device discovery/state calls use URL of type
/api/hash_id_key_whatever/lights

This is blocked when API is disabled and fails
also when http API is enabled because no apikey is
sent by amazon calls

This commit grabs the "whatever Amazon Hash" during
device recognition and save it. All requets to URLS
starting with /api/hash_id_key_whatever are now
passing the auth check
@mcspr
Copy link
Collaborator

mcspr commented Oct 27, 2020

Check this out:
https://github.com/vintlabs/fauxmoESP/blob/20956d966a1c7c6a7e171f42f2a7d1410b1bef2d/src/fauxmoESP.cpp#L168-L192
Maybe we want to fix things there?

So we have twothree problems as far as I can gather:

  • we have ordering issue with ours /api prefix and need to ensure api.cpp ignores or straight up knows about the /api/+/lights
  • fauxmoesp internal logic rejects the endpoint since it expects numeric ID instead of hashstring
  • :edit: we still want to ignore auth even if we use apiregister to ensure path is known

@aL1aL7
Copy link
Contributor Author

aL1aL7 commented Oct 27, 2020

Yes indeed, it would be better to move the check maybe to alexa.cpp and have only minimal changes in api.cpp ?

About topic 2:
I can't find any issue at fauxmoesp lib. When the api auth is passed, I get a correct json response from fauxmo and alexa shows correct device states / discovers devices. Is there any bug report or something else where I can see the problem?

EDIT: Do you have any idea about length of "amazon_hash" from "/api/amazon_hash/.." ? At my account it has a length of 40 bytes

@mcspr
Copy link
Collaborator

mcspr commented Oct 27, 2020

edit: ok, my bad twice, coffee time... it's meant to parse the stuff after the lights i.e. /api/+/lights/whatever, so it is doing the right thing

No idea where we might find the 'official' hue api, but http://www.burgestrand.se/hue-api/api/auth/registration/ refers to this as username and sets this value between 10 and 40 bytes.

It is a weird workaround either way, but I guess we will achieve the same thing by modifying web{Body,Request}Register (from web.cpp) to optionally prepend the callback instead of appending it, so alexa handler is called first

@mcspr
Copy link
Collaborator

mcspr commented Oct 27, 2020

And we can also assume the path starts with /api/2WLEDHardQrI3WHYTHoMcXHgEspsM8ZZRpSKtBQr?
https://github.com/vintlabs/fauxmoESP/blob/c31b0771ff4f7554cb6347ee5155d6941fbbb68b/src/fauxmoESP.cpp#L203

@aL1aL7
Copy link
Contributor Author

aL1aL7 commented Oct 27, 2020

Hmm,
the hash must be generated by amazon.
The hash is independent from device (I checked more devices with my alexa account -> always same hash) but differs from alexa account. At another alexa account you get a different hash.
So the hash ist not always the same

@aL1aL7
Copy link
Contributor Author

aL1aL7 commented Oct 27, 2020

Uuuh, I had also not enough coffee
I think it can be solved much more easier.

This little patch should fix alexa:
Check length of 14 because of "/api/" + mysterious hash with length 10 - 40
Would this be ok ?

--- a/code/espurna/api.cpp
+++ b/code/espurna/api.cpp
@@ -224,6 +224,8 @@ bool _apiRequestCallback(AsyncWebServerRequest* request) {
     }
 
     if (!url.startsWith("/api/")) return false;
+    if (url.indexOf("/lights") > 14 ) return false;
+
     if (!apiAuthenticate(request)) return false;
 
     return _apiDispatchRequest(url, request);

@mcspr
Copy link
Collaborator

mcspr commented Oct 27, 2020

For now, yes. With a comment notice + #if ALEXA_SUPPORT ... #endif around it. Isn't it ≥16 though? 5 bytes /api/, 10 bytes hash data and 1 byte slash before lights.

@aL1aL7
Copy link
Contributor Author

aL1aL7 commented Oct 27, 2020

EDIT: Ok, I will modify the pull request .

index > 14 is ok, because the slash at "/lights" is included and the if condition of the patch uses ">" and not ">="

String mytest = "/api/0123456789/lights";
int result = mytest.indexOf("/lights");

--> result = 15

aL1aL7 and others added 3 commits October 27, 2020 17:07
Don't try to use the http api for alexa calls.
Responses for alexa are done by fauxmoesp lib,
so there's no need for the http api call.
@mcspr mcspr merged commit f6582ec into xoseperez:dev Oct 28, 2020
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.

None yet

2 participants