diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index fddd941df44..fcb1587a3e2 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -520,9 +520,15 @@ class Job( object, HasJobMetrics, Dictifiable ): dataset.blurb = 'deleted' dataset.peek = 'Job deleted' dataset.info = 'Job output deleted by user before job completed' - def to_dict( self, view='collection' ): + + def to_dict( self, view='collection', system_details=False ): rval = super( Job, self ).to_dict( view=view ) rval['tool_id'] = self.tool_id + if system_details: + # System level details that only admins should have. + rval['external_id'] = self.job_runner_external_id + rval['command_line'] = self.command_line + if view == 'element': param_dict = dict( [ ( p.name, p.value ) for p in self.parameters ] ) rval['params'] = param_dict diff --git a/lib/galaxy/web/security/__init__.py b/lib/galaxy/web/security/__init__.py index 95998a7d21a..00a9e370050 100644 --- a/lib/galaxy/web/security/__init__.py +++ b/lib/galaxy/web/security/__init__.py @@ -73,7 +73,7 @@ class SecurityHelper( object ): if not isinstance( rval, dict ): return rval for k, v in rval.items(): - if ( k == 'id' or k.endswith( '_id' ) ) and v is not None and k not in [ 'tool_id' ]: + if ( k == 'id' or k.endswith( '_id' ) ) and v is not None and k not in [ 'tool_id', 'external_id' ]: try: rval[ k ] = self.encode_id( v ) except Exception: diff --git a/lib/galaxy/webapps/galaxy/api/jobs.py b/lib/galaxy/webapps/galaxy/api/jobs.py index adc933aa619..691baa73568 100644 --- a/lib/galaxy/webapps/galaxy/api/jobs.py +++ b/lib/galaxy/webapps/galaxy/api/jobs.py @@ -55,12 +55,10 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses :returns: list of dictionaries containing summary job information """ state = kwd.get( 'state', None ) - if trans.user_is_admin() and kwd.get('user_details', False): - is_extended_service = True - else: - is_extended_service = False + is_admin = trans.user_is_admin() + user_details = kwd.get('user_details', False) - if is_extended_service: + if is_admin: query = trans.sa_session.query( trans.app.model.Job ) else: query = trans.sa_session.query( trans.app.model.Job ).filter(trans.app.model.Job.user == trans.user) @@ -98,9 +96,9 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses else: order_by = trans.app.model.Job.update_time.desc() for job in query.order_by( order_by ).all(): - j = self.encode_all_ids( trans, job.to_dict( 'collection' ), True ) - if is_extended_service: - j['external_id'] = job.job_runner_external_id + job_dict = job.to_dict( 'collection', system_details=is_admin ) + j = self.encode_all_ids( trans, job_dict, True ) + if user_details: j['user_email'] = job.user.email out.append(j) @@ -123,12 +121,13 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses :returns: dictionary containing full description of job data """ job = self.__get_job( trans, id ) - job_dict = self.encode_all_ids( trans, job.to_dict( 'element' ), True ) + is_admin = trans.user_is_admin() + job_dict = self.encode_all_ids( trans, job.to_dict( 'element', system_details=is_admin ), True ) full_output = util.asbool( kwd.get( 'full', 'false' ) ) if full_output: job_dict.update( dict( stderr=job.stderr, stdout=job.stdout ) ) - if trans.user_is_admin(): - job_dict['command_line'] = job.command_line + if is_admin: + job_dict['user_email'] = job.user.email def metric_to_dict(metric): metric_name = metric.metric_name @@ -199,10 +198,16 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses decoded_job_id = trans.security.decode_id( id ) except Exception: raise exceptions.MalformedId() - query = trans.sa_session.query( trans.app.model.Job ).filter( - trans.app.model.Job.user == trans.user, - trans.app.model.Job.id == decoded_job_id - ) + query = trans.sa_session.query( trans.app.model.Job ) + if trans.user_is_admin(): + query = query.filter( + trans.app.model.Job.id == decoded_job_id + ) + else: + query = query.filter( + trans.app.model.Job.user == trans.user, + trans.app.model.Job.id == decoded_job_id + ) job = query.first() if job is None: raise exceptions.ObjectNotFound() diff --git a/test/api/test_jobs.py b/test/api/test_jobs.py index b962f0b914b..d5d6c3d3d51 100644 --- a/test/api/test_jobs.py +++ b/test/api/test_jobs.py @@ -1,4 +1,6 @@ +import datetime import json +import time from operator import itemgetter from base import api @@ -11,14 +13,19 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ): def test_index( self ): # Create HDA to ensure at least one job exists... self.__history_with_new_dataset() - jobs_response = self._get( "jobs" ) - - self._assert_status_code_is( jobs_response, 200 ) - - jobs = jobs_response.json() - assert isinstance( jobs, list ) + jobs = self.__jobs_index() assert "upload1" in map( itemgetter( "tool_id" ), jobs ) + def test_system_details_admin_only( self ): + self.__history_with_new_dataset() + jobs = self.__jobs_index( admin=False ) + job = jobs[0] + self._assert_not_has_keys( job, "command_line", "external_id" ) + + jobs = self.__jobs_index( admin=True ) + job = jobs[0] + self._assert_has_keys( job, "command_line", "external_id" ) + def test_index_state_filter( self ): # Initial number of ok jobs original_count = len( self.__uploads_with_state( "ok" ) ) @@ -31,6 +38,33 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ): new_count = len( self.__uploads_with_state( "ok" ) ) assert original_count < new_count + def test_index_date_filter( self ): + self.__history_with_new_dataset() + two_weeks_ago = (datetime.datetime.utcnow() - datetime.timedelta(7)).isoformat() + last_week = (datetime.datetime.utcnow() - datetime.timedelta(7)).isoformat() + next_week = (datetime.datetime.utcnow() + datetime.timedelta(7)).isoformat() + today = datetime.datetime.utcnow().isoformat() + tomorrow = (datetime.datetime.utcnow() + datetime.timedelta(1)).isoformat() + + jobs = self.__jobs_index( data={"date_range_min": today[0:10], "date_range_max": tomorrow[0:10]} ) + assert len( jobs ) > 0 + today_job_id = jobs[0]["id"] + + jobs = self.__jobs_index( data={"date_range_min": two_weeks_ago, "date_range_max": last_week} ) + assert today_job_id not in map(itemgetter("id"), jobs) + + jobs = self.__jobs_index( data={"date_range_min": last_week, "date_range_max": next_week} ) + assert today_job_id in map(itemgetter("id"), jobs) + + def test_index_history( self ): + history_id, _ = self.__history_with_new_dataset() + jobs = self.__jobs_index( data={"history_id": history_id} ) + assert len( jobs ) > 0 + + history_id = self._new_history() + jobs = self.__jobs_index( data={"history_id": history_id} ) + assert len( jobs ) == 0 + def test_index_multiple_states_filter( self ): # Initial number of ok jobs original_count = len( self.__uploads_with_state( "ok", "new" ) ) @@ -58,6 +92,22 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ): job_details = show_jobs_response.json() self._assert_has_key( job_details, 'id', 'state', 'exit_code', 'update_time', 'create_time' ) + def test_show_security( self ): + history_id, _ = self.__history_with_new_dataset() + jobs_response = self._get( "jobs", data={"history_id": history_id} ) + job = jobs_response.json()[ 0 ] + job_id = job[ "id" ] + + show_jobs_response = self._get( "jobs/%s" % job_id, admin=False ) + self._assert_not_has_keys( show_jobs_response.json(), "command_line", "external_id" ) + + with self._different_user(): + show_jobs_response = self._get( "jobs/%s" % job_id, admin=False ) + self._assert_status_code_is( show_jobs_response, 404 ) + + show_jobs_response = self._get( "jobs/%s" % job_id, admin=True ) + self._assert_has_keys( show_jobs_response.json(), "command_line", "external_id" ) + def test_search( self ): history_id, dataset_id = self.__history_with_ok_dataset() @@ -133,3 +183,10 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ): history_id, dataset_id = self.__history_with_new_dataset() self._wait_for_history( history_id, assert_ok=True ) return history_id, dataset_id + + def __jobs_index( self, **kwds ): + jobs_response = self._get( "jobs", **kwds ) + self._assert_status_code_is( jobs_response, 200 ) + jobs = jobs_response.json() + assert isinstance( jobs, list ) + return jobs diff --git a/test/base/api.py b/test/base/api.py index 0a4d1f4f1ab..7ef7cebca2e 100644 --- a/test/base/api.py +++ b/test/base/api.py @@ -11,6 +11,7 @@ from .api_util import get_user_api_key from .api_asserts import ( assert_status_code_is, assert_has_keys, + assert_not_has_keys, assert_error_code_is, ) @@ -83,6 +84,9 @@ class ApiTestCase( TwillTestCase ): def _assert_has_keys( self, response, *keys ): assert_has_keys( response, *keys ) + def _assert_not_has_keys( self, response, *keys ): + assert_not_has_keys( response, *keys ) + def _assert_error_code_is( self, response, error_code ): assert_error_code_is( response, error_code ) diff --git a/test/base/api_asserts.py b/test/base/api_asserts.py index 129b63fca53..12f7b554eb9 100644 --- a/test/base/api_asserts.py +++ b/test/base/api_asserts.py @@ -20,6 +20,11 @@ def assert_has_keys( response, *keys ): assert key in response, "Response [%s] does not contain key [%s]" % ( response, key ) +def assert_not_has_keys( response, *keys ): + for key in keys: + assert key not in response, "Response [%s] contains invalid key [%s]" % ( response, key ) + + def assert_error_code_is( response, error_code ): if hasattr( response, "json" ): response = response.json()