Stop requiring home-assistant authentication for meraki #34752
Conversation
probot-home-assistant
bot
added
small-pr
jbergler
changed the title
The Meraki Scanning API is configured with a shared secret that are part of every request the Meraki Cloud makes to configured receivers. As such we can allow requests to this integration to go without authentication by homeassistant. It's worth noting that the GET method returns a static value that should not be considered a secret and thus it is ok not to be protected. Validation serves to ensure that the receiver you are configuring in the Meraki Dashboard is ready to receive requests. This change also refactors the secret validation logic slightly to improve readability.
MartinHjelmare
changed the title
|
In my travels since opening this PR, I stumbled across a conversation here #15376 (comment) which suggests that the meraki implementation should switch to the webhook api's. I haven't been able to find good documentation, but (at least superficially) it would seem that this approach does the same thing the webhook component does (disable api auth) |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
|
When this PR will be merged ? I already integrate the change on my setup in a custom component but I would prefer to have it integrate officially. |
| def validate_secret(self, secret): | ||
| """Validate shared secret.""" | ||
| if not secret: | ||
| _LOGGER.error("no secret supplied") |
There was a problem hiding this comment.
Please start logging messages with capital letter.
| _LOGGER.error("no secret supplied") | |
| _LOGGER.error("No secret supplied") |
| @@ -43,8 +48,22 @@ def __init__(self, config, async_see): | |||
| self.validator = config[CONF_VALIDATOR] | |||
| self.secret = config[CONF_SECRET] | |||
|
|
|||
| def validate_secret(self, secret): | |||
| """Validate shared secret.""" | |||
| if not secret: | |||
There was a problem hiding this comment.
We should move this check to the caller of this method. We shouldn't pass an empty secret or False as secret to this method.
| _LOGGER.error("invalid secret supplied") | ||
| _LOGGER.debug("received secret %s", secret) |
There was a problem hiding this comment.
We shouldn't log secrets.
| _LOGGER.error("invalid secret supplied") | |
| _LOGGER.debug("received secret %s", secret) | |
| _LOGGER.error("Invalid secret supplied") |
| _LOGGER.debug("received secret %s", secret) | ||
| return False | ||
|
|
||
| _LOGGER.debug("valid secret") |
There was a problem hiding this comment.
| _LOGGER.debug("valid secret") | |
| _LOGGER.debug("Valid secret") |
| async def get(self, request): | ||
| """Meraki message received as GET.""" | ||
| _LOGGER.info("testing") |
There was a problem hiding this comment.
| _LOGGER.info("testing") |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Breaking change
None
Proposed change
The Meraki Scanning API is configured with a shared secret that are part of every request the Meraki Cloud makes to configured receivers.
As such we can allow requests to this integration to go without authentication by homeassistant.
It's worth noting that the GET method returns a static value that should not be considered a secret and thus it is ok not to be protected. Validation serves to ensure that the receiver you are configuring in the Meraki Dashboard is ready to receive requests.
This change also refactors the secret validation logic slightly to improve readability.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following [Integration Quality Scale][quality-scale]: