Skip to content

Commit

Permalink
Merge pull request #1610 from gemini-hlsw/sc-3897-multiple-goa-users
Browse files Browse the repository at this point in the history
ShortCut 3897: allow multiple GOA users and staff
  • Loading branch information
swalker2m authored Feb 12, 2025
2 parents 0b54a24 + e74bb85 commit 12ad0cb
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9746,7 +9746,7 @@ type Query {
has GOA data-download access privileges. These will be those for which the
user is a ProgramUser of any role with the `hasDataAccess` flag set.
This query is for use by the GOA and will fail for other users.
This query is for use by staff and the GOA and will fail for other users.
"""
goaDataDownloadAccess(orcidId: String!): [ProgramReferenceLabel!]!

Expand Down
10 changes: 7 additions & 3 deletions modules/service/src/main/scala/lucuma/odb/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ case class Config(
corsOverHttps: Boolean, // Whether to require CORS over HTTPS
domain: List[String], // Domains, for CORS headers
commitHash: CommitHash, // From Heroku Dyno Metadata
goaUser: Option[User.Id] // Gemini Observatory Archive user id
goaUsers: Set[User.Id] // Gemini Observatory Archive user id(s)
) {

// People send us their JWTs. We need to be able to extract them from the request, decode them,
Expand Down Expand Up @@ -277,6 +277,10 @@ object Config {
private given [A](using Gid[A]): ConfigDecoder[String, A] =
ConfigDecoder[String].mapOption("Gid[A]")(Gid[A].fromString.getOption)

private given [A](using ConfigDecoder[String, A]): ConfigDecoder[String, List[A]] =
ConfigDecoder[String].map(_.split(",").map(_.trim).toList).mapEither: (key, ss) =>
ss.traverse(ConfigDecoder[String, A].decode(key, _))

private def envOrProp(name: String): ConfigValue[Effect, String] =
env(name) or prop(name)

Expand All @@ -290,9 +294,9 @@ object Config {
Aws.fromCiris,
Email.fromCirrus,
envOrProp("CORS_OVER_HTTPS").as[Boolean].default(true), // By default require https
envOrProp("ODB_DOMAIN").map(_.split(",").map(_.trim).toList).as[List[String]],
envOrProp("ODB_DOMAIN").as[List[String]],
envOrProp("HEROKU_SLUG_COMMIT").as[CommitHash].default(CommitHash.Zero),
envOrProp("GOA_USER_ID").as[User.Id].option
envOrProp("GOA_USER_IDS").as[List[User.Id]].map(_.toSet).default(Set.empty)
).parMapN(Config.apply)

}
Expand Down
6 changes: 3 additions & 3 deletions modules/service/src/main/scala/lucuma/odb/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ object FMain extends MainParams {
config.email,
config.itcClient,
config.commitHash,
config.goaUser,
config.goaUsers,
config.ssoClient,
config.corsOverHttps,
config.domain,
Expand All @@ -223,7 +223,7 @@ object FMain extends MainParams {
emailConfig: Config.Email,
itcClientResource: Resource[F, ItcClient[F]],
commitHash: CommitHash,
goaUser: Option[User.Id],
goaUsers: Set[User.Id],
ssoClientResource: Resource[F, SsoClient[F, User]],
corsOverHttps: Boolean,
domain: List[String],
Expand All @@ -240,7 +240,7 @@ object FMain extends MainParams {
middleware <- Resource.eval(ServerMiddleware(corsOverHttps, domain, ssoClient, userSvc))
enums <- Resource.eval(pool.use(Enums.load))
ptc <- Resource.eval(pool.use(TimeEstimateCalculatorImplementation.fromSession(_, enums)))
graphQLRoutes <- GraphQLRoutes(itcClient, commitHash, goaUser, ssoClient, pool, SkunkMonitor.noopMonitor[F], GraphQLServiceTTL, userSvc, enums, ptc, httpClient, emailConfig)
graphQLRoutes <- GraphQLRoutes(itcClient, commitHash, goaUsers, ssoClient, pool, SkunkMonitor.noopMonitor[F], GraphQLServiceTTL, userSvc, enums, ptc, httpClient, emailConfig)
s3ClientOps <- s3OpsResource
s3Presigner <- s3PresignerResource
s3FileService = S3FileService.fromS3ConfigAndClient(awsConfig, s3ClientOps, s3Presigner)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object GraphQLRoutes {
def apply[F[_]: Async: Parallel: Trace: Logger](
itcClient: ItcClient[F],
commitHash: CommitHash,
goaUser: Option[User.Id],
goaUsers: Set[User.Id],
ssoClient: SsoClient[F, User],
pool: Resource[F, Session[F]],
monitor: SkunkMonitor[F],
Expand Down Expand Up @@ -103,7 +103,7 @@ object GraphQLRoutes {
_ <- OptionT.liftF(Services.asSuperUser(userSvc.canonicalizeUser(user).retryOnInvalidCursorName))

_ <- OptionT.liftF(info(user, s"New service instance."))
map = OdbMapping(pool, monitor, user, topics, itcClient, commitHash, goaUser, enums, ptc, httpClient, emailConfig)
map = OdbMapping(pool, monitor, user, topics, itcClient, commitHash, goaUsers, enums, ptc, httpClient, emailConfig)
svc = new GraphQLService(map) {
override def query(request: Operation): F[Result[Json]] =
super.query(request).retryOnInvalidCursorName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ object OdbMapping {
topics0: Topics[F],
itcClient0: ItcClient[F],
commitHash0: CommitHash,
goaUser0: Option[User.Id],
goaUsers0: Set[User.Id],
enums: Enums,
tec: TimeEstimateCalculatorImplementation.ForInstrumentMode,
httpClient0: Client[F],
Expand Down Expand Up @@ -263,7 +263,7 @@ object OdbMapping {

// Our services and resources needed by various mappings.
override val commitHash = commitHash0
override val goaUser = goaUser0
override val goaUsers = goaUsers0
override val itcClient = itcClient0
override val user: User = user0
override val topics: Topics[F] = topics0
Expand All @@ -280,7 +280,7 @@ object OdbMapping {
topics0,
itcClient0,
commitHash0,
goaUser0,
goaUsers0,
enums,
tec,
httpClient0,
Expand Down Expand Up @@ -581,7 +581,7 @@ object OdbMapping {
def timeEstimateCalculator = sys.error("OdbMapping.forMetadata: no timeEstimateCalculator available")
def itcClient = sys.error("OdbMapping.forMetadata: no itcClient available")
def commitHash = sys.error("OdbMapping.forMetadata: no commitHash available")
def goaUser = sys.error("OdbMapping.forMetadata: no goaUser available")
def goaUsers = sys.error("OdbMapping.forMetadata: no goaUsers available")

// Our schema
val schema: Schema =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package lucuma.odb.graphql

package mapping

import cats.Order.catsKernelOrderingForOrder
import cats.effect.Resource
import cats.syntax.all.*
import grackle.Context
Expand All @@ -25,6 +26,7 @@ import io.circe.Json
import io.circe.syntax.*
import lucuma.core.enums.ObservationWorkflowState
import lucuma.core.model
import lucuma.core.model.Access
import lucuma.core.model.CallForProposals
import lucuma.core.model.ConfigurationRequest
import lucuma.core.model.ObservationReference
Expand Down Expand Up @@ -69,7 +71,7 @@ trait QueryMapping[F[_]] extends Predicates[F] {
def timeEstimateCalculator: TimeEstimateCalculatorImplementation.ForInstrumentMode
def itcClient: ItcClient[F]
def commitHash: CommitHash
def goaUser: Option[User.Id]
def goaUsers: Set[User.Id]

lazy val QueryMapping: ObjectMapping =
ObjectMapping(QueryType)(
Expand Down Expand Up @@ -196,8 +198,8 @@ trait QueryMapping[F[_]] extends Predicates[F] {
private val goaDataDownloadAccess: (Path, Env) => F[Result[Json]] = (p, e) =>
val notAuthorized = OdbError.NotAuthorized(user.id, "Only the GOA user may access this field.".some).asFailure
val goaUserCheck = user match
case StandardUser(id, _, _, _) => notAuthorized.unlessA(goaUser.contains(id))
case _ => notAuthorized
case StandardUser(id, r, rs, _) => notAuthorized.unlessA(goaUsers.contains(id) || ((r::rs).map(_.access).max >= Access.Staff))
case _ => notAuthorized

def decode(r: Json): Result[List[ProgramReference]] =
r.hcursor.downFields("programUsers", "matches").values.toList.flatMap(_.toList).traverse: json =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,8 @@ abstract class OdbSuite(debug: Boolean = false) extends CatsEffectSuite with Tes
protected def s3PresignerResource: Resource[IO, S3Presigner] =
S3FileService.s3PresignerResource[IO](awsConfig)

protected def goaUser: Option[User.Id] =
none
protected def goaUsers: Set[User.Id] =
Set.empty

private def httpApp: Resource[IO, WebSocketBuilder2[IO] => HttpApp[IO]] =
FMain.routesResource[IO](
Expand All @@ -303,7 +303,7 @@ abstract class OdbSuite(debug: Boolean = false) extends CatsEffectSuite with Tes
emailConfig,
itcClient.pure[Resource[IO, *]],
CommitHash.Zero,
goaUser,
goaUsers,
ssoClient.pure[Resource[IO, *]],
true,
List("unused"),
Expand All @@ -322,7 +322,7 @@ abstract class OdbSuite(debug: Boolean = false) extends CatsEffectSuite with Tes
itc = itcClient
enm <- db.evalMap(Enums.load)
ptc <- db.evalMap(TimeEstimateCalculatorImplementation.fromSession(_, enm))
map = OdbMapping(db, mon, usr, top, itc, CommitHash.Zero, goaUser, enm, ptc, httpClient, emailConfig)
map = OdbMapping(db, mon, usr, top, itc, CommitHash.Zero, goaUsers, enm, ptc, httpClient, emailConfig)
} yield map

protected def server: Resource[IO, Server] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,22 @@ class goaAccessQuery extends OdbSuite:
val pi4 = TestUsers.Standard.pi(4, 34)
val staff = TestUsers.Standard.staff(10, 100)

val goa = TestUsers.Standard(
val goa1 = TestUsers.Standard(
20,
StandardRole.Pi(Gid[StandardRole.Id].fromLong.getOption(200).get),
email = "[email protected]".some
)

override protected def goaUser: Option[User.Id] =
goa.id.some
val goa2 = TestUsers.Standard(
21,
StandardRole.Pi(Gid[StandardRole.Id].fromLong.getOption(201).get),
email = "[email protected]".some
)

override protected def goaUsers: Set[User.Id] =
Set(goa1.id, goa2.id)

val validUsers = List(pi, pi2, pi3, pi4, staff, goa).toList
val validUsers = List(pi, pi2, pi3, pi4, staff, goa1, goa2).toList

val sem2025B = Semester.unsafeFromString("2025B")

Expand Down Expand Up @@ -69,7 +75,7 @@ class goaAccessQuery extends OdbSuite:
for
ref <- setup(none)
_ <- expect(
user = goa,
user = goa1,
query = queryString(pi),
expected =
json"""
Expand All @@ -82,7 +88,7 @@ class goaAccessQuery extends OdbSuite:
)
yield ()

test("only GOA user may perform this query"):
test("non-GOA, non-staff users may not perform this query"):
for
_ <- setup(none)
_ <- expect(
Expand All @@ -96,7 +102,7 @@ class goaAccessQuery extends OdbSuite:
for
ref <- setup((pi2, ProgramUserRole.External).some)
_ <- expect(
user = goa,
user = goa2,
query = queryString(pi2),
expected =
json"""
Expand All @@ -113,7 +119,7 @@ class goaAccessQuery extends OdbSuite:
for
ref <- setup((pi3, ProgramUserRole.Coi).some)
_ <- expect(
user = goa,
user = goa1,
query = queryString(pi3),
expected =
json"""
Expand Down Expand Up @@ -156,7 +162,7 @@ class goaAccessQuery extends OdbSuite:
"""
).void
_ <- expect(
user = goa,
user = goa2,
query = queryString(pi4),
expected =
json"""
Expand All @@ -168,18 +174,27 @@ class goaAccessQuery extends OdbSuite:
)
yield ()

test("GOA data access: multiple"):
def label(idx: Int): String =
ProgramReference.fromString.unsafeGet(s"G-2025B-000$idx-Q").label
private def label(idx: Int): String =
ProgramReference.fromString.unsafeGet(s"G-2025B-000$idx-Q").label

val labels = (1 to 5).map(label).asJson
test("GOA data access: multiple"):
expect(
user = goa1,
query = queryString(pi),
expected = json"""
{
"goaDataDownloadAccess": ${(1 to 5).map(label).asJson}
}
""".asRight
)

test("GOA data access: by staff"):
expect(
user = goa,
user = staff,
query = queryString(pi),
expected = json"""
{
"goaDataDownloadAccess": $labels
"goaDataDownloadAccess": ${(1 to 5).map(label).asJson}
}
""".asRight
)

0 comments on commit 12ad0cb

Please sign in to comment.