Fix/disableHashedFactorkey #223
Conversation
src/mpcCoreKit.ts
Outdated
| if (this.sessionId) { | ||
| const payload: SessionData = { | ||
| postBoxKey: this.state.postBoxKey, | ||
| postboxKeyNodeIndexes: this.state.postboxKeyNodeIndexes || [], | ||
| factorKey: this.state.factorKey?.toString("hex"), | ||
| tssShareIndex: this.state.tssShareIndex as number, | ||
| tssPubKey: this.state.tssPubKey?.toString("hex"), | ||
| signatures: this.signatures, | ||
| userInfo: this.state.userInfo, | ||
| }; | ||
| this.sessionManager.updateSession(payload); | ||
| } |
There was a problem hiding this comment.
@arch1995 should double check this. I'm not familiar with sessionManager.
There was a problem hiding this comment.
revert the changes as finalizeTkey will create new session
| shareDescription: FactorKeyTypeShareDescription.Other, | ||
| updateMetadata: false, | ||
| }); | ||
| await this.setDeviceFactor(factorKey); |
There was a problem hiding this comment.
So this means the factor key was previously not persisted here, which is kind of catastrophic. We should have a test that asserts this.
| ]; | ||
|
|
||
| const email = "testmail99"; | ||
| const email = "testmail99-1"; |
There was a problem hiding this comment.
Why that change? (We briefly talked about it in our call, but I think it would be good to document it here.)
| await coreKitInstance.tKey.storageLayer.setMetadata({ | ||
| privKey: new BN(coreKitInstance.state.postBoxKey!, "hex"), | ||
| input: { message: "KEY_NOT_FOUND" }, | ||
| }); |
There was a problem hiding this comment.
Why we no longer delete key on setup?
There was a problem hiding this comment.
critical_delete require tkey reconstructed.
some testcases and user flow, we do not have the 2nd factor key, hence we are reseting the acocunt via setMetadata with KEY_NOT_FOUND
|
|
||
| const { idToken, parsedToken } = login ? await login(email) : await mockLogin(email); | ||
| await instance.init(); | ||
| await instance.init({ handleRedirectResult: false, rehydrate: false }); |
There was a problem hiding this comment.
What are the default values for these parameters? Did we change these? If so, why? Or are we just being more explicit here?
There was a problem hiding this comment.
The previous testcases do not take rehydration via session manager in to consideration.
This new options added to make core kit instance to be initialized without rehydration for the test flow
|
close in favor of #224 |
Motivation and Context
Jira Link:
Description
Fix feature disableHashedFactorkey
device factorkey is not set when disableHashedFactoryKey flag is set.
this is causing user lose the device factorkey during signup.
fix atomic sync for
update tests
update tests cases
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist: