Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add config flow and 2FA support for Blink #35396

Merged
merged 16 commits into from
May 13, 2020

Conversation

fronzbot
Copy link
Contributor

@fronzbot fronzbot commented May 8, 2020

Breaking change

As of May 11, 2020 Blink has removed the old username/password authentication method which means all current Blink integrations will be broken. In order to support this change, a 2FA key must be entered before setup can continue. Some users may not have 2FA enabled on their account yet, but you will still receive an email at login asking you to allow the device to continue setting up. Your current YAML config will be converted to a GUI-based config flow but the only supported entries are username, password, and scan_interval. All other entries must be removed otherwise the integration will not be configured.

Proposed change

Fixes #35214
More information on fronzbot/blinkpy#210

Moving forward, Blink is migrating to 2FA. What this means is that I'll need to remove YAML config at some point, but for now we can migrate users since another endpoint exists that just requires you to click "Allow Device" in your email (no time limit as far as I can tell...which is clearly excellent security 😏). So here is a summary of changes (many changes not mentioned here were made to the core library: fronzbot/blinkpy)

  • Added config flow to the integration
    • I've tested this and it works. I've also test yaml configuration and it works.
    • Currently using the show_advanced_options for scan_interval in config_flow.py doesn't seem to work, so I could use some help here. Not sure why it's broken. Given that users can still use yaml config, I figured I can save this as a "to-do" since we've got a bit of a time crunch before this integration breaks for all users.
  • Made many things async (as much as possible, and where I understood it to be useful/necessary)
  • I've double checked log entries to ensure there's no I/O in event loop issues, since a lot of things have been changed to async

I'm not sure my implementation of the config flow is correct. I know it works, but "works" and "correct" are rarely the same thing 😄. So I appreciate any and all feedback.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
blink:
  username: EMAIL ADDRESS
  password: PASSWORD
  scan_interval: 300

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

@fronzbot
Copy link
Contributor Author

@balloob i hate to tag you, but is there any way i can get this expedited for a review? Blink integration is now broken for everyone without this change

fronzbot added 2 commits May 11, 2020 15:28
- Changed strings to common references
- Removed async_abort since username is unique id
- Pass in data via import method rather than dict (per review)
- Use entry_id for config entries
- Add startup wrapper to minimize API calls during setup
- Better handling of invalid auth/2fa keys
- Store authtokens and login info for improved setup
@MartinHjelmare
Copy link
Member

The breaking change paragraph needs to be adjusted and say that extra items in configuration.yaml will not be accepted and stop the integration from being set up. They will not be ignored.

@fronzbot
Copy link
Contributor Author

fronzbot commented May 12, 2020

@MartinHjelmare - thanks for the review, much appreciated! Just updated the breaking changes comment.

And for general organization purposes (ie. I will forget about all of these unless I write them down), here are things being tabled for future PR's:

If I missed any, just let me know so I can hold myself accountable 😃

@MartinHjelmare
Copy link
Member

@fronzbot there's one more comment to address.

#35396 (comment)

When can you do that? We're going to cut the beta today very soon and we'd like to include this PR.

@fronzbot
Copy link
Contributor Author

Missed that. I'll get to that within the next hour, should be quick 👍

- Will be added to options flow at later time
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Thanks!

@frenck frenck merged commit 85726b6 into home-assistant:dev May 13, 2020
@fronzbot fronzbot deleted the blink-new-auth branch May 13, 2020 14:02
@darraghbr
Copy link

Great work all! Can't wait to be able to use my blink cameras!

@woobenjamin
Copy link

woobenjamin commented May 13, 2020 via email

@hackthis02
Copy link

When are these changes going to get pushed to the clients or is that something I need to do on my server?

@home-assistant home-assistant locked as resolved and limited conversation to collaborators May 18, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blink Integration being blocked
8 participants