From 2c3bd8761060e63fd62d53aab57a76391c0ba4a3 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 29 Oct 2024 20:03:04 +0100 Subject: [PATCH 01/59] init add secrets to tools --- lib/galaxy/tool_util/deps/requirements.py | 55 ++++++++++++++++++++--- lib/galaxy/tools/__init__.py | 22 +++++++++ 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index bfb02e0ea606..57ea63ca24c0 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -20,6 +20,7 @@ from galaxy.util import ( asbool, + string_as_bool, xml_text, ) from galaxy.util.oset import OrderedSet @@ -41,17 +42,30 @@ def __init__( type: Optional[str] = None, version: Optional[str] = None, specs: Optional[Iterable["RequirementSpecification"]] = None, + inject_as_env: Optional[str] = None, + interfaces: Optional[List[Dict[str, Any]]] = None, ) -> None: if specs is None: specs = [] + if interfaces is None: + interfaces = [] self.name = name self.type = type self.version = version self.specs = specs + self.inject_as_env = inject_as_env + self.interfaces = interfaces def to_dict(self) -> Dict[str, Any]: specs = [s.to_dict() for s in self.specs] - return dict(name=self.name, type=self.type, version=self.version, specs=specs) + return dict( + name=self.name, + type=self.type, + version=self.version, + specs=specs, + inject_as_env=self.inject_as_env, + interfaces=self.interfaces, + ) def copy(self) -> "ToolRequirement": return copy.deepcopy(self) @@ -62,7 +76,11 @@ def from_dict(cls, d: Dict[str, Any]) -> "ToolRequirement": name = d["name"] type = d.get("type") specs = [RequirementSpecification.from_dict(s) for s in d.get("specs", [])] - return cls(name=name, type=type, version=version, specs=specs) + inject_as_env = d.get("inject_as_env") + interfaces = d.get("interfaces", []) + return cls( + name=name, type=type, version=version, specs=specs, inject_as_env=inject_as_env, interfaces=interfaces + ) def __eq__(self, other: Any) -> bool: return ( @@ -70,13 +88,24 @@ def __eq__(self, other: Any) -> bool: and self.type == other.type and self.version == other.version and self.specs == other.specs + and self.inject_as_env == other.inject_as_env + and self.interfaces == other.interfaces ) def __hash__(self) -> int: - return hash((self.name, self.type, self.version, frozenset(self.specs))) + return hash( + ( + self.name, + self.type, + self.version, + frozenset(self.specs), + self.inject_as_env, + frozenset(tuple(i.items()) for i in self.interfaces), + ) + ) def __str__(self) -> str: - return f"ToolRequirement[{self.name},version={self.version},type={self.type},specs={self.specs}]" + return f"ToolRequirement[{self.name},version={self.version},type={self.type},specs={self.specs},inject_as_env={self.inject_as_env},interfaces={self.interfaces}]" __repr__ = __str__ @@ -352,7 +381,11 @@ def parse_requirements_from_xml(xml_root, parse_resources: bool = False): name = xml_text(requirement_elem) type = requirement_elem.get("type", DEFAULT_REQUIREMENT_TYPE) version = requirement_elem.get("version", DEFAULT_REQUIREMENT_VERSION) - requirement = ToolRequirement(name=name, type=type, version=version) + inject_as_env = requirement_elem.get("inject_as_env") + interfaces = parse_interfaces(requirement_elem) + requirement = ToolRequirement( + name=name, type=type, version=version, inject_as_env=inject_as_env, interfaces=interfaces + ) requirements.append(requirement) container_elems = [] @@ -368,6 +401,18 @@ def parse_requirements_from_xml(xml_root, parse_resources: bool = False): return requirements, containers +def parse_interfaces(requirement_elem): + interfaces = [] + for interface_elem in requirement_elem.findall("interface"): + interface = { + "name": interface_elem.get("name"), + "label": interface_elem.get("label"), + "required": string_as_bool(interface_elem.get("required", "false")), + } + interfaces.append(interface) + return interfaces + + def resource_from_element(resource_elem) -> ResourceRequirement: value_or_expression = xml_text(resource_elem) resource_type = resource_elem.get("type") diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index bb62f87e05d5..e8a26a16dcb0 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -816,6 +816,8 @@ def __init__( self.display_interface = True self.require_login = False self.rerun = False + self.inject_as_env: Optional[str] = None + self.interfaces: List[Dict[str, Any]] = [] # This will be non-None for tools loaded from the database (DynamicTool objects). self.dynamic_tool = None # Define a place to keep track of all input These @@ -1220,6 +1222,26 @@ def parse(self, tool_source: ToolSource, guid: Optional[str] = None, dynamic: bo self.requirements = requirements self.containers = containers self.resource_requirements = resource_requirements + for requirement in self.requirements: + if requirement.type == "secret": + self.inject_as_env = requirement.inject_as_env + self.interfaces = requirement.interfaces + for interface in self.interfaces: + preferences = self.app.config.user_preferences_extra["preferences"] + interface_name = interface.get("name", "") + main_key, input_key = interface_name.split("|") + preferences_inputs = preferences.get(main_key, {}).get("inputs", []) + required = interface.get("required", False) + for input_item in preferences_inputs: + if any(input_item.get("name") == input_key): + input_item["required"] = required + # now this should add it the environment variables to the job as the name self.inject_as_env + # value should be get from vault (trans.user_vault.read_secret(vault_key)) + + self.environment_variables.append({"name": self.inject_as_env, "value": None}) + break + else: + raise exceptions.ConfigurationError(f"Interface {interface_name} not found in user preferences") required_files = tool_source.parse_required_files() if required_files is None: From ec62a63178d01d8d339b8f236678f83eaf377470 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 30 Oct 2024 13:58:23 +0100 Subject: [PATCH 02/59] add secret requirement in tools schema --- lib/galaxy/tool_util/xsd/galaxy.xsd | 70 ++++++++++++++++++++++------- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 1c23f88066fe..40ce2575b014 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -614,7 +614,7 @@ serve as complete descriptions of the runtime of a tool. - + ``` +This is possible to add secrets into tools directly by adding a requirement of +type ``secret``. This will inject the value of the requirement into the +environment of the tool. The value of the requirement is the value of the +environment variable to inject the secret into. + +```xml + + + + + +``` + + ]]> - - - - - Valid values are ``package``, ``set_environment``, ``python-module`` (deprecated), ``binary`` (deprecated) - - - - - For requirements of type ``package`` this value defines a specific version of the tool dependency. - - - - + + + + + + + + Valid values are ``package``, ``set_environment``, ``secret``, ``python-module`` (deprecated), ``binary`` (deprecated) + + + + + For requirements of type ``package`` this value defines a specific version of the tool dependency. + + + + + For requirements of type ``secret`` this value defines the name of the environment variable to inject the value of the requirement into. + + + + + + + + The name of the interface. + + + + + The label of the interface. + + + + + Whether the interface is required. + + + + From 08adae47b878869a585fe3332b3e3a20671c8a0b Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 30 Oct 2024 13:59:05 +0100 Subject: [PATCH 03/59] check the required field with user_preferences_extra --- lib/galaxy/tools/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index e8a26a16dcb0..38a3b11ad9d4 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1234,11 +1234,10 @@ def parse(self, tool_source: ToolSource, guid: Optional[str] = None, dynamic: bo required = interface.get("required", False) for input_item in preferences_inputs: if any(input_item.get("name") == input_key): - input_item["required"] = required - # now this should add it the environment variables to the job as the name self.inject_as_env - # value should be get from vault (trans.user_vault.read_secret(vault_key)) - - self.environment_variables.append({"name": self.inject_as_env, "value": None}) + if input_item["required"] != required: + raise exceptions.ConfigurationError( + f"Interface {interface_name} required mismatch between tool and user preferences" + ) break else: raise exceptions.ConfigurationError(f"Interface {interface_name} not found in user preferences") From ccbc10f22f92cde502c017ba81d848de4a3eb65d Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 30 Oct 2024 14:02:20 +0100 Subject: [PATCH 04/59] validate secret type and store for tool interface in user preferences --- lib/galaxy/tools/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 38a3b11ad9d4..80bf6cf92350 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1234,10 +1234,14 @@ def parse(self, tool_source: ToolSource, guid: Optional[str] = None, dynamic: bo required = interface.get("required", False) for input_item in preferences_inputs: if any(input_item.get("name") == input_key): - if input_item["required"] != required: + if input_item.get("required") != required: raise exceptions.ConfigurationError( f"Interface {interface_name} required mismatch between tool and user preferences" ) + if input_item.get("type") != "secret" or input_item.get("store") != "vault": + raise exceptions.ConfigurationError( + f"Interface {interface_name} type should be 'secret' and store should be 'vault'." + ) break else: raise exceptions.ConfigurationError(f"Interface {interface_name} not found in user preferences") From 45ad57ef66640a197914168bee4bd4d35dfae561 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 30 Oct 2024 17:52:43 +0100 Subject: [PATCH 05/59] Add secrets into tools --- lib/galaxy/tool_util/deps/requirements.py | 135 ++++++++++++---------- lib/galaxy/tool_util/parser/interface.py | 7 +- lib/galaxy/tool_util/parser/xml.py | 2 +- lib/galaxy/tool_util/xsd/galaxy.xsd | 120 ++++++++++--------- lib/galaxy/tools/__init__.py | 34 ++---- lib/galaxy/tools/evaluation.py | 17 ++- 6 files changed, 170 insertions(+), 145 deletions(-) diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index 57ea63ca24c0..3c1a51171f4d 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -42,30 +42,17 @@ def __init__( type: Optional[str] = None, version: Optional[str] = None, specs: Optional[Iterable["RequirementSpecification"]] = None, - inject_as_env: Optional[str] = None, - interfaces: Optional[List[Dict[str, Any]]] = None, ) -> None: if specs is None: specs = [] - if interfaces is None: - interfaces = [] self.name = name self.type = type self.version = version self.specs = specs - self.inject_as_env = inject_as_env - self.interfaces = interfaces def to_dict(self) -> Dict[str, Any]: specs = [s.to_dict() for s in self.specs] - return dict( - name=self.name, - type=self.type, - version=self.version, - specs=specs, - inject_as_env=self.inject_as_env, - interfaces=self.interfaces, - ) + return dict(name=self.name, type=self.type, version=self.version, specs=specs) def copy(self) -> "ToolRequirement": return copy.deepcopy(self) @@ -76,11 +63,7 @@ def from_dict(cls, d: Dict[str, Any]) -> "ToolRequirement": name = d["name"] type = d.get("type") specs = [RequirementSpecification.from_dict(s) for s in d.get("specs", [])] - inject_as_env = d.get("inject_as_env") - interfaces = d.get("interfaces", []) - return cls( - name=name, type=type, version=version, specs=specs, inject_as_env=inject_as_env, interfaces=interfaces - ) + return cls(name=name, type=type, version=version, specs=specs) def __eq__(self, other: Any) -> bool: return ( @@ -88,24 +71,13 @@ def __eq__(self, other: Any) -> bool: and self.type == other.type and self.version == other.version and self.specs == other.specs - and self.inject_as_env == other.inject_as_env - and self.interfaces == other.interfaces ) def __hash__(self) -> int: - return hash( - ( - self.name, - self.type, - self.version, - frozenset(self.specs), - self.inject_as_env, - frozenset(tuple(i.items()) for i in self.interfaces), - ) - ) + return hash((self.name, self.type, self.version, frozenset(self.specs))) def __str__(self) -> str: - return f"ToolRequirement[{self.name},version={self.version},type={self.type},specs={self.specs},inject_as_env={self.inject_as_env},interfaces={self.interfaces}]" + return f"ToolRequirement[{self.name},version={self.version},type={self.type},specs={self.specs}]" __repr__ = __str__ @@ -334,6 +306,54 @@ def resource_requirements_from_list(requirements: Iterable[Dict[str, Any]]) -> L return rr +class SecretsRequirement: + def __init__( + self, + type: str, + user_preferences_key: str, + inject_as_env: str, + label: Optional[str] = "", + required: Optional[bool] = False, + ) -> None: + self.type = type + self.user_preferences_key = user_preferences_key + self.inject_as_env = inject_as_env + self.label = label + self.required = required + if not self.user_preferences_key: + raise ValueError("Missing user_preferences_key") + seperated_key = user_preferences_key.split("/") + if len(seperated_key) != 2 or not seperated_key[0] or not seperated_key[1]: + raise ValueError("Invalid user_preferences_key") + if self.type not in {"vault"}: + raise ValueError(f"Invalid secret type '{self.type}'") + if not self.inject_as_env: + raise ValueError("Missing inject_as_env") + + def to_dict(self) -> Dict[str, Any]: + return { + "type": self.type, + "user_preferences_key": self.user_preferences_key, + "inject_as_env": self.inject_as_env, + "label": self.label, + "required": self.required, + } + + def from_dict(self, dict: Dict[str, Any]) -> "SecretsRequirement": + type = dict["type"] + user_preferences_key = dict["user_preferences_key"] + inject_as_env = dict["inject_as_env"] + label = dict.get("label", "") + required = dict.get("required", False) + return SecretsRequirement( + type=type, + user_preferences_key=user_preferences_key, + inject_as_env=inject_as_env, + label=label, + required=required, + ) + + def parse_requirements_from_lists( software_requirements: List[Union[ToolRequirement, Dict[str, Any]]], containers: Iterable[Dict[str, Any]], @@ -346,15 +366,15 @@ def parse_requirements_from_lists( ) -def parse_requirements_from_xml(xml_root, parse_resources: bool = False): +def parse_requirements_from_xml(xml_root, parse_resources_and_secrets: bool = False): """ Parses requirements, containers and optionally resource requirements from Xml tree. >>> from galaxy.util import parse_xml_string - >>> def load_requirements(contents, parse_resources=False): + >>> def load_requirements(contents, parse_resources_and_secrets=False): ... contents_document = '''%s''' ... root = parse_xml_string(contents_document % contents) - ... return parse_requirements_from_xml(root, parse_resources=parse_resources) + ... return parse_requirements_from_xml(root, parse_resources_and_secrets=parse_resources_and_secrets) >>> reqs, containers = load_requirements('''bwa''') >>> reqs[0].name 'bwa' @@ -373,46 +393,30 @@ def parse_requirements_from_xml(xml_root, parse_resources: bool = False): requirements_elem = xml_root.find("requirements") requirement_elems = [] + container_elems = [] if requirements_elem is not None: requirement_elems = requirements_elem.findall("requirement") + container_elems = requirements_elem.findall("container") requirements = ToolRequirements() for requirement_elem in requirement_elems: name = xml_text(requirement_elem) type = requirement_elem.get("type", DEFAULT_REQUIREMENT_TYPE) version = requirement_elem.get("version", DEFAULT_REQUIREMENT_VERSION) - inject_as_env = requirement_elem.get("inject_as_env") - interfaces = parse_interfaces(requirement_elem) - requirement = ToolRequirement( - name=name, type=type, version=version, inject_as_env=inject_as_env, interfaces=interfaces - ) + requirement = ToolRequirement(name=name, type=type, version=version) requirements.append(requirement) - container_elems = [] - if requirements_elem is not None: - container_elems = requirements_elem.findall("container") - containers = [container_from_element(c) for c in container_elems] - if parse_resources: + if parse_resources_and_secrets: resource_elems = requirements_elem.findall("resource") if requirements_elem is not None else [] resources = [resource_from_element(r) for r in resource_elems] - return requirements, containers, resources + secret_elems = requirements_elem.findall("secret") if requirements_elem is not None else [] + secrets = [secret_from_element(s) for s in secret_elems] + return requirements, containers, resources, secrets return requirements, containers -def parse_interfaces(requirement_elem): - interfaces = [] - for interface_elem in requirement_elem.findall("interface"): - interface = { - "name": interface_elem.get("name"), - "label": interface_elem.get("label"), - "required": string_as_bool(interface_elem.get("required", "false")), - } - interfaces.append(interface) - return interfaces - - def resource_from_element(resource_elem) -> ResourceRequirement: value_or_expression = xml_text(resource_elem) resource_type = resource_elem.get("type") @@ -431,3 +435,18 @@ def container_from_element(container_elem) -> ContainerDescription: shell=shell, ) return container + + +def secret_from_element(secret_elem) -> SecretsRequirement: + type = secret_elem.get("type") + user_preferences_key = secret_elem.get("user_preferences_key") + inject_as_env = secret_elem.get("inject_as_env") + label = secret_elem.get("label", "") + required = string_as_bool(secret_elem.get("required", "false")) + return SecretsRequirement( + type=type, + user_preferences_key=user_preferences_key, + inject_as_env=inject_as_env, + label=label, + required=required, + ) diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index af72bf4a4825..d7c26c82b359 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -35,6 +35,7 @@ from galaxy.tool_util.deps.requirements import ( ContainerDescription, ResourceRequirement, + SecretsRequirement, ToolRequirements, ) @@ -307,8 +308,10 @@ def parse_required_files(self) -> Optional["RequiredFiles"]: @abstractmethod def parse_requirements_and_containers( self, - ) -> Tuple["ToolRequirements", List["ContainerDescription"], List["ResourceRequirement"]]: - """Return triple of ToolRequirement, ContainerDescription and ResourceRequirement lists.""" + ) -> Tuple[ + "ToolRequirements", List["ContainerDescription"], List["ResourceRequirement"], List["SecretsRequirement"] + ]: + """Return triple of ToolRequirement, ContainerDescription, ResourceRequirement, and SecretsRequirement objects.""" @abstractmethod def parse_input_pages(self) -> "PagesSource": diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 863a316e45df..240a60b71d44 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -413,7 +413,7 @@ def parse_include_exclude_list(tag_name): return RequiredFiles.from_dict(as_dict) def parse_requirements_and_containers(self): - return requirements.parse_requirements_from_xml(self.root, parse_resources=True) + return requirements.parse_requirements_from_xml(self.root, parse_resources_and_secrets=True) def parse_input_pages(self) -> "XmlPagesSource": return XmlPagesSource(self.root) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 40ce2575b014..3612b92db24d 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -612,9 +612,10 @@ serve as complete descriptions of the runtime of a tool. + - + ``` -This is possible to add secrets into tools directly by adding a requirement of -type ``secret``. This will inject the value of the requirement into the -environment of the tool. The value of the requirement is the value of the -environment variable to inject the secret into. - -```xml - - - - - -``` - - ]]> - - - - - - - - Valid values are ``package``, ``set_environment``, ``secret``, ``python-module`` (deprecated), ``binary`` (deprecated) - - - - - For requirements of type ``package`` this value defines a specific version of the tool dependency. - - - - - For requirements of type ``secret`` this value defines the name of the environment variable to inject the value of the requirement into. - - - - - - - - The name of the interface. - - - - - The label of the interface. - - - - - Whether the interface is required. - - + + + + + Valid values are ``package``, ``set_environment``, ``python-module`` (deprecated), ``binary`` (deprecated) + + + + + For requirements of type ``package`` this value defines a specific version of the tool dependency. + + + + - + + + + + +``` +]]> + + + + The type of secret to inject. Valid value is ``vault`` for now. + + + + + The name of the user preference key to store the secret in. + + + + + The name of the environment variable to inject the secret into. + + + + + The label of the secret. + + + + + Whether the secret is required to run the tool. + + + Document type of tool help @@ -7809,7 +7812,6 @@ and ``bibtex`` are the only supported options. - @@ -7888,6 +7890,14 @@ and ``bibtex`` are the only supported options. + + + Type of secret for tool execution. + + + + + Documentation for ToolTypeType diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 80bf6cf92350..0930b228a1a6 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -816,8 +816,6 @@ def __init__( self.display_interface = True self.require_login = False self.rerun = False - self.inject_as_env: Optional[str] = None - self.interfaces: List[Dict[str, Any]] = [] # This will be non-None for tools loaded from the database (DynamicTool objects). self.dynamic_tool = None # Define a place to keep track of all input These @@ -1218,33 +1216,17 @@ def parse(self, tool_source: ToolSource, guid: Optional[str] = None, dynamic: bo raise Exception(message) # Requirements (dependencies) - requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() self.requirements = requirements self.containers = containers self.resource_requirements = resource_requirements - for requirement in self.requirements: - if requirement.type == "secret": - self.inject_as_env = requirement.inject_as_env - self.interfaces = requirement.interfaces - for interface in self.interfaces: - preferences = self.app.config.user_preferences_extra["preferences"] - interface_name = interface.get("name", "") - main_key, input_key = interface_name.split("|") - preferences_inputs = preferences.get(main_key, {}).get("inputs", []) - required = interface.get("required", False) - for input_item in preferences_inputs: - if any(input_item.get("name") == input_key): - if input_item.get("required") != required: - raise exceptions.ConfigurationError( - f"Interface {interface_name} required mismatch between tool and user preferences" - ) - if input_item.get("type") != "secret" or input_item.get("store") != "vault": - raise exceptions.ConfigurationError( - f"Interface {interface_name} type should be 'secret' and store should be 'vault'." - ) - break - else: - raise exceptions.ConfigurationError(f"Interface {interface_name} not found in user preferences") + self.secrets = secrets + for secret in self.secrets: + preferences = self.app.config.user_preferences_extra["preferences"] + main_key, input_key = secret.user_preferences_key.split("/") + preferences_input = preferences.get(main_key, {}).get("inputs", []) + if not any(input_item.get("name") == input_key for input_item in preferences_input): + raise exceptions.ConfigurationError(f"User preferences key {secret.user_preferences_key} not found") required_files = tool_source.parse_required_files() if required_files is None: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 582bd65be06e..cf8939f71336 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -28,9 +28,10 @@ ) from galaxy.model.none_like import NoneDataset from galaxy.security.object_wrapper import wrap_with_safe_string +from galaxy.security.vault import UserVaultWrapper from galaxy.structured_app import ( BasicSharedApp, - MinimalToolApp, + StructuredApp, ) from galaxy.tool_util.data import TabularToolDataTable from galaxy.tools.parameters import ( @@ -126,11 +127,11 @@ class ToolEvaluator: tool inputs in an isolated, testable manner. """ - app: MinimalToolApp + app: StructuredApp job: model.Job materialize_datasets: bool = True - def __init__(self, app: MinimalToolApp, tool, job, local_working_directory): + def __init__(self, app: StructuredApp, tool, job, local_working_directory): self.app = app self.job = job self.tool = tool @@ -188,6 +189,16 @@ def set_compute_environment(self, compute_environment: ComputeEnvironment, get_s ) self.execute_tool_hooks(inp_data=inp_data, out_data=out_data, incoming=incoming) + if self.tool.secrets: + user_vault = UserVaultWrapper(self.app.vault, self._user) + for secret in self.tool.secrets: + vault_key = secret.user_preferences_key + secret_value = user_vault.read_secret("preferences/" + vault_key) + if secret_value is not None: + self.environment_variables.append({"name": secret.inject_as_env, "value": secret_value}) + else: + log.warning(f"Failed to read secret from vault with key {vault_key}") + def execute_tool_hooks(self, inp_data, out_data, incoming): # Certain tools require tasks to be completed prior to job execution # ( this used to be performed in the "exec_before_job" hook, but hooks are deprecated ). From b3c3d6cc7f76d8cf54bdb6c90c1bb27fe8341741 Mon Sep 17 00:00:00 2001 From: Arash Date: Wed, 30 Oct 2024 18:59:32 +0100 Subject: [PATCH 06/59] Add tests for secrets in tools --- test/functional/tools/secret_tool.xml | 15 ++++++++++ test/integration/test_vault_extra_prefs.py | 32 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 test/functional/tools/secret_tool.xml diff --git a/test/functional/tools/secret_tool.xml b/test/functional/tools/secret_tool.xml new file mode 100644 index 000000000000..bbcf9b7f91a3 --- /dev/null +++ b/test/functional/tools/secret_tool.xml @@ -0,0 +1,15 @@ + + + + + '$output' + ]]> + + + + + + + + diff --git a/test/integration/test_vault_extra_prefs.py b/test/integration/test_vault_extra_prefs.py index 5072ac69d270..64fbb7a6ea40 100644 --- a/test/integration/test_vault_extra_prefs.py +++ b/test/integration/test_vault_extra_prefs.py @@ -11,6 +11,11 @@ ) from galaxy.model.db.user import get_user_by_email +from galaxy_test.api.test_tools import TestsTools +from galaxy_test.base.populators import ( + DatasetPopulator, + skip_without_tool, +) from galaxy_test.driver import integration_util TEST_USER_EMAIL = "vault_test_user@bx.psu.edu" @@ -134,3 +139,30 @@ def __url(self, action, user): def _get_dbuser(self, app, user): return get_user_by_email(app.model.session, user["email"]) + + +class TestSecretsInExtraUserPreferences( + integration_util.IntegrationTestCase, integration_util.ConfiguresDatabaseVault, TestsTools +): + dataset_populator: DatasetPopulator + + @classmethod + def handle_galaxy_config_kwds(cls, config): + super().handle_galaxy_config_kwds(config) + cls._configure_database_vault(config) + config["user_preferences_extra_conf_path"] = os.path.join( + os.path.dirname(__file__), "user_preferences_extra_conf.yml" + ) + + def setUp(self): + super().setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + + @skip_without_tool("secret_tool") + def test_secrets_tool(self, history_id): + user = self._setup_user(TEST_USER_EMAIL) + url = self._api_url(f"users/{user['id']}/information/inputs", params=dict(key=self.master_api_key)) + put(url, data=json.dumps({"secret_tool|api_key": "test"})) + run_response = self._run("secret", history_id, assert_ok=True) + outputs = run_response["outputs"] + assert outputs[0]["extra_files"][0]["value"] == "test" From 6770499bcbb2c066082c92f3f9b20e61bdb84708 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 12:34:37 +0100 Subject: [PATCH 07/59] Avoid log vault_key --- lib/galaxy/tools/evaluation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index cf8939f71336..d8cfe27e9575 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -197,7 +197,7 @@ def set_compute_environment(self, compute_environment: ComputeEnvironment, get_s if secret_value is not None: self.environment_variables.append({"name": secret.inject_as_env, "value": secret_value}) else: - log.warning(f"Failed to read secret from vault with key {vault_key}") + log.warning("Failed to read secret from vault") def execute_tool_hooks(self, inp_data, out_data, incoming): # Certain tools require tasks to be completed prior to job execution From 336c5af78a1b251a56b09e61a495f3f223080b73 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 13:03:45 +0100 Subject: [PATCH 08/59] fix typo --- lib/galaxy/tool_util/xsd/galaxy.xsd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 3612b92db24d..db5c621c17f2 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -736,7 +736,7 @@ environment variable to inject the secret into. ```xml - + ``` ]]> From e8006e263b7409fc6b18c33b9c7ad14a4fd8d0ed Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 13:13:26 +0100 Subject: [PATCH 09/59] cast app for using vault into StructuredApp --- lib/galaxy/tools/evaluation.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index d8cfe27e9575..96be3a6ca028 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -9,6 +9,7 @@ from typing import ( Any, Callable, + cast, Dict, List, Optional, @@ -31,6 +32,7 @@ from galaxy.security.vault import UserVaultWrapper from galaxy.structured_app import ( BasicSharedApp, + MinimalToolApp, StructuredApp, ) from galaxy.tool_util.data import TabularToolDataTable @@ -127,11 +129,11 @@ class ToolEvaluator: tool inputs in an isolated, testable manner. """ - app: StructuredApp + app: MinimalToolApp job: model.Job materialize_datasets: bool = True - def __init__(self, app: StructuredApp, tool, job, local_working_directory): + def __init__(self, app: MinimalToolApp, tool, job, local_working_directory): self.app = app self.job = job self.tool = tool @@ -190,7 +192,8 @@ def set_compute_environment(self, compute_environment: ComputeEnvironment, get_s self.execute_tool_hooks(inp_data=inp_data, out_data=out_data, incoming=incoming) if self.tool.secrets: - user_vault = UserVaultWrapper(self.app.vault, self._user) + app = cast(StructuredApp, self.app) + user_vault = UserVaultWrapper(app.vault, self._user) for secret in self.tool.secrets: vault_key = secret.user_preferences_key secret_value = user_vault.read_secret("preferences/" + vault_key) From 1b328b390a9b13c6e9204807eab8ef6a344ee839 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 13:13:41 +0100 Subject: [PATCH 10/59] Add secrets parameter to parse_requirements_and_containers method in linters --- lib/galaxy/tool_util/linters/general.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index eb3a98fd3b82..2e13abd0ea53 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -183,7 +183,7 @@ class RequirementNameMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -195,7 +195,7 @@ class RequirementVersionMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -207,7 +207,7 @@ class RequirementVersionWhitespace(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -223,7 +223,7 @@ class ResourceRequirementExpression(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() for rr in resource_requirements: if rr.runtime_required: lint_ctx.warn( From e7c45d40c718328369e9232d2f81a121cc81ac1f Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 15:00:23 +0100 Subject: [PATCH 11/59] add secrets into cwl and yml --- lib/galaxy/tool_util/cwl/parser.py | 3 +++ lib/galaxy/tool_util/deps/requirements.py | 9 ++++++--- lib/galaxy/tool_util/parser/cwl.py | 2 ++ lib/galaxy/tool_util/parser/yaml.py | 1 + 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tool_util/cwl/parser.py b/lib/galaxy/tool_util/cwl/parser.py index e4a4ff83fb20..d2deec42a874 100644 --- a/lib/galaxy/tool_util/cwl/parser.py +++ b/lib/galaxy/tool_util/cwl/parser.py @@ -219,6 +219,9 @@ def software_requirements(self) -> List: def resource_requirements(self) -> List: return self.hints_or_requirements_of_class("ResourceRequirement") + def secrets(self) -> List: + return self.hints_or_requirements_of_class("Secrets") + class CommandLineToolProxy(ToolProxy): _class = "CommandLineTool" diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index 3c1a51171f4d..7b74ec2575be 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -339,13 +339,14 @@ def to_dict(self) -> Dict[str, Any]: "required": self.required, } - def from_dict(self, dict: Dict[str, Any]) -> "SecretsRequirement": + @classmethod + def from_dict(cls, dict: Dict[str, Any]) -> "SecretsRequirement": type = dict["type"] user_preferences_key = dict["user_preferences_key"] inject_as_env = dict["inject_as_env"] label = dict.get("label", "") required = dict.get("required", False) - return SecretsRequirement( + return cls( type=type, user_preferences_key=user_preferences_key, inject_as_env=inject_as_env, @@ -358,11 +359,13 @@ def parse_requirements_from_lists( software_requirements: List[Union[ToolRequirement, Dict[str, Any]]], containers: Iterable[Dict[str, Any]], resource_requirements: Iterable[Dict[str, Any]], -) -> Tuple[ToolRequirements, List[ContainerDescription], List[ResourceRequirement]]: + secrets: Iterable[Dict[str, Any]], +) -> Tuple[ToolRequirements, List[ContainerDescription], List[ResourceRequirement], List[SecretsRequirement]]: return ( ToolRequirements.from_list(software_requirements), [ContainerDescription.from_dict(c) for c in containers], resource_requirements_from_list(resource_requirements), + [SecretsRequirement.from_dict(s) for s in secrets], ) diff --git a/lib/galaxy/tool_util/parser/cwl.py b/lib/galaxy/tool_util/parser/cwl.py index 26d2fdfdc16c..171ab7ce817f 100644 --- a/lib/galaxy/tool_util/parser/cwl.py +++ b/lib/galaxy/tool_util/parser/cwl.py @@ -162,10 +162,12 @@ def parse_requirements_and_containers(self): software_requirements = self.tool_proxy.software_requirements() resource_requirements = self.tool_proxy.resource_requirements() + secrets = self.tool_proxy.secrets() return requirements.parse_requirements_from_lists( software_requirements=[{"name": r[0], "version": r[1], "type": "package"} for r in software_requirements], containers=containers, resource_requirements=resource_requirements, + secrets=secrets, ) def parse_profile(self): diff --git a/lib/galaxy/tool_util/parser/yaml.py b/lib/galaxy/tool_util/parser/yaml.py index 88be9c72846a..4de318b44ddd 100644 --- a/lib/galaxy/tool_util/parser/yaml.py +++ b/lib/galaxy/tool_util/parser/yaml.py @@ -115,6 +115,7 @@ def parse_requirements_and_containers(self): software_requirements=[r for r in mixed_requirements if r.get("type") != "resource"], containers=self.root_dict.get("containers", []), resource_requirements=[r for r in mixed_requirements if r.get("type") == "resource"], + secrets=self.root_dict.get("secrets", []), ) def parse_input_pages(self) -> PagesSource: From f706a1ac6b0eb929bf721029125bc1a680668a93 Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 15:00:45 +0100 Subject: [PATCH 12/59] Fix tool parsing test to get secrets --- test/unit/tool_util/test_parsing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index 21ec92387694..f2b311fc188b 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -347,7 +347,7 @@ def test_action(self): assert self._tool_source.parse_action_module() is None def test_requirements(self): - requirements, containers, resource_requirements = self._tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, secrets = self._tool_source.parse_requirements_and_containers() assert requirements[0].type == "package" assert list(containers)[0].identifier == "mycool/bwa" assert resource_requirements[0].resource_type == "cores_min" From e8a527d3ff418517c6daad2bf1afc58d190bd5ab Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 31 Oct 2024 15:05:34 +0100 Subject: [PATCH 13/59] Fix tool tests to include secrets --- test/unit/tool_util/test_cwl.py | 2 +- test/unit/tool_util/test_parsing.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/tool_util/test_cwl.py b/test/unit/tool_util/test_cwl.py index a3bae657b132..5977ec3565d0 100644 --- a/test/unit/tool_util/test_cwl.py +++ b/test/unit/tool_util/test_cwl.py @@ -281,7 +281,7 @@ def test_load_proxy_simple(): outputs, output_collections = tool_source.parse_outputs(None) assert len(outputs) == 1 - software_requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + software_requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() assert software_requirements.to_dict() == [] assert len(containers) == 1 assert containers[0].to_dict() == { diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index f2b311fc188b..ae7290179abf 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -533,7 +533,9 @@ def test_action(self): assert self._tool_source.parse_action_module() is None def test_requirements(self): - software_requirements, containers, resource_requirements = self._tool_source.parse_requirements_and_containers() + software_requirements, containers, resource_requirements, secrets = ( + self._tool_source.parse_requirements_and_containers() + ) assert software_requirements.to_dict() == [{"name": "bwa", "type": "package", "version": "1.0.1", "specs": []}] assert len(containers) == 1 assert containers[0].to_dict() == { From 7fb353bc2f479b6009aaeec44b8bcaf988ec03fb Mon Sep 17 00:00:00 2001 From: Arash Date: Thu, 28 Nov 2024 17:05:09 +0100 Subject: [PATCH 14/59] Rename 'secrets' to 'credentials' in tool parsing and related methods and use the discussed XML format #19196 --- lib/galaxy/tool_util/cwl/parser.py | 4 +- lib/galaxy/tool_util/deps/requirements.py | 133 +++++++++++++++------- lib/galaxy/tool_util/linters/general.py | 8 +- lib/galaxy/tool_util/parser/cwl.py | 4 +- lib/galaxy/tool_util/parser/interface.py | 6 +- lib/galaxy/tool_util/parser/xml.py | 2 +- lib/galaxy/tool_util/parser/yaml.py | 2 +- lib/galaxy/tool_util/xsd/galaxy.xsd | 113 +++++++++++++----- lib/galaxy/tools/__init__.py | 17 +-- lib/galaxy/tools/evaluation.py | 37 +++--- test/unit/tool_util/test_cwl.py | 2 +- test/unit/tool_util/test_parsing.py | 4 +- 12 files changed, 224 insertions(+), 108 deletions(-) diff --git a/lib/galaxy/tool_util/cwl/parser.py b/lib/galaxy/tool_util/cwl/parser.py index d2deec42a874..cc0b9944483a 100644 --- a/lib/galaxy/tool_util/cwl/parser.py +++ b/lib/galaxy/tool_util/cwl/parser.py @@ -219,8 +219,8 @@ def software_requirements(self) -> List: def resource_requirements(self) -> List: return self.hints_or_requirements_of_class("ResourceRequirement") - def secrets(self) -> List: - return self.hints_or_requirements_of_class("Secrets") + def credentials(self) -> List: + return self.hints_or_requirements_of_class("Credentials") class CommandLineToolProxy(ToolProxy): diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index 7b74ec2575be..7865bdb59435 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -306,52 +306,99 @@ def resource_requirements_from_list(requirements: Iterable[Dict[str, Any]]) -> L return rr -class SecretsRequirement: +class SecretOrVariable: def __init__( self, type: str, - user_preferences_key: str, + name: str, inject_as_env: str, - label: Optional[str] = "", - required: Optional[bool] = False, + label: str = "", + description: str = "", ) -> None: self.type = type - self.user_preferences_key = user_preferences_key + self.name = name self.inject_as_env = inject_as_env self.label = label - self.required = required - if not self.user_preferences_key: - raise ValueError("Missing user_preferences_key") - seperated_key = user_preferences_key.split("/") - if len(seperated_key) != 2 or not seperated_key[0] or not seperated_key[1]: - raise ValueError("Invalid user_preferences_key") - if self.type not in {"vault"}: - raise ValueError(f"Invalid secret type '{self.type}'") + self.description = description + if self.type not in {"secret", "variable"}: + raise ValueError(f"Invalid credential type '{self.type}'") if not self.inject_as_env: raise ValueError("Missing inject_as_env") def to_dict(self) -> Dict[str, Any]: return { "type": self.type, - "user_preferences_key": self.user_preferences_key, + "name": self.name, "inject_as_env": self.inject_as_env, "label": self.label, - "required": self.required, + "description": self.description, } @classmethod - def from_dict(cls, dict: Dict[str, Any]) -> "SecretsRequirement": + def from_element(cls, elem) -> "SecretOrVariable": + return cls( + type=elem.tag, + name=elem.get("name"), + inject_as_env=elem.get("inject_as_env"), + label=elem.get("label", ""), + description=elem.get("description", ""), + ) + + @classmethod + def from_dict(cls, dict: Dict[str, Any]) -> "SecretOrVariable": type = dict["type"] - user_preferences_key = dict["user_preferences_key"] + name = dict["name"] inject_as_env = dict["inject_as_env"] label = dict.get("label", "") + description = dict.get("description", "") + return cls(type=type, name=name, inject_as_env=inject_as_env, label=label, description=description) + + +class CredentialsRequirement: + def __init__( + self, + name: str, + reference: str, + required: bool = False, + label: str = "", + description: str = "", + secrets_and_variables: Optional[List[SecretOrVariable]] = None, + ) -> None: + self.name = name + self.reference = reference + self.required = required + self.label = label + self.description = description + self.secrets_and_variables = secrets_and_variables if secrets_and_variables is not None else [] + + if not self.reference: + raise ValueError("Missing reference") + + def to_dict(self) -> Dict[str, Any]: + return { + "name": self.name, + "reference": self.reference, + "required": self.required, + "label": self.label, + "description": self.description, + "secrets_and_variables": [s.to_dict() for s in self.secrets_and_variables], + } + + @classmethod + def from_dict(cls, dict: Dict[str, Any]) -> "CredentialsRequirement": + name = dict["name"] + reference = dict["reference"] required = dict.get("required", False) + label = dict.get("label", "") + description = dict.get("description", "") + secrets_and_variables = [SecretOrVariable.from_dict(s) for s in dict.get("secrets_and_variables", [])] return cls( - type=type, - user_preferences_key=user_preferences_key, - inject_as_env=inject_as_env, - label=label, + name=name, + reference=reference, required=required, + label=label, + description=description, + secrets_and_variables=secrets_and_variables, ) @@ -359,25 +406,25 @@ def parse_requirements_from_lists( software_requirements: List[Union[ToolRequirement, Dict[str, Any]]], containers: Iterable[Dict[str, Any]], resource_requirements: Iterable[Dict[str, Any]], - secrets: Iterable[Dict[str, Any]], -) -> Tuple[ToolRequirements, List[ContainerDescription], List[ResourceRequirement], List[SecretsRequirement]]: + credentials: Iterable[Dict[str, Any]], +) -> Tuple[ToolRequirements, List[ContainerDescription], List[ResourceRequirement], List[CredentialsRequirement]]: return ( ToolRequirements.from_list(software_requirements), [ContainerDescription.from_dict(c) for c in containers], resource_requirements_from_list(resource_requirements), - [SecretsRequirement.from_dict(s) for s in secrets], + [CredentialsRequirement.from_dict(s) for s in credentials], ) -def parse_requirements_from_xml(xml_root, parse_resources_and_secrets: bool = False): +def parse_requirements_from_xml(xml_root, parse_resources_and_credentials: bool = False): """ Parses requirements, containers and optionally resource requirements from Xml tree. >>> from galaxy.util import parse_xml_string - >>> def load_requirements(contents, parse_resources_and_secrets=False): + >>> def load_requirements(contents, parse_resources_and_credentials=False): ... contents_document = '''%s''' ... root = parse_xml_string(contents_document % contents) - ... return parse_requirements_from_xml(root, parse_resources_and_secrets=parse_resources_and_secrets) + ... return parse_requirements_from_xml(root, parse_resources_and_credentials=parse_resources_and_credentials) >>> reqs, containers = load_requirements('''bwa''') >>> reqs[0].name 'bwa' @@ -410,12 +457,12 @@ def parse_requirements_from_xml(xml_root, parse_resources_and_secrets: bool = Fa requirements.append(requirement) containers = [container_from_element(c) for c in container_elems] - if parse_resources_and_secrets: + if parse_resources_and_credentials: resource_elems = requirements_elem.findall("resource") if requirements_elem is not None else [] resources = [resource_from_element(r) for r in resource_elems] - secret_elems = requirements_elem.findall("secret") if requirements_elem is not None else [] - secrets = [secret_from_element(s) for s in secret_elems] - return requirements, containers, resources, secrets + credentials_elems = requirements_elem.findall("credentials") if requirements_elem is not None else [] + credentials = [credentials_from_element(s) for s in credentials_elems] + return requirements, containers, resources, credentials return requirements, containers @@ -440,16 +487,18 @@ def container_from_element(container_elem) -> ContainerDescription: return container -def secret_from_element(secret_elem) -> SecretsRequirement: - type = secret_elem.get("type") - user_preferences_key = secret_elem.get("user_preferences_key") - inject_as_env = secret_elem.get("inject_as_env") - label = secret_elem.get("label", "") - required = string_as_bool(secret_elem.get("required", "false")) - return SecretsRequirement( - type=type, - user_preferences_key=user_preferences_key, - inject_as_env=inject_as_env, - label=label, +def credentials_from_element(credentials_elem) -> CredentialsRequirement: + name = credentials_elem.get("name") + reference = credentials_elem.get("reference") + required = string_as_bool(credentials_elem.get("required", "false")) + label = credentials_elem.get("label", "") + description = credentials_elem.get("description", "") + secrets_and_variables = [SecretOrVariable.from_element(elem) for elem in credentials_elem.findall("*")] + return CredentialsRequirement( + name=name, + reference=reference, required=required, + label=label, + description=description, + secrets_and_variables=secrets_and_variables, ) diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index 2e13abd0ea53..ddc2c4e6621c 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -183,7 +183,7 @@ class RequirementNameMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, *_ = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -195,7 +195,7 @@ class RequirementVersionMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, *_ = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -207,7 +207,7 @@ class RequirementVersionWhitespace(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, *_ = tool_source.parse_requirements_and_containers() for r in requirements: if r.type != "package": continue @@ -223,7 +223,7 @@ class ResourceRequirementExpression(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): _, tool_node = _tool_xml_and_root(tool_source) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + *_, resource_requirements, _ = tool_source.parse_requirements_and_containers() for rr in resource_requirements: if rr.runtime_required: lint_ctx.warn( diff --git a/lib/galaxy/tool_util/parser/cwl.py b/lib/galaxy/tool_util/parser/cwl.py index 171ab7ce817f..540f8edcfbdd 100644 --- a/lib/galaxy/tool_util/parser/cwl.py +++ b/lib/galaxy/tool_util/parser/cwl.py @@ -162,12 +162,12 @@ def parse_requirements_and_containers(self): software_requirements = self.tool_proxy.software_requirements() resource_requirements = self.tool_proxy.resource_requirements() - secrets = self.tool_proxy.secrets() + credentials = self.tool_proxy.credentials() return requirements.parse_requirements_from_lists( software_requirements=[{"name": r[0], "version": r[1], "type": "package"} for r in software_requirements], containers=containers, resource_requirements=resource_requirements, - secrets=secrets, + credentials=credentials, ) def parse_profile(self): diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index d7c26c82b359..0a8f3c2978c7 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -34,8 +34,8 @@ if TYPE_CHECKING: from galaxy.tool_util.deps.requirements import ( ContainerDescription, + CredentialsRequirement, ResourceRequirement, - SecretsRequirement, ToolRequirements, ) @@ -309,9 +309,9 @@ def parse_required_files(self) -> Optional["RequiredFiles"]: def parse_requirements_and_containers( self, ) -> Tuple[ - "ToolRequirements", List["ContainerDescription"], List["ResourceRequirement"], List["SecretsRequirement"] + "ToolRequirements", List["ContainerDescription"], List["ResourceRequirement"], List["CredentialsRequirement"] ]: - """Return triple of ToolRequirement, ContainerDescription, ResourceRequirement, and SecretsRequirement objects.""" + """Return triple of ToolRequirement, ContainerDescription, ResourceRequirement, and CredentialsRequirement objects.""" @abstractmethod def parse_input_pages(self) -> "PagesSource": diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 240a60b71d44..255fe9482519 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -413,7 +413,7 @@ def parse_include_exclude_list(tag_name): return RequiredFiles.from_dict(as_dict) def parse_requirements_and_containers(self): - return requirements.parse_requirements_from_xml(self.root, parse_resources_and_secrets=True) + return requirements.parse_requirements_from_xml(self.root, parse_resources_and_credentials=True) def parse_input_pages(self) -> "XmlPagesSource": return XmlPagesSource(self.root) diff --git a/lib/galaxy/tool_util/parser/yaml.py b/lib/galaxy/tool_util/parser/yaml.py index 4de318b44ddd..673e5a2cb418 100644 --- a/lib/galaxy/tool_util/parser/yaml.py +++ b/lib/galaxy/tool_util/parser/yaml.py @@ -115,7 +115,7 @@ def parse_requirements_and_containers(self): software_requirements=[r for r in mixed_requirements if r.get("type") != "resource"], containers=self.root_dict.get("containers", []), resource_requirements=[r for r in mixed_requirements if r.get("type") == "resource"], - secrets=self.root_dict.get("secrets", []), + credentials=self.root_dict.get("credentials", []), ) def parse_input_pages(self) -> PagesSource: diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index db5c621c17f2..c688fc4f8692 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -600,10 +600,10 @@ practice. @@ -612,7 +612,7 @@ serve as complete descriptions of the runtime of a tool. - + @@ -726,44 +726,111 @@ Read more about configuring Galaxy to run Docker jobs - + - + + + + + ``` ]]> - + + + + + + + The name of the credential set. + + + + + A reference to the source of the credentials. + + + + + The label of the credential set. + + + - The type of secret to inject. Valid value is ``vault`` for now. + The description of the credential set. - + - The name of the user preference key to store the secret in. + Whether the credentials are required for the tool to run. + + + + + + + + + + The name of the variable. - The name of the environment variable to inject the secret into. + The environment variable name to inject the value as. - The label of the secret. + The label for the variable. - + - Whether the secret is required to run the tool. + The description for the variable. + + + + + + + + + + The name of the secret. + + + + + The environment variable name to inject the value as. + + + + + The label for the secret. + + + + + The description for the secret. @@ -7890,14 +7957,6 @@ and ``bibtex`` are the only supported options. - - - Type of secret for tool execution. - - - - - Documentation for ToolTypeType diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 0930b228a1a6..c24733be9975 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1216,17 +1216,18 @@ def parse(self, tool_source: ToolSource, guid: Optional[str] = None, dynamic: bo raise Exception(message) # Requirements (dependencies) - requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, credentials = tool_source.parse_requirements_and_containers() self.requirements = requirements self.containers = containers self.resource_requirements = resource_requirements - self.secrets = secrets - for secret in self.secrets: - preferences = self.app.config.user_preferences_extra["preferences"] - main_key, input_key = secret.user_preferences_key.split("/") - preferences_input = preferences.get(main_key, {}).get("inputs", []) - if not any(input_item.get("name") == input_key for input_item in preferences_input): - raise exceptions.ConfigurationError(f"User preferences key {secret.user_preferences_key} not found") + self.credentials = credentials + # for credential in self.credentials: + # pass + # preferences = self.app.config.user_preferences_extra["preferences"] + # main_key, input_key = credential.user_preferences_key.split("/") + # preferences_input = preferences.get(main_key, {}).get("inputs", []) + # if not any(input_item.get("name") == input_key for input_item in preferences_input): + # raise exceptions.ConfigurationError(f"User preferences key {credential.user_preferences_key} not found") required_files = tool_source.parse_required_files() if required_files is None: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 96be3a6ca028..bbb72308cc1e 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -6,10 +6,9 @@ import string import tempfile from datetime import datetime -from typing import ( +from typing import ( # cast, Any, Callable, - cast, Dict, List, Optional, @@ -29,11 +28,11 @@ ) from galaxy.model.none_like import NoneDataset from galaxy.security.object_wrapper import wrap_with_safe_string -from galaxy.security.vault import UserVaultWrapper -from galaxy.structured_app import ( + +# from galaxy.security.vault import UserVaultWrapper +from galaxy.structured_app import ( # StructuredApp, BasicSharedApp, MinimalToolApp, - StructuredApp, ) from galaxy.tool_util.data import TabularToolDataTable from galaxy.tools.parameters import ( @@ -191,16 +190,24 @@ def set_compute_environment(self, compute_environment: ComputeEnvironment, get_s ) self.execute_tool_hooks(inp_data=inp_data, out_data=out_data, incoming=incoming) - if self.tool.secrets: - app = cast(StructuredApp, self.app) - user_vault = UserVaultWrapper(app.vault, self._user) - for secret in self.tool.secrets: - vault_key = secret.user_preferences_key - secret_value = user_vault.read_secret("preferences/" + vault_key) - if secret_value is not None: - self.environment_variables.append({"name": secret.inject_as_env, "value": secret_value}) - else: - log.warning("Failed to read secret from vault") + if self.tool.credentials: + # app = cast(StructuredApp, self.app) + # user_vault = UserVaultWrapper(app.vault, self._user) + for credentials in self.tool.credentials: + reference = credentials.reference + for secret_or_variable in credentials.secrets_and_variables: + if secret_or_variable.type == "variable": + # variable_value = self.param_dict.get(f"{reference}/{secret_or_variable.name}") + variable_value = f"A variable: {reference}/{secret_or_variable.name}" + self.environment_variables.append( + {"name": secret_or_variable.inject_as_env, "value": variable_value} + ) + elif secret_or_variable.type == "secret": + # secret_value = user_vault.read_secret(f"{reference}/{secret_or_variable.name}") + secret_value = f"A secret: {reference}/{secret_or_variable.name}" + self.environment_variables.append( + {"name": secret_or_variable.inject_as_env, "value": secret_value} + ) def execute_tool_hooks(self, inp_data, out_data, incoming): # Certain tools require tasks to be completed prior to job execution diff --git a/test/unit/tool_util/test_cwl.py b/test/unit/tool_util/test_cwl.py index 5977ec3565d0..8e95793cc8a9 100644 --- a/test/unit/tool_util/test_cwl.py +++ b/test/unit/tool_util/test_cwl.py @@ -281,7 +281,7 @@ def test_load_proxy_simple(): outputs, output_collections = tool_source.parse_outputs(None) assert len(outputs) == 1 - software_requirements, containers, resource_requirements, secrets = tool_source.parse_requirements_and_containers() + software_requirements, containers, resource_requirements, _ = tool_source.parse_requirements_and_containers() assert software_requirements.to_dict() == [] assert len(containers) == 1 assert containers[0].to_dict() == { diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index ae7290179abf..6973606b7f75 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -347,7 +347,7 @@ def test_action(self): assert self._tool_source.parse_action_module() is None def test_requirements(self): - requirements, containers, resource_requirements, secrets = self._tool_source.parse_requirements_and_containers() + requirements, containers, resource_requirements, _ = self._tool_source.parse_requirements_and_containers() assert requirements[0].type == "package" assert list(containers)[0].identifier == "mycool/bwa" assert resource_requirements[0].resource_type == "cores_min" @@ -533,7 +533,7 @@ def test_action(self): assert self._tool_source.parse_action_module() is None def test_requirements(self): - software_requirements, containers, resource_requirements, secrets = ( + software_requirements, containers, resource_requirements, _ = ( self._tool_source.parse_requirements_and_containers() ) assert software_requirements.to_dict() == [{"name": "bwa", "type": "package", "version": "1.0.1", "specs": []}] From 55616dea5364817f1a9960afb97f72a1a084aa8d Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 2 Dec 2024 15:24:20 +0100 Subject: [PATCH 15/59] Refactor test cases to remove unused TestSecretsInExtraUserPreferences class and add credentials parsing test --- test/integration/test_vault_extra_prefs.py | 50 +++++++++++----------- test/unit/tool_util/test_parsing.py | 15 +++++++ 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/test/integration/test_vault_extra_prefs.py b/test/integration/test_vault_extra_prefs.py index 64fbb7a6ea40..2fde51a0201a 100644 --- a/test/integration/test_vault_extra_prefs.py +++ b/test/integration/test_vault_extra_prefs.py @@ -141,28 +141,28 @@ def _get_dbuser(self, app, user): return get_user_by_email(app.model.session, user["email"]) -class TestSecretsInExtraUserPreferences( - integration_util.IntegrationTestCase, integration_util.ConfiguresDatabaseVault, TestsTools -): - dataset_populator: DatasetPopulator - - @classmethod - def handle_galaxy_config_kwds(cls, config): - super().handle_galaxy_config_kwds(config) - cls._configure_database_vault(config) - config["user_preferences_extra_conf_path"] = os.path.join( - os.path.dirname(__file__), "user_preferences_extra_conf.yml" - ) - - def setUp(self): - super().setUp() - self.dataset_populator = DatasetPopulator(self.galaxy_interactor) - - @skip_without_tool("secret_tool") - def test_secrets_tool(self, history_id): - user = self._setup_user(TEST_USER_EMAIL) - url = self._api_url(f"users/{user['id']}/information/inputs", params=dict(key=self.master_api_key)) - put(url, data=json.dumps({"secret_tool|api_key": "test"})) - run_response = self._run("secret", history_id, assert_ok=True) - outputs = run_response["outputs"] - assert outputs[0]["extra_files"][0]["value"] == "test" +# class TestSecretsInExtraUserPreferences( +# integration_util.IntegrationTestCase, integration_util.ConfiguresDatabaseVault, TestsTools +# ): +# dataset_populator: DatasetPopulator + +# @classmethod +# def handle_galaxy_config_kwds(cls, config): +# super().handle_galaxy_config_kwds(config) +# cls._configure_database_vault(config) +# config["user_preferences_extra_conf_path"] = os.path.join( +# os.path.dirname(__file__), "user_preferences_extra_conf.yml" +# ) + +# def setUp(self): +# super().setUp() +# self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + +# @skip_without_tool("secret_tool") +# def test_secrets_tool(self, history_id): +# user = self._setup_user(TEST_USER_EMAIL) +# url = self._api_url(f"users/{user['id']}/information/inputs", params=dict(key=self.master_api_key)) +# put(url, data=json.dumps({"secret_tool|api_key": "test"})) +# run_response = self._run("secret", history_id, assert_ok=True) +# outputs = run_response["outputs"] +# assert outputs[0]["extra_files"][0]["value"] == "test" diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index 6973606b7f75..169d69ff8759 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -50,6 +50,11 @@ 1 2 67108864 + + + + + @@ -359,6 +364,16 @@ def test_requirements(self): assert resource_requirements[6].resource_type == "shm_size" assert not resource_requirements[0].runtime_required + def test_credentials(self): + *_, credentials = self._tool_source.parse_requirements_and_containers() + assert credentials[0].name == "Apollo" + assert credentials[0].reference == "gmod.org/apollo" + assert credentials[0].required + assert len(credentials[0].secrets_and_variables) == 3 + assert credentials[0].secrets_and_variables[0].type == "variable" + assert credentials[0].secrets_and_variables[1].type == "secret" + assert credentials[0].secrets_and_variables[2].type == "secret" + def test_outputs(self): outputs, output_collections = self._tool_source.parse_outputs(object()) assert len(outputs) == 1, outputs From 49462babf122c068615ff13ae71e79ff6e79cbaa Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 3 Dec 2024 16:08:24 +0100 Subject: [PATCH 16/59] updating the credentials to the new format --- lib/galaxy/tool_util/deps/requirements.py | 98 ++++++++++++++++------ lib/galaxy/tool_util/xsd/galaxy.xsd | 19 +++-- lib/galaxy/tools/__init__.py | 22 +++++ lib/galaxy/tools/evaluation.py | 19 ++--- test/functional/tools/secret_tool.xml | 12 +-- test/integration/test_vault_extra_prefs.py | 11 +-- test/unit/tool_util/test_parsing.py | 16 ++-- 7 files changed, 132 insertions(+), 65 deletions(-) diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index 7865bdb59435..b77566cc3cd9 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -306,28 +306,64 @@ def resource_requirements_from_list(requirements: Iterable[Dict[str, Any]]) -> L return rr -class SecretOrVariable: +class Secret: + def __init__( + self, + name: str, + inject_as_env: str, + label: str = "", + description: str = "", + ) -> None: + self.name = name + self.inject_as_env = inject_as_env + self.label = label + self.description = description + if not self.inject_as_env: + raise ValueError("Missing inject_as_env") + + def to_dict(self) -> Dict[str, Any]: + return { + "name": self.name, + "inject_as_env": self.inject_as_env, + "label": self.label, + "description": self.description, + } + + @classmethod + def from_element(cls, elem) -> "Secret": + return cls( + name=elem.get("name"), + inject_as_env=elem.get("inject_as_env"), + label=elem.get("label", ""), + description=elem.get("description", ""), + ) + + @classmethod + def from_dict(cls, dict: Dict[str, Any]) -> "Secret": + name = dict["name"] + inject_as_env = dict["inject_as_env"] + label = dict.get("label", "") + description = dict.get("description", "") + return cls(name=name, inject_as_env=inject_as_env, label=label, description=description) + + +class Variable: def __init__( self, - type: str, name: str, inject_as_env: str, label: str = "", description: str = "", ) -> None: - self.type = type self.name = name self.inject_as_env = inject_as_env self.label = label self.description = description - if self.type not in {"secret", "variable"}: - raise ValueError(f"Invalid credential type '{self.type}'") if not self.inject_as_env: raise ValueError("Missing inject_as_env") def to_dict(self) -> Dict[str, Any]: return { - "type": self.type, "name": self.name, "inject_as_env": self.inject_as_env, "label": self.label, @@ -335,9 +371,8 @@ def to_dict(self) -> Dict[str, Any]: } @classmethod - def from_element(cls, elem) -> "SecretOrVariable": + def from_element(cls, elem) -> "Variable": return cls( - type=elem.tag, name=elem.get("name"), inject_as_env=elem.get("inject_as_env"), label=elem.get("label", ""), @@ -345,13 +380,12 @@ def from_element(cls, elem) -> "SecretOrVariable": ) @classmethod - def from_dict(cls, dict: Dict[str, Any]) -> "SecretOrVariable": - type = dict["type"] + def from_dict(cls, dict: Dict[str, Any]) -> "Variable": name = dict["name"] inject_as_env = dict["inject_as_env"] label = dict.get("label", "") description = dict.get("description", "") - return cls(type=type, name=name, inject_as_env=inject_as_env, label=label, description=description) + return cls(name=name, inject_as_env=inject_as_env, label=label, description=description) class CredentialsRequirement: @@ -359,17 +393,21 @@ def __init__( self, name: str, reference: str, - required: bool = False, + optional: bool = True, + multiple: bool = False, label: str = "", description: str = "", - secrets_and_variables: Optional[List[SecretOrVariable]] = None, + secrets: Optional[List[Secret]] = None, + variables: Optional[List[Variable]] = None, ) -> None: self.name = name self.reference = reference - self.required = required + self.optional = optional + self.multiple = multiple self.label = label self.description = description - self.secrets_and_variables = secrets_and_variables if secrets_and_variables is not None else [] + self.secrets = secrets if secrets is not None else [] + self.variables = variables if variables is not None else [] if not self.reference: raise ValueError("Missing reference") @@ -378,27 +416,33 @@ def to_dict(self) -> Dict[str, Any]: return { "name": self.name, "reference": self.reference, - "required": self.required, + "optional": self.optional, + "multiple": self.multiple, "label": self.label, "description": self.description, - "secrets_and_variables": [s.to_dict() for s in self.secrets_and_variables], + "secrets": [s.to_dict() for s in self.secrets], + "variables": [v.to_dict() for v in self.variables], } @classmethod def from_dict(cls, dict: Dict[str, Any]) -> "CredentialsRequirement": name = dict["name"] reference = dict["reference"] - required = dict.get("required", False) + optional = dict.get("optional", True) + multiple = dict.get("multiple", False) label = dict.get("label", "") description = dict.get("description", "") - secrets_and_variables = [SecretOrVariable.from_dict(s) for s in dict.get("secrets_and_variables", [])] + secrets = [Secret.from_dict(s) for s in dict.get("secrets", [])] + variables = [Variable.from_dict(v) for v in dict.get("variables", [])] return cls( name=name, reference=reference, - required=required, + optional=optional, + multiple=multiple, label=label, description=description, - secrets_and_variables=secrets_and_variables, + secrets=secrets, + variables=variables, ) @@ -490,15 +534,19 @@ def container_from_element(container_elem) -> ContainerDescription: def credentials_from_element(credentials_elem) -> CredentialsRequirement: name = credentials_elem.get("name") reference = credentials_elem.get("reference") - required = string_as_bool(credentials_elem.get("required", "false")) + optional = string_as_bool(credentials_elem.get("optional", "true")) + multiple = string_as_bool(credentials_elem.get("multiple", "false")) label = credentials_elem.get("label", "") description = credentials_elem.get("description", "") - secrets_and_variables = [SecretOrVariable.from_element(elem) for elem in credentials_elem.findall("*")] + secrets = [Secret.from_element(elem) for elem in credentials_elem.findall("secret")] + variables = [Variable.from_element(elem) for elem in credentials_elem.findall("variable")] return CredentialsRequirement( name=name, reference=reference, - required=required, + optional=optional, + multiple=multiple, label=label, description=description, - secrets_and_variables=secrets_and_variables, + secrets=secrets, + variables=variables, ) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index c688fc4f8692..6700dbd18a22 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -739,11 +739,11 @@ It can contain multiple ``variable`` and ``secret`` tags. ```xml - - - - - + + + + + ``` ]]> @@ -772,9 +772,14 @@ It can contain multiple ``variable`` and ``secret`` tags. The description of the credential set. - + - Whether the credentials are required for the tool to run. + Whether the credentials are optional for the tool to run. + + + + + Indicates multiple sets of credentials can be provided. diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index c24733be9975..839b32d963c6 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -2667,6 +2667,27 @@ def to_json(self, trans, kwd=None, job=None, workflow_building_mode=False, histo state_inputs_json: ToolStateDumpedToJsonT = params_to_json(self.inputs, state_inputs, self.app) + credentials = [] + for credential in self.credentials: + credential_dict = credential.to_dict() + credential_dict["variables"] = [ + { + "name": variable.name, + "label": variable.label, + "description": variable.description, + } + for variable in credential.variables + ] + credential_dict["secrets"] = [ + { + "name": secret.name, + "label": secret.label, + "description": secret.description, + } + for secret in credential.secrets + ] + credentials.append(credential_dict) + # update tool model tool_model.update( { @@ -2679,6 +2700,7 @@ def to_json(self, trans, kwd=None, job=None, workflow_building_mode=False, histo "warnings": tool_warnings, "versions": self.tool_versions, "requirements": [{"name": r.name, "version": r.version} for r in self.requirements], + "credentials": credentials, "errors": state_errors, "tool_errors": self.tool_errors, "state_inputs": state_inputs_json, diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index bbb72308cc1e..12d81a55a0be 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -195,19 +195,12 @@ def set_compute_environment(self, compute_environment: ComputeEnvironment, get_s # user_vault = UserVaultWrapper(app.vault, self._user) for credentials in self.tool.credentials: reference = credentials.reference - for secret_or_variable in credentials.secrets_and_variables: - if secret_or_variable.type == "variable": - # variable_value = self.param_dict.get(f"{reference}/{secret_or_variable.name}") - variable_value = f"A variable: {reference}/{secret_or_variable.name}" - self.environment_variables.append( - {"name": secret_or_variable.inject_as_env, "value": variable_value} - ) - elif secret_or_variable.type == "secret": - # secret_value = user_vault.read_secret(f"{reference}/{secret_or_variable.name}") - secret_value = f"A secret: {reference}/{secret_or_variable.name}" - self.environment_variables.append( - {"name": secret_or_variable.inject_as_env, "value": secret_value} - ) + for secret in credentials.secret: + secret_value = f"{reference}/{secret.name}" + self.environment_variables.append({"name": secret.inject_as_env, "value": secret_value}) + for variable in credentials.variable: + variable_value = f"{reference}/{variable.name}" + self.environment_variables.append({"name": variable.inject_as_env, "value": variable_value}) def execute_tool_hooks(self, inp_data, out_data, incoming): # Certain tools require tasks to be completed prior to job execution diff --git a/test/functional/tools/secret_tool.xml b/test/functional/tools/secret_tool.xml index bbcf9b7f91a3..998bf14dcb1a 100644 --- a/test/functional/tools/secret_tool.xml +++ b/test/functional/tools/secret_tool.xml @@ -1,15 +1,15 @@ - + + + + + '$output' +echo \$service1_url > '$output' && echo \$service1_user >> '$output' && echo \$service1_pass >> '$output' ]]> - - - - diff --git a/test/integration/test_vault_extra_prefs.py b/test/integration/test_vault_extra_prefs.py index 2fde51a0201a..d9166a553b8a 100644 --- a/test/integration/test_vault_extra_prefs.py +++ b/test/integration/test_vault_extra_prefs.py @@ -11,11 +11,12 @@ ) from galaxy.model.db.user import get_user_by_email -from galaxy_test.api.test_tools import TestsTools -from galaxy_test.base.populators import ( - DatasetPopulator, - skip_without_tool, -) + +# from galaxy_test.api.test_tools import TestsTools +# from galaxy_test.base.populators import ( +# DatasetPopulator, +# skip_without_tool, +# ) from galaxy_test.driver import integration_util TEST_USER_EMAIL = "vault_test_user@bx.psu.edu" diff --git a/test/unit/tool_util/test_parsing.py b/test/unit/tool_util/test_parsing.py index 169d69ff8759..ea4ddb1f334f 100644 --- a/test/unit/tool_util/test_parsing.py +++ b/test/unit/tool_util/test_parsing.py @@ -50,10 +50,10 @@ 1 2 67108864 - - - - + + + + @@ -368,11 +368,9 @@ def test_credentials(self): *_, credentials = self._tool_source.parse_requirements_and_containers() assert credentials[0].name == "Apollo" assert credentials[0].reference == "gmod.org/apollo" - assert credentials[0].required - assert len(credentials[0].secrets_and_variables) == 3 - assert credentials[0].secrets_and_variables[0].type == "variable" - assert credentials[0].secrets_and_variables[1].type == "secret" - assert credentials[0].secrets_and_variables[2].type == "secret" + assert credentials[0].optional + assert len(credentials[0].secrets) == 2 + assert len(credentials[0].variables) == 1 def test_outputs(self): outputs, output_collections = self._tool_source.parse_outputs(object()) From 5a9b45a1c7452289f7ac78f9071cc80146edcb16 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 3 Dec 2024 17:33:38 +0100 Subject: [PATCH 17/59] Refactor credential classes (Variable and Secret) to inherit from BaseCredential class --- lib/galaxy/tool_util/deps/requirements.py | 48 +++-------------------- 1 file changed, 6 insertions(+), 42 deletions(-) diff --git a/lib/galaxy/tool_util/deps/requirements.py b/lib/galaxy/tool_util/deps/requirements.py index b77566cc3cd9..62d32dbfb6ca 100644 --- a/lib/galaxy/tool_util/deps/requirements.py +++ b/lib/galaxy/tool_util/deps/requirements.py @@ -306,7 +306,7 @@ def resource_requirements_from_list(requirements: Iterable[Dict[str, Any]]) -> L return rr -class Secret: +class BaseCredential: def __init__( self, name: str, @@ -329,6 +329,8 @@ def to_dict(self) -> Dict[str, Any]: "description": self.description, } + +class Secret(BaseCredential): @classmethod def from_element(cls, elem) -> "Secret": return cls( @@ -338,38 +340,8 @@ def from_element(cls, elem) -> "Secret": description=elem.get("description", ""), ) - @classmethod - def from_dict(cls, dict: Dict[str, Any]) -> "Secret": - name = dict["name"] - inject_as_env = dict["inject_as_env"] - label = dict.get("label", "") - description = dict.get("description", "") - return cls(name=name, inject_as_env=inject_as_env, label=label, description=description) - - -class Variable: - def __init__( - self, - name: str, - inject_as_env: str, - label: str = "", - description: str = "", - ) -> None: - self.name = name - self.inject_as_env = inject_as_env - self.label = label - self.description = description - if not self.inject_as_env: - raise ValueError("Missing inject_as_env") - - def to_dict(self) -> Dict[str, Any]: - return { - "name": self.name, - "inject_as_env": self.inject_as_env, - "label": self.label, - "description": self.description, - } +class Variable(BaseCredential): @classmethod def from_element(cls, elem) -> "Variable": return cls( @@ -379,14 +351,6 @@ def from_element(cls, elem) -> "Variable": description=elem.get("description", ""), ) - @classmethod - def from_dict(cls, dict: Dict[str, Any]) -> "Variable": - name = dict["name"] - inject_as_env = dict["inject_as_env"] - label = dict.get("label", "") - description = dict.get("description", "") - return cls(name=name, inject_as_env=inject_as_env, label=label, description=description) - class CredentialsRequirement: def __init__( @@ -432,8 +396,8 @@ def from_dict(cls, dict: Dict[str, Any]) -> "CredentialsRequirement": multiple = dict.get("multiple", False) label = dict.get("label", "") description = dict.get("description", "") - secrets = [Secret.from_dict(s) for s in dict.get("secrets", [])] - variables = [Variable.from_dict(v) for v in dict.get("variables", [])] + secrets = [Secret.from_element(s) for s in dict.get("secrets", [])] + variables = [Variable.from_element(v) for v in dict.get("variables", [])] return cls( name=name, reference=reference, From 9e7a63e332b49601a2f347ff43e562ee2b04f371 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 6 Dec 2024 15:19:04 +0100 Subject: [PATCH 18/59] user credential model --- lib/galaxy/model/__init__.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 2b443cc985a7..19d24df44034 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -11606,6 +11606,37 @@ def __repr__(self): ) +class UserCredentials(Base): + """ + Represents a credential associated with a user for a specific service. + """ + + __tablename__ = "user_credentials" + + id: Mapped[int] = mapped_column(primary_key=True) + user_id: Mapped[int] = mapped_column(ForeignKey("galaxy_user.id"), index=True, nullable=False) + service_reference: Mapped[str] = mapped_column(nullable=False) # tool|tool_id|refrence|set + create_time: Mapped[Optional[datetime]] = mapped_column(default=now) + update_time: Mapped[Optional[datetime]] = mapped_column(default=now, onupdate=now) + + +class Credentials(Base): + """ + Represents a credential associated with a user for a specific + service. + """ + + __tablename__ = "credentials" + + id: Mapped[int] = mapped_column(primary_key=True) + user_credentials_id: Mapped[int] = mapped_column(ForeignKey("user_credentials.id"), index=True, nullable=False) + name: Mapped[str] = mapped_column(nullable=False) + type: Mapped[str] = mapped_column(nullable=False) + value: Mapped[str] = mapped_column(nullable=False) + create_time: Mapped[Optional[datetime]] = mapped_column(default=now) + update_time: Mapped[Optional[datetime]] = mapped_column(default=now, onupdate=now) + + # The following models (HDA, LDDA) are mapped imperatively (for details see discussion in PR #12064) # TLDR: there are issues ('metadata' property, Galaxy object wrapping) that need to be addressed separately # before these models can be mapped declaratively. Keeping them in the mapping module breaks the auth package From 7500986f0b11a1095fd03ab475eaf64041523b9c Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 6 Dec 2024 15:19:13 +0100 Subject: [PATCH 19/59] Add API and schema for user credentials management --- lib/galaxy/schema/credentials.py | 106 ++++++++ lib/galaxy/webapps/galaxy/api/credentials.py | 128 +++++++++ .../webapps/galaxy/services/credentials.py | 253 ++++++++++++++++++ 3 files changed, 487 insertions(+) create mode 100644 lib/galaxy/schema/credentials.py create mode 100644 lib/galaxy/webapps/galaxy/api/credentials.py create mode 100644 lib/galaxy/webapps/galaxy/services/credentials.py diff --git a/lib/galaxy/schema/credentials.py b/lib/galaxy/schema/credentials.py new file mode 100644 index 000000000000..99d9821187fe --- /dev/null +++ b/lib/galaxy/schema/credentials.py @@ -0,0 +1,106 @@ +from enum import Enum +from typing import List + +from pydantic import ( + Field, + RootModel, +) + +from galaxy.schema.fields import EncodedDatabaseIdField +from galaxy.schema.schema import Model + + +class CredentialType(str, Enum): + secret = "secret" + variable = "variable" + + +class CredentialResponse(Model): + id: EncodedDatabaseIdField = Field( + ..., + title="ID", + description="ID of the credential", + ) + name: str = Field( + ..., + title="Credential Name", + description="Name of the credential", + ) + type: CredentialType = Field( + ..., + title="Type", + description="Type of the credential", + ) + + +class CredentialsListResponse(Model): + service_reference: str = Field( + ..., + title="Service Reference", + description="Reference to the service", + ) + user_credentials_id: EncodedDatabaseIdField = Field( + ..., + title="User Credentials ID", + description="ID of the user credentials", + ) + credentials: List[CredentialResponse] = Field( + ..., + title="Credentials", + description="List of credentials", + ) + + +class UserCredentialsListResponse(RootModel): + root: List[CredentialsListResponse] = Field( + ..., + title="User Credentials", + description="List of user credentials", + ) + + +class CredentialPayload(Model): + name: str = Field( + ..., + title="Credential Name", + description="Name of the credential", + ) + type: CredentialType = Field( + ..., + title="Type", + description="Type of the credential(secret/variable)", + ) + value: str = Field( + ..., + title="Credential Value", + description="Value of the credential", + ) + + +class CredentialsPayload(Model): + service_reference: str = Field( + ..., + title="Service Reference", + description="Reference to the service", + ) + credentials: List[CredentialPayload] = Field( + ..., + title="Credentials", + description="List of credentials", + ) + + +class VerifyCredentialsResponse(Model): + exists: bool = Field( + ..., + title="Exists", + description="Indicates if the credentials exist", + ) + + +class DeleteCredentialsResponse(Model): + deleted: bool = Field( + ..., + title="Deleted", + description="Indicates if the credentials were deleted", + ) diff --git a/lib/galaxy/webapps/galaxy/api/credentials.py b/lib/galaxy/webapps/galaxy/api/credentials.py new file mode 100644 index 000000000000..a2567b16d74c --- /dev/null +++ b/lib/galaxy/webapps/galaxy/api/credentials.py @@ -0,0 +1,128 @@ +""" +API operations on credentials (credentials and variables). +""" + +import logging +from typing import Optional + +from fastapi import Query + +from galaxy.managers.context import ProvidesUserContext +from galaxy.schema.credentials import ( + CredentialsListResponse, + CredentialsPayload, + DeleteCredentialsResponse, + UserCredentialsListResponse, + VerifyCredentialsResponse, +) +from galaxy.schema.fields import DecodedDatabaseIdField +from galaxy.webapps.galaxy.api import ( + depends, + DependsOnTrans, + Router, +) +from galaxy.webapps.galaxy.api.common import UserIdPathParam +from galaxy.webapps.galaxy.services.credentials import CredentialsService + +log = logging.getLogger(__name__) + +router = Router(tags=["users"]) + + +@router.cbv +class FastAPICredentials: + service: CredentialsService = depends(CredentialsService) + + @router.get( + "/api/users/{user_id}/credentials", + summary="Lists all credentials the user has provided", + ) + def list_user_credentials( + self, + user_id: UserIdPathParam, + trans: ProvidesUserContext = DependsOnTrans, + source_type: Optional[str] = Query( + None, + description="The type of source to filter by.", + ), + source_id: Optional[str] = Query( + None, + description="The ID of the source to filter by.", + ), + ) -> UserCredentialsListResponse: + return self.service.list_user_credentials(trans, user_id, source_type, source_id) + + @router.get( + "/api/users/{user_id}/credentials/{user_credentials_id}", + summary="Verifies if credentials have been provided for a specific service", + ) + def verify_service_credentials( + self, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + trans: ProvidesUserContext = DependsOnTrans, + ) -> VerifyCredentialsResponse: + return self.service.verify_service_credentials(trans, user_id, user_credentials_id) + + @router.get( + "/api/users/{user_id}/credentials/{user_credentials_id}/{credentials_id}", + summary="Verifies if a credential have been provided", + ) + def verify_credentials( + self, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + credentials_id: DecodedDatabaseIdField, + trans: ProvidesUserContext = DependsOnTrans, + ) -> VerifyCredentialsResponse: + return self.service.verify_credentials(trans, user_credentials_id, credentials_id) + + @router.post( + "/api/users/{user_id}/credentials", + summary="Allows users to provide credentials for a secret/variable", + ) + def provide_credential( + self, + user_id: UserIdPathParam, + payload: CredentialsPayload, + trans: ProvidesUserContext = DependsOnTrans, + ) -> CredentialsListResponse: + return self.service.provide_credential(trans, user_id, payload) + + @router.put( + "/api/users/{user_id}/credentials/{credentials_id}", + summary="Updates credentials for a specific secret/variable", + ) + def update_credential( + self, + user_id: UserIdPathParam, + credentials_id: DecodedDatabaseIdField, + payload: CredentialsPayload, + trans: ProvidesUserContext = DependsOnTrans, + ) -> CredentialsListResponse: + return self.service.update_credential(trans, user_id, credentials_id, payload) + + @router.delete( + "/api/users/{user_id}/credentials/{user_credentials_id}", + summary="Deletes all credentials for a specific service", + ) + def delete_service_credentials( + self, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + trans: ProvidesUserContext = DependsOnTrans, + ) -> DeleteCredentialsResponse: + return self.service.delete_service_credentials(trans, user_id, user_credentials_id) + + @router.delete( + "/api/users/{user_id}/credentials/{user_credentials_id}/{credentials_id}", + summary="Deletes a specific credential", + ) + def delete_credentials( + self, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + credentials_id: DecodedDatabaseIdField, + trans: ProvidesUserContext = DependsOnTrans, + ) -> DeleteCredentialsResponse: + return self.service.delete_credentials(trans, user_id, user_credentials_id, credentials_id) diff --git a/lib/galaxy/webapps/galaxy/services/credentials.py b/lib/galaxy/webapps/galaxy/services/credentials.py new file mode 100644 index 000000000000..769d8e2f52db --- /dev/null +++ b/lib/galaxy/webapps/galaxy/services/credentials.py @@ -0,0 +1,253 @@ +from typing import ( + Dict, + List, + Optional, + Tuple, + Union, +) + +from galaxy import exceptions +from galaxy.managers.context import ProvidesUserContext +from galaxy.model import ( + Credentials, + UserCredentials, +) +from galaxy.model.base import transaction +from galaxy.schema.credentials import ( + CredentialResponse, + CredentialsListResponse, + CredentialsPayload, + DeleteCredentialsResponse, + UserCredentialsListResponse, + VerifyCredentialsResponse, +) +from galaxy.schema.fields import DecodedDatabaseIdField +from galaxy.security.vault import UserVaultWrapper +from galaxy.structured_app import StructuredApp +from galaxy.webapps.galaxy.api.common import UserIdPathParam + + +class CredentialsService: + """Interface/service object shared by controllers for interacting with credentials.""" + + def __init__(self, app: StructuredApp) -> None: + self._app = app + + def list_user_credentials( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + source_type: Optional[str] = None, + source_id: Optional[str] = None, + ) -> UserCredentialsListResponse: + """Lists all credentials the user has provided (credentials themselves are not included).""" + service_reference = f"{source_type}|{source_id}".strip("|") if source_type else None + user_credentials, credentials_dict = self._user_credentials( + trans, user_id=user_id, service_reference=service_reference + ) + user_credentials_list = [ + CredentialsListResponse( + service_reference=sref, + user_credentials_id=next( + (uc.id for uc in user_credentials if uc.service_reference == sref), + None, + ), + credentials=self._credentials_response(creds), + ) + for sref, creds in credentials_dict.items() + ] + return UserCredentialsListResponse(root=user_credentials_list) + + def verify_service_credentials( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + ) -> VerifyCredentialsResponse: + """Verifies if credentials have been provided for a specific service (no credential data returned).""" + _, credentials_dict = self._user_credentials(trans, user_id=user_id, user_credentials_id=user_credentials_id) + return VerifyCredentialsResponse(exists=bool(credentials_dict)) + + def verify_credentials( + self, + trans: ProvidesUserContext, + user_credentials_id: DecodedDatabaseIdField, + credentials_id: DecodedDatabaseIdField, + ) -> VerifyCredentialsResponse: + """Verifies if a credential have been provided (no credential data returned).""" + credentials = self._credentials(trans, user_credentials_id=user_credentials_id, id=credentials_id) + return VerifyCredentialsResponse(exists=bool(credentials)) + + def provide_credential( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + payload: CredentialsPayload, + ) -> CredentialsListResponse: + """Allows users to provide credentials for a secret/variable.""" + return self._create_user_credential(trans, user_id, payload) + + def update_credential( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + credentials_id: DecodedDatabaseIdField, + payload: CredentialsPayload, + ) -> CredentialsListResponse: + """Updates credentials for a specific secret/variable.""" + return self._create_user_credential(trans, user_id, payload, credentials_id) + + def delete_service_credentials( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + ) -> DeleteCredentialsResponse: + """Deletes all credentials for a specific service.""" + user_credentials, credentials_dict = self._user_credentials( + trans, user_id=user_id, user_credentials_id=user_credentials_id + ) + session = trans.sa_session + for credentials in credentials_dict.values(): + for credential in credentials: + session.delete(credential) + for user_credential in user_credentials: + session.delete(user_credential) + with transaction(session): + session.commit() + return DeleteCredentialsResponse(deleted=True) + + def delete_credentials( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + user_credentials_id: DecodedDatabaseIdField, + credentials_id: DecodedDatabaseIdField, + ) -> DeleteCredentialsResponse: + """Deletes a specific credential.""" + credentials = self._credentials(trans, user_credentials_id=user_credentials_id, id=credentials_id) + session = trans.sa_session + for credential in credentials: + session.delete(credential) + with transaction(session): + session.commit() + return DeleteCredentialsResponse(deleted=True) + + def _user_credentials( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + service_reference: Optional[str] = None, + user_credentials_id: Optional[DecodedDatabaseIdField] = None, + ) -> Tuple[List[UserCredentials], Dict[str, List[Credentials]]]: + if not trans.user_is_admin and (not trans.user or trans.user != user_id): + raise exceptions.ItemOwnershipException( + "Only admins and the user can manage their own credentials.", type="error" + ) + query = trans.sa_session.query(UserCredentials).filter(UserCredentials.user_id == user_id) + if service_reference: + query = query.filter(UserCredentials.service_reference.startswith(service_reference)) + if user_credentials_id: + query = query.filter(UserCredentials.id == user_credentials_id) + user_credentials_list = query.all() + credentials_dict = {} + for user_credential in user_credentials_list: + credentials_list = self._credentials(trans, user_credentials_id=user_credential.id) + credentials_dict[user_credential.service_reference] = credentials_list + return user_credentials_list, credentials_dict + + def _credentials( + self, + trans: ProvidesUserContext, + user_credentials_id: Optional[DecodedDatabaseIdField] = None, + id: Optional[DecodedDatabaseIdField] = None, + name: Optional[str] = None, + type: Optional[str] = None, + ) -> List[Credentials]: + query = trans.sa_session.query(Credentials) + if user_credentials_id: + query = query.filter(Credentials.user_credentials_id == user_credentials_id) + if id: + query = query.filter(Credentials.id == id) + if name: + query = query.filter(Credentials.name == name) + if type: + query = query.filter(Credentials.type == type) + return query.all() + + def _credentials_response(self, credentials_list: List[Credentials]) -> List[CredentialResponse]: + return [ + CredentialResponse( + id=credential.id, + name=credential.name, + type=credential.type, + ) + for credential in credentials_list + ] + + def _create_user_credential( + self, + trans: ProvidesUserContext, + user_id: UserIdPathParam, + payload: CredentialsPayload, + credentials_id: Optional[DecodedDatabaseIdField] = None, + ) -> CredentialsListResponse: + service_reference = payload.service_reference + user_credentials_list, credentials_dict = self._user_credentials( + trans, user_id, service_reference=service_reference + ) + user_credential = user_credentials_list[0] if user_credentials_list else None + + session = trans.sa_session + + if not user_credential: + user_credential = UserCredentials( + user_id=user_id, + service_reference=service_reference, + ) + session.add(user_credential) + session.flush() + + user_credential_id = user_credential.id + + provided_credentials_list: List[Credentials] = [] + for credential_payload in payload.credentials: + credential_name = credential_payload.name + credential_type = credential_payload.type + credential_value = credential_payload.value + + credential = next( + ( + cred + for sref, creds in credentials_dict.items() + if sref == service_reference + for cred in creds + if cred.name == credential_name and cred.type == credential_type + ), + None, + ) + + if not credential: + credential = Credentials( + user_credentials_id=user_credential_id, + name=credential_name, + type=credential_type, + ) + elif not credentials_id: + raise exceptions.RequestParameterInvalidException( + f"Credential {service_reference}|{credential_name} already exists.", type="error" + ) + if credential_type == "secret": + user_vault = UserVaultWrapper(self._app.vault, trans.user) + user_vault.write_secret(f"{service_reference}|{credential_name}", credential_value) + elif credential_type == "variable": + credential.value = credential_value + provided_credentials_list.append(credential) + session.add(credential) + with transaction(session): + session.commit() + return CredentialsListResponse( + service_reference=service_reference, + user_credentials_id=user_credential_id, + credentials=self._credentials_response(provided_credentials_list), + ) From 08d4ba9b822c7656f440ab5b1c8bac2512aad55c Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 6 Dec 2024 15:24:32 +0100 Subject: [PATCH 20/59] Remove unused Union import from credentials service --- lib/galaxy/webapps/galaxy/services/credentials.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/services/credentials.py b/lib/galaxy/webapps/galaxy/services/credentials.py index 769d8e2f52db..6843b765e007 100644 --- a/lib/galaxy/webapps/galaxy/services/credentials.py +++ b/lib/galaxy/webapps/galaxy/services/credentials.py @@ -3,7 +3,6 @@ List, Optional, Tuple, - Union, ) from galaxy import exceptions From c8b623d8053744ede123299f4798da56c0a99fe1 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 5 Dec 2024 12:44:57 +0100 Subject: [PATCH 21/59] Add basic ToolCredentials component and related interfaces for managing tool credentials --- client/src/api/users.ts | 34 ++++ client/src/components/Tool/ToolCard.vue | 7 + .../src/components/Tool/ToolCredentials.vue | 166 ++++++++++++++++++ .../User/Credentials/CredentialsInput.vue | 38 ++++ .../Credentials/ManageToolCredentials.vue | 56 ++++++ 5 files changed, 301 insertions(+) create mode 100644 client/src/components/Tool/ToolCredentials.vue create mode 100644 client/src/components/User/Credentials/CredentialsInput.vue create mode 100644 client/src/components/User/Credentials/ManageToolCredentials.vue diff --git a/client/src/api/users.ts b/client/src/api/users.ts index a6086985d7a9..8ef110ef24a3 100644 --- a/client/src/api/users.ts +++ b/client/src/api/users.ts @@ -35,3 +35,37 @@ export async function fetchCurrentUserQuotaSourceUsage(quotaSourceLabel?: string return toQuotaUsage(data); } + +// TODO: Temporarily using these interfaces until the new API is implemented +export interface CredentialsDefinition { + name: string; + reference: string; + optional: boolean; + multiple: boolean; + label?: string; + description?: string; +} +export interface UserCredentials extends CredentialsDefinition { + variables: Variable[]; + secrets: Secret[]; +} + +export interface ToolCredentials extends CredentialsDefinition { + variables: CredentialDetail[]; + secrets: CredentialDetail[]; +} + +export interface CredentialDetail { + name: string; + label?: string; + description?: string; +} + +export interface Secret extends CredentialDetail { + alreadySet: boolean; + value?: string; +} + +export interface Variable extends CredentialDetail { + value?: string; +} diff --git a/client/src/components/Tool/ToolCard.vue b/client/src/components/Tool/ToolCard.vue index f76955803d2b..599e51e3fab9 100644 --- a/client/src/components/Tool/ToolCard.vue +++ b/client/src/components/Tool/ToolCard.vue @@ -14,6 +14,7 @@ import { useUserStore } from "@/stores/userStore"; import ToolSelectPreferredObjectStore from "./ToolSelectPreferredObjectStore"; import ToolTargetPreferredObjectStorePopover from "./ToolTargetPreferredObjectStorePopover"; +import ToolCredentials from "./ToolCredentials.vue"; import ToolHelpForum from "./ToolHelpForum.vue"; import ToolTutorialRecommendations from "./ToolTutorialRecommendations.vue"; import ToolFavoriteButton from "components/Tool/Buttons/ToolFavoriteButton.vue"; @@ -174,6 +175,12 @@ const showHelpForum = computed(() => isConfigLoaded.value && config.value.enable + +
diff --git a/client/src/components/Tool/ToolCredentials.vue b/client/src/components/Tool/ToolCredentials.vue new file mode 100644 index 000000000000..8554aef08b64 --- /dev/null +++ b/client/src/components/Tool/ToolCredentials.vue @@ -0,0 +1,166 @@ + + + + + diff --git a/client/src/components/User/Credentials/CredentialsInput.vue b/client/src/components/User/Credentials/CredentialsInput.vue new file mode 100644 index 000000000000..9eea61a64dc6 --- /dev/null +++ b/client/src/components/User/Credentials/CredentialsInput.vue @@ -0,0 +1,38 @@ + + + + + diff --git a/client/src/components/User/Credentials/ManageToolCredentials.vue b/client/src/components/User/Credentials/ManageToolCredentials.vue new file mode 100644 index 000000000000..f82461a99b8b --- /dev/null +++ b/client/src/components/User/Credentials/ManageToolCredentials.vue @@ -0,0 +1,56 @@ + + + + + From c6bbc86fe9aa232d50b146ca12da65cbb7f7d526 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 5 Dec 2024 12:56:04 +0100 Subject: [PATCH 22/59] Refactor ToolCredentials component To improve loading state management and user feedback --- .../src/components/Tool/ToolCredentials.vue | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/client/src/components/Tool/ToolCredentials.vue b/client/src/components/Tool/ToolCredentials.vue index 8554aef08b64..d1ee8e682c94 100644 --- a/client/src/components/Tool/ToolCredentials.vue +++ b/client/src/components/Tool/ToolCredentials.vue @@ -18,7 +18,8 @@ const props = defineProps(); const userStore = useUserStore(); -const checkingUserCredentials = ref(true); +const isBusy = ref(true); +const busyMessage = ref(""); const userCredentials = ref(undefined); const hasUserProvidedCredentials = computed(() => { @@ -42,7 +43,7 @@ const provideCredentialsButtonTitle = computed(() => { }); const bannerVariant = computed(() => { - if (checkingUserCredentials.value) { + if (isBusy.value) { return "info"; } return hasUserProvidedCredentials.value ? "success" : "warning"; @@ -54,7 +55,8 @@ const showModal = ref(false); * Check if the user has credentials for the tool. */ async function checkUserCredentials(providedCredentials?: UserCredentials[]) { - checkingUserCredentials.value = true; + busyMessage.value = "Checking your credentials..."; + isBusy.value = true; try { if (userStore.isAnonymous) { return; @@ -81,7 +83,7 @@ async function checkUserCredentials(providedCredentials?: UserCredentials[]) { // TODO: Implement error handling. console.error("Error checking user credentials", error); } finally { - checkingUserCredentials.value = false; + isBusy.value = false; } } @@ -91,10 +93,19 @@ function provideCredentials() { async function onSavedCredentials(providedCredentials: UserCredentials[]) { showModal.value = false; - await saveCredentials(providedCredentials); + busyMessage.value = "Saving your credentials..."; + try { + isBusy.value = true; + userCredentials.value = await saveCredentials(providedCredentials); + } catch (error) { + // TODO: Implement error handling. + console.error("Error saving user credentials", error); + } finally { + isBusy.value = false; + } } -async function saveCredentials(providedCredentials: UserCredentials[]) { +async function saveCredentials(providedCredentials: UserCredentials[]): Promise { // TODO: Implement store and real API request to save the provided credentials. console.log("SAVING CREDENTIALS", providedCredentials); await new Promise((resolve) => setTimeout(resolve, 1000)); @@ -103,6 +114,7 @@ async function saveCredentials(providedCredentials: UserCredentials[]) { secret.alreadySet = true; } } + return providedCredentials; } checkUserCredentials(); @@ -111,7 +123,7 @@ checkUserCredentials();