diff --git a/lib/galaxy/exceptions/__init__.py b/lib/galaxy/exceptions/__init__.py index c3bd4bde319..75c9aba18bd 100644 --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -6,33 +6,66 @@ from galaxy import eggs eggs.require( "Paste" ) from paste import httpexceptions +from ..exceptions import error_codes + class MessageException( Exception ): """ - Exception to make throwing errors from deep in controllers easier + Exception to make throwing errors from deep in controllers easier. """ - def __init__( self, err_msg, type="info" ): - self.err_msg = err_msg + # status code to be set when used with API. + status_code = 400 + # Error code information embedded into API json responses. + err_code = error_codes.UNKNOWN + + def __init__( self, err_msg=None, type="info", **extra_error_info ): + self.err_msg = err_msg or self.err_code.default_error_message self.type = type + self.extra_error_info = extra_error_info + def __str__( self ): return self.err_msg + class ItemDeletionException( MessageException ): pass + class ItemAccessibilityException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_CANNOT_ACCESS_ITEM + class ItemOwnershipException( MessageException ): - pass + status_code = 403 + err_code = error_codes.USER_DOES_NOT_OWN_ITEM + + +class DuplicatedSlugException( MessageException ): + status_code = 400 + err_code = error_codes.USER_SLUG_DUPLICATE + + +class ObjectAttributeInvalidException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_INVALID + + +class ObjectAttributeMissingException( MessageException ): + status_code = 400 + err_code = error_codes.USER_OBJECT_ATTRIBUTE_MISSING + class ActionInputError( MessageException ): def __init__( self, err_msg, type="error" ): super( ActionInputError, self ).__init__( err_msg, type ) -class ObjectNotFound( Exception ): + +class ObjectNotFound( MessageException ): """ Accessed object was not found """ - pass + status_code = 404 + err_code = error_codes.USER_OBJECT_NOT_FOUND + class ObjectInvalid( Exception ): """ Accessed object store ID is invalid """ diff --git a/lib/galaxy/exceptions/error_codes.json b/lib/galaxy/exceptions/error_codes.json new file mode 100644 index 00000000000..f896d87d261 --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.json @@ -0,0 +1,57 @@ +[ + { + "name": "UNKNOWN", + "code": 0, + "message": "Unknown error occurred while processing request." + }, + { + "name": "USER_CANNOT_RUN_AS", + "code": 400001, + "message": "User does not have permissions to run jobs as another user." + }, + { + "name": "USER_INVALID_RUN_AS", + "code": 400002, + "message": "Invalid run_as request - run_as user does not exist." + }, + { + "name": "USER_INVALID_JSON", + "code": 400003, + "message": "Your request did not appear to be valid JSON, please consult the API documentation." + }, + { + "name": "USER_OBJECT_ATTRIBUTE_INVALID", + "code": 400004, + "message": "Attempted to create or update object with invalid attribute value." + }, + { + "name": "USER_OBJECT_ATTRIBUTE_MISSING", + "code": 400005, + "message": "Attempted to create object without required attribute." + }, + { + "name": "USER_SLUG_DUPLICATE", + "code": 400006, + "message": "Slug must be unique per user." + }, + { + "name": "USER_NO_API_KEY", + "code": 403001, + "message": "API Authentication Required for this request" + }, + { + "name": "USER_CANNOT_ACCESS_ITEM", + "code": 403002, + "message": "User cannot access specified item." + }, + { + "name": "USER_DOES_NOT_OWN_ITEM", + "code": 403003, + "message": "User does not own specified item." + }, + { + "name": "USER_OBJECT_NOT_FOUND", + "code": 404001, + "message": "No such object not found." + } +] diff --git a/lib/galaxy/exceptions/error_codes.py b/lib/galaxy/exceptions/error_codes.py new file mode 100644 index 00000000000..4afe67bea80 --- /dev/null +++ b/lib/galaxy/exceptions/error_codes.py @@ -0,0 +1,32 @@ +from pkg_resources import resource_string +from json import loads + +# Error codes are provided as a convience to Galaxy API clients, but at this +# time they do represent part of the more stable interface. They can change +# without warning between releases. +UNKNOWN_ERROR_MESSAGE = "Unknown error occurred while processing request." + + +class ErrorCode( object ): + + def __init__( self, code, default_error_message ): + self.code = code + self.default_error_message = default_error_message or UNKNOWN_ERROR_MESSAGE + + def __str__( self ): + return str( self.default_error_message ) + + def __int__( self ): + return int( self.code ) + + @staticmethod + def from_dict( entry ): + name = entry.get("name") + code = entry.get("code") + message = entry.get("message") + return ( name, ErrorCode( code, message ) ) + +error_codes_json = resource_string( __name__, 'error_codes.json' ) +for entry in loads( error_codes_json ): + name, error_code_obj = ErrorCode.from_dict( entry ) + globals()[ name ] = error_code_obj diff --git a/lib/galaxy/web/__init__.py b/lib/galaxy/web/__init__.py index 107c5002d23..70b016c53e4 100644 --- a/lib/galaxy/web/__init__.py +++ b/lib/galaxy/web/__init__.py @@ -15,3 +15,7 @@ from framework import expose_api_anonymous from framework import expose_api_raw from framework import expose_api_raw_anonymous from framework.base import httpexceptions + +# TODO: Drop and make these the default. +from framework import _future_expose_api +from framework import _future_expose_api_anonymous diff --git a/lib/galaxy/web/framework/__init__.py b/lib/galaxy/web/framework/__init__.py index 4432d743924..eecdfb9f83c 100644 --- a/lib/galaxy/web/framework/__init__.py +++ b/lib/galaxy/web/framework/__init__.py @@ -10,9 +10,9 @@ import random import socket import string import time - -from functools import wraps +from traceback import format_exc from Cookie import CookieError +from functools import wraps pkg_resources.require( "Cheetah" ) from Cheetah.Template import Template @@ -23,6 +23,7 @@ import helpers from galaxy import util from galaxy.exceptions import MessageException +from galaxy.exceptions import error_codes from galaxy.util import asbool from galaxy.util import safe_str_cmp from galaxy.util.backports.importlib import import_module @@ -103,6 +104,35 @@ def require_login( verb="perform this action", use_panels=False, webapp='galaxy' return decorator return argcatcher + +def __extract_payload_from_request(trans, func, kwargs): + content_type = trans.request.headers['content-type'] + if content_type.startswith('application/x-www-form-urlencoded') or content_type.startswith('multipart/form-data'): + # If the content type is a standard type such as multipart/form-data, the wsgi framework parses the request body + # and loads all field values into kwargs. However, kwargs also contains formal method parameters etc. which + # are not a part of the request body. This is a problem because it's not possible to differentiate between values + # which are a part of the request body, and therefore should be a part of the payload, and values which should not be + # in the payload. Therefore, the decorated method's formal arguments are discovered through reflection and removed from + # the payload dictionary. This helps to prevent duplicate argument conflicts in downstream methods. + payload = kwargs.copy() + named_args, _, _, _ = inspect.getargspec(func) + for arg in named_args: + payload.pop(arg, None) + for k, v in payload.iteritems(): + if isinstance(v, (str, unicode)): + try: + payload[k] = from_json_string(v) + except: + # may not actually be json, just continue + pass + payload = util.recursively_stringify_dictionary_keys( payload ) + else: + # Assume application/json content type and parse request body manually, since wsgi won't do it. However, the order of this check + # should ideally be in reverse, with the if clause being a check for application/json and the else clause assuming a standard encoding + # such as multipart/form-data. Leaving it as is for backward compatibility, just in case. + payload = util.recursively_stringify_dictionary_keys( from_json_string( trans.request.body ) ) + return payload + def expose_api_raw( func ): """ Expose this function via the API but don't dump the results @@ -140,35 +170,8 @@ def expose_api( func, to_json=True, user_required=True ): error_message = "API Authentication Required for this request" return error if trans.request.body: - def extract_payload_from_request(trans, func, kwargs): - content_type = trans.request.headers['content-type'] - if content_type.startswith('application/x-www-form-urlencoded') or content_type.startswith('multipart/form-data'): - # If the content type is a standard type such as multipart/form-data, the wsgi framework parses the request body - # and loads all field values into kwargs. However, kwargs also contains formal method parameters etc. which - # are not a part of the request body. This is a problem because it's not possible to differentiate between values - # which are a part of the request body, and therefore should be a part of the payload, and values which should not be - # in the payload. Therefore, the decorated method's formal arguments are discovered through reflection and removed from - # the payload dictionary. This helps to prevent duplicate argument conflicts in downstream methods. - payload = kwargs.copy() - named_args, _, _, _ = inspect.getargspec(func) - for arg in named_args: - payload.pop(arg, None) - for k, v in payload.iteritems(): - if isinstance(v, (str, unicode)): - try: - payload[k] = from_json_string(v) - except: - # may not actually be json, just continue - pass - payload = util.recursively_stringify_dictionary_keys( payload ) - else: - # Assume application/json content type and parse request body manually, since wsgi won't do it. However, the order of this check - # should ideally be in reverse, with the if clause being a check for application/json and the else clause assuming a standard encoding - # such as multipart/form-data. Leaving it as is for backward compatibility, just in case. - payload = util.recursively_stringify_dictionary_keys( from_json_string( trans.request.body ) ) - return payload try: - kwargs['payload'] = extract_payload_from_request(trans, func, kwargs) + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) except ValueError: error_status = '400 Bad Request' error_message = 'Your request did not appear to be valid JSON, please consult the API documentation' @@ -210,6 +213,143 @@ def expose_api( func, to_json=True, user_required=True ): decorator.exposed = True return decorator +API_RESPONSE_CONTENT_TYPE = "application/json" + + +def __api_error_message( trans, **kwds ): + exception = kwds.get( "exception", None ) + if exception: + # If we are passed a MessageException use err_msg. + default_error_code = getattr( exception, "err_code", error_codes.UNKNOWN ) + default_error_message = getattr( exception, "err_msg", default_error_code.default_error_message ) + extra_error_info = getattr( exception, 'extra_error_info', {} ) + if not isinstance( extra_error_info, dict ): + extra_error_info = {} + else: + default_error_message = "Error processing API request." + default_error_code = error_codes.UNKNOWN + extra_error_info = {} + traceback_string = kwds.get( "traceback", "No traceback available." ) + err_msg = kwds.get( "err_msg", default_error_message ) + error_code_object = kwds.get( "err_code", default_error_code ) + try: + error_code = error_code_object.code + except AttributeError: + # Some sort of bad error code sent in, logic failure on part of + # Galaxy developer. + error_code = error_codes.UNKNOWN.code + # Would prefer the terminology of error_code and error_message, but + # err_msg used a good number of places already. Might as well not change + # it? + error_response = dict( err_msg=err_msg, err_code=error_code, **extra_error_info ) + if trans.debug: # TODO: Should admins get to see traceback as well? + error_response[ "traceback" ] = traceback_string + return error_response + + +def __api_error_response( trans, **kwds ): + error_dict = __api_error_message( trans, **kwds ) + exception = kwds.get( "exception", None ) + # If we are given an status code directly - use it - otherwise check + # the exception for a status_code attribute. + if "status_code" in kwds: + status_code = int( kwds.get( "status_code" ) ) + elif hasattr( exception, "status_code" ): + status_code = int( exception.status_code ) + else: + status_code = 500 + response = trans.response + if not response.status or str(response.status).startswith("20"): + # Unset status code appears to be string '200 OK', if anything + # non-success (i.e. not 200 or 201) has been set, do not override + # underlying controller. + response.status = status_code + return to_json_string( error_dict ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api_anonymous( func, to_json=True ): + """ + Expose this function via the API but don't require a set user. + """ + return _future_expose_api( func, to_json=to_json, user_required=False ) + + +# TODO: rename as expose_api and make default. +def _future_expose_api( func, to_json=True, user_required=True ): + """ + Expose this function via the API. + """ + @wraps(func) + def decorator( self, trans, *args, **kwargs ): + if trans.error_message: + # TODO: Document this branch, when can this happen, + # I don't understand it. + return __api_error_response( trans, err_msg=trans.error_message ) + if user_required and trans.anonymous: + error_code = error_codes.USER_NO_API_KEY + # Use error codes default error message. + return __api_error_response( trans, err_code=error_code, status_code=403 ) + if trans.request.body: + try: + kwargs['payload'] = __extract_payload_from_request(trans, func, kwargs) + except ValueError: + error_code = error_codes.USER_INVALID_JSON + return __api_error_response( trans, status_code=400, err_code=error_code ) + + trans.response.set_content_type( API_RESPONSE_CONTENT_TYPE ) + # send 'do not cache' headers to handle IE's caching of ajax get responses + trans.response.headers[ 'Cache-Control' ] = "max-age=0,no-cache,no-store" + # TODO: Refactor next block out into a helper procedure. + # Perform api_run_as processing, possibly changing identity + if 'payload' in kwargs and 'run_as' in kwargs['payload']: + if not trans.user_can_do_run_as(): + error_code = error_codes.USER_CANNOT_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=403 ) + try: + decoded_user_id = trans.security.decode_id( kwargs['payload']['run_as'] ) + except TypeError: + error_message = "Malformed user id ( %s ) specified, unable to decode." % str( kwargs['payload']['run_as'] ) + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, err_msg=error_message, status_code=400) + try: + user = trans.sa_session.query( trans.app.model.User ).get( decoded_user_id ) + trans.api_inherit_admin = trans.user_is_admin() + trans.set_user(user) + except: + error_code = error_codes.USER_INVALID_RUN_AS + return __api_error_response( trans, err_code=error_code, status_code=400 ) + try: + rval = func( self, trans, *args, **kwargs) + if to_json and trans.debug: + rval = to_json_string( rval, indent=4, sort_keys=True ) + elif to_json: + rval = to_json_string( rval ) + return rval + except MessageException as e: + traceback_string = format_exc() + return __api_error_response( trans, exception=e, traceback=traceback_string ) + except paste.httpexceptions.HTTPException: + # TODO: Allow to pass or format for the API??? + raise # handled + except Exception as e: + traceback_string = format_exc() + error_message = 'Uncaught exception in exposed API method:' + log.exception( error_message ) + return __api_error_response( + trans, + status_code=500, + exception=e, + traceback=traceback_string, + err_msg=error_message, + err_code=error_codes.UNKNOWN + ) + if not hasattr(func, '_orig'): + decorator._orig = func + decorator.exposed = True + return decorator + + def require_admin( func ): @wraps(func) def decorator( self, trans, *args, **kwargs ): diff --git a/lib/galaxy/webapps/galaxy/api/histories.py b/lib/galaxy/webapps/galaxy/api/histories.py index 5935016ee7d..6045b5f2152 100644 --- a/lib/galaxy/webapps/galaxy/api/histories.py +++ b/lib/galaxy/webapps/galaxy/api/histories.py @@ -9,6 +9,8 @@ pkg_resources.require( "Paste" ) from paste.httpexceptions import HTTPBadRequest, HTTPForbidden, HTTPInternalServerError, HTTPException from galaxy import web +from galaxy.web import _future_expose_api as expose_api +from galaxy.web import _future_expose_api_anonymous as expose_api_anonymous from galaxy.util import string_as_bool, restore_text from galaxy.util.sanitize_html import sanitize_html from galaxy.web.base.controller import BaseAPIController, UsesHistoryMixin, UsesTagsMixin @@ -20,7 +22,7 @@ log = logging.getLogger( __name__ ) class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): - @web.expose_api_anonymous + @expose_api_anonymous def index( self, trans, deleted='False', **kwd ): """ index( trans, deleted='False' ) @@ -152,7 +154,7 @@ class HistoriesController( BaseAPIController, UsesHistoryMixin, UsesTagsMixin ): return history_data - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( trans, payload ) diff --git a/lib/galaxy/webapps/galaxy/api/page_revisions.py b/lib/galaxy/webapps/galaxy/api/page_revisions.py index 25cbbc3f1ad..56e3bded8dd 100644 --- a/lib/galaxy/webapps/galaxy/api/page_revisions.py +++ b/lib/galaxy/webapps/galaxy/api/page_revisions.py @@ -2,8 +2,9 @@ API for updating Galaxy Pages """ import logging -from galaxy import web +from galaxy.web import _future_expose_api as expose_api from galaxy.web.base.controller import SharableItemSecurityMixin, BaseAPIController, SharableMixin +from galaxy import exceptions from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.sanitize_html import sanitize_html @@ -12,7 +13,7 @@ log = logging.getLogger( __name__ ) class PageRevisionsController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotations, SharableMixin ): - @web.expose_api + @expose_api def index( self, trans, page_id, **kwd ): """ index( self, trans, page_id, **kwd ) @@ -24,14 +25,16 @@ class PageRevisionsController( BaseAPIController, SharableItemSecurityMixin, Use :rtype: list :returns: dictionaries containing different revisions of the page """ + page = self._get_page( trans, page_id ) + self._verify_page_ownership( trans, page ) + r = trans.sa_session.query( trans.app.model.PageRevision ).filter_by( page_id=trans.security.decode_id(page_id) ) out = [] for page in r: - if self.security_check( trans, page, True, True ): - out.append( self.encode_all_ids( trans, page.to_dict(), True) ) + out.append( self.encode_all_ids( trans, page.to_dict(), True) ) return out - @web.expose_api + @expose_api def create( self, trans, page_id, payload, **kwd ): """ create( self, trans, page_id, payload **kwd ) @@ -46,39 +49,42 @@ class PageRevisionsController( BaseAPIController, SharableItemSecurityMixin, Use :rtype: dictionary :returns: Dictionary with 'success' or 'error' element to indicate the result of the request """ - error_str = "" + content = payload.get("content", None) + if not content: + raise exceptions.ObjectAttributeMissingException("content undefined or empty") - if not page_id: - error_str = "page_id is required" - elif not payload.get("content", None): - error_str = "content is required" + page = self._get_page( trans, page_id ) + self._verify_page_ownership( trans, page ) + + if 'title' in payload: + title = payload['title'] else: + title = page.title - # Create the new stored page + content = sanitize_html( content, 'utf-8', 'text/html' ) + + page_revision = trans.app.model.PageRevision() + page_revision.title = title + page_revision.page = page + page.latest_revision = page_revision + page_revision.content = content + + # Persist + session = trans.sa_session + session.flush() + + return page_revision.to_dict( view="element" ) + + def _get_page( self, trans, page_id ): + page = None + try: page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id(page_id) ) - if page is None: - return { "error" : "page not found"} + except Exception: + pass + if not page: + raise exceptions.ObjectNotFound() + return page - if not self.security_check( trans, page, True, True ): - return { "error" : "page not found"} - - if 'title' in payload: - title = payload['title'] - else: - title = page.title - - content = payload.get("content", "") - content = sanitize_html( content, 'utf-8', 'text/html' ) - - page_revision = trans.app.model.PageRevision() - page_revision.title = title - page_revision.page = page - page.latest_revision = page_revision - page_revision.content = content - # Persist - session = trans.sa_session - session.flush() - - return { "success" : "revision posted" } - - return { "error" : error_str } + def _verify_page_ownership( self, trans, page ): + if not self.security_check( trans, page, True, True ): + raise exceptions.ItemOwnershipException() diff --git a/lib/galaxy/webapps/galaxy/api/pages.py b/lib/galaxy/webapps/galaxy/api/pages.py index 92e01fcbebb..aa27c6afc09 100644 --- a/lib/galaxy/webapps/galaxy/api/pages.py +++ b/lib/galaxy/webapps/galaxy/api/pages.py @@ -2,8 +2,9 @@ API for updating Galaxy Pages """ import logging -from galaxy import web +from galaxy.web import _future_expose_api as expose_api from galaxy.web.base.controller import SharableItemSecurityMixin, BaseAPIController, SharableMixin +from galaxy import exceptions from galaxy.model.item_attrs import UsesAnnotations from galaxy.util.sanitize_html import sanitize_html @@ -12,7 +13,7 @@ log = logging.getLogger( __name__ ) class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotations, SharableMixin ): - @web.expose_api + @expose_api def index( self, trans, deleted=False, **kwd ): """ index( self, trans, deleted=False, **kwd ) @@ -47,7 +48,7 @@ class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotat return out - @web.expose_api + @expose_api def create( self, trans, payload, **kwd ): """ create( self, trans, payload, **kwd ) @@ -64,45 +65,41 @@ class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotat :returns: Dictionary return of the Page.to_dict call """ user = trans.get_user() - error_str = "" if not payload.get("title", None): - error_str = "Page name is required" + raise exceptions.ObjectAttributeMissingException( "Page name is required" ) elif not payload.get("slug", None): - error_str = "Page id is required" + raise exceptions.ObjectAttributeMissingException( "Page id is required" ) elif not self._is_valid_slug( payload["slug"] ): - error_str = "Page identifier must consist of only lowercase letters, numbers, and the '-' character" + raise exceptions.ObjectAttributeInvalidException( "Page identifier must consist of only lowercase letters, numbers, and the '-' character" ) elif trans.sa_session.query( trans.app.model.Page ).filter_by( user=user, slug=payload["slug"], deleted=False ).first(): - error_str = "Page id must be unique" - else: + raise exceptions.DuplicatedSlugException( "Page slug must be unique" ) - content = payload.get("content", "") - content = sanitize_html( content, 'utf-8', 'text/html' ) + content = payload.get("content", "") + content = sanitize_html( content, 'utf-8', 'text/html' ) - # Create the new stored page - page = trans.app.model.Page() - page.title = payload['title'] - page.slug = payload['slug'] - page_annotation = sanitize_html( payload.get( "annotation", "" ), 'utf-8', 'text/html' ) - self.add_item_annotation( trans.sa_session, trans.get_user(), page, page_annotation ) - page.user = user - # And the first (empty) page revision - page_revision = trans.app.model.PageRevision() - page_revision.title = payload['title'] - page_revision.page = page - page.latest_revision = page_revision - page_revision.content = content - # Persist - session = trans.sa_session - session.add( page ) - session.flush() + # Create the new stored page + page = trans.app.model.Page() + page.title = payload['title'] + page.slug = payload['slug'] + page_annotation = sanitize_html( payload.get( "annotation", "" ), 'utf-8', 'text/html' ) + self.add_item_annotation( trans.sa_session, trans.get_user(), page, page_annotation ) + page.user = user + # And the first (empty) page revision + page_revision = trans.app.model.PageRevision() + page_revision.title = payload['title'] + page_revision.page = page + page.latest_revision = page_revision + page_revision.content = content + # Persist + session = trans.sa_session + session.add( page ) + session.flush() - rval = self.encode_all_ids( trans, page.to_dict(), True ) - return rval + rval = self.encode_all_ids( trans, page.to_dict(), True ) + return rval - return { "error" : error_str } - - @web.expose_api + @expose_api def delete( self, trans, id, **kwd ): """ delete( self, trans, id, **kwd ) @@ -114,22 +111,14 @@ class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotat :rtype: dict :returns: Dictionary with 'success' or 'error' element to indicate the result of the request """ - page_id = id - try: - page = trans.sa_session.query(self.app.model.Page).get(trans.security.decode_id(page_id)) - except Exception, e: - return { "error" : "Page with ID='%s' can not be found\n Exception: %s" % (page_id, str( e )) } + page = self._get_page( trans, id ) - # check to see if user has permissions to selected workflow - if page.user != trans.user and not trans.user_is_admin(): - return { "error" : "Workflow is not owned by or shared with current user" } - - #Mark a workflow as deleted + #Mark a page as deleted page.deleted = True trans.sa_session.flush() - return { "success" : "Deleted", "id" : page_id } + return '' # TODO: Figure out what to return on DELETE, document in guidelines! - @web.expose_api + @expose_api def show( self, trans, id, **kwd ): """ show( self, trans, id, **kwd ) @@ -141,8 +130,22 @@ class PagesController( BaseAPIController, SharableItemSecurityMixin, UsesAnnotat :rtype: dict :returns: Dictionary return of the Page.to_dict call with the 'content' field populated by the most recent revision """ - page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id( id ) ) + page = self._get_page( trans, id ) self.security_check( trans, page, check_ownership=False, check_accessible=True) rval = self.encode_all_ids( trans, page.to_dict(), True ) rval['content'] = page.latest_revision.content return rval + + def _get_page( self, trans, id ): # Fetches page object and verifies security. + try: + page = trans.sa_session.query( trans.app.model.Page ).get( trans.security.decode_id( id ) ) + except Exception: + page = None + + if not page: + raise exceptions.ObjectNotFound() + + if page.user != trans.user and not trans.user_is_admin(): + raise exceptions.ItemOwnershipException() + + return page diff --git a/test/base/api.py b/test/base/api.py index d1d9a1f7583..aea87bfa811 100644 --- a/test/base/api.py +++ b/test/base/api.py @@ -60,6 +60,12 @@ class ApiTestCase( TwillTestCase ): for key in keys: assert key in response, "Response [%s] does not contain key [%s]" % ( response, key ) + def _assert_error_code_is( self, response, error_code ): + if hasattr( response, "json" ): + response = response.json() + self._assert_has_keys( response, "err_code" ) + self.assertEquals( response[ "err_code" ], int( error_code ) ) + def _random_key( self ): # Used for invalid request testing... return "1234567890123456" diff --git a/test/functional/api/test_page_revisions.py b/test/functional/api/test_page_revisions.py new file mode 100644 index 00000000000..2fa91b0992a --- /dev/null +++ b/test/functional/api/test_page_revisions.py @@ -0,0 +1,35 @@ +from galaxy.exceptions import error_codes +from functional.api.pages import BasePageApiTestCase + + +class PageRevisionsApiTestCase( BasePageApiTestCase ): + + def test_create( self ): + page_json = self._create_valid_page_with_slug( "pr1" ) + revision_data = dict( content="

NewContent!

" ) + page_revision_response = self._post( "pages/%s/revisions" % page_json[ 'id' ], data=revision_data ) + self._assert_status_code_is( page_revision_response, 200 ) + page_revision_json = page_revision_response.json() + self._assert_has_keys( page_revision_json, 'id', 'content' ) + + def test_403_if_create_revision_on_unowned_page( self ): + page_json = self._create_valid_page_as( "pr2@bx.psu.edu", "pr2" ) + revision_data = dict( content="

NewContent!

" ) + page_revision_response = self._post( "pages/%s/revisions" % page_json[ 'id' ], data=revision_data ) + self._assert_status_code_is( page_revision_response, 403 ) + + def test_revision_index( self ): + page_json = self._create_valid_page_with_slug( "pr3" ) + revision_data = dict( content="

NewContent!

" ) + revisions_url = "pages/%s/revisions" % page_json[ 'id' ] + self._post( revisions_url, data=revision_data ) + revisions_response = self._get( revisions_url ) + self._assert_status_code_is( revisions_response, 200 ) + revisions_json = revisions_response.json() + assert len( revisions_json ) == 2 # Original revision and new one + + def test_404_if_index_unknown_page( self ): + revisions_url = "pages/%s/revisions" % self._random_key() + revisions_response = self._get( revisions_url ) + self._assert_status_code_is( revisions_response, 404 ) + self._assert_error_code_is( revisions_response, error_codes.USER_OBJECT_NOT_FOUND ) diff --git a/test/functional/api/test_pages.py b/test/functional/api/test_pages.py new file mode 100644 index 00000000000..8b46d8b071d --- /dev/null +++ b/test/functional/api/test_pages.py @@ -0,0 +1,105 @@ +from galaxy.exceptions import error_codes +from base import api +from base.interactor import delete_request + +from operator import itemgetter + + +class BasePageApiTestCase( api.ApiTestCase ): + + def _create_valid_page_with_slug( self, slug ): + page_request = self._test_page_payload( slug=slug ) + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 200 ) + return page_response.json() + + def _create_valid_page_as( self, other_email, slug ): + run_as_user = self._setup_user( other_email ) + page_request = self._test_page_payload( slug=slug ) + page_request[ "run_as" ] = run_as_user[ "id" ] + page_response = self._post( "pages", page_request, admin=True ) + self._assert_status_code_is( page_response, 200 ) + return page_response.json() + + def _test_page_payload( self, **kwds ): + request = dict( + slug="mypage", + title="MY PAGE", + content="

Page!

", + ) + request.update( **kwds ) + return request + + +class PageApiTestCase( BasePageApiTestCase ): + + def test_create( self ): + response_json = self._create_valid_page_with_slug( "mypage" ) + self._assert_has_keys( response_json, "slug", "title", "id" ) + + def test_index( self ): + create_response_json = self._create_valid_page_with_slug( "indexpage" ) + assert self._users_index_has_page_with_id( create_response_json[ "id" ] ) + + def test_index_doesnt_show_unavailable_pages( self ): + create_response_json = self._create_valid_page_as( "others_page_index@bx.psu.edu", "otherspageindex" ) + assert not self._users_index_has_page_with_id( create_response_json[ "id" ] ) + + def test_cannot_create_pages_with_same_slug( self ): + page_request = self._test_page_payload( slug="mypage1" ) + page_response_1 = self._post( "pages", page_request ) + self._assert_status_code_is( page_response_1, 200 ) + page_response_2 = self._post( "pages", page_request ) + self._assert_status_code_is( page_response_2, 400 ) + self._assert_error_code_is( page_response_2, error_codes.USER_SLUG_DUPLICATE ) + + def test_page_requires_name( self ): + page_request = self._test_page_payload() + del page_request[ 'title' ] + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 400 ) + self._assert_error_code_is( page_response, error_codes.USER_OBJECT_ATTRIBUTE_MISSING ) + + def test_page_requires_slug( self ): + page_request = self._test_page_payload() + del page_request[ 'slug' ] + page_response = self._post( "pages", page_request ) + self._assert_status_code_is( page_response, 400 ) + + def test_delete( self ): + response_json = self._create_valid_page_with_slug( "testdelete" ) + delete_response = delete_request( self._api_url( "pages/%s" % response_json[ 'id' ], use_key=True ) ) + self._assert_status_code_is( delete_response, 200 ) + + def test_404_on_delete_unknown_page( self ): + delete_response = delete_request( self._api_url( "pages/%s" % self._random_key(), use_key=True ) ) + self._assert_status_code_is( delete_response, 404 ) + self._assert_error_code_is( delete_response, error_codes.USER_OBJECT_NOT_FOUND ) + + def test_403_on_delete_unowned_page( self ): + page_response = self._create_valid_page_as( "others_page@bx.psu.edu", "otherspage" ) + delete_response = delete_request( self._api_url( "pages/%s" % page_response[ "id" ], use_key=True ) ) + self._assert_status_code_is( delete_response, 403 ) + self._assert_error_code_is( delete_response, error_codes.USER_DOES_NOT_OWN_ITEM ) + + def test_show( self ): + response_json = self._create_valid_page_with_slug( "pagetoshow" ) + show_response = self._get( "pages/%s" % response_json['id'] ) + self._assert_status_code_is( show_response, 200 ) + show_json = show_response.json() + self._assert_has_keys( show_json, "slug", "title", "id" ) + self.assertEquals( show_json["slug"], "pagetoshow" ) + self.assertEquals( show_json["title"], "MY PAGE" ) + self.assertEquals( show_json["content"], "

Page!

" ) + + def test_403_on_unowner_show( self ): + response_json = self._create_valid_page_as( "others_page_show@bx.psu.edu", "otherspageshow" ) + show_response = self._get( "pages/%s" % response_json['id'] ) + self._assert_status_code_is( show_response, 403 ) + self._assert_error_code_is( show_response, error_codes.USER_DOES_NOT_OWN_ITEM ) + + def _users_index_has_page_with_id( self, id ): + index_response = self._get( "pages" ) + self._assert_status_code_is( index_response, 200 ) + pages = index_response.json() + return id in map( itemgetter( "id" ), pages )