Skip to content

Commit

Permalink
Upgrade Editorial permissions library to latest version
Browse files Browse the repository at this point in the history
This upgrades the Media Atom Maker to use the latest version of
the client for the Guardian's Editorial Permissions service - we
need the latest version of the client to support the upgrade to
Scala 2.13 in #1140

* Before: https://github.com/guardian/editorial-permissions-client/tree/v0.8 - supporting Scala 2.11 & 2.12
* After: https://github.com/guardian/permissions/tree/v2.15/client - supporting Scala 2.12 & 2.13

As you can see, the permissions client has moved repositories, to the
main `permissions` repo - this happened in July 2018 with PR
guardian/permissions#103. This PR is also
important because it removed use of `Future` from the permissions
client API - as Michael Barton explained, permission lookups should
be mostly instantaneous because they now come from an in-memory cache.

The removal of `Future` means that this commit, upgrading permissions in
Media Atom Maker, needs to remove several for-comprehensions/map-statements.
The diff on these can look quite big, but they look much smaller if whitespace
changes are ignored.

# Permission to change Privacy Status of a Media Atom

* #789
* #791
  • Loading branch information
rtyley committed Feb 1, 2024
1 parent 1eb384c commit b47b949
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 146 deletions.
19 changes: 5 additions & 14 deletions app/controllers/AtomController.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package controllers

import com.gu.editorial.permissions.client.{Permission, PermissionGranted, PermissionsUser}
import com.gu.media.Permissions._
import com.gu.media.{MediaAtomMakerPermissionsProvider, Permissions}
import com.gu.pandahmac.HMACAuthActions
Expand All @@ -10,6 +9,7 @@ import data.UnpackedDataStores
import play.api.libs.json.{JsObject, JsString}
import play.api.mvc._
import com.gu.media.util.ThriftUtil._
import com.gu.permissions.PermissionDefinition

import scala.concurrent.{ExecutionContext, Future}

Expand Down Expand Up @@ -41,9 +41,7 @@ trait AtomController extends BaseController with UnpackedDataStores {
object LookupPermissions extends ActionBuilder[UploadUserRequest, AnyContent] {
override def invokeBlock[A](request: Request[A], block: UploadUserRequest[A] => Future[Result]): Future[Result] = {
APIAuthAction.invokeBlock(request, { req: UserRequest[A] =>
permissions.getAll(req.user).flatMap { p =>
block(new UploadUserRequest(req.user, request, p))
}
block(UploadUserRequest(req.user, request, permissions.getAll(req.user)))
})
}

Expand All @@ -52,18 +50,11 @@ trait AtomController extends BaseController with UnpackedDataStores {
override protected def executionContext: ExecutionContext = controllerComponents.executionContext
}

class PermissionedAction(permission: Permission) extends ActionBuilder[UserRequest, AnyContent] {
class PermissionedAction(permission: PermissionDefinition) extends ActionBuilder[UserRequest, AnyContent] {
override def invokeBlock[A](request: Request[A], block: UserRequest[A] => Future[Result]): Future[Result] = {
APIAuthAction.invokeBlock(request, { req: UserRequest[A] =>

permissions.get(permission)(PermissionsUser(req.user.email)).flatMap {
case PermissionGranted =>
block(req)

case _ =>
Future.successful(Unauthorized(s"User ${req.user.email} is not authorised for permission ${permission.name}"))

}
if (permissions.hasPermission(permission, req.user)) block(req)
else Future.successful(Unauthorized(s"User ${req.user.email} is not authorised for permission ${permission.name}"))
})
}

Expand Down
62 changes: 30 additions & 32 deletions app/controllers/VideoUIApp.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package controllers


import com.gu.editorial.permissions.client.{Permission, PermissionDenied, PermissionsUser}
import com.gu.media.MediaAtomMakerPermissionsProvider
import com.gu.media.logging.Logging
import com.gu.media.youtube.YouTubeAccess
Expand All @@ -17,13 +16,13 @@ import views.html.helper.CSRF
import scala.concurrent.ExecutionContext

class VideoUIApp(val authActions: HMACAuthActions, conf: Configuration, awsConfig: AWSConfig,
permissions: MediaAtomMakerPermissionsProvider, youtube: YouTubeAccess,
permissionsProvider: MediaAtomMakerPermissionsProvider, youtube: YouTubeAccess,
val controllerComponents: ControllerComponents) extends BaseController with Logging with TrainingMode {
import authActions.AuthAction

implicit lazy val executionContext: ExecutionContext = defaultExecutionContext

def index(id: String = ""): Action[AnyContent] = AuthAction.async { implicit req =>
def index(id: String = ""): Action[AnyContent] = AuthAction { implicit req =>
val isTrainingMode = isInTrainingMode(req)

val jsFileName = "video-ui/build/main.js"
Expand All @@ -43,36 +42,35 @@ class VideoUIApp(val authActions: HMACAuthActions, conf: Configuration, awsConfi

val composerUrl = awsConfig.composerUrl

permissions.getAll(req.user).map { permissions =>
val clientConfig = ClientConfig(
presence = presenceConfig(req.user),
youtubeEmbedUrl = "https://www.youtube.com/embed/",
youtubeThumbnailUrl = "https://img.youtube.com/vi/",
reauthUrl = "/reauth",
gridUrl = awsConfig.gridUrl,
capiProxyUrl = "/support/previewCapi",
liveCapiProxyUrl = "/support/liveCapi",
composerUrl = composerUrl,
ravenUrl = conf.get[String]("raven.url"),
stage = conf.get[String]("stage"),
viewerUrl = awsConfig.viewerUrl,
permissions,
minDurationForAds = youtube.minDurationForAds,
isTrainingMode = isTrainingMode,
workflowUrl = awsConfig.workflowUrl,
targetingUrl = awsConfig.targetingUrl
)
val permissions = permissionsProvider.getAll(req.user)
val clientConfig = ClientConfig(
presence = presenceConfig(req.user),
youtubeEmbedUrl = "https://www.youtube.com/embed/",
youtubeThumbnailUrl = "https://img.youtube.com/vi/",
reauthUrl = "/reauth",
gridUrl = awsConfig.gridUrl,
capiProxyUrl = "/support/previewCapi",
liveCapiProxyUrl = "/support/liveCapi",
composerUrl = composerUrl,
ravenUrl = conf.get[String]("raven.url"),
stage = conf.get[String]("stage"),
viewerUrl = awsConfig.viewerUrl,
permissions,
minDurationForAds = youtube.minDurationForAds,
isTrainingMode = isTrainingMode,
workflowUrl = awsConfig.workflowUrl,
targetingUrl = awsConfig.targetingUrl
)

Ok(views.html.VideoUIApp.app(
title = "Media Atom Maker",
jsLocation,
presenceJsLocation = clientConfig.presence.map(_.jsLocation),
pinboardJsLocation = if(permissions.pinboard) awsConfig.pinboardLoaderUrl else None,
Json.toJson(clientConfig).toString(),
isHotReloading,
CSRF.getToken.value
))
}
Ok(views.html.VideoUIApp.app(
title = "Media Atom Maker",
jsLocation,
presenceJsLocation = clientConfig.presence.map(_.jsLocation),
pinboardJsLocation = if(permissions.pinboard) awsConfig.pinboardLoaderUrl else None,
Json.toJson(clientConfig).toString(),
isHotReloading,
CSRF.getToken.value
))
}

def training(inTraining: Boolean): Action[AnyContent] = AuthAction { req =>
Expand Down
19 changes: 7 additions & 12 deletions app/controllers/Youtube.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,24 @@ import com.gu.media.MediaAtomMakerPermissionsProvider

import scala.concurrent.ExecutionContext.Implicits.global

class Youtube (val authActions: HMACAuthActions, youtube: YouTube, permissions: MediaAtomMakerPermissionsProvider, val controllerComponents: ControllerComponents)
class Youtube (val authActions: HMACAuthActions, youtube: YouTube, permissionsProvider: MediaAtomMakerPermissionsProvider, val controllerComponents: ControllerComponents)
extends BaseController with TrainingMode {
import authActions.AuthAction

def listCategories() = AuthAction {
Ok(Json.toJson(youtube.categories))
}

def listChannels() = AuthAction.async { req =>
val isTrainingMode = isInTrainingMode(req)
def listChannels() = AuthAction { req =>
val user = req.user

permissions.getStatusPermissions(user).map(permissions => {
val hasMakePublicPermission = permissions.setVideosOnAllChannelsPublic
val hasMakePublicPermission = permissionsProvider.getStatusPermissions(user).setVideosOnAllChannelsPublic

val channels = if (isTrainingMode) {
youtube.channelsWithData(hasMakePublicPermission).filter(c => youtube.trainingChannels.contains(c.id))
} else {
youtube.channelsWithData(hasMakePublicPermission).filter(c => youtube.allChannels.contains(c.id))
}
val requiredChannels = if (isInTrainingMode(req)) youtube.trainingChannels else youtube.allChannels

Ok(Json.toJson(channels))
})
val channels = youtube.channelsWithData(hasMakePublicPermission).filter(c => requiredChannels.contains(c.id))

Ok(Json.toJson(channels))
}

def commercialVideoInfo(id: String) = AuthAction {
Expand Down
2 changes: 1 addition & 1 deletion app/di.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class MediaAtomMaker(context: Context)
private val capi = new Capi(config)

private val stores = new DataStores(aws, capi)
private val permissions = new MediaAtomMakerPermissionsProvider(aws.stage, aws.credentials.instance)
private val permissions = new MediaAtomMakerPermissionsProvider(aws.stage, aws.region.getName, aws.credentials.instance)

private val reindexer = buildReindexer()

Expand Down
74 changes: 32 additions & 42 deletions app/model/commands/PublishAtomCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ case class PublishAtomCommand(
youtube: YouTube,
user: PandaUser,
capi: Capi,
permissions: MediaAtomMakerPermissionsProvider,
permissionsProvider: MediaAtomMakerPermissionsProvider,
awsConfig: AWSConfig,
thumbnailGenerator: ThumbnailGenerator)
extends Command with Logging {
Expand Down Expand Up @@ -64,34 +64,31 @@ case class PublishAtomCommand(
}
case (_, _, _) => {
previewAtom.getActiveYouTubeAsset() match {
case Some(asset) => {
case Some(asset) =>
val publishedAtom = getPublishedAtom()

val adSettings = AdSettings(youtube.minDurationForAds, youtube.minDurationForMidroll, previewAtom)
val privacyStatus: Future[PrivacyStatus] = getPrivacyStatus(previewAtom, publishedAtom)

privacyStatus.flatMap(status => {
val updatedPreviewAtom = if (publishedAtom.isDefined) {
previewAtom.copy(
blockAds = adSettings.blockAds,
privacyStatus = Some(status)
)
} else {
// on first publish, set YouTube title and description to that of the Atom
// this is because there's no guarantee that the YouTube furniture gets subbed before publication and can result in draft furniture being used
previewAtom.copy(
blockAds = adSettings.blockAds,
privacyStatus = Some(status),
youtubeTitle = previewAtom.title,
youtubeDescription = YoutubeDescription.clean(previewAtom.description)
)
}

updateYouTube(publishedAtom, updatedPreviewAtom, asset).map(atomWithYoutubeUpdates => {
publish(atomWithYoutubeUpdates, user)
})
})
}
val status = getPrivacyStatus(previewAtom, publishedAtom)

val updatedPreviewAtom = if (publishedAtom.isDefined) {
previewAtom.copy(
blockAds = adSettings.blockAds,
privacyStatus = Some(status)
)
} else {
// on first publish, set YouTube title and description to that of the Atom
// this is because there's no guarantee that the YouTube furniture gets subbed before publication and can result in draft furniture being used
previewAtom.copy(
blockAds = adSettings.blockAds,
privacyStatus = Some(status),
youtubeTitle = previewAtom.title,
youtubeDescription = YoutubeDescription.clean(previewAtom.description)
)
}

updateYouTube(publishedAtom, updatedPreviewAtom, asset).map { atomWithYoutubeUpdates =>
publish(atomWithYoutubeUpdates, user)
}
case _ => Future.successful(publish(previewAtom, user))
}
}
Expand All @@ -107,24 +104,17 @@ case class PublishAtomCommand(
}
}

private def getPrivacyStatus(previewAtom: MediaAtom, maybePublishedAtom: Option[MediaAtom]): Future[PrivacyStatus] = {
(previewAtom.channelId, maybePublishedAtom) match {
case (Some(channel), Some(publishedAtom)) if youtube.channelsRequiringPermission.contains(channel) => {
permissions.getStatusPermissions(user).map(permissions => {
val canChangeVideoStatus = permissions.setVideosOnAllChannelsPublic

if (canChangeVideoStatus) {
previewAtom.privacyStatus.getOrElse(PrivacyStatus.Unlisted)
} else {
// don't have permission to change the status, use the published atom status
publishedAtom.privacyStatus.getOrElse(PrivacyStatus.Unlisted)
}
})
}
case _ => Future.successful(previewAtom.privacyStatus.getOrElse(PrivacyStatus.Unlisted))
}
private def getPrivacyStatus(previewAtom: MediaAtom, maybePublishedAtom: Option[MediaAtom]): PrivacyStatus = {
val atomToReadPrivacyStatusFrom =
if (userCanModifyPrivacyStatusOf(previewAtom, user)) previewAtom else maybePublishedAtom.getOrElse(previewAtom)

atomToReadPrivacyStatusFrom.privacyStatus.getOrElse(PrivacyStatus.Unlisted)
}

private def userCanModifyPrivacyStatusOf(atom: MediaAtom, user: PandaUser): Boolean =
!atom.channelId.exists(youtube.channelsRequiringPermission) ||
permissionsProvider.getStatusPermissions(user).setVideosOnAllChannelsPublic

private def publish(atom: MediaAtom, user: PandaUser): MediaAtom = {
log.info(s"Publishing atom $id")

Expand Down
4 changes: 1 addition & 3 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ val awsLambdaEventsVersion = "1.3.0"
val logbackClassicVersion = "1.2.3"
val logstashLogbackEncoderVersion = "4.8"

val permissionsClientVersion = "0.8"

val guavaVersion = "31.1-jre"
val googleOauthVersion = "1.33.3"
val googleHttpJacksonVersion = "1.41.7"
Expand Down Expand Up @@ -116,7 +114,7 @@ lazy val common = (project in file("common"))
"org.scalacheck" %% "scalacheck" % scalaCheckVersion % "test", // to match ScalaTest version
"com.amazonaws" % "aws-java-sdk-sns" % awsVersion,
"com.amazonaws" % "aws-java-sdk-sqs" % awsVersion,
"com.gu" %% "editorial-permissions-client" % permissionsClientVersion,
"com.gu" %% "editorial-permissions-client" % "2.15",
"com.amazonaws" % "aws-java-sdk-stepfunctions" % awsVersion,
"com.amazonaws" % "aws-java-sdk-ses" % awsVersion,
"com.gu" %% "content-api-client-aws" % capiAwsVersion,
Expand Down
69 changes: 27 additions & 42 deletions common/src/main/scala/com/gu/media/Permissions.scala
Original file line number Diff line number Diff line change
@@ -1,64 +1,49 @@
package com.gu.media

import com.amazonaws.auth.AWSCredentialsProvider
import com.gu.editorial.permissions.client._
import com.gu.permissions._
import ai.x.play.json.Jsonx
import com.gu.media.Permissions.setVideosOnAllChannelsPublic
import play.api.libs.json.Format
import com.gu.pandomainauth.model.{User => PandaUser}
import scala.concurrent.Future
import com.gu.permissions.PermissionDefinition


case class Permissions(
deleteAtom: Boolean,
addSelfHostedAsset: Boolean,
setVideosOnAllChannelsPublic: Boolean,
pinboard: Boolean
deleteAtom: Boolean = false,
addSelfHostedAsset: Boolean = false,
setVideosOnAllChannelsPublic: Boolean = false,
pinboard: Boolean = false
)
object Permissions {
implicit val format: Format[Permissions] = Jsonx.formatCaseClass[Permissions]

val app = "atom-maker"
val deleteAtom = Permission("delete_atom", app, defaultVal = PermissionDenied)
val addSelfHostedAsset = Permission("add_self_hosted_asset", app, defaultVal = PermissionDenied)
val setVideosOnAllChannelsPublic = Permission("set_videos_on_all_channels_public", app, defaultVal = PermissionDenied)
val pinboard = Permission("pinboard", "pinboard", defaultVal = PermissionDenied)
val deleteAtom = PermissionDefinition("delete_atom", app)
val addSelfHostedAsset = PermissionDefinition("add_self_hosted_asset", app)
val setVideosOnAllChannelsPublic = PermissionDefinition("set_videos_on_all_channels_public", app)
val pinboard = PermissionDefinition("pinboard", "pinboard")
}

class MediaAtomMakerPermissionsProvider(stage: String, credsProvider: AWSCredentialsProvider) extends PermissionsProvider {
class MediaAtomMakerPermissionsProvider(stage: String, region: String, credsProvider: AWSCredentialsProvider) {
import Permissions._

implicit def config = PermissionsConfig(
app = "media-atom-maker",
all = Seq(deleteAtom, addSelfHostedAsset, setVideosOnAllChannelsPublic, pinboard),
s3BucketPrefix = if(stage == "PROD") "PROD" else "CODE",
awsCredentials = credsProvider
)
private val permissions: PermissionsProvider = PermissionsProvider(PermissionsConfig(stage, region, credsProvider))

def getAll(user: PandaUser): Future[Permissions] = for {
deleteAtom <- hasPermission(deleteAtom, user)
selfHostedMediaAtom <- hasPermission(addSelfHostedAsset, user)
publicStatusPermissions <- hasPermission(setVideosOnAllChannelsPublic, user)
pinboard <- hasPermission(pinboard, user)
} yield Permissions(deleteAtom, selfHostedMediaAtom, publicStatusPermissions, pinboard)
def getAll(user: PandaUser): Permissions = Permissions(
deleteAtom = hasPermission(deleteAtom, user),
addSelfHostedAsset = hasPermission(addSelfHostedAsset, user),
setVideosOnAllChannelsPublic = hasPermission(setVideosOnAllChannelsPublic, user),
pinboard = hasPermission(pinboard, user)
)

def getStatusPermissions(user: PandaUser): Future[Permissions] = for {
publicStatus <- hasPermission(setVideosOnAllChannelsPublic, user)
} yield {
Permissions(deleteAtom = false, addSelfHostedAsset = false, publicStatus, pinboard = false)
}
def getStatusPermissions(user: PandaUser): Permissions =
Permissions(setVideosOnAllChannelsPublic = hasPermission(setVideosOnAllChannelsPublic, user))

private def hasPermission(permission: Permission, user: PandaUser): Future[Boolean] = {
user.email match {
// TODO be better
// HACK: HMAC authenticated users are a `PandaUser` without an email
case "" if user.firstName == "media-atom-scheduler-lambda" => {
Future.successful(true)
}
case _ => {
get(permission)(PermissionsUser(user.email)).map {
case PermissionGranted => true
case _ => false
}
}
}
def hasPermission(permission: PermissionDefinition, user: PandaUser): Boolean = user.email match {
// TODO be better
// HACK: HMAC authenticated users are a `PandaUser` without an email
case "" if user.firstName == "media-atom-scheduler-lambda" => true
case _ => permissions.hasPermission(permission, user.email)
}
}

0 comments on commit b47b949

Please sign in to comment.