feat: keyring access & Monereo address generation #926
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
Description --- Revamp the way we handle keyring access and use it to store the monero seed and the tari seed passphrase. Motivation and Context --- The user could deny access to the system Keychain, if they do we wouldn't be able to successfully store needed credentials for the app. Provide a wrapper to encapsulate storing the credentials in either the system keychain, or fallback to file IO storage. How Has This Been Tested? --- Manually What process can a PR reviewer use to test or verify this change? --- There are (at least) 7 important scenarios that need to be covered. 1. A new wallet is created and access to the keychain is denied 2. A new wallet is created and access to the keychain is granted 3. A new wallet is created and access to the keychain is granted, but on the next startup access to the keychain is denied 4. An old wallet was created access is granted (from a previous version). The wallet is upgraded to this version, granted access, and continues to work 5. An old wallet was created access is granted (from a previous version). The wallet is upgraded to this version, access is denied, and continues to work 6. An old wallet was created, and access to the keychain is denied. The wallet is upgraded to this version, access is granted, and continues to work 7. An old wallet was created, and access to the keychain is denied. The wallet is upgraded to this version, access is denied, and continues to work Additional scenarios that don't require the grid: 8. Create a new wallet, do what you will with the password, get your monero seed words, ensure they are unique between new wallet creations. Recovery the monero wallet using the monero GUI wallet, on Mainnet. Validate the recovered wallet has the same monero address we provided. | Scenario | Old wallet Key Chain Access | New wallet Key Chain Access Denied | Unique Monero Address | Monero Seed words visible | Passed | |------------------------------------------------------|------------------------------------------|------------------------------------------|-----------------------|----------------------------|------------| | **1. New wallet, denied access** | N/A | ❌ | ✅ | ✅ | ✅ / ❌ | | **2. New wallet, granted access** | N/A | ✅ | ✅ | ✅ | ✅ / ❌ | | **3. New wallet, granted then denied** | N/A | ✅ & ❌ | ✅ | ❌ | ✅ / ❌ | | **4. Old wallet, granted access, upgraded and granted access** | ✅ | ✅ | ❌ | ❌ | ✅ / ❌ | | **5. Old wallet, granted access, upgraded and denied access** | ✅ | ❌ | ❌ | ❌ | ✅ / ❌ | | **6. Old wallet, denied access, upgraded and granted access** | ❌ | ✅ | ❌ | ❌ | ✅ / ❌ | | **7. Old wallet, granted access, upgraded and denied access** | ❌ | ❌ | ❌ | ❌ | ✅ / ❌ | **A FULL WALLET RESET, FROM THIS VERSION,** is required between each scenario Testing Steps --- Scenario 1: 1. Create a new wallet 2. When prompted for a password to access secure storage / keychain select deny, and do not enter a password 3. Validate you have a unique monero address 4. Validate you can see the monero address seed words Scenario 2: 1. Create a new wallet 2. When prompted for a password to access secure storage / keychain enter a password correctly (however many times prompted, or select Always Allow) 3. Validate you have a unique monero address 4. Validate you can see the monero address seed words Scenario 3: 1. Create a new wallet 2. When prompted for a password to access secure storage / keychain enter a password correctly (however many times prompted, or select Always Allow) 3. Validate you have a unique monero address 4. Restart the application 6. When prompted for a password deny, and do not enter a password 7. Attempt to view monero seed words 8. Receive an error because you did not provide the password Scenario 4: 1. Start with an old wallet that was created in a previous version. 2. When prompted for a password to access secure storage / keychain enter a password correctly (however many times prompted, or select Always Allow) 3. Upgrade the application to the current version. 5. When prompted for a password to access secure storage / keychain, enter the correct password (or select Always Allow). 5. Wallet should continue to operate without issue. 6. Monero address should be the default monero community address. `44AFFq5kSiGBoZ4NMDwYtN18obc8AemS33DBLWs3H7otXft3XjrpDtQGv7SqSsaBYBb98uNbr2VBBEt7f2wfn3RVGQBEP3A` 7. Monero seed words area should not be present to be viewed Scenario 5: 1. Start with an old wallet that was created in a previous version. 2. When prompted for a password to access secure storage / keychain enter a password correctly (however many times prompted, or select Always Allow) 3. Upgrade the application to the current version. 4. When prompted for a password to access secure storage / keychain, deny access, and do not enter a password. 5. Wallet should still operate 6. Monero address should be the default monero community address. `44AFFq5kSiGBoZ4NMDwYtN18obc8AemS33DBLWs3H7otXft3XjrpDtQGv7SqSsaBYBb98uNbr2VBBEt7f2wfn3RVGQBEP3A` 7. Monero seed words area should not be present to be viewed Scenario 6: 1. Start with an old wallet that was created in a previous version. 2. When prompted for a password to access secure storage / keychain, deny access, and do not enter a password. 3. Upgrade the application to the current version. 4. When prompted for a password to access secure storage / keychain enter a password correctly (however many times prompted, or select Always Allow) 5. Wallet should still operate 6. Monero address should be the default monero community address. `44AFFq5kSiGBoZ4NMDwYtN18obc8AemS33DBLWs3H7otXft3XjrpDtQGv7SqSsaBYBb98uNbr2VBBEt7f2wfn3RVGQBEP3A` 7. Monero seed words area should not be present to be viewed Scenario 7: 1. Start with an old wallet that was created in a previous version. 2. When prompted for a password to access secure storage / keychain, deny access, and do not enter a password. 3. Upgrade the application to the current version. 4. When prompted for a password to access secure storage / keychain, deny access, and do not enter a password. 5. Wallet should still operate 6. Monero address should be the default monero community address. `44AFFq5kSiGBoZ4NMDwYtN18obc8AemS33DBLWs3H7otXft3XjrpDtQGv7SqSsaBYBb98uNbr2VBBEt7f2wfn3RVGQBEP3A` 7. Monero seed words area should not be present to be viewed Scenario 8: 1. Create a new wallet 2. When prompted for a password to access secure storage / keychain accept, or deny, dealers choice 3. Validate you have a unique monero address 4. Validate you can see the monero address seed words 5. Run the monero wallet GUI 6. Set the GUI to mainnet  8. Recover with seed words  9. Validate address matches  <!-- Checklist --> <!-- 1. Is the title of your PR in the form that would make nice release notes? The title, excluding the conventional commit tag, will be included exactly as is in the CHANGELOG, so please think about it carefully. --> Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify <!-- Does this include a breaking change? If so, include this line as a footer --> <!-- BREAKING CHANGE: Description what the user should do, e.g. delete a database, resync the chain --> --------- Co-authored-by: Marcin Papież <[email protected]> Co-authored-by: stringhandler <[email protected]> Co-authored-by: shan <[email protected]> Co-authored-by: Aaron Pepper <[email protected]> Co-authored-by: Juan <[email protected]> Co-authored-by: Juan De Luca <[email protected]> Co-authored-by: stringhandler <[email protected]> Co-authored-by: Misieq01 <[email protected]> Co-authored-by: Shannon Tenner <[email protected]>
|
@mmrrnn @MCozhusheck please do a code review. As I'll be looking to merge this today, and get it out of my life 😃 |
|
Tested successfully on windows and linux |
mmrrnn
reviewed
Nov 22, 2024
| @@ -61,6 +61,7 @@ | |||
| "placeholder": "Enter Monero address", | |||
| "title": "Monero address" | |||
| }, | |||
| "monero-seed-words": "Monero Seed Words", | |||
There was a problem hiding this comment.
npm run translate settings monero-seed-words="Monero Seed Words" to generate for all languages
mmrrnn
approved these changes
Nov 22, 2024
MCozhusheck
approved these changes
Nov 22, 2024
|
Tested successfully on Windows 11, app version 0.8.2 for:
I will test the first scenario after completing the rest of the PR's |
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.
Description
Revamp the way we handle keyring access and use it to store the monero seed and the tari seed passphrase.
Motivation and Context
The user could deny access to the system Keychain, if they do we wouldn't be able to successfully store needed credentials for the app. Provide a wrapper to encapsulate storing the credentials in either the system keychain, or fallback to file IO storage.
How Has This Been Tested?
Manually
What process can a PR reviewer use to test or verify this change?
Scenario 1:
Scenario 2:
Breaking Changes