diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index cccb1aaa9f58..bb470ba8c23c 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -429,7 +429,12 @@ def publish_history(self, history_id: str) -> None: def test_data_path(self, tool_id, filename, tool_version=None): version_fragment = f"&tool_version={tool_version}" if tool_version else "" response = self._get(f"tools/{tool_id}/test_data_path?filename={filename}{version_fragment}", admin=True) - result = response.json() + try: + result = response.json() + except Exception: + raise Exception( + f"Failed to parse test_data_path from Galaxy. An admin key is required for this feature and it is probably not set or not a valid admin key. Status Code: {response.status_code}. Response: {response.text}." + ) if response.status_code in [200, 404]: return result raise Exception(result["err_msg"]) diff --git a/lib/galaxy/webapps/galaxy/api/tools.py b/lib/galaxy/webapps/galaxy/api/tools.py index dc27f7c3408e..27382897de12 100644 --- a/lib/galaxy/webapps/galaxy/api/tools.py +++ b/lib/galaxy/webapps/galaxy/api/tools.py @@ -249,8 +249,8 @@ def build(self, trans: GalaxyWebTransaction, id, **kwd): tool = self.service._get_tool(trans, id, tool_version=tool_version, user=trans.user) return tool.to_json(trans, kwd.get("inputs", kwd), history=history) - @web.require_admin @expose_api + @web.require_admin def test_data_path(self, trans: GalaxyWebTransaction, id, **kwd): """ GET /api/tools/{tool_id}/test_data_path?tool_version={tool_version} @@ -347,8 +347,8 @@ def test_data(self, trans: GalaxyWebTransaction, id, **kwd) -> List[ToolTestDesc test_defs.extend([t.to_dict() for t in tool.tests]) return test_defs - @web.require_admin @expose_api + @web.require_admin def reload(self, trans: GalaxyWebTransaction, id, **kwd): """ GET /api/tools/{tool_id}/reload @@ -360,8 +360,8 @@ def reload(self, trans: GalaxyWebTransaction, id, **kwd): raise exceptions.MessageException(message) return {"message": message} - @web.require_admin @expose_api + @web.require_admin def all_requirements(self, trans: GalaxyWebTransaction, **kwds): """ GET /api/tools/all_requirements @@ -370,8 +370,8 @@ def all_requirements(self, trans: GalaxyWebTransaction, **kwds): return trans.app.toolbox.all_requirements - @web.require_admin @expose_api + @web.require_admin def requirements(self, trans: GalaxyWebTransaction, id, **kwds): """ GET /api/tools/{tool_id}/requirements @@ -381,8 +381,8 @@ def requirements(self, trans: GalaxyWebTransaction, id, **kwds): tool = self.service._get_tool(trans, id, user=trans.user) return tool.tool_requirements_status - @web.require_admin @expose_api + @web.require_admin def install_dependencies(self, trans: GalaxyWebTransaction, id, **kwds): """ POST /api/tools/{tool_id}/dependencies @@ -409,8 +409,8 @@ def install_dependencies(self, trans: GalaxyWebTransaction, id, **kwds): # _view.install_dependencies should return a dict with stdout, stderr and success status return tool.tool_requirements_status - @web.require_admin @expose_api + @web.require_admin def uninstall_dependencies(self, trans: GalaxyWebTransaction, id, **kwds): """ DELETE /api/tools/{tool_id}/dependencies @@ -431,8 +431,8 @@ def uninstall_dependencies(self, trans: GalaxyWebTransaction, id, **kwds): # TODO: rework resolver install system to log and report what has been done. return tool.tool_requirements_status - @web.require_admin @expose_api + @web.require_admin def build_dependency_cache(self, trans: GalaxyWebTransaction, id, **kwds): """ POST /api/tools/{tool_id}/build_dependency_cache @@ -446,8 +446,8 @@ def build_dependency_cache(self, trans: GalaxyWebTransaction, id, **kwds): # TODO: Should also have a more meaningful return. return tool.tool_requirements_status - @web.require_admin @expose_api + @web.require_admin def diagnostics(self, trans: GalaxyWebTransaction, id, **kwd): """ GET /api/tools/{tool_id}/diagnostics @@ -563,8 +563,8 @@ def raw_tool_source(self, trans: GalaxyWebTransaction, id, **kwds): trans.response.headers["language"] = tool.tool_source.language return tool.tool_source.to_string() - @web.require_admin @expose_api + @web.require_admin def error_stack(self, trans: GalaxyWebTransaction, **kwd): """ GET /api/tools/error_stack diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 46c6cdad3473..784ef1848a84 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -16,11 +16,13 @@ put, ) +from galaxy.exceptions import error_codes from galaxy.tool_util.verify.interactor import ValidToolTestDict from galaxy.util import galaxy_root_path from galaxy.util.unittest_utils import skip_if_github_down from galaxy_test.base import rules_test_data from galaxy_test.base.api_asserts import ( + assert_error_code_is, assert_has_keys, assert_status_code_is, ) @@ -388,7 +390,10 @@ def test_test_data_filepath_security(self): @skip_without_tool("composite_output") def test_test_data_admin_security(self): test_data_response = self._get("tools/composite_output/test_data_path?filename=../CONTRIBUTORS.md") - assert test_data_response.status_code == 403, test_data_response.text + assert_status_code_is(test_data_response, 403) + error_json = test_data_response.json() + assert_has_keys(error_json, "err_msg", "err_code") + assert_error_code_is(error_json, error_codes.error_codes_by_name["ADMIN_REQUIRED"].code) @skip_without_tool("dbkey_filter_multi_input") def test_data_table_requirement_annotated(self): diff --git a/lib/galaxy_test/base/api.py b/lib/galaxy_test/base/api.py index 63e33dd5e90a..d89c158385f1 100644 --- a/lib/galaxy_test/base/api.py +++ b/lib/galaxy_test/base/api.py @@ -147,7 +147,7 @@ def _setup_user_get_key(self, email, password=None): return user, self._post(f"users/{user['id']}/api_key", admin=True).json() @contextmanager - def _different_user(self, email=OTHER_USER, anon=False): + def _different_user(self, email=OTHER_USER, anon=False, invalid_admin_key=False): """Use in test cases to switch get/post operations to act as new user ..code-block:: python @@ -157,6 +157,7 @@ def _different_user(self, email=OTHER_USER, anon=False): """ original_api_key = self.user_api_key + original_admin_api_key = self.master_api_key original_interactor_key = self.galaxy_interactor.api_key original_cookies = self.galaxy_interactor.cookies if anon: @@ -168,10 +169,12 @@ def _different_user(self, email=OTHER_USER, anon=False): try: self.user_api_key = new_key self.galaxy_interactor.api_key = new_key + self.galaxy_interactor.master_api_key = None if not invalid_admin_key else new_key yield finally: self.user_api_key = original_api_key self.galaxy_interactor.api_key = original_interactor_key + self.master_api_key = original_admin_api_key self.galaxy_interactor.cookies = original_cookies def _get(self, *args, **kwds): diff --git a/test/integration/test_galaxy_interactor.py b/test/integration/test_galaxy_interactor.py index 4e3283dba605..54d358d01c44 100644 --- a/test/integration/test_galaxy_interactor.py +++ b/test/integration/test_galaxy_interactor.py @@ -15,6 +15,18 @@ def test_local_test_data_download(self): b"chr1\t147962192\t147962580" ) + def test_data_path_error_message(self): + with self._different_user(invalid_admin_key=True): # other user is not admin, attempt to use their key anyway + exc = None + try: + data_path = self.galaxy_interactor.test_data_path("cat1", "1.bed") + print(data_path) + assert not data_path + except Exception as e: + exc = e + assert exc is not None + assert "You must be an administrator" in str(exc) + def test_run_test_select_version(self): self._run_tool_test(tool_id="multiple_versions", tool_version="0.1")