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

Enhancing the profile manager to selectively show the password tab, contingent upon the presence of securely hashed credentials stored in the local storage. #509

Merged
merged 8 commits into from
Jun 12, 2023

Conversation

Mahmoud-Emad
Copy link
Contributor

@Mahmoud-Emad Mahmoud-Emad commented Jun 11, 2023

Description

It enhances the profile manager to selectively show the password tab, contingent upon the presence of securely hashed credentials stored in the local storage.

Changes

  • created a new interface named Credentials that includes passwordHash and mnemonicHash fields to represent securely hashed credentials.
  • added new helper functions to manage the Credentials interface like getCredentials, setCredentials and isStoredCredentials to facilitate the handling and retrieval of stored credentials.
  • Implemented a key-down function to enable login or connect functions when the Enter key is pressed on the password input, provided there are no validation errors.
  • Change the Store and login button text to Connect

Related Issues

Screenshots

  • image
  • image

Update

  • image

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstrings
  • Screenshots/Video attached (needed for UI changes)

…e is credentials hashed and stored in the local storage.
@MohamedElmdary
Copy link
Member

Enter doesn't respect Mnemonic value

connector_issue_1.mp4

Using keydown.enter will cause some unexpected cases; It's better to use <form> with onsubmit event as it won't fire event if the button is disabled

Comment on lines 290 to 303
let mountedTimeout: any;
watch(
() => props.modelValue,
m => {
if (m) {
if (mountedTimeout) {
clearTimeout(mountedTimeout);
}
mountedTimeout = setTimeout(() => {
mounted();
});
}
},
);
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use setTimeout api to wait as it might lead to unexpected results like old playground

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used requestAnimationFrame instead

Copy link
Member

Choose a reason for hiding this comment

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

Don't use requestAnimationFrame either.
Please check nextTick

import { nextTick } from "vue"

nextTicket().then(mounte)

Copy link
Member

Choose a reason for hiding this comment

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

@Mahmoud-Emad any updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be done

Comment on lines 332 to 341

function getCredentials() {
const getCredentials = localStorage.getItem("wallet");
let credentials: Credentials = {};

if (getCredentials) {
credentials = JSON.parse(getCredentials);
}
return credentials;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function getCredentials() {
const getCredentials = localStorage.getItem("wallet");
let credentials: Credentials = {};
if (getCredentials) {
credentials = JSON.parse(getCredentials);
}
return credentials;
}
const version = 1; // for example
const WALLET_KEY = "wallet.v" + version
function getCredentials() {
const getCredentials = localStorage.getItem(WALLET_KEY);
let credentials: Credentials = {};
if (getCredentials) {
credentials = JSON.parse(getCredentials);
}
return credentials;
}

Support versioning for wallet as we might change how we manage password and mnemonic from time to time.
let's imagine if we changed the storing way when the user open his browser it will be broken as we will be expecting another way of parsing data and here versioning comes into play it won't respect any wallet beside the new one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's assume that we changed the version of storing the wallet, in that case, we have to update the version, which is the way we're going to update the wallet key, in both cases we have to change the key, what is the difference?

Copy link
Member

Choose a reason for hiding this comment

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

Updating the version should be done whenever we change the behaviour of the wallet so it will break the user browser
@AhmedHanafy725

Copy link
Contributor

Choose a reason for hiding this comment

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

the version will make it easier to migrate from the old key to the new one. if the same key is used, you will have to check which one is used first and then decide whether to migrate to the new one or not.

…r_shared_password

Clear profile manager fields when switching between Login and Connect
@Mahmoud-Emad Mahmoud-Emad merged commit 9515fff into development Jun 12, 2023
@Mahmoud-Emad Mahmoud-Emad deleted the development_profile_manager_view branch June 12, 2023 08:29
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.

3 participants