diff --git a/endpoints/api.py b/endpoints/api.py index c0f10effc..bd2ddf78f 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -55,6 +55,38 @@ def csrf_protect(): req_user) +@api.errorhandler(404) +def endpoint_not_found(e): + return jsonify({ + 'error_code': 404, + 'message': 'Resource not found' + }) + + +@api.errorhandler(403) +def endpoint_forbidden(e): + return jsonify({ + 'error_code': 403, + 'message': 'Permission Denied' + }) + + +@api.errorhandler(400) +def endpoint_invalid_request(e): + return jsonify({ + 'error_code': 400, + 'message': 'Invalid Request' + }) + + +def request_error(exception=None, **kwargs): + data = kwargs.copy() + if exception: + data['message'] = exception.message + + return make_response(jsonify(data), 400) + + def get_route_data(): global route_data if route_data: @@ -139,7 +171,7 @@ def discovery(): @api.route('/') @internal_api_call def welcome(): - return make_response('welcome', 200) + return jsonify({'version': '0.5'}) @api.route('/plans/') @@ -229,20 +261,14 @@ def convert_user_to_organization(): # Ensure that the new admin user is the not user being converted. admin_username = convert_data['adminUser'] if admin_username == user.username: - error_resp = jsonify({ - 'reason': 'invaliduser' - }) - error_resp.status_code = 400 - return error_resp + return request_error(reason='invaliduser', + message='The admin user is not valid') # Ensure that the sign in credentials work. admin_password = convert_data['adminPassword'] if not model.verify_user(admin_username, admin_password): - error_resp = jsonify({ - 'reason': 'invaliduser' - }) - error_resp.status_code = 400 - return error_resp + return request_error(reason='invaliduser', + message='The admin user credentials are not valid') # Subscribe the organization to the new plan. plan = convert_data['plan'] @@ -278,22 +304,15 @@ def change_user_details(): new_email = user_data['email'] if model.find_user_by_email(new_email): # Email already used. - error_resp = jsonify({ - 'message': 'E-mail address already used' - }) - error_resp.status_code = 400 - return error_resp + return request_error(message='E-mail address already used') - logger.debug('Sending email to change email address for user: %s', user.username) + logger.debug('Sending email to change email address for user: %s', + user.username) code = model.create_confirm_email_code(user, new_email=new_email) send_change_email(user.username, user_data['email'], code.code) except model.InvalidPasswordException, ex: - error_resp = jsonify({ - 'message': ex.message, - }) - error_resp.status_code = 400 - return error_resp + return request_error(exception=ex) return jsonify(user_view(user)) @@ -305,11 +324,7 @@ def create_new_user(): existing_user = model.get_user(user_data['username']) if existing_user: - error_resp = jsonify({ - 'message': 'The username already exists' - }) - error_resp.status_code = 400 - return error_resp + return request_error(message='The username already exists') try: new_user = model.create_user(user_data['username'], user_data['password'], @@ -318,11 +333,7 @@ def create_new_user(): send_confirmation_email(new_user.username, new_user.email, code.code) return make_response('Created', 201) except model.DataModelException as ex: - error_resp = jsonify({ - 'message': ex.message, - }) - error_resp.status_code = 400 - return error_resp + return request_error(exception=ex) @api.route('/signin', methods=['POST']) @@ -343,7 +354,7 @@ def conduct_signin(username_or_email, password): verified = model.verify_user(username_or_email, password) if verified: if common_login(verified): - return make_response('Success', 200) + return jsonify({'success': True}) else: needs_email_verification = True @@ -364,7 +375,7 @@ def conduct_signin(username_or_email, password): def logout(): logout_user() identity_changed.send(app, identity=AnonymousIdentity()) - return make_response('Success', 200) + return jsonify({'success': True}) @api.route("/recovery", methods=['POST']) @@ -466,22 +477,15 @@ def create_organization(): pass if existing: - error_resp = jsonify({ - 'message': 'A user or organization with this name already exists' - }) - error_resp.status_code = 400 - return error_resp + msg = 'A user or organization with this name already exists' + return request_error(message=msg) try: model.create_organization(org_data['name'], org_data['email'], current_user.db_user()) return make_response('Created', 201) except model.DataModelException as ex: - error_resp = jsonify({ - 'message': ex.message, - }) - error_resp.status_code = 400 - return error_resp + return request_error(exception=ex) def org_view(o, teams): @@ -536,12 +540,7 @@ def change_organization_details(orgname): if 'email' in org_data and org_data['email'] != org.email: new_email = org_data['email'] if model.find_user_by_email(new_email): - # Email already used. - error_resp = jsonify({ - 'message': 'E-mail address already used' - }) - error_resp.status_code = 400 - return error_resp + return request_error(message='E-mail address already used') logger.debug('Changing email address for organization: %s', org.username) model.update_email(org, new_email) @@ -626,7 +625,8 @@ def create_organization_prototype_permission(orgname): details = request.get_json() activating_username = None - if 'activating_user' in details and details['activating_user'] and 'name' in details['activating_user']: + if ('activating_user' in details and details['activating_user'] and + 'name' in details['activating_user']): activating_username = details['activating_user']['name'] delegate = details['delegate'] @@ -644,10 +644,10 @@ def create_organization_prototype_permission(orgname): if delegate_teamname else None) if activating_username and not activating_user: - abort(404) + return request_error(message='Unknown activating user') if not delegate_user and not delegate_team: - abort(400) + return request_error(message='Missing delagate user or team') role_name = details['role'] @@ -905,7 +905,7 @@ def update_organization_team_member(orgname, teamname, membername): # Find the user. user = model.get_user(membername) if not user: - abort(400) + return request_error(message='Unknown user') # Add the user to the team. model.add_user_to_team(user, team) @@ -946,7 +946,7 @@ def create_repo(): existing = model.get_repository(namespace_name, repository_name) if existing: - return make_response('Repository already exists', 400) + return request_error(message='Repository already exists') visibility = req['visibility'] @@ -1019,7 +1019,7 @@ def list_repos(): if page: try: page = int(page) - except: + except Exception: page = None username = None @@ -1550,11 +1550,7 @@ def change_user_permissions(namespace, repository, username): # This repository is not part of an organization pass except model.DataModelException as ex: - error_resp = jsonify({ - 'message': ex.message, - }) - error_resp.status_code = 400 - return error_resp + return request_error(exception=ex) log_action('change_repo_permission', namespace, {'username': username, 'repo': repository, @@ -1607,11 +1603,7 @@ def delete_user_permissions(namespace, repository, username): try: model.delete_user_permission(username, namespace, repository) except model.DataModelException as ex: - error_resp = jsonify({ - 'message': ex.message, - }) - error_resp.status_code = 400 - return error_resp + return request_error(exception=ex) log_action('delete_repo_permission', namespace, {'username': username, 'repo': repository}, @@ -1867,7 +1859,7 @@ def subscribe(user, plan, token, require_business_plan): plan_found['price'] == 0): logger.warning('Business attempting to subscribe to personal plan: %s', user.username) - abort(400) + return request_error(message='No matching plan found') private_repos = model.get_private_repo_count(user.username) @@ -2097,7 +2089,7 @@ def delete_user_robot(robot_shortname): parent = current_user.db_user() model.delete_robot(format_robot_username(parent.username, robot_shortname)) log_action('delete_robot', parent.username, {'robot': robot_shortname}) - return make_response('No Content', 204) + return make_response('Deleted', 204) @api.route('/organization//robots/', @@ -2109,7 +2101,7 @@ def delete_org_robot(orgname, robot_shortname): if permission.can(): model.delete_robot(format_robot_username(orgname, robot_shortname)) log_action('delete_robot', orgname, {'robot': robot_shortname}) - return make_response('No Content', 204) + return make_response('Deleted', 204) abort(403) diff --git a/endpoints/common.py b/endpoints/common.py index 2648f5e9c..ec4727edb 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -2,7 +2,7 @@ import logging import os import base64 -from flask import request, abort, session +from flask import request, abort, session, make_response from flask.ext.login import login_user, UserMixin from flask.ext.principal import identity_changed diff --git a/endpoints/index.py b/endpoints/index.py index b3896bf91..94a5d4fef 100644 --- a/endpoints/index.py +++ b/endpoints/index.py @@ -2,12 +2,12 @@ import json import logging import urlparse -from flask import request, make_response, jsonify, abort, session, Blueprint +from flask import request, make_response, jsonify, session, Blueprint from functools import wraps from data import model from data.queue import webhook_queue -from app import app, mixpanel +from app import mixpanel from auth.auth import (process_auth, get_authenticated_user, get_validated_token) from util.names import parse_repository_name @@ -16,6 +16,9 @@ from auth.permissions import (ModifyRepositoryPermission, UserPermission, ReadRepositoryPermission, CreateRepositoryPermission) +from util.http import abort + + logger = logging.getLogger(__name__) index = Blueprint('index', __name__) @@ -52,6 +55,19 @@ def generate_headers(role='read'): return decorator_method +@index.errorhandler(404) +def fallback_not_found(e): + return make_response('Not Found', 404) + +@index.errorhandler(403) +def fallback_forbidden(e): + return make_response('Forbidden', 403) + +@index.errorhandler(400) +def fallback_invalid_request(e): + return make_response('Invalid Request', 400) + + @index.route('/users', methods=['POST']) @index.route('/users/', methods=['POST']) def create_user(): @@ -64,14 +80,14 @@ def create_user(): model.load_token_data(password) return make_response('Verified', 201) except model.InvalidTokenException: - return make_response('Invalid access token.', 400) + abort(400, 'Invalid access token.', issue='invalid-access-token') elif '+' in username: try: model.verify_robot(username, password) return make_response('Verified', 201) except model.InvalidRobotException: - return make_response('Invalid robot account or password.', 400) + abort(400, 'Invalid robot account or password.', issue='robot-login-failure') existing_user = model.get_user(username) if existing_user: @@ -79,7 +95,8 @@ def create_user(): if verified: return make_response('Verified', 201) else: - return make_response('Invalid password.', 400) + abort(400, 'Invalid password.', issue='login-failure') + else: # New user case new_user = model.create_user(username, password, user_data['email']) @@ -131,23 +148,30 @@ def update_user(username): @generate_headers(role='write') def create_repository(namespace, repository): image_descriptions = json.loads(request.data) - repo = model.get_repository(namespace, repository) if not repo and get_authenticated_user() is None: logger.debug('Attempt to create new repository without user auth.') - abort(401) + abort(401, + message='Cannot create a repository as a guest. Please login via "docker login" first.', + issue='no-login') elif repo: permission = ModifyRepositoryPermission(namespace, repository) if not permission.can(): - abort(403) + abort(403, + message='You do not have permission to modify repository %(namespace)s/%(repository)s', + issue='no-repo-write-permission', + namespace=namespace, repository=repository) else: permission = CreateRepositoryPermission(namespace) if not permission.can(): logger.info('Attempt to create a new repo with insufficient perms.') - abort(403) + abort(403, + message='You do not have permission to create repositories in namespace "%(namespace)s"', + issue='no-create-permission', + namespace=namespace) logger.debug('Creaing repository with owner: %s' % get_authenticated_user().username) @@ -200,7 +224,7 @@ def update_images(namespace, repository): repo = model.get_repository(namespace, repository) if not repo: # Make sure the repo actually exists. - abort(404) + abort(404, message='Unknown repository', issue='unknown-repo') image_with_checksums = json.loads(request.data) @@ -248,7 +272,7 @@ def get_repository_images(namespace, repository): # We can't rely on permissions to tell us if a repo exists anymore repo = model.get_repository(namespace, repository) if not repo: - abort(404) + abort(404, message='Unknown repository', issue='unknown-repo') all_images = [] for image in model.get_repository_images(namespace, repository): @@ -296,18 +320,18 @@ def get_repository_images(namespace, repository): @parse_repository_name @generate_headers(role='write') def delete_repository_images(namespace, repository): - return make_response('Not Implemented', 501) + abort(501, 'Not Implemented', issue='not-implemented') @index.route('/repositories//auth', methods=['PUT']) @parse_repository_name def put_repository_auth(namespace, repository): - return make_response('Not Implemented', 501) + abort(501, 'Not Implemented', issue='not-implemented') @index.route('/search', methods=['GET']) def get_search(): - return make_response('Not Implemented', 501) + abort(501, 'Not Implemented', issue='not-implemented') @index.route('/_ping') diff --git a/endpoints/registry.py b/endpoints/registry.py index 57895aebe..27c98d44e 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -1,8 +1,8 @@ import logging import json -from flask import (make_response, request, session, Response, abort, - redirect, Blueprint) +from flask import (make_response, request, session, Response, redirect, + Blueprint) from functools import wraps from datetime import datetime from time import time @@ -12,6 +12,7 @@ from data.queue import image_diff_queue from app import app from auth.auth import process_auth, extract_namespace_repo_from_session from util import checksums, changes +from util.http import abort from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission) from data import model @@ -22,6 +23,19 @@ store = app.config['STORAGE'] logger = logging.getLogger(__name__) +@registry.errorhandler(404) +def fallback_not_found(e): + return make_response('Not Found', 404) + +@registry.errorhandler(403) +def fallback_forbidden(e): + return make_response('Forbidden', 403) + +@registry.errorhandler(400) +def fallback_invalid_request(e): + return make_response('Invalid Request', 400) + + class SocketReader(object): def __init__(self, fp): self._fp = fp @@ -45,8 +59,9 @@ def require_completion(f): def wrapper(namespace, repository, *args, **kwargs): if store.exists(store.image_mark_path(namespace, repository, kwargs['image_id'])): - logger.warning('Image is already being uploaded: %s', kwargs['image_id']) - abort(400) # 'Image is being uploaded, retry later') + abort(400, 'Image %(image_id)s is being uploaded, retry later', + issue='upload-in-progress', image_id=kwargs['image_id']) + return f(namespace, repository, *args, **kwargs) return wrapper @@ -90,9 +105,8 @@ def get_image_layer(namespace, repository, image_id, headers): try: return Response(store.stream_read(path), headers=headers) except IOError: - logger.warning('Image not found: %s', image_id) - abort(404) # 'Image not found', 404) - + abort(404, 'Image %(image_id)s not found', issue='unknown-image', image_id=image_id) + abort(403) @@ -108,16 +122,20 @@ def put_image_layer(namespace, repository, image_id): json_data = store.get_content(store.image_json_path(namespace, repository, image_id)) except IOError: - abort(404) # 'Image not found', 404) + abort(404, 'Image %(image_id)s not found', issue='unknown-image', image_id=image_id) + layer_path = store.image_layer_path(namespace, repository, image_id) mark_path = store.image_mark_path(namespace, repository, image_id) + if store.exists(layer_path) and not store.exists(mark_path): - abort(409) # 'Image already exists', 409) + abort(409, 'Image already exists', issue='image-exists', image_id=image_id) + input_stream = request.stream if request.headers.get('transfer-encoding') == 'chunked': # Careful, might work only with WSGI servers supporting chunked # encoding (Gunicorn) input_stream = request.environ['wsgi.input'] + # compute checksums csums = [] sr = SocketReader(input_stream) @@ -127,6 +145,7 @@ def put_image_layer(namespace, repository, image_id): sr.add_handler(sum_hndlr) store.stream_write(layer_path, sr) csums.append('sha256:{0}'.format(h.hexdigest())) + try: image_size = tmp.tell() @@ -139,6 +158,7 @@ def put_image_layer(namespace, repository, image_id): except (IOError, checksums.TarError) as e: logger.debug('put_image_layer: Error when computing tarsum ' '{0}'.format(e)) + try: checksum = store.get_content(store.image_checksum_path(namespace, repository, @@ -148,10 +168,13 @@ def put_image_layer(namespace, repository, image_id): # Not removing the mark though, image is not downloadable yet. session['checksum'] = csums return make_response('true', 200) + # We check if the checksums provided matches one the one we computed if checksum not in csums: logger.warning('put_image_layer: Wrong checksum') - abort(400) # 'Checksum mismatch, ignoring the layer') + abort(400, 'Checksum mismatch; ignoring the layer for image %(image_id)s', + issue='checksum-mismatch', image_id=image_id) + # Checksum is ok, we remove the marker store.remove(mark_path) @@ -177,24 +200,31 @@ def put_image_checksum(namespace, repository, image_id): checksum = request.headers.get('X-Docker-Checksum') if not checksum: - logger.warning('Missing Image\'s checksum: %s', image_id) - abort(400) # 'Missing Image\'s checksum') + abort(400, "Missing checksum for image %(image_id)s", issue='missing-checksum', image_id=image_id) + if not session.get('checksum'): - logger.warning('Checksum not found in Cookie for image: %s', image_id) - abort(400) # 'Checksum not found in Cookie') + abort(400, 'Checksum not found in Cookie for image %(imaage_id)s', + issue='missing-checksum-cookie', image_id=image_id) + if not store.exists(store.image_json_path(namespace, repository, image_id)): - abort(404) # 'Image not found', 404) + abort(404, 'Image not found: %(image_id)s', issue='unknown-image', image_id=image_id) + mark_path = store.image_mark_path(namespace, repository, image_id) if not store.exists(mark_path): - abort(409) # 'Cannot set this image checksum', 409) + abort(409, 'Cannot set checksum for image %(image_id)s', + issue='image-write-error', image_id=image_id) + err = store_checksum(namespace, repository, image_id, checksum) if err: - abort(err) + abort(400, err) + if checksum not in session.get('checksum', []): logger.debug('session checksums: %s' % session.get('checksum', [])) logger.debug('client supplied checksum: %s' % checksum) logger.debug('put_image_layer: Wrong checksum') - abort(400) # 'Checksum mismatch') + abort(400, 'Checksum mismatch for image: %(image_id)s', + issue='checksum-mismatch', image_id=image_id) + # Checksum is ok, we remove the marker store.remove(mark_path) @@ -225,16 +255,19 @@ def get_image_json(namespace, repository, image_id, headers): data = store.get_content(store.image_json_path(namespace, repository, image_id)) except IOError: - abort(404) # 'Image not found', 404) + abort(404, 'Image %(image_id)%s not found', issue='unknown-image', image_id=image_id) + try: size = store.get_size(store.image_layer_path(namespace, repository, image_id)) headers['X-Docker-Size'] = str(size) except OSError: pass + checksum_path = store.image_checksum_path(namespace, repository, image_id) if store.exists(checksum_path): headers['X-Docker-Checksum'] = store.get_content(checksum_path) + response = make_response(data, 200) response.headers.extend(headers) return response @@ -255,7 +288,8 @@ def get_image_ancestry(namespace, repository, image_id, headers): data = store.get_content(store.image_ancestry_path(namespace, repository, image_id)) except IOError: - abort(404) # 'Image not found', 404) + abort(404, 'Image %(image_id)s not found', issue='unknown-image', image_id=image_id) + response = make_response(json.dumps(json.loads(data)), 200) response.headers.extend(headers) return response @@ -280,6 +314,7 @@ def store_checksum(namespace, repository, image_id, checksum): checksum_parts = checksum.split(':') if len(checksum_parts) != 2: return 'Invalid checksum format' + # We store the checksum checksum_path = store.image_checksum_path(namespace, repository, image_id) store.put_content(checksum_path, checksum) @@ -298,36 +333,39 @@ def put_image_json(namespace, repository, image_id): except json.JSONDecodeError: pass if not data or not isinstance(data, dict): - logger.warning('Invalid JSON for image: %s json: %s', image_id, - request.data) - abort(400) # 'Invalid JSON') + abort(400, 'Invalid JSON for image: %(image_id)s\nJSON: %(json)s', + issue='invalid-request', image_id=image_id, json=request.data) + if 'id' not in data: - logger.warning('Missing key `id\' in JSON for image: %s', image_id) - abort(400) # 'Missing key `id\' in JSON') + abort(400, 'Missing key `id` in JSON for image: %(image_id)s', + issue='invalid-request', image_id=image_id) + # Read the checksum checksum = request.headers.get('X-Docker-Checksum') if checksum: # Storing the checksum is optional at this stage err = store_checksum(namespace, repository, image_id, checksum) if err: - abort(err) + abort(400, err, issue='write-error') + else: # We cleanup any old checksum in case it's a retry after a fail store.remove(store.image_checksum_path(namespace, repository, image_id)) if image_id != data['id']: - logger.warning('JSON data contains invalid id for image: %s', image_id) - abort(400) # 'JSON data contains invalid id') + abort(400, 'JSON data contains invalid id for image: %(image_id)s', + issue='invalid-request', image_id=image_id) + parent_id = data.get('parent') - if parent_id and not store.exists(store.image_json_path(namespace, - repository, - data['parent'])): - logger.warning('Image depends on a non existing parent image: %s', - image_id) - abort(400) # 'Image depends on a non existing parent') + if (parent_id and not + store.exists(store.image_json_path(namespace, repository, parent_id))): + abort(400, 'Image %(image_id)s depends on non existing parent image %(parent_id)s', + issue='invalid-request', image_id=image_id, parent_id=parent_id) + json_path = store.image_json_path(namespace, repository, image_id) mark_path = store.image_mark_path(namespace, repository, image_id) if store.exists(json_path) and not store.exists(mark_path): - abort(409) # 'Image already exists', 409) + abort(409, 'Image already exists', issue='image-exists', image_id=image_id) + # If we reach that point, it means that this is a new image or a retry # on a failed push # save the metadata diff --git a/static/js/controllers.js b/static/js/controllers.js index 8f10ca5ef..fc79963d1 100644 --- a/static/js/controllers.js +++ b/static/js/controllers.js @@ -1242,7 +1242,7 @@ function NewRepoCtrl($scope, $location, $http, $timeout, UserService, ApiService $location.path('/repository/' + created.namespace + '/' + created.name); }, function(result) { $scope.creating = false; - $scope.createError = result.data; + $scope.createError = result.data ? result.data.message : 'Cannot create repository'; $timeout(function() { $('#repoName').popover('show'); }); diff --git a/util/http.py b/util/http.py new file mode 100644 index 000000000..6592768cc --- /dev/null +++ b/util/http.py @@ -0,0 +1,59 @@ +import logging + +from app import mixpanel +from flask import request, abort as flask_abort, jsonify +from auth.auth import get_authenticated_user, get_validated_token + +logger = logging.getLogger(__name__) + + +DEFAULT_MESSAGE = {} +DEFAULT_MESSAGE[400] = 'Invalid Request' +DEFAULT_MESSAGE[401] = 'Unauthorized' +DEFAULT_MESSAGE[403] = 'Permission Denied' +DEFAULT_MESSAGE[404] = 'Not Found' +DEFAULT_MESSAGE[409] = 'Conflict' +DEFAULT_MESSAGE[501] = 'Not Implemented' + +def abort(status_code, message=None, issue=None, **kwargs): + message = (str(message) % kwargs if message else + DEFAULT_MESSAGE.get(status_code, '')) + + params = dict(request.view_args) + params.update(kwargs) + + params['url'] = request.url + params['status_code'] = status_code + params['message'] = message + + # Add the user information. + auth_user = get_authenticated_user() + auth_token = get_validated_token() + if auth_user: + mixpanel.track(auth_user.username, 'http_error', params) + message = '%s (user: %s)' % (message, auth_user.username) + elif auth_token: + mixpanel.track(auth_token.code, 'http_error', params) + message = '%s (token: %s)' % (message, + auth_token.friendly_name or auth_token.code) + + # Log the abort. + logger.error('Error %s: %s; Arguments: %s' % (status_code, message, params)) + + # Calculate the issue URL (if the issue ID was supplied). + issue_url = None + if issue: + issue_url = 'http://docs.quay.io/issues/%s.html' % (issue) + + # Create the final response data and message. + data = {} + data['error'] = message + + if issue_url: + data['info_url'] = issue_url + + resp = jsonify(data) + resp.status_code = status_code + + # Report the abort to the user. + flask_abort(resp)