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

ID-910 uami email #1248

Merged
merged 6 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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 src/main/resources/sam.conf
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ admin {

azureServices {
azureEnabled = ${?AZURE_ENABLED}
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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@ object Boot extends IOApp with LazyLogging {
config.adminConfig.allowedEmailDomains
)
val tosService = new TosService(cloudExtensionsInitializer.cloudExtensions, directoryDAO, config.termsOfServiceConfig)
val userService = new UserService(directoryDAO, cloudExtensionsInitializer.cloudExtensions, config.blockedEmailDomains, tosService)
val userService =
new UserService(directoryDAO, cloudExtensionsInitializer.cloudExtensions, config.blockedEmailDomains, tosService, config.azureServicesConfig)
val statusService =
new StatusService(directoryDAO, cloudExtensionsInitializer.cloudExtensions, 10 seconds, 1 minute)
val managedGroupService =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ trait StandardSamUserDirectives extends SamUserDirectives with LazyLogging with

object StandardSamUserDirectives {
val SAdomain: Regex = "(\\S+@\\S*gserviceaccount\\.com$)".r
val UAMIdomain: Regex = "(\\S+@\\S*uami\\.terra\\.com$)".r
// UAMI == "User Assigned Managed Identity" in Azure
val UamiPattern: Regex = "(^/subscriptions/\\S+/resourcegroups/\\S+/providers/Microsoft\\.ManagedIdentity/userAssignedIdentities/\\S+$)".r
val accessTokenHeader = "OIDC_access_token"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ object AppConfig {
config.getString("managedAppClientId"),
config.getString("managedAppClientSecret"),
config.getString("managedAppTenantId"),
config.as[Seq[ManagedAppPlan]]("managedAppPlans")
config.as[Seq[ManagedAppPlan]]("managedAppPlans"),
config.as[Option[Boolean]]("allowManagedIdentityUserCreation").getOrElse(false)
)
)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ case class AzureServicesConfig(
managedAppClientId: String,
managedAppClientSecret: String,
managedAppTenantId: String,
managedAppPlans: Seq[ManagedAppPlan]
managedAppPlans: Seq[ManagedAppPlan],
allowManagedIdentityUserCreation: Boolean
) {}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class TosService(
return true
}
// Service Account users do not need to accept ToS
val userIsServiceAccount = StandardSamUserDirectives.SAdomain.matches(user.email.value)
val userIsServiceAccount = StandardSamUserDirectives.SAdomain.matches(user.email.value) || StandardSamUserDirectives.UAMIdomain.matches(user.email.value)
if (userIsServiceAccount) {
return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.model.google.ServiceAccountSubjectId
import org.broadinstitute.dsde.workbench.openTelemetry.OpenTelemetryMetrics
import org.broadinstitute.dsde.workbench.sam.azure.ManagedIdentityObjectId
import org.broadinstitute.dsde.workbench.sam.config.AzureServicesConfig
import org.broadinstitute.dsde.workbench.sam.dataAccess.DirectoryDAO
import org.broadinstitute.dsde.workbench.sam.model._
import org.broadinstitute.dsde.workbench.sam.model.api.{
Expand All @@ -32,15 +33,35 @@ import scala.util.matching.Regex

/** Created by dvoet on 7/14/17.
*/
class UserService(val directoryDAO: DirectoryDAO, val cloudExtensions: CloudExtensions, blockedEmailDomains: Seq[String], tosService: TosService)(implicit
class UserService(
val directoryDAO: DirectoryDAO,
val cloudExtensions: CloudExtensions,
blockedEmailDomains: Seq[String],
tosService: TosService,
azureConfig: Option[AzureServicesConfig] = None
)(implicit
val executionContext: ExecutionContext,
val openTelemetry: OpenTelemetryMetrics[IO]
) extends LazyLogging {
def createUser(possibleNewUser: SamUser, samRequestContext: SamRequestContext): IO[UserStatus] =
createUser(possibleNewUser, None, samRequestContext)

def createUser(possibleNewUser: SamUser, registrationRequest: Option[SamUserRegistrationRequest], samRequestContext: SamRequestContext): IO[UserStatus] =
def createUser(
possibleNewUserMaybeWithEmail: SamUser,
registrationRequest: Option[SamUserRegistrationRequest],
samRequestContext: SamRequestContext
): IO[UserStatus] =
openTelemetry.time("api.v1.user.create.time", API_TIMING_DURATION_BUCKET) {
val email =
if (shouldCreateUamiEmail(possibleNewUserMaybeWithEmail)) {
// If the email is missing but Azure B2C ID exist, this is a managed identity. If the config allows it, create an
// email address for the user. This is because the email address is used as the unique identifier for the user.
WorkbenchEmail(s"${possibleNewUserMaybeWithEmail.azureB2CId.get.value}@uami.terra.bio")
} else {
possibleNewUserMaybeWithEmail.email
}
val possibleNewUser = possibleNewUserMaybeWithEmail.copy(email = email)

// Validate the values set on the possible new user, short circuit if there's a problem
val validationErrors = validateUser(possibleNewUser)
if (validationErrors.nonEmpty) {
Expand Down Expand Up @@ -76,6 +97,11 @@ class UserService(val directoryDAO: DirectoryDAO, val cloudExtensions: CloudExte
)
}

private def shouldCreateUamiEmail(possibleNewUserMaybeWithEmail: SamUser) =
possibleNewUserMaybeWithEmail.email.value.isEmpty && possibleNewUserMaybeWithEmail.azureB2CId.nonEmpty && azureConfig.exists(
_.allowManagedIdentityUserCreation
)

private def registerNewUserAttributes(
userId: WorkbenchUserId,
registrationRequest: Option[SamUserRegistrationRequest],
Expand Down
10 changes: 10 additions & 0 deletions src/test/scala/Generator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ object Generator {
userId <- genWorkbenchUserId
} yield SamUser(userId, None, email, Option(azureB2CId), false)

val genNewWorkbenchUserAzureUami = for {
azureB2CId <- genAzureB2CId
userId <- genWorkbenchUserId
} yield SamUser(userId, None, WorkbenchEmail(""), Option(azureB2CId), false)

val genWorkbenchUserAzureUami = for {
azureB2CId <- genAzureB2CId
userId <- genWorkbenchUserId
} yield SamUser(userId, None, WorkbenchEmail(s"${azureB2CId.value}@uami.terra.bio"), Option(azureB2CId), false)

val genWorkbenchUserBoth = for {
email <- genNonPetEmail
googleSubjectId <- genGoogleSubjectId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class TosServiceSpec(_system: ActorSystem)

private val defaultUser = Generator.genWorkbenchUserBoth.sample.get
private val serviceAccountUser = Generator.genWorkbenchUserServiceAccount.sample.get
private val uamiUser = Generator.genWorkbenchUserAzureUami.sample.get

before {
clearDatabase()
Expand Down Expand Up @@ -147,6 +148,21 @@ class TosServiceSpec(_system: ActorSystem)
complianceStatus.permitsSystemUsage shouldBe true
}

"always allows UAMI users to use the system" in {
val tosVersion = "2"
val previousTosVersion = Option("1")
val tosService =
new TosService(NoExtensions, dirDAO, TestSupport.tosConfig.copy(version = tosVersion, previousVersion = previousTosVersion))
when(dirDAO.getUserTos(uamiUser.id, samRequestContext))
.thenReturn(IO.pure(Some(SamUserTos(uamiUser.id, tosVersion, TosTable.ACCEPT, Instant.now()))))

when(dirDAO.getUserTosVersion(uamiUser.id, previousTosVersion, samRequestContext))
.thenReturn(IO.pure(Some(SamUserTos(uamiUser.id, previousTosVersion.get, TosTable.ACCEPT, Instant.now()))))

val complianceStatus = tosService.getTosComplianceStatus(uamiUser, samRequestContext).unsafeRunSync()
complianceStatus.permitsSystemUsage shouldBe true
}

"loads the Terms of Service text when TosService is instantiated" in {
val tosService = new TosService(NoExtensions, dirDAO, TestSupport.tosConfig.copy(version = "2"))
tosService.termsOfServiceText contains "Test Terms of Service"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package org.broadinstitute.dsde.workbench.sam.service.UserServiceSpecs

import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.sam.Generator.{genWorkbenchUserAzure, genWorkbenchUserBoth, genWorkbenchUserGoogle}
import org.broadinstitute.dsde.workbench.sam.Generator.{genNewWorkbenchUserAzureUami, genWorkbenchUserAzure, genWorkbenchUserBoth, genWorkbenchUserGoogle}
import org.broadinstitute.dsde.workbench.sam.config.AzureServicesConfig
import org.broadinstitute.dsde.workbench.sam.dataAccess.{DirectoryDAO, MockDirectoryDaoBuilder}
import org.broadinstitute.dsde.workbench.sam.matchers.BeEnabledInMatcher.beEnabledIn
import org.broadinstitute.dsde.workbench.sam.matchers.BeForUserMatcher.beForUser
Expand Down Expand Up @@ -70,6 +71,28 @@ class CreateUserSpec extends UserServiceTestTraits {
}
}

it("after authenticating with a Microsoft managed identity") {
// Arrange
val newAzureUser = genNewWorkbenchUserAzureUami.sample.get
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)))

// Act
val newUsersStatus = runAndWait(userService.createUser(newAzureUser, samRequestContext))

// Assert
inside(newUsersStatus) { status =>
status should beForUser(newAzureUser.copy(email = WorkbenchEmail(s"${newAzureUser.azureB2CId.get.value}@uami.terra.bio")))
"google" should beEnabledIn(status)
"ldap" should beEnabledIn(status)
"allUsersGroup" should beEnabledIn(status)
"adminEnabled" should beEnabledIn(status)
"tosAccepted" shouldNot beEnabledIn(status)
}
}

it("after authenticating with Google through Azure B2C") {
// Arrange
val newUserWithBothCloudIds = genWorkbenchUserBoth.sample.get
Expand Down Expand Up @@ -230,6 +253,33 @@ class CreateUserSpec extends UserServiceTestTraits {
runAndWait(userService.createUser(userWithoutIds, Some(invalidRegistrationRequest), samRequestContext))
}
}

it("with a Microsoft managed identity if the feature is disabled") {
// Arrange
val newAzureUser = genNewWorkbenchUserAzureUami.sample.get
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)))

// Act and Assert
assertThrows[WorkbenchExceptionWithErrorReport] {
runAndWait(userServiceDisabledFeature.createUser(newAzureUser, samRequestContext))
}
}

it("with a Microsoft managed identity if the azure is disabled") {
// Arrange
val newAzureUser = genNewWorkbenchUserAzureUami.sample.get
val directoryDAO: DirectoryDAO = MockDirectoryDaoBuilder(allUsersGroup).build
val cloudExtensions: CloudExtensions = MockCloudExtensionsBuilder(allUsersGroup).build
val userServiceNoAzureConfig: UserService = new UserService(directoryDAO, cloudExtensions, Seq.empty, defaultTosService)

// Act and Assert
assertThrows[WorkbenchExceptionWithErrorReport] {
runAndWait(userServiceNoAzureConfig.createUser(newAzureUser, samRequestContext))
}
}
}

describe("should be added to the All Users group") {
Expand Down
Loading