Explicit handling of IO errors related to credentials file (#113)

* Combine _login and _silent_login into a single function
* Catch JSONDecodeError (as ValueError, for Python2 compat)
* Catch PermissionError/FileNotFoundError (as OSError/IOError or using `os`, for Python2 compat)
* Call warnings.warn() from API, click.echo() from CLI

onecodex/api.py

@@ -10,6 +10,7 @@
 import logging
 import os
 import warnings
+import errno
 
 from potion_client import Client as PotionClient
 from potion_client.converter import PotionJSONSchemaDecoder, PotionJSONDecoder, PotionJSONEncoder
@@ -18,7 +19,7 @@
 
 from onecodex.lib.auth import BearerTokenAuth
 from onecodex.models import _model_lookup
-from onecodex.utils import ModuleAlias, get_raven_client
+from onecodex.utils import ModuleAlias, get_raven_client, collapse_user
 
 log = logging.getLogger(__name__)
 
@@ -50,10 +51,19 @@ def __init__(self, api_key=None,
         # TODO: Consider only doing this if an add'l env var like
         #       'ONE_CODEX_AUTO_LOGIN' or similar is set.
         if api_key is None and bearer_token is None:
-            try:
-                api_key = json.load(open(os.path.expanduser('~/.onecodex')))['api_key']
-            except Exception:
+            creds_file = os.path.expanduser('~/.onecodex')
+
+            if not os.path.exists(creds_file):
                 pass
+            elif not os.access(creds_file, os.R_OK):
+                warnings.warn('Check permissions on {}'.format(collapse_user(creds_file)))
+            else:
+                try:
+                    api_key = json.load(open(creds_file, 'r'))['api_key']
+                except (KeyError, ValueError):
+                    warnings.warn('Credentials file ({}) is corrupt'
+                                  .format(collapse_user(creds_file)))
+
             if api_key is None:
                 api_key = os.environ.get('ONE_CODEX_API_KEY')
             if bearer_token is None:
@@ -86,12 +96,20 @@ def __init__(self, api_key=None,
                 setattr(self, module._name, module)
 
     def _fetch_account_email(self):
-        creds_fp = os.path.expanduser('~/.onecodex')
-        if os.path.exists(creds_fp):
-            with open(creds_fp) as f:
-                creds = json.load(f)
-                if 'email' in creds:
-                    return creds['email']
+        creds_file = os.path.expanduser('~/.onecodex')
+
+        if not os.path.exists(creds_file):
+            pass
+        elif not os.access(creds_file, os.R_OK):
+            warnings.warn('Check permissions on {}'.format(collapse_user(creds_file)))
+        else:
+            try:
+                return json.load(open(creds_file, 'r'))['email']
+            except KeyError:
+                pass
+            except ValueError:
+                warnings.warn('Credentials file ({}) is corrupt'.format(collapse_user(creds_file)))
+
         return os.environ.get('ONE_CODEX_USER_EMAIL', os.environ.get('ONE_CODEX_USER_UUID'))
 
     def _copy_resources(self):
@@ -128,12 +146,18 @@ def fetch(self, uri, cls=PotionJSONDecoder, **kwargs):
 
     def _fetch_schema(self, cache_schema=False, creds_file=None):
         self._cached_schema = {}
-        creds_fp = os.path.expanduser('~/.onecodex') if creds_file is None else creds_file
+        creds_file = os.path.expanduser('~/.onecodex') if creds_file is None else creds_file
+        creds = {}
 
-        if os.path.exists(creds_fp):
-            creds = json.load(open(creds_fp, 'r'))
+        if not os.path.exists(creds_file):
+            pass
+        elif not os.access(creds_file, os.R_OK):
+            warnings.warn('Check permissions on {}'.format(collapse_user(creds_file)))
         else:
-            creds = {}
+            try:
+                creds = json.load(open(creds_file, 'r'))
+            except ValueError:
+                warnings.warn('Credentials file ({}) is corrupt'.format(collapse_user(creds_file)))
 
         schema = None
         serialized_schema = None
@@ -191,7 +215,17 @@ def _fetch_schema(self, cache_schema=False, creds_file=None):
 
             # always resave the creds (to make sure we're removing schema if we need to be or
             # saving if we need to do that instead)
-            json.dump(creds, open(creds_fp, mode='w'))
+            try:
+                if creds:
+                    json.dump(creds, open(creds_file, 'w'))
+                else:
+                    os.remove(creds_file)
+            except OSError as e:
+                if e.errno == errno.EACCES:
+                    warnings.warn('Check permissions on {}'.format(collapse_user(creds_file)))
+            except IOError as e:
+                if e.errno == errno.ENOENT:
+                    pass
 
         for name, resource_schema in schema['properties'].items():
             class_name = upper_camel_case(name)

onecodex/auth.py

@@ -9,6 +9,7 @@
 import logging
 import os
 import sys
+import errno
 
 import click
 
@@ -30,66 +31,111 @@ def login_uname_pwd(server, api_key=None):
     Prompts user for username and password, gets API key from server
     if not provided.
     """
-    username = click.prompt("Please enter your One Codex (email)")
+    username = click.prompt('Please enter your One Codex (email)')
     if api_key is not None:
         return username, api_key
 
-    password = click.prompt("Please enter your password (typing will be hidden)",
+    password = click.prompt('Please enter your password (typing will be hidden)',
                             hide_input=True)
 
-    # fetech_api_key expects server to end in /
-    if server[-1] != "/":
-        server = server + "/"
+    # fetch_api_key expects server to end in /
+    if server[-1] != '/':
+        server = server + '/'
 
     # now get the API key
     api_key = fetch_api_key_from_uname(username, password, server)
     return username, api_key
 
 
-def _login(server, creds_file=None, api_key=None):
+def _login(server=None, creds_file=None, api_key=None, silent=False):
     """
     Login main function
     """
     # creds file path setup
     if creds_file is None:
-        fp = os.path.expanduser("~/.onecodex")
+        creds_file = os.path.expanduser('~/.onecodex')
+
+    # check if the creds file exists and is readable
+    if not os.path.exists(creds_file):
+        if silent:
+            return None
+
+        creds = {}
+    elif not os.access(creds_file, os.R_OK):
+        click.echo('Please check the permissions on {}'.format(collapse_user(creds_file)),
+                   err=True)
+        sys.exit(1)
     else:
-        fp = creds_file
-
-    # check if the creds file exists and has an api_key in it
-    creds = {}
-    if os.path.exists(fp):
-        try:
-            with open(fp, mode='r') as f:
-                creds = json.load(f)
-                if 'email' in creds:
-                    click.echo('Credentials file already exists ({})'.format(collapse_user(fp)),
+        # it is, so let's read it!
+        with open(creds_file, 'r') as fp:
+            try:
+                creds = json.load(fp)
+
+                # this is just so the exception is triggered if api_key and e-mail aren't there
+                creds_email = creds['email']
+                creds_api_key = creds['api_key']
+            except (KeyError, ValueError):
+                click.echo('Your ~/.onecodex credentials file appears to be corrupted. '  # noqa
+                           'Please delete it and re-authorize.', err=True)
+                sys.exit(1)
+
+        # check for updates if logged in more than one day ago
+        last_update = creds['updated_at'] if creds.get('updated_at') else creds['saved_at']
+        diff = datetime.datetime.now() - datetime.datetime.strptime(last_update,
+                                                                    DATE_FORMAT)
+        if diff.days >= 1:
+            # if creds_file is old, check for updates
+            upgrade_required, msg = check_version(__version__, 'https://app.onecodex.com/')
+            creds['updated_at'] = datetime.datetime.now().strftime(DATE_FORMAT)
+
+            try:
+                json.dump(creds, open(creds_file, 'w'))
+            except OSError as e:
+                if e.errno == errno.EACCES:
+                    click.echo('Please check the permissions on {}'
+                               .format(collapse_user(creds_file)),
                                err=True)
-                    return creds['email']
-        except ValueError:
-            click.echo("Your ~/.onecodex credentials file appears to be corrupted."  # noqa
-                       "Please delete it and re-authorize.", err=True)
-            sys.exit(1)
+                    sys.exit(1)
+
+            if upgrade_required:
+                click.echo('\nWARNING: {}\n'.format(msg), err=True)
+
+        # finally, give the user back what they want (whether silent or not)
+        if silent:
+            return creds_api_key
+
+        click.echo('Credentials file already exists ({})'.format(collapse_user(creds_file)),
+                   err=True)
+        return creds_email
 
-    # else make it
+    # creds_file was not found and we're not silent, so prompt user to login
     email, api_key = login_uname_pwd(server, api_key=api_key)
 
     if api_key is None:
-        click.echo("We could not verify your credentials. Either you mistyped your email "
-                   "or password, or your account does not currently have API access. "
-                   "Please contact help@onecodex.com if you continue to experience problems.")
+        click.echo('We could not verify your credentials. Either you mistyped your email '
+                   'or password, or your account does not currently have API access. '
+                   'Please contact help@onecodex.com if you continue to experience problems.')
         sys.exit(1)
 
-    now = datetime.datetime.now().strftime(DATE_FORMAT)
     creds.update({
         'api_key': api_key,
-        'saved_at': now,
+        'saved_at': datetime.datetime.now().strftime(DATE_FORMAT),
         'updated_at': None,
         'email': email,
     })
-    with open(fp, mode='w') as f:
-        json.dump(creds, f)
-    click.echo("Your ~/.onecodex credentials file was successfully created.", err=True)
+
+    try:
+        json.dump(creds, open(creds_file, 'w'))
+    except OSError as e:
+        if e.errno == errno.EACCES:
+            click.echo('Please check the permissions on {}'.format(creds_file),
+                       err=True)
+            sys.exit(1)
+
+        raise
+
+    click.echo('Your ~/.onecodex credentials file was successfully created.', err=True)
+
     return email
 
 
@@ -98,67 +144,33 @@ def _remove_creds(creds_file=None):
     Remove ~/.onecodex file, returning True if successul or False if the file didn't exist
     """
     if creds_file is None:
-        fp = os.path.expanduser("~/.onecodex")
-    else:
-        fp = creds_file
+        creds_file = os.path.expanduser('~/.onecodex')
+
+    if not os.path.exists(creds_file):
+        return False
 
-    if os.path.exists(fp):
-        os.remove(fp)
-        return True
-    return False
+    try:
+        os.remove(creds_file)
+    except OSError as e:
+        if e.errno == errno.EACCES:
+            click.echo('Please check the permissions on {}'.format(collapse_user(creds_file)),
+                       err=True)
+            sys.exit(1)
+
+        raise
+
+    return True
 
 
 def _logout(creds_file=None):
     """
     Logout main function, just rm ~/.onecodex more or less
     """
     if _remove_creds(creds_file=creds_file):
-        click.echo("Successfully removed One Codex credentials.", err=True)
+        click.echo('Successfully removed One Codex credentials.', err=True)
         sys.exit(0)
     else:
-        click.echo("No One Codex API keys found.", err=True)
-        sys.exit(1)
-
-
-def _silent_login(check_for_update=True):
-    """
-    This attempts to get an API key when user doesn't pass it. Used only in the CLI.
-    """
-    fp = os.path.expanduser('~/.onecodex')
-    if not os.path.exists(fp):
-        return None
-
-    try:
-        with open(fp, mode='r') as f:
-            creds = json.load(f)
-
-            # Temporary format bridge: Warn user to log out and log back in
-            # if they need an email but have an API key. Can remove on next
-            # major version bump (to v0.3.0 or later)
-            if creds.get('api_key') is not None and creds.get('email') is None:
-                click.echo('Your login has expired. Please login again using `onecodex login`')
-                return
-
-            if creds.get('email') is None or creds.get('api_key') is None:
-                return
-
-        if check_for_update:
-            last_update = creds['updated_at'] if creds.get('updated_at') else creds['saved_at']
-            diff = datetime.datetime.now() - datetime.datetime.strptime(last_update,
-                                                                        DATE_FORMAT)
-            if diff.days >= 1:
-                # Check and print warning. TODO: Consider moving this to login command as well
-                upgrade_required, msg = check_version(__version__, 'https://app.onecodex.com/')
-                creds['updated_at'] = datetime.datetime.now().strftime(DATE_FORMAT)
-                json.dump(creds, open(fp, mode='w'))
-                if upgrade_required:
-                    click.echo('\nWARNING: {}\n'.format(msg), err=True)
-
-        return creds['api_key']
-    except (KeyError, ValueError):
-        click.echo("Your ~/.onecodex credentials "
-                   "file appears to be corrupted. "
-                   "Please delete it and re-authorize.", err=True)
+        click.echo('No One Codex API keys found.', err=True)
         sys.exit(1)
 
 
@@ -174,7 +186,7 @@ def login_wrapper(ctx, *args, **kwargs):
                                  api_key=ctx.obj['API_KEY'], telemetry=ctx.obj['TELEMETRY'])
         else:
             # try and find it
-            api_key = _silent_login()
+            api_key = _login(silent=True)
             if api_key is not None:
                 ctx.obj['API'] = Api(cache_schema=True, api_key=api_key, telemetry=ctx.obj['TELEMETRY'])
             else:

onecodex/cli.py

@@ -293,9 +293,9 @@ def login(ctx):
     """Add an API key (saved in ~/.onecodex)"""
     base_url = os.environ.get("ONE_CODEX_API_BASE", "https://app.onecodex.com")
     if not ctx.obj['API_KEY']:
-        _login(base_url)
+        _login(server=base_url)
     else:
-        email = _login(base_url, api_key=ctx.obj['API_KEY'])
+        email = _login(server=base_url, api_key=ctx.obj['API_KEY'])
         ocx = Api(cache_schema=True, api_key=ctx.obj['API_KEY'], telemetry=ctx.obj['TELEMETRY'])
 
         # TODO: This should be protected or built in as a first class resource