[native] Don't allow using cached SIWE signature if nonce is expired
ClosedPublic
Actions

Authored by ashoat on Sun, May 12, 7:32 PM.

Details

Reviewers

inka

Commits

rCOMM97b80c5d49a3: [native] Don't allow using cached SIWE signature if nonce is expired

Summary

If the nonce used for SIWE will be rejected by the relevant backend (authoritative keyserver or identity service, depending on usingCommServicesAccessToken) due to expiration, then we should force the user to generate a new SIWE signature.

Would normally put @varun on the review here, but going to ask @inka instead because he is out.

Depends on D12007

Test Plan

I tested (or will test) this diff stack as follows:

Be in a multi-keyserver environment, testing SIWE with iOS simulator
Do a SIWE and then wait 2 minutes to let the nonce expire in the following screens:
- FullscreenSIWEPanel for an account that doesn't exist yet when the new registration flow is disabled
- FullscreenSIWEPanel for an account that doesn't exist yet when the new registration flow is enabled
- FullscreenSIWEPanel for an account that does exist
- New registration flow for an account that doesn't exist yet (RegistrationTerms)
- New registration flow for an account that does exist (ExistingEthereumAccount)
Make sure there are no duplicate Alerts, that in all cases an Alert is shown, and that the "back" action activates when the user confirms the Alert

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Sun, May 12, 7:32 PM

Herald added a subscriber: tomek. · View Herald TranscriptSun, May 12, 7:32 PM

ashoat added a child revision: D12009: [native] Display meaningful alert if ExistingEthereumAccount login fails due to expired nonce.Sun, May 12, 7:33 PM

Harbormaster completed remote builds in B28860: Diff 40101.Sun, May 12, 7:51 PM

ashoat requested review of this revision.Sun, May 12, 7:51 PM

inka added inline comments.Mon, May 13, 3:42 AM

native/account/registration/connect-ethereum.react.js
224–231 ↗	(On Diff #40101)	Are we sure this effect will be triggered when the nonce becomes expired?

ashoat added inline comments.Mon, May 13, 6:09 AM

native/account/registration/connect-ethereum.react.js
224–231 ↗	(On Diff #40101)	It's not very important, to be honest. I added the effect for "correctness", but the important thing is that every place we use `cachedSelections.ethereumAccount` (just here and in D12007), we check if the nonce is expired before we use it. I added the effect because I figured it's better to clear the expired SIWE state than to have invalid state lying around. But since we check it at all places we use it, there is no risk in the expired SIWE state being there.