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 API #1617

Merged
merged 21 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
49 changes: 49 additions & 0 deletions src/main/resources/swagger/api-docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,32 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/UserStatus'
/api/admin/v1/user/email/{email}/supportSummary:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally favor the usage of Sam user ids over emails when identifying users, but I expect emails will usually be more convenient for callers of this API. It might be helpful to expose this information via both email and user id in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I am expecting that support will be dealing with email addresses instead of userids - but it would be trivial to add a supportSummary API that accepts an id if we needed one

get:
tags:
- Admin
summary: Get information about a user useful for Terra support
operationId: getAdminUserSummaryInfo
parameters:
- name: email
in: path
description: Email address of user to check
required: true
schema:
type: string
responses:
200:
description: user summary information
content:
application/json:
schema:
$ref: '#/components/schemas/SamUserCombinedState'
500:
description: Internal Server Error
content:
application/json:
schema:
$ref: '#/components/schemas/ErrorReport'
/api/admin/v2/users:
get:
tags:
Expand Down Expand Up @@ -4236,6 +4262,27 @@ components:
type: string
description: The id of the resource
description: type and id of a resource
GroupMembershipCounts:
type: object
properties:
directSynchronized:
type: integer
description: |
Number of groups to which the user is a direct member
and which have been successfully synchronized to the cloud.
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.
Does not include groups from public resources.
ManagedGroupMembershipEntry:
required:
- groupEmail
Expand Down Expand Up @@ -4904,6 +4951,8 @@ components:
type: object
description: >
additional details about the user, such as enterprise features they have access to example: '{ "enterpriseFeatures": ["feature1", "feature2"] }'
groupMemberhipCounts:
$ref: '#/components/schemas/GroupMembershipCounts'
SamUserRegistrationRequest:
type: object
properties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,24 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi
def adminUserRoutes(samUser: SamUser, samRequestContext: SamRequestContext): server.Route =
pathPrefix("user") {
asWorkbenchAdmin(samUser) {
path("email" / Segment) { email =>
pathPrefix("email" / Segment) { email =>
val workbenchEmail = WorkbenchEmail(email)
getWithTelemetry(samRequestContext, emailParam(workbenchEmail)) {
complete {
userService
.getUserStatusFromEmail(workbenchEmail, samRequestContext)
.map(status => (if (status.isDefined) OK else NotFound) -> status)
pathEndOrSingleSlash {
getWithTelemetry(samRequestContext, emailParam(workbenchEmail)) {
complete {
userService
.getUserStatusFromEmail(workbenchEmail, samRequestContext)
.map(status => (if (status.isDefined) OK else NotFound) -> status)
}
}
} ~
pathPrefix("supportSummary") {
pathEndOrSingleSlash {
getWithTelemetry(samRequestContext, emailParam(workbenchEmail)) {
complete {
userService.getSamUserCombinedState(workbenchEmail, samRequestContext, resourceService)
}
}
}
}
} ~
Expand Down Expand Up @@ -284,4 +295,5 @@ trait AdminRoutes extends SecurityDirectives with SamRequestContextDirectives wi
user.id,
samRequestContext
)

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.sam.model.api.SamUserResponse._
import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._
import org.broadinstitute.dsde.workbench.sam.model.api._
import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, ResourceId, ResourceRoleName, ResourceTypeName, TermsOfServiceDetails}
import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, ResourceId, ResourceTypeName}
import org.broadinstitute.dsde.workbench.sam.service.{ResourceService, TosService, UserService}
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
import spray.json.enrichAny
import spray.json.DefaultJsonProtocol._

/** Created by tlangs on 10/12/2023.
Expand Down Expand Up @@ -80,7 +79,11 @@ trait UserRoutesV2 extends SamUserDirectives with SamRequestContextDirectives wi
withUserAllowInactive(samRequestContextWithoutUser) { samUser: SamUser =>
val samRequestContext = samRequestContextWithoutUser.copy(samUser = Some(samUser))
pathEndOrSingleSlash {
getSamUserCombinedState(samUser, samRequestContext)
get {
complete {
userService.getSamUserCombinedState(samUser, samRequestContext, resourceService)
}
}
}
}
} ~
Expand Down Expand Up @@ -200,35 +203,6 @@ trait UserRoutesV2 extends SamUserDirectives with SamRequestContextDirectives wi
}
}

private def getSamUserCombinedState(samUser: SamUser, samRequestContext: SamRequestContext): Route =
get {
complete {
for {
allowances <- userService.getUserAllowances(samUser, samRequestContext)
maybeAttributes <- userService.getUserAttributes(samUser.id, samRequestContext)
termsOfServiceDetails <- tosService.getTermsOfServiceDetailsForUser(samUser.id, samRequestContext)
enterpriseFeatures <- resourceService
.listResourcesFlat(
samUser.id,
Set(ResourceTypeName("enterprise-feature")),
Set.empty,
Set(ResourceRoleName("user")),
Set.empty,
includePublic = false,
samRequestContext
)
favoriteResources <- resourceService.getUserFavoriteResources(samUser.id, samRequestContext)
} yield SamUserCombinedStateResponse(
samUser,
allowances,
maybeAttributes,
termsOfServiceDetails.getOrElse(TermsOfServiceDetails(None, None, permitsSystemUsage = false, isCurrentVersion = false)),
Map("enterpriseFeatures" -> enterpriseFeatures.toJson),
favoriteResources
)
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic moved to UserService to make it reusable

private def postUserRegistration(newUser: SamUser, samRequestContext: SamRequestContext): Route =
postWithTelemetry(samRequestContext) {
entity(as[SamUserRegistrationRequest]) { userRegistrationRequest =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ trait DirectoryDAO {

def setUserAzureB2CId(userId: WorkbenchUserId, b2cId: AzureB2CId, samRequestContext: SamRequestContext): IO[Unit]

def loadUserByEmail(email: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Option[SamUser]]

def updateUserEmail(userId: WorkbenchUserId, email: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Unit]

def deleteUser(userId: WorkbenchUserId, samRequestContext: SamRequestContext): IO[Unit]
Expand All @@ -98,6 +100,12 @@ trait DirectoryDAO {

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

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

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 @@ -528,6 +528,20 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
}
}

override def loadUserByEmail(email: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Option[SamUser]] =
readOnlyTransaction("loadUserByEmail", samRequestContext) { implicit session =>
val userTable = UserTable.syntax

val loadUserQuery = samsql"""select ${userTable.resultAll}
from ${UserTable as userTable}
where ${userTable.email} = ${email}"""
loadUserQuery
.map(UserTable(userTable))
.single()
.apply()
.map(UserTable.unmarshalUserRecord)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modeled after loadUserByAzureB2CId

override def updateUserEmail(userId: WorkbenchUserId, email: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Unit] = IO.unit

override def updateUser(samUser: SamUser, userUpdate: AdminUpdateUserRequest, samRequestContext: SamRequestContext): IO[Option[SamUser]] =
Expand Down Expand Up @@ -718,6 +732,39 @@ class PostgresDirectoryDAO(protected val writeDbRef: DbReference, protected val
}
}

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

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

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

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

davidangb marked this conversation as resolved.
Show resolved Hide resolved
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
@@ -0,0 +1,13 @@
package org.broadinstitute.dsde.workbench.sam.model.api

import spray.json.DefaultJsonProtocol._
import spray.json._

object GroupMembershipCounts {
implicit val GroupMembershipCountsFormat: RootJsonFormat[GroupMembershipCounts] = jsonFormat3(GroupMembershipCounts.apply)
}
final case class GroupMembershipCounts(
directSynchronized: Int,
totalSynchronized: Int,
unsynchronized: Int
)
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ package org.broadinstitute.dsde.workbench.sam.model.api
import org.broadinstitute.dsde.workbench.sam.model.{FullyQualifiedResourceId, TermsOfServiceDetails}
import spray.json.DefaultJsonProtocol._
import spray.json._
import org.broadinstitute.dsde.workbench.sam.model.api.GroupMembershipCounts.GroupMembershipCountsFormat
import org.broadinstitute.dsde.workbench.sam.model.api.SamUserAllowances.SamUserAllowedResponseFormat
import org.broadinstitute.dsde.workbench.sam.model.api.SamUserAttributes.SamUserAttributesFormat
import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._

object SamUserCombinedStateResponse {
implicit val SamUserResponseFormat: RootJsonFormat[SamUserCombinedStateResponse] = jsonFormat6(SamUserCombinedStateResponse.apply)
implicit val SamUserResponseFormat: RootJsonFormat[SamUserCombinedStateResponse] = jsonFormat7(SamUserCombinedStateResponse.apply)
}
final case class SamUserCombinedStateResponse(
samUser: SamUser,
allowances: SamUserAllowances,
attributes: Option[SamUserAttributes],
termsOfServiceDetails: TermsOfServiceDetails,
groupMembershipCounts: GroupMembershipCounts,
additionalDetails: Map[String, JsValue],
favoriteResources: Set[FullyQualifiedResourceId]
)
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.broadinstitute.dsde.workbench.sam.model.api._
import org.broadinstitute.dsde.workbench.sam.service.UserService.genWorkbenchUserId
import org.broadinstitute.dsde.workbench.sam.util.AsyncLogging.IOWithLogging
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
import spray.json.enrichAny

import java.security.SecureRandom
import java.time.Instant
Expand Down Expand Up @@ -279,6 +280,9 @@ class UserService(
def getUserFromPetManagedIdentity(petManagedIdentityObjectId: ManagedIdentityObjectId, samRequestContext: SamRequestContext): IO[Option[SamUser]] =
directoryDAO.getUserFromPetManagedIdentity(petManagedIdentityObjectId, samRequestContext)

def getUserFromEmail(email: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Option[SamUser]] =
directoryDAO.loadUserByEmail(email, samRequestContext)

def getSubjectFromGoogleSubjectId(googleSubjectId: GoogleSubjectId, samRequestContext: SamRequestContext): IO[Option[WorkbenchSubject]] =
directoryDAO.loadSubjectFromGoogleSubjectId(googleSubjectId, samRequestContext)
def getSubjectFromEmail(email: WorkbenchEmail, samRequestContext: SamRequestContext): IO[Option[WorkbenchSubject]] =
Expand Down Expand Up @@ -505,6 +509,60 @@ class UserService(
case None => IO.raiseError(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.NotFound, s"User $workbenchUserId not found")))
}
}

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

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,
resourceService: ResourceService
): IO[SamUserCombinedStateResponse] =
for {
samUserOption <- getUserFromEmail(workbenchEmail, samRequestContext)
samUser = samUserOption.getOrElse(throw new WorkbenchException("user not found"))
combinedState <- getSamUserCombinedState(samUser, samRequestContext, resourceService)
} yield combinedState

def getSamUserCombinedState(samUser: SamUser, samRequestContext: SamRequestContext, resourceService: ResourceService): IO[SamUserCombinedStateResponse] =
for {
allowances <- getUserAllowances(samUser, samRequestContext)
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(
samUser.id,
Set(ResourceTypeName("enterprise-feature")),
Set.empty,
Set(ResourceRoleName("user")),
Set.empty,
includePublic = false,
samRequestContext
)
favoriteResources <- resourceService.getUserFavoriteResources(samUser.id, samRequestContext)
} yield SamUserCombinedStateResponse(
samUser,
allowances,
maybeAttributes,
termsOfServiceDetails.getOrElse(TermsOfServiceDetails(None, None, permitsSystemUsage = false, isCurrentVersion = false)),
GroupMembershipCounts(
directSynchronized = directGroupMemberships,
totalSynchronized = indirectGroupMemberships,
unsynchronized = unsynchronizedGroupMemberships
),
Map("enterpriseFeatures" -> enterpriseFeatures.toJson),
favoriteResources
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's kinda smelly to require ResourceService as an argument to this method. But I also didn't want to update all 67 calls to the UserService constructor just to support this one method.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I agree. An alternative would be to create another Service class that depends on both but 🤷 . I leave it up to you.

}

object UserService {
Expand Down
Loading
Loading