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.

Sign up for GitHub

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

BC-7811 - refactor getSignedUrl to use storageProviderId and bucket #5174

Merged
merged 8 commits into from
Aug 13, 2024
Merged

BC-7811 - refactor getSignedUrl to use storageProviderId and bucket #5174

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
da71f8d
BC-7811 - refactor `getSignedUrl` to use `storageProviderId` and `buc…
bergatco Aug 8, 2024
d013571
BC-7811 - use async/await for getStorageProviderIdAndBucket in proxy-…
bergatco Aug 8, 2024
7307043
BC-7811 - refactor new handling to use correct objects
bergatco Aug 9, 2024
02b2fa0
Merge branch 'main' into BC-7811-request-to-files-calls-for-wrong-s3
bergatco Aug 9, 2024
d5ee92c
Merge branch 'main' into BC-7811-request-to-files-calls-for-wrong-s3
bergatco Aug 9, 2024
9fccc4d
Merge branch 'main' into BC-7811-request-to-files-calls-for-wrong-s3
bergatco Aug 12, 2024
df1e2d4
Merge branch 'main' into BC-7811-request-to-files-calls-for-wrong-s3
bergatco Aug 12, 2024
d99fced
BC-7811 - remove TODO
bergatco Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 92 additions & 48 deletions src/services/fileStorage/proxy-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const { equal: equalIds } = require('../../helper/compare').ObjectId;
const {
FILE_PREVIEW_SERVICE_URI,
FILE_PREVIEW_CALLBACK_URI,
ENABLE_THUMBNAIL_GENERATION,
FILE_SECURITY_CHECK_MAX_FILE_SIZE,
SECURITY_CHECK_SERVICE_PATH,
} = require('../../../config/globals');
Expand All @@ -44,40 +43,89 @@ const sanitizeObj = (obj) => {
return obj;
};

const prepareThumbnailGeneration = (file, strategy, userId, { name: dataName }, { storageFileName, name: propName }) =>
ENABLE_THUMBNAIL_GENERATION
? Promise.all([
strategy.getSignedUrl({
userId,
flatFileName: storageFileName,
localFileName: storageFileName,
download: true,
Expires: 3600 * 24,
}),
strategy.generateSignedUrl({
userId,
flatFileName: storageFileName.replace(/(\..+)$/, '-thumbnail.png'),
fileType: returnFileType(dataName || propName), // data.type
}),
]).then(([downloadUrl, signedS3Url]) =>
rp
.post({
url: FILE_PREVIEW_SERVICE_URI,
body: {
downloadUrl,
signedS3Url,
callbackUrl: url.resolve(FILE_PREVIEW_CALLBACK_URI, file.thumbnailRequestToken),
options: {
width: 120,
},
const getStorageProviderIdAndBucket = async (userId, fileObject, strategy) => {
let storageProviderId = fileObject.storageProviderId;
let bucket = fileObject.bucket;

// TODO: How to handle, if only one is set?
if (!storageProviderId) {
// deprecated: author check via file.permissions[0]?.refId is deprecated and will be removed in the next release
const creatorId =
bergatco marked this conversation as resolved.
Show resolved Hide resolved
fileObject.creator ||
(fileObject.permissions[0]?.refPermModel !== 'user' ? userId : fileObject.permissions[0]?.refId);

const creator = await userModel.findById(creatorId).exec();
if (!creator || !creator.schoolId) {
throw new NotFound('User not found');
}

const { schoolId } = creator;

const school = await schoolModel
.findOne({ _id: schoolId }, null, { readPreference: 'primary' }) // primary for afterhook in school.create
.populate('storageProvider')
.select(['storageProvider'])
.lean()
.exec();
if (school === null) {
throw new NotFound('School not found.');
}

storageProviderId = school.storageProvider;
bucket = strategy.getBucket(schoolId);
}

return {
storageProviderId,
bucket,
};
};

const prepareThumbnailGeneration = (
file,
strategy,
userId,
{ name: dataName },
{ storageFileName, name: propName }
) => {
if (Configuration.get('ENABLE_THUMBNAIL_GENERATION') === true) {
const { storageProviderId, bucket } = getStorageProviderIdAndBucket(userId, file);

Promise.all([
strategy.getSignedUrl({
storageProviderId,
bucket,
flatFileName: storageFileName,
localFileName: storageFileName,
download: true,
Expires: 3600 * 24,
}),
strategy.generateSignedUrl({
userId,
flatFileName: storageFileName.replace(/(\..+)$/, '-thumbnail.png'),
fileType: returnFileType(dataName || propName), // data.type
}),
]).then(([downloadUrl, signedS3Url]) =>
rp
.post({
url: FILE_PREVIEW_SERVICE_URI,
body: {
downloadUrl,
signedS3Url,
callbackUrl: url.resolve(FILE_PREVIEW_CALLBACK_URI, file.thumbnailRequestToken),
options: {
width: 120,
},
json: true,
})
.catch((err) => {
logger.warning(new Error('Can not create tumbnail', err)); // todo err message is lost and throw error
})
)
: Promise.resolve();
},
json: true,
})
.catch((err) => {
logger.warning(new Error('Can not create tumbnail', err)); // todo err message is lost and throw error
})
);
}
return Promise.resolve();
};

/**
*
Expand All @@ -86,7 +134,7 @@ const prepareThumbnailGeneration = (file, strategy, userId, { name: dataName },
* @param {FileStorageStrategy} strategy the file storage strategy used
* @returns {Promise} Promise that rejects with errors or resolves with no data otherwise
*/
const prepareSecurityCheck = (file, userId, strategy) => {
const prepareSecurityCheck = async (file, userId, strategy) => {
if (Configuration.get('ENABLE_FILE_SECURITY_CHECK') === true) {
if (file.size > FILE_SECURITY_CHECK_MAX_FILE_SIZE) {
return FileModel.updateOne(
Expand All @@ -99,10 +147,12 @@ const prepareSecurityCheck = (file, userId, strategy) => {
}
).exec();
}
const { storageProviderId, bucket } = getStorageProviderIdAndBucket(userId, file);
// create a temporary signed URL and provide it to the virus scan service
return strategy
.getSignedUrl({
userId,
storageProviderId,
bucket,
flatFileName: file.storageFileName,
localFileName: file.storageFileName,
download: true,
Expand Down Expand Up @@ -422,10 +472,7 @@ const signedUrlService = {
throw new NotFound('File seems not to be there.');
}

// deprecated: author check via file.permissions[0]?.refId is deprecated and will be removed in the next release
const creatorId =
fileObject.creator ||
(fileObject.permissions[0]?.refPermModel !== 'user' ? userId : fileObject.permissions[0]?.refId);
const { storageProviderId, bucket } = getStorageProviderIdAndBucket(userId, file);

if (download && fileObject.securityCheck && fileObject.securityCheck.status === SecurityCheckStatusTypes.BLOCKED) {
throw new Forbidden('File access blocked by security check.');
Expand All @@ -434,11 +481,11 @@ const signedUrlService = {
return canRead(userId, file)
.then(() =>
strategy.getSignedUrl({
userId: creatorId,
storageProviderId,
bucket,
flatFileName: fileObject.storageFileName,
localFileName: query.name || fileObject.name,
download: true,
bucket: fileObject.bucket,
})
)
.then((res) => ({
Expand All @@ -457,16 +504,13 @@ const signedUrlService = {
throw new NotFound('File seems not to be there.');
}

// deprecated: author check via file.permissions[0]?.refId is deprecated and will be removed in the next release
const creatorId =
fileObject.creator || fileObject.permissions[0]?.refPermModel !== 'user'
? userId
: fileObject.permissions[0]?.refId;
const { storageProviderId, bucket } = getStorageProviderIdAndBucket(userId, file);

return canRead(userId, id)
.then(() =>
strategy.getSignedUrl({
userId: creatorId,
storageProviderId,
bucket,
flatFileName: fileObject.storageFileName,
action: 'putObject',
})
Expand Down
79 changes: 46 additions & 33 deletions src/services/fileStorage/strategies/awsS3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@

const getBucketName = (schoolId) => `${BUCKET_NAME_PREFIX}${schoolId}`;

const createAWSObject = async (schoolId) => {
const createAWSObjectFromSchoolId = async (schoolId) => {
const school = await schoolModel
.findOne({ _id: schoolId }, null, { readPreference: 'primary' }) // primary for afterhook in school.create
.populate('storageProvider')
Expand Down Expand Up @@ -152,6 +152,27 @@
// end legacy
};

const createAWSObjectFromStorageProviderIdAndBucket = async (storageProviderId, bucket) => {
if (Configuration.get('FEATURE_MULTIPLE_S3_PROVIDERS_ENABLED') === true) {
const s3 = getS3(storageProviderId);
return {
s3,
bucket,
};
}

// begin legacy
if (!awsConfig.endpointUrl) throw new Error('S3 integration is not configured on the server');
const config = new aws.Config(awsConfig);
config.endpoint = new aws.Endpoint(awsConfig.endpointUrl);

return {
s3: new aws.S3(config),
bucket,
};
// end legacy
};

/**
* split files-list in files, that are in current directory, and the sub-directories
* @param data is the files-list
Expand Down Expand Up @@ -305,7 +326,7 @@
throw new BadRequest('No school id parameter given.');
}

const awsObject = await createAWSObject(schoolId);
const awsObject = await createAWSObjectFromSchoolId(schoolId);
const data = await createBucket(awsObject);
return {
message: 'Successfully created s3-bucket!',
Expand Down Expand Up @@ -358,7 +379,7 @@
return new GeneralError('school not set');
}

return createAWSObject(result.schoolId).then((awsObject) => {
return createAWSObjectFromSchoolId(result.schoolId).then((awsObject) => {
const params = {
Bucket: awsObject.bucket,
Prefix: path,
Expand All @@ -384,7 +405,7 @@
return new NotFound('User not found');
}

return createAWSObject(result.schoolId).then((awsObject) => {
return createAWSObjectFromSchoolId(result.schoolId).then((awsObject) => {
// files can be copied to different schools
const sourceBucket = `bucket-${externalSchoolId || result.schoolId}`;

Expand Down Expand Up @@ -413,7 +434,7 @@
if (!result || !result.schoolId) {
return new NotFound('User not found');
}
return createAWSObject(result.schoolId).then((awsObject) => {
return createAWSObjectFromSchoolId(result.schoolId).then((awsObject) => {
const params = {
Bucket: awsObject.bucket,
Delete: {
Expand Down Expand Up @@ -444,7 +465,7 @@
if (!result || !result.schoolId) {
return new NotFound('User not found');
}
return createAWSObject(result.schoolId).then((awsObject) =>
return createAWSObjectFromSchoolId(result.schoolId).then((awsObject) =>
this.createIfNotExists(awsObject).then((safeAwsObject) => {
const params = {
Bucket: safeAwsObject.bucket,
Expand All @@ -462,33 +483,25 @@
});
}

getSignedUrl({ userId, flatFileName, localFileName, download, action = 'getObject', bucket = undefined }) {
if (!userId || !flatFileName) {
return Promise.reject(new BadRequest('Missing parameters by getSignedUrl.', { userId, flatFileName }));
getSignedUrl({ storageProviderId, bucket, flatFileName, localFileName, download, action = 'getObject' }) {
if (!storageProviderId || !bucket || !flatFileName) {
return Promise.reject(
new BadRequest('Missing parameters by getSignedUrl.', { storageProviderId, bucket, flatFileName })
);
}

return UserModel.userModel
.findById(userId)
.lean()
.exec()
.then((result) => {
if (!result || !result.schoolId) {
return new NotFound('User not found');
}

return createAWSObject(result.schoolId).then((awsObject) => {
const params = {
Bucket: bucket || awsObject.bucket,
Key: flatFileName,
Expires: Configuration.get('STORAGE_SIGNED_URL_EXPIRE'),
};
const getBoolean = (value) => value === true || value === 'true';
if (getBoolean(download)) {
params.ResponseContentDisposition = `attachment; filename = "${localFileName.replace('"', '')}"`;
}
return promisify(awsObject.s3.getSignedUrl.bind(awsObject.s3), awsObject.s3)(action, params);
});
});
return createAWSObjectFromStorageProviderIdAndBucket(storageProviderId, bucket).then((awsObject) => {
const params = {
Bucket: bucket,
Key: flatFileName,
Expires: Configuration.get('STORAGE_SIGNED_URL_EXPIRE'),
};
const getBoolean = (value) => value === true || value === 'true';
if (getBoolean(download)) {
params.ResponseContentDisposition = `attachment; filename = "${localFileName.replace('"', '')}"`;
Dismissed Show dismissed Hide dismissed
}
return promisify(awsObject.s3.getSignedUrl.bind(awsObject.s3), awsObject.s3)(action, params);
});
}

/** ** @DEPRECATED *** */
Expand All @@ -508,7 +521,7 @@
return new NotFound('User not found');
}

return createAWSObject(result.schoolId).then((awsObject) => {
return createAWSObjectFromSchoolId(result.schoolId).then((awsObject) => {
const fileStream = fs.createReadStream(pathUtil.join(__dirname, '..', 'resources', '.scfake'));
const params = {
Bucket: awsObject.bucket,
Expand Down Expand Up @@ -539,7 +552,7 @@
if (!result || !result.schoolId) {
return new NotFound('User not found');
}
return createAWSObject(result.schoolId).then((awsObject) => {
return createAWSObjectFromSchoolId(result.schoolId).then((awsObject) => {
const params = {
Bucket: awsObject.bucket,
Prefix: removeLeadingSlash(path),
Expand Down
Loading