-
Notifications
You must be signed in to change notification settings - Fork 12
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
Changes from all commits
7994f6d
80aa8d4
8c9af9c
7f38a14
b81101d
78cf618
1279a3b
d159d67
026ce34
af9404a
33a7224
4cff3e4
8b8c6f9
a3bd5ef
a668d3e
a78b1d9
b81e35b
6b5a816
2460032
fdcf6dd
19aaa1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
@@ -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) | ||
} | ||
} | ||
} | ||
} | ||
} ~ | ||
|
@@ -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 | ||
) | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic moved to |
||
private def postUserRegistration(newUser: SamUser, samRequestContext: SamRequestContext): Route = | ||
postWithTelemetry(samRequestContext) { | ||
entity(as[SamUserRegistrationRequest]) { userRegistrationRequest => | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. modeled after |
||
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]] = | ||
|
@@ -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 => | ||
|
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 |
---|---|---|
|
@@ -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 | ||
|
@@ -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]] = | ||
|
@@ -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 | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's kinda smelly to require There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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