Skip to content

Commit

Permalink
fix(sdk): local bucket files can't be accessed in simulator
Browse files Browse the repository at this point in the history
  • Loading branch information
@Chriscbr
Chriscbr committed Sep 18, 2024
1 parent b9728a1 commit 24ad3c5
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 196 deletions.
11 changes: 0 additions & 11 deletions packages/@winglang/sdk/src/shared-aws/bucket.inflight.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,6 @@ export class BucketClient implements IAwsBucketClient {
throw new Error("Cannot provide public url for a non-public bucket");
}

if (!(await this.exists(key))) {
throw new Error(
`Cannot provide public url for a non-existent key (key=${key})`
);
}

const region = await this.getLocation();

return encodeURI(
Expand All @@ -401,11 +395,6 @@ export class BucketClient implements IAwsBucketClient {
// Set the S3 command
switch (action) {
case BucketSignedUrlAction.DOWNLOAD:
if (!(await this.exists(key))) {
throw new Error(
`Cannot provide signed url for a non-existent key (key=${key})`
);
}
s3Command = new GetObjectCommand({
Bucket: this.bucketName,
Key: key,
Expand Down
5 changes: 0 additions & 5 deletions packages/@winglang/sdk/src/shared-azure/bucket.inflight.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,6 @@ export class BucketClient implements IBucketClient {
if (!accessPolicy?.blobPublicAccess) {
throw new Error("Cannot provide public url for a non-public bucket");
}
if (!(await this.exists(key))) {
throw new Error(
`Cannot provide public url for a non-existent key (key=${key})`
);
}

return encodeURI(
`https://${this.storageAccount}.blob.core.windows.net/${this.bucketName}/${key}`
Expand Down
11 changes: 0 additions & 11 deletions packages/@winglang/sdk/src/shared-gcp/bucket.inflight.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,6 @@ export class BucketClient implements IBucketClient {
throw new Error("Cannot provide public url for a non-public bucket");
}

if (!(await this.exists(key))) {
throw new Error(
`Cannot provide public url for a non-existent key (key=${key})`
);
}

return `https://storage.googleapis.com/${this.bucketName}/${key}`;
}

Expand All @@ -257,11 +251,6 @@ export class BucketClient implements IBucketClient {
// Set the GCS action
switch (action) {
case BucketSignedUrlAction.DOWNLOAD:
if (!(await this.exists(key))) {
throw new Error(
`Cannot provide signed url for a non-existent key (key=${key})`
);
}
gcsAction = "read";
break;
case BucketSignedUrlAction.UPLOAD:
Expand Down
38 changes: 11 additions & 27 deletions packages/@winglang/sdk/src/target-sim/bucket.inflight.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as crypto from "crypto";
import * as fs from "fs";
import { Server } from "http";
import { dirname, join } from "path";
import { pathToFileURL } from "url";
import express from "express";
import mime from "mime-types";
import { BucketAttributes, BucketSchema } from "./schema-resources";
Expand Down Expand Up @@ -141,22 +140,24 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance {
return;
});

// Handle signed URL downloads.
// Handle file accesses and signed URL downloads.
this.app.get("*", (req, res) => {
const action = req.query.action;
if (action !== BucketSignedUrlAction.DOWNLOAD) {
return res.status(403).send("Operation not allowed");
}
if (!this._public) {
if (action !== BucketSignedUrlAction.DOWNLOAD) {
return res.status(403).send("Operation not allowed");
}

const validUntil = req.query.validUntil?.toString();
if (!validUntil || Date.now() > parseInt(validUntil)) {
return res.status(403).send("Signed URL has expired");
const validUntil = req.query.validUntil?.toString();
if (!validUntil || Date.now() > parseInt(validUntil)) {
return res.status(403).send("Signed URL has expired");
}
}

const key = req.path.slice(1); // remove leading slash
const hash = this.hashKey(key);
const filename = join(this._fileDir, hash);
return res.download(filename);
return res.sendFile(filename);
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
Loading
, but is not rate-limited.
}

Expand Down Expand Up @@ -458,15 +459,7 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance {
return this.context.withTrace({
message: `Public URL (key=${key}).`,
activity: async () => {
const filePath = join(this._fileDir, key);

if (!this._metadata.has(key)) {
throw new Error(
`Cannot provide public url for an non-existent key (key=${key})`
);
}

return pathToFileURL(filePath).href;
return new URL(key, this.url).toString();
},
});
}
Expand All @@ -480,15 +473,6 @@ export class Bucket implements IBucketClient, ISimulatorResourceInstance {
// a POJO with seconds, but TypeScript thinks otherwise.
const duration = options?.duration?.seconds ?? 900;

if (
action === BucketSignedUrlAction.DOWNLOAD &&
!(await this.exists(key))
) {
throw new Error(
`Cannot provide signed url for a non-existent key (key=${key})`
);
}

const url = new URL(key, this.url);
url.searchParams.set("action", action);
url.searchParams.set(
Expand Down
56 changes: 0 additions & 56 deletions packages/@winglang/sdk/test/shared-aws/bucket.inflight.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,38 +254,6 @@ test("Given a non public bucket when reaching to a key public url it should thro
);
});

test("Given a public bucket when reaching to a non-existent key, public url it should throw an error", async () => {
// GIVEN
let error;
const BUCKET_NAME = "BUCKET_NAME";
const KEY = "KEY";

s3Mock.on(GetPublicAccessBlockCommand, { Bucket: BUCKET_NAME }).resolves({
PublicAccessBlockConfiguration: {
BlockPublicAcls: false,
BlockPublicPolicy: false,
RestrictPublicBuckets: false,
IgnorePublicAcls: false,
},
});
s3Mock
.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
.rejects(new NotFound({ message: "Object not found", $metadata: {} }));

// WHEN
const client = new BucketClient({ $bucketName: BUCKET_NAME });
try {
await client.publicUrl(KEY);
} catch (err) {
error = err;
}

// THEN
expect(error?.message).toBe(
"Cannot provide public url for a non-existent key (key=KEY)"
);
});

test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
// GIVEN
const BUCKET_NAME = "BUCKET_NAME";
Expand Down Expand Up @@ -557,30 +525,6 @@ test("tryDelete a non-existent object from the bucket", async () => {
expect(objectTryDelete).toEqual(false);
});

test("Given a bucket when reaching to a non-existent key, signed url it should throw an error", async () => {
// GIVEN
let error;
const BUCKET_NAME = "BUCKET_NAME";
const KEY = "KEY";

s3Mock
.on(HeadObjectCommand, { Bucket: BUCKET_NAME, Key: KEY })
.rejects(new NotFound({ message: "Object not found", $metadata: {} }));

// WHEN
const client = new BucketClient({ $bucketName: BUCKET_NAME });
try {
await client.signedUrl(KEY);
} catch (err) {
error = err;
}

// THEN
expect(error?.message).toBe(
`Cannot provide signed url for a non-existent key (key=${KEY})`
);
});

// Skipped due to issue with mocking getSignedUrl:
// https://github.com/m-radzikowski/aws-sdk-client-mock/issues/62
test.skip("Given a bucket, when giving one of its keys, we should get its signed url", async () => {
Expand Down
25 changes: 0 additions & 25 deletions packages/@winglang/sdk/test/shared-azure/bucket.inflight.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,31 +409,6 @@ test("Given a non public bucket when reaching to a key public url it should thro
);
});

test("Given a public bucket when reaching to a non existent key, public url it should throw an error", async () => {
// GIVEN
const BUCKET_NAME = "BUCKET_NAME";
const STORAGE_NAME = "STORAGE_NAME";
const KEY = "non-existent-key";

// WHEN
const client = new BucketClient({
$bucketName: BUCKET_NAME,
$storageAccount: STORAGE_NAME,
$blobServiceClient: mockBlobServiceClient,
});
TEST_PATH = "sad";
// @ts-expect-error - accessing private property
client.containerClient.getAccessPolicy = vi
.fn()
.mockResolvedValue({ blobPublicAccess: "container" });
client.exists = vi.fn().mockResolvedValue(false);

// THEN
await expect(() => client.publicUrl(KEY)).rejects.toThrowError(
`Cannot provide public url for a non-existent key (key=${KEY})`
);
});

test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
// GIVEN
const BUCKET_NAME = "BUCKET_NAME";
Expand Down
86 changes: 30 additions & 56 deletions packages/@winglang/sdk/test/target-sim/bucket.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ test("Given a non public bucket when reaching to a key public url it should thro
await s.stop();
});

test("Given a public bucket when reaching to a non existent key, public url it should throw an error", async () => {
test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
// GIVEN
const app = new SimApp();
new cloud.Bucket(app, "my_bucket", { public: true });
Expand All @@ -583,15 +583,18 @@ test("Given a public bucket when reaching to a non existent key, public url it s
const client = s.getResource("/my_bucket") as cloud.IBucketClient;

const KEY = "KEY";
const VALUE = "VALUE";

// WHEN
await client.put(KEY, VALUE);
const response = await client.publicUrl(KEY);

// THEN
await expect(() => client.publicUrl(KEY)).rejects.toThrowError(
/Cannot provide public url for an non-existent key/
);
await s.stop();
expect(response).toMatch(/https?:\/\/(localhost|127\.0\.0\.1):\d+\/KEY/);
});

test("Given a public bucket, when giving one of its keys, we should get its public url", async () => {
test("accessing the publicUrl of a valid key should return the file contents", async () => {
// GIVEN
const app = new SimApp();
new cloud.Bucket(app, "my_bucket", { public: true });
Expand All @@ -604,12 +607,31 @@ test("Given a public bucket, when giving one of its keys, we should get its publ

// WHEN
await client.put(KEY, VALUE);
const response = await client.publicUrl(KEY);
const publicUrl = await client.publicUrl(KEY);

// THEN
const response = await fetch(publicUrl);
const text = await response.text();
expect(text).toBe(VALUE);
});

test("accessing the publicUrl of a non existent key should throw an error", async () => {
// GIVEN
const app = new SimApp();
new cloud.Bucket(app, "my_bucket", { public: true });

const s = await app.startSimulator();
const client = s.getResource("/my_bucket") as cloud.IBucketClient;

const KEY = "KEY";

// WHEN
const publicUrl = await client.publicUrl(KEY);
const response = await fetch(publicUrl);

// THEN
expect(response.status).toBe(404);
await s.stop();
// file paths are different on windows and linux
expect(response.endsWith("KEY")).toBe(true);
});

test("check if an object exists in the bucket", async () => {
Expand Down Expand Up @@ -994,51 +1016,3 @@ test("signedUrl doesn't accept multipart uploads yet", async () => {

await s.stop();
});

// Deceided to seperate this feature in a different release,(see https://github.com/winglang/wing/issues/4143)

// test("Given a bucket when reaching to a non existent key, signed url it should throw an error", async () => {
// //GIVEN
// let error;
// const app = new SimApp();
// new cloud.Bucket(app, "my_bucket", { public: true });

// const s = await app.startSimulator();
// const client = s.getResource("/my_bucket") as cloud.IBucketClient;

// const KEY = "KEY";

// // WHEN
// try {
// await client.signedUrl(KEY);
// } catch (err) {
// error = err;
// }

// expect(error?.message).toBe(
// "Cannot provide signed url for an non-existent key (key=${KEY})"
// );
// // THEN
// await s.stop();
// });

// test("Given a bucket, when giving one of its keys, we should get it's signed url", async () => {
// // GIVEN
// const app = new SimApp();
// new cloud.Bucket(app, "my_bucket", { public: true });

// const s = await app.startSimulator();
// const client = s.getResource("/my_bucket") as cloud.IBucketClient;

// const KEY = "KEY";
// const VALUE = "VALUE";

// // WHEN
// await client.put(KEY, VALUE);
// const response = await client.signedUrl(KEY);

// // THEN
// await s.stop();
// const filePath = `${client.fileDir}/${KEY}`;
// expect(response).toEqual(url.pathToFileURL(filePath).searchParams.append("Expires","86400"));
// });
6 changes: 1 addition & 5 deletions tests/sdk_tests/bucket/public_url.test.w
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ test "publicUrl" {

let publicUrl = publicBucket.publicUrl("file1.txt");
assert(publicUrl != "");

// TODO: works in aws, doesn't work in sim since publicUrl is returning a path to the file, remove condition when #2833 is resolved.
if (util.env("WING_TARGET") != "sim") {
assert(http.get(publicUrl).body == "Foo");
}
assert(http.get(publicUrl).body == "Foo");

assertThrows(BUCKET_NOT_PUBLIC_ERROR, () => {
privateBucket.publicUrl("file2.txt");
Expand Down

0 comments on commit 24ad3c5

Please sign in to comment.