fix(appstore): Don't double-send requests for SMS codes during 2FA auth #29500
Conversation
| push_mode=info["pushMode"], | ||
|
|
||
| # The code has already been sent to the only trusted phone number | ||
| sms_automatically_sent = len(info.get("trustedPhoneNumbers", [])) == 1 and info.get( |
There was a problem hiding this comment.
this logic is basically pulled from fastlane's implementation.
they also have more complex logic to handle other cases, such as when a user has multiple trusted phone numbers, etc.
in the same file, they also skip hitting https://idmsa.apple.com/appleauth/auth/verify/phone when these two checks are true.
| self.state = ClientState.AUTHENTICATED | ||
|
|
||
| def _request_trusted_phone_info(self) -> TrustedPhoneInfo: | ||
| def _request_trusted_phone_info(self) -> Tuple[TrustedPhoneInfo, bool]: |
There was a problem hiding this comment.
i'm not particularly married to this return type.
flub
left a comment
flub
left a comment
There was a problem hiding this comment.
Nice find!
Might as well try and merge this still before the weekend. (or you can wait till Mon if like me you don't like deploying on Fridays)
|
|
||
| if sms_automatically_sent: | ||
| self.state = ClientState.SMS_AUTH_REQUESTED | ||
| self._trusted_phone = trusted_phone |
There was a problem hiding this comment.
This seems a bit odd together with the new assert on line 378?
There was a problem hiding this comment.
sorry about that, this is a little strange. i'm pretty sure this assert is supposed to sit inside of sms_code 🤔 i'll fix this.
| raise ITunesError(f"Unexpected response status: {response.status_code}") | ||
|
|
||
| self.state = ClientState.SMS_AUTH_REQUESTED | ||
| self._trusted_phone = trusted_phone |
There was a problem hiding this comment.
can't we be already sure this is already set by now?
Hitting
https://idmsa.apple.com/appleauth/authwhen an account has zero trusted devices and one trusted phone number will request a new code. This endpoint is being requested twice during 2FA auth. The second request is sent right before the first code is verified, making it impossible for the user to submit a valid code.For users with different configurations, such as a single trusted device and a trusted phone number, the aforementioned endpoint merely returns information about trusted phone numbers.
This PR contains two primary changes which fix this issue:
https://idmsa.apple.com/appleauth/auth, and caching its results so it can be used during verification.https://idmsa.apple.com/appleauth/authhas already sent a code