Merge branch 'release_24.2' into dev

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
         ]
 
 

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]

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))
 

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

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