-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Add config for trusted networks auth provider #21111
Add config for trusted networks auth provider #21111
Conversation
May I suggest a different syntax now that this is changed? May not be required, but id like to mention it. It would be nice to be able to map a range to a user. Also allow X-Forwarded-User header from some ip:s to map to hass user. So we'd need config options on each ip entry. Simplest might be to just make it a dict instead of list. |
IP / user mapping could be done in separate pull request. I want to keep PR small. I won't add X-Forwarded-User support, that should be done by another "trusted proxy" auth provider, not trusted networks auth provider |
I understand that. But I just meant change config structure to dict instead
of list.
|
@elupus I think it's fine. We can always add a new key |
has_trusted_networks = bool((config.get('http') or {}) | ||
.get('trusted_networks')) | ||
has_api_password = bool(config.get('http', {}).get('api_password')) | ||
trusted_networks = config.get('http', {}).get('trusted_networks') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we had an or
is because people can have a config like this:
http:
And then it can be None? Although if I remember correctly, we do replace None
with {}
…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trusted_networks
will be None
for the case
http:
My idea was just to be able to do:
But sure it's no problem to handle this in a different format. |
@elupus that feature will change several places in the login flow, not only in the config of trusted networks auth provider. A separate pull request is need. |
homeassistant/config.py
Outdated
|
||
mfa_conf = config.get(CONF_AUTH_MFA_MODULES, [ | ||
{'type': 'totp', 'id': 'totp', 'name': 'Authenticator app'}, | ||
]) | ||
_LOGGER.info('A Authenticator app MFA module is automatic configured') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's drop this one.
Description:
Add config for trusted networks auth provider.
Breaking changes
It is a breaking changes for user who manual configured trusted network auth provider. An invalid config error will be thrown and HA won't be able to fully started.
It is NOT a breaking changes for user who didn't manual configured trusted network auth provider.
If trusted_networks configured in http component, and no auth_provider configured in core config, http.trusted_networks value will be used for default loaded trusted network auth provider.
Related issue (if applicable): fixes #16149
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8605
Example entry for
configuration.yaml
(if applicable):Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices: