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

Add trusted networks auth provider #15812

Merged
merged 3 commits into from
Aug 13, 2018

Conversation

awarecan
Copy link
Contributor

@awarecan awarecan commented Aug 3, 2018

Description:

Need to wait #15914 merged first

Add Trusted Networks Authentication Provider

It shows list of users if access from trusted network. User can select which user want to login, no password need

image

Breaking change:

Given new auth is enabled, current websocket API will not request auth if access was from trusted networks, After this change, websocket API will always request auth if new auth was enabled.

Any integration leverage on websocket API and trusted_networks together should first get access_token by trusted networks authentication provider, then provide access_token to websocket API.

Related issue (if applicable): fixes #15589

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

# configuration.yaml
homeassistant:
  auth_providers:
    - type: trusted_networks  

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@quthla
Copy link
Contributor

quthla commented Aug 4, 2018

Maybe a config setting for trusted networks default user so one can completely skip authentication if wanted?

@awarecan
Copy link
Contributor Author

awarecan commented Aug 4, 2018

@quthla we have discussion about it, please check history of #15589 (comment)

@@ -151,8 +153,13 @@ def __init__(self, flow_mgr):
else:
handler = data['handler']

if handler[0] == 'trusted_networks':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not good.

By doing this, we're starting to conflate concerns of the auth provider with the generic abstraction. It means the abstraction is not good and that needs to be changed.

I don't know right now how we should evolve it, maybe we need to decouple the manager from the flow, allowing the manager to pass in an extra auth context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also comes with having to decouple the HTTP views from using the helper.

And all these things should happen in separate PRs. One thing at a time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part 3 of #15700 was implement login's own flow manager.

@awarecan
Copy link
Contributor Author

This included the changes of #15914 , should rebase/merge after #15914 got merged

Two features not included, can be implement in future

  1. /auth/providers should only return trusted networks provider option if user access from trusted networks
  2. If only one user exist, by pass the user selection step

@balloob
Copy link
Member

balloob commented Aug 13, 2018

Rebased.

@balloob
Copy link
Member

balloob commented Aug 13, 2018

Beautiful.

@balloob balloob merged commit da8f93d into home-assistant:dev Aug 13, 2018
@ghost ghost removed the in progress label Aug 13, 2018
@awarecan awarecan deleted the trusted-networks-aut-prov branch August 22, 2018 12:20
@balloob balloob mentioned this pull request Aug 29, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* Add context to login flow

* Add trusted networks auth provider

* source -> context
vrih pushed a commit to vrih/home-assistant that referenced this pull request Sep 29, 2018
* Add context to login flow

* Add trusted networks auth provider

* source -> context
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Honor trusted network config in new auth system
4 participants