diff --git a/client/src/components/User/ExternalIdentities/ExternalIdentities.vue b/client/src/components/User/ExternalIdentities/ExternalIdentities.vue index c4002bfeefc..d638749afa6 100644 --- a/client/src/components/User/ExternalIdentities/ExternalIdentities.vue +++ b/client/src/components/User/ExternalIdentities/ExternalIdentities.vue @@ -89,7 +89,7 @@ import Vue from "vue"; import BootstrapVue from "bootstrap-vue"; import { getGalaxyInstance } from "app"; import svc from "./service"; -import { logoutClick } from "layout/menu"; +import { userLogout } from "layout/menu"; import ExternalLogin from "components/User/ExternalIdentities/ExternalLogin.vue"; Vue.use(BootstrapVue); @@ -180,7 +180,7 @@ export default { disconnectAndReset() { // Disconnects the user's final ext id and logouts of current session this.disconnectID(); - logoutClick(); + userLogout(); }, removeItem(item) { this.items = this.items.filter((o) => o != item); diff --git a/client/src/components/User/UserPreferences.vue b/client/src/components/User/UserPreferences.vue index 8521aab0e53..fde374718ac 100644 --- a/client/src/components/User/UserPreferences.vue +++ b/client/src/components/User/UserPreferences.vue @@ -88,6 +88,7 @@ import _l from "utils/localization"; import axios from "axios"; import QueryStringParsing from "utils/query-string-parsing"; import { getUserPreferencesModel } from "components/User/UserPreferencesModel"; +import { userLogoutAll, userLogoutClient } from "layout/menu"; import "@fortawesome/fontawesome-svg-core"; Vue.use(BootstrapVue); @@ -217,11 +218,7 @@ export default { Cancel: function () { Galaxy.modal.hide(); }, - "Sign out": function () { - window.location.href = `${getAppRoot()}user/logout?session_csrf_token=${ - Galaxy.session_csrf_token - }`; - }, + "Sign out": userLogoutAll, }, }); }, @@ -241,15 +238,13 @@ export default { this.handleSubmit(); }, async handleSubmit() { - const Galaxy = getGalaxyInstance(); - const userId = Galaxy.user.id; if (!this.checkFormValidity()) { return false; } if (this.email === this.name) { this.nameState = true; try { - await axios.delete(`${getAppRoot()}api/users/${userId}`); + await axios.delete(`${getAppRoot()}api/users/${this.userId}`); } catch (e) { if (e.response.status === 403) { this.deleteError = @@ -257,7 +252,7 @@ export default { return false; } } - window.location.href = `${getAppRoot()}user/logout?session_csrf_token=${Galaxy.session_csrf_token}`; + userLogoutClient(); } else { this.nameState = false; return false; diff --git a/client/src/layout/menu.js b/client/src/layout/menu.js index 48a864e7f6a..9c9802da72f 100644 --- a/client/src/layout/menu.js +++ b/client/src/layout/menu.js @@ -3,10 +3,16 @@ import { getGalaxyInstance } from "app"; import _l from "utils/localization"; import { CommunicationServerView } from "layout/communication-server-view"; -export function logoutClick() { +const POST_LOGOUT_URL = "root/login?is_logout_redirect=true"; + +/** + * Handles user logout. Invalidates the current session, checks to see if we + * need to log out of OIDC too, and goes to our POST_LOGOUT_URL (or some other + * configured redirect). */ +export function userLogout(logoutAll = false) { const galaxy = getGalaxyInstance(); const session_csrf_token = galaxy.session_csrf_token; - const url = `${galaxy.root}user/logout?session_csrf_token=${session_csrf_token}`; + const url = `${galaxy.root}user/logout?session_csrf_token=${session_csrf_token}&logout_all=${logoutAll}`; axios .get(url) .then(() => { @@ -21,14 +27,29 @@ export function logoutClick() { } }) .then((response) => { - if (response.data && response.data.redirect_uri) { + if (response.data?.redirect_uri) { window.top.location.href = response.data.redirect_uri; } else { - window.top.location.href = `${galaxy.root}root/login?is_logout_redirect=true`; + window.top.location.href = `${galaxy.root}${POST_LOGOUT_URL}`; } }); } +/** User logout with 'log out all sessions' flag set. This will invalidate all + * active sessions a user might have. */ +export function userLogoutAll() { + return userLogout(true); +} + +/** Purely clientside logout, dumps session and redirects without invalidating + * serverside. Currently only used when marking an account deleted -- any + * subsequent navigation after the deletion API request would fail otherwise */ +export function userLogoutClient() { + const galaxy = getGalaxyInstance(); + galaxy.user?.clearSessionStorage(); + window.top.location.href = `${galaxy.root}${POST_LOGOUT_URL}`; +} + export function fetchMenu(options = {}) { const Galaxy = getGalaxyInstance(); const menu = []; @@ -244,7 +265,7 @@ export function fetchMenu(options = {}) { { title: _l("Logout"), divider: true, - onclick: logoutClick, + onclick: userLogout, }, { title: _l("Datasets"), diff --git a/lib/galaxy/jobs/runners/kubernetes.py b/lib/galaxy/jobs/runners/kubernetes.py index f64de8d82ab..5e03f9d26a4 100644 --- a/lib/galaxy/jobs/runners/kubernetes.py +++ b/lib/galaxy/jobs/runners/kubernetes.py @@ -2,7 +2,6 @@ Offload jobs to a Kubernetes cluster. """ -import errno import logging import math import os @@ -459,30 +458,14 @@ class KubernetesJobRunner(AsynchronousJobRunner): # there is no job responding to this job_id, it is either lost or something happened. log.error("No Jobs are available under expected selector app=%s", job_state.job_id) self.mark_as_failed(job_state) - try: - with open(job_state.error_file, 'w') as error_file: - error_file.write("No Kubernetes Jobs are available under expected selector app=%s\n" % job_state.job_id) - except OSError as e: - # Python 2/3 compatible handling of FileNotFoundError - if e.errno == errno.ENOENT: - log.error("Job directory already cleaned up. Assuming already handled for selector app=%s", job_state.job_id) - else: - raise - return job_state + # job is no longer viable - remove from watched jobs + return None else: # there is more than one job associated to the expected unique job id used as selector. log.error("More than one Kubernetes Job associated to job id '%s'", job_state.job_id) self.mark_as_failed(job_state) - try: - with open(job_state.error_file, 'w') as error_file: - error_file.write("More than one Kubernetes Job associated with job id '%s'\n" % job_state.job_id) - except OSError as e: - # Python 2/3 compatible handling of FileNotFoundError - if e.errno == errno.ENOENT: - log.error("Job directory already cleaned up. Assuming already handled for selector app=%s", job_state.job_id) - else: - raise - return job_state + # job is no longer viable - remove from watched jobs + return None def _handle_job_failure(self, job, job_state): # Figure out why job has failed diff --git a/lib/galaxy/tool_util/cwl/util.py b/lib/galaxy/tool_util/cwl/util.py index a4605653f18..f7c5018e458 100644 --- a/lib/galaxy/tool_util/cwl/util.py +++ b/lib/galaxy/tool_util/cwl/util.py @@ -338,7 +338,7 @@ class FileLiteralTarget: self.path = path def __str__(self): - return "FileLiteralTarget[path={}] with {}".format(self.path, self.properties) + return "FileLiteralTarget[contents={}] with {}".format(self.contents, self.properties) class FileUploadTarget: diff --git a/lib/galaxy/tools/repositories.py b/lib/galaxy/tools/repositories.py index 6852574cf3b..a843a18e725 100644 --- a/lib/galaxy/tools/repositories.py +++ b/lib/galaxy/tools/repositories.py @@ -29,6 +29,7 @@ class ValidationContext: self.temporary_path = tempfile.mkdtemp(prefix='tool_validation_') self.config.tool_data_table_config = os.path.join(self.temporary_path, 'tool_data_table_conf.xml') self.config.shed_tool_data_table_config = os.path.join(self.temporary_path, 'shed_tool_data_table_conf.xml') + self.config.interactivetools_enable = True self.tool_data_tables = tool_data_tables self.tool_shed_registry = Bunch(tool_sheds={}) self.datatypes_registry = registry diff --git a/lib/galaxy/webapps/galaxy/controllers/tool_runner.py b/lib/galaxy/webapps/galaxy/controllers/tool_runner.py index d7df059f86e..51f77266c40 100644 --- a/lib/galaxy/webapps/galaxy/controllers/tool_runner.py +++ b/lib/galaxy/webapps/galaxy/controllers/tool_runner.py @@ -47,6 +47,9 @@ class ToolRunner(BaseUIController): # tool id not available, redirect to main page if tool_id is None: return trans.response.send_redirect(url_for(controller='root', action='welcome')) + if tool_id.endswith('/'): + # Probably caused by a redirect + tool_id = tool_id[:-1] tool = self.__get_tool(tool_id) # tool id is not matching, display an error if not tool: diff --git a/lib/galaxy/webapps/galaxy/controllers/user.py b/lib/galaxy/webapps/galaxy/controllers/user.py index 4eefb7eb315..58007f20bc0 100644 --- a/lib/galaxy/webapps/galaxy/controllers/user.py +++ b/lib/galaxy/webapps/galaxy/controllers/user.py @@ -200,7 +200,7 @@ class User(BaseUIController, UsesFormDefinitionsMixin, CreatesApiKeysMixin): """ if email is None: # User is coming from outside registration form, load email from trans if not trans.user: - trans.show_error_message("No session found, cannot send activation email.") + return "No session found, cannot send activation email.", None email = trans.user.email if username is None: # User is coming from outside registration form, load email from trans username = trans.user.username diff --git a/lib/galaxy_test/selenium/test_sign_out.py b/lib/galaxy_test/selenium/test_sign_out.py index 9a9162aef83..8bbd5890aa1 100644 --- a/lib/galaxy_test/selenium/test_sign_out.py +++ b/lib/galaxy_test/selenium/test_sign_out.py @@ -18,4 +18,5 @@ class SignOutTestCase(SeleniumTestCase): self.assertTrue(email == new_email) self.components.preferences.sign_out.wait_for_and_click() self.components.sign_out.sign_out_button.wait_for_and_click() + self.sleep_for(self.wait_types.UX_TRANSITION) assert not self.is_logged_in() diff --git a/test/integration/test_kubernetes_runner.py b/test/integration/test_kubernetes_runner.py index 09b325b92b0..dada94cdfb3 100644 --- a/test/integration/test_kubernetes_runner.py +++ b/test/integration/test_kubernetes_runner.py @@ -216,6 +216,17 @@ class BaseKubernetesIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase, M job_env = self._run_and_get_environment_properties() assert job_env.some_env == '42' + @staticmethod + def _wait_for_external_state(sa_session, job, expected): + # Not checking the state here allows the change from queued to running to overwrite + # the change from queued to deleted_new in the API thread - this is a problem because + # the job will still run. See issue https://github.com/galaxyproject/galaxy/issues/4960. + max_tries = 60 + while max_tries > 0 and job.job_runner_external_id is None or job.state != expected: + sa_session.refresh(job) + time.sleep(1) + max_tries -= 1 + @skip_without_tool('cat_data_and_sleep') def test_kill_process(self): with self.dataset_populator.test_history() as history_id: @@ -234,22 +245,12 @@ class BaseKubernetesIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase, M app = self._app sa_session = app.model.context.current - external_id = None - state = False + job = sa_session.query(app.model.Job).get(app.security.decode_id(job_dict["id"])) - job = sa_session.query(app.model.Job).filter_by(tool_id="cat_data_and_sleep").one() - # Not checking the state here allows the change from queued to running to overwrite - # the change from queued to deleted_new in the API thread - this is a problem because - # the job will still run. See issue https://github.com/galaxyproject/galaxy/issues/4960. - max_tries = 60 - while max_tries > 0 and external_id is None or state != app.model.Job.states.RUNNING: - sa_session.refresh(job) - assert not job.finished - external_id = job.job_runner_external_id - state = job.state - time.sleep(1) - max_tries -= 1 + self._wait_for_external_state(sa_session, job, app.model.Job.states.RUNNING) + assert not job.finished + external_id = job.job_runner_external_id output = unicodify(subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json'])) status = json.loads(output) assert status['status']['active'] == 1 @@ -264,6 +265,42 @@ class BaseKubernetesIntegrationTestCase(BaseJobEnvironmentIntegrationTestCase, M subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json'], stderr=subprocess.STDOUT) assert "not found" in unicodify(excinfo.value.output) + @skip_without_tool('cat_data_and_sleep') + def test_external_job_delete(self): + with self.dataset_populator.test_history() as history_id: + hda1 = self.dataset_populator.new_dataset(history_id, content="1 2 3") + running_inputs = { + "input1": {"src": "hda", "id": hda1["id"]}, + "sleep_time": 240, + } + running_response = self.dataset_populator.run_tool( + "cat_data_and_sleep", + running_inputs, + history_id, + assert_ok=False, + ) + job_dict = running_response.json()["jobs"][0] + + app = self._app + sa_session = app.model.context.current + job = sa_session.query(app.model.Job).get(app.security.decode_id(job_dict["id"])) + + self._wait_for_external_state(sa_session, job, app.model.Job.states.RUNNING) + + external_id = job.job_runner_external_id + output = unicodify(subprocess.check_output(['kubectl', 'get', 'job', external_id, '-o', 'json'])) + status = json.loads(output) + assert status['status']['active'] == 1 + + output = unicodify(subprocess.check_output(['kubectl', 'delete', 'job', external_id, '-o', 'name'])) + assert 'job.batch/%s' % external_id in output + + result = self.dataset_populator.wait_for_tool_run(run_response=running_response, history_id=history_id, + assert_ok=False).json() + details = self.dataset_populator.get_job_details(result['jobs'][0]['id'], full=True).json() + + assert details['state'] == app.model.Job.states.ERROR, details + @skip_without_tool('job_properties') def test_exit_code_127(self): inputs = {