Update smartcam credential handling and tests#1664
Conversation
|
This should replace PR #1661, working with the user on this PR before they created their own. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1664 +/- ##
==========================================
+ Coverage 93.29% 93.31% +0.02%
==========================================
Files 157 157
Lines 9932 9965 +33
Branches 1022 1027 +5
==========================================
+ Hits 9266 9299 +33
Misses 471 471
Partials 195 195 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds first-class support for updating SMARTCAM admin password (library + CLI) and corrects SMARTCAM local credential updates to target the proper local account identity, with expanded test coverage to validate request payloads and fallback behavior.
Changes:
- Add
SmartCamDevice.update_admin_password()plus shared password encryption/hash helpers. - Update
SmartCamDevice.update_credentials()to usesecname="local_account"and improved old-password fallback handling. - Add CLI
update_admin_passwordcommand and extend smartcam/CLI tests for new/updated behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
kasa/smartcam/smartcamdevice.py |
Adds admin password update API, refactors RSA encryption into _encrypt_password, and updates credential update request construction/fallbacks. |
kasa/cli/device.py |
Adds device update-admin-password command (implemented as update_admin_password). |
kasa/cli/main.py |
Exposes update_admin_password as a top-level runnable device command via lazy subcommand mapping. |
tests/test_cli.py |
Adds CLI test covering confirmation prompt + invocation of update_admin_password. |
tests/smartcam/test_smartcamdevice.py |
Adds extensive unit tests validating request payloads for LV2/LV3 and fallback behavior for both admin password and credentials updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Works for me; though the --username requirement is a bit odd, the camera accept anything (translates it to admin). @rytilahti I just bought one of these and was super frustrating to see half the stuff not working on master (well the pip release was even more confusing :) so what else can we do to help get this merged? :) |
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
I did not yet review the tests, because I think it makes sense to simplify the logic inside update_credentials before doing that. Let me know what do you think about my suggestions inline!
d215861 to
e9e33c9
Compare
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
LGTM, feel free to merge as you wish! I added a quick nit on the way the candidates are collected as I think it makes no sense to optimize here, feel free to act on it or merge as is.
|
after commenting, i spent quite some time on getting stuff working. well not me, claude ;) so i got a lot of the features to work. I haven't had time to play with it over the past month, but in a few weeks I have some time again to clean stuff up. |
|
@oliv3r What stuff working? Just curious. Is this PR still working for you? |
c12fd8a to
d7b91f3
Compare
54c37d1 to
0e9d545
Compare
This PR adds update_admin_password in the library and CLI for SMARTCAM devices. It also updates the update_credentials function to correctly handle the local_account for local device authentication. It updates all of the required tests and code coverage as well.