Skip to content

Commit

Permalink
Accept array of permCodes for user creating a bucket
Browse files Browse the repository at this point in the history
  • Loading branch information
TimCsaky committed Jun 20, 2024
1 parent cae86f5 commit d6d71fd
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 16 deletions.
8 changes: 6 additions & 2 deletions app/src/controllers/bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 }));
});
Expand Down
14 changes: 12 additions & 2 deletions app/src/docs/v1.api-spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 10 additions & 9 deletions app/src/services/bucket.js
Original file line number Diff line number Diff line change
@@ -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');

/**
Expand All @@ -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<object>} The result of running the insert operation
Expand All @@ -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
}));
Expand All @@ -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
Expand Down Expand Up @@ -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
}));
Expand Down
4 changes: 3 additions & 1 deletion app/src/validators/bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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(),
},

Expand Down
5 changes: 3 additions & 2 deletions app/tests/unit/controllers/bucket.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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'));
Expand Down
1 change: 1 addition & 0 deletions app/tests/unit/services/bucket.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const data = {
secretAccessKey: 'secretaccesskey',
region: 'region',
active: 'true',
permCodes: ['READ'],
createdBy: SYSTEM_USER,
userId: SYSTEM_USER,
lastSyncRequestedDate: new Date().toISOString()
Expand Down

0 comments on commit d6d71fd

Please sign in to comment.