[error handling][xl]: refs #312

- controllers refactored to handle internal server errors without try/catch
- new tests, as previous ones were not full covering all cases
- tests refactored to assert new error messages

app/auth/annotations.py

@@ -9,7 +9,7 @@
 from flask import current_app as app
 from flask import request, _request_ctx_stack
 
-from app.utils import handle_error
+from app.utils import InvalidUsage
 from app.auth.models import JWT
 from app.auth.authorization import is_authorize
 from app.package.models import Package
@@ -38,7 +38,7 @@ def wrapped(*args, **kwargs):
                 user_id = user_info['user']
             status = check_is_authorized(action, kwargs['publisher'], kwargs['package'], user_id)
             if not status:
-                return handle_error("NOT_ALLOWED", "The operation is not allowed", 403)
+                raise InvalidUsage("The operation is not allowed", 403)
             return f(*args, **kwargs)
         return wrapped
     return wrapper
@@ -54,12 +54,11 @@ def get_user_from_jwt(req, api_key):
         token = req.values.get('jwt')
 
     if not token:
-        return False, handle_error('authorization_header_missing',
-                                   'Authorization header is expected', 401)
+        raise InvalidUsage('Authorization header is expected', 401)
     try:
         return True, jwt_helper.decode(token)
     except Exception as e:
-        return False, handle_error('jwt_error', e.message, 400)
+        raise InvalidUsage(e.message, 400)
 
 
 def check_is_authorized(action, publisher, package=None, user_id=None):
@@ -73,6 +72,6 @@ def check_is_authorized(action, publisher, package=None, user_id=None):
         publisher_name = publisher
         instance = Publisher.query.filter_by(name=publisher_name).one()
     else:
-        return handle_error("INVALID_ENTITY", "{e} is not a valid one".format(e=entity_str), 401)
+        raise InvalidUsage("{e} is not a valid one".format(e=entity_str), 401)
 
     return is_authorize(user_id, instance, action)

app/auth/controllers.py

@@ -12,7 +12,7 @@
 from app.profile.models import User
 from app.auth.models import JWT, FileData
 from app.package.models import Package
-from app.utils import handle_error
+from app.utils import InvalidUsage
 from app.auth.annotations import check_is_authorized, get_user_from_jwt
 
 auth_blueprint = Blueprint('auth', __name__, url_prefix='/api/auth')
@@ -49,36 +49,29 @@ def callback_handling():
                         description: Returns back email, nickname,
                                      picture, name
     """
-    try:
-        github = app.config['github']
-        resp = github.authorized_response()
-        if resp is None or resp.get('access_token') is None:
-            return handle_error('Access Denied',
-                                request.args.get('error_description'),
-                                400)
-        session['github_token'] = (resp['access_token'], '')
-        user_info = github.get('user')
-        user_info = user_info.data
-        # in case user Email is not public
-        if not user_info.get('email'):
-            emails = github.get('user/emails').data
-            if not len(emails):
-                return handle_error('Not Found', 'Email not found', 404)
-            for email in emails:
-                if email.get('primary'):
-                    user_info['email'] = email.get('email')
-        user = User().create_or_update_user_from_callback(user_info)
-        jwt_helper = JWT(app.config['JWT_SEED'], user.id)
-        session.pop('github_token', None)
-        g.current_user = user
-        resp = make_response(render_template("dashboard.html",
-                             title='Dashboard'), 200)
-        resp.set_cookie('jwt', jwt_helper.encode())
-        return resp
-    except Exception as e:
-        app.logger.error(e)
-        return handle_error('GENERIC_ERROR', e.message, 500)
-
+    github = app.config['github']
+    resp = github.authorized_response()
+    if resp is None or resp.get('access_token') is None:
+        raise InvalidUsage('Access Denied', 400)
+    session['github_token'] = (resp['access_token'], '')
+    user_info = github.get('user')
+    user_info = user_info.data
+    # in case user Email is not public
+    if not user_info.get('email'):
+        emails = github.get('user/emails').data
+        if not len(emails):
+            raise InvalidUsage('Email Not Found', 404)
+        for email in emails:
+            if email.get('primary'):
+                user_info['email'] = email.get('email')
+    user = User().create_or_update_user_from_callback(user_info)
+    jwt_helper = JWT(app.config['JWT_SEED'], user.id)
+    session.pop('github_token', None)
+    g.current_user = user
+    resp = make_response(render_template("dashboard.html",
+                         title='Dashboard'), 200)
+    resp.set_cookie('jwt', jwt_helper.encode())
+    return resp
 
 @auth_blueprint.route("/token", methods=['POST'])
 def get_jwt():
@@ -122,54 +115,40 @@ def get_jwt():
             description: Secret key do not match
 
     """
-    try:
-        data = request.get_json()
-        user_name = data.get('username', None)
-        email = data.get('email', None)
-        secret = data.get('secret', None)
-        verify = False
-        user_id = None
-        if user_name is None and email is None:
-            return handle_error('INVALID_INPUT',
-                                'User name or email both can not be empty',
-                                400)
-
-        if secret is None:
-            return handle_error('INVALID_INPUT',
-                                'secret can not be empty',
-                                400)
-        elif user_name is not None:
-            try:
-                user = User.query.filter_by(name=user_name).one()
-            except NoResultFound as e:
-                app.logger.error(e)
-                return handle_error('USER_NOT_FOUND',
-                                    'user does not exists',
-                                    404)
-            if secret == user.secret:
-                verify = True
-                user_id = user.id
-        elif email is not None:
-            try:
-                user = User.query.filter_by(email=email).one()
-            except NoResultFound as e:
-                app.logger.error(e)
-                return handle_error('USER_NOT_FOUND',
-                                    'user does not exists',
-                                    404)
-            if secret == user.secret:
-                verify = True
-                user_id = user.id
-        if verify:
-            return jsonify({'token': JWT(app.config['JWT_SEED'],
-                                         user_id).encode()}), 200
-        else:
-            return handle_error('SECRET_ERROR',
-                                'Secret key do not match',
-                                403)
-    except Exception as e:
-        app.logger.error(e)
-        return handle_error('GENERIC_ERROR', e.message, 500)
+    data = request.get_json()
+    user_name = data.get('username', None)
+    email = data.get('email', None)
+    secret = data.get('secret', None)
+    verify = False
+    user_id = None
+    if user_name is None and email is None:
+        raise InvalidUsage('User name or email both can not be empty', 400)
+
+    if secret is None:
+        raise InvalidUsage('Secret can not be empty', 400)
+    elif user_name is not None:
+        try:
+            user = User.query.filter_by(name=user_name).one()
+        except NoResultFound as e:
+            app.logger.error(e)
+            raise InvalidUsage('user does not exists', 404)
+        if secret == user.secret:
+            verify = True
+            user_id = user.id
+    elif email is not None:
+        try:
+            user = User.query.filter_by(email=email).one()
+        except NoResultFound as e:
+            app.logger.error(e)
+            raise InvalidUsage('user does not exists', 404)
+        if secret == user.secret:
+            verify = True
+            user_id = user.id
+    if verify:
+        return jsonify({'token': JWT(app.config['JWT_SEED'],
+                                     user_id).encode()}), 200
+    else:
+        raise InvalidUsage('Secret key do not match', 403)
 
 
 @auth_blueprint.route("/login", methods=['GET'])
@@ -211,33 +190,29 @@ def authorize_upload():
         500:
             description: Internal Server Error
     """
-    try:
-        user_id = None
-        jwt_status, user_info = get_user_from_jwt(request, app.config['JWT_SEED'])
-        if jwt_status:
-            user_id = user_info['user']
-
-        data = request.get_json()
-        metadata, filedata = data['metadata'], data['filedata']
-        publisher, package_name = metadata['owner'], metadata['name']
-        res_payload = {'filedata': {}}
-
-        if Package.is_package_exists(publisher, package_name):
-            status = check_is_authorized('Package::Update', publisher, package_name, user_id)
-        else:
-            status = check_is_authorized('Package::Create', publisher, package_name, user_id)
-
-        if not status:
-            return handle_error('UN-AUTHORIZE', 'not authorized to upload data', 400)
-
-        for relative_path in filedata.keys():
-            response = FileData(package_name=package_name,
-                                publisher=publisher,
-                                relative_path=relative_path,
-                                props=filedata[relative_path])
-            res_payload['filedata'][relative_path] = response.build_file_information()
-
-        return jsonify(res_payload), 200
-    except Exception as e:
-        app.logger.error(e)
-        return handle_error('GENERIC_ERROR', e.message, 500)
+    user_id = None
+    jwt_status, user_info = get_user_from_jwt(request, app.config['JWT_SEED'])
+    if jwt_status:
+        user_id = user_info['user']
+
+    data = request.get_json()
+    metadata, filedata = data['metadata'], data['filedata']
+    publisher, package_name = metadata['owner'], metadata['name']
+    res_payload = {'filedata': {}}
+
+    if Package.is_package_exists(publisher, package_name):
+        status = check_is_authorized('Package::Update', publisher, package_name, user_id)
+    else:
+        status = check_is_authorized('Package::Create', publisher, package_name, user_id)
+
+    if not status:
+        raise InvalidUsage('Not authorized to upload data', 400)
+
+    for relative_path in filedata.keys():
+        response = FileData(package_name=package_name,
+                            publisher=publisher,
+                            relative_path=relative_path,
+                            props=filedata[relative_path])
+        res_payload['filedata'][relative_path] = response.build_file_information()
+
+    return jsonify(res_payload), 200

app/auth/models.py

@@ -7,7 +7,7 @@
 import datetime
 import jwt
 from app.package.models import BitStore
-
+from app.utils import InvalidUsage
 
 class JWT(object):
 
@@ -40,9 +40,9 @@ def decode(self, token):
                               algorithm=self.algorithm,
                               issuer=self.issuer)
         except jwt.ExpiredSignature:
-            raise Exception("token is expired")
+            raise InvalidUsage("Token is expired")
         except jwt.DecodeError:
-            raise Exception('token signature is invalid')
+            raise InvalidUsage('Token signature is invalid')
         except Exception:
             raise Exception('Unable to parse authentication token.')