fix: Unable to store/read in avatars bucket with local file system storage

closes #5873
This commit is contained in:
Tom Moor
2023-09-23 15:00:48 -04:00
parent 5c7c9ceeb1
commit 65d3c8309e
6 changed files with 48 additions and 25 deletions

View File

@@ -7,6 +7,7 @@ import {
} from "@shared/types";
import { traceFunction } from "@server/logging/tracing";
import { Collection, Event, Team, User, FileOperation } from "@server/models";
import { Buckets } from "@server/models/helpers/AttachmentHelper";
type Props = {
collection?: Collection;
@@ -19,8 +20,7 @@ type Props = {
};
function getKeyForFileOp(teamId: string, name: string) {
const bucket = "uploads";
return `${bucket}/${teamId}/${uuidv4()}/${name}-export.zip`;
return `${Buckets.uploads}/${teamId}/${uuidv4()}/${name}-export.zip`;
}
async function collectionExporter({

View File

@@ -2,6 +2,12 @@ import { addHours } from "date-fns";
import { AttachmentPreset } from "@shared/types";
import env from "@server/env";
export enum Buckets {
public = "public",
uploads = "uploads",
avatars = "avatars",
}
export default class AttachmentHelper {
/**
* Get the upload location for the given upload details
@@ -22,7 +28,7 @@ export default class AttachmentHelper {
name: string;
userId: string;
}) {
const bucket = acl === "public-read" ? "public" : "uploads";
const bucket = acl === "public-read" ? Buckets.public : Buckets.uploads;
const keyPrefix = `${bucket}/${userId}/${id}`;
return `${keyPrefix}/${name}`;
}

View File

@@ -1,5 +1,6 @@
import { v4 as uuidv4 } from "uuid";
import { Team } from "@server/models";
import { Buckets } from "@server/models/helpers/AttachmentHelper";
import FileStorage from "@server/storage/files";
import BaseTask, { TaskPriority } from "./BaseTask";
@@ -22,7 +23,7 @@ export default class UploadTeamAvatarTask extends BaseTask<Props> {
const res = await FileStorage.storeFromUrl(
props.avatarUrl,
`avatars/${team.id}/${uuidv4()}`,
`${Buckets.avatars}/${team.id}/${uuidv4()}`,
"public-read"
);

View File

@@ -1,5 +1,6 @@
import { v4 as uuidv4 } from "uuid";
import { User } from "@server/models";
import { Buckets } from "@server/models/helpers/AttachmentHelper";
import FileStorage from "@server/storage/files";
import BaseTask, { TaskPriority } from "./BaseTask";
@@ -22,7 +23,7 @@ export default class UploadUserAvatarTask extends BaseTask<Props> {
const res = await FileStorage.storeFromUrl(
props.avatarUrl,
`avatars/${user.id}/${uuidv4()}`,
`${Buckets.avatars}/${user.id}/${uuidv4()}`,
"public-read"
);

View File

@@ -1,32 +1,42 @@
import { v4 as uuidv4 } from "uuid";
import { Buckets } from "./models/helpers/AttachmentHelper";
import { ValidateKey } from "./validation";
describe("#ValidateKey.isValid", () => {
it("should return false if number of key components are not equal to 4", () => {
expect(ValidateKey.isValid(`uploads/${uuidv4()}/${uuidv4()}`)).toBe(false);
expect(ValidateKey.isValid(`uploads/${uuidv4()}/${uuidv4()}/foo/bar`)).toBe(
false
);
expect(
ValidateKey.isValid(`${Buckets.uploads}/${uuidv4()}/${uuidv4()}`)
).toBe(false);
expect(
ValidateKey.isValid(`${Buckets.uploads}/${uuidv4()}/${uuidv4()}/foo/bar`)
).toBe(false);
});
it("should return false if the first key component is neither 'public' nor 'uploads' ", () => {
it("should return false if the first key component is not a valid bucket", () => {
expect(ValidateKey.isValid(`foo/${uuidv4()}/${uuidv4()}/bar.png`)).toBe(
false
);
});
it("should return false if second and third key components are not UUID", () => {
expect(ValidateKey.isValid(`uploads/foo/${uuidv4()}/bar.png`)).toBe(false);
expect(ValidateKey.isValid(`uploads/${uuidv4()}/foo/bar.png`)).toBe(false);
expect(
ValidateKey.isValid(`${Buckets.uploads}/foo/${uuidv4()}/bar.png`)
).toBe(false);
expect(
ValidateKey.isValid(`${Buckets.uploads}/${uuidv4()}/foo/bar.png`)
).toBe(false);
});
it("should return true successfully validating key", () => {
expect(ValidateKey.isValid(`public/${uuidv4()}/${uuidv4()}/foo.png`)).toBe(
true
);
expect(ValidateKey.isValid(`uploads/${uuidv4()}/${uuidv4()}/foo.png`)).toBe(
true
);
expect(
ValidateKey.isValid(`${Buckets.public}/${uuidv4()}/${uuidv4()}/foo.png`)
).toBe(true);
expect(
ValidateKey.isValid(`${Buckets.uploads}/${uuidv4()}/${uuidv4()}/foo.png`)
).toBe(true);
expect(
ValidateKey.isValid(`${Buckets.avatars}/${uuidv4()}/${uuidv4()}`)
).toBe(true);
});
});

View File

@@ -4,13 +4,14 @@ import { Primitive } from "utility-types";
import validator from "validator";
import isIn from "validator/lib/isIn";
import isUUID from "validator/lib/isUUID";
import { CollectionPermission } from "@shared/types";
import { validateColorHex } from "@shared/utils/color";
import { validateIndexCharacters } from "@shared/utils/indexCharacters";
import parseMentionUrl from "@shared/utils/parseMentionUrl";
import { SLUG_URL_REGEX } from "@shared/utils/urlHelpers";
import { isUrl } from "@shared/utils/urls";
import { CollectionPermission } from "../shared/types";
import { validateColorHex } from "../shared/utils/color";
import { validateIndexCharacters } from "../shared/utils/indexCharacters";
import { ParamRequiredError, ValidationError } from "./errors";
import { Buckets } from "./models/helpers/AttachmentHelper";
type IncomingValue = Primitive | string[];
@@ -174,10 +175,15 @@ export const assertCollectionPermission = (
export class ValidateKey {
public static isValid = (key: string) => {
const parts = key.split("/").slice(0, -1);
let parts = key.split("/");
const bucket = parts[0];
// Avatars do not have a file name at the end of the key
parts = bucket === Buckets.avatars ? parts : parts.slice(0, -1);
return (
parts.length === 3 &&
isIn(parts[0], ["uploads", "public"]) &&
isIn(parts[0], Object.values(Buckets)) &&
isUUID(parts[1]) &&
isUUID(parts[2])
);
@@ -192,8 +198,7 @@ export class ValidateKey {
.concat(`/${sanitize(filename)}`);
};
public static message =
"Must be of the form uploads/<uuid>/<uuid>/<name> or public/<uuid>/<uuid>/<name>";
public static message = "Must be of the form <bucket>/<uuid>/<uuid>/<name>";
}
export class ValidateDocumentId {