diff --git a/lib/middleware/authentication.js b/lib/middleware/authentication.js index ddbd6df4..ceac7b89 100644 --- a/lib/middleware/authentication.js +++ b/lib/middleware/authentication.js @@ -7,7 +7,11 @@ const querystring = require('querystring'); const AWSAccount = require('../models/account'); const S3Error = require('../models/error'); const { RESPONSE_HEADERS } = require('./response-header-override'); -const { parseDate, parseISO8601String } = require('../utils'); +const { + encodeURIComponentRFC3986, + parseDate, + parseISO8601String, +} = require('../utils'); const SUBRESOURCES = { acl: 1, @@ -51,13 +55,9 @@ module.exports = () => return next(); } - const query = mapKeys(ctx.query, (value, key) => key.toLowerCase()); const amzQueryHeaders = pickBy( - query, - (value, key) => - key.startsWith('x-amz-') && - // x-amz-website-redirect-location isn't a canonical header - key !== 'x-amz-website-redirect-location', + mapKeys(ctx.query, (value, key) => key.toLowerCase()), + (value, key) => key.startsWith('x-amz-'), ); // x-amz-* values specified in query params take precedence over those in the headers Object.assign(ctx.headers, amzQueryHeaders); @@ -85,32 +85,49 @@ module.exports = () => ); } - const canonicalizedAmzHeaders = Object.keys(ctx.headers) + let canonicalizedResource = ctx.mountPath || ''; + if (ctx.params.bucket) { + // the following behavior is derived from the behavior of the JS aws-sdk + if (ctx.state.vhost) { + canonicalizedResource = '/' + ctx.params.bucket + canonicalizedResource; + } else { + canonicalizedResource += '/' + ctx.params.bucket; + } + if (ctx.params.key) { + canonicalizedResource += '/' + ctx.params.key; + } + } else { + canonicalizedResource += '/'; + } + canonicalizedResource = canonicalizedResource + .split('/') + .map(encodeURIComponentRFC3986) + .join('/'); + + const canonicalizedQueryString = Object.entries(ctx.query) .filter( - headerName => - headerName.startsWith('x-amz-') && - headerName !== 'x-amz-website-redirect-location', + ([param]) => + SUBRESOURCES[param] || + RESPONSE_HEADERS[param] || + (mechanisms.queryV4 && param.startsWith('x-amz-')), + ) + .map(([param, value]) => + // v2 signing doesn't encode values in the signature calculation + mechanisms.queryV2 + ? [param, value].join('=') + : querystring.stringify({ [param]: value }), ) .sort() + .join('&') + .replace(/=(&|$)/g, ''); // remove trailing = for empty params + + const canonicalizedAmzHeaders = Object.keys(ctx.headers) + .filter(headerName => headerName.startsWith('x-amz-')) + .sort() .map( headerName => `${headerName}:${ctx.get(headerName).replace(/ +/g, ' ')}`, ); - const canonicalizedQueryString = Object.keys(query) - .filter( - paramName => - SUBRESOURCES[paramName] || - RESPONSE_HEADERS[paramName] || - (mechanisms.queryV4 && paramName.startsWith('x-amz-')), - ) - .map(paramName => - // v2 signing doesn't encode values in the signature calculation - mechanisms.queryV2 - ? [paramName, query[paramName]].join('=') - : querystring.stringify({ [paramName]: query[paramName] }), - ) - .sort() - .join('&'); const canonicalRequest = { method: @@ -120,10 +137,7 @@ module.exports = () => contentMD5: ctx.get('Content-MD5'), contentType: ctx.get('Content-Type'), timestamp: undefined, - // the following behavior is derived from the behavior of the JS aws-sdk - uri: ctx.state.vhost - ? `/${ctx.params.bucket}${ctx.mountPath || ''}/${ctx.params.key}` - : `${ctx.mountPath || ''}/${ctx.params.bucket}/${ctx.params.key}`, + uri: canonicalizedResource, querystring: canonicalizedQueryString, amzHeaders: canonicalizedAmzHeaders, }; @@ -134,7 +148,6 @@ module.exports = () => if (mechanisms.header) { const result = parseHeader(ctx.headers); ({ signature, signatureProvided } = result); - canonicalRequest.timestamp = result.timestamp; } else if (mechanisms.queryV2) { const result = parseQueryV2(ctx.query, ctx.headers); ({ signature, signatureProvided } = result); diff --git a/lib/routes.js b/lib/routes.js index 48b2fa17..a96d7dc7 100644 --- a/lib/routes.js +++ b/lib/routes.js @@ -231,7 +231,7 @@ router // append trailing slash to key when applicable router.param('key', (key, ctx, next) => { - if (ctx.path.endsWith('/')) { + if (key && ctx.path.endsWith('/')) { ctx.params.key = key + '/'; } return next(); diff --git a/lib/utils.js b/lib/utils.js index 1361600e..ebb29106 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -55,6 +55,24 @@ exports.concatStreams = function(streams) { return passThrough; }; +/** + * URI-encodes a string according to RFC 3986. This is what AWS uses for + * S3 resource URIs. + * + * @param {string} string + */ +exports.encodeURIComponentRFC3986 = function(string) { + return encodeURIComponent(string).replace( + /[!'()*]/g, + ch => + '%' + + ch + .charCodeAt(0) + .toString(16) + .toUpperCase(), + ); +}; + exports.getXmlRootTag = function(xml) { const traversal = xmlParser.getTraversalObj(xml.toString()); const [[root]] = Object.values(traversal.child); diff --git a/test/controllers/object.spec.js b/test/controllers/object.spec.js index 98368da2..a76d39af 100644 --- a/test/controllers/object.spec.js +++ b/test/controllers/object.spec.js @@ -1,6 +1,5 @@ 'use strict'; -const AWS = require('aws-sdk'); const { expect } = require('chai'); const express = require('express'); const FormData = require('form-data'); @@ -14,8 +13,6 @@ const request = require('request-promise-native').defaults({ }); const { URL, URLSearchParams } = require('url'); -const S3rver = require('../..'); - const { createServerAndClient, generateTestObjects } = require('../helpers'); describe('Operations on Objects', () => { @@ -849,35 +846,19 @@ describe('Operations on Objects', () => { it('stores an object in a bucket after all objects are deleted', async function() { const bucket = 'foobars'; - - const server = new S3rver(); - const { port } = await server.run(); - const s3Client = new AWS.S3({ - accessKeyId: 'S3RVER', - secretAccessKey: 'S3RVER', - endpoint: `http://localhost:${port}`, - sslEnabled: false, - s3ForcePathStyle: true, - }); - try { - await s3Client.createBucket({ Bucket: bucket }).promise(); - await s3Client - .putObject({ Bucket: bucket, Key: 'foo.txt', Body: 'Hello!' }) - .promise(); - await s3Client - .deleteObject({ Bucket: bucket, Key: 'foo.txt' }) - .promise(); - await s3Client - .putObject({ Bucket: bucket, Key: 'foo2.txt', Body: 'Hello2!' }) - .promise(); - } finally { - await server.close(); - } + await s3Client.createBucket({ Bucket: bucket }).promise(); + await s3Client + .putObject({ Bucket: bucket, Key: 'foo.txt', Body: 'Hello!' }) + .promise(); + await s3Client.deleteObject({ Bucket: bucket, Key: 'foo.txt' }).promise(); + await s3Client + .putObject({ Bucket: bucket, Key: 'foo2.txt', Body: 'Hello2!' }) + .promise(); }); }); describe('PUT Object - Copy', () => { - it('copys an image object into another bucket', async function() { + it('copies an image object into another bucket', async function() { const srcKey = 'image'; const destKey = 'image/jamie'; @@ -895,7 +876,7 @@ describe('Operations on Objects', () => { .copyObject({ Bucket: 'bucket-b', Key: destKey, - CopySource: '/' + 'bucket-a' + '/' + srcKey, + CopySource: '/bucket-a/' + srcKey, }) .promise(); expect(copyResult.ETag).to.equal(data.ETag); @@ -909,7 +890,7 @@ describe('Operations on Objects', () => { expect(object.ETag).to.equal(data.ETag); }); - it('copys an image object into another bucket including its metadata', async function() { + it('copies an image object into another bucket including its metadata', async function() { const srcKey = 'image'; const destKey = 'image/jamie'; @@ -931,7 +912,7 @@ describe('Operations on Objects', () => { Bucket: 'bucket-b', Key: destKey, // MetadataDirective is implied to be COPY - CopySource: '/' + 'bucket-a' + '/' + srcKey, + CopySource: '/bucket-a/' + srcKey, }) .promise(); const object = await s3Client @@ -942,7 +923,7 @@ describe('Operations on Objects', () => { expect(object.ETag).to.equal(data.ETag); }); - it('copys an object using spaces/unicode chars in keys', async function() { + it('copies an object using spaces/unicode chars in keys', async function() { const srcKey = 'awesome 驚くばかり.jpg'; const destKey = 'new 新しい.jpg'; @@ -960,14 +941,14 @@ describe('Operations on Objects', () => { .copyObject({ Bucket: 'bucket-a', Key: destKey, - CopySource: '/' + 'bucket-a' + '/' + encodeURI(srcKey), + CopySource: '/bucket-a/' + encodeURI(srcKey), }) .promise(); expect(copyResult.ETag).to.equal(data.ETag); expect(moment(copyResult.LastModified).isValid()).to.be.true; }); - it('copys an image object into another bucket and update its metadata', async function() { + it('copies an image object into another bucket and update its metadata', async function() { const srcKey = 'image'; const destKey = 'image/jamie'; @@ -984,7 +965,7 @@ describe('Operations on Objects', () => { .copyObject({ Bucket: 'bucket-b', Key: destKey, - CopySource: '/' + 'bucket-a' + '/' + srcKey, + CopySource: '/bucket-a/' + srcKey, MetadataDirective: 'REPLACE', Metadata: { someKey: 'value', @@ -1016,7 +997,7 @@ describe('Operations on Objects', () => { .copyObject({ Bucket: 'bucket-b', Key: destKey, - CopySource: '/' + 'bucket-a' + '/' + srcKey, + CopySource: '/bucket-a/' + srcKey, MetadataDirective: 'REPLACE', Metadata: { someKey: 'value', @@ -1049,7 +1030,7 @@ describe('Operations on Objects', () => { .copyObject({ Bucket: 'bucket-a', Key: key, - CopySource: '/' + 'bucket-a' + '/' + key, + CopySource: '/bucket-a/' + key, Metadata: { someKey: 'value', }, @@ -1069,7 +1050,7 @@ describe('Operations on Objects', () => { .copyObject({ Bucket: 'bucket-b', Key: 'image/jamie', - CopySource: '/' + 'bucket-a' + '/doesnotexist', + CopySource: '/bucket-a/doesnotexist', }) .promise(); } catch (err) { diff --git a/test/helpers.js b/test/helpers.js index a6cfc0be..de156246 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -50,6 +50,7 @@ exports.createServerAndClient = async function createServerAndClient(options) { endpoint: `http://localhost:${port}`, sslEnabled: false, s3ForcePathStyle: true, + signatureVersion: 'v2', }); return { s3rver, s3Client };