Skip to content

Commit

Permalink
Merge pull request #385 from jamhall/cors-validation
Browse files Browse the repository at this point in the history
Additional CORS validation
  • Loading branch information
kherock authored Feb 7, 2019
2 parents fc54b88 + 46def44 commit 6fc676f
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 0 deletions.
12 changes: 12 additions & 0 deletions lib/cors.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ const xml2js = require("xml2js");

const templateBuilder = require("./xml-template-builder");

// https://docs.aws.amazon.com/AmazonS3/latest/dev/cors.html#cors-allowed-methods
const corsAllowedMethods = ["GET", "PUT", "POST", "DELETE", "HEAD"];

function createWildcardRegExp(str, flags = "") {
const parts = str.split("*");
if (parts.length > 2)
Expand Down Expand Up @@ -32,6 +35,15 @@ module.exports = function cors(config) {
);
}

for (const method of rule.AllowedMethod) {
if (!corsAllowedMethods.includes(method)) {
throw new Error(
"Found unsupported HTTP method in CORS config. Unsupported method is " +
method
);
}
}

// Keep track if the rule has the plain wildcard '*' origin since S3 responds with '*'
// instead of echoing back the request origin in this case
rule.hasWildcardOrigin = rule.AllowedOrigin.includes("*");
Expand Down
10 changes: 10 additions & 0 deletions test/resources/cors_invalid1.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<CORSConfiguration>
<CORSRule>
<!-- Only one wildcard is allowed -->
<AllowedOrigin>https://*.*</AllowedOrigin>
<AllowedMethod>HEAD</AllowedMethod>
<AllowedMethod>GET</AllowedMethod>
<MaxAgeSeconds>3000</MaxAgeSeconds>
<AllowedHeader>*</AllowedHeader>
</CORSRule>
</CORSConfiguration>
9 changes: 9 additions & 0 deletions test/resources/cors_invalid2.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<CORSConfiguration>
<CORSRule>
<AllowedOrigin>*</AllowedOrigin>
<!-- Method must be one of GET, PUT, POST, DELETE, HEAD -->
<AllowedMethod>*</AllowedMethod>
<MaxAgeSeconds>3000</MaxAgeSeconds>
<AllowedHeader>*</AllowedHeader>
</CORSRule>
</CORSConfiguration>
7 changes: 7 additions & 0 deletions test/resources/cors_invalid3.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<CORSConfiguration>
<CORSRule>
<!-- At least one AllowedOrigin or AllowedMethod must be specified -->
<MaxAgeSeconds>3000</MaxAgeSeconds>
<AllowedHeader>*</AllowedHeader>
</CORSRule>
</CORSConfiguration>
64 changes: 64 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,70 @@ describe("S3rver CORS Policy Tests", function() {
}
});

it("should fail to initialize a configuration with multiple wildcard characters", function*() {
let error;
try {
let server;
yield thunkToPromise(done => {
server = new S3rver({
port: 4569,
hostname: "localhost",
silent: true,
cors: fs.readFileSync("./test/resources/cors_invalid1.xml")
}).run(done);
});
yield thunkToPromise(done => server.close(done));
} catch (err) {
error = err;
}
expect(error).to.exist;
expect(error.message).to.include(" can not have more than one wildcard.");
});

it("should fail to initialize a configuration with an illegal AllowedMethod", function*() {
let error;
try {
let server;
yield thunkToPromise(done => {
server = new S3rver({
port: 4569,
hostname: "localhost",
silent: true,
cors: fs.readFileSync("./test/resources/cors_invalid2.xml")
}).run(done);
});
yield thunkToPromise(done => server.close(done));
} catch (err) {
error = err;
}
expect(error).to.exist;
expect(error.message).to.include(
"Found unsupported HTTP method in CORS config."
);
});

it("should fail to initialize a configuration with missing required fields", function*() {
let error;
try {
let server;
yield thunkToPromise(done => {
server = new S3rver({
port: 4569,
hostname: "localhost",
silent: true,
cors: fs.readFileSync("./test/resources/cors_invalid3.xml")
}).run(done);
});
yield thunkToPromise(done => server.close(done));
} catch (err) {
error = err;
}
expect(error).to.exist;
expect(error.message).to.include(
"CORSRule must have at least one AllowedOrigin and AllowedMethod"
);
});

it("should add the Access-Control-Allow-Origin header for default (wildcard) configurations", function*() {
const origin = "http://a-test.example.com";
const params = { Bucket: bucket, Key: "image" };
Expand Down

0 comments on commit 6fc676f

Please sign in to comment.