Skip to content

Commit

Permalink
Merge branch 'bugfix/CLDSRV-489-redirect-folder-index' into q/7.10
Browse files Browse the repository at this point in the history
  • Loading branch information
@bert-e
bert-e committed Jan 15, 2024
2 parents c7c5545 + 557f3dc commit 30fb64e
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 83 deletions.
13 changes: 8 additions & 5 deletions lib/api/apiUtils/object/websiteServing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,24 @@ function validateWebsiteHeader(header) {
* appendWebsiteIndexDocument - append index to objectKey if necessary
* @param {object} request - normalized request object
* @param {string} indexDocumentSuffix - index document from website config
* @param {boolean} force - flag to force append index
* @return {undefined}
*/
function appendWebsiteIndexDocument(request, indexDocumentSuffix) {
function appendWebsiteIndexDocument(request, indexDocumentSuffix, force = false) {
const reqObjectKey = request.objectKey ? request.objectKey : '';
/* eslint-disable no-param-reassign */

// find index document if "directory" sent in request
if (reqObjectKey.endsWith('/')) {
// eslint-disable-next-line no-param-reassign
request.objectKey += indexDocumentSuffix;
}
// find index document if no key provided
if (reqObjectKey === '') {
// eslint-disable-next-line no-param-reassign
} else if (reqObjectKey === '') {
request.objectKey = indexDocumentSuffix;
// force for redirect 302 on folder without trailing / that has an index
} else if (force) {
request.objectKey += `/${indexDocumentSuffix}`;
}
/* eslint-enable no-param-reassign */
}

module.exports = {
Expand Down
184 changes: 108 additions & 76 deletions lib/api/website.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function _errorActions(err, errorDocument, routingRules,
// eslint-disable-next-line no-param-reassign
request.objectKey = errorDocument;
if (!isObjAuthorized(bucket, errObjMD, request.apiMethods || 'objectGet',
constants.publicId, null, log, request, request.actionImplicitDenies)) {
constants.publicId, null, log, request, request.actionImplicitDenies, true)) {
log.trace('errorObj not authorized', { error: err });
return callback(err, true, null, corsHeaders);
}
Expand Down Expand Up @@ -170,87 +170,119 @@ function website(request, log, callback) {

appendWebsiteIndexDocument(request, websiteConfig.getIndexDocument());

// get object metadata and check authorization and header
// validation
return metadata.getObjectMD(bucketName, request.objectKey, {}, log,
(err, objMD) => {
// Note: In case of error, we intentionally send the original
// object key to _errorActions as in case of a redirect, we do
// not want to append index key to redirect location
if (err) {
log.trace('error retrieving object metadata',
{ error: err });
let returnErr = err;
const bucketAuthorized = isBucketAuthorized(bucket, request.apiMethods || 'bucketGet',
constants.publicId, null, log, request, request.actionImplicitDenies, true);
// if index object does not exist and bucket is private AWS
// returns 403 - AccessDenied error.
if (err.is.NoSuchKey && !bucketAuthorized) {
returnErr = errors.AccessDenied;
/**
* Recursive function with 1 recursive call to look for index
* in case of error for potential redirect to folder notation
* if there is not already an index appended
* @param {Error} [originalError] - presence of this argument
* differentiates original user request from recursive call to /index.
* This error is returned if /index is not found
* @returns {undefined}
*/
function runWebsite(originalError) {
// get object metadata and check authorization and header
// validation
return metadata.getObjectMD(bucketName, request.objectKey, {}, log,
(err, objMD) => {
// Note: In case of error, we intentionally send the original
// object key to _errorActions as in case of a redirect, we do
// not want to append index key to redirect location
if (err) {
log.trace('error retrieving object metadata',
{ error: err });

let returnErr = err;
const bucketAuthorized = isBucketAuthorized(bucket, request.apiMethods || 'bucketGet',
constants.publicId, null, log, request, request.actionImplicitDenies, true);
// if index object does not exist and bucket is private AWS
// returns 403 - AccessDenied error.
if (err.is.NoSuchKey && !bucketAuthorized) {
returnErr = errors.AccessDenied;
}

// Check if key is a folder containing index for redirect 302
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/IndexDocumentSupport.html
if (!originalError && reqObjectKey && !reqObjectKey.endsWith('/')) {
appendWebsiteIndexDocument(request, websiteConfig.getIndexDocument(), true);
// propagate returnErr as originalError to be used if index is not found
return runWebsite(returnErr);
}

return _errorActions(originalError || returnErr,
websiteConfig.getErrorDocument(), routingRules,
bucket, reqObjectKey, corsHeaders, request, log,
callback);
}
if (!isObjAuthorized(bucket, objMD, request.apiMethods || 'objectGet',
constants.publicId, null, log, request, request.actionImplicitDenies, true)) {
const err = errors.AccessDenied;
log.trace('request not authorized', { error: err });
return _errorActions(err, websiteConfig.getErrorDocument(),
routingRules, bucket,
reqObjectKey, corsHeaders, request, log, callback);
}
return _errorActions(returnErr,
websiteConfig.getErrorDocument(), routingRules,
bucket, reqObjectKey, corsHeaders, request, log,
callback);
}
if (!isObjAuthorized(bucket, objMD, request.apiMethods || 'objectGet',
constants.publicId, null, log, request, request.actionImplicitDenies, true)) {
const err = errors.AccessDenied;
log.trace('request not authorized', { error: err });
return _errorActions(err, websiteConfig.getErrorDocument(),
routingRules, bucket,
reqObjectKey, corsHeaders, request, log, callback);
}

const headerValResult = validateHeaders(request.headers,
objMD['last-modified'], objMD['content-md5']);
if (headerValResult.error) {
const err = headerValResult.error;
log.trace('header validation error', { error: err });
return _errorActions(err, websiteConfig.getErrorDocument(),
routingRules, bucket, reqObjectKey,
corsHeaders, request, log, callback);
}
// check if object to serve has website redirect header
// Note: AWS prioritizes website configuration rules over
// object key's website redirect header, so we make the
// check at the end.
if (objMD['x-amz-website-redirect-location']) {
const redirectLocation =
objMD['x-amz-website-redirect-location'];
const redirectInfo =
extractRedirectInfo(redirectLocation);
log.trace('redirecting to x-amz-website-redirect-location',
{ location: redirectLocation });
return callback(null, false, null, corsHeaders,
redirectInfo, '');
}
// got obj metadata, authorized and headers validated,
// good to go
const responseMetaHeaders = collectResponseHeaders(objMD,
corsHeaders);
// access granted to index document, needs a redirect 302
// to the original key with trailing /
if (originalError) {
const redirectInfo = { withError: true,
location: `/${reqObjectKey}/` };
return callback(errors.Found, false, null, corsHeaders,
redirectInfo, '');
}

if (request.method === 'HEAD') {
pushMetric('headObject', log, { bucket: bucketName });
return callback(null, false, null, responseMetaHeaders);
}
const headerValResult = validateHeaders(request.headers,
objMD['last-modified'], objMD['content-md5']);
if (headerValResult.error) {
const err = headerValResult.error;
log.trace('header validation error', { error: err });
return _errorActions(err, websiteConfig.getErrorDocument(),
routingRules, bucket, reqObjectKey,
corsHeaders, request, log, callback);
}
// check if object to serve has website redirect header
// Note: AWS prioritizes website configuration rules over
// object key's website redirect header, so we make the
// check at the end.
if (objMD['x-amz-website-redirect-location']) {
const redirectLocation =
objMD['x-amz-website-redirect-location'];
const redirectInfo =
extractRedirectInfo(redirectLocation);
log.trace('redirecting to x-amz-website-redirect-location',
{ location: redirectLocation });
return callback(null, false, null, corsHeaders,
redirectInfo, '');
}
// got obj metadata, authorized and headers validated,
// good to go
const responseMetaHeaders = collectResponseHeaders(objMD,
corsHeaders);

const dataLocator = objMD.location;
if (objMD['x-amz-server-side-encryption']) {
for (let i = 0; i < dataLocator.length; i++) {
dataLocator[i].masterKeyId =
objMD['x-amz-server-side-encryption-aws-' +
'kms-key-id'];
dataLocator[i].algorithm =
objMD['x-amz-server-side-encryption'];
if (request.method === 'HEAD') {
pushMetric('headObject', log, { bucket: bucketName });
return callback(null, false, null, responseMetaHeaders);
}
}
pushMetric('getObject', log, {
bucket: bucketName,
newByteLength: responseMetaHeaders['Content-Length'],

const dataLocator = objMD.location;
if (objMD['x-amz-server-side-encryption']) {
for (let i = 0; i < dataLocator.length; i++) {
dataLocator[i].masterKeyId =
objMD['x-amz-server-side-encryption-aws-' +
'kms-key-id'];
dataLocator[i].algorithm =
objMD['x-amz-server-side-encryption'];
}
}
pushMetric('getObject', log, {
bucket: bucketName,
newByteLength: responseMetaHeaders['Content-Length'],
});
return callback(null, false, dataLocator, responseMetaHeaders);
});
return callback(null, false, dataLocator, responseMetaHeaders);
});
}

return runWebsite();
});
}

Expand Down
4 changes: 2 additions & 2 deletions tests/functional/aws-node-sdk/lib/utility/website-util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ function _assertResponseHtmlRedirect(response, type, redirectUrl, method,
assert.strictEqual(response.headers['x-amz-error-message'],
'Resource Found');
_assertContainsHtml(response.body);
_assertResponseHtml(response.body, 'title', '302 Moved Temporarily');
_assertResponseHtml(response.body, 'h1', '302 Moved Temporarily');
_assertResponseHtml(response.body, 'title', '302 Found');
_assertResponseHtml(response.body, 'h1', '302 Found');
_assertResponseHtml(response.body, 'ul', [
'Code: Found',
'Message: Resource Found',
Expand Down
98 changes: 98 additions & 0 deletions tests/functional/aws-node-sdk/test/object/websiteGet.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,5 +761,103 @@ describe('User visits bucket website endpoint', () => {
}, done);
});
});

describe('without trailing / for recursive index check', () => {
beforeEach(done => {
const webConfig = new WebsiteConfigTester('index.html');
const object = {
Bucket: bucket,
Body: fs.readFileSync(path.join(__dirname,
'/websiteFiles/index.html')),
ContentType: 'text/html',
};
async.waterfall([
next => s3.putBucketWebsite({ Bucket: bucket,
WebsiteConfiguration: webConfig }, next),
(data, next) => s3.putBucketPolicy({ Bucket: bucket,
Policy: JSON.stringify({
Version: '2012-10-17',
Statement: [{
Sid: 'PublicReadGetObject',
Effect: 'Allow',
Principal: '*',
Action: ['s3:GetObject'],
Resource: [
`arn:aws:s3:::${bucket}/original_key_file`,
`arn:aws:s3:::${bucket}/original_key_nofile`,
`arn:aws:s3:::${bucket}/file/*`,
`arn:aws:s3:::${bucket}/nofile/*`,
],
}],
}),
}, next),
(data, next) => s3.putObject(Object.assign({}, object,
{ Key: 'original_key_file/index.html' }), next),
(data, next) => s3.putObject(Object.assign({}, object,
{ Key: 'file/index.html' }), next), // the redirect 302
(data, next) => s3.putObject(Object.assign({}, object,
{ Key: 'no_access_file/index.html' }), next),
], err => {
assert.ifError(err);
done();
});
});

afterEach(done => {
async.waterfall([
next => s3.deleteObject({ Bucket: bucket,
Key: 'original_key_file/index.html' }, next),
(data, next) => s3.deleteObject({ Bucket: bucket,
Key: 'file/index.html' }, next),
(data, next) => s3.deleteObject({ Bucket: bucket,
Key: 'no_access_file/index.html' }, next),
], err => {
assert.ifError(err);
done();
});
});

it('should redirect 302 with trailing / on folder with index', done => {
WebsiteConfigTester.checkHTML({
method: 'GET',
url: `${endpoint}/file`,
responseType: 'redirect-error-found',
redirectUrl: '/file/',
}, done);
});

it('should return 404 on original key access without index',
done => {
WebsiteConfigTester.checkHTML({
method: 'GET',
url: `${endpoint}/original_key_nofile`,
responseType: '404-not-found',
}, done);
});

describe('should return 403', () => {
[
{
it: 'on original key access with index no access',
key: 'original_key_file',
},
{
it: 'on folder access without index',
key: 'nofile',
},
{
it: 'on no access with index',
key: 'no_access_file',
},
].forEach(test =>
it(test.it, done => {
WebsiteConfigTester.checkHTML({
method: 'GET',
url: `${endpoint}/${test.key}`,
responseType: '403-access-denied',
}, done);
}));
});
});
});
});
Loading

0 comments on commit 30fb64e

Please sign in to comment.