Merge pull request #19610 from davelopez/24.0_fix_user_prefs_secret_without_vault (d550b77e) · Commits · NDIP / Galaxy

lib/galaxy/webapps/galaxy/api/users.py

+12 −2

Original line number	Diff line number	Diff line
		@@ -962,6 +962,7 @@ class UserAPIController(BaseGalaxyAPIController, UsesTagsMixin, BaseUIController
		extra_user_pref_data = dict()
		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}\|"
		@@ -974,8 +975,17 @@ class UserAPIController(BaseGalaxyAPIController, UsesTagsMixin, BaseUIController
		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

+83 −0

Original line number	Diff line number	Diff line
		@@ -129,6 +129,89 @@ class TestExtraUserPreferences(integration_util.IntegrationTestCase, integration
		== "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

+12 −0

Original line number	Diff line number	Diff line
		@@ -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