diff --git a/env/local.env b/env/local.env index e42196a1b..dc6d48a41 100644 --- 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="service-admin@dev.test.firecloud.org, service-admin2@dev.test.firecloud.org" 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 index c0c134826..69563d35d 100644 --- a/env/test.env +++ b/env/test.env @@ -3,6 +3,9 @@ export AZURE_ENABLED="false" 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/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala index eda4d0114..858d97b30 100644 --- a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala +++ b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/MockTestSupport.scala @@ -65,6 +65,7 @@ object MockTestSupport extends MockTestSupport { val googleServicesConfig: GoogleServicesConfig = appConfig.googleConfig.get.googleServicesConfig val configResourceTypes: Map[ResourceTypeName, ResourceType] = config.as[Map[String, ResourceType]]("resourceTypes").values.map(rt => rt.name -> rt).toMap val adminConfig: AdminConfig = config.as[AdminConfig]("admin") + val azureServicesConfig: Option[AzureServicesConfig] = appConfig.azureServicesConfig val databaseEnabled: Boolean = config.getBoolean("db.enabled") val databaseEnabledClue = "-- skipping tests that talk to a real database" @@ -141,7 +142,11 @@ object MockTestSupport extends MockTestSupport { val mockManagedGroupService = new ManagedGroupService(mockResourceService, policyEvaluatorService, resourceTypes, policyDAO, directoryDAO, googleExt, "example.com") val tosService = new TosService(googleExt, directoryDAO, tosConfig) - val azureService = new AzureService(MockCrlService(), directoryDAO, new MockAzureManagedResourceGroupDAO) + + val azureService = azureServicesConfig.map { azureConfig => + new AzureService(azureConfig, MockCrlService(), directoryDAO, new MockAzureManagedResourceGroupDAO) + } + MockSamDependencies( mockResourceService, policyEvaluatorService, @@ -173,7 +178,7 @@ object MockTestSupport extends MockTestSupport { samDependencies.tosService, LiquibaseConfig("", initWithLiquibase = false), samDependencies.oauth2Config, - Some(samDependencies.azureService) + samDependencies.azureService ) with MockSamUserDirectives with GoogleExtensionRoutes { override val cloudExtensions: CloudExtensions = samDependencies.cloudExtensions override val googleExtensions: GoogleExtensions = samDependencies.cloudExtensions match { @@ -287,7 +292,7 @@ final case class MockSamDependencies( policyDao: AccessPolicyDAO, cloudExtensions: CloudExtensions, oauth2Config: OpenIDConnectConfiguration, - azureService: AzureService + azureService: Option[AzureService] ) object ConnectedTest extends Tag("connected test") diff --git a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala index 3365fe9af..e972c241c 100644 --- a/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala +++ b/pact4s/src/test/scala/org/broadinstitute/dsde/workbench/sam/provider/SamProviderSpec.scala @@ -192,7 +192,7 @@ class SamProviderSpec accessPolicyDAO, googleExt, FakeOpenIDConnectConfiguration, - azureService + Option(azureService) ) override def beforeAll(): Unit = { diff --git a/src/main/resources/sam.conf b/src/main/resources/sam.conf index 29d7860ff..763676443 100644 --- a/src/main/resources/sam.conf +++ b/src/main/resources/sam.conf @@ -184,6 +184,10 @@ admin { azureServices { azureEnabled = ${?AZURE_ENABLED} allowManagedIdentityUserCreation = ${?AZURE_ALLOW_MANAGED_IDENTITY_USER_CREATION} + azureServiceCatalogAppsEnabled = ${?AZURE_SERVICE_CATALOG_APPS_ENABLED} + authorizedUserKey = "authorizedTerraUser"; + managedAppTypeServiceCatalog = "ServiceCatalog"; + managedAppWorkloadClientId = ${?AZURE_MANAGED_APP_WORKLOAD_CLIENT_ID} managedAppClientId = ${?AZURE_MANAGED_APP_CLIENT_ID} managedAppClientSecret = ${?AZURE_MANAGED_APP_CLIENT_SECRET} managedAppTenantId = ${?AZURE_MANAGED_APP_TENANT_ID} diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala index 2f9b647d2..79c138138 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala @@ -407,7 +407,7 @@ object Boot extends IOApp with LazyLogging { val resourceTypeMap = config.resourceTypes.map(rt => rt.name -> rt).toMap val policyEvaluatorService = PolicyEvaluatorService(config.emailDomain, resourceTypeMap, accessPolicyDAO, directoryDAO) val azureService = config.azureServicesConfig.map { azureConfig => - new AzureService(new CrlService(azureConfig, config.janitorConfig), directoryDAO, azureManagedResourceGroupDAO) + new AzureService(azureConfig, new CrlService(azureConfig, config.janitorConfig), directoryDAO, azureManagedResourceGroupDAO) } val resourceService = new ResourceService( resourceTypeMap, 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 index 37cfae4bf..283f12715 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureService.scala @@ -14,6 +14,7 @@ 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.AzureServicesConfig 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 @@ -23,6 +24,7 @@ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext import scala.jdk.CollectionConverters._ class AzureService( + config: AzureServicesConfig, crlService: CrlService, directoryDAO: DirectoryDAO, azureManagedResourceGroupDAO: AzureManagedResourceGroupDAO @@ -44,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, @@ -244,11 +251,11 @@ class AzureService( samRequestContext: SamRequestContext ): IO[Option[BillingProfileId]] = traceIOWithContext("getBillingProfileIdFromAzureTag", samRequestContext) { _ => for { - mrg <- validateManagedResourceGroup(request.toManagedResourceGroupCoordinates, samRequestContext, false) + mrg <- validateManagedResourceGroup(request.toManagedResourceGroupCoordinates, samRequestContext, validateUser = false) } yield getBillingProfileFromTag(mrg) } - /** Validates a managed resource group. Algorithm: + /** Validates a managed resource group deployed from Azure Marketplace. 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 */ @@ -265,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/azure/CrlService.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala index 3150cc9eb..2a733df63 100644 --- a/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala +++ b/src/main/scala/org/broadinstitute/dsde/workbench/sam/azure/CrlService.scala @@ -4,9 +4,10 @@ import bio.terra.cloudres.azure.resourcemanager.common.Defaults import bio.terra.cloudres.common.ClientConfig import bio.terra.cloudres.common.cleanup.CleanupConfig import cats.effect.IO +import com.azure.core.credential.TokenCredential import com.azure.core.management.AzureEnvironment import com.azure.core.management.profile.AzureProfile -import com.azure.identity.{ClientSecretCredential, ClientSecretCredentialBuilder} +import com.azure.identity.{ChainedTokenCredentialBuilder, ClientSecretCredentialBuilder, ManagedIdentityCredentialBuilder} import com.azure.resourcemanager.managedapplications.ApplicationManager import com.azure.resourcemanager.msi.MsiManager import com.azure.resourcemanager.resources.ResourceManager @@ -59,15 +60,31 @@ class CrlService(config: AzureServicesConfig, janitorConfig: JanitorConfig) { def getManagedAppPlans: Seq[ManagedAppPlan] = config.managedAppPlans - private def getCredentialAndProfile(tenantId: TenantId, subscriptionId: SubscriptionId): (ClientSecretCredential, AzureProfile) = { - val credential = new ClientSecretCredentialBuilder() + private def getCredentialAndProfile(tenantId: TenantId, subscriptionId: SubscriptionId): (TokenCredential, AzureProfile) = { + + val managedIdentityCredential = new ManagedIdentityCredentialBuilder() + .clientId(config.managedAppWorkloadClientId) + .build + + val servicePrincipalCredential = new ClientSecretCredentialBuilder() .clientId(config.managedAppClientId) .clientSecret(config.managedAppClientSecret) .tenantId(config.managedAppTenantId) .build + // When an access token is requested, the chain will try each + // credential in order, stopping when one provides a token + // + // For Managed Identity auth, SAM must be deployed to an Azure service + // other platforms will fall through to Service Principal auth + val credential = new ChainedTokenCredentialBuilder() + .addLast(managedIdentityCredential) + .addLast(servicePrincipalCredential) + .build + val profile = new AzureProfile(tenantId.value, subscriptionId.value, AzureEnvironment.AZURE) (credential, profile) } + } 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 index 8af2a2859..ae96037d6 100644 --- 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("kindServiceCatalog"), + 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 index eb5df244b..0032fecb4 100644 --- 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,7 +1,12 @@ package org.broadinstitute.dsde.workbench.sam.config -case class ManagedAppPlan(name: String, publisher: String, authorizedUserKey: String) -case class AzureServicesConfig( +final case class ManagedAppPlan(name: String, publisher: String, authorizedUserKey: String) {} + +final 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/TestSupport.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/TestSupport.scala index aab7064d4..08442fe37 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/TestSupport.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/TestSupport.scala @@ -1,9 +1,8 @@ package org.broadinstitute.dsde.workbench.sam - +import cats.effect._ import akka.actor.ActorSystem import akka.http.scaladsl.server.{Directive, Directive0} import akka.stream.Materializer -import cats.effect._ import cats.effect.unsafe.implicits.global import cats.kernel.Eq import com.typesafe.config.ConfigFactory @@ -70,6 +69,7 @@ object TestSupport extends TestSupport { val googleServicesConfig = appConfig.googleConfig.get.googleServicesConfig val configResourceTypes = config.as[Map[String, ResourceType]]("resourceTypes").values.map(rt => rt.name -> rt).toMap val adminConfig = config.as[AdminConfig]("admin") + val azureServicesConfig = appConfig.azureServicesConfig val databaseEnabled = config.getBoolean("db.enabled") val databaseEnabledClue = "-- skipping tests that talk to a real database" @@ -149,7 +149,11 @@ object TestSupport extends TestSupport { val mockManagedGroupService = new ManagedGroupService(mockResourceService, policyEvaluatorService, resourceTypes, policyDAO, directoryDAO, googleExt, "example.com") val tosService = new TosService(googleExt, directoryDAO, tosConfig) - val azureService = new AzureService(MockCrlService(), directoryDAO, new MockAzureManagedResourceGroupDAO) + + val azureService = azureServicesConfig.map { azureConfig => + new AzureService(azureConfig, MockCrlService(), directoryDAO, new MockAzureManagedResourceGroupDAO) + } + SamDependencies( mockResourceService, policyEvaluatorService, @@ -182,7 +186,7 @@ object TestSupport extends TestSupport { LiquibaseConfig("", false), samDependencies.oauth2Config, samDependencies.adminConfig, - Some(samDependencies.azureService) + samDependencies.azureService ) with MockSamUserDirectives with GoogleExtensionRoutes { override val cloudExtensions: CloudExtensions = samDependencies.cloudExtensions override val googleExtensions: GoogleExtensions = samDependencies.cloudExtensions match { @@ -293,5 +297,5 @@ final case class SamDependencies( cloudExtensions: CloudExtensions, oauth2Config: OpenIDConnectConfiguration, adminConfig: AdminConfig, - azureService: AzureService + azureService: Option[AzureService] ) 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 index c3a8cd542..534e2e6d5 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/TestSamRoutes.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/TestSamRoutes.scala @@ -6,6 +6,7 @@ import akka.http.scaladsl.server.Directives.reject import akka.http.scaladsl.server.{Directive, Directive0} import akka.stream.Materializer import cats.effect.unsafe.implicits.global +import com.typesafe.config.ConfigFactory import org.broadinstitute.dsde.workbench.google.GoogleDirectoryDAO import org.broadinstitute.dsde.workbench.google.mock.MockGoogleDirectoryDAO import org.broadinstitute.dsde.workbench.model.WorkbenchEmail @@ -13,7 +14,7 @@ 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.{LiquibaseConfig, TermsOfServiceConfig} +import org.broadinstitute.dsde.workbench.sam.config.{AppConfig, AzureServicesConfig, LiquibaseConfig, ManagedAppPlan, 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._ @@ -107,6 +108,9 @@ class TestSamTosEnabledRoutes( } object TestSamRoutes { + val config = ConfigFactory.load() + val appConfig = AppConfig.readConfig(config) + val defaultUserInfo = Generator.genWorkbenchUserGoogle.sample.get object SamResourceActionPatterns { @@ -203,8 +207,27 @@ 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), + allowManagedIdentityUserCreation = true + ) + val azureService = - new AzureService(crlService.getOrElse(MockCrlService(Option(user))), directoryDAO, new MockAzureManagedResourceGroupDAO) + new AzureService( + mockAzureServicesConfig, + crlService.getOrElse(MockCrlService(Option(user))), + directoryDAO, + new MockAzureManagedResourceGroupDAO + ) + new TestSamRoutes( mockResourceService, policyEvaluatorService, @@ -214,7 +237,7 @@ object TestSamRoutes { user, tosService = mockTosService, cloudExtensions = cloudXtns, - azureService = Some(azureService) + azureService = Option(azureService) ) } } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureRoutesSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureRoutesSpec.scala index 5d82b2f1b..3a8261bf8 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureRoutesSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureRoutesSpec.scala @@ -98,6 +98,7 @@ class AzureRoutesSpec extends AnyFlatSpec with Matchers with ScalatestRouteTest it should "return 403 if user has access to the billing profile but the MRG could not be validated" in { // Change the managed app plan val mockCrlService = MockCrlService() + when(mockCrlService.getManagedAppPlans) .thenReturn(Seq(ManagedAppPlan("some-other-plan", "publisher", "auth"), ManagedAppPlan("yet-another-plan", "publisher", "auth"))) val samRoutes = genSamRoutes(crlService = Some(mockCrlService)) 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 index 19e5af2d4..fac2dc691 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceSpec.scala @@ -1,13 +1,15 @@ package org.broadinstitute.dsde.workbench.sam.azure import akka.actor.ActorSystem +import org.broadinstitute.dsde.workbench.model.WorkbenchExceptionWithErrorReport import akka.http.scaladsl.model.StatusCodes import akka.testkit.TestKit import com.azure.resourcemanager.managedapplications.models.Plan -import org.broadinstitute.dsde.workbench.model.{WorkbenchEmail, WorkbenchExceptionWithErrorReport} +import org.broadinstitute.dsde.workbench.model.WorkbenchEmail import org.broadinstitute.dsde.workbench.sam.Generator.genWorkbenchUserAzure import org.broadinstitute.dsde.workbench.sam.TestSupport._ import org.broadinstitute.dsde.workbench.sam.api.TestSamRoutes.SamResourceActionPatterns +import org.broadinstitute.dsde.workbench.sam.config.AzureServicesConfig import org.broadinstitute.dsde.workbench.sam.dataAccess.{ AccessPolicyDAO, DirectoryDAO, @@ -77,7 +79,7 @@ class AzureServiceSpec(_system: ActorSystem) val userService = new UserService(directoryDAO, NoExtensions, Seq.empty, tosService) val azureTestConfig = config.getConfig("testStuff.azure") setUpResources(directoryDAO) - val azureService = new AzureService(crlService, directoryDAO, new MockAzureManagedResourceGroupDAO) + val azureService = new AzureService(azureServicesConfig.get, crlService, directoryDAO, new MockAzureManagedResourceGroupDAO) // create user val defaultUser = genWorkbenchUserAzure.sample.get @@ -187,7 +189,7 @@ class AzureServiceSpec(_system: ActorSystem) val azureManagedResourceGroupDAO = new PostgresAzureManagedResourceGroupDAO(TestSupport.dbRef, TestSupport.dbRef) val azureTestConfig = config.getConfig("testStuff.azure") val (resourceService, defaultResourceType, viewAction) = setUpResources(directoryDAO) - val azureService = new AzureService(crlService, directoryDAO, azureManagedResourceGroupDAO) + val azureService = new AzureService(azureServicesConfig.get, crlService, directoryDAO, azureManagedResourceGroupDAO) // create user val defaultUser = Generator.genWorkbenchUserAzure.sample.map(_.copy(email = WorkbenchEmail("hermione.owner@test.firecloud.org"))).get @@ -281,13 +283,24 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO + val azureServicesConfig = appConfig.azureServicesConfig + val mockCrlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) + val mockAzureServicesConfig = mock[AzureServicesConfig] + val svc = new AzureService( - MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName), + mockAzureServicesConfig, + mockCrlService, new MockDirectoryDAO(), 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) } @@ -301,7 +314,7 @@ class AzureServiceSpec(_system: ActorSystem) // create dependencies val crlService = new CrlService(azureServicesConfig.get, janitorConfig) val mockMrgDAO = new MockAzureManagedResourceGroupDAO - val azureService = new AzureService(crlService, new MockDirectoryDAO(), mockMrgDAO) + val azureService = new AzureService(azureServicesConfig.get, crlService, new MockDirectoryDAO(), mockMrgDAO) // build request val azureTestConfig = config.getConfig("testStuff.azure") @@ -324,13 +337,18 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO + val mockAzureServicesConfig = mock[AzureServicesConfig] val svc = new AzureService( + mockAzureServicesConfig, MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName), new MockDirectoryDAO(), 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() @@ -343,13 +361,23 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO + val mockCrlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) + val mockAzureServicesConfig = mock[AzureServicesConfig] + val svc = new AzureService( - MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName), + mockAzureServicesConfig, + mockCrlService, new MockDirectoryDAO(), mockMrgDAO ) + when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled) + .thenReturn(false) + + when(mockCrlService.getManagedAppPlans) + .thenReturn(Seq(MockCrlService.defaultManagedAppPlan)) + mockMrgDAO .insertManagedResourceGroup( managedResourceGroup @@ -368,7 +396,11 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO - val svc = new AzureService(MockCrlService(user), new MockDirectoryDAO(), mockMrgDAO) + 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() @@ -382,7 +414,9 @@ class AzureServiceSpec(_system: ActorSystem) val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO val crlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) - val svc = new AzureService(crlService, new MockDirectoryDAO(), mockMrgDAO) + val mockAzureServicesConfig = mock[AzureServicesConfig] + + val svc = new AzureService(mockAzureServicesConfig, crlService, new MockDirectoryDAO(), mockMrgDAO) val mockApplication = crlService .buildApplicationManager( @@ -394,8 +428,12 @@ class AzureServiceSpec(_system: ActorSystem) .list() .asScala .head + when(mockApplication.managedResourceGroupId()).thenReturn("something else") + when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled) + .thenReturn(false) + val err = intercept[WorkbenchExceptionWithErrorReport] { svc.createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user)).unsafeRunSync() } @@ -407,8 +445,10 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO + val mockAzureServicesConfig = mock[AzureServicesConfig] val crlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) - val svc = new AzureService(crlService, new MockDirectoryDAO(), mockMrgDAO) + + val svc = new AzureService(mockAzureServicesConfig, crlService, new MockDirectoryDAO(), mockMrgDAO) val mockApplication = crlService .buildApplicationManager( @@ -423,6 +463,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() } @@ -435,7 +478,8 @@ class AzureServiceSpec(_system: ActorSystem) val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO val crlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) - val svc = new AzureService(crlService, new MockDirectoryDAO(), mockMrgDAO) + val mockAzureServicesConfig = mock[AzureServicesConfig] + val svc = new AzureService(mockAzureServicesConfig, crlService, new MockDirectoryDAO(), mockMrgDAO) val mockApplication = crlService .buildApplicationManager( @@ -447,6 +491,10 @@ class AzureServiceSpec(_system: ActorSystem) .list() .asScala .head + + when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled) + .thenReturn(false) + when(mockApplication.plan()) .thenReturn(new Plan().withName(MockCrlService.defaultManagedAppPlan.name).withPublisher("other publisher")) @@ -462,7 +510,8 @@ class AzureServiceSpec(_system: ActorSystem) val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO val crlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) - val svc = new AzureService(crlService, new MockDirectoryDAO(), mockMrgDAO) + val mockAzureServicesConfig = mock[AzureServicesConfig] + val svc = new AzureService(mockAzureServicesConfig, crlService, new MockDirectoryDAO(), mockMrgDAO) val mockApplication = crlService .buildApplicationManager( @@ -474,6 +523,10 @@ class AzureServiceSpec(_system: ActorSystem) .list() .asScala .head + + when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled) + .thenReturn(false) + when(mockApplication.plan()) .thenReturn(new Plan().withName("other name").withPublisher(MockCrlService.defaultManagedAppPlan.publisher)) @@ -488,13 +541,18 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO + val mockAzureServicesConfig = mock[AzureServicesConfig] val svc = new AzureService( + mockAzureServicesConfig, MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName), new MockDirectoryDAO(), mockMrgDAO ) + when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled) + .thenReturn(false) + val err = intercept[WorkbenchExceptionWithErrorReport] { svc .createManagedResourceGroup(managedResourceGroup, samRequestContext.copy(samUser = user.map(_.copy(email = WorkbenchEmail("other@email.com"))))) @@ -509,7 +567,11 @@ class AzureServiceSpec(_system: ActorSystem) val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO val crlService = MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName) - val svc = new AzureService(crlService, new MockDirectoryDAO(), mockMrgDAO) + val mockAzureServicesConfig = mock[AzureServicesConfig] + val svc = new AzureService(mockAzureServicesConfig, crlService, new MockDirectoryDAO(), mockMrgDAO) + + when(mockAzureServicesConfig.azureServiceCatalogAppsEnabled) + .thenReturn(false) val mockApplication = crlService .buildApplicationManager( @@ -546,8 +608,11 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val managedResourceGroup = Generator.genManagedResourceGroup.sample.get val mockMrgDAO = new MockAzureManagedResourceGroupDAO + val mockAzureServicesConfig = mock[AzureServicesConfig] + val svc = new AzureService( + mockAzureServicesConfig, MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName), new MockDirectoryDAO(), mockMrgDAO @@ -562,8 +627,10 @@ class AzureServiceSpec(_system: ActorSystem) val user = Generator.genWorkbenchUserAzure.sample val mockMrgDAO = new MockAzureManagedResourceGroupDAO val managedResourceGroup = Generator.genManagedResourceGroup.sample.get + val mockAzureServicesConfig = mock[AzureServicesConfig] val svc = new AzureService( + mockAzureServicesConfig, MockCrlService(user, managedResourceGroup.managedResourceGroupCoordinates.managedResourceGroupName), new MockDirectoryDAO(), mockMrgDAO diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceUnitSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceUnitSpec.scala index ad8f1eae6..f994bdeea 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceUnitSpec.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/AzureServiceUnitSpec.scala @@ -12,7 +12,7 @@ import com.azure.resourcemanager.msi.models.Identity.DefinitionStages import com.azure.resourcemanager.msi.models.{Identities, Identity} import com.azure.resourcemanager.resources.ResourceManager import com.azure.resourcemanager.resources.models.{ResourceGroup, ResourceGroups} -import org.broadinstitute.dsde.workbench.sam.config.ManagedAppPlan +import org.broadinstitute.dsde.workbench.sam.config.{AzureServicesConfig, ManagedAppPlan} import org.broadinstitute.dsde.workbench.sam.dataAccess.{AzureManagedResourceGroupDAO, DirectoryDAO} import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, ResourceAction, ResourceId, ResourceTypeName} import org.broadinstitute.dsde.workbench.sam.{Generator, PropertyBasedTesting, TestSupport} @@ -48,8 +48,8 @@ class AzureServiceUnitSpec extends AnyFreeSpec with Matchers with ScalaFutures w val mockIdentityWithGroup = mock[DefinitionStages.WithGroup] val mockIdentityWithCreate = mock[DefinitionStages.WithCreate] val mockIdentity = mock[Identity] - - val azureService = new AzureService(mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) + val mockAzureServicesConfig = mock[AzureServicesConfig] + val azureService = new AzureService(mockAzureServicesConfig, mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) val testMrgCoordinates = ManagedResourceGroupCoordinates( TenantId(UUID.randomUUID().toString), @@ -110,8 +110,9 @@ class AzureServiceUnitSpec extends AnyFreeSpec with Matchers with ScalaFutures w val mockCrlService = mock[CrlService] val mockDirectoryDAO = mock[DirectoryDAO] val mockAzureManagedResourceGroupDAO = mock[AzureManagedResourceGroupDAO] + val mockAzureServicesConfig = mock[AzureServicesConfig] - val azureService = new AzureService(mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) + val azureService = new AzureService(mockAzureServicesConfig, mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) val testMrgCoordinates = ManagedResourceGroupCoordinates( TenantId(UUID.randomUUID().toString), @@ -145,8 +146,9 @@ class AzureServiceUnitSpec extends AnyFreeSpec with Matchers with ScalaFutures w val mockAzureManagedResourceGroupDAO = mock[AzureManagedResourceGroupDAO] val mockMsiManager = mock[MsiManager] val mockIdentities = mock[Identities] + val mockAzureServicesConfig = mock[AzureServicesConfig] - val azureService = new AzureService(mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) + val azureService = new AzureService(mockAzureServicesConfig, mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) val testMrgCoordinates = ManagedResourceGroupCoordinates( TenantId(UUID.randomUUID().toString), @@ -180,8 +182,9 @@ class AzureServiceUnitSpec extends AnyFreeSpec with Matchers with ScalaFutures w val mockAzureManagedResourceGroupDAO = mock[AzureManagedResourceGroupDAO] val mockMsiManager = mock[MsiManager] val mockIdentities = mock[Identities] + val mockAzureServicesConfig = mock[AzureServicesConfig] - val azureService = new AzureService(mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) + val azureService = new AzureService(mockAzureServicesConfig, mockCrlService, mockDirectoryDAO, mockAzureManagedResourceGroupDAO) val testMrgCoordinates = ManagedResourceGroupCoordinates( TenantId(UUID.randomUUID().toString), diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/MockCrlService.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/MockCrlService.scala index eb02bb549..11eff0132 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/MockCrlService.scala +++ b/src/test/scala/org/broadinstitute/dsde/workbench/sam/azure/MockCrlService.scala @@ -22,16 +22,16 @@ import org.mockito.scalatest.MockitoSugar import scala.jdk.CollectionConverters._ object MockCrlService extends MockitoSugar { - val mockMrgName = ManagedResourceGroupName("test-mrg") - val mockSamSpendProfileResource = FullyQualifiedResourceId(SamResourceTypes.spendProfile, ResourceId("test-spend-profile")) - val defaultManagedAppPlan = ManagedAppPlan("mock-plan", "mock-publisher", "mock-auth-user-key") + val mockMrgName: ManagedResourceGroupName = ManagedResourceGroupName("test-mrg") + val mockSamSpendProfileResource: FullyQualifiedResourceId = FullyQualifiedResourceId(SamResourceTypes.spendProfile, ResourceId("test-spend-profile")) + val defaultManagedAppPlan: ManagedAppPlan = ManagedAppPlan("mock-plan", "mock-publisher", "mock-auth-user-key") def apply( user: Option[SamUser] = None, mrgName: ManagedResourceGroupName = mockMrgName, managedAppPlan: ManagedAppPlan = defaultManagedAppPlan, includeBillingProfileTag: Boolean = false - ) = { + ): CrlService = { val mockCrlService = mock[CrlService](RETURNS_SMART_NULLS) val mockRm = mockResourceManager(mrgName, includeBillingProfileTag) val mockAppMgr = mockApplicationManager(user, mrgName, managedAppPlan) @@ -45,14 +45,14 @@ object MockCrlService extends MockitoSugar { .when(mockCrlService.buildMsiManager(any[TenantId], any[SubscriptionId])) .thenReturn(IO.pure(mockMsi)) - lenient() - .when(mockCrlService.getManagedAppPlans) - .thenReturn(Seq(managedAppPlan)) - lenient() .when(mockCrlService.buildApplicationManager(any[TenantId], any[SubscriptionId])) .thenReturn(IO.pure(mockAppMgr)) + lenient() + .when(mockCrlService.getManagedAppPlans) + .thenReturn(Seq(managedAppPlan)) + mockCrlService } diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala index 3940a4823..21ded059a 100644 --- a/src/test/scala/org/broadinstitute/dsde/workbench/sam/service/UserServiceSpecs/CreateUserSpec.scala +++ b/src/test/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] {