Expose better authentication errors - #1742
blakeembrey wants to merge 1 commit into
Conversation
| 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
Additional details and impacted files@@ 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. 🚀 New features to boost your workflow:
|
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.
| 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?
| 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:
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.