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

CORE-232: user supportSummary should use resource_type_admin #1621

Merged
merged 6 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 2 additions & 10 deletions src/main/resources/swagger/api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4268,20 +4268,12 @@ components:
directSynchronized:
type: integer
description: |
Number of groups to which the user is a direct member
and which have been successfully synchronized to the cloud.
Number of groups to which the user is a direct member.
Does not include groups from public resources.
totalSynchronized:
type: integer
description: |
Number of groups to which the user is a direct or indirect member
and which have been successfully synchronized to the cloud.
Does not include groups from public resources.
unsynchronized:
type: integer
description: |
Number of groups to which the user is a direct or indirect member
and which have not yet been synchronized to the cloud.
Number of groups to which the user is a direct or indirect member.
Does not include groups from public resources.
ManagedGroupMembershipEntry:
required:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import org.broadinstitute.dsde.workbench.sam.model.SamResourceActions.{adminAddM
import org.broadinstitute.dsde.workbench.sam.model.SamResourceTypes.resourceTypeAdminName
import org.broadinstitute.dsde.workbench.sam.model._
import org.broadinstitute.dsde.workbench.sam.model.api.{AccessPolicyMembershipRequest, AdminUpdateUserRequest, SamUser}
import org.broadinstitute.dsde.workbench.sam.service.{ManagedGroupService, ResourceService}
import org.broadinstitute.dsde.workbench.sam.service.{ManagedGroupService, ResourceService, UserService}
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
import spray.json.DefaultJsonProtocol._
import spray.json.JsBoolean
Expand Down Expand Up @@ -47,6 +47,20 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi

def adminUserRoutes(samUser: SamUser, samRequestContext: SamRequestContext): server.Route =
pathPrefix("user") {
// routes requiring resource_type_admin/user actions:
path("email" / Segment / "supportSummary") { email =>
withResourceType(UserService.userTypeName) { userType =>
requireAdminResourceAction(SamResourceActions.adminReadSummaryInformation, userType, samUser, samRequestContext) {
val workbenchEmail = WorkbenchEmail(email)
getWithTelemetry(samRequestContext, emailParam(workbenchEmail)) {
complete {
userService.getSamUserCombinedState(workbenchEmail, samRequestContext, resourceService)
}
}
}
}
} ~
// routes requiring admin:
asWorkbenchAdmin(samUser) {
pathPrefix("email" / Segment) { email =>
val workbenchEmail = WorkbenchEmail(email)
Expand All @@ -58,15 +72,6 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi
.map(status => (if (status.isDefined) OK else NotFound) -> status)
}
}
} ~
pathPrefix("supportSummary") {
pathEndOrSingleSlash {
getWithTelemetry(samRequestContext, emailParam(workbenchEmail)) {
complete {
userService.getSamUserCombinedState(workbenchEmail, samRequestContext, resourceService)
}
}
}
}
} ~
pathPrefix(Segment) { userId =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,16 @@ trait DirectoryDAO {

def listFlattenedGroupMembers(groupName: WorkbenchGroupName, samRequestContext: SamRequestContext): IO[Set[WorkbenchUserId]]

/** @return
* the number of groups to which the user is a direct member, not including public resources.
*/
def countDirectSynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int]

/** @return
* the number of groups to which the user is a direct or indirect member, not including public resources.
*/
def countIndirectSynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int]

def countUnsynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int]

def enableIdentity(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Unit]

def disableIdentity(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Unit]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,17 +754,6 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
query.map(rs => rs.int(1)).single().apply().getOrElse(0)
}

override def countUnsynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] =
readOnlyTransaction("countUnsynchronizedGroupMemberships", samRequestContext) { implicit session =>
val query = samsql"""select count(distinct g.id) unsynchronizedMembershipCount
from sam_group g
join sam_group_member_flat gmf on g.id = gmf.group_id
where g.synchronized_date is null
and gmf.member_user_id = ${samUser.id}"""

query.map(rs => rs.int(1)).single().apply().getOrElse(0)
}

override def enableIdentity(subject: WorkbenchSubject, samRequestContext: SamRequestContext): IO[Unit] =
subject match {
case userId: WorkbenchUserId =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import spray.json.DefaultJsonProtocol._
import spray.json._

object GroupMembershipCounts {
implicit val GroupMembershipCountsFormat: RootJsonFormat[GroupMembershipCounts] = jsonFormat3(GroupMembershipCounts.apply)
implicit val GroupMembershipCountsFormat: RootJsonFormat[GroupMembershipCounts] = jsonFormat2(GroupMembershipCounts.apply)
}
final case class GroupMembershipCounts(
directSynchronized: Int,
totalSynchronized: Int,
unsynchronized: Int
totalSynchronized: Int
)
Original file line number Diff line number Diff line change
Expand Up @@ -516,9 +516,6 @@ class UserService(
def countIndirectSynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] =
directoryDAO.countIndirectSynchronizedGroupMemberships(samUser, samRequestContext)

def countUnsynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] =
directoryDAO.countUnsynchronizedGroupMemberships(samUser, samRequestContext)

def getSamUserCombinedState(
workbenchEmail: WorkbenchEmail,
samRequestContext: SamRequestContext,
Expand All @@ -536,7 +533,6 @@ class UserService(
maybeAttributes <- getUserAttributes(samUser.id, samRequestContext)
directGroupMemberships <- countDirectSynchronizedGroupMemberships(samUser, samRequestContext)
indirectGroupMemberships <- countIndirectSynchronizedGroupMemberships(samUser, samRequestContext)
unsynchronizedGroupMemberships <- countUnsynchronizedGroupMemberships(samUser, samRequestContext)
termsOfServiceDetails <- tosService.getTermsOfServiceDetailsForUser(samUser.id, samRequestContext)
enterpriseFeatures <- resourceService
.listResourcesFlat(
Expand All @@ -556,8 +552,7 @@ class UserService(
termsOfServiceDetails.getOrElse(TermsOfServiceDetails(None, None, permitsSystemUsage = false, isCurrentVersion = false)),
GroupMembershipCounts(
directSynchronized = directGroupMemberships,
totalSynchronized = indirectGroupMemberships,
unsynchronized = unsynchronizedGroupMemberships
totalSynchronized = indirectGroupMemberships
),
Map("enterpriseFeatures" -> enterpriseFeatures.toJson),
favoriteResources
Expand All @@ -567,6 +562,9 @@ class UserService(

object UserService {

// the "user" resource type is only used for resource_type_admin checks
val userTypeName: ResourceTypeName = ResourceTypeName("user")

val random = SecureRandom.getInstance("NativePRNGNonBlocking")

// from https://www.regular-expressions.info/email.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,6 @@ class MockDirectoryDAO(val groups: mutable.Map[WorkbenchGroupIdentity, Workbench

override def countIndirectSynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] = ???

override def countUnsynchronizedGroupMemberships(samUser: SamUser, samRequestContext: SamRequestContext): IO[Int] = ???

override def loadUserByAzureB2CId(userId: AzureB2CId, samRequestContext: SamRequestContext): IO[Option[SamUser]] =
IO.pure(users.values.find(_.azureB2CId.contains(userId)))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -752,19 +752,6 @@ class PostgresDirectoryDAOSpec extends RetryableAnyFreeSpec with Matchers with B
}
}

"countUnsynchronizedGroupMemberships" - {
"calculate the unsynchronized count" in {
assume(databaseEnabled, databaseEnabledClue)

// create but do not synchronize the test groups;
// countUnsynchronizedGroupMemberships() only counts unsynchronized groups.
createDirectAndIndirectGroups(syncGroups = false)

val unsyncedCount = dao.countUnsynchronizedGroupMemberships(defaultUser, samRequestContext).unsafeRunSync()
unsyncedCount shouldBe 2
}
}

"createPetServiceAccount" - {
"create pet service accounts" in {
assume(databaseEnabled, databaseEnabledClue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,6 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
.doReturn(IO.pure(42))
.when(directoryDAO)
.countIndirectSynchronizedGroupMemberships(any[SamUser], any[SamRequestContext])
lenient()
.doReturn(IO.pure(1234))
.when(directoryDAO)
.countUnsynchronizedGroupMemberships(any[SamUser], any[SamRequestContext])

// TOS
lenient()
Expand All @@ -108,7 +104,7 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
SamUserAllowances(enabled = true, termsOfService = true),
Option(SamUserAttributes(testUser.id, marketingConsent = true)),
TermsOfServiceDetails(Option("v1"), Option(nowInstant), permitsSystemUsage = true, isCurrentVersion = true),
GroupMembershipCounts(7, 42, 1234),
GroupMembershipCounts(7, 42),
Map("enterpriseFeatures" -> FilteredResourcesFlat(Set(enterpriseFeature)).toJson),
favoriteResources
)
Expand All @@ -128,7 +124,6 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
response.favoriteResources should be(favoriteResources)
response.groupMembershipCounts.directSynchronized should be(7)
response.groupMembershipCounts.totalSynchronized should be(42)
response.groupMembershipCounts.unsynchronized should be(1234)
}
it("return null attributes if the user has no attributes") {
// Arrange
Expand All @@ -144,7 +139,7 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
SamUserAllowances(enabled = true, termsOfService = true),
None,
TermsOfServiceDetails(Option("v1"), Option(nowInstant), permitsSystemUsage = true, isCurrentVersion = true),
GroupMembershipCounts(7, 42, 1234),
GroupMembershipCounts(7, 42),
Map("enterpriseFeatures" -> FilteredResourcesFlat(Set(enterpriseFeature)).toJson),
favoriteResources
)
Expand All @@ -164,7 +159,6 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
response.favoriteResources should be(favoriteResources)
response.groupMembershipCounts.directSynchronized should be(7)
response.groupMembershipCounts.totalSynchronized should be(42)
response.groupMembershipCounts.unsynchronized should be(1234)
}
it("return falsy terms of service if the user has no tos history") {
// Arrange
Expand All @@ -182,7 +176,7 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
SamUserAllowances(enabled = true, termsOfService = false),
Option(SamUserAttributes(testUser.id, marketingConsent = true)),
TermsOfServiceDetails(None, None, permitsSystemUsage = false, isCurrentVersion = false),
GroupMembershipCounts(7, 42, 1234),
GroupMembershipCounts(7, 42),
Map("enterpriseFeatures" -> FilteredResourcesFlat(Set(enterpriseFeature)).toJson),
favoriteResources
)
Expand All @@ -202,7 +196,6 @@ class SupportSummarySpec extends UserServiceTestTraits with TimeMatchers {
response.favoriteResources should be(favoriteResources)
response.groupMembershipCounts.directSynchronized should be(7)
response.groupMembershipCounts.totalSynchronized should be(42)
response.groupMembershipCounts.unsynchronized should be(1234)
}
}
}
Loading