From d6d71fdd5f87440b8209319c3ccb9cdb3c44edf4 Mon Sep 17 00:00:00 2001 From: Csaky Date: Wed, 19 Jun 2024 17:15:57 -0700 Subject: [PATCH] Accept array of permCodes for user creating a bucket --- app/src/controllers/bucket.js | 8 ++++++-- app/src/docs/v1.api-spec.yaml | 14 ++++++++++++-- app/src/services/bucket.js | 19 ++++++++++--------- app/src/validators/bucket.js | 4 +++- app/tests/unit/controllers/bucket.spec.js | 5 +++-- app/tests/unit/services/bucket.spec.js | 1 + 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/app/src/controllers/bucket.js b/app/src/controllers/bucket.js index 5d8ee28f..849ff8f8 100644 --- a/app/src/controllers/bucket.js +++ b/app/src/controllers/bucket.js @@ -2,7 +2,7 @@ const Problem = require('api-problem'); const { UniqueViolationError } = require('objection'); const { NIL: SYSTEM_USER } = require('uuid'); -const { DEFAULTREGION } = require('../components/constants'); +const { DEFAULTREGION, Permissions } = require('../components/constants'); const errorToProblem = require('../components/errorToProblem'); const log = require('../components/log')(module.filename); const { @@ -117,11 +117,15 @@ const controller = { await controller._validateCredentials(data); data.userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER)); + // if permCodes array (eg: ['READ', 'UPDATE'] or []) was provided use that (de-duped), + // otherwise assign all permissions + data.permCodes = req.body.permCodes ? Array.from(new Set(req.body.permCodes)) : Object.values(Permissions); + response = await bucketService.create(data); } catch (e) { // If bucket exists, check if credentials precisely match if (e instanceof UniqueViolationError) { - // Grant all permissions if credentials precisely match + // Grant permissions if credentials precisely match response = await bucketService.checkGrantPermissions(data).catch(permErr => { next(new Problem(403, { detail: permErr.message, instance: req.originalUrl })); }); diff --git a/app/src/docs/v1.api-spec.yaml b/app/src/docs/v1.api-spec.yaml index fa1fb8b2..31e60b1e 100644 --- a/app/src/docs/v1.api-spec.yaml +++ b/app/src/docs/v1.api-spec.yaml @@ -67,8 +67,9 @@ paths: description: >- Creates a bucket record. Bucket should exist in S3. If the set of `bucket`, `endpoint` and `key` match an existing record, the user will - be added to that existing bucket with full permissions instead of - generating a new bucket record. + be added to that existing bucket with the provided permissions instead of + generating a new bucket record. This endpoint can be used to grant the current user + permission to upload to a new or existing bucket. operationId: createBucket tags: - Bucket @@ -2395,6 +2396,15 @@ components: type: string default: / example: coms/env + permCodes: + type: array + items: + type: string + description: >- + Optional array of permCodes for each permission being given to the current user. + Accepts any of `"READ", "CREATE", "UPDATE", "DELETE", "MANAGE"`. + Defaults to all permCodes if unspecified. + example: ["READ", "CREATE", "UPDATE"] Request-CreateBucketChild: title: Request Create Bucket Child type: object diff --git a/app/src/services/bucket.js b/app/src/services/bucket.js index 9412465a..13bda63c 100644 --- a/app/src/services/bucket.js +++ b/app/src/services/bucket.js @@ -1,7 +1,6 @@ const { v4: uuidv4, NIL: SYSTEM_USER } = require('uuid'); const bucketPermissionService = require('./bucketPermission'); -const { Permissions } = require('../components/constants'); const { Bucket } = require('../db/models'); /** @@ -10,12 +9,13 @@ const { Bucket } = require('../db/models'); const service = { /** * @function checkGrantPermissions - * Grants a user full permissions to the bucket if the data precisely matches + * Grants a user provided permissions to the bucket if the data precisely matches * accessKeyId and secretAccessKey values. * @param {string} data.accessKeyId The S3 bucket access key id * @param {string} data.bucket The S3 bucket identifier * @param {string} data.endpoint The S3 bucket endpoint * @param {string} data.key The relative S3 key/subpath managed by this bucket + * @param {string} data.permCodes Permissions to give to current user for the bucket * @param {string} data.secretAccessKey The S3 bucket secret access key * @param {object} [etrx=undefined] An optional Objection Transaction object * @returns {Promise} The result of running the insert operation @@ -37,9 +37,9 @@ const service = { bucket.accessKeyId === data.accessKeyId && bucket.secretAccessKey === data.secretAccessKey ) { - // Add all permission codes for the uploader - if (data.userId && data.userId !== SYSTEM_USER) { - const perms = Object.values(Permissions).map((p) => ({ + // Add permissions + if (data.permCodes.length && data.userId && data.userId !== SYSTEM_USER) { + const perms = data.permCodes.map((p) => ({ userId: data.userId, permCode: p })); @@ -59,12 +59,13 @@ const service = { /** * @function create - * Create a bucket record and give the uploader (if authed) permissions + * Create a bucket record and give the current user the provided permissions * @param {string} data.bucketName The user-defined bucket name identifier * @param {string} data.accessKeyId The S3 bucket access key id * @param {string} data.bucket The S3 bucket identifier * @param {string} data.endpoint The S3 bucket endpoint * @param {string} data.key The relative S3 key/subpath managed by this bucket + * @param {string} data.permCodes Permissions to give to current user for the bucket * @param {string} data.secretAccessKey The S3 bucket secret access key * @param {string} [data.region] The optional S3 bucket region * @param {boolean} [data.active] The optional active flag - defaults to true if undefined @@ -93,9 +94,9 @@ const service = { const response = await Bucket.query(trx).insert(obj).returning('*'); - // Add all permission codes for the uploader - if (data.userId && data.userId !== SYSTEM_USER) { - const perms = Object.values(Permissions).map((p) => ({ + // Add permissions for a current oidc user + if (data.permCodes.length && data.userId && data.userId !== SYSTEM_USER) { + const perms = data.permCodes.map((p) => ({ userId: data.userId, permCode: p })); diff --git a/app/src/validators/bucket.js b/app/src/validators/bucket.js index 41020c29..93599644 100644 --- a/app/src/validators/bucket.js +++ b/app/src/validators/bucket.js @@ -2,6 +2,7 @@ const Joi = require('joi'); const { scheme, type } = require('./common'); const { validate } = require('../middleware/validation'); +const { Permissions } = require('../components/constants'); const schema = { createBucket: { @@ -13,7 +14,8 @@ const schema = { key: Joi.string().max(255).trim().strict(), secretAccessKey: Joi.string().max(255).required(), region: Joi.string().max(255), - active: type.truthy + active: type.truthy, + permCodes: Joi.array().items(...Object.values(Permissions)) }).required(), }, diff --git a/app/tests/unit/controllers/bucket.spec.js b/app/tests/unit/controllers/bucket.spec.js index ffbd6d45..3118aa65 100644 --- a/app/tests/unit/controllers/bucket.spec.js +++ b/app/tests/unit/controllers/bucket.spec.js @@ -9,6 +9,7 @@ const { userService, } = require('../../../src/services'); const utils = require('../../../src/components/utils'); +const { Permissions } = require('../../../src/components/constants'); const mockResponse = () => { const res = {}; @@ -74,7 +75,7 @@ describe('createBucket', () => { expect(getCurrentUserIdSpy).toHaveBeenCalledTimes(1); expect(getCurrentUserIdSpy).toHaveBeenCalledWith(USR_IDENTITY); expect(createSpy).toHaveBeenCalledTimes(1); - expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID }); + expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID, permCodes: Object.values(Permissions) }); expect(res.status).toHaveBeenCalledWith(201); }); @@ -168,7 +169,7 @@ describe('createBucket', () => { expect(getCurrentUserIdSpy).toHaveBeenCalledTimes(1); expect(getCurrentUserIdSpy).toHaveBeenCalledWith(USR_IDENTITY); expect(createSpy).toHaveBeenCalledTimes(1); - expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID }); + expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID, permCodes: Object.values(Permissions) }); expect(next).toHaveBeenCalledTimes(1); expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error')); diff --git a/app/tests/unit/services/bucket.spec.js b/app/tests/unit/services/bucket.spec.js index a834416f..168f74bf 100644 --- a/app/tests/unit/services/bucket.spec.js +++ b/app/tests/unit/services/bucket.spec.js @@ -34,6 +34,7 @@ const data = { secretAccessKey: 'secretaccesskey', region: 'region', active: 'true', + permCodes: ['READ'], createdBy: SYSTEM_USER, userId: SYSTEM_USER, lastSyncRequestedDate: new Date().toISOString()