Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TOAZ-355] [TOAZ-356] Use Managed Identity auth when running Azure Control Plane, support for Service Catalog deployed Azure Managed Apps #1404

Merged
merged 31 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0417ecc
ServiceCatalog deployed managed app support
Apr 9, 2024
e1cc7e7
Temporary debug logging
Apr 11, 2024
0534c51
Testing with Service Principal auth
Apr 12, 2024
6f4d602
Testing managed app plan validation
Apr 12, 2024
7877c5f
Testing managed app plan validation
Apr 12, 2024
7e0274d
Testing managed app plan validation
Apr 12, 2024
5f49ba6
Testing managed app plan validation
Apr 16, 2024
cb56476
Testing managed app plan validation with managed identity auth
Apr 16, 2024
f5b53c3
Use ChainedTokenCrendentialBuilder to support both Managed Identity a…
Apr 18, 2024
aee2371
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
Apr 18, 2024
296812a
Scala formatting
Apr 18, 2024
cdf3d4f
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
Apr 25, 2024
e10a094
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
May 1, 2024
4b3ca19
Added azureControlPlane flag to mockCrlService
May 2, 2024
c230179
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
May 7, 2024
c7662f9
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
May 14, 2024
8036d83
Refactor injection of AzureServicesConfig to AzureService
May 14, 2024
75fa4f2
Resets postgres urls
May 14, 2024
f4cafc0
AzureServiceCatalogAppsEnabled flag rename
May 14, 2024
4288821
Inject AzureServicesConfig to AzureService
May 14, 2024
94f3437
Restores CrlService.getManagedAppPlans
May 15, 2024
20b96ce
Clean up of AzureConfig reading and ClrService mocks
May 16, 2024
a9ad792
Refactor of validateAuthorizedAppUser and AzureServicesConfig mock cl…
May 16, 2024
953012f
Handles servicecatalogapps flag check when AzureServicesConfig is None
May 17, 2024
8a22dd0
AzureService now takes an Option AzureServiceConfig, reworked the log…
May 23, 2024
74203df
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
May 23, 2024
8b42ab9
Removed AzureServicesConfig as Option to AzureService, adjusted tests
May 23, 2024
64d6dfe
Merge remote-tracking branch 'origin/develop' into grantaar-azuremana…
May 23, 2024
114b114
AzureServiceConfig update for MockTestSupport
May 23, 2024
c8ca065
AzureServiceConfig update for SamProviderSpec
May 23, 2024
123d29d
Rename of config option to mananagedAppTypeServiceCatalog
May 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions env/local.env
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions env/test.env
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: AzureServicesConfig = appConfig.azureServicesConfig.orNull
val databaseEnabled: Boolean = config.getBoolean("db.enabled")
val databaseEnabledClue = "-- skipping tests that talk to a real database"

Expand Down Expand Up @@ -141,7 +142,7 @@ 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 = new AzureService(azureServicesConfig, MockCrlService(), directoryDAO, new MockAzureManagedResourceGroupDAO)
MockSamDependencies(
mockResourceService,
policyEvaluatorService,
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/sam.conf
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ admin {
azureServices {
azureEnabled = ${?AZURE_ENABLED}
allowManagedIdentityUserCreation = ${?AZURE_ALLOW_MANAGED_IDENTITY_USER_CREATION}
azureServiceCatalogAppsEnabled = ${?AZURE_SERVICE_CATALOG_APPS_ENABLED}
authorizedUserKey = "authorizedTerraUser";
kindServiceCatalog = "ServiceCatalog";
Shakespeared marked this conversation as resolved.
Show resolved Hide resolved
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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -44,7 +46,12 @@ class AzureService(

def createManagedResourceGroup(managedResourceGroup: ManagedResourceGroup, samRequestContext: SamRequestContext): IO[Unit] =
for {
_ <- validateManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
_ <-
if (Option(config).isDefined && config.azureServiceCatalogAppsEnabled) {
aaronegrant marked this conversation as resolved.
Show resolved Hide resolved
validateServiceCatalogManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
} else {
validateManagedResourceGroup(managedResourceGroup.managedResourceGroupCoordinates, samRequestContext)
}

existingByCoords <- azureManagedResourceGroupDAO.getManagedResourceGroupByCoordinates(
managedResourceGroup.managedResourceGroupCoordinates,
Expand Down Expand Up @@ -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
*/
Expand All @@ -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(
aaronegrant marked this conversation as resolved.
Show resolved Hide resolved
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.kindServiceCatalog && validateUser) {
aaronegrant marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
aaronegrant marked this conversation as resolved.
Show resolved Hide resolved
.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)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
kindServiceCatalog: String,
managedAppWorkloadClientId: String,
managedAppClientId: String,
managedAppClientSecret: String,
managedAppTenantId: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.orNull
val databaseEnabled = config.getBoolean("db.enabled")
val databaseEnabledClue = "-- skipping tests that talk to a real database"

Expand Down Expand Up @@ -149,7 +149,7 @@ 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 = new AzureService(azureServicesConfig, MockCrlService(), directoryDAO, new MockAzureManagedResourceGroupDAO)
SamDependencies(
mockResourceService,
policyEvaluatorService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ 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
import org.broadinstitute.dsde.workbench.oauth2.mock.FakeOpenIDConnectConfiguration
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, 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._
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -203,8 +207,15 @@ object TestSamRoutes {
mockResourceService.initResourceTypes(samRequestContext).unsafeRunSync()

val mockStatusService = new StatusService(directoryDAO, cloudXtns)
val mockAzureServicesConfig = appConfig.azureServicesConfig.orNull

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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading
Loading