Skip to content
Open
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
12 changes: 10 additions & 2 deletions api/src/changeRequests/documentProcessing/processImageDto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ async function migrateImagesBetweenBuckets(
* Processes an embedded image upload by resizing the image and uploading to S3
* Requires bucket-specific credentials configured at the post/tag level
* Bucket ID is passed from the parent post/tag document for consistency
* `sourceBucketId` is where a duplicated image's files are read from, resolved from the source
* document rather than the incoming one.
* Returns object with migration failure status and warnings
*/
export async function processImage(
Expand All @@ -195,20 +197,24 @@ export async function processImage(
db: DbService,
parentBucketId?: string,
prevParentBucketId?: string,
sourceBucketId?: string,
): Promise<{ migrationFailed: boolean; warnings: string[] }> {
const warnings: string[] = [];
let migrationFailed = false;
let duplicatedNow = false;

try {
if (image.duplicate && image.fileCollections.length > 0) {
if ((image.duplicateFrom || image.duplicate) && image.fileCollections.length > 0) {
const prevHasImageFiles = !!prevImage?.fileCollections?.some(
(collection) => collection.imageFiles?.length > 0,
);

if (prevHasImageFiles) {
image.fileCollections = prevImage.fileCollections;
} else {
// Copy out of the bucket the source files actually live in. It falls back to the
// target bucket for older clients, which only send `duplicate` and no source id.
const copyFromBucketId = sourceBucketId || parentBucketId;
if (!parentBucketId) {
warnings.push("Parent bucket ID is required for duplicated image copy.");
return { migrationFailed, warnings };
Expand All @@ -217,7 +223,7 @@ export async function processImage(
const duplicateResult = await duplicateImageFilesWithoutReencoding(
image,
db,
parentBucketId,
copyFromBucketId,
parentBucketId,
);
warnings.push(...duplicateResult.warnings);
Expand All @@ -226,6 +232,7 @@ export async function processImage(
// Avoid persisting stale source filenames when copy fails.
image.fileCollections = [];
delete image.duplicate;
delete image.duplicateFrom;
return { migrationFailed, warnings };
}
duplicatedNow = true;
Expand Down Expand Up @@ -323,6 +330,7 @@ export async function processImage(
}

delete image.duplicate;
delete image.duplicateFrom;
} catch (error) {
warnings.push(`Image processing failed: ${error.message}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ describe("processPostTagDto", () => {
await processChangeRequest("test-user", postCr, ["group-super-admins"], db);
await processChangeRequest("test-user", contentCr, ["group-super-admins"], db);


let contentRes = await db.getDoc(contentCr.doc._id);
expect(contentRes.docs[0].parentAlwaysOffline).toBeUndefined();

Expand Down Expand Up @@ -282,6 +281,7 @@ describe("processPostTagDto", () => {
db,
(changeRequest.doc as PostDto).imageBucketId,
undefined,
undefined, // no duplication source bucket
);
});

Expand Down Expand Up @@ -441,6 +441,7 @@ describe("processPostTagDto", () => {
db,
undefined, // no bucketId
undefined, // no prevBucketId
undefined, // no duplication source bucket
);
expect(result.warnings).toContain("Bucket is not specified for image processing.");
});
Expand Down Expand Up @@ -638,4 +639,88 @@ describe("processPostTagDto", () => {
(changeRequest.doc as PostDto).mediaBucketId,
);
});
it("resolves the source bucket and files from the document named by duplicateFrom", async () => {
const sourceRequest = changeRequest_post();
sourceRequest.doc._id = "test-duplicate-source-post";
(sourceRequest.doc as any).imageBucketId = "storage-bucket-1";
(sourceRequest.doc as any).imageData = {
fileCollections: [
{
aspectRatio: 1.5,
imageFiles: [{ width: 100, height: 66, filename: "src.webp" }],
},
],
};
await db.upsertDoc(sourceRequest.doc);

// The duplicate carries neither a bucket nor the source files — the API supplies both.
const duplicateRequest = changeRequest_post();
duplicateRequest.doc._id = "test-duplicate-target-post";
delete (duplicateRequest.doc as any).imageBucketId;
(duplicateRequest.doc as any).imageData = {
fileCollections: [],
duplicateFrom: "test-duplicate-source-post",
};

await processChangeRequest(
"test-user",
duplicateRequest as ChangeReqDto,
["group-super-admins"],
db,
);

const stored = (await db.getDoc("test-duplicate-target-post")).docs[0] as PostDto;
expect(stored.imageBucketId).toBe("storage-bucket-1");

expect(processImage).toHaveBeenCalledWith(
expect.objectContaining({
fileCollections: (sourceRequest.doc as any).imageData.fileCollections,
}),
undefined,
db,
"storage-bucket-1",
undefined,
"storage-bucket-1",
);
});

it("keeps the duplicate's own bucket as the copy target when it has one", async () => {
const sourceRequest = changeRequest_post();
sourceRequest.doc._id = "test-duplicate-source-post-2";
(sourceRequest.doc as any).imageBucketId = "storage-bucket-1";
(sourceRequest.doc as any).imageData = {
fileCollections: [
{
aspectRatio: 1.5,
imageFiles: [{ width: 100, height: 66, filename: "src.webp" }],
},
],
};
await db.upsertDoc(sourceRequest.doc);

const duplicateRequest = changeRequest_post();
duplicateRequest.doc._id = "test-duplicate-target-post-2";
(duplicateRequest.doc as any).imageBucketId = "storage-bucket-2";
(duplicateRequest.doc as any).imageData = {
fileCollections: [],
duplicateFrom: "test-duplicate-source-post-2",
};

await processChangeRequest(
"test-user",
duplicateRequest as ChangeReqDto,
["group-super-admins"],
db,
);

// Source bucket is read from, the duplicate's own bucket is written to.
expect(processImage).toHaveBeenCalledWith(
expect.anything(),
undefined,
db,
"storage-bucket-2",
undefined,
"storage-bucket-1",
);
});
});
21 changes: 20 additions & 1 deletion api/src/changeRequests/documentProcessing/processPostTagDto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,37 @@ export default async function processPostTagDto(
if (doc.imageData) {
let imageWarnings: string[] = [];

// The source document is authoritative for where a duplicated image's files live and what
// they are, so a client that has lost or never synced its own bucket reference still
// produces a correct copy.
let sourceBucketId: Uuid | undefined;
if (doc.imageData.duplicateFrom) {
const sourceDoc = (await db.getDoc(doc.imageData.duplicateFrom)).docs[0] as
| PostDto
| TagDto
| undefined;
sourceBucketId = sourceDoc?.imageBucketId;
if (sourceDoc?.imageData?.fileCollections?.length) {
doc.imageData.fileCollections = sourceDoc.imageData.fileCollections;
}
// The copy stays in the source bucket unless the duplicate names its own.
if (!doc.imageBucketId) doc.imageBucketId = sourceBucketId;
}

if (!doc.imageBucketId) {
imageWarnings.push("Bucket is not specified for image processing.");
}

// prevDoc is undefined on first upsert. A duplication request must include
// existing file references and a source bucket on the parent document.
if (!prevDoc && doc.imageData.duplicate) {
if (!prevDoc && (doc.imageData.duplicateFrom || doc.imageData.duplicate)) {
const hasSourceFiles = doc.imageData.fileCollections?.some(
(collection) => collection.imageFiles?.length > 0,
);
if (!doc.imageBucketId || !hasSourceFiles) {
imageWarnings.push("Image duplication request is invalid.");
delete doc.imageData.duplicate;
delete doc.imageData.duplicateFrom;
doc.imageData.fileCollections = [];
}
}
Expand All @@ -80,6 +98,7 @@ export default async function processPostTagDto(
db,
doc.imageBucketId,
prevDoc?.imageBucketId, // Pass previous bucket ID for migration
sourceBucketId,
);
imageWarnings = result.warnings;

Expand Down
47 changes: 47 additions & 0 deletions api/src/changeRequests/validateChangeRequestAccess.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -809,4 +809,51 @@ describe("validateChangeRequestAccess", () => {
expect(res.validated).toBe(true);
});
});
describe("image duplication source access", () => {
// An existing public post is saved so the request clears the edit and group assign checks,
// leaving the duplication source as the only difference between these tests.
const duplicateChangeReq = (duplicateFrom: string) =>
plainToClass(ChangeReqDto, {
doc: {
_id: "post-page1",
type: "post",
memberOf: ["group-public-content"],
image: "",
tags: [],
imageData: { fileCollections: [], duplicateFrom },
},
});

it("rejects a duplicate naming a source the user cannot view", async () => {
const res = await validateChangeRequestAccess(
duplicateChangeReq("post-blog2"),
["group-public-editors"],
db,
);

expect(res.validated).toBe(false);
expect(res.error).toBe("No 'View' access to the document the image is duplicated from");
});

it("rejects a duplicate naming a source that does not exist", async () => {
const res = await validateChangeRequestAccess(
duplicateChangeReq("post-does-not-exist"),
["group-public-editors"],
db,
);

expect(res.validated).toBe(false);
expect(res.error).toBe("No 'View' access to the document the image is duplicated from");
});

it("accepts a duplicate naming a source the user can view", async () => {
const res = await validateChangeRequestAccess(
duplicateChangeReq("post-blog1"),
["group-public-editors"],
db,
);

expect(res.validated).toBe(true);
});
});
});
26 changes: 26 additions & 0 deletions api/src/changeRequests/validateChangeRequestAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,32 @@ export async function validateChangeRequestAccess(
};
}

// A duplication names the document its image is copied from, and the server reads that
// document's files and bucket. The id is client-supplied, so it needs the same view access
// as reading the source document directly would.
if (doc.type === DocType.Post || doc.type === DocType.Tag) {
const duplicateFrom = (doc as _contentParentDto).imageData?.duplicateFrom;
if (duplicateFrom) {
const sourceDoc = (await dbService.getDoc(duplicateFrom)).docs[0];
if (
!sourceDoc ||
sourceDoc.type !== doc.type ||
!PermissionSystem.verifyAccess(
(sourceDoc as _contentBaseDto).memberOf,
sourceDoc.type,
AclPermission.View,
groupMembership,
"all",
)
) {
return {
validated: false,
error: "No 'View' access to the document the image is duplicated from",
};
}
}
}

// Validate edit, translate and group ACL assign access
// ====================================================

Expand Down
13 changes: 12 additions & 1 deletion api/src/dto/ImageDto.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import "reflect-metadata"; // https://stackoverflow.com/questions/72009995/typeerror-reflect-getmetadata-is-not-a-function
import { IsArray, IsBoolean, IsOptional, ValidateNested } from "class-validator";
import { IsArray, IsBoolean, IsOptional, IsString, ValidateNested } from "class-validator";
import { Expose, Type } from "class-transformer";
import { ImageUploadDto } from "./ImageUploadDto";
import { ImageFileCollectionDto } from "./ImageFileCollectionDto";
import { Uuid } from "../enums";

/**
* Database structured Image object
Expand All @@ -20,6 +21,16 @@ export class ImageDto {
@Expose()
uploadData?: ImageUploadDto[];

/**
* Id of the parent this image is being copied from. The source bucket and files are resolved
* from that document server-side, so the copy does not depend on the client's own state.
*/
@IsOptional()
@IsString()
@Expose()
duplicateFrom?: Uuid;

/** @deprecated Superseded by `duplicateFrom`; still accepted from older clients. */
@IsOptional()
@IsBoolean()
@Expose()
Expand Down
7 changes: 5 additions & 2 deletions cms/src/components/content/EditContent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,11 @@ const acceptServerVersion = () => window.location.reload();

const icon = props.docType === DocType.Tag ? TagIcon : DocumentIcon;

const notify = (state: "success" | "error" | "info", title: string, description: string) =>
addNotification({ title, description, state });
const notify = (
state: "success" | "error" | "info" | "warning",
title: string,
description: string,
) => addNotification({ title, description, state });

// Which translation is being edited (driven by the route), + the language lists.
const {
Expand Down
Loading
Loading