Webhook component - pass headers to webhook handler #17091
Conversation
homeassistant
added
cla-signed
integration: webhook
small-pr
ghost
added
the
homeassistant/components/webhook.py
Outdated
| @@ -85,7 +85,7 @@ class WebhookView(HomeAssistantView): | |||
| return Response(status=200) | |||
|
|
|||
| try: | |||
| response = await handler(hass, webhook_id, data) | |||
| response = await handler(hass, webhook_id, data, headers) | |||
kirichkov
force-pushed
the
webhook-pass-request
branch
from
55f28b2
to
c2789a8
Compare
kirichkov
force-pushed
the
webhook-pass-request
branch
from
c2789a8
to
7ca18ad
Compare
| @@ -85,7 +85,7 @@ class WebhookView(HomeAssistantView): | |||
| return Response(status=200) | |||
|
|
|||
| try: | |||
| response = await handler(hass, webhook_id, data) | |||
There was a problem hiding this comment.
I suggest that we drop data and just pass request
There was a problem hiding this comment.
OK. I'll clean up the rest of webhook and make the necessary changes in ifttt and update the tests (if needed)
There was a problem hiding this comment.
I just looked at the code. If we drop data altogether, each component that depends on webhook will have to implement the JSON validation on its own. Upside is that the webhook component can be used to accept calls that submit non-JSON data.
There was a problem hiding this comment.
Yeah I think that is fine . Gives most flexibility.
kirichkov
changed the title
tests/components/test_webhook.py
Outdated
| assert hooks[0][2] == { | ||
| 'data': True | ||
| } | ||
| # assert await hooks[0][2].text() == '{\'data\':true}' # This line raises PayloadAccessError |
kirichkov
force-pushed
the
webhook-pass-request
branch
from
09684a4
to
29250fb
Compare
| except ValueError: | ||
| _LOGGER.warning( | ||
| 'Received webhook %s with invalid JSON', webhook_id) | ||
| return Response(status=200) |
There was a problem hiding this comment.
just return None. webhook defaults to this.
tests/components/test_webhook.py
Outdated
| 'data': True | ||
| } | ||
| # The line below causes PayloadAccessError | ||
| # assert await hooks[0][2].text() == '{\'data\':true}' |
There was a problem hiding this comment.
You can't just comment it out 😉 Instead, just extract the info inside handle and store it in hooks
There was a problem hiding this comment.
Obviously. My intention was to put a comment here, and ask for advice, but Travis is taking longer than usual :-)
| import logging | ||
| from urllib.parse import urlparse | ||
|
|
||
| from aiohttp.web import Response |
There was a problem hiding this comment.
'aiohttp.web.Response' imported but unused
kirichkov
force-pushed
the
webhook-pass-request
branch
from
184d5b4
to
eb8467c
Compare
|
Nice! 🐬 🎉 |
Description:
I'm suggesting to pass the headers (or the whole request object) to the webhook handler, besides the data and the
webhook_id.As I'm working on switching OwnTracks HTTP to use the webhooks component, I found out that the webhook component will pass to the handler only the data that is POST-ed by the OwnTracks app. The app, however, posts some necessary data as the username and the device ID as part of the headers, and the username and device ID are needed to match the device with the
known_devices.yamlentry.If I have access to the headers I can extract the necessary data automatically, and not require the user to create separate webhook IDs for each device they want to add.
If this suggestion is accepted, I'll need to update the IFTTT component too.Related issue (if applicable): home-assistant/architecture/issues/80, /pull/16817
Checklist:
tox. Your PR cannot be merged unless tests pass