Fix: Submitting wrong code for SMS 2FA can result in a vague error message

[staskus]

Issue: https://github.com/wordpress-mobile/WordPress-iOS/issues/20983 Related PR with testing steps: https://github.com/wordpress-mobile/WordPress-iOS/pull/21863

When a user enters an incorrect length SMS code, authTypeAndNonce is constructed as empty. There's no point attempting to log in with an empty nonce since the user would receive a non-descriptive "two_step_nonce required" message.

Changes made https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/788 have slightly changed the originally reported issue. Now it incorrectly displays: "Please fill out all the fields" after entering too short an SMS code.

Before we would call loginWithNonce with an empty nonce resulting in "two_step_nonce required" error. Now we called validateFormAndLogin with an empty nonce resulting in "Please fill out all the fields" error.

Changes

Explicitly show "invalid 2fa code message" when entering an incorrect SMS code.

---

• [x] I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

[staskus]

Is there a way to know what length is expected and disable the Continue button if the entered code length doesn't match the expected code length? If so, we could avoid showing an error message since the Continue button would be un-tappable.

@guarani, this is an excellent idea. I also thought about it, but as you noticed, I couldn't determine if we can consistently know which source we're expecting, so I left the logic itself unchanged.

I'm not sure if it doesn't always return the expected value on purpose (e.g. due to security reasons) or if it's a bug.

Yes, this is a great question 🤔 I will time-box a bit more time to gain more understanding.

[guarani]

👋 Hi @staskus, this change caused a regression with passkey support and it was reverted in https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/802. I haven't investigated an alternative solution to fix this issue without affecting passkeys.

It's worth noting that @enejb and I looked at replacing the login endpoint in https://github.com/wordpress-mobile/WordPressKit-iOS/pull/646. This allows the app to get an accurate list of supported 2FA methods and might give us the info we need to approach this issue differently (e.g. we'll know what length to expect in the 2FA text field and be able to avoid the error message entirely).

[staskus]

👋 Hi @staskus, this change caused a regression with passkey support and it was reverted in https://github.com/wordpress-mobile/WordPressAuthenticator-iOS/pull/802. I haven't investigated an alternative solution to fix this issue without affecting passkeys.

@guarani thanks for fixing it 😓 I couldn't have imagined it would break passkey support. My assumption here was wrong:

There's no point attempting to log in with an empty nonce since the user would receive a non-descriptive "two_step_nonce required" message.

[guarani]

I reviewed this and didn't catch it either. It crossed my mind that this could be related on Nov 2 (p1698934406482319/1698884760.893009-slack-C0180B5PRJ4) but I didn't follow up on this particular lead until a week had passed.

Yeah, the empty nonce was it.