Testing create-bee failures with azureconfig update

broadinstitute · Jun 6, 2024 · 51e2828 · 51e2828
1 parent c294d82
commit 51e2828
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 11 deletions.
diff --git a/env/local.env b/env/local.env
@@ -5,6 +5,7 @@ export ADMIN_SERVICE_ACCOUNT_4="src/main/resources/rendered/admin-service-accoun
 export ADMIN_SERVICE_ACCOUNT_5="src/main/resources/rendered/admin-service-account-5.json"
 export SERVICE_ACCOUNT_ADMINS="[email protected], [email protected]"
 export AZURE_ENABLED="false"
+export AZURE_SERVICE_CATALOG_APPS_ENABLED="false"
 export EMAIL_DOMAIN="dev.test.firecloud.org"
 export ENVIRONMENT="dev"
 export GOOGLE_APPS_DOMAIN="test.firecloud.org"

diff --git a/env/test.env b/env/test.env
@@ -4,6 +4,9 @@ export CREATE_BEE_WORKFLOW_TEST="true"
 export AZURE_MANAGED_APP_CLIENT_ID="foo"
 export AZURE_MANAGED_APP_CLIENT_SECRET="foo"
 export AZURE_MANAGED_APP_TENANT_ID="foo"
+export AZURE_MANAGED_APP_WORKLOAD_CLIENT_ID="foo"
+export AZURE_ALLOW_MANAGED_IDENTITY_USER_CREATION="false"
+export AZURE_SERVICE_CATALOG_APPS_ENABLED="false"
 export EMAIL_DOMAIN="dev.test.firecloud.org"
 export ENVIRONMENT="local"
 export GOOGLE_APPS_DOMAIN="test.firecloud.org"

diff --git a/src/main/resources/sam.conf b/src/main/resources/sam.conf
@@ -187,6 +187,9 @@ azureServices {
   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 = [
     {
         name = "terra-prod"

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
@@ -46,7 +46,12 @@ class AzureService(
 
   def createManagedResourceGroup(managedResourceGroup: ManagedResourceGroup, samRequestContext: SamRequestContext): IO[Unit] =
     for {
-      _ <- validateManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
+      _ <-
+        if (config.azureServiceCatalogAppsEnabled) {
+          validateServiceCatalogManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
+        } else {
+          validateManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
+        }
 
       existingByCoords <- azureManagedResourceGroupDAO.getManagedResourceGroupByCoordinates(
         managedResourceGroup.managedResourceGroupCoordinates,
@@ -267,22 +272,49 @@ class AzureService(
         appsInSubscription <- IO(appManager.applications().list().asScala.toSeq)
         managedApp <- IO.fromOption(appsInSubscription.find(_.managedResourceGroupId() == mrg.id()))(managedAppValidationFailure)
         plan <- validatePlan(managedApp, crlService.getManagedAppPlans)
-        _ <- if (validateUser) validateAuthorizedAppUser(managedApp, plan, samRequestContext) else IO.unit
+        _ <- if (validateUser) validateAuthorizedAppUser(managedApp, plan.authorizedUserKey, samRequestContext) else IO.unit
+      } yield mrg
+    }
+
+  /** Validates a managed resource group deployed from Azure Service Catalog. Service Catalog apps do not contain a "plan" or publisher. Algorithm:
+    *   1. Resolve the MRG in Azure 2. Get the managed app id from the MRG 3. Resolve the managed app 4. Validate the app kind is "ServiceCatalog" 5. Validate
+    *      that the caller is on the list of authorized users for the app
+    */
+  private def validateServiceCatalogManagedResourceGroup(
+      mrgCoords: ManagedResourceGroupCoordinates,
+      samRequestContext: SamRequestContext,
+      validateUser: Boolean = true
+  ): IO[ResourceGroup] =
+    traceIOWithContext("validateServiceCatalogManagedResourceGroup", samRequestContext) { _ =>
+      for {
+        resourceManager <- crlService.buildResourceManager(mrgCoords.tenantId, mrgCoords.subscriptionId)
+        mrg <- lookupMrg(mrgCoords, resourceManager)
+        appManager <- crlService.buildApplicationManager(mrgCoords.tenantId, mrgCoords.subscriptionId)
+        appsInSubscription <- IO(appManager.applications().list().asScala)
+        managedApp <- IO.fromOption(appsInSubscription.find(_.managedResourceGroupId() == mrg.id()))(managedAppValidationFailure)
+        _ <-
+          if (managedApp.kind() == config.managedAppTypeServiceCatalog && validateUser) {
+            validateAuthorizedAppUser(
+              managedApp,
+              config.authorizedUserKey,
+              samRequestContext
+            )
+          } else IO.unit
       } yield mrg
     }
 
   /** The users authorized to setup a managed application are stored as a comma separated list of email addresses in the parameters of the application. The
     * azure api is java so this code needs to deal with possible nulls and java Maps. Also the application parameters are untyped, fun.
     * @param app
-    * @param plan
+    * @param authorizedUserKey
     * @param samRequestContext
     * @return
     */
-  private def validateAuthorizedAppUser(app: Application, plan: ManagedAppPlan, samRequestContext: SamRequestContext): IO[Unit] = {
+  private def validateAuthorizedAppUser(app: Application, authorizedUserKey: String, samRequestContext: SamRequestContext): IO[Unit] = {
     val authorizedUsersValue = for {
       parametersObj <- Option(app.parameters()) if parametersObj.isInstanceOf[java.util.Map[_, _]]
       parametersMap = parametersObj.asInstanceOf[java.util.Map[_, _]]
-      paramValuesObj <- Option(parametersMap.get(plan.authorizedUserKey)) if paramValuesObj.isInstanceOf[java.util.Map[_, _]]
+      paramValuesObj <- Option(parametersMap.get(authorizedUserKey)) if paramValuesObj.isInstanceOf[java.util.Map[_, _]]
       paramValues = paramValuesObj.asInstanceOf[java.util.Map[_, _]]
       authorizedUsersValue <- Option(paramValues.get("value"))
     } yield authorizedUsersValue.toString

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
@@ -174,6 +174,10 @@ object AppConfig {
         if (azureEnabled) {
           Option(
             AzureServicesConfig(
+              config.getBoolean("azureServiceCatalogAppsEnabled"),
+              config.getString("authorizedUserKey"),
+              config.getString("managedAppTypeServiceCatalog"),
+              config.getString("managedAppWorkloadClientId"),
               config.getString("managedAppClientId"),
               config.getString("managedAppClientSecret"),
               config.getString("managedAppTenantId"),

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
@@ -2,6 +2,10 @@ package org.broadinstitute.dsde.workbench.sam.config
 
 case class ManagedAppPlan(name: String, publisher: String, authorizedUserKey: String)
 case class AzureServicesConfig(
+    azureServiceCatalogAppsEnabled: Boolean,
+    authorizedUserKey: String,
+    managedAppTypeServiceCatalog: String,
+    managedAppWorkloadClientId: String,
     managedAppClientId: String,
     managedAppClientSecret: String,
     managedAppTenantId: String,

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
@@ -209,10 +209,10 @@ object TestSamRoutes {
     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",
+      azureServiceCatalogAppsEnabled = false,
+      "mock-auth-user-key",
+      "mock-kind",
+      "mock-managedapp-workload-clientid",
       "mock-managedapp-clientid",
       "mock-managedapp-clientsecret",
       "mock-managedapp-tenantid",

diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceSpec.scala
@@ -294,6 +294,12 @@ class AzureServiceSpec(_system: ActorSystem)
         mockMrgDAO
       )
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
+    when(mockCrlService.getManagedAppPlans)
+      .thenReturn(Seq(MockCrlService.defaultManagedAppPlan))
+
     svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
     mockMrgDAO.mrgs should contain(managedResourceGroup)
   }
@@ -340,6 +346,9 @@ class AzureServiceSpec(_system: ActorSystem)
         mockMrgDAO
       )
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     mockMrgDAO.insertManagedResourceGroup(managedResourceGroup.copy(billingProfileId = BillingProfileId("no the same")), samRequestContext).unsafeRunSync()
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
@@ -363,6 +372,9 @@ class AzureServiceSpec(_system: ActorSystem)
         mockMrgDAO
       )
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     when(mockCrlService.getManagedAppPlans)
       .thenReturn(Seq(MockCrlService.defaultManagedAppPlan))
 
@@ -387,6 +399,9 @@ class AzureServiceSpec(_system: ActorSystem)
     val mockAzureServicesConfig = mock[AzureServicesConfig]
     val svc = new AzureService(mockAzureServicesConfig, MockCrlService(user), new MockDirectoryDAO(), mockMrgDAO)
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
     }
@@ -415,6 +430,9 @@ class AzureServiceSpec(_system: ActorSystem)
       .head
     when(mockApplication.managedResourceGroupId()).thenReturn("something else")
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
     }
@@ -444,6 +462,9 @@ class AzureServiceSpec(_system: ActorSystem)
     when(mockApplication.plan())
       .thenReturn(null)
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
     }
@@ -472,6 +493,9 @@ class AzureServiceSpec(_system: ActorSystem)
     when(mockApplication.plan())
       .thenReturn(new Plan().withName(MockCrlService.defaultManagedAppPlan.name).withPublisher("other publisher"))
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
     }
@@ -500,6 +524,9 @@ class AzureServiceSpec(_system: ActorSystem)
     when(mockApplication.plan())
       .thenReturn(new Plan().withName("other name").withPublisher(MockCrlService.defaultManagedAppPlan.publisher))
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
     }
@@ -521,6 +548,9 @@ class AzureServiceSpec(_system: ActorSystem)
         mockMrgDAO
       )
 
+    when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+      .thenReturn(false)
+
     val err = intercept[WorkbenchExceptionWithErrorReport] {
       svc
         .createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user.map(_.copy(email = WorkbenchEmail("[email protected]")))))
@@ -562,6 +592,9 @@ class AzureServiceSpec(_system: ActorSystem)
       when(mockApplication.parameters())
         .thenReturn(parameters)
 
+      when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled)
+        .thenReturn(false)
+
       val err = intercept[WorkbenchExceptionWithErrorReport] {
         svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync()
       }

diff --git a/...scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala b/...scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala
@@ -76,7 +76,13 @@ class CreateUserSpec extends UserServiceTestTraits {
         val directoryDAO: DirectoryDAO = MockDirectoryDaoBuilder(allUsersGroup).build
         val cloudExtensions: CloudExtensions = MockCloudExtensionsBuilder(allUsersGroup).build
         val userService: UserService =
-          new UserService(directoryDAO, cloudExtensions, Seq.empty, defaultTosService, Some(AzureServicesConfig("", "", "", Seq.empty, true)))
+          new UserService(
+            directoryDAO,
+            cloudExtensions,
+            Seq.empty,
+            defaultTosService,
+            Some(AzureServicesConfig(azureServiceCatalogAppsEnabled = false, "", "", "", "", "", "", Seq.empty, allowManagedIdentityUserCreation = true))
+          )
 
         // Act
         val newUsersStatus = runAndWait(userService.createUser(newAzureUser, samRequestContext))
@@ -284,7 +290,13 @@ class CreateUserSpec extends UserServiceTestTraits {
         val directoryDAO: DirectoryDAO = MockDirectoryDaoBuilder(allUsersGroup).build
         val cloudExtensions: CloudExtensions = MockCloudExtensionsBuilder(allUsersGroup).build
         val userServiceDisabledFeature: UserService =
-          new UserService(directoryDAO, cloudExtensions, Seq.empty, defaultTosService, Some(AzureServicesConfig("", "", "", Seq.empty, false)))
+          new UserService(
+            directoryDAO,
+            cloudExtensions,
+            Seq.empty,
+            defaultTosService,
+            Some(AzureServicesConfig(azureServiceCatalogAppsEnabled = false, "", "", "", "", "", "", Seq.empty, allowManagedIdentityUserCreation = false))
+          )
 
         // Act and Assert
         assertThrows[WorkbenchExceptionWithErrorReport] {