From 238fccf7fc16694d18d3d5c8f334bf132e11f140 Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 25 Jun 2020 10:14:54 +0300 Subject: [PATCH 1/9] MVP multi user support --- src/cryptoadvance/specter/__main__.py | 2 +- src/cryptoadvance/specter/controller.py | 163 ++++++++++++------ src/cryptoadvance/specter/helpers.py | 24 ++- src/cryptoadvance/specter/hwi_server.py | 6 +- src/cryptoadvance/specter/server.py | 53 ++---- src/cryptoadvance/specter/specter.py | 122 +++++++++---- .../specter/templates/includes/hwi/hwi.jinja | 6 +- .../specter/templates/login.jinja | 6 +- .../specter/templates/register.jinja | 42 +++++ .../specter/templates/settings.jinja | 63 ++++--- src/cryptoadvance/specter/user.py | 64 +++++++ 11 files changed, 390 insertions(+), 161 deletions(-) create mode 100644 src/cryptoadvance/specter/templates/register.jinja create mode 100644 src/cryptoadvance/specter/user.py diff --git a/src/cryptoadvance/specter/__main__.py b/src/cryptoadvance/specter/__main__.py index 60010e70c1..b8ff6aacd9 100644 --- a/src/cryptoadvance/specter/__main__.py +++ b/src/cryptoadvance/specter/__main__.py @@ -11,7 +11,7 @@ from .bitcoind import (BitcoindDockerController, fetch_wallet_addresses_for_mining) -from .helpers import load_jsons, which +from .helpers import which from .server import DATA_FOLDER, create_app, init_app from os import path diff --git a/src/cryptoadvance/specter/controller.py b/src/cryptoadvance/specter/controller.py index 77b30401f4..8e74fc1490 100644 --- a/src/cryptoadvance/specter/controller.py +++ b/src/cryptoadvance/specter/controller.py @@ -13,11 +13,12 @@ from flask_login.config import EXEMPT_METHODS -from .helpers import get_devices_with_keys_by_type, run_shell, set_loglevel, get_loglevel, get_version_info +from .helpers import alias, get_devices_with_keys_by_type, run_shell, set_loglevel, get_loglevel, get_version_info from .specter import Specter from .specter_error import SpecterError from .wallet_manager import purposes from .rpc import RpcError +from .user import User from datetime import datetime import urllib @@ -98,43 +99,92 @@ def index(): @app.route('/login', methods=['GET', 'POST']) def login(): ''' login ''' - app.specter.check() - if request.method == 'POST': - # ToDo: check the password via RPC-call - if app.specter.cli is None: - flash("We could not check your password, maybe Bitcoin Core is not running or not configured?","error") - app.logger.info("AUDIT: Failed to check password") - return render_template('login.jinja', specter=app.specter, data={'controller':'controller.login'}), 401 - cli = app.specter.cli.clone() - cli.passwd = request.form['password'] - if cli.test_connection(): - app.login() - app.logger.info("AUDIT: Successfull Login via RPC-credentials") - flash('Logged in successfully.',"info") - if request.form.get('next') and request.form.get('next').startswith("http"): - response = redirect(request.form['next']) - else: - response = redirect(url_for('index')) - return response - else: - flash('Invalid username or password', "error") - app.logger.info("AUDIT: Invalid password login attempt") - return render_template('login.jinja', specter=app.specter, data={'controller':'controller.login'}), 401 + app.specter.check(None) + if request.method == 'POST': + if app.specter.config['auth'] == 'rpcpasswordaspin' or (app.specter.config['auth'] == 'usernamepassword' and request.form['username'] == 'admin'): + # TODO: check the password via RPC-call + if app.specter.cli is None: + flash("We could not check your password, maybe Bitcoin Core is not running or not configured?","error") + app.logger.info("AUDIT: Failed to check password") + return render_template('login.jinja', specter=app.specter, data={'controller':'controller.login'}), 401 + cli = app.specter.cli.clone() + cli.passwd = request.form['password'] + if cli.test_connection(): + app.login('admin') + app.logger.info("AUDIT: Successfull Login via RPC-credentials") + return redirect_login(request) + elif app.specter.config['auth'] == 'usernamepassword': + # TODO: Hash passwords - no cleartext! + # TODO: This way both "User" and "user" will pass as usernames, should there be strict check on that here? Or should we keep it like this? + username = request.form['username'] + password = request.form['password'] + user_id = alias(username) + user = User.get_user(app.specter, user_id) + if user: + if user.password == password: + app.login(user_id) + return redirect_login(request) + # Either invalid method or incorrect credentials + flash('Invalid username or password', "error") + app.logger.info("AUDIT: Invalid password login attempt") + return render_template('login.jinja', specter=app.specter, data={'controller':'controller.login'}), 401 else: if app.config.get('LOGIN_DISABLED'): return redirect('/') return render_template('login.jinja', specter=app.specter, data={'next':request.args.get('next')}) +def redirect_login(request): + flash('Logged in successfully.',"info") + if request.form.get('next') and request.form.get('next').startswith("http"): + response = redirect(request.form['next']) + else: + response = redirect(url_for('index')) + return response + +@app.route('/register', methods=['GET', 'POST']) +def register(): + ''' register ''' + app.specter.check() + if request.method == 'POST': + username = request.form['username'] + password = request.form['password'] + otp = request.form['otp'] + user_id = alias(username) + user = User.get_user(app.specter, user_id) + if user: + flash('Username is already taken, please choose another one', "error") + return redirect('/register?otp={}'.format(otp)) + # TODO: Verify and delete OTP + if app.specter.burn_new_user_otp(otp): + config = { + "explorers": { + "main": "", + "test": "", + "regtest": "", + "signet": "", + }, + "hwi_bridge_url": "/hwi/api/", + } + user = User(user_id, username, password, config) + user.save_info(app.specter) + return redirect('/login') + else: + flash('Invalid registration link, please request a new link from the node operator.', 'error') + return redirect('/register?otp={}'.format(otp)) + return render_template('register.jinja', specter=app.specter) + @app.route('/logout', methods=['GET', 'POST']) def logout(): logout_user() flash('You were logged out',"info") + app.specter.clear_user_session() return redirect("/login") @app.route('/settings/', methods=['GET', 'POST']) @login_required def settings(): current_version = notify_upgrade() + print(current_user) app.specter.check() rpc = app.specter.config['rpc'] user = rpc['user'] @@ -143,22 +193,25 @@ def settings(): host = rpc['host'] protocol = 'http' explorer = app.specter.explorer - auth = app.specter.config["auth"] - hwi_bridge_url = app.specter.config['hwi_bridge_url'] + auth = app.specter.config['auth'] + hwi_bridge_url = app.specter.hwi_bridge_url + new_otp = -1 loglevel = get_loglevel(app) if "protocol" in rpc: protocol = rpc["protocol"] test = None if request.method == 'POST': - user = request.form['username'] - passwd = request.form['password'] - port = request.form['port'] - host = request.form['host'] - explorer = request.form["explorer"] - auth = request.form['auth'] - loglevel = request.form["loglevel"] - hwi_bridge_url = request.form['hwi_bridge_url'] action = request.form['action'] + explorer = request.form['explorer'] + hwi_bridge_url = request.form['hwi_bridge_url'] + if current_user.is_admin: + user = request.form['username'] + passwd = request.form['password'] + port = request.form['port'] + host = request.form['host'] + auth = request.form['auth'] + loglevel = request.form['loglevel'] + # protocol://host if "://" in host: arr = host.split("://") @@ -173,26 +226,33 @@ def settings(): protocol=protocol, autodetect=False ) - if action == "save": - app.specter.update_rpc( user=user, - password=passwd, - port=port, - host=host, - protocol=protocol, - autodetect=False - ) - app.specter.update_explorer(explorer) - app.specter.update_auth(auth) - app.specter.update_hwi_bridge_url(hwi_bridge_url) - if auth == "rpcpasswordaspin": - app.config['LOGIN_DISABLED'] = False - else: - app.config['LOGIN_DISABLED'] = True - set_loglevel(app,loglevel) + elif action == "save": + if current_user.is_admin: + app.specter.update_rpc( user=user, + password=passwd, + port=port, + host=host, + protocol=protocol, + autodetect=False + ) + app.specter.update_auth(auth) + if auth == "rpcpasswordaspin" or auth == "usernamepassword": + app.config['LOGIN_DISABLED'] = False + else: + app.config['LOGIN_DISABLED'] = True + set_loglevel(app,loglevel) + + app.specter.update_explorer(explorer, current_user) + app.specter.update_hwi_bridge_url(hwi_bridge_url, current_user) app.specter.check() return redirect("/") - else: - pass + elif action == "adduser": + if current_user.is_admin: + new_otp = random.randint(100000, 999999) + app.specter.add_new_user_otp({ 'otp': new_otp, 'created_at': time.time() }) + flash('New user link generated successfully: {}register?otp={}'.format(request.url_root, new_otp), 'info') + else: + flash('Error: Only the admin account can issue new registration links.', 'error') return render_template("settings.jinja", test=test, username=user, @@ -203,6 +263,7 @@ def settings(): explorer=explorer, auth=auth, hwi_bridge_url=hwi_bridge_url, + new_otp=new_otp, loglevel=loglevel, specter=app.specter, current_version=current_version, diff --git a/src/cryptoadvance/specter/helpers.py b/src/cryptoadvance/specter/helpers.py index f31488692b..81ad8d4214 100644 --- a/src/cryptoadvance/specter/helpers.py +++ b/src/cryptoadvance/specter/helpers.py @@ -178,12 +178,34 @@ def get_version_info(): return current_version, latest_version, latest_version != current_version +def get_users_json(specter): + users = [ + { + 'id': 'admin', + 'username': 'admin' + } + ] + + + # if users.json file exists - load from it + if os.path.isfile(os.path.join(specter.data_folder, "users.json")): + with open(os.path.join(specter.data_folder, "users.json"), "r") as f: + users = json.loads(f.read()) + # otherwise - create one and assign unique id + else: + save_users_json(specter, users) + return users + +def save_users_json(specter, users): + with open(os.path.join(specter.data_folder, 'users.json'), "w") as f: + f.write(json.dumps(users, indent=4)) + def hwi_get_config(specter): config = { 'whitelisted_domains': 'http://127.0.0.1:25441/' } - # if config.json file exists - load from it + # if hwi_bridge_config.json file exists - load from it if os.path.isfile(os.path.join(specter.data_folder, "hwi_bridge_config.json")): with open(os.path.join(specter.data_folder, "hwi_bridge_config.json"), "r") as f: file_config = json.loads(f.read()) diff --git a/src/cryptoadvance/specter/hwi_server.py b/src/cryptoadvance/specter/hwi_server.py index a2f0507dbe..e4e0bc9752 100644 --- a/src/cryptoadvance/specter/hwi_server.py +++ b/src/cryptoadvance/specter/hwi_server.py @@ -36,7 +36,7 @@ def api(): "error": { "code": -32700, "message": "Parse error" }, "id": None }), 500 - if ('forwarded_request' not in data or not data['forwarded_request']) and (app.specter.config['hwi_bridge_url'].startswith('http://') or app.specter.config['hwi_bridge_url'].startswith('https://')): + if ('forwarded_request' not in data or not data['forwarded_request']) and (app.specter.hwi_bridge_url.startswith('http://') or app.specter.hwi_bridge_url.startswith('https://')): if ('HTTP_ORIGIN' not in request.environ): return jsonify({ "jsonrpc": "2.0", @@ -46,11 +46,11 @@ def api(): data['forwarded_request'] = True requests_session = requests.Session() requests_session.headers.update({'origin': request.environ['HTTP_ORIGIN']}) - if '.onion/' in app.specter.config['hwi_bridge_url']: + if '.onion/' in app.specter.hwi_bridge_url: requests_session.proxies = {} requests_session.proxies['http'] = 'socks5h://localhost:9050' requests_session.proxies['https'] = 'socks5h://localhost:9050' - forwarded_request = requests_session.post(app.specter.config['hwi_bridge_url'], data=json.dumps(data)) + forwarded_request = requests_session.post(app.specter.hwi_bridge_url, data=json.dumps(data)) response = json.loads(forwarded_request.content) return jsonify(response) diff --git a/src/cryptoadvance/specter/server.py b/src/cryptoadvance/specter/server.py index ab22cd527f..d992150e14 100644 --- a/src/cryptoadvance/specter/server.py +++ b/src/cryptoadvance/specter/server.py @@ -10,12 +10,14 @@ from .helpers import hwi_get_config from .specter import Specter from .hwi_server import hwi_server +from .user import User logger = logging.getLogger(__name__) env_path = Path('.') / '.flaskenv' load_dotenv(env_path) +DATA_FOLDER = "~/.specter" def create_app(config="cryptoadvance.specter.config.DevelopmentConfig"): # Enables injection of a different config via Env-Variable @@ -41,21 +43,22 @@ def init_app(app, hwibridge=False, specter=None): app.logger.info("Initializing QRcode") # Login via Flask-Login app.logger.info("Initializing LoginManager") + if specter == None: + # the default. If not None, then it got injected for testing + app.logger.info("Initializing Specter") + specter = Specter(DATA_FOLDER) + login_manager = LoginManager() login_manager.init_app(app) # Enable Login login_manager.login_view = "login" # Enable redirects if unauthorized @login_manager.user_loader - def load_user(user_id): - return AuthenticatedUser() - # let's make it a bit more convenient - def login(): - login_user(load_user("")) + def user_loader(id): + return User.get_user(specter, id) + + def login(id): + login_user(user_loader(id)) app.login = login - if specter==None: - # the default. If not None, then it got injected for testing - app.logger.info("Initializing Specter") - specter = Specter(DATA_FOLDER) # Attach specter instance so child views (e.g. hwi) can access it app.specter = specter if specter.config.get('auth') == "none": @@ -86,35 +89,3 @@ def create_and_init(): app.app_context().push() init_app(app) return app - - -DATA_FOLDER = "~/.specter" - -MSIG_TYPES = { - "legacy": "P2SH", - "p2sh-segwit": "P2SH_P2WSH", - "bech32": "P2WSH" -} -SINGLE_TYPES = { - "legacy": "P2PKH", - "p2sh-segwit": "P2SH_P2WPKH", - "bech32": "P2WPKH" -} - -class AuthenticatedUser: - ''' A minimal implementation implementing the User Class needed for Flask-Login ''' - - @property - def is_authenticated(self): - return True - - @property - def is_active(self): - return True - - @property - def is_anonymous(self): - return False - - def get_id(self): - return "there-is-only-one-User" diff --git a/src/cryptoadvance/specter/specter.py b/src/cryptoadvance/specter/specter.py index ff3583678b..5385771a9c 100644 --- a/src/cryptoadvance/specter/specter.py +++ b/src/cryptoadvance/specter/specter.py @@ -4,6 +4,8 @@ from .rpc_cache import BitcoinCLICached from .device_manager import DeviceManager from .wallet_manager import WalletManager +from .user import User +from flask_login import current_user logger = logging.getLogger(__name__) @@ -70,7 +72,7 @@ def __init__(self, data_folder="./data", config={}): # health check: loads config and tests rpc self.check() - def check(self): + def check(self, user=current_user): # if config.json file exists - load from it if os.path.isfile(os.path.join(self.data_folder, "config.json")): with open(os.path.join(self.data_folder, "config.json"), "r") as f: @@ -100,26 +102,37 @@ def check(self): self._info["chain"] = None chain = self._info["chain"] - if self.device_manager is None: - self.device_manager = DeviceManager(os.path.join(self.data_folder, "devices")) + if hasattr(user, 'is_admin'): + user_folder_id = '_' + user.id if user and not user.is_admin else '' else: - self.device_manager.update() - - if self.wallet_manager is None or chain is None: - wallets_path = "specter%s" % self.config["uid"] - self.wallet_manager = WalletManager( - os.path.join(self.data_folder, "wallets"), - self.cli, - chain, - self.device_manager, - path=wallets_path - ) - else: - self.wallet_manager.update( - os.path.join(self.data_folder, "wallets"), - self.cli, - chain=chain - ) + user_folder_id = '' + + if self.config['auth'] != 'usernamepassword' or user: + print("devices{}".format(user_folder_id)) + if self.device_manager is None: + self.device_manager = DeviceManager(os.path.join(self.data_folder, "devices{}".format(user_folder_id))) + else: + self.device_manager.update() + + if self.wallet_manager is None or chain is None: + wallets_path = "specter%s" % self.config["uid"] + self.wallet_manager = WalletManager( + os.path.join(self.data_folder, "wallets{}".format(user_folder_id)), + self.cli, + chain, + self.device_manager, + path=wallets_path + ) + else: + self.wallet_manager.update( + os.path.join(self.data_folder, "wallets{}".format(user_folder_id)), + self.cli, + chain=chain + ) + + def clear_user_session(self): + self.device_manager = None + self.wallet_manager = None def test_rpc(self, **kwargs): conf = copy.deepcopy(self.config["rpc"]) @@ -187,25 +200,49 @@ def update_auth(self, auth): self.config["auth"] = auth self._save() - def update_explorer(self, explorer): + def update_explorer(self, explorer, user): ''' update the block explorers urls ''' if explorer and not explorer.endswith("/"): # make sure the urls end with a "/" explorer += "/" # update the urls in the app config - if self.config["explorers"][self.chain] != explorer: - self.config["explorers"][self.chain] = explorer - self._save() + if user.id == 'admin': + if self.config["explorers"][self.chain] != explorer: + self.config["explorers"][self.chain] = explorer + self._save() + else: + user.set_explorer(self, explorer) - def update_hwi_bridge_url(self, url): + def update_hwi_bridge_url(self, url, user): ''' update the hwi bridge url to use ''' - if self.config["hwi_bridge_url"] != url: - if url and not url.endswith("/"): - # make sure the urls end with a "/" - url += "/" + if url and not url.endswith("/"): + # make sure the urls end with a "/" + url += "/" + if user.id == 'admin': self.config["hwi_bridge_url"] = url + self._save() + else: + user.set_hwi_bridge_url(self, url) + + def add_new_user_otp(self, otp_dict): + ''' adds an OTP for user registration ''' + if 'new_user_otps' not in self.config: + self.config['new_user_otps'] = [] + self.config['new_user_otps'].append(otp_dict) self._save() + def burn_new_user_otp(self, otp): + ''' validates an OTP for user registration and removes it if valid''' + if 'new_user_otps' not in self.config: + return False + for i, otp_dict in enumerate(self.config['new_user_otps']): + # TODO: Validate OTP did not expire based on created_at + if otp_dict['otp'] == int(otp): + del self.config['new_user_otps'][i] + self._save() + return True + return False + def combine(self, psbt_arr): final_psbt = self.cli.combinepsbt(psbt_arr) return final_psbt @@ -231,7 +268,28 @@ def chain(self): @property def explorer(self): - if "explorers" in self.config and self.chain in self.config["explorers"]: - return self.config["explorers"][self.chain] + # TODO: Unify for user and admin + if current_user.is_admin: + if "explorers" in self.config and self.chain in self.config["explorers"]: + return self.config["explorers"][self.chain] + else: + return "" else: - return "" + if "explorers" in current_user.config and self.chain in current_user.config["explorers"]: + return current_user.config["explorers"][self.chain] + else: + return "" + + @property + def hwi_bridge_url(self): + # TODO: Unify for user and admin + if current_user.is_admin: + if "hwi_bridge_url" in self.config: + return self.config["hwi_bridge_url"] + else: + return "" + else: + if "hwi_bridge_url" in current_user.config: + return current_user.config["hwi_bridge_url"] + else: + return "" diff --git a/src/cryptoadvance/specter/templates/includes/hwi/hwi.jinja b/src/cryptoadvance/specter/templates/includes/hwi/hwi.jinja index 863f6bfb96..1b2846df51 100644 --- a/src/cryptoadvance/specter/templates/includes/hwi/hwi.jinja +++ b/src/cryptoadvance/specter/templates/includes/hwi/hwi.jinja @@ -47,10 +47,10 @@ try { let hwiURL = '/hwi/api/' if ( - '{{ specter.config.hwi_bridge_url }}'.startsWith('http://localhost') || - '{{ specter.config.hwi_bridge_url }}'.startsWith('http://127.0.0.1') + '{{ specter.hwi_bridge_url }}'.startsWith('http://localhost') || + '{{ specter.hwi_bridge_url }}'.startsWith('http://127.0.0.1') ) { - hwiURL = '{{ specter.config.hwi_bridge_url }}' + hwiURL = '{{ specter.hwi_bridge_url }}' } let data = await fetch(hwiURL,{ method: 'POST', diff --git a/src/cryptoadvance/specter/templates/login.jinja b/src/cryptoadvance/specter/templates/login.jinja index e4c37705ee..5d55968e49 100644 --- a/src/cryptoadvance/specter/templates/login.jinja +++ b/src/cryptoadvance/specter/templates/login.jinja @@ -5,8 +5,12 @@ {% block main %}
+

Login to Specter

- + {% if specter.config['auth'] == 'usernamepassword' %} +

+ {% endif %} +

diff --git a/src/cryptoadvance/specter/templates/register.jinja b/src/cryptoadvance/specter/templates/register.jinja new file mode 100644 index 0000000000..5f2ff24f79 --- /dev/null +++ b/src/cryptoadvance/specter/templates/register.jinja @@ -0,0 +1,42 @@ +{% extends "base.jinja" %} + +{% block sidebar %} +{% endblock %} + +{% block main %} +
+

Register to the Specter Node

+ + {% if specter.config['auth'] == 'usernamepassword' %} +

+ {% endif %} + + +
+
+
+ +
+ +
+{% endblock %} + +{% block scripts %} + +{% endblock %} diff --git a/src/cryptoadvance/specter/templates/settings.jinja b/src/cryptoadvance/specter/templates/settings.jinja index 6e20f58a5b..1d15d62f86 100644 --- a/src/cryptoadvance/specter/templates/settings.jinja +++ b/src/cryptoadvance/specter/templates/settings.jinja @@ -3,27 +3,41 @@

App settings - Specter Desktop {{ current_version }}

-

Bitcoin JSON-RPC configuration

-
- Username:
-

- Password:
-

- Host:
-

- Port:
-
- Default ports: 8332 for mainnet, 18332 for testnet, 18443 for Regtest, 38332 for signet -
-
-

Miscellanous

-
- Authentication:
- -

+ {% if specter.config.auth == "none" or current_user.is_admin %} +

Bitcoin JSON-RPC configuration

+
+ Username:
+

+ Password:
+

+ Host:
+

+ Port:
+
+ Default ports: 8332 for mainnet, 18332 for testnet, 18443 for Regtest, 38332 for signet +
+
+

Miscellanous

+
+ Authentication:
+ +

+ {% if specter.config['auth'] == 'usernamepassword' %} + +
+ {% endif %} + Log Level:
+ +

+ {% endif %} Block explorer URL ({{ specter.chain }}):
@@ -41,13 +55,6 @@ Then update this setting here to the URL you are running Specter on (default: http://127.0.0.1:25441/hwi/api/).


- Log Level:
- -

  diff --git a/src/cryptoadvance/specter/user.py b/src/cryptoadvance/specter/user.py new file mode 100644 index 0000000000..86ec7d64d3 --- /dev/null +++ b/src/cryptoadvance/specter/user.py @@ -0,0 +1,64 @@ +from flask_login import UserMixin +from .helpers import get_users_json, save_users_json +from .specter_error import SpecterError + +class User(UserMixin): + def __init__(self, id, username, password, config): + self.id = id + self.username = username + self.password = password + self.config = config + + @classmethod + def from_json(cls, user_dict): + # TODO: Unify admin in backwards compatible way + try: + if user_dict['id'] != 'admin': + return cls(user_dict['id'], user_dict['username'], user_dict['password'], user_dict['config']) + else: + return cls(user_dict['id'], user_dict['username'], '', {}) + except: + raise SpecterError('Unable to parse user JSON.') + + @classmethod + def get_user(cls, specter, id): + users = get_users_json(specter) + for user_dict in users: + user = User.from_json(user_dict) + if user.id == id: + return user + + @property + def is_admin(self): + return self.id == 'admin' + + @property + def json(self): + user_dict = { + 'id': self.id, + 'username': self.username, + } + if not self.is_admin: + user_dict['password'] = self.password + user_dict['config'] = self.config + return user_dict + + def save_info(self, specter): + users = get_users_json(specter) + existing = False + for i in range(len(users)): + if users[i]['id'] == self.id: + users[i] = self.json + existing = True + if not existing: + users.append(self.json) + + save_users_json(specter, users) + + def set_explorer(self, specter, explorer): + self.config['explorers'][specter.chain] = explorer + self.save_info(specter) + + def set_hwi_bridge_url(self, specter, url): + self.config['hwi_bridge_url'] = url + self.save_info(specter) From 904e1f69f3db608f31bf0580aab4741a14c7b299 Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 25 Jun 2020 17:01:25 +0300 Subject: [PATCH 2/9] Hash passwords + bugfix device_manager persisting between users --- src/cryptoadvance/specter/controller.py | 10 ++++------ src/cryptoadvance/specter/helpers.py | 21 ++++++++++++++++++++- src/cryptoadvance/specter/specter.py | 3 +-- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/cryptoadvance/specter/controller.py b/src/cryptoadvance/specter/controller.py index 8e74fc1490..fe72dc6b04 100644 --- a/src/cryptoadvance/specter/controller.py +++ b/src/cryptoadvance/specter/controller.py @@ -13,7 +13,7 @@ from flask_login.config import EXEMPT_METHODS -from .helpers import alias, get_devices_with_keys_by_type, run_shell, set_loglevel, get_loglevel, get_version_info +from .helpers import alias, get_devices_with_keys_by_type, hash_password, get_loglevel, get_version_info, run_shell, set_loglevel, verify_password from .specter import Specter from .specter_error import SpecterError from .wallet_manager import purposes @@ -99,7 +99,7 @@ def index(): @app.route('/login', methods=['GET', 'POST']) def login(): ''' login ''' - app.specter.check(None) + app.specter.check() if request.method == 'POST': if app.specter.config['auth'] == 'rpcpasswordaspin' or (app.specter.config['auth'] == 'usernamepassword' and request.form['username'] == 'admin'): # TODO: check the password via RPC-call @@ -114,14 +114,13 @@ def login(): app.logger.info("AUDIT: Successfull Login via RPC-credentials") return redirect_login(request) elif app.specter.config['auth'] == 'usernamepassword': - # TODO: Hash passwords - no cleartext! # TODO: This way both "User" and "user" will pass as usernames, should there be strict check on that here? Or should we keep it like this? username = request.form['username'] password = request.form['password'] user_id = alias(username) user = User.get_user(app.specter, user_id) if user: - if user.password == password: + if verify_password(user.password, password): app.login(user_id) return redirect_login(request) # Either invalid method or incorrect credentials @@ -147,14 +146,13 @@ def register(): app.specter.check() if request.method == 'POST': username = request.form['username'] - password = request.form['password'] + password = hash_password(request.form['password']) otp = request.form['otp'] user_id = alias(username) user = User.get_user(app.specter, user_id) if user: flash('Username is already taken, please choose another one', "error") return redirect('/register?otp={}'.format(otp)) - # TODO: Verify and delete OTP if app.specter.burn_new_user_otp(otp): config = { "explorers": { diff --git a/src/cryptoadvance/specter/helpers.py b/src/cryptoadvance/specter/helpers.py index 81ad8d4214..523dd44194 100644 --- a/src/cryptoadvance/specter/helpers.py +++ b/src/cryptoadvance/specter/helpers.py @@ -1,4 +1,4 @@ -import collections, copy, hashlib, json, logging, os, six, subprocess, sys +import binascii, collections, copy, hashlib, json, logging, os, six, subprocess, sys from collections import OrderedDict from .descriptor import AddChecksum @@ -295,3 +295,22 @@ def sort_descriptor(cli, descriptor, index=None, change=False): desc = f"{p}({desc})" return AddChecksum(desc) + +def hash_password(password): + """Hash a password for storing.""" + salt = hashlib.sha256(os.urandom(60)).hexdigest().encode('ascii') + pwdhash = hashlib.pbkdf2_hmac('sha512', password.encode('utf-8'), + salt, 10000) + pwdhash = binascii.hexlify(pwdhash) + return (salt + pwdhash).decode('ascii') + +def verify_password(stored_password, provided_password): + """Verify a stored password against one provided by user""" + salt = stored_password[:64] + stored_password = stored_password[64:] + pwdhash = hashlib.pbkdf2_hmac('sha512', + provided_password.encode('utf-8'), + salt.encode('ascii'), + 10000) + pwdhash = binascii.hexlify(pwdhash).decode('ascii') + return pwdhash == stored_password diff --git a/src/cryptoadvance/specter/specter.py b/src/cryptoadvance/specter/specter.py index 5385771a9c..ede1caf0da 100644 --- a/src/cryptoadvance/specter/specter.py +++ b/src/cryptoadvance/specter/specter.py @@ -107,8 +107,7 @@ def check(self, user=current_user): else: user_folder_id = '' - if self.config['auth'] != 'usernamepassword' or user: - print("devices{}".format(user_folder_id)) + if self.config['auth'] != 'usernamepassword' or (user and not user.is_anonymous): if self.device_manager is None: self.device_manager = DeviceManager(os.path.join(self.data_folder, "devices{}".format(user_folder_id))) else: From 0f73225aa9bcf8a1d7e49d1e3e692b344fe3fa86 Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 25 Jun 2020 18:11:31 +0300 Subject: [PATCH 3/9] Fix the tests --- src/cryptoadvance/specter/controller.py | 5 ++++- src/cryptoadvance/specter/specter.py | 4 ++-- tests/conftest.py | 2 +- tests/test_controller.py | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cryptoadvance/specter/controller.py b/src/cryptoadvance/specter/controller.py index fe72dc6b04..ca493b0b52 100644 --- a/src/cryptoadvance/specter/controller.py +++ b/src/cryptoadvance/specter/controller.py @@ -101,6 +101,10 @@ def login(): ''' login ''' app.specter.check() if request.method == 'POST': + if app.specter.config['auth'] == 'none': + app.login('admin') + app.logger.info("AUDIT: Successfull Login no credentials") + return redirect_login(request) if app.specter.config['auth'] == 'rpcpasswordaspin' or (app.specter.config['auth'] == 'usernamepassword' and request.form['username'] == 'admin'): # TODO: check the password via RPC-call if app.specter.cli is None: @@ -182,7 +186,6 @@ def logout(): @login_required def settings(): current_version = notify_upgrade() - print(current_user) app.specter.check() rpc = app.specter.config['rpc'] user = rpc['user'] diff --git a/src/cryptoadvance/specter/specter.py b/src/cryptoadvance/specter/specter.py index ede1caf0da..f1fce3f73a 100644 --- a/src/cryptoadvance/specter/specter.py +++ b/src/cryptoadvance/specter/specter.py @@ -268,7 +268,7 @@ def chain(self): @property def explorer(self): # TODO: Unify for user and admin - if current_user.is_admin: + if (not current_user or current_user.is_anonymous) or current_user.is_admin: if "explorers" in self.config and self.chain in self.config["explorers"]: return self.config["explorers"][self.chain] else: @@ -282,7 +282,7 @@ def explorer(self): @property def hwi_bridge_url(self): # TODO: Unify for user and admin - if current_user.is_admin: + if (not current_user or current_user.is_anonymous) or current_user.is_admin: if "hwi_bridge_url" in self.config: return self.config["hwi_bridge_url"] else: diff --git a/tests/conftest.py b/tests/conftest.py index a15b320e51..afa9e82717 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -226,7 +226,7 @@ def specter_regtest_configured(bitcoin_regtest, devices_filled_data_folder): "host": bitcoin_regtest.rpcconn.ipaddress, "protocol": "http" }, - "auth": "none" + "auth": "rpcpasswordaspin" } specter = Specter(data_folder=devices_filled_data_folder, config=config) specter.check() diff --git a/tests/test_controller.py b/tests/test_controller.py index 6a1e01c2a6..529648ef17 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -5,6 +5,7 @@ def test_home(caplog, client): ''' The root of the app ''' caplog.set_level(logging.INFO) caplog.set_level(logging.DEBUG,logger="cryptoadvance.specter") + login(client, 'secret') result = client.get('/') # By default there is no authentication assert result.status_code == 200 # OK. @@ -24,11 +25,10 @@ def test_home(caplog, client): def test_login_logout(caplog, app, client): ''' whether we can login or logout ''' caplog.set_level(logging.DEBUG,logger="cryptoadvance.specter") - app.config['LOGIN_DISABLED'] = False result = client.get('/login', follow_redirects=False) assert result.status_code == 200 - assert b'Pin' in result.data + assert b'Password' in result.data result = login(client, 'secret') assert b'Logged in successfully.' in result.data result = logout(client) From ec816b3219bdcad6cb632f6581d6b29cb4cac9c8 Mon Sep 17 00:00:00 2001 From: benk10 Date: Fri, 26 Jun 2020 16:22:32 +0300 Subject: [PATCH 4/9] Allow username and password editing --- src/cryptoadvance/specter/controller.py | 39 ++++++++++++++++--- src/cryptoadvance/specter/helpers.py | 4 +- .../specter/templates/settings.jinja | 22 +++++++---- src/cryptoadvance/specter/user.py | 22 +++++++---- 4 files changed, 64 insertions(+), 23 deletions(-) diff --git a/src/cryptoadvance/specter/controller.py b/src/cryptoadvance/specter/controller.py index ca493b0b52..752a0d06f1 100644 --- a/src/cryptoadvance/specter/controller.py +++ b/src/cryptoadvance/specter/controller.py @@ -105,7 +105,7 @@ def login(): app.login('admin') app.logger.info("AUDIT: Successfull Login no credentials") return redirect_login(request) - if app.specter.config['auth'] == 'rpcpasswordaspin' or (app.specter.config['auth'] == 'usernamepassword' and request.form['username'] == 'admin'): + if app.specter.config['auth'] == 'rpcpasswordaspin': # TODO: check the password via RPC-call if app.specter.cli is None: flash("We could not check your password, maybe Bitcoin Core is not running or not configured?","error") @@ -121,11 +121,10 @@ def login(): # TODO: This way both "User" and "user" will pass as usernames, should there be strict check on that here? Or should we keep it like this? username = request.form['username'] password = request.form['password'] - user_id = alias(username) - user = User.get_user(app.specter, user_id) + user = User.get_user_by_name(app.specter, username) if user: if verify_password(user.password, password): - app.login(user_id) + app.login(user.id) return redirect_login(request) # Either invalid method or incorrect credentials flash('Invalid username or password', "error") @@ -153,8 +152,7 @@ def register(): password = hash_password(request.form['password']) otp = request.form['otp'] user_id = alias(username) - user = User.get_user(app.specter, user_id) - if user: + if User.get_user(app.specter, user_id) or User.get_user_by_name(app.specter, username): flash('Username is already taken, please choose another one', "error") return redirect('/register?otp={}'.format(otp)) if app.specter.burn_new_user_otp(otp): @@ -205,6 +203,12 @@ def settings(): action = request.form['action'] explorer = request.form['explorer'] hwi_bridge_url = request.form['hwi_bridge_url'] + if 'specter_username' in request.form: + specter_username = request.form['specter_username'] + specter_password = request.form['specter_password'] + else: + specter_username = None + specter_password = None if current_user.is_admin: user = request.form['username'] passwd = request.form['password'] @@ -228,6 +232,29 @@ def settings(): autodetect=False ) elif action == "save": + if specter_username: + if current_user.username != specter_username: + if User.get_user_by_name(app.specter, specter_username): + flash('Username is already taken, please choose another one', "error") + return render_template("settings.jinja", + test=test, + username=user, + password=passwd, + port=port, + host=host, + protocol=protocol, + explorer=explorer, + auth=auth, + hwi_bridge_url=hwi_bridge_url, + new_otp=new_otp, + loglevel=loglevel, + specter=app.specter, + current_version=current_version, + rand=rand) + current_user.username = specter_username + if specter_password: + current_user.password = hash_password(specter_password) + current_user.save_info(app.specter) if current_user.is_admin: app.specter.update_rpc( user=user, password=passwd, diff --git a/src/cryptoadvance/specter/helpers.py b/src/cryptoadvance/specter/helpers.py index 523dd44194..f0fc1bb5bb 100644 --- a/src/cryptoadvance/specter/helpers.py +++ b/src/cryptoadvance/specter/helpers.py @@ -182,7 +182,9 @@ def get_users_json(specter): users = [ { 'id': 'admin', - 'username': 'admin' + 'username': 'admin', + 'password': hash_password('admin'), + 'is_admin': True } ] diff --git a/src/cryptoadvance/specter/templates/settings.jinja b/src/cryptoadvance/specter/templates/settings.jinja index 1d15d62f86..baf67391a8 100644 --- a/src/cryptoadvance/specter/templates/settings.jinja +++ b/src/cryptoadvance/specter/templates/settings.jinja @@ -19,6 +19,13 @@

Miscellanous


+ Log Level:
+ +

Authentication:
- - - - -

+ Default is username: admin, password: admin.
+ {% endif %} + {% if specter.config['auth'] == 'usernamepassword' %} + Specter Username:


+ Specter Password:

+

{% endif %} Block explorer URL ({{ specter.chain }}):
@@ -45,7 +51,7 @@ This setting is only to allow opening trnsactions and addresses in a block explorer directly from Specter.
All data Specter uses still comes directly from the your own connected full node.
-

+
HWI Bridge URL:
diff --git a/src/cryptoadvance/specter/user.py b/src/cryptoadvance/specter/user.py index 86ec7d64d3..f95a3daffa 100644 --- a/src/cryptoadvance/specter/user.py +++ b/src/cryptoadvance/specter/user.py @@ -3,20 +3,21 @@ from .specter_error import SpecterError class User(UserMixin): - def __init__(self, id, username, password, config): + def __init__(self, id, username, password, config, is_admin=False): self.id = id self.username = username self.password = password self.config = config + self.is_admin = is_admin @classmethod def from_json(cls, user_dict): # TODO: Unify admin in backwards compatible way try: - if user_dict['id'] != 'admin': + if not user_dict['is_admin']: return cls(user_dict['id'], user_dict['username'], user_dict['password'], user_dict['config']) else: - return cls(user_dict['id'], user_dict['username'], '', {}) + return cls(user_dict['id'], user_dict['username'], user_dict['password'], {}, is_admin=True) except: raise SpecterError('Unable to parse user JSON.') @@ -27,19 +28,24 @@ def get_user(cls, specter, id): user = User.from_json(user_dict) if user.id == id: return user - - @property - def is_admin(self): - return self.id == 'admin' + + @classmethod + def get_user_by_name(cls, specter, username): + users = get_users_json(specter) + for user_dict in users: + user = User.from_json(user_dict) + if user.username == username: + return user @property def json(self): user_dict = { 'id': self.id, 'username': self.username, + 'password': self.password, + 'is_admin': self.is_admin } if not self.is_admin: - user_dict['password'] = self.password user_dict['config'] = self.config return user_dict From 31b15715423e7ff937ce27fb85d51bfcd3f6ec77 Mon Sep 17 00:00:00 2001 From: benk10 Date: Fri, 26 Jun 2020 16:25:14 +0300 Subject: [PATCH 5/9] Fix note in settings UI --- src/cryptoadvance/specter/templates/settings.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptoadvance/specter/templates/settings.jinja b/src/cryptoadvance/specter/templates/settings.jinja index baf67391a8..357e7d625e 100644 --- a/src/cryptoadvance/specter/templates/settings.jinja +++ b/src/cryptoadvance/specter/templates/settings.jinja @@ -36,8 +36,8 @@ {% if specter.config['auth'] == 'usernamepassword' %}
+ Default is username: admin, password: admin.
{% endif %} - Default is username: admin, password: admin.
{% endif %} {% if specter.config['auth'] == 'usernamepassword' %} Specter Username:


From b467a294a0e8e22b702740522aea3e1d36837e23 Mon Sep 17 00:00:00 2001 From: benk10 Date: Fri, 26 Jun 2020 16:59:28 +0300 Subject: [PATCH 6/9] Login as admin if auth is none --- src/cryptoadvance/specter/controller.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cryptoadvance/specter/controller.py b/src/cryptoadvance/specter/controller.py index 752a0d06f1..7ffe3523dd 100644 --- a/src/cryptoadvance/specter/controller.py +++ b/src/cryptoadvance/specter/controller.py @@ -132,6 +132,7 @@ def login(): return render_template('login.jinja', specter=app.specter, data={'controller':'controller.login'}), 401 else: if app.config.get('LOGIN_DISABLED'): + app.login('admin') return redirect('/') return render_template('login.jinja', specter=app.specter, data={'next':request.args.get('next')}) From dc09e63e1f446fb43de77b4c1a24296bd25209b8 Mon Sep 17 00:00:00 2001 From: benk10 Date: Fri, 26 Jun 2020 17:23:00 +0300 Subject: [PATCH 7/9] Fix bug moving to multi user --- src/cryptoadvance/specter/controller.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cryptoadvance/specter/controller.py b/src/cryptoadvance/specter/controller.py index 7ffe3523dd..01dd4b1ee6 100644 --- a/src/cryptoadvance/specter/controller.py +++ b/src/cryptoadvance/specter/controller.py @@ -194,6 +194,8 @@ def settings(): protocol = 'http' explorer = app.specter.explorer auth = app.specter.config['auth'] + if auth == 'none': + app.login('admin') hwi_bridge_url = app.specter.hwi_bridge_url new_otp = -1 loglevel = get_loglevel(app) From a7f45a7ac748c1e4f9591558e7b2bfae1044e43e Mon Sep 17 00:00:00 2001 From: benk10 Date: Fri, 26 Jun 2020 19:47:38 +0300 Subject: [PATCH 8/9] Fix device manager bug in multi user mode --- src/cryptoadvance/specter/device_manager.py | 6 +++--- src/cryptoadvance/specter/specter.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cryptoadvance/specter/device_manager.py b/src/cryptoadvance/specter/device_manager.py index 0203987c52..cd5f210178 100644 --- a/src/cryptoadvance/specter/device_manager.py +++ b/src/cryptoadvance/specter/device_manager.py @@ -29,6 +29,9 @@ class DeviceManager: ''' # of them via json-files in an empty data folder def __init__(self, data_folder): + self.update(data_folder=data_folder) + + def update(self, data_folder=None): if data_folder is not None: self.data_folder = data_folder if data_folder.startswith("~"): @@ -36,9 +39,6 @@ def __init__(self, data_folder): # creating folders if they don't exist if not os.path.isdir(data_folder): os.mkdir(data_folder) - self.update() - - def update(self): self.devices = {} devices_files = load_jsons(self.data_folder, key="name") for device_alias in devices_files: diff --git a/src/cryptoadvance/specter/specter.py b/src/cryptoadvance/specter/specter.py index f1fce3f73a..02349760ce 100644 --- a/src/cryptoadvance/specter/specter.py +++ b/src/cryptoadvance/specter/specter.py @@ -111,7 +111,7 @@ def check(self, user=current_user): if self.device_manager is None: self.device_manager = DeviceManager(os.path.join(self.data_folder, "devices{}".format(user_folder_id))) else: - self.device_manager.update() + self.device_manager.update(data_folder=os.path.join(self.data_folder, "devices{}".format(user_folder_id))) if self.wallet_manager is None or chain is None: wallets_path = "specter%s" % self.config["uid"] From e54e2156571ef89e6393adcfcfdf638e3a455f38 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sat, 27 Jun 2020 09:52:28 +0300 Subject: [PATCH 9/9] Improve password storing and username password settings UI --- src/cryptoadvance/specter/helpers.py | 17 ++++------- .../specter/templates/settings.jinja | 29 +++++++++++++++---- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/cryptoadvance/specter/helpers.py b/src/cryptoadvance/specter/helpers.py index f0fc1bb5bb..1d6e318229 100644 --- a/src/cryptoadvance/specter/helpers.py +++ b/src/cryptoadvance/specter/helpers.py @@ -300,19 +300,14 @@ def sort_descriptor(cli, descriptor, index=None, change=False): def hash_password(password): """Hash a password for storing.""" - salt = hashlib.sha256(os.urandom(60)).hexdigest().encode('ascii') - pwdhash = hashlib.pbkdf2_hmac('sha512', password.encode('utf-8'), - salt, 10000) - pwdhash = binascii.hexlify(pwdhash) - return (salt + pwdhash).decode('ascii') + salt = binascii.b2a_base64(hashlib.sha256(os.urandom(60)).digest()).strip() + pwdhash = binascii.b2a_base64(hashlib.pbkdf2_hmac('sha256', password.encode('utf-8'), salt, 10000)).strip().decode() + return { 'salt': salt.decode(), 'pwdhash': pwdhash } def verify_password(stored_password, provided_password): """Verify a stored password against one provided by user""" - salt = stored_password[:64] - stored_password = stored_password[64:] - pwdhash = hashlib.pbkdf2_hmac('sha512', + pwdhash = hashlib.pbkdf2_hmac('sha256', provided_password.encode('utf-8'), - salt.encode('ascii'), + stored_password['salt'].encode(), 10000) - pwdhash = binascii.hexlify(pwdhash).decode('ascii') - return pwdhash == stored_password + return pwdhash == binascii.a2b_base64(stored_password['pwdhash']) diff --git a/src/cryptoadvance/specter/templates/settings.jinja b/src/cryptoadvance/specter/templates/settings.jinja index 357e7d625e..5ce80f6314 100644 --- a/src/cryptoadvance/specter/templates/settings.jinja +++ b/src/cryptoadvance/specter/templates/settings.jinja @@ -27,7 +27,7 @@

Authentication:
- @@ -39,11 +39,11 @@ Default is username: admin, password: admin.
{% endif %} {% endif %} - {% if specter.config['auth'] == 'usernamepassword' %} - Specter Username:


- Specter Password:

-

- {% endif %} +
+ Specter Username:


+ Specter Password:

+

+
Block explorer URL ({{ specter.chain }}):
@@ -112,3 +112,20 @@
{% endblock %} + +{% block scripts %} + +{% endblock %}