From ffc0b9ccb687045f5ccebd75d83a891332180f79 Mon Sep 17 00:00:00 2001 From: Andrew Nowak Date: Thu, 9 Apr 2026 12:08:00 +0100 Subject: [PATCH 1/4] on undeletion, mark image as archived so it cannot be immediately rereaped --- .../com/gu/mediaservice/GridClient.scala | 7 ++++++- .../gr-undelete-image/gr-un-delete-image.js | 1 + media-api/app/controllers/MediaApi.scala | 20 +++++++++++-------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala index c9058ea2850..73bc0d2a378 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala @@ -8,7 +8,7 @@ import com.gu.mediaservice.model.leases.LeasesByMedia import com.gu.mediaservice.model.usage.Usage import com.typesafe.scalalogging.LazyLogging import play.api.http.HeaderNames -import play.api.libs.json.{JsArray, JsObject, JsValue, Json, Reads} +import play.api.libs.json.{JsArray, JsObject, JsTrue, JsValue, Json, Reads} import scala.concurrent.duration.{Duration, DurationInt} import scala.concurrent.{ExecutionContext, Future} @@ -260,6 +260,11 @@ class GridClient(services: Services)(implicit wsClient: WSClient) extends LazyLo authorisedRequest.post(Json.obj("data" -> data)).map { response => validateResponse(response, url)} } + def putArchived(mediaId: String, authFn: WSRequest => WSRequest)(implicit ec: ExecutionContext) = { + val url = new URL(s"${services.metadataBaseUri}/metadata/$mediaId/archived") + val request = authFn(wsClient.url(url.toString)) + request.put(Json.obj("data" -> JsTrue)).map { response => validateResponse(response, url)} + } } class DownstreamApiInBadStateException(message: String, downstreamMessage: String) extends IllegalStateException(message) { diff --git a/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js b/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js index aeff5022cfe..0e25d4be036 100644 --- a/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js +++ b/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js @@ -23,6 +23,7 @@ undeleteImage.controller('grUnDeleteImageCtrl', [ apiPoll(findImage); } + // TODO remove "unusable" banner and recognise that the image is now archived ctrl.unDeleteImage = function (image) { return mediaApi.undelete(image.data.id) .then(() => pollDeleted(image)) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 9c2ad1369a3..7c53dcbcad3 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -404,15 +404,19 @@ class MediaApi( && ImageExtras.userMayUndeleteImage(request.user, image, authorisation) => logger.info(logMarker, s"undeleting image $id") - softDeletedMetadataTable.updateStatus(id, isDeleted = false) - .map { _ => - messageSender.publish( - UpdateMessage( - subject = UnSoftDeleteImage, - id = Some(id) - ) + for { + // Take an "undeletion" as a marker that the file should be kept and not reaped. + // Mark as archived before undeleting, to make sure the reaper doesn't immediately reap it. + _ <- gridClient.putArchived(id, auth.getOnBehalfOfPrincipal(request.user)) + _ <- softDeletedMetadataTable.updateStatus(id, isDeleted = false) + _ = messageSender.publish( + UpdateMessage( + subject = UnSoftDeleteImage, + id = Some(id) ) - }.map { _ => Accepted } + ) + } yield Accepted + case Some(image) if isVisibleToAccessor(request.user, image) => logger.info(logMarker, s"user ${request.user.accessor.identity} was not permitted to undelete image $id") Future.successful(ImageDeleteForbidden) From e9e098aeef166ad2f7c880e7cf77c46e675ba8b3 Mon Sep 17 00:00:00 2001 From: Andrew Nowak Date: Mon, 22 Jun 2026 12:41:36 +0100 Subject: [PATCH 2/4] fix polling for undeletion --- .../js/components/gr-archiver/gr-archiver.js | 26 ++++++++++++--- .../gr-undelete-image/gr-un-delete-image.js | 21 ++++++------ kahuna/public/js/image/controller.js | 7 ++++ kahuna/public/js/search/results.js | 32 +++++++++++-------- 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/kahuna/public/js/components/gr-archiver/gr-archiver.js b/kahuna/public/js/components/gr-archiver/gr-archiver.js index d2a5b4d28c4..0d676369fb4 100644 --- a/kahuna/public/js/components/gr-archiver/gr-archiver.js +++ b/kahuna/public/js/components/gr-archiver/gr-archiver.js @@ -27,6 +27,7 @@ module.controller('grArchiverCtrl', [ 'humanJoin', 'mediaApi', 'getDeletedState', + 'apiPoll', function ($scope, $window, archiveService, @@ -34,7 +35,8 @@ module.controller('grArchiverCtrl', [ imageLogic, humanJoin, mediaApi, - getDeletedState) { + getDeletedState, + apiPoll) { const ctrl = this; @@ -115,14 +117,30 @@ module.controller('grArchiverCtrl', [ reduce((all, items) => all.union(items)). toArray(); } + + function pollUndeleted(imageId) { + const findImage = () => mediaApi.find(imageId).then( + (i) => i.data?.softDeletedMetadata === undefined && i.data.userMetadata?.data?.archived + ? Promise.resolve() + : Promise.reject() + ); + return apiPoll(findImage); + } function undelete() { ctrl.undeleting = true; const imageId = ctrl.image.data.id; mediaApi.undelete(imageId) - .then( - ctrl.canUndelete = ctrl.isDeleted = false - ).catch(() => { + .then(() => pollUndeleted(imageId)) + .then(() => { + ctrl.canUndelete = ctrl.isDeleted = false; + ctrl.image.softDeletedMetadata = undefined; + if (ctrl.image.userMetadata?.data !== undefined) { + ctrl.image.userMetadata.data.archived = true; + } + $scope.$emit('images-updated', [ctrl.image]); + }).catch((e) => { $window.alert('Failed to undelete image!, please try again.'); + console.error('Image undeletion failed', e); }).finally(() => { ctrl.undeleting = false; }); diff --git a/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js b/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js index 0e25d4be036..d9f326f0d4c 100644 --- a/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js +++ b/kahuna/public/js/components/gr-undelete-image/gr-un-delete-image.js @@ -7,28 +7,27 @@ export const undeleteImage = angular.module('gr.undeleteImage', [ ]); undeleteImage.controller('grUnDeleteImageCtrl', [ - '$rootScope', '$q', '$timeout', 'mediaApi', 'apiPoll', - function ($rootScope, $q, $timeout, mediaApi, apiPoll) { + '$rootScope', '$q', 'mediaApi', 'apiPoll', + function ($rootScope, $q, mediaApi, apiPoll) { var ctrl = this; ctrl.$onInit = () => { - function pollDeleted (image) { + function pollUndeleted (image) { const findImage = () => mediaApi.find(image.data.id).then( - () => $q.reject(), - // resolve when image cannot be found, i.e. image has been deleted. - () => $q.resolve() + (i) => i.data.softDeletedMetadata === undefined && i.data.userMetadata?.data?.archived + ? $q.resolve() + : $q.reject() ); - apiPoll(findImage); + return apiPoll(findImage); } - // TODO remove "unusable" banner and recognise that the image is now archived ctrl.unDeleteImage = function (image) { return mediaApi.undelete(image.data.id) - .then(() => pollDeleted(image)) + .then(() => pollUndeleted(image)) .catch((err) => { - $rootScope.$emit('image-delete-failure', err, image); + $rootScope.$emit('image-undelete-failure', err, image); }); }; @@ -36,7 +35,7 @@ undeleteImage.controller('grUnDeleteImageCtrl', [ // HACK to wait for thrall to process the message so that when we // poll the api, it will be up to date. return $q.all(Array.from(ctrl.images.values()).map(image => ctrl.unDeleteImage(image))) - .then(() => $rootScope.$emit('images-deleted', ctrl.images)); + .then(() => $rootScope.$emit('images-undeleted', ctrl.images)); }; }; } diff --git a/kahuna/public/js/image/controller.js b/kahuna/public/js/image/controller.js index c9a42b0b551..e2a6247a64d 100644 --- a/kahuna/public/js/image/controller.js +++ b/kahuna/public/js/image/controller.js @@ -156,6 +156,13 @@ image.controller('ImageCtrl', [ ctrl.image = image; if (ctrl.image && ctrl.image.data.softDeletedMetadata !== undefined) { ctrl.isDeleted = true; } + + $scope.$watch('ctrl.image.data.softDeletedMetadata', () => { + if (ctrl.image) { + ctrl.isDeleted = ctrl.image?.data.softDeletedMetadata !== undefined; + } + }); + ctrl.optimisedImageUri = optimisedImageUri; ctrl.lowResImageUri = lowResImageUri; diff --git a/kahuna/public/js/search/results.js b/kahuna/public/js/search/results.js index 6721bf53ba5..d0665452c9d 100644 --- a/kahuna/public/js/search/results.js +++ b/kahuna/public/js/search/results.js @@ -827,24 +827,27 @@ results.controller('SearchResultsCtrl', [ }); }; - const freeImageDeleteListener = $rootScope.$on('images-deleted', (e, images) => { - images.forEach(image => { - // TODO: should not be needed here, the selection and - // results should listen to these events and update - // itself outside of any controller - ctrl.deselect(image); + const imageDeleteHandler = (_, images) => { + images.forEach(image => { + // TODO: should not be needed here, the selection and + // results should listen to these events and update + // itself outside of any controller + ctrl.deselect(image); - const indexAll = ctrl.imagesAll.findIndex(i => image.data.id === i.data.id); - results.removeAt(indexAll); + const indexAll = ctrl.imagesAll.findIndex(i => image.data.id === i.data.id); + results.removeAt(indexAll); - updateImageArray(ctrl.images, image); - updateImageArray(ctrl.imagesAll, image); + updateImageArray(ctrl.images, image); + updateImageArray(ctrl.imagesAll, image); - updatePositions(image); + updatePositions(image); - ctrl.totalResults--; - }); - }); + ctrl.totalResults--; + }); + }; + + const freeImageDeleteListener = $rootScope.$on('images-deleted', imageDeleteHandler); + const freeImageUndeleteListener = $rootScope.$on('images-undeleted', imageDeleteHandler); // Safer than clearing the timeout in case of race conditions // FIXME: nicer (reactive?) way to do this? @@ -913,6 +916,7 @@ results.controller('SearchResultsCtrl', [ } freeUpdatesListener(); freeImageDeleteListener(); + freeImageUndeleteListener(); scopeGone = true; }); } From e22351726b2c306ebebaae02114c515e43a2411c Mon Sep 17 00:00:00 2001 From: Andrew Nowak Date: Mon, 22 Jun 2026 15:25:33 +0100 Subject: [PATCH 3/4] work around the csrf filter for app-to-app requests by passing along the Origin header from the original request As we pass along the authentication cookie used in the original request, we also need to pass along the original Origin header so that the subsequent request belongs takes the same cross-origin status and passes through the CSRF filter --- .../com/gu/mediaservice/GridClient.scala | 6 ++-- image-loader/app/ImageLoaderComponents.scala | 2 +- media-api/app/controllers/MediaApi.scala | 2 +- .../app/controllers/EditsController.scala | 2 +- .../auth/PandaAuthenticationProvider.scala | 30 ++++++++++++------- thrall/app/ThrallComponents.scala | 2 +- 6 files changed, 27 insertions(+), 17 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala index 73bc0d2a378..bf017e31473 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala @@ -38,8 +38,8 @@ object ClientResponse { case class ClientErrorMessages(errorMessage: String, downstreamErrorMessage: String) object GridClient extends LazyLogging { - def apply(services: Services)(implicit wsClient: WSClient): GridClient = - new GridClient(services) + def apply(services: Services, originUri: String)(implicit wsClient: WSClient): GridClient = + new GridClient(services, originUri) sealed trait Response { def status: Int @@ -96,7 +96,7 @@ object GridClient extends LazyLogging { } -class GridClient(services: Services)(implicit wsClient: WSClient) extends LazyLogging { +class GridClient(services: Services, originDomain: String)(implicit wsClient: WSClient) extends LazyLogging { /* * `requestTimeout` will set the max duration of the request before timing out. You may also want to increase the diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index db2b9a059e3..be7748d9525 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -21,7 +21,7 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne logger.info(s" $index -> ${processor.description}") } - private val gridClient = GridClient(config.services)(wsClient) + private val gridClient = GridClient(config.services, config.services.loaderBaseUri)(wsClient) val store = new ImageLoaderStore(config) val maybeIngestQueue = config.maybeIngestSqsQueueUrl.map(queueUrl => new SimpleSqsMessageConsumer(queueUrl, config)) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 7c53dcbcad3..af13798fa07 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -50,7 +50,7 @@ class MediaApi( )(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition { val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services)(ws) + val gridClient: GridClient = GridClient(services, services.apiBaseUri)(ws) // Process-local cache keyed on normalised query text. Stores the Bedrock Future so that // concurrent requests for the same query share a single in-flight Bedrock call, and diff --git a/metadata-editor/app/controllers/EditsController.scala b/metadata-editor/app/controllers/EditsController.scala index 44be486d4c1..b8585656f52 100644 --- a/metadata-editor/app/controllers/EditsController.scala +++ b/metadata-editor/app/controllers/EditsController.scala @@ -59,7 +59,7 @@ class EditsController( import com.gu.mediaservice.lib.metadata.UsageRightsMetadataMapper.usageRightsToMetadata val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services)(ws) + val gridClient: GridClient = GridClient(services, services.metadataBaseUri)(ws) val metadataBaseUri = config.services.metadataBaseUri private val AuthenticatedAndAuthorised = auth andThen authorisation.CommonActionFilters.authorisedForArchive diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/guardian/auth/PandaAuthenticationProvider.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/guardian/auth/PandaAuthenticationProvider.scala index 00f45b4690a..22d97cd35e3 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/guardian/auth/PandaAuthenticationProvider.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/guardian/auth/PandaAuthenticationProvider.scala @@ -112,6 +112,22 @@ class PandaAuthenticationProvider( override def flushToken: Option[(RequestHeader, Result) => Result] = Some((rh, _) => processLogout(rh)) val PandaCookieKey: TypedKey[Cookie] = TypedKey[Cookie]("PandaCookie") + val OriginKey: TypedKey[String] = TypedKey[String]("Origin") + + private def withPersistedOrigin(request: Principal): WSRequest => WSRequest = { wsRequest => + request.attributes.get(OriginKey) match { + case Some(origin) => wsRequest.withHttpHeaders(("Origin", origin)) + case None => wsRequest + } + } + + private def withPersistedCookie(request: Principal): Either[String, WSRequest => WSRequest] = { + val cookieName = panDomainSettings.settings.cookieSettings.cookieName + request.attributes.get(PandaCookieKey) match { + case Some(cookie) => Right(wsRequest => wsRequest.addCookies(DefaultWSCookie(cookieName, cookie.value))) + case None => Left(s"Pan domain cookie $cookieName is missing in principal.") + } + } /** * A function that allows downstream API calls to be made using the credentials of the inflight request @@ -119,19 +135,13 @@ class PandaAuthenticationProvider( * @param request The request header of the inflight call * @return A function that adds appropriate data to a WSRequest */ - override def onBehalfOf(request: Principal): Either[String, WSRequest => WSRequest] = { - val cookieName = panDomainSettings.settings.cookieSettings.cookieName - request.attributes.get(PandaCookieKey) match { - case Some(cookie) => Right { wsRequest: WSRequest => - wsRequest.addCookies(DefaultWSCookie(cookieName, cookie.value)) - } - case None => Left(s"Pan domain cookie $cookieName is missing in principal.") - } - } + override def onBehalfOf(request: Principal): Either[String, WSRequest => WSRequest] = + withPersistedCookie(request).map(_.andThen(withPersistedOrigin(request))) private def gridUserFrom(pandaUser: User, request: RequestHeader): UserPrincipal = { val maybePandaCookie: Option[TypedEntry[Cookie]] = request.cookies.get(panDomainSettings.settings.cookieSettings.cookieName).map(TypedEntry[Cookie](PandaCookieKey, _)) - val attributes = TypedMap.empty + (maybePandaCookie.toSeq:_*) + val maybeOrigin = request.headers.get("Origin").map(TypedEntry[String](OriginKey, _)) + val attributes = TypedMap(Seq(maybePandaCookie, maybeOrigin).flatten:_*) UserPrincipal( firstName = pandaUser.firstName, lastName = pandaUser.lastName, diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index 781ab62bace..cf9c5e75add 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -31,7 +31,7 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr es.ensureIndexExistsAndAliasAssigned() val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services)(wsClient) + val gridClient: GridClient = GridClient(services, services.thrallBaseUri)(wsClient) // before firing up anything to consume streams or say we are OK let's do the critical good to go check private val goodToGoCheckResult = Await.ready(GoodToGoCheck.run(es), 30 seconds) From bcf6b9b29c3ad333162b466180bd3198ac4a425b Mon Sep 17 00:00:00 2001 From: Andrew Nowak Date: Fri, 26 Jun 2026 14:11:20 +0100 Subject: [PATCH 4/4] make double assignment intent clearer --- kahuna/public/js/components/gr-archiver/gr-archiver.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kahuna/public/js/components/gr-archiver/gr-archiver.js b/kahuna/public/js/components/gr-archiver/gr-archiver.js index 0d676369fb4..dd6633b53d3 100644 --- a/kahuna/public/js/components/gr-archiver/gr-archiver.js +++ b/kahuna/public/js/components/gr-archiver/gr-archiver.js @@ -132,7 +132,8 @@ module.controller('grArchiverCtrl', [ mediaApi.undelete(imageId) .then(() => pollUndeleted(imageId)) .then(() => { - ctrl.canUndelete = ctrl.isDeleted = false; + ctrl.canUndelete = false; + ctrl.isDeleted = false; ctrl.image.softDeletedMetadata = undefined; if (ctrl.image.userMetadata?.data !== undefined) { ctrl.image.userMetadata.data.archived = true;