Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factorkey validation and rehydration error #132

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lwin-kyaw
Copy link
Contributor

@lwin-kyaw lwin-kyaw commented May 3, 2024

This PR includes -

  • check 'SHARE_DELETED' on getFactorKeyMetadata and metadata validation
  • throw error instead of logging, for rehydration fail.
  • validate current factor key before copy or create new shares

lwin-kyaw added 2 commits May 3, 2024 11:01
'SHARE_DELETED' on getMetadata when using manualSync mode
@ieow
Copy link
Contributor

ieow commented May 7, 2024

Can you elaborate the issue ?
why the need to throw on rehydrate failed?

const hashedFactorKey = getHashedPrivateKey(this.state.oAuthKey, this.options.hashedFactorNonce);
if ((await this.checkIfFactorKeyValid(hashedFactorKey)) && !this.options.disableHashedFactorKey) {
const isHashedFactorKeyValid = this.checkIfFactorKeyValid(hashedFactorKey);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkIfFactorKeyValid is async function

@lwin-kyaw
Copy link
Contributor Author

lwin-kyaw commented May 7, 2024

Can you elaborate the issue ? why the need to throw on rehydrate failed?

Hi, we don't necessarily need to throw rehydration error as when rehydration failed, all the later steps will also fail (factor login, create factor, etc...).
However throwing error on rehydration failed, can catch some errors in early stage and prevent users proceeding to other steps.
Anyway, we have logged out the error message when there's an error during rehydration, so, I think it would be better if we could throw the error also. It will also help us to write better test cases.

Sample case on rehydration failure

  • create first core kit with manualSync:true
  • create factor (factorKey1) and don't commit the changes
  • create second core kit and init (with default params, Mpc-Corekit will rehydrate with existing factorKey1 from sessionManager)
  • during rehydrate, we try to get the metadata for factorKey1, but since we don't commit changes in first corekit, factor metadata is not available, resulting in rehydration failure

@lwin-kyaw lwin-kyaw changed the title Throw error during rehydration Factorkey validation and rehydration error May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants