doge refactor

broadinstitute · Jun 17, 2024 · 001523d · 001523d
1 parent 2083091
commit 001523d
Show file tree

Hide file tree

Showing 11 changed files with 154 additions and 140 deletions.
diff --git a/src/main/resources/sam.conf b/src/main/resources/sam.conf
@@ -183,48 +183,56 @@ admin {
 
 azureServices {
   azureEnabled = ${?AZURE_ENABLED}
-  azureServiceCatalogAppsEnabled = false
   allowManagedIdentityUserCreation = ${?AZURE_ALLOW_MANAGED_IDENTITY_USER_CREATION}
   managedAppClientId = ${?AZURE_MANAGED_APP_CLIENT_ID}
   managedAppClientSecret = ${?AZURE_MANAGED_APP_CLIENT_SECRET}
   managedAppTenantId = ${?AZURE_MANAGED_APP_TENANT_ID}
-  authorizedUserKey = "authorizedTerraUser";
-  managedAppTypeServiceCatalog = "ServiceCatalog";
   managedAppWorkloadClientId = ${?AZURE_MANAGED_APP_WORKLOAD_CLIENT_ID}
-  managedAppPlans = [
-    {
+
+  azureServiceCatalog {
+    enabled = ${?AZURE_SERVICE_CATALOG_ENABLED} # defaults to false
+    authorizedUserKey = "authorizedTerraUser";
+    managedAppTypeServiceCatalog = "ServiceCatalog";
+  }
+
+  azureMarketPlace {
+    enabled = ${?AZURE_MARKET_PLACE_ENABLED} # defaults to true
+    managedAppPlans = [
+      {
         name = "terra-prod"
         publisher = "thebroadinstituteinc1615909626976"
         authorizedUserKey = authorizedTerraUser
-    }
-    {
+      }
+      {
         name = "terra-dev"
         publisher = "thebroadinstituteinc1615909626976"
         authorizedUserKey = authorizedTerraUser
-    }
-    {
+      }
+      {
         name = "terra-workspace-dev-plan"
         publisher = "thebroadinstituteinc1615909626976"
         authorizedUserKey = authorizedTerraUser
-    }
-    {
+      }
+      {
         name = "terra-aster-prod"
         publisher = "thebroadinstituteinc1615909626976"
         authorizedUserKey = "authorizedTerraUser"
-    }
-    {
+      }
+      {
         name = "tdr-dev"
         publisher = "thebroadinstituteinc1615909626976"
         authorizedUserKey = "authorizedTDRUser"
-    }
-    {
+      }
+      {
         name = "tdr-prod"
         publisher = "thebroadinstituteinc1615909626976"
         authorizedUserKey = "authorizedTDRUser"
-    }
-  ]
+      }
+    ]
+  }
 }
 
+
 janitor {
   enabled = ${?JANITOR_ENABLED}
   clientCredentialFilePath = ${?JANITOR_CLIENT_CREDENTIAL_FILE_PATH}

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala
@@ -3,7 +3,6 @@ package org.broadinstitute.dsde.workbench.sam.azure
 import akka.http.scaladsl.model.StatusCodes
 import bio.terra.cloudres.azure.resourcemanager.common.Defaults
 import bio.terra.cloudres.azure.resourcemanager.msi.data.CreateUserAssignedManagedIdentityRequestData
-import cats.data.OptionT
 import cats.effect.IO
 import cats.implicits.toTraverseOps
 import com.azure.core.management.Region
@@ -13,13 +12,12 @@ import com.azure.resourcemanager.resources.ResourceManager
 import com.azure.resourcemanager.resources.models.ResourceGroup
 import org.broadinstitute.dsde.workbench.model.{ErrorReport, WorkbenchEmail, WorkbenchException, WorkbenchExceptionWithErrorReport, WorkbenchUserId}
 import org.broadinstitute.dsde.workbench.sam._
-import org.broadinstitute.dsde.workbench.sam.config.ManagedAppPlan
+import org.broadinstitute.dsde.workbench.sam.config.{AzureMarketPlace, AzureServiceCatalog, AzureServicesConfig, ManagedAppPlan}
 import org.broadinstitute.dsde.workbench.sam.dataAccess.{AzureManagedResourceGroupDAO, DirectoryDAO}
 import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, ResourceAction}
 import org.broadinstitute.dsde.workbench.sam.model.api.SamUser
 import org.broadinstitute.dsde.workbench.sam.util.OpenTelemetryIOUtils._
 import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
-import org.broadinstitute.dsde.workbench.sam.config.AzureServicesConfig
 
 import scala.jdk.CollectionConverters._
 
@@ -46,12 +44,7 @@ class AzureService(
 
   def createManagedResourceGroup(managedResourceGroup: ManagedResourceGroup, samRequestContext: SamRequestContext): IO[Unit] =
     for {
-      _ <-
-        if (config.azureServiceCatalogAppsEnabled) {
-          validateServiceCatalogManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
-        } else {
-          validateManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
-        }
+      _ <- validateManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
 
       existingByCoords <- azureManagedResourceGroupDAO.getManagedResourceGroupByCoordinates(
         managedResourceGroup.managedResourceGroupCoordinates,
@@ -235,31 +228,41 @@ class AzureService(
   /** Resolves a managed resource group in Azure and returns the terra.billingProfileId tag value. This is used for access control checks during route handling.
     */
   def getBillingProfileId(request: GetOrCreatePetManagedIdentityRequest, samRequestContext: SamRequestContext): IO[Option[BillingProfileId]] =
-    // get the billing profile id from the database
-    // if not there, for backwards compatibility, get the billing profile id from a tag on the Azure resource
-    OptionT(getBillingProfileIdFromSamDb(request, samRequestContext))
-      .orElseF(getBillingProfileIdFromAzureTag(request, samRequestContext))
-      .value
+    getBillingProfileIdFromSamDb(request, samRequestContext)
 
   private def getBillingProfileIdFromSamDb(request: GetOrCreatePetManagedIdentityRequest, samRequestContext: SamRequestContext): IO[Option[BillingProfileId]] =
     for {
       maybeMrg <- azureManagedResourceGroupDAO.getManagedResourceGroupByCoordinates(request.toManagedResourceGroupCoordinates, samRequestContext)
     } yield maybeMrg.map(_.billingProfileId)
 
-  private def getBillingProfileIdFromAzureTag(
-      request: GetOrCreatePetManagedIdentityRequest,
-      samRequestContext: SamRequestContext
-  ): IO[Option[BillingProfileId]] = traceIOWithContext("getBillingProfileIdFromAzureTag", samRequestContext) { _ =>
+  private def validateManagedResourceGroup(
+      mrgCoords: ManagedResourceGroupCoordinates,
+      samRequestContext: SamRequestContext,
+      validateUser: Boolean = true
+  ): IO[Unit] =
     for {
-      mrg <- validateManagedResourceGroup(request.toManagedResourceGroupCoordinates, samRequestContext, false)
-    } yield getBillingProfileFromTag(mrg)
-  }
+      _ <- IO.raiseWhen(config.azureServiceCatalog.isEmpty && config.azureMarketPlace.isEmpty)(
+        new WorkbenchException("Either azure service catalog or azure market place must be configured")
+      )
+      _ <- config.azureServiceCatalog
+        .map { serviceCatalog =>
+          validateServiceCatalogManagedResourceGroup(serviceCatalog, mrgCoords, samRequestContext, validateUser)
+        }
+        .getOrElse(IO.unit)
+
+      _ <- config.azureMarketPlace
+        .map { marketPlace =>
+          validateMarketPlaceManagedResourceGroup(marketPlace, mrgCoords, samRequestContext, validateUser)
+        }
+        .getOrElse(IO.unit)
+    } yield ()
 
   /** Validates a managed resource group. Algorithm:
     *   1. Resolve the MRG in Azure 2. Get the managed app id from the MRG 3. Resolve the managed app 4. Get the managed app "plan" name and publisher 5.
     *      Validate the plan name and publisher matches a configured value 6. Validate that the caller is on the list of authorized users for the app
     */
-  private def validateManagedResourceGroup(
+  private def validateMarketPlaceManagedResourceGroup(
+      marketPlace: AzureMarketPlace,
       mrgCoords: ManagedResourceGroupCoordinates,
       samRequestContext: SamRequestContext,
       validateUser: Boolean = true
@@ -271,7 +274,7 @@ class AzureService(
         appManager <- crlService.buildApplicationManager(mrgCoords.tenantId, mrgCoords.subscriptionId)
         appsInSubscription <- IO(appManager.applications().list().asScala.toSeq)
         managedApp <- IO.fromOption(appsInSubscription.find(_.managedResourceGroupId() == mrg.id()))(managedAppValidationFailure)
-        plan <- validatePlan(managedApp, crlService.getManagedAppPlans)
+        plan <- validatePlan(managedApp, marketPlace.managedAppPlans)
         _ <- if (validateUser) validateAuthorizedAppUser(managedApp, plan.authorizedUserKey, samRequestContext) else IO.unit
       } yield mrg
     }
@@ -281,6 +284,7 @@ class AzureService(
     *      that the caller is on the list of authorized users for the app
     */
   private def validateServiceCatalogManagedResourceGroup(
+      serviceCatalog: AzureServiceCatalog,
       mrgCoords: ManagedResourceGroupCoordinates,
       samRequestContext: SamRequestContext,
       validateUser: Boolean = true
@@ -293,10 +297,10 @@ class AzureService(
         appsInSubscription <- IO(appManager.applications().list().asScala)
         managedApp <- IO.fromOption(appsInSubscription.find(_.managedResourceGroupId() == mrg.id()))(managedAppValidationFailure)
         _ <-
-          if (managedApp.kind() == config.managedAppTypeServiceCatalog && validateUser) {
+          if (managedApp.kind() == serviceCatalog.managedAppTypeServiceCatalog && validateUser) {
             validateAuthorizedAppUser(
               managedApp,
-              config.authorizedUserKey,
+              serviceCatalog.authorizedUserKey,
               samRequestContext
             )
           } else IO.unit

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala
@@ -12,7 +12,7 @@ import com.azure.resourcemanager.managedapplications.ApplicationManager
 import com.azure.resourcemanager.msi.MsiManager
 import com.azure.resourcemanager.resources.ResourceManager
 import com.google.auth.oauth2.ServiceAccountCredentials
-import org.broadinstitute.dsde.workbench.sam.config.{AzureServicesConfig, JanitorConfig, ManagedAppPlan}
+import org.broadinstitute.dsde.workbench.sam.config.{AzureServicesConfig, JanitorConfig}
 
 import java.io.FileInputStream
 import scala.concurrent.duration._
@@ -58,8 +58,6 @@ class CrlService(config: AzureServicesConfig, janitorConfig: JanitorConfig) {
     IO(ApplicationManager.authenticate(credential, profile))
   }
 
-  def getManagedAppPlans: Seq[ManagedAppPlan] = config.managedAppPlans
-
   private def getCredentialAndProfile(tenantId: TenantId, subscriptionId: SubscriptionId): (TokenCredential, AzureProfile) = {
 
     // temp change to test how create-bee-workflow handles failure on MI auth and fall through to SP auth

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AppConfig.scala
@@ -167,21 +167,37 @@ object AppConfig {
     ManagedAppPlan(config.getString("name"), config.getString("publisher"), config.getString("authorizedUserKey"))
   }
 
+  implicit val azureMarketPlaceReader: ValueReader[Option[AzureMarketPlace]] = ValueReader.relative { config =>
+    // enabled by default
+    if (config.as[Option[Boolean]]("enabled").getOrElse(true)) {
+      Option(AzureMarketPlace(config.as[Seq[ManagedAppPlan]]("managedAppPlans")))
+    } else {
+      None
+    }
+  }
+
+  implicit val azureServiceCatalogReader: ValueReader[Option[AzureServiceCatalog]] = ValueReader.relative { config =>
+    // disabled by default
+    if (config.as[Option[Boolean]]("enabled").getOrElse(false)) {
+      Option(AzureServiceCatalog(config.getString("authorizedUserKey"), config.getString("managedAppTypeServiceCatalog")))
+    } else {
+      None
+    }
+  }
+
   implicit val azureServicesConfigReader: ValueReader[Option[AzureServicesConfig]] = ValueReader.relative { config =>
     config
       .getAs[Boolean]("azureEnabled")
       .flatMap(azureEnabled =>
         if (azureEnabled) {
           Option(
             AzureServicesConfig(
-              config.as[Option[Boolean]]("azureServiceCatalogAppsEnabled").getOrElse(false),
-              config.getString("authorizedUserKey"),
-              config.getString("managedAppTypeServiceCatalog"),
               config.getString("managedAppWorkloadClientId"),
               config.getString("managedAppClientId"),
               config.getString("managedAppClientSecret"),
               config.getString("managedAppTenantId"),
-              config.as[Seq[ManagedAppPlan]]("managedAppPlans"),
+              config.as[Option[AzureMarketPlace]]("azureMarketPlace"),
+              config.as[Option[AzureServiceCatalog]]("azureServiceCatalog"),
               config.as[Option[Boolean]]("allowManagedIdentityUserCreation").getOrElse(false)
             )
           )

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AzureServicesConfig.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/config/AzureServicesConfig.scala
@@ -1,14 +1,14 @@
 package org.broadinstitute.dsde.workbench.sam.config
 
 case class ManagedAppPlan(name: String, publisher: String, authorizedUserKey: String)
+case class AzureMarketPlace(managedAppPlans: Seq[ManagedAppPlan])
+case class AzureServiceCatalog(authorizedUserKey: String, managedAppTypeServiceCatalog: String)
 case class AzureServicesConfig(
-    azureServiceCatalogAppsEnabled: Boolean,
-    authorizedUserKey: String,
-    managedAppTypeServiceCatalog: String,
     managedAppWorkloadClientId: String,
     managedAppClientId: String,
     managedAppClientSecret: String,
     managedAppTenantId: String,
-    managedAppPlans: Seq[ManagedAppPlan],
+    azureMarketPlace: Option[AzureMarketPlace],
+    azureServiceCatalog: Option[AzureServiceCatalog],
     allowManagedIdentityUserCreation: Boolean
 ) {}
diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/TestSamRoutes.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/TestSamRoutes.scala
@@ -14,7 +14,14 @@ import org.broadinstitute.dsde.workbench.oauth2.mock.FakeOpenIDConnectConfigurat
 import org.broadinstitute.dsde.workbench.sam.TestSupport.samRequestContext
 import org.broadinstitute.dsde.workbench.sam.azure.{AzureService, CrlService, MockCrlService}
 import org.broadinstitute.dsde.workbench.sam.config.AppConfig.AdminConfig
-import org.broadinstitute.dsde.workbench.sam.config.{AppConfig, AzureServicesConfig, LiquibaseConfig, ManagedAppPlan, TermsOfServiceConfig}
+import org.broadinstitute.dsde.workbench.sam.config.{
+  AppConfig,
+  AzureMarketPlace,
+  AzureServiceCatalog,
+  AzureServicesConfig,
+  LiquibaseConfig,
+  TermsOfServiceConfig
+}
 import org.broadinstitute.dsde.workbench.sam.dataAccess._
 import org.broadinstitute.dsde.workbench.sam.model.SamResourceActions.{adminAddMember, adminReadPolicies, adminRemoveMember}
 import org.broadinstitute.dsde.workbench.sam.model._
@@ -169,7 +176,9 @@ object TestSamRoutes {
       cloudExtensions: Option[CloudExtensions] = None,
       adminEmailDomains: Option[Set[String]] = None,
       crlService: Option[CrlService] = None,
-      acceptTermsOfService: Boolean = true
+      acceptTermsOfService: Boolean = true,
+      azureMarketPlace: Option[AzureMarketPlace] = None,
+      azureServiceCatalog: Option[AzureServiceCatalog] = None
   )(implicit system: ActorSystem, materializer: Materializer, executionContext: ExecutionContext) = {
     val dbRef = TestSupport.dbRef
     val resourceTypesWithAdmin = resourceTypes + (resourceTypeAdmin.name -> resourceTypeAdmin)
@@ -209,16 +218,13 @@ object TestSamRoutes {
     mockResourceService.initResourceTypes(samRequestContext).unsafeRunSync()
 
     val mockStatusService = new StatusService(directoryDAO, cloudXtns)
-    val defaultManagedAppPlan: ManagedAppPlan = ManagedAppPlan("mock-plan", "mock-publisher", "mock-auth-user-key")
     val mockAzureServicesConfig = AzureServicesConfig(
-      azureServiceCatalogAppsEnabled = false,
-      "mock-auth-user-key",
-      "mock-kind",
       "mock-managedapp-workload-clientid",
       "mock-managedapp-clientid",
       "mock-managedapp-clientsecret",
       "mock-managedapp-tenantid",
-      Seq(defaultManagedAppPlan),
+      azureMarketPlace,
+      azureServiceCatalog,
       allowManagedIdentityUserCreation = true
     )