diff --git a/lib/galaxy/tool_util/deps/mulled/mulled_search.py b/lib/galaxy/tool_util/deps/mulled/mulled_search.py index d936802a0840..cf350f76cd9c 100755 --- a/lib/galaxy/tool_util/deps/mulled/mulled_search.py +++ b/lib/galaxy/tool_util/deps/mulled/mulled_search.py @@ -5,6 +5,10 @@ import logging import sys import tempfile +from typing import ( + Dict, + List, +) from galaxy.tool_util.deps.conda_util import CondaContext from galaxy.util import ( @@ -122,7 +126,7 @@ class CondaSearch: def __init__(self, channel): self.channel = channel - def get_json(self, search_string): + def get_json(self, search_string) -> List[Dict[str, str]]: """ Function takes search_string variable and returns results from the bioconda channel in JSON format @@ -135,8 +139,16 @@ def get_json(self, search_string): except Exception as e: logging.info(f"Search failed with: {e}") return [] + header_found = False + lines_fields: List[List[str]] = [] + for line in raw_out.splitlines(): + if line.startswith("#"): + header_found = True + elif header_found: + lines_fields.append(line.split()) return [ - {"package": n.split()[0], "version": n.split()[1], "build": n.split()[2]} for n in raw_out.split("\n")[2:-1] + {"package": line_fields[0], "version": line_fields[1], "build": line_fields[2]} + for line_fields in lines_fields ] diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index 3b10c9668b01..cbf9b670ad93 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -985,6 +985,7 @@ def set_information(self, trans, id, payload=None, **kwd): extra_user_pref_data = {} extra_pref_keys = self._get_extra_user_preferences(trans) user_vault = UserVaultWrapper(trans.app.vault, user) + current_extra_user_pref_data = json.loads(user.preferences.get("extra_user_preferences", "{}")) if extra_pref_keys is not None: for key in extra_pref_keys: key_prefix = f"{key}|" @@ -997,8 +998,17 @@ def set_information(self, trans, id, payload=None, **kwd): input = matching_input[0] if input.get("required") and payload[item] == "": raise exceptions.ObjectAttributeMissingException("Please fill the required field") - if not (input.get("type") == "secret" and payload[item] == "__SECRET_PLACEHOLDER__"): - if input.get("store") == "vault": + input_type = input.get("type") + is_secret_value_unchanged = ( + input_type == "secret" and payload[item] == "__SECRET_PLACEHOLDER__" + ) + is_stored_in_vault = input.get("store") == "vault" + if is_secret_value_unchanged: + if not is_stored_in_vault: + # If the value is unchanged, keep the current value + extra_user_pref_data[item] = current_extra_user_pref_data.get(item, "") + else: + if is_stored_in_vault: user_vault.write_secret(f"preferences/{keys[0]}/{keys[1]}", str(payload[item])) else: extra_user_pref_data[item] = payload[item] diff --git a/test/integration/test_vault_extra_prefs.py b/test/integration/test_vault_extra_prefs.py index 5072ac69d270..583ce9d72f30 100644 --- a/test/integration/test_vault_extra_prefs.py +++ b/test/integration/test_vault_extra_prefs.py @@ -129,6 +129,89 @@ def test_extra_prefs_vault_storage_update_secret(self): == "an_updated_secret_value" ) + def test_extra_prefs_secret_not_in_vault(self): + user = self._setup_user(TEST_USER_EMAIL) + url = self.__url("information/inputs", user) + app = cast(Any, self._test_driver.app if self._test_driver else None) + db_user = self._get_dbuser(app, user) + + # create some initial data + put( + url, + data=json.dumps( + { + "non_vault_test_section|user": "test_user", + "non_vault_test_section|pass": "test_pass", + } + ), + ) + + # retrieve saved data + response = get(url).json() + + def get_input_by_name(inputs, name): + return [input for input in inputs if input["name"] == name][0] + + inputs = [section for section in response["inputs"] if section["name"] == "non_vault_test_section"][0]["inputs"] + + # value should be what we saved + input_user = get_input_by_name(inputs, "user") + assert input_user["value"] == "test_user" + + # the secret should not be stored in the vault + assert app.vault.read_secret(f"user/{db_user.id}/preferences/non_vault_test_section/pass") is None + # it should be in the user preferences model + app.model.session.refresh(db_user) + assert db_user.extra_preferences["non_vault_test_section|pass"] == "test_pass" + + # secret type values should not be retrievable by the client + input_pass = get_input_by_name(inputs, "pass") + assert input_pass["value"] != "test_pass" + assert input_pass["value"] == "__SECRET_PLACEHOLDER__" + + # changing the text property value should not change the secret property value + put( + url, + data=json.dumps( + { + "non_vault_test_section|user": "a_new_test_user", + "non_vault_test_section|pass": "__SECRET_PLACEHOLDER__", + } + ), + ) + + response = get(url).json() + inputs = [section for section in response["inputs"] if section["name"] == "non_vault_test_section"][0]["inputs"] + input_user = get_input_by_name(inputs, "user") + assert input_user["value"] == "a_new_test_user" + # the secret value is not exposed to the client + input_pass = get_input_by_name(inputs, "pass") + assert input_pass["value"] == "__SECRET_PLACEHOLDER__" + + # The secret value should not have changed in the internal user preferences model + app.model.session.refresh(db_user) + assert db_user.extra_preferences["non_vault_test_section|pass"] == "test_pass" + + # changing the secret value should update the secret value + put( + url, + data=json.dumps( + { + "non_vault_test_section|user": "a_new_test_user", + "non_vault_test_section|pass": "a_new_test_pass", + } + ), + ) + + response = get(url).json() + inputs = [section for section in response["inputs"] if section["name"] == "non_vault_test_section"][0]["inputs"] + input_pass = get_input_by_name(inputs, "pass") + assert input_pass["value"] == "__SECRET_PLACEHOLDER__" + + # the secret value should be updated in the internal user preferences model + app.model.session.refresh(db_user) + assert db_user.extra_preferences["non_vault_test_section|pass"] == "a_new_test_pass" + def __url(self, action, user): return self._api_url(f"users/{user['id']}/{action}", params=dict(key=self.master_api_key)) diff --git a/test/integration/user_preferences_extra_conf.yml b/test/integration/user_preferences_extra_conf.yml index b678edeed82c..9fe29b8f22a7 100644 --- a/test/integration/user_preferences_extra_conf.yml +++ b/test/integration/user_preferences_extra_conf.yml @@ -21,3 +21,15 @@ preferences: type: secret store: vault required: True + + non_vault_test_section: + description: For testing Settings + inputs: + - name: user + label: User + type: text + required: True + - name: pass + label: Pass + type: secret # It's a secret but not stored in vault + required: True diff --git a/test/unit/tool_util/mulled/test_mulled_search.py b/test/unit/tool_util/mulled/test_mulled_search.py index dd17ab79d0ee..8fc0868eda28 100644 --- a/test/unit/tool_util/mulled/test_mulled_search.py +++ b/test/unit/tool_util/mulled/test_mulled_search.py @@ -25,10 +25,10 @@ def test_quay_search(): @skip_unless_executable("conda") def test_conda_search(): t = CondaSearch("bioconda") - search1 = t.get_json("asdfasdf") - search2 = t.get_json("bioconductor-gosemsim") - assert search1 == [] - assert all(r["package"] == "bioconductor-gosemsim" for r in search2) + search = t.get_json("asdfasdf") + assert search == [] + search = t.get_json("bioconductor-gosemsim") + assert all(r["package"] == "bioconductor-gosemsim" for r in search) @external_dependency_management