Handle forbidden auth check with token user ID - #1159
Conversation
|
There hasn't been any activity on this pull request recently, so it has automatically been marked as stale and will be closed if no further action occurs within 7 days. Thank you for your contributions. |
github-actions
Bot
added
the
stale
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #1159 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1024 1041 +17
=========================================
+ Hits 1024 1041 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bachya
removed
the
stale
| err.status != 403 | ||
| or not self.access_token | ||
| or (user_id := self._user_id_from_access_token(self.access_token)) | ||
| is None |
There was a problem hiding this comment.
This ternary is really confusing to follow. I would consider catching each scneario and raising the exception with a different message.
| IndexError, | ||
| KeyError, | ||
| TypeError, | ||
| ValueError, |
There was a problem hiding this comment.
Feels odd that we're catching a large-ish subset of multiple core exception types.
|
@davidrimshnick @bachya : I've been impacted by this a few times in Home Assistant and I wouldn't mind trying to work on this PR if it's stuck. |
I'll leave that up to @davidrimshnick; I've left feedback above. |
3 participants
Describe what the PR does:
SimpliSafe can return
403 Forbiddenfromapi/authCheckeven after successfully issuing a valid access token. When that specific response occurs, decode thehttp://simplisafe.com/uidclaim from the newly issued access token and use it as the account user ID.All other HTTP failures continue to propagate unchanged, and a 403 still propagates when the token is malformed or does not contain a usable user ID.
Does this fix a specific issue?
Related to home-assistant/core#149151
Checklist:
Testing:
pytest --cov simplipy tests: 97 passed, 100% coverage