From 7072e282b1cd1a296d81512c57130237610c1c1e Mon Sep 17 00:00:00 2001 From: Saad Jutt Date: Mon, 14 Mar 2022 05:05:59 +0500 Subject: [PATCH] fix(drive): update file API is same as create file --- api/public/swagger.yaml | 48 ++++---- api/src/controllers/drive.ts | 52 +++++---- api/src/routes/api/drive.ts | 34 ++++-- api/src/routes/api/spec/drive.spec.ts | 160 ++++++++++++++++++++++++++ web/src/containers/Drive/main.tsx | 12 +- 5 files changed, 247 insertions(+), 59 deletions(-) diff --git a/api/public/swagger.yaml b/api/public/swagger.yaml index e22f671..47d2cf1 100644 --- a/api/public/swagger.yaml +++ b/api/public/swagger.yaml @@ -233,21 +233,6 @@ components: - status type: object additionalProperties: false - FilePayload: - properties: - filePath: - type: string - description: 'Path of the file' - example: /Public/somefolder/some.file - fileContent: - type: string - description: 'Contents of the file' - example: 'Contents of the File' - required: - - filePath - - fileContent - type: object - additionalProperties: false TreeNode: properties: name: @@ -626,7 +611,7 @@ paths: examples: 'Example 1': value: {status: success} - '400': + '403': description: 'File already exists' content: application/json: @@ -635,7 +620,7 @@ paths: examples: 'Example 1': value: {status: failure, message: 'File request failed.'} - description: "It's optional to either provide `_filePath` in url as query parameter\nOr provide `filePath` in body as form field.\nBut it's required to provided else API will respond with Bad Request." + description: "It's optional to either provide `_filePath` in url as query parameter\nOr provide `filePath` in body as form field.\nBut it's required to provide else API will respond with Bad Request." summary: 'Create a file in SASjs Drive' tags: - Drive @@ -677,8 +662,8 @@ paths: examples: 'Example 1': value: {status: success} - '400': - description: 'Unable to get File' + '403': + description: "" content: application/json: schema: @@ -686,19 +671,36 @@ paths: examples: 'Example 1': value: {status: failure, message: 'File request failed.'} + description: "It's optional to either provide `_filePath` in url as query parameter\nOr provide `filePath` in body as form field.\nBut it's required to provide else API will respond with Bad Request." summary: 'Modify a file in SASjs Drive' tags: - Drive security: - bearerAuth: [] - parameters: [] + parameters: + - + description: 'Location of SAS program' + in: query + name: _filePath + required: false + schema: + type: string + example: /Public/somefolder/some.file.sas requestBody: required: true content: - application/json: + multipart/form-data: schema: - $ref: '#/components/schemas/FilePayload' + type: object + properties: + file: + type: string + format: binary + filePath: + type: string + required: + - file /SASjsApi/drive/filetree: get: operationId: GetFileTree @@ -1054,7 +1056,7 @@ paths: anyOf: - {type: string} - {type: string, format: byte} - description: "Trigger a SAS program using it's location in the _program URL parameter.\nEnable debugging using the _debug URL parameter. Setting _debug=131 will\ncause the log to be streamed in the output.\n\nAdditional URL parameters are turned into SAS macro variables.\n\nAny files provided in the request body are placed into the SAS session with\ncorresponding _WEBIN_XXX variables created.\n\nThe response headers can be adjusted using the mfs_httpheader() macro. Any\nfile type can be returned, including binary files such as zip or xls.\n\nThis behaviour differs for POST requests, in which case the reponse is\nalways JSON." + description: "Trigger a SAS program using it's location in the _program URL parameter.\nEnable debugging using the _debug URL parameter. Setting _debug=131 will\ncause the log to be streamed in the output.\n\nAdditional URL parameters are turned into SAS macro variables.\n\nAny files provided in the request body are placed into the SAS session with\ncorresponding _WEBIN_XXX variables created.\n\nThe response headers can be adjusted using the mfs_httpheader() macro. Any\nfile type can be returned, including binary files such as zip or xls.\n\nIf _debug is >= 131, response headers will contain Content-Type: 'text/plain'\n\nThis behaviour differs for POST requests, in which case the response is\nalways JSON." summary: 'Execute Stored Program, return raw _webout content.' tags: - STP diff --git a/api/src/controllers/drive.ts b/api/src/controllers/drive.ts index 154557e..d8174a1 100644 --- a/api/src/controllers/drive.ts +++ b/api/src/controllers/drive.ts @@ -108,7 +108,7 @@ export class DriveController { /** * It's optional to either provide `_filePath` in url as query parameter * Or provide `filePath` in body as form field. - * But it's required to provided else API will respond with Bad Request. + * But it's required to provide else API will respond with Bad Request. * * @summary Create a file in SASjs Drive * @param _filePath Location of SAS program @@ -118,7 +118,7 @@ export class DriveController { @Example({ status: 'success' }) - @Response(400, 'File already exists', { + @Response(403, 'File already exists', { status: 'failure', message: 'File request failed.' }) @@ -132,21 +132,29 @@ export class DriveController { } /** + * It's optional to either provide `_filePath` in url as query parameter + * Or provide `filePath` in body as form field. + * But it's required to provide else API will respond with Bad Request. + * * @summary Modify a file in SASjs Drive + * @param _filePath Location of SAS program + * @example _filePath "/Public/somefolder/some.file.sas" * */ @Example({ status: 'success' }) - @Response(400, 'Unable to get File', { + @Response(403, `File doesn't exist`, { status: 'failure', message: 'File request failed.' }) @Patch('/file') public async updateFile( - @Body() body: FilePayload + @UploadedFile() file: Express.Multer.File, + @Query() _filePath?: string, + @FormField() filePath?: string ): Promise { - return updateFile(body) + return updateFile((_filePath ?? filePath)!, file) } /** @@ -227,25 +235,27 @@ const saveFile = async ( return { status: 'success' } } -const updateFile = async (body: FilePayload): Promise => { - const { filePath, fileContent } = body - try { - const filePathFull = path - .join(getTmpFilesFolderPath(), filePath) - .replace(new RegExp('/', 'g'), path.sep) +const updateFile = async ( + filePath: string, + multerFile: Express.Multer.File +): Promise => { + const driveFilesPath = getTmpFilesFolderPath() - await validateFilePath(filePathFull) - await createFile(filePathFull, fileContent) + const filePathFull = path + .join(driveFilesPath, filePath) + .replace(new RegExp('/', 'g'), path.sep) - return { status: 'success' } - } catch (err: any) { - throw { - code: 400, - status: 'failure', - message: 'File request failed.', - error: typeof err === 'object' ? err.toString() : err - } + if (!filePathFull.includes(driveFilesPath)) { + throw new Error('Cannot modify file outside drive.') } + + if (!(await fileExists(filePathFull))) { + throw new Error(`File doesn't exist.`) + } + + await moveFile(multerFile.path, filePathFull) + + return { status: 'success' } } const validateFilePath = async (filePath: string) => { diff --git a/api/src/routes/api/drive.ts b/api/src/routes/api/drive.ts index bee9375..102ada3 100644 --- a/api/src/routes/api/drive.ts +++ b/api/src/routes/api/drive.ts @@ -66,21 +66,33 @@ driveRouter.post( } ) -driveRouter.patch('/file', async (req, res) => { - const { error, value: body } = updateFileDriveValidation(req.body) - if (error) return res.status(400).send(error.details[0].message) +driveRouter.patch( + '/file', + (...arg) => multerSingle('file', arg), + async (req, res) => { + const { error: errQ, value: query } = uploadFileParamValidation(req.query) + const { error: errB, value: body } = uploadFileBodyValidation(req.body) - try { - const response = await controller.updateFile(body) - res.send(response) - } catch (err: any) { - const statusCode = err.code + if (errQ && errB) { + if (req.file) await deleteFile(req.file.path) + return res.status(400).send(errB.details[0].message) + } - delete err.code + if (!req.file) return res.status(400).send('"file" is not present.') - res.status(statusCode).send(err) + try { + const response = await controller.updateFile( + req.file, + query._filePath, + body.filePath + ) + res.send(response) + } catch (err: any) { + await deleteFile(req.file.path) + res.status(403).send(err.toString()) + } } -}) +) driveRouter.get('/fileTree', async (req, res) => { try { diff --git a/api/src/routes/api/spec/drive.spec.ts b/api/src/routes/api/spec/drive.spec.ts index 1f70a56..1d979bb 100644 --- a/api/src/routes/api/spec/drive.spec.ts +++ b/api/src/routes/api/spec/drive.spec.ts @@ -321,6 +321,166 @@ describe('files', () => { expect(res.body).toEqual({}) }) }) + + describe('update', () => { + it('should update a SAS file on drive having filePath as form field', async () => { + const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') + const pathToUpload = '/my/path/code.sas' + + const pathToCopy = path.join( + fileUtilModules.getTmpFilesFolderPath(), + pathToUpload + ) + await copy(fileToAttachPath, pathToCopy) + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .attach('file', fileToAttachPath) + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ + status: 'success' + }) + }) + + it('should update a SAS file on drive having _filePath as query param', async () => { + const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') + const pathToUpload = '/my/path/code.sas' + + const pathToCopy = path.join( + fileUtilModules.getTmpFilesFolderPath(), + pathToUpload + ) + await copy(fileToAttachPath, pathToCopy) + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .attach('file', fileToAttachPath) + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ + status: 'success' + }) + }) + + it('should respond with Unauthorized if access token is not present', async () => { + const res = await request(app) + .patch('/SASjsApi/drive/file') + .field('filePath', '/my/path/code.sas') + .attach('file', path.join(__dirname, 'files', 'sample.sas')) + .expect(401) + + expect(res.text).toEqual('Unauthorized') + expect(res.body).toEqual({}) + }) + + it('should respond with Forbidden if file is not present', async () => { + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', `/my/path/code-${generateTimestamp()}.sas`) + .attach('file', path.join(__dirname, 'files', 'sample.sas')) + .expect(403) + + expect(res.text).toEqual(`Error: File doesn't exist.`) + expect(res.body).toEqual({}) + }) + + it('should respond with Forbidden if filePath outside Drive', async () => { + const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') + const pathToUpload = '/../path/code.sas' + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .attach('file', fileToAttachPath) + .expect(403) + + expect(res.text).toEqual('Error: Cannot modify file outside drive.') + expect(res.body).toEqual({}) + }) + + it('should respond with Bad Request if filePath is missing', async () => { + const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .attach('file', fileToAttachPath) + .expect(400) + + expect(res.text).toEqual(`"filePath" is required`) + expect(res.body).toEqual({}) + }) + + it("should respond with Bad Request if filePath doesn't has correct extension", async () => { + const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') + const pathToUpload = '/my/path/code.oth' + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .attach('file', fileToAttachPath) + .expect(400) + + expect(res.text).toEqual('Valid extensions for filePath: .sas') + expect(res.body).toEqual({}) + }) + + it('should respond with Bad Request if file is missing', async () => { + const pathToUpload = '/my/path/code.sas' + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .expect(400) + + expect(res.text).toEqual('"file" is not present.') + expect(res.body).toEqual({}) + }) + + it("should respond with Bad Request if attached file doesn't has correct extension", async () => { + const fileToAttachPath = path.join(__dirname, 'files', 'sample.oth') + const pathToUpload = '/my/path/code.sas' + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .attach('file', fileToAttachPath) + .expect(400) + + expect(res.text).toEqual( + `File extension '.oth' not acceptable. Valid extension(s): .sas` + ) + expect(res.body).toEqual({}) + }) + + it('should respond with Bad Request if attached file exceeds file limit', async () => { + const pathToUpload = '/my/path/code.sas' + + const attachedFile = Buffer.from('.'.repeat(20 * 1024 * 1024)) + + const res = await request(app) + .patch('/SASjsApi/drive/file') + .auth(accessToken, { type: 'bearer' }) + .field('filePath', pathToUpload) + .attach('file', attachedFile, 'another.sas') + .expect(400) + + expect(res.text).toEqual( + 'File size is over limit. File limit is: 10 MB' + ) + expect(res.body).toEqual({}) + }) + }) }) }) diff --git a/web/src/containers/Drive/main.tsx b/web/src/containers/Drive/main.tsx index 365b950..7fbf730 100644 --- a/web/src/containers/Drive/main.tsx +++ b/web/src/containers/Drive/main.tsx @@ -42,11 +42,15 @@ const Main = (props: any) => { setEditMode(true) } else { setIsLoading(true) + + const formData = new FormData() + + const stringBlob = new Blob([fileContent], { type: 'text/plain' }) + formData.append('file', stringBlob, 'filename.sas') + formData.append('filePath', props.selectedFilePath) + axios - .patch(`/SASjsApi/drive/file`, { - filePath: props.selectedFilePath, - fileContent: fileContent - }) + .patch(`/SASjsApi/drive/file`, formData) .then((res) => { setEditMode(false) })