Page MenuHomePhabricator
Differential D10881

[native] use identityGenerateNonce in SIWEPanel component
ClosedPublic
Actions

Authored by varun on Jan 30 2024, 8:25 PM.
Tags
None
Referenced Files
F3281752: D10881.diff
Sat, Nov 16, 10:10 AM
Unknown Object (File)
Wed, Nov 6, 8:05 PM
Unknown Object (File)
Tue, Oct 22, 12:53 PM
Unknown Object (File)
Tue, Oct 22, 12:39 PM
Unknown Object (File)
Tue, Oct 22, 5:32 AM
Unknown Object (File)
Tue, Oct 22, 5:32 AM
Unknown Object (File)
Oct 14 2024, 8:33 PM
Unknown Object (File)
Oct 14 2024, 5:55 PM
View All 59 Files
Subscribers
None

Details

Reviewers
ashoat
inka
Commits
rCOMM5900d8e9de00: [native] use identityGenerateNonce in SIWEPanel component
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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun created this revision.Jan 30 2024, 8:25 PM
Harbormaster completed remote builds in B26316: Diff 36364.Jan 30 2024, 8:43 PM
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

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 mentioned this in D10880: [lib][native][web] add generate nonce action types.Jan 31 2024, 8:24 AM
ashoat added inline comments.
native/account/siwe-panel.react.js
64–66

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

varun updated this revision to Diff 36543.Feb 1 2024, 10:28 AM

address feedback

varun added inline comments.Feb 1 2024, 10:29 AM
native/account/siwe-panel.react.js
64–66

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

89–106

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

Harbormaster completed remote builds in B26430: Diff 36543.Feb 1 2024, 10:46 AM
ashoat accepted this revision.Feb 1 2024, 11:25 AM

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

ashoat removed a reviewer: tomek.Feb 1 2024, 11:25 AM
This revision is now accepted and ready to land.Feb 1 2024, 11:25 AM
varun added a child revision: D10958: [native] rename siwe auth loading status selector.Feb 5 2024, 9:53 PM
Closed by commit rCOMM5900d8e9de00: [native] use identityGenerateNonce in SIWEPanel component (authored by varun). · Explain WhyFeb 7 2024, 9:57 PM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMM5900d8e9de00: [native] use identityGenerateNonce in SIWEPanel component.

Revision Contents
Changeset List

PathSize
native/
account/
32 lines

Diff 36364

View Options

native/account/siwe-panel.react.js

Loading...