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

login before showing mnemonic phrase #1834

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

piyalbasu
Copy link
Contributor

Similar to the last bugfix, making sure we log in before trying to show the user their backup phrase if their session has timed out

@@ -78,15 +78,6 @@ export const sessionSlice = createSlice({
reducers: {
reset: () => initialState,
logOut: () => initialState,
setActivePrivateKey: (state, action: { payload: AppData }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is no longer used with the new flow, so removing

}) => {
const { publicKey, privateKey } = keyPair;

const allAccounts = allAccountsSelector(sessionStore.getState());
const accountName = `Account ${allAccounts.length + 1}`;

let activeHashKey = await getActiveHashKeyCryptoKey({ sessionStore });
if (activeHashKey === null && isSettingHashKey) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm adding an explicit flag here just to avoid any weird edge cases where a user gets to this block of code and unexpectedly doesn't have their hash key already. Deriving a new hash key would cause a user to lose access to anything already stored in temporaryDataStorage.

This is an edge case that I don't think was really possible, but I'd rather be explicit about it just in case I'm missing some scenario in which this might happen

}

if (activeHashKey === null) {
throw new Error("Error deriving hash key");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Short circuit if we unexpectedly don't have the hash key. The error will be picked up and reported by the caller

@@ -976,12 +985,27 @@ export const popupMessageListener = (request: Request, sessionStore: Store) => {
return { error: "Incorrect Password" };
}

let mnemonicPhrase = await getEncryptedTemporaryData({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same change as the last bugfix: if we don't have the mnemonic, let's log in and get it

@@ -1428,6 +1452,7 @@ export const popupMessageListener = (request: Request, sessionStore: Store) => {

const signOut = async () => {
sessionStore.dispatch(logOut());
await localStore.remove(TEMPORARY_STORE_ID);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

temporaryStore should be overwritten properly when a user logs back in, but no harm in explicitly removing this to prevent any edge cases

@piyalbasu piyalbasu merged commit ae05a5b into release/5.27.2 Feb 4, 2025
3 checks passed
@piyalbasu piyalbasu deleted the fix/show-phrase-when-timed-out branch February 4, 2025 20:30
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants