Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxy image gets #1686

Merged
merged 29 commits into from Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f3e6fb8
proxy image gets
tzjames Sep 22, 2023
18ca8d1
Allow legacy links to work
tzjames Sep 25, 2023
f150d5c
Add comment
tzjames Sep 25, 2023
e34783b
fix bug where we only showed Loading on first image
tzjames Sep 25, 2023
dc16a56
Use LoadingSpinner
tzjames Sep 25, 2023
be409f5
add Error viewing
tzjames Sep 26, 2023
5a85276
imageCache
tzjames Sep 26, 2023
1cd8a3f
fix lint
tzjames Sep 26, 2023
dea266d
check organization
tzjames Sep 26, 2023
779ce18
remove svg as option in MarkdownInput
tzjames Sep 26, 2023
236ddb3
Replace markdown images. Replace old urls with proxy.
tzjames Sep 27, 2023
7c5a25c
return s3/google-cloud urls letting AuthorizedImage rewrite.
tzjames Sep 27, 2023
2ccf83b
Fix lint
tzjames Sep 27, 2023
64b456b
Fix cache
tzjames Oct 2, 2023
7810def
use s3domain env var
tzjames Oct 2, 2023
252090c
Merge remote-tracking branch 'origin/main' into ji/proxy_image_gets
tzjames Oct 17, 2023
c475395
allow tables and other github flavor markdown
tzjames Oct 17, 2023
5a66f77
update libraries
tzjames Oct 17, 2023
a264716
have all methods use the same router
tzjames Oct 17, 2023
ca3f152
null byte check
tzjames Oct 17, 2023
6cec73d
save new route for s3 and google-cloud
tzjames Oct 17, 2023
1c30a8f
add missing upload router
tzjames Oct 17, 2023
a3fb736
Move carousel css up so it works in the new design
tzjames Oct 17, 2023
db62acf
Merge branch 'main' into ji/proxy_image_gets
tzjames Nov 21, 2023
658f107
Use uuidv4. Save original URL. Replace only GCS bucket. New env var f…
tzjames Nov 23, 2023
49eafd0
Use streams
tzjames Nov 28, 2023
7dedf54
remove async backend & use locks on frontend
tzjames Dec 22, 2023
6dc8e4e
Merge branch 'main' into ji/proxy_image_gets
tzjames Dec 22, 2023
3418d79
Merge branch 'main' into ji/proxy_image_gets
tzjames Jan 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/docs/self-host/env.mdx
Expand Up @@ -128,6 +128,7 @@ Store uploads in an AWS S3 bucket.
- **S3_DOMAIN** (defaults to `https://${S3_BUCKET}.s3.amazonaws.com/`)
- **AWS_ACCESS_KEY_ID** (not required when deployed to AWS with an instance role)
- **AWS_SECRET_ACCESS_KEY** (not required when deployed to AWS with an instance role)
- **USE_FILE_PROXY** set this to true for access to uploads to proxy through your self hosted server allowing you to keep the bucket private.

### google-cloud

Expand All @@ -136,6 +137,7 @@ Store uploads in a Google Cloud Storage bucket.
- **GCS_BUCKET_NAME**
- **GCS_DOMAIN** (defaults to `https://storage.googleapis.com/${GCS_BUCKET_NAME}/`)
- **GOOGLE_APPLICATION_CREDENTIALS** (not required when deployed to GCP with an instance service account)
- **USE_FILE_PROXY** set this to true for access to uploads to proxy through your self hosted server allowing you to keep the bucket private.

## Enterprise Settings

Expand Down
7 changes: 2 additions & 5 deletions packages/back-end/src/app.ts
Expand Up @@ -87,7 +87,7 @@ import { getBuild } from "./util/handler";
import { getCustomLogProps, httpLogger } from "./util/logger";
import { usersRouter } from "./routers/users/users.router";
import { organizationsRouter } from "./routers/organizations/organizations.router";
import { putUploadRouter } from "./routers/upload/put-upload.router";
import { uploadRouter } from "./routers/upload/upload.router";
import { eventsRouter } from "./routers/events/events.router";
import { eventWebHooksRouter } from "./routers/event-webhooks/event-webhooks.router";
import { tagRouter } from "./routers/tag/tag.router";
Expand All @@ -104,7 +104,6 @@ import { dataExportRouter } from "./routers/data-export/data-export.router";
import { demoDatasourceProjectRouter } from "./routers/demo-datasource-project/demo-datasource-project.router";
import { environmentRouter } from "./routers/environment/environment.router";
import { teamRouter } from "./routers/teams/teams.router";
import { staticFilesRouter } from "./routers/upload/static-files.router";
import { githubIntegrationRouter } from "./routers/github-integration/github-integration.router";

const app = express();
Expand Down Expand Up @@ -304,8 +303,6 @@ app.post("/auth/refresh", authController.postRefresh);
app.post("/auth/logout", authController.postLogout);
app.get("/auth/hasorgs", authController.getHasOrganizations);

app.use("/upload", staticFilesRouter);

// All other routes require a valid JWT
const auth = getAuthConnection();
app.use(auth.middleware);
Expand Down Expand Up @@ -651,7 +648,7 @@ app.delete(
discussionsController.deleteComment
);
app.get("/discussions/recent/:num", discussionsController.getRecentDiscussions);
app.use("/putupload", putUploadRouter);
app.use("/upload", uploadRouter);

// Teams
app.use("/teams", teamRouter);
Expand Down
22 changes: 0 additions & 22 deletions packages/back-end/src/routers/upload/static-files.router.ts

This file was deleted.

48 changes: 39 additions & 9 deletions packages/back-end/src/routers/upload/upload.controller.ts
@@ -1,19 +1,26 @@
import { Response } from "express";
import uniqid from "uniqid";
import { uploadFile } from "../../services/files";
import { v4 as uuidv4 } from "uuid";
import { getImageData, uploadFile } from "../../services/files";
import { AuthRequest } from "../../types/AuthRequest";
import { getOrgFromReq } from "../../services/organizations";

const mimetypes: { [key: string]: string } = {
"image/png": "png",
"image/jpeg": "jpeg",
"image/gif": "gif",
};

// Inverted object mapping extensions to mimetypes
const extensionsToMimetype: { [key: string]: string } = {};
for (const mimetype in mimetypes) {
const extension = mimetypes[mimetype];
extensionsToMimetype[extension] = mimetype;
}

export async function putUpload(req: AuthRequest<Buffer>, res: Response) {
const contentType = req.headers["content-type"] as string;
req.checkPermissions("addComments", "");

const mimetypes: { [key: string]: string } = {
"image/png": "png",
"image/jpeg": "jpeg",
"image/gif": "gif",
};

if (!(contentType in mimetypes)) {
throw new Error(
`Invalid image file type. Only ${Object.keys(mimetypes).join(
Expand All @@ -27,7 +34,7 @@ export async function putUpload(req: AuthRequest<Buffer>, res: Response) {

const now = new Date();
const pathPrefix = `${org.id}/${now.toISOString().substr(0, 7)}/`;
const fileName = uniqid("img_");
const fileName = "img_" + uuidv4();
const filePath = `${pathPrefix}${fileName}.${ext}`;
const fileURL = await uploadFile(filePath, contentType, req.body);

Expand All @@ -36,3 +43,26 @@ export async function putUpload(req: AuthRequest<Buffer>, res: Response) {
fileURL,
});
}

export function getImage(req: AuthRequest<{ path: string }>, res: Response) {
const { org } = getOrgFromReq(req);

const path = req.path[0] === "/" ? req.path.substr(1) : req.path;
tzjames marked this conversation as resolved.
Show resolved Hide resolved

const orgFromPath = path.split("/")[0];
if (orgFromPath !== org.id) {
throw new Error("Invalid organization");
}

const ext = path.split(".").pop() || "";
const contentType = extensionsToMimetype[ext];

if (!contentType) {
throw new Error(`Invalid file extension: ${ext}`);
}

res.status(200).contentType(contentType);

const stream = getImageData(path);
stream.pipe(res);
}
Expand Up @@ -7,6 +7,7 @@ const router = express.Router();

const uploadController = wrapController(uploadControllerRaw);

router.get("/:path*", uploadController.getImage);
router.put(
"/",
bodyParser.raw({
Expand All @@ -16,4 +17,4 @@ router.put(
uploadController.putUpload
);

export { router as putUploadRouter };
export { router as uploadRouter };
44 changes: 42 additions & 2 deletions packages/back-end/src/services/files.ts
Expand Up @@ -4,8 +4,8 @@ import AWS from "aws-sdk";
import { Storage } from "@google-cloud/storage";
import {
S3_BUCKET,
S3_DOMAIN,
S3_REGION,
S3_DOMAIN,
UPLOAD_METHOD,
GCS_BUCKET_NAME,
GCS_DOMAIN,
Expand Down Expand Up @@ -33,6 +33,7 @@ export async function uploadFile(
if (filePath.indexOf("\0") !== -1) {
throw new Error("Error: Filename must not contain null bytes");
}

let fileURL = "";

if (UPLOAD_METHOD === "s3") {
Expand All @@ -51,7 +52,6 @@ export async function uploadFile(
.bucket(GCS_BUCKET_NAME)
.file(filePath)
.save(contents, { contentType: contentType });

fileURL = GCS_DOMAIN + (GCS_DOMAIN.endsWith("/") ? "" : "/") + filePath;
} else {
const rootDirectory = getUploadsDir();
Expand All @@ -71,3 +71,43 @@ export async function uploadFile(
}
return fileURL;
}

export function getImageData(filePath: string) {
// Watch out for poison null bytes
if (filePath.indexOf("\0") !== -1) {
throw new Error("Error: Filename must not contain null bytes");
}

if (UPLOAD_METHOD === "s3") {
const params = {
Bucket: S3_BUCKET,
Key: filePath,
};
const s3Stream = getS3().getObject(params).createReadStream();
return s3Stream;
} else if (UPLOAD_METHOD === "google-cloud") {
const storage = new Storage();
const bucket = storage.bucket(GCS_BUCKET_NAME);
const file = bucket.file(filePath);
const readableStream = file.createReadStream();
return readableStream;
} else {
// local image
const rootDirectory = getUploadsDir();
const fullPath = path.join(rootDirectory, filePath);

// Prevent directory traversal
if (fullPath.indexOf(rootDirectory) !== 0) {
throw new Error(
"Error: Path must not escape out of the 'uploads' directory."
);
}

if (!fs.existsSync(fullPath)) {
throw new Error("File not found");
}

const readableStream = fs.createReadStream(fullPath);
return readableStream;
}
}
103 changes: 103 additions & 0 deletions packages/front-end/components/AuthorizedImage.tsx
@@ -0,0 +1,103 @@
import React, { useEffect, useState, FC } from "react";
import { FaExclamationTriangle } from "react-icons/fa";
import { useAuth } from "@/services/auth";
import {
getApiHost,
getGcsDomain,
getS3Domain,
usingFileProxy,
} from "@/services/env";
import LoadingSpinner from "./LoadingSpinner";

interface AuthorizedImageProps extends React.HTMLProps<HTMLImageElement> {
imageCache?: Record<string, string>;
}

/**
* This component is used to display images that are stored in a private bucket.
* It will fetch the image from the backend and convert it to a blob url that can be displayed.
* It will also cache the image if imageCache is set so that it does not need to be fetched again.
* */
const AuthorizedImage: FC<AuthorizedImageProps> = ({
imageCache = {},
src = "",
...props
}) => {
const [imageSrc, setImageSrc] = useState<string | null>(null);
const [error, setError] = useState<string | null>(null);

const { apiCall } = useAuth();

useEffect(() => {
const fetchData = async (src, apiUrl) => {
try {
const imageData: Blob = await apiCall(new URL(apiUrl).pathname);
const imageUrl = URL.createObjectURL(imageData);
imageCache[src] = imageUrl;
setImageSrc(imageUrl);
} catch (error) {
setError(error.message);
}
};

navigator.locks.request(src, async () => {
if (imageCache[src]) {
// Images in the cache do not need to be fetched again
setImageSrc(imageCache[src]);
} else if (
usingFileProxy() &&
getGcsDomain() &&
src.startsWith(getGcsDomain())
) {
// We convert GCS images to the GB url that acts as a proxy using the correct credentials
// This way they can lock their bucket down to only allow access from the proxy.
const withoutDomain = src.replace(
"https://storage.googleapis.com/",
""
);
const parts = withoutDomain.split("/");
parts.shift(); // remove bucket name
const apiUrl = getApiHost() + "/upload/" + parts.join("/");
await fetchData(src, apiUrl);
} else if (
usingFileProxy() &&
getS3Domain() &&
src.startsWith(getS3Domain())
) {
// We convert s3 images to the GB url that acts as a proxy using the correct credentials
// This way they can lock their bucket down to only allow access from the proxy.
const apiUrl =
getApiHost() + "/upload/" + src.substring(getS3Domain().length);
await fetchData(src, apiUrl);
} else if (!src.startsWith(getApiHost())) {
// Images in markdown that are not from our host we will treat as a normal image
setImageSrc(src);
} else {
// Images to the proxy we will fetch from the backend
const apiUrl = src;
await fetchData(src, apiUrl);
}
});
}, [src, imageCache, apiCall]);

if (error) {
return (
<div>
<FaExclamationTriangle
size={14}
className="text-danger ml-1"
style={{ marginTop: -4 }}
/>
<span> Error: {error} </span>{" "}
</div>
);
}

if (!imageSrc) {
return <LoadingSpinner />;
}

return <img src={imageSrc} {...props} crossOrigin="" />;
};

export default AuthorizedImage;
1 change: 1 addition & 0 deletions packages/front-end/components/Carousel.tsx
Expand Up @@ -50,6 +50,7 @@ const Carousel: FC<{
open={true}
header={"Screenshot"}
close={() => setModalOpen(false)}
bodyClassName="d-flex justify-content-center align-items-center"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This aligns the loading spinner in the middle of the variation area.

size="max"
sizeY="max"
>
Expand Down
Expand Up @@ -28,7 +28,7 @@ const ScreenshotUpload = ({
const [loading, setLoading] = useState(0);

const onDrop = async (files: File[]) => {
setLoading(loading + files.length);
setLoading((previous) => previous + files.length);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the ones blow fixed a pre-existing bug where we were only showing the loading spinner on put on the first image upload, but not subsequent ones.


for (const file of files) {
try {
Expand All @@ -46,15 +46,15 @@ const ScreenshotUpload = ({
}
);

setLoading(loading - 1);
setLoading((previous) => previous - 1);

onSuccess(variation, {
path: fileURL,
description: "",
});
} catch (e) {
alert(e.message);
setLoading(loading - 1);
setLoading((previous) => previous - 1);
}
}
};
Expand Down