Expose better authentication errors - #1742
Conversation
Sorry, something went wrong.
| owner = discovery_info.get("owner") | ||
| if credentials and self._is_owner_hash(owner): | ||
| supplied_owner = hashlib.md5( # noqa: S324 | ||
| credentials.username.encode() |
Codecov Report❌ Patch coverage is 90.47619% with 4 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## master #1742 +/- ##
==========================================
- Coverage 93.29% 93.27% -0.02%
==========================================
Files 157 157
Lines 9932 9967 +35
Branches 1022 1030 +8
==========================================
+ Hits 9266 9297 +31
- Misses 471 473 +2
- Partials 195 197 +2 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
rytilahti
left a comment
There was a problem hiding this comment.
Hi @blakeembrey and thanks for your PR! I think this would be really useful addition, I added a couple of things that should be taken care of before this can be merged.
Sorry, something went wrong.
| return error | ||
|
|
||
| @staticmethod | ||
| def _is_owner_hash(owner: Any) -> TypeGuard[str]: |
There was a problem hiding this comment.
Could we avoid typeguard import, set correct type for the input parameter, and make it return a regular bool?
Sorry, something went wrong.
| supplied_owner = hashlib.md5( # noqa: S324 | ||
| credentials.username.encode() | ||
| ).hexdigest() | ||
| if supplied_owner.casefold() != owner.casefold(): |
There was a problem hiding this comment.
Would you mind moving this logic into a better place (likely to the corresponding auth protocol) and make it reusable?
Logic to detect mistyped username (I always assumed it'd be incorrect casing) would be useful, and we could try that to help people with issues like:
Sorry, something went wrong.
Reviewers
Requested changes must be addressed to merge this pull request.
I was trying to set up the P316M but run into a bunch of issues, these failures would have helped me a lot earlier. I'm sure it could still be further improved, but it's what worked for me.
Debug process:
Prior to diving into this package I had assumed the board was either broken or unsupported. I tried more than I should have to re-authenticate in the home assistant UI.