Allow re-unlock of wallets via /unlock #1136
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
Fixes #1121. Prior to this commit, if a user lost access to the authentication token for a session with the wallet RPC, they would not be able to lock the existing wallet before authenticating again (getting a new token for the same wallet, or a different one). After this commit, a call to the /unlock endpoint will succeed even if an existing wallet is currently unlocked (whether a different one or the same one). The existing wallet service, if present, is shut down, if the attempt to authenticate a new wallet, with a password is successful (otherwise nothing happens). A test is added to make sure that this re-unlock can work.
This was referenced Jan 3, 2022
AdamISZ
added a commit
that referenced
this pull request
Feb 3, 2022
This behaviour was originally intended by #1136. If a wallet is currently in an unlocked state in the daemon, but the user loses the auth token, and tries to re-unlock, then before this commit, the unlock operation would fail because there was an unsuccessful attempt to open the wallet file. After this commit, if the unlock request is applied to the wallet with the same name as the one currently active, then the password is authenticated by opening the wallet in read-only mode, preventing the above issue from occurring.
AdamISZ
added a commit
that referenced
this pull request
Feb 4, 2022
Fix shutdown of already open wallet on new unlock Prior to this commit, if a JSON-RPC API client called /unlock on a new wallet while an existing one was currently loaded, the preexisting wallet would not have its wallet service shut down, and thus the .lock file would not be removed, resulting in an inability to open that wallet a second time. After this commit, we correctly refer to the wallet service internally such that its service does get shut down, and the close event of the wallet occurs, removing the .lock file as required. Fix re-unlock of currently unlocked wallet This behaviour was originally intended by #1136. If a wallet is currently in an unlocked state in the daemon, but the user loses the auth token, and tries to re-unlock, then before this commit, the unlock operation would fail because there was an unsuccessful attempt to open the wallet file. After this commit, if the unlock request is applied to the wallet with the same name as the one currently active, then the password is authenticated by opening the wallet in read-only mode, preventing the above issue from occurring. Fix bug that lockwallet doesn't delete wallet_name Before this commit, the member variable wallet_name in the JMWalletDaemon object was not being reset to None in a call to the lockwallet endpoint which meant that the unlock event erroneously treated the wallet as already being active. This corrects that error by setting walet_name to None in lockwallet.
takinbo
pushed a commit
to takinbo/joinmarket-clientserver
that referenced
this pull request
May 27, 2022
Fix shutdown of already open wallet on new unlock Prior to this commit, if a JSON-RPC API client called /unlock on a new wallet while an existing one was currently loaded, the preexisting wallet would not have its wallet service shut down, and thus the .lock file would not be removed, resulting in an inability to open that wallet a second time. After this commit, we correctly refer to the wallet service internally such that its service does get shut down, and the close event of the wallet occurs, removing the .lock file as required. Fix re-unlock of currently unlocked wallet This behaviour was originally intended by JoinMarket-Org#1136. If a wallet is currently in an unlocked state in the daemon, but the user loses the auth token, and tries to re-unlock, then before this commit, the unlock operation would fail because there was an unsuccessful attempt to open the wallet file. After this commit, if the unlock request is applied to the wallet with the same name as the one currently active, then the password is authenticated by opening the wallet in read-only mode, preventing the above issue from occurring. Fix bug that lockwallet doesn't delete wallet_name Before this commit, the member variable wallet_name in the JMWalletDaemon object was not being reset to None in a call to the lockwallet endpoint which meant that the unlock event erroneously treated the wallet as already being active. This corrects that error by setting walet_name to None in lockwallet.
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.
Fixes #1121.
Prior to this commit, if a user
lost access to the authentication token for
a session with the wallet RPC, they would not
be able to lock the existing wallet before
authenticating again (getting a new token for
the same wallet, or a different one).
After this commit, a call to the /unlock endpoint
will succeed even if an existing wallet is currently
unlocked (whether a different one or the same one).
The existing wallet service, if present, is shut down,
if the attempt to authenticate a new wallet, with a
password is successful (otherwise nothing happens).
A test is added to make sure that this re-unlock can work.