From 4e7ad82e8af97d9d8a1e829b4b2935343bdbd3bc Mon Sep 17 00:00:00 2001 From: Sky Rubenstein Date: Thu, 6 Jun 2024 16:47:37 -0400 Subject: [PATCH] Move stuff into the flight --- .../controller/SnapshotsApiController.java | 14 ++------ .../service/snapshot/SnapshotService.java | 19 ----------- .../create/AuthorizeSourceDatasetUseStep.java | 33 +++++++++++++++++++ .../flight/create/SnapshotCreateFlight.java | 22 ++++++++++++- .../flight/create/VerifyDuosDatasetStep.java | 33 +++++++++++++++++++ 5 files changed, 90 insertions(+), 31 deletions(-) create mode 100644 src/main/java/bio/terra/service/snapshot/flight/create/AuthorizeSourceDatasetUseStep.java create mode 100644 src/main/java/bio/terra/service/snapshot/flight/create/VerifyDuosDatasetStep.java diff --git a/src/main/java/bio/terra/app/controller/SnapshotsApiController.java b/src/main/java/bio/terra/app/controller/SnapshotsApiController.java index 6ca04f275d..9bd457815e 100644 --- a/src/main/java/bio/terra/app/controller/SnapshotsApiController.java +++ b/src/main/java/bio/terra/app/controller/SnapshotsApiController.java @@ -137,17 +137,9 @@ private List getUnauthorizedSources( public ResponseEntity createSnapshot( @Valid @RequestBody SnapshotRequestModel snapshotRequestModel) { AuthenticatedUserRequest userReq = getAuthenticatedInfo(); - List snapshotSourceDatasetIds = - snapshotService.getSourceDatasetIdsFromSnapshotRequest(snapshotRequestModel); - // TODO: auth should be put into flight - List unauthorized = getUnauthorizedSources(snapshotSourceDatasetIds, userReq); - if (unauthorized.isEmpty()) { - String jobId = snapshotService.createSnapshot(snapshotRequestModel, userReq); - // we can retrieve the job we just created - return jobToResponse(jobService.retrieveJob(jobId, userReq)); - } - throw new IamForbiddenException( - "User is not authorized to create snapshots for these datasets " + unauthorized); + String jobId = snapshotService.createSnapshot(snapshotRequestModel, userReq); + // we can retrieve the job we just created + return jobToResponse(jobService.retrieveJob(jobId, userReq)); } @Override diff --git a/src/main/java/bio/terra/service/snapshot/SnapshotService.java b/src/main/java/bio/terra/service/snapshot/SnapshotService.java index 51776d1b7a..252b5b0597 100644 --- a/src/main/java/bio/terra/service/snapshot/SnapshotService.java +++ b/src/main/java/bio/terra/service/snapshot/SnapshotService.java @@ -172,22 +172,6 @@ public SnapshotService( */ public String createSnapshot( SnapshotRequestModel snapshotRequestModel, AuthenticatedUserRequest userReq) { - String sourceDatasetName = snapshotRequestModel.getContents().get(0).getDatasetName(); - Dataset dataset = datasetService.retrieveByName(sourceDatasetName); - if (snapshotRequestModel.getProfileId() == null) { - snapshotRequestModel.setProfileId(dataset.getDefaultProfileId()); - logger.warn( - "Enriching {} snapshot {} request with dataset default profileId {}", - userReq.getEmail(), - snapshotRequestModel.getName(), - dataset.getDefaultProfileId()); - } - String duosId = snapshotRequestModel.getDuosId(); - if (duosId != null) { - // We fetch the DUOS dataset to confirm its existence, but do not need the returned value. - duosClient.getDataset(duosId, userReq); - } - UUID snapshotId = UUID.randomUUID(); String description = "Create snapshot %s with ID %s".formatted(snapshotRequestModel.getName(), snapshotId); @@ -195,9 +179,6 @@ public String createSnapshot( return jobService .newJob(description, SnapshotCreateFlight.class, snapshotRequestModel, userReq) .addParameter(CommonMapKeys.CREATED_AT, Instant.now().toEpochMilli()) - .addParameter(JobMapKeys.IAM_RESOURCE_TYPE.getKeyName(), IamResourceType.DATASET) - .addParameter(JobMapKeys.IAM_RESOURCE_ID.getKeyName(), dataset.getId()) - .addParameter(JobMapKeys.IAM_ACTION.getKeyName(), IamAction.LINK_SNAPSHOT) .addParameter(JobMapKeys.SNAPSHOT_ID.getKeyName(), snapshotId.toString()) .submit(); } diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/AuthorizeSourceDatasetUseStep.java b/src/main/java/bio/terra/service/snapshot/flight/create/AuthorizeSourceDatasetUseStep.java new file mode 100644 index 0000000000..ad72ec7693 --- /dev/null +++ b/src/main/java/bio/terra/service/snapshot/flight/create/AuthorizeSourceDatasetUseStep.java @@ -0,0 +1,33 @@ +package bio.terra.service.snapshot.flight.create; + +import bio.terra.common.iam.AuthenticatedUserRequest; +import bio.terra.model.BillingProfileModel; +import bio.terra.service.auth.iam.IamAction; +import bio.terra.service.auth.iam.IamResourceType; +import bio.terra.service.auth.iam.IamService; +import bio.terra.service.job.DefaultUndoStep; +import bio.terra.service.profile.ProfileService; +import bio.terra.service.profile.flight.ProfileMapKeys; +import bio.terra.service.snapshot.SnapshotService; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.FlightMap; +import bio.terra.stairway.StepResult; +import java.util.UUID; + +public class AuthorizeSourceDatasetUseStep extends DefaultUndoStep { + private final IamService iamService; + private final UUID datasetId; + private final AuthenticatedUserRequest user; + + public AuthorizeSourceDatasetUseStep(IamService iamService, UUID datasetId, AuthenticatedUserRequest user) { + this.iamService = iamService; + this.datasetId = datasetId; + this.user = user; + } + + @Override + public StepResult doStep(FlightContext context) { + iamService.verifyAuthorization(user, IamResourceType.DATASET, datasetId.toString(), IamAction.LINK_SNAPSHOT); + return StepResult.getStepResultSuccess(); + } +} diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java b/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java index 797129291e..5a948609c3 100644 --- a/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java +++ b/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java @@ -17,6 +17,7 @@ import bio.terra.service.dataset.DatasetService; import bio.terra.service.dataset.flight.LockDatasetStep; import bio.terra.service.dataset.flight.UnlockDatasetStep; +import bio.terra.service.duos.DuosClient; import bio.terra.service.duos.DuosDao; import bio.terra.service.duos.DuosService; import bio.terra.service.filedata.DrsIdService; @@ -63,9 +64,12 @@ import java.util.Objects; import java.util.UUID; import java.util.concurrent.TimeUnit; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; public class SnapshotCreateFlight extends Flight { + private static final Logger logger = LoggerFactory.getLogger(SnapshotCreateFlight.class); public SnapshotCreateFlight(FlightMap inputParameters, Object applicationContext) { super(inputParameters, applicationContext); @@ -109,6 +113,7 @@ public SnapshotCreateFlight(FlightMap inputParameters, Object applicationContext SnapshotBuilderService snapshotBuilderService = appContext.getBean(SnapshotBuilderService.class); IamService iamService = appContext.getBean(IamService.class); + DuosClient duosClient = appContext.getBean(DuosClient.class); SnapshotRequestModel snapshotReq = inputParameters.get(JobMapKeys.REQUEST.getKeyName(), SnapshotRequestModel.class); @@ -134,6 +139,22 @@ public SnapshotCreateFlight(FlightMap inputParameters, Object applicationContext var platform = CloudPlatformWrapper.of(sourceDataset.getDatasetSummary().getStorageCloudPlatform()); + addStep(new AuthorizeSourceDatasetUseStep(iamService, datasetId, userReq)); + + if (snapshotReq.getProfileId() == null) { + snapshotReq.setProfileId(sourceDataset.getDefaultProfileId()); + logger.warn( + "Enriching {} snapshot {} request with dataset default profileId {}", + userReq.getEmail(), + snapshotReq.getName(), + sourceDataset.getDefaultProfileId()); + } + + String duosId = snapshotReq.getDuosId(); + if (duosId != null) { + addStep(new VerifyDuosDatasetStep(duosClient, duosId, userReq)); + } + // Take out a shared lock on the source dataset, to guard against it being deleted out from // under us (for example) addStep(new LockDatasetStep(datasetService, datasetId, true), randomBackoffRetry); @@ -161,7 +182,6 @@ public SnapshotCreateFlight(FlightMap inputParameters, Object applicationContext } // if DUOS id in request, get/create firecloud group (needed for CreateSnapshotMetadataStep) - String duosId = snapshotReq.getDuosId(); if (duosId != null) { addStep(new RetrieveDuosFirecloudGroupStep(duosDao, duosId)); addStep( diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/VerifyDuosDatasetStep.java b/src/main/java/bio/terra/service/snapshot/flight/create/VerifyDuosDatasetStep.java new file mode 100644 index 0000000000..56f7a3d828 --- /dev/null +++ b/src/main/java/bio/terra/service/snapshot/flight/create/VerifyDuosDatasetStep.java @@ -0,0 +1,33 @@ +package bio.terra.service.snapshot.flight.create; + +import bio.terra.common.iam.AuthenticatedUserRequest; +import bio.terra.model.BillingProfileModel; +import bio.terra.service.auth.iam.IamService; +import bio.terra.service.duos.DuosClient; +import bio.terra.service.job.DefaultUndoStep; +import bio.terra.service.profile.flight.ProfileMapKeys; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.FlightMap; +import bio.terra.stairway.StepResult; +import java.util.UUID; + +public class VerifyDuosDatasetStep extends DefaultUndoStep { + private final DuosClient duosClient; + private final String duosId; + private final AuthenticatedUserRequest user; + + public VerifyDuosDatasetStep(DuosClient duosClient, String duosId, AuthenticatedUserRequest user) { + this.duosClient = duosClient; + this.duosId = duosId; + this.user = user; + } + @Override + public StepResult doStep(FlightContext context) { + if (duosId != null) { + // We fetch the DUOS dataset to confirm its existence, but do not need the returned value. + duosClient.getDataset(duosId, user); + } + return StepResult.getStepResultSuccess(); + } + +}