Fix/import acct when timed out #1832
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… session has timed out
piyalbasu
commented
Feb 3, 2025
| @@ -466,6 +469,8 @@ export const popupMessageListener = (request: Request, sessionStore: Store) => { | |||
|
|
|||
| const currentState = sessionStore.getState(); | |||
|
|
|||
| sessionTimer.startSession(); | |||
There was a problem hiding this comment.
Small oversight - we were not starting the session timer on account creation
piyalbasu
commented
Feb 3, 2025
| @@ -491,9 +496,6 @@ export const popupMessageListener = (request: Request, sessionStore: Store) => { | |||
| keyName: TEMPORARY_STORE_EXTRA_ID, | |||
| }); | |||
| } catch (e) { | |||
| captureException( | |||
| `Could not login when adding account: ${JSON.stringify(e)}`, | |||
| ); | |||
There was a problem hiding this comment.
removing this Sentry alert because this will be triggered every time a user enters an incorrect password. This will pollute our Sentry dashboard with non-errors
piyalbasu
commented
Feb 3, 2025
| let activePrivateKey = ""; | ||
| try { | ||
| // await _unlockKeystore({ keyID, password }); | ||
| await _unlockKeystore({ keyID, password }); |
There was a problem hiding this comment.
mistakenly left this out - make sure to check the user's password is correct
piyalbasu
commented
Feb 3, 2025
| @@ -555,12 +560,39 @@ export const popupMessageListener = (request: Request, sessionStore: Store) => { | |||
| const importAccount = async () => { | |||
| const { password, privateKey } = request; | |||
| let sourceKeys; | |||
|
|
|||
| let mnemonicPhrase = await getEncryptedTemporaryData({ | |||
There was a problem hiding this comment.
these changes are similar to the changes I added here for add account: cb9d65c
We're making sure that if our session has timed out, we log back in with the user's password before continuing.
This is actually a bug that already exists in production, so making sure the UX is better in the new version
piyalbasu
commented
Feb 3, 2025
| @@ -1121,9 +1155,6 @@ export const popupMessageListener = (request: Request, sessionStore: Store) => { | |||
| try { | |||
| await loginToAllAccounts(password); | |||
| } catch (e) { | |||
| captureException( | |||
| `Could not login when confirming password: ${JSON.stringify(e)}`, | |||
| ); | |||
There was a problem hiding this comment.
same change as above - don't report to Sentry every time a user enter's an incorrect password
aristidesstaffieri
approved these changes
Feb 3, 2025
piyalbasu
added a commit
that referenced
this pull request
Feb 4, 2025
* Feature/memory security improvement 5.27.2 (#1827) * add temporary store extra data * add tests for switching accounts * reset session length * upgrade jest and add unit tests * rm unused selectors * fix sendpayment test * rm npm package and add unit tests * add better error handling and Sentry capture * rm console.log * make sure to login before adding new stellar address * add a test for imported S key payment * Fix/import acct when timed out (#1832) * make sure to login to all accounts before importing by private key if session has timed out * update comment * login before showing mnemonic phrase (#1834) * login before showing mnemonic phrase * add more Sentry error capture
aristidesstaffieri
pushed a commit
that referenced
this pull request
Feb 21, 2025
* Feature/memory security improvement 5.27.2 (#1827) * add temporary store extra data * add tests for switching accounts * reset session length * upgrade jest and add unit tests * rm unused selectors * fix sendpayment test * rm npm package and add unit tests * add better error handling and Sentry capture * rm console.log * make sure to login before adding new stellar address * add a test for imported S key payment * Fix/import acct when timed out (#1832) * make sure to login to all accounts before importing by private key if session has timed out * update comment * login before showing mnemonic phrase (#1834) * login before showing mnemonic phrase * add more Sentry error capture
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar to
add account, if our session has timed out, make sure to log back in before adding a new account to the mix