Page MenuHomePhabricator

[web] dispatch setAccessTokenActionType after successful identity login on web
AbandonedPublic

Authored by varun on Feb 9 2024, 8:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 19 2024, 1:33 PM
Unknown Object (File)
Feb 17 2024, 11:15 PM
Unknown Object (File)
Feb 17 2024, 11:00 AM
Unknown Object (File)
Feb 17 2024, 10:53 AM
Unknown Object (File)
Feb 15 2024, 9:10 AM
Unknown Object (File)
Feb 14 2024, 2:29 PM
Unknown Object (File)
Feb 14 2024, 2:14 PM
Unknown Object (File)
Feb 14 2024, 2:07 PM
Subscribers
None

Details

Reviewers
ashoat
inka
tomek
Summary

On successful identity login we should dispatch the setAccessTokenActionType action to set the CSAT in redux

Depends on D11021

Test Plan

logged in with eth wallet on web and confirmed that access token was set successfully in redux state

Diff Detail

Repository
rCOMM Comm
Branch
siwe (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Feb 9 2024, 8:52 PM
tomek requested changes to this revision.Feb 12 2024, 1:33 AM
tomek added inline comments.
web/grpc/identity-service-client-wrapper.js
293–296

It looks reasonable and convenient, but I'm worried about this approach. This function is used (through a couple of layers) in dispatchActionPromise, which could cause some issues:

  1. Are we allowed to call dispatch as a part of a promise passed to dispatchActionPromise? I guess that yes, but we should make sure.
  2. After the action passed to dispatchActionPromise is successfully resolved, e.g. login success, it is dispatched. This happens after we dispatch setAccessTokenActionType - is it a problem? Can this action e.g. clear a token?

I think it would be safer, and probably easier to reason about, to include a token in the login success payload, but if the current approach is correct, we can keep it.

This revision now requires changes to proceed.Feb 12 2024, 1:33 AM
inka requested changes to this revision.Feb 12 2024, 2:38 AM
inka added inline comments.
web/grpc/identity-service-client-wrapper.js
293–296

login action will be clearing the access token. This is not implemented yet, because I was waiting for login actions to land, but basically we will be calling resetUserSpecificState on logins that are not recovery logins. This is tracked in ENG-6598
I would rather we pass the token in the login actions payload, if that's possible. Is this separate action needed? If so, we need to handle invalidSessionRecovery for it most likely.