diff --git a/api/actions/models.py b/api/actions/models.py index 95486cf6..9b854963 100644 --- a/api/actions/models.py +++ b/api/actions/models.py @@ -4,7 +4,7 @@ from enum import Enum from typing import List, Dict, Any -from sqlmodel import SQLModel +from sqlmodel import SQLModel, Field from api.jobs.models import AwsBatchConfig @@ -36,6 +36,15 @@ class PlatformConfig(SQLModel): export_command: str | None = None +class ActionAwsBatchConfig(AwsBatchConfig): + """AWS Batch configuration for an action, which runs as the submitting user.""" + # Hours an API key minted for the submitter stays valid, for a job (such as a + # pipeline launcher) that calls back into NGS360 as that user. Unset means no + # key is minted. Sized to the job definition's timeout: the job retires the + # key when it exits, and the expiry covers a job that is killed first. + user_api_key_hours: int | None = Field(default=None, gt=0, le=24 * 7) + + class ActionConfig(SQLModel): """Model for action workflow configuration.""" workflow_id: str | None = None @@ -44,7 +53,7 @@ class ActionConfig(SQLModel): inputs: List[ActionInput] | None = None platforms: Dict[str, PlatformConfig] export_command: str | None = None - aws_batch: AwsBatchConfig | None = None + aws_batch: ActionAwsBatchConfig | None = None class ActionConfigsResponse(SQLModel): diff --git a/api/jobs/services.py b/api/jobs/services.py index 1e333e2f..5b954bf4 100644 --- a/api/jobs/services.py +++ b/api/jobs/services.py @@ -219,6 +219,22 @@ def update_batch_job( return job +# Environment variable names that mark a value as a credential. The values reach +# the container; they are kept out of this service's logs. +_SECRET_NAME_MARKERS = ("TOKEN", "KEY", "SECRET", "PASSWORD") + + +def _loggable_overrides(container_overrides: Dict[str, Any]) -> Dict[str, Any]: + """A copy of container overrides with credential values masked, for logging.""" + environment = [ + {**env, "value": ""} + if any(marker in env.get("name", "").upper() for marker in _SECRET_NAME_MARKERS) + else env + for env in container_overrides.get("environment", []) + ] + return {**container_overrides, "environment": environment} + + def submit_batch_job( session: Session, job_name: str, @@ -264,7 +280,7 @@ def submit_batch_job( container_overrides["environment"].append( {"name": "NGS360_API_ENDPOINT", "value": settings.client_origin} ) - logger.info(f"Container overrides: {container_overrides}") + logger.info(f"Container overrides: {_loggable_overrides(container_overrides)}") try: batch_client = boto3.client("batch", region_name=settings.AWS_REGION) diff --git a/api/project/routes.py b/api/project/routes.py index 44bca910..59a21598 100755 --- a/api/project/routes.py +++ b/api/project/routes.py @@ -626,7 +626,8 @@ def submit_pipeline_job( email=current_user.email, reference=request.reference, auto_release=request.auto_release, - s3_client=s3_client + s3_client=s3_client, + submitter=current_user, ) return BatchJobPublic.model_validate(batch_job) diff --git a/api/project/services.py b/api/project/services.py index 8cefb101..b16a803a 100755 --- a/api/project/services.py +++ b/api/project/services.py @@ -2,7 +2,7 @@ Services for the Project API """ -from datetime import datetime, timezone as tz +from datetime import datetime, timedelta, timezone as tz from typing import Literal import boto3 import logging @@ -21,6 +21,8 @@ from api.actions.services import get_all_action_configs from api.actions.models import ActionOption, ActionPlatform from api.jobs.services import submit_batch_job +from api.auth.models import APIKeyCreate, User +from api.auth.services import create_user_api_key, revoke_user_api_key from core.utils import define_search_body, interpolate @@ -672,7 +674,8 @@ def submit_pipeline_job( email: str | None = None, reference: str | None = None, auto_release: bool | None = None, - s3_client=None + s3_client=None, + submitter: User | None = None, ) -> BatchJob: """ Submit a pipeline job to AWS Batch. @@ -692,6 +695,8 @@ def submit_pipeline_job( reference: Export reference label (required for export action) auto_release: Auto-release flag (only valid for export action) s3_client: Optional boto3 S3 client + submitter: The submitting user, who owns the job's API key when the + config's aws_batch.user_api_key_hours asks for one Returns: BatchJob instance @@ -838,16 +843,50 @@ def submit_pipeline_job( detail=f"Failed to interpolate environment variable '{env.name}': {str(e)}" ) from e + # A job that calls back into NGS360 as the submitter (a pipeline launcher) + # outlives any session token they hold, so it gets an API key of its own. + # The key is the submitter's: every call the job makes is authorized against + # their roles and project memberships, as if they had made it themselves. + api_key = None + if action_config.aws_batch.user_api_key_hours: + if submitter is None: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="An API key was requested for a job with no submitter" + ) + api_key, raw_key = create_user_api_key(session, submitter, APIKeyCreate( + name=f"Batch job {interpolated_job_name}"[:100], + expires_at=datetime.now(tz.utc) + timedelta( + hours=action_config.aws_batch.user_api_key_hours), + )) + container_overrides["environment"].extend([ + {"name": "NGS360_AUTH_TOKEN", "value": raw_key}, + # Lets the job retire its own key when it exits + {"name": "NGS360_AUTH_KEY_ID", "value": str(api_key.id)}, + ]) + # Submit job to AWS Batch - batch_job = submit_batch_job( - session=session, - job_name=interpolated_job_name, - container_overrides=container_overrides, - job_def=action_config.aws_batch.job_definition, - job_queue=action_config.aws_batch.job_queue, - user=username, - project_id=project.project_id - ) + try: + batch_job = submit_batch_job( + session=session, + job_name=interpolated_job_name, + container_overrides=container_overrides, + job_def=action_config.aws_batch.job_definition, + job_queue=action_config.aws_batch.job_queue, + user=username, + project_id=project.project_id + ) + except Exception: + if api_key is not None: + revoke_user_api_key(session, submitter, str(api_key.id)) + raise + + if api_key is not None: + # Renamed for the job's id once there is one, so a key in the user's list + # can be matched to the job that held it. + api_key.name = f"Batch job {batch_job.id}"[:100] + session.add(api_key) + session.commit() return batch_job diff --git a/tests/api/test_jobs.py b/tests/api/test_jobs.py index 926b8b4e..39430fed 100644 --- a/tests/api/test_jobs.py +++ b/tests/api/test_jobs.py @@ -1360,3 +1360,26 @@ def test_submit_batch_job_injects_alongside_existing_env( env_dict = {e["name"]: e["value"] for e in env} assert env_dict["MY_VAR"] == "my_value" assert env_dict["NGS360_API_ENDPOINT"] == get_settings().client_origin + + +def test_loggable_overrides_masks_credentials(): + """Test credential values are masked in the logged overrides, and only those""" + from api.jobs.services import _loggable_overrides + + overrides = { + "command": ["launch"], + "environment": [ + {"name": "NGS360_AUTH_TOKEN", "value": "ngs360_secret"}, + {"name": "PROJECT_ID", "value": "P-1"}, + ], + } + + logged = _loggable_overrides(overrides) + + assert logged["environment"] == [ + {"name": "NGS360_AUTH_TOKEN", "value": ""}, + {"name": "PROJECT_ID", "value": "P-1"}, + ] + assert logged["command"] == ["launch"] + # The overrides sent to AWS Batch are untouched + assert overrides["environment"][0]["value"] == "ngs360_secret" diff --git a/tests/api/test_projects.py b/tests/api/test_projects.py index dd5fd844..8567568d 100644 --- a/tests/api/test_projects.py +++ b/tests/api/test_projects.py @@ -3,9 +3,13 @@ """ +from datetime import datetime, timedelta, timezone from unittest.mock import patch, MagicMock +import uuid + +import botocore.exceptions from fastapi.testclient import TestClient -from sqlmodel import Session +from sqlmodel import Session, select import yaml from api.project.models import Project, ProjectAttribute @@ -915,6 +919,135 @@ def test_submit_create_project_job( assert call_args["jobDefinition"] == "pipeline-job-def:1" +def _submit_rna_seq_job(client, project, mock_s3_client, aws_batch_extra=None): + """Submit a create-project job against a minimal RNA-Seq action config""" + pipeline_config = { + "project_type": "RNA-Seq", + "project_admins": ["admin@example.com"], + "platforms": { + "Arvados": {"create_project_command": "launch {{projectid}}"} + }, + "aws_batch": { + "job_name": "launch-{{projectid}}", + "job_definition": "launcher-job-def:1", + "job_queue": "batch-queue", + "command": "launch", + **(aws_batch_extra or {}), + } + } + mock_s3_client.setup_bucket( + "ngs360-resources", "pipeline_configs/", + [{"Key": "pipeline_configs/rna-seq_pipeline.yaml"}], [] + ) + mock_s3_client.uploaded_files["ngs360-resources"] = { + "pipeline_configs/rna-seq_pipeline.yaml": yaml.dump(pipeline_config).encode("utf-8") + } + return client.post( + f"/api/v1/projects/{project.project_id}/actions/submit", + json={"action": "create-project", "platform": "Arvados", "project_type": "RNA-Seq"} + ) + + +def _batch_environment(mock_batch: MagicMock) -> dict: + """The environment a mocked submit_job was called with, as a dict""" + overrides = mock_batch.submit_job.call_args[1]["containerOverrides"] + return {env["name"]: env["value"] for env in overrides["environment"]} + + +@patch("api.jobs.services.boto3.client") +@patch("api.actions.services.get_setting_value") +def test_submit_job_mints_submitter_api_key( + mock_get_setting: MagicMock, + mock_boto_client: MagicMock, + client: TestClient, + session: Session, + test_project: Project, + mock_s3_client +): + """Test a config asking for a key gets one owned by the submitter, named for the job""" + from api.auth.models import APIKey, User + from core.security import hash_api_key + + mock_get_setting.return_value = "s3://ngs360-resources/pipeline_configs/" + mock_batch = MagicMock() + mock_batch.submit_job.return_value = {"jobId": "aws-batch-job-123"} + mock_boto_client.return_value = mock_batch + + before = datetime.now(timezone.utc) + response = _submit_rna_seq_job( + client, test_project, mock_s3_client, {"user_api_key_hours": 72} + ) + + assert response.status_code == 201 + environment = _batch_environment(mock_batch) + api_key = session.get(APIKey, uuid.UUID(environment["NGS360_AUTH_KEY_ID"])) + assert api_key.hashed_key == hash_api_key(environment["NGS360_AUTH_TOKEN"]) + assert session.get(User, api_key.user_id).username == "testuser" + assert api_key.is_active + assert api_key.name == "Batch job aws-batch-job-123" + expires_at = api_key.expires_at.replace(tzinfo=timezone.utc) + ttl = timedelta(hours=72) + assert before + ttl <= expires_at <= datetime.now(timezone.utc) + ttl + + +@patch("api.jobs.services.boto3.client") +@patch("api.actions.services.get_setting_value") +def test_submit_job_without_key_config_mints_nothing( + mock_get_setting: MagicMock, + mock_boto_client: MagicMock, + client: TestClient, + session: Session, + test_project: Project, + mock_s3_client +): + """Test jobs whose config does not ask for a key get no credential""" + from api.auth.models import APIKey + + mock_get_setting.return_value = "s3://ngs360-resources/pipeline_configs/" + mock_batch = MagicMock() + mock_batch.submit_job.return_value = {"jobId": "aws-batch-job-123"} + mock_boto_client.return_value = mock_batch + + response = _submit_rna_seq_job(client, test_project, mock_s3_client) + + assert response.status_code == 201 + environment = _batch_environment(mock_batch) + assert "NGS360_AUTH_TOKEN" not in environment + assert "NGS360_AUTH_KEY_ID" not in environment + assert session.exec(select(APIKey)).all() == [] + + +@patch("api.jobs.services.boto3.client") +@patch("api.actions.services.get_setting_value") +def test_submit_job_failure_revokes_minted_key( + mock_get_setting: MagicMock, + mock_boto_client: MagicMock, + client: TestClient, + session: Session, + test_project: Project, + mock_s3_client +): + """Test a key minted for a job AWS Batch refused does not stay usable""" + from api.auth.models import APIKey + + mock_get_setting.return_value = "s3://ngs360-resources/pipeline_configs/" + mock_batch = MagicMock() + mock_batch.submit_job.side_effect = botocore.exceptions.ClientError( + {"Error": {"Code": "ClientException", "Message": "no such queue"}}, "SubmitJob" + ) + mock_boto_client.return_value = mock_batch + + response = _submit_rna_seq_job( + client, test_project, mock_s3_client, {"user_api_key_hours": 72} + ) + + assert response.status_code == 500 + (api_key,) = session.exec(select(APIKey)).all() + session.refresh(api_key) + assert not api_key.is_active + assert api_key.revoked_at is not None + + @patch("api.jobs.services.boto3.client") @patch("api.actions.services.get_setting_value") def test_submit_export_results_job(