Page MenuHomePhabricator

[native] use identityGenerateNonce in SIWEPanel component
ClosedPublic

Authored by varun on Jan 30 2024, 8:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 21, 3:41 AM
Unknown Object (File)
Thu, Nov 21, 3:41 AM
Unknown Object (File)
Sat, Nov 16, 12:18 PM
Unknown Object (File)
Sat, Nov 16, 10:10 AM
Unknown Object (File)
Wed, Nov 6, 8:05 PM
Unknown Object (File)
Oct 22 2024, 12:53 PM
Unknown Object (File)
Oct 22 2024, 12:39 PM
Unknown Object (File)
Oct 22 2024, 5:32 AM
Subscribers
None

Details

Summary
NOTE: SIWE won't work yet if usingCommServicesAccessToken is enabled.

This change just sets the nonce state variable to the value received from the identity service

Depends on D10880

Test Plan

logged the nonce and confirmed that the state variable was set successfully

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun requested review of this revision.Jan 30 2024, 8:43 PM
tomek requested changes to this revision.Jan 31 2024, 1:28 AM
tomek added inline comments.
native/account/siwe-panel.react.js
89–106 ↗(On Diff #36364)

This shouldn't work and the test plan should fail in this case, so I'm a little surprised. We don't await a promise passed to dispatchActionPromise which makes it possible that setNonce will be called without response being set. The setNonce call should be kept inside the promises, just like it was before this diff.

This revision now requires changes to proceed.Jan 31 2024, 1:28 AM
ashoat added inline comments.
native/account/siwe-panel.react.js
64–66 ↗(On Diff #36364)

Doesn't this need to be modified for your new action type?

native/account/siwe-panel.react.js
64–66 ↗(On Diff #36364)

yes, was going to be the next diff but makes more sense in this one

89–106 ↗(On Diff #36364)

you're right, i was logging response inside the promise passed to dispatchActionPromise, not the nonce state variable

Accepting on @tomek's behalf, as he's off until Monday

This revision is now accepted and ready to land.Feb 1 2024, 11:25 AM