zebra.lucky
/
joinmarket-clientserver
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity
Browse Source

Fix bugs in unlock-lock logic 

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.
master
Adam Gibson 4 years ago
parent
4ba817e5d2
commit
ed7b4e1926
No known key found for this signature in database
GPG Key ID: 141001A1AF77F20B
2 changed files with 99 additions and 58 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 145
      jmclient/jmclient/wallet_rpc.py
  2. 12
      jmclient/test/test_wallet_rpc.py

145
jmclient/jmclient/wallet_rpc.py
Unescape View File

@ -143,8 +143,6 @@ class JMWalletDaemon(Service):
# allow the client to start/stop.
self.services["wallet"] = None
self.wallet_name = "None"
# label for convenience:
self.wallet_service = self.services["wallet"]
# Client may start other services, but only
# one instance.
self.services["snicker"] = None
@ -324,6 +322,23 @@ class JMWalletDaemon(Service):
request_cookie) + ", request rejected.")
raise NotAuthorized()
def set_token(self, wallet_name):
""" This function creates a new JWT token and sets it as our
'cookie' for API and WS. Note this always creates a new fresh token,
there is no option to manually set it, intentionally.
"""
# any random secret is OK, as long as it is not deducible/predictable:
secret_key = bintohex(os.urandom(16))
encoded_token = jwt.encode({"wallet": wallet_name,
"exp" :datetime.datetime.utcnow(
)+datetime.timedelta(minutes=30)},
secret_key)
self.cookie = encoded_token.strip()
# We want to make sure that any websocket clients use the correct
# token. The wss_factory should have been created on JMWalletDaemon
# startup, so any failure to exist here is a logic error:
self.wss_factory.valid_token = self.cookie
def get_POST_body(self, request, keys):
""" given a request object, retrieve values corresponding
to keys keys in a dict, assuming they were encoded using JSON.
@ -349,24 +364,15 @@ class JMWalletDaemon(Service):
Here we must also register transaction update callbacks, to fire
events in the websocket connection.
"""
if self.wallet_service:
if self.services["wallet"]:
# we allow a new successful authorization (with password)
# to shut down the currently running service(s), if there
# are any.
# This will stop all supporting services and wipe
# state (so wallet, maker service and cookie/token):
self.stopService()
# any random secret is OK, as long as it is not deducible/predictable:
secret_key = bintohex(os.urandom(16))
encoded_token = jwt.encode({"wallet": wallet_name,
"exp" :datetime.datetime.utcnow(
)+datetime.timedelta(minutes=30)},
secret_key)
encoded_token = encoded_token.strip()
self.cookie = encoded_token
if self.cookie is None:
raise NotAuthorized("No cookie")
self.wallet_service = WalletService(wallet)
self.services["wallet"] = WalletService(wallet)
# restart callback needed, otherwise wallet creation will
# automatically lead to shutdown.
# TODO: this means that it's possible, in non-standard usage
@ -375,28 +381,26 @@ class JMWalletDaemon(Service):
# or requesting rescans, none are implemented yet.
def dummy_restart_callback(msg):
jlog.warn("Ignoring rescan request from backend wallet service: " + msg)
self.wallet_service.add_restart_callback(dummy_restart_callback)
self.services["wallet"].add_restart_callback(dummy_restart_callback)
self.wallet_name = wallet_name
self.wallet_service.register_callbacks(
self.services["wallet"].register_callbacks(
[self.wss_factory.sendTxNotification], None)
self.wallet_service.startService()
# now that the base WalletService is started, we want to
# make sure that any websocket clients use the correct
# token. The wss_factory should have been created on JMWalletDaemon
# startup, so any failure to exist here is a logic error:
self.wss_factory.valid_token = encoded_token
self.services["wallet"].startService()
# now that the WalletService instance is active and ready to
# respond to requests, we return the status to the client:
# First, prepare authentication for the calling client:
self.set_token(wallet_name)
if('seedphrase' in kwargs):
return make_jmwalletd_response(request,
status=201,
walletname=self.wallet_name,
token=encoded_token,
token=self.cookie,
seedphrase=kwargs.get('seedphrase'))
else:
return make_jmwalletd_response(request,
walletname=self.wallet_name,
token=encoded_token)
token=self.cookie)
def taker_finished(self, res, fromtx=False, waittime=0.0, txdetails=None):
# This is a slimmed down version compared with what is seen in
@ -468,14 +472,14 @@ class JMWalletDaemon(Service):
def displaywallet(self, request, walletname):
print_req(request)
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
jlog.warn("displaywallet called, but no wallet loaded")
raise NoWalletFound()
if not self.wallet_name == walletname:
jlog.warn("called displaywallet with wrong wallet")
raise InvalidRequestFormat()
else:
walletinfo = wallet_display(self.wallet_service, False, jsonified=True)
walletinfo = wallet_display(self.services["wallet"], False, jsonified=True)
return make_jmwalletd_response(request, walletname=walletname, walletinfo=walletinfo)
@app.route('/session', methods=['GET'])
@ -489,8 +493,8 @@ class JMWalletDaemon(Service):
session = not self.cookie==None
maker_running = self.coinjoin_state == CJ_MAKER_RUNNING
coinjoin_in_process = self.coinjoin_state == CJ_TAKER_RUNNING
if self.wallet_service:
if self.wallet_service.isRunning():
if self.services["wallet"]:
if self.services["wallet"].isRunning():
wallet_name = self.wallet_name
else:
wallet_name = "not yet loaded"
@ -512,12 +516,12 @@ class JMWalletDaemon(Service):
"destination"])
if not payment_info_json:
raise InvalidRequestFormat()
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
try:
tx = direct_send(self.wallet_service,
tx = direct_send(self.services["wallet"],
int(payment_info_json["amount_sats"]),
int(payment_info_json["mixdepth"]),
destination=payment_info_json["destination"],
@ -542,7 +546,7 @@ class JMWalletDaemon(Service):
"ordertype", "minsize"])
if not config_json:
raise InvalidRequestFormat()
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
@ -563,7 +567,7 @@ class JMWalletDaemon(Service):
config_json["cjfee_factor"] = None
config_json["size_factor"] = None
self.services["maker"] = YieldGeneratorService(self.wallet_service,
self.services["maker"] = YieldGeneratorService(self.services["wallet"],
dhost, dport,
[config_json[x] for x in ["txfee", "cjfee_a",
"cjfee_r", "ordertype", "minsize",
@ -585,7 +589,7 @@ class JMWalletDaemon(Service):
# sync has already happened (this is different from CLI yg).
# note: an edge case of dusty amounts is lost here; it will get
# picked up by Maker.try_to_create_my_orders().
if not len(self.wallet_service.get_balance_by_mixdepth(
if not len(self.services["wallet"].get_balance_by_mixdepth(
verbose=False, minconfs=1)) > 0:
raise NotEnoughCoinsForMaker()
@ -600,7 +604,7 @@ class JMWalletDaemon(Service):
@app.route('/wallet/<string:walletname>/maker/stop', methods=['GET'])
def stop_maker(self, request, walletname):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
@ -643,19 +647,20 @@ class JMWalletDaemon(Service):
def lockwallet(self, request, walletname):
print_req(request)
self.check_cookie(request)
if self.wallet_service and not self.wallet_name == walletname:
if self.services["wallet"] and not self.wallet_name == walletname:
raise InvalidRequestFormat()
if not self.wallet_service:
if not self.services["wallet"]:
jlog.warn("Called lock, but no wallet loaded")
# we could raise NoWalletFound here, but is
# easier for clients if they can gracefully call
# lock multiple times:
already_locked = True
else:
self.wallet_service.stopService()
self.services["wallet"].stopService()
self.cookie = None
self.wss_factory.valid_token = None
self.wallet_service = None
self.services["wallet"] = None
self.wallet_name = None
already_locked = False
return make_jmwalletd_response(request, walletname=walletname,
already_locked=already_locked)
@ -666,7 +671,7 @@ class JMWalletDaemon(Service):
# we only handle one wallet at a time;
# if there is a currently unlocked wallet,
# refuse to process the request:
if self.wallet_service:
if self.services["wallet"]:
raise WalletAlreadyUnlocked()
request_data = self.get_POST_body(request,
["walletname", "password", "wallettype"])
@ -716,8 +721,44 @@ class JMWalletDaemon(Service):
if not auth_json:
raise InvalidRequestFormat()
password = auth_json["password"]
wallet_path = get_wallet_path(walletname, None)
# for someone trying to re-access the wallet, using their
# password as authentication, and get a fresh token, we still
# need to authenticate against the password, but we cannot directly
# re-open the wallet file (it is currently locked), but we don't
# yet want to bother to shut down services - because if their
# authentication is successful, we can happily leave everything
# running (wallet service and e.g. a yieldgenerator).
# Hence here, if it is the same name, we do a read-only open
# and proceed to issue a new token if the open is successful,
# otherwise error.
if walletname == self.wallet_name:
try:
# returned wallet object is ditched:
open_test_wallet_maybe(
wallet_path, walletname, 4,
password=password.encode("utf-8"),
ask_for_password=False,
read_only=True)
except StoragePasswordError:
# actually effects authentication
raise NotAuthorized()
except StorageError:
# wallet is not openable, this should not happen
raise NoWalletFound()
except Exception:
# wallet file doesn't exist or is wrong format,
# this also shouldn't happen so raise:
raise NoWalletFound()
# no exceptions raised means we just return token:
self.set_token(self.wallet_name)
return make_jmwalletd_response(request,
walletname=self.wallet_name,
token=self.cookie)
# This is a different wallet than the one currently open;
# try to open it, then initialize the service(s):
try:
wallet = open_test_wallet_maybe(
wallet_path, walletname, 4,
@ -756,7 +797,7 @@ class JMWalletDaemon(Service):
@app.route('/wallet/<string:walletname>/address/new/<string:mixdepth>', methods=['GET'])
def getaddress(self, request, walletname, mixdepth):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
@ -764,19 +805,19 @@ class JMWalletDaemon(Service):
mixdepth = int(mixdepth)
except ValueError:
raise InvalidRequestFormat()
address = self.wallet_service.get_external_addr(mixdepth)
address = self.services["wallet"].get_external_addr(mixdepth)
return make_jmwalletd_response(request, address=address)
@app.route('/wallet/<string:walletname>/address/timelock/new/<string:lockdate>', methods=['GET'])
def gettimelockaddress(self, request, walletname, lockdate):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
try:
timelockaddress = wallet_gettimelockaddress(
self.wallet_service.wallet, lockdate)
self.services["wallet"].wallet, lockdate)
except Exception:
raise InvalidRequestFormat()
if timelockaddress == "":
@ -847,7 +888,7 @@ class JMWalletDaemon(Service):
# note: this does not raise or fail if the applied
# disable state (true/false) is the same as the current
# one; that is accepted and not an error.
self.wallet_service.disable_utxo(txid, index, to_disable)
self.services["wallet"].disable_utxo(txid, index, to_disable)
except AssertionError:
# should be impossible because format checked by
# utxostr_to_utxo:
@ -865,13 +906,13 @@ class JMWalletDaemon(Service):
@app.route('/wallet/<string:walletname>/utxos',methods=['GET'])
def listutxos(self, request, walletname):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
# note: the output of `showutxos` is already a string for CLI;
# but we return json:
utxos = json.loads(wallet_showutxos(self.wallet_service, False))
utxos = json.loads(wallet_showutxos(self.services["wallet"], False))
utxos_response = self.get_listutxos_response(utxos)
return make_jmwalletd_response(request, utxos=utxos_response)
@ -879,7 +920,7 @@ class JMWalletDaemon(Service):
@app.route('/wallet/<string:walletname>/taker/stop', methods=['GET'])
def stopcoinjoin(self, request, walletname):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
@ -892,7 +933,7 @@ class JMWalletDaemon(Service):
@app.route('/wallet/<string:walletname>/taker/coinjoin', methods=['POST'])
def docoinjoin(self, request, walletname):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
@ -928,7 +969,7 @@ class JMWalletDaemon(Service):
raise ConfigNotPresent()
max_cj_fee= get_max_cj_fee_values(jm_single().config,
None, user_callback=dummy_user_callback)
self.taker = Taker(self.wallet_service, schedule,
self.taker = Taker(self.services["wallet"], schedule,
max_cj_fee = max_cj_fee,
callbacks=(self.filter_orders_callback,
None, self.taker_finished))
@ -948,9 +989,9 @@ class JMWalletDaemon(Service):
@app.route('/wallet/<walletname>/getseed', methods=['GET'])
def getseed(self, request, walletname):
self.check_cookie(request)
if not self.wallet_service:
if not self.services["wallet"]:
raise NoWalletFound()
if not self.wallet_name == walletname:
raise InvalidRequestFormat()
seedphrase, _ = self.wallet_service.get_mnemonic_words()
seedphrase, _ = self.services["wallet"].get_mnemonic_words()
return make_jmwalletd_response(request, seedphrase=seedphrase)

12
jmclient/test/test_wallet_rpc.py
Unescape View File

@ -71,11 +71,11 @@ class WalletRPCTestBase(object):
# test down from 9 seconds to 1 minute 40s, which is too slow
# to be acceptable. TODO: add a test with FB by speeding up
# the sync for test, by some means or other.
self.daemon.wallet_service = make_wallets_to_list(make_wallets(
self.daemon.services["wallet"] = make_wallets_to_list(make_wallets(
1, wallet_structures=[wallet_structures[0]],
mean_amt=self.mean_amt, wallet_cls=SegwitWalletFidelityBonds))[0]
jm_single().bc_interface.tickchain()
sync_wallets([self.daemon.wallet_service])
sync_wallets([self.daemon.services["wallet"]])
# dummy tx example to force a notification event:
self.test_tx = CTransaction.deserialize(hextobin(test_tx_hex_1))
@ -176,7 +176,7 @@ class TrialTestWRPC_DisplayWallet(WalletRPCTestBase, unittest.TestCase):
"""
# before starting, we have to shut down the existing
# wallet service (usually this would be `lock`):
self.daemon.wallet_service = None
self.daemon.services["wallet"] = None
self.daemon.stopService()
self.daemon.auth_disabled = False
@ -268,12 +268,12 @@ class TrialTestWRPC_DisplayWallet(WalletRPCTestBase, unittest.TestCase):
yield self.do_request(agent, b"POST", addr, body,
self.process_direct_send_response)
# before querying the wallet display, set a label to check:
labeladdr = self.daemon.wallet_service.get_addr(0,0,0)
self.daemon.wallet_service.set_address_label(labeladdr,
labeladdr = self.daemon.services["wallet"].get_addr(0,0,0)
self.daemon.services["wallet"].set_address_label(labeladdr,
"test-wallet-rpc-label")
# force the wallet service txmonitor to wake up, to see the new
# tx before querying /display:
self.daemon.wallet_service.transaction_monitor()
self.daemon.services["wallet"].transaction_monitor()
addr = self.get_route_root()
addr += "/wallet/"
addr += self.daemon.wallet_name

Write Preview
Loading…
Cancel
Save