From a3949c17c879b1555d83bc82546fc8a5f7a4c343 Mon Sep 17 00:00:00 2001 From: Jonathan Harris Date: Fri, 27 Jan 2017 13:38:40 +0000 Subject: [PATCH] Use keyring for password storage Fixes #89 --- EDMC.py | 3 ++- EDMarketConnector.py | 12 +++++++++++- README.md | 6 +++--- config.py | 29 ++++++++++++++++++++++++----- prefs.py | 19 ++++++++++--------- setup.py | 4 ++-- 6 files changed, 52 insertions(+), 21 deletions(-) diff --git a/EDMC.py b/EDMC.py index d720c809..126e1b5a 100755 --- a/EDMC.py +++ b/EDMC.py @@ -83,7 +83,8 @@ try: else: session = companion.Session() if config.get('cmdrs'): - session.login(config.get('fdev_usernames')[0], config.get('fdev_passwords')[0]) + username = config.get('fdev_usernames')[0] + session.login(username, config.get_password(username)) else: # <= 2.25 not yet migrated session.login(config.get('username'), config.get('password')) querytime = int(time()) diff --git a/EDMarketConnector.py b/EDMarketConnector.py index dd853f60..de0cf687 100755 --- a/EDMarketConnector.py +++ b/EDMarketConnector.py @@ -6,6 +6,7 @@ from sys import platform from collections import OrderedDict from functools import partial import json +import keyring from os import chdir, mkdir from os.path import dirname, expanduser, isdir, join import re @@ -276,6 +277,11 @@ class AppWindow: if not getattr(sys, 'frozen', False): self.updater.checkForUpdates() # Sparkle / WinSparkle does this automatically for packaged apps + try: + config.get_password('') # Prod SecureStorage on Linux to initialise + except RuntimeError: + pass + # Migration from <= 2.25 if not config.get('cmdrs') and config.get('username') and config.get('password'): try: @@ -287,6 +293,9 @@ class AppWindow: self.postprefs() # Companion login happens in callback from monitor + if keyring.get_keyring().priority < 1: + self.status['text'] = 'Warning: Storing passwords as text' # Shouldn't happen unless no secure storage on Linux + # Try to obtain exclusive lock on journal cache, even if we don't need it yet if not eddn.load(): self.status['text'] = 'Error: Is another copy of this app already running?' # Shouldn't happen - don't bother localizing @@ -345,7 +354,8 @@ class AppWindow: if not monitor.cmdr or not config.get('cmdrs') or monitor.cmdr not in config.get('cmdrs'): raise companion.CredentialsError() idx = config.get('cmdrs').index(monitor.cmdr) - self.session.login(config.get('fdev_usernames')[idx], config.get('fdev_passwords')[idx]) + username = config.get('fdev_usernames')[idx] + self.session.login(username, config.get_password(username)) self.status['text'] = '' except companion.VerificationRequired: return prefs.AuthenticationDialog(self.w, partial(self.verify, self.login)) diff --git a/README.md b/README.md index 83d63a94..e754840e 100644 --- a/README.md +++ b/README.md @@ -165,17 +165,17 @@ Download and extract the source code of the [latest release](https://github.com/ Mac: -* Requires the Python “requests” and “watchdog” modules, plus an up-to-date “py2app” module if you also want to package the app - install these with `easy_install -U requests watchdog py2app` . +* Requires the Python “keyring”, “requests” and “watchdog” modules, plus an up-to-date “py2app” module if you also want to package the app - install these with `easy_install -U keyring requests watchdog py2app` . * Run with `./EDMarketConnector.py` . Windows: -* Requires Python2.7 and the Python “requests” and “watchdog” modules. +* Requires Python2.7 and the Python “keyring”, “requests” and “watchdog” modules, plus “py2exe” 0.6 if you also want to package the app. * Run with `EDMarketConnector.py` . Linux: -* Requires the Python “imaging-tk”, “iniparse” and “requests” modules. On Debian-based systems install these with `sudo apt-get install python-imaging-tk python-iniparse python-requests` . +* Requires the Python “imaging-tk”, “iniparse”, “keyring” and “requests” modules. On Debian-based systems install these with `sudo apt-get install python-imaging-tk python-iniparse python-keyring python-requests` . * Run with `./EDMarketConnector.py` . Command-line diff --git a/config.py b/config.py index be76293d..5481af72 100644 --- a/config.py +++ b/config.py @@ -1,3 +1,4 @@ +import keyring import numbers import sys from os import getenv, makedirs, mkdir, pardir @@ -130,12 +131,12 @@ class Config: if not getattr(sys, 'frozen', False): # Don't use Python's settings if interactive - self.bundle = 'uk.org.marginal.%s' % appname.lower() - NSBundle.mainBundle().infoDictionary()['CFBundleIdentifier'] = self.bundle + self.identifier = 'uk.org.marginal.%s' % appname.lower() + NSBundle.mainBundle().infoDictionary()['CFBundleIdentifier'] = self.identifier else: - self.bundle = NSBundle.mainBundle().bundleIdentifier() + self.identifier = NSBundle.mainBundle().bundleIdentifier() self.defaults = NSUserDefaults.standardUserDefaults() - self.settings = dict(self.defaults.persistentDomainForName_(self.bundle) or {}) # make writeable + self.settings = dict(self.defaults.persistentDomainForName_(self.identifier) or {}) # make writeable # Check out_dir exists if not self.get('outdir') or not isdir(self.get('outdir')): @@ -161,7 +162,7 @@ class Config: self.settings.pop(key, None) def save(self): - self.defaults.setPersistentDomain_forName_(self.settings, self.bundle) + self.defaults.setPersistentDomain_forName_(self.settings, self.identifier) self.defaults.synchronize() def close(self): @@ -188,6 +189,8 @@ class Config: self.respath = dirname(getattr(sys, 'frozen', False) and sys.executable or __file__) + self.identifier = applongname + self.hkey = HKEY() disposition = DWORD() if RegCreateKeyEx(HKEY_CURRENT_USER, r'Software\Marginal\EDMarketConnector', 0, None, 0, KEY_ALL_ACCESS, None, ctypes.byref(self.hkey), ctypes.byref(disposition)): @@ -281,6 +284,8 @@ class Config: self.respath = dirname(__file__) + self.identifier = 'uk.org.marginal.%s' % appname.lower() + self.filename = join(getenv('XDG_CONFIG_HOME', expanduser('~/.config')), appname, '%s.ini' % appname) if not isdir(dirname(self.filename)): makedirs(dirname(self.filename)) @@ -334,5 +339,19 @@ class Config: def __init__(self): raise NotImplementedError('Implement me') + # Common + + def get_password(self, account): + return keyring.get_password(self.identifier, account) + + def set_password(self, account, password): + keyring.set_password(self.identifier, account, password) + + def delete_password(self, account): + try: + keyring.delete_password(self.identifier, account) + except: + pass # don't care - silently fail + # singleton config = Config() diff --git a/prefs.py b/prefs.py index 5fb95e10..a96a10a8 100644 --- a/prefs.py +++ b/prefs.py @@ -352,15 +352,15 @@ class PreferencesDialog(tk.Toplevel): if monitor.cmdr and config.get('cmdrs') and monitor.cmdr in config.get('cmdrs'): config_idx = config.get('cmdrs').index(monitor.cmdr) self.username.insert(0, config.get('fdev_usernames')[config_idx] or '') - self.password.insert(0, config.get('fdev_passwords')[config_idx] or '') + self.password.insert(0, config.get_password(config.get('fdev_usernames')[config_idx]) or '') self.edsm_user.insert(0, config.get('edsm_usernames')[config_idx] or '') self.edsm_apikey.insert(0, config.get('edsm_apikeys')[config_idx] or '') elif monitor.cmdr and not config.get('cmdrs') and config.get('username') and config.get('password'): # migration from <= 2.25 - self.username.insert(0, config.get('username')[config_idx] or '') - self.password.insert(0, config.get('password')[config_idx] or '') - self.edsm_user.insert(0,config.get('edsm_cmdrname')[config_idx] or '') - self.edsm_apikey.insert(0, config.get('edsm_apikey')[config_idx] or '') + self.username.insert(0, config.get('username') or '') + self.password.insert(0, config.get('password') or '') + self.edsm_user.insert(0,config.get('edsm_cmdrname') or '') + self.edsm_apikey.insert(0, config.get('edsm_apikey') or '') self.cmdr = monitor.cmdr cmdr_state = not monitor.is_beta and monitor.cmdr and tk.NORMAL or tk.DISABLED @@ -519,17 +519,19 @@ class PreferencesDialog(tk.Toplevel): def apply(self): if self.cmdr and not monitor.is_beta: + if self.password.get().strip(): + config.set_password(self.username.get().strip(), self.password.get().strip()) # Can fail if keyring not unlocked + else: + config.delete_password(self.username.get().strip()) # user may have cleared the password field if not config.get('cmdrs'): config.set('cmdrs', [self.cmdr]) config.set('fdev_usernames', [self.username.get().strip()]) - config.set('fdev_passwords', [self.password.get().strip()]) config.set('edsm_usernames', [self.edsm_user.get().strip()]) config.set('edsm_apikeys', [self.edsm_apikey.get().strip()]) else: idx = config.get('cmdrs').index(self.cmdr) if self.cmdr in config.get('cmdrs') else -1 _putfirst('cmdrs', idx, self.cmdr) _putfirst('fdev_usernames', idx, self.username.get().strip()) - _putfirst('fdev_passwords', idx, self.password.get().strip()) _putfirst('edsm_usernames', idx, self.edsm_user.get().strip()) _putfirst('edsm_apikeys', idx, self.edsm_apikey.get().strip()) @@ -677,9 +679,9 @@ class AuthenticationDialog(tk.Toplevel): # migration from <= 2.25. Assumes current Cmdr corresponds to the saved credentials def migrate(current_cmdr): if current_cmdr and not config.get('cmdrs') and config.get('username') and config.get('password'): + config.set_password(config.get('username'), config.get('password')) # Can fail on Linux config.set('cmdrs', [current_cmdr]) config.set('fdev_usernames', [config.get('username')]) - config.set('fdev_passwords', [config.get('password')]) config.set('edsm_usernames', [config.get('edsm_cmdrname') or '']) config.set('edsm_apikeys', [config.get('edsm_apikey') or '']) # XXX to be done for release @@ -694,7 +696,6 @@ def make_current(current_cmdr): idx = config.get('cmdrs').index(current_cmdr) _putfirst('cmdrs', idx) _putfirst('fdev_usernames', idx) - _putfirst('fdev_passwords', idx) _putfirst('edsm_usernames', idx) _putfirst('edsm_apikeys', idx) diff --git a/setup.py b/setup.py index 299826d6..a02c7417 100755 --- a/setup.py +++ b/setup.py @@ -70,7 +70,7 @@ if sys.platform=='darwin': OPTIONS = { 'py2app': {'dist_dir': dist_dir, 'optimize': 2, - 'packages': [ 'requests' ], + 'packages': [ 'requests', 'keyring.backends' ], 'frameworks': [ 'Sparkle.framework' ], 'excludes': [ 'PIL', 'simplejson' ], 'iconfile': '%s.icns' % APPNAME, @@ -100,7 +100,7 @@ elif sys.platform=='win32': OPTIONS = { 'py2exe': {'dist_dir': dist_dir, 'optimize': 2, - 'packages': [ 'requests' ], + 'packages': [ 'requests', 'keyring.backends' ], 'excludes': [ 'PIL', 'simplejson' ], } }