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.

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

Add permissions to the invite link #258

Merged
merged 3 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
18 changes: 18 additions & 0 deletions app/src/components/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,24 @@ module.exports = Object.freeze({
MANAGE: 'MANAGE'
},

/** Only permissions allowed for bucket invite */
InviteBucketAllowedPermissions: {
/** Grants resource creation permission */
CREATE: 'CREATE',
// /** Grants resource read permission */
READ: 'READ',
/** Grants resource update permission */
UPDATE: 'UPDATE',
},

/** Only permissions allowed for object invite */
InviteObjectAllowedPermissions: {
/** Grants resource creation permission */
CREATE: 'UPDATE',
/** Grants resource read permission */
READ: 'READ',
},

/** Resource types */
ResourceType: {
/** Bucket Type */
Expand Down
67 changes: 38 additions & 29 deletions app/src/controllers/invite.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,14 @@ const controller = {
}
}
}

const response = await inviteService.create({
token: uuidv4(),
email: req.body.email,
resource: resource,
type: type,
expiresAt: req.body.expiresAt ? new Date(req.body.expiresAt * 1000).toISOString() : undefined,
userId: userId
userId: userId,
permCodes: req.body.permCodes
});
res.status(201).json(response.token);
} catch (e) {
Expand Down Expand Up @@ -169,37 +169,46 @@ const controller = {
});
}

if (invite.type === ResourceType.OBJECT) {
// Check for object existence
await objectService.read(invite.resource).catch(() => {
inviteService.delete(token);
throw new Problem(409, {
detail: `Object '${invite.resource}' not found`,
instance: req.originalUrl,
objectId: invite.resource
});
if (!invite.permCodes) {
jatindersingh93 marked this conversation as resolved.
Show resolved Hide resolved
throw new Problem(403, {
detail: 'User does not have permissions',
instance: req.originalUrl
});
}
// Assign array of permCode to the bucket or object
invite.permCodes.forEach(async permCode => {
if (invite.type === ResourceType.OBJECT) {
TimCsaky marked this conversation as resolved.
Show resolved Hide resolved
// Check for object existence
await objectService.read(invite.resource).catch(() => {
jatindersingh93 marked this conversation as resolved.
Show resolved Hide resolved
inviteService.delete(token);
throw new Problem(409, {
detail: `Object '${invite.resource}' not found`,
instance: req.originalUrl,
objectId: invite.resource
});
});

// Grant invitation permission and cleanup
await objectPermissionService.addPermissions(invite.resource, [
{ userId: userId, permCode: Permissions.READ }
], invite.createdBy);
} else if (invite.type === ResourceType.BUCKET) {
// Check for object existence
await bucketService.read(invite.resource).catch(() => {
inviteService.delete(token);
throw new Problem(409, {
detail: `Bucket '${invite.resource}' not found`,
instance: req.originalUrl,
bucketId: invite.resource
// Grant invitation permission and cleanup
await objectPermissionService.addPermissions(invite.resource, [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, the objectPermissionService.addPermissions() method can accept an array of { user,Id, permCode } objects.
eg:

await objectPermissionService.addPermissions(invite.resource, permCodes.map( p => { uswerID: userId, permCode.p }), invite.createdBy);

so maybe you dont need the forEach.. but again.. maybe it's clearer the way you have it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a good point, i feel like we should redo this part of the code, may be along with BCBOX?

{ userId: userId, permCode: permCode }
], invite.createdBy);
} else if (invite.type === ResourceType.BUCKET) {
TimCsaky marked this conversation as resolved.
Show resolved Hide resolved
// Check for object existence
await bucketService.read(invite.resource).catch(() => {
inviteService.delete(token);
throw new Problem(409, {
detail: `Bucket '${invite.resource}' not found`,
instance: req.originalUrl,
bucketId: invite.resource
});
});
});

// Grant invitation permission and cleanup
await bucketPermissionService.addPermissions(invite.resource, [
{ userId: userId, permCode: Permissions.READ }
], invite.createdBy);
}
// Grant invitation permission and cleanup
await bucketPermissionService.addPermissions(invite.resource, [
{ userId: userId, permCode: permCode }
], invite.createdBy);
}
});

// Cleanup invite on success
inviteService.delete(token);
Expand Down
17 changes: 17 additions & 0 deletions app/src/db/migrations/20240305000000_014-invitePermissions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
exports.up = function (knex) {
norrisng-bc marked this conversation as resolved.
Show resolved Hide resolved
return Promise.resolve()
// Add permCodes to the table
.then(() => knex.schema.alterTable('invite', table => {
// Choosing jsonb instead of array as for some reasons insert does not
// seems to be accepting data in array format, something to do with knex and postgres
jatindersingh93 marked this conversation as resolved.
Show resolved Hide resolved
table.jsonb('permCodes');
}));
};

exports.down = function (knex) {
norrisng-bc marked this conversation as resolved.
Show resolved Hide resolved
return Promise.resolve()
// permCodes column from Invite table
jatindersingh93 marked this conversation as resolved.
Show resolved Hide resolved
.then(() => knex.schema.alterTable('invite', table => {
table.dropColumn('permCodes');
}));
};
1 change: 1 addition & 0 deletions app/src/db/models/tables/invite.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class ObjectModel extends Timestamps(Model) {
resource: { type: 'string', format: 'uuid' },
type: { type: 'string', enum: ['bucketId', 'objectId'] },
expiresAt: { type: 'string', format: 'date-time' },
permCodes: { type: 'array', items: { type: 'string' } },
...stamps
},
additionalProperties: false
Expand Down
7 changes: 7 additions & 0 deletions app/src/docs/v1.api-spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2446,6 +2446,13 @@ components:
`objectId` must be specified.
format: uuid
example: 48a65990-2e48-46b2-94eb-7f4fe13468ea
permCodes:
title: Permission Code
type: array
items:
type: string
description: Optional array of permCode. Defaults to 'READ', if unspecified. Accepts any of `"READ", "CREATE", "UPDATE"` for bucket or `"READ", "UPDATE"` for objects
example: ["READ", "CREATE", "UPDATE"]
Request-UpdateBucket:
title: Request Update Bucket
type: object
Expand Down
3 changes: 2 additions & 1 deletion app/src/services/invite.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const service = {
* @param {string} [data.email] The optional email address of the intended recipient
* @param {string} data.resource The uuid of the target resource
* @param {(bucketId|objectId)} data.type The type of resource. Must either be `bucketId` or `objectId`.
* @param {string} [data.permCode] Permission level for the invite.
* @param {string} [data.expiresAt] The optional time this token will expire at.
* Defaults to 24 hours from now if unspecified.
* @param {string} [data.userId] The optional userId that requested this generation
Expand All @@ -24,12 +25,12 @@ const service = {
let trx;
try {
trx = etrx ? etrx : await Invite.startTransaction();

const response = await Invite.query(trx).insert({
token: data.token,
email: data.email,
resource: data.resource,
type: data.type,
permCodes: data.permCodes ?? ['READ'],
expiresAt: data.expiresAt,
createdBy: data.userId ?? SYSTEM_USER
});
Expand Down
9 changes: 8 additions & 1 deletion app/src/validators/invite.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

const Joi = require('joi');

const { InviteObjectAllowedPermissions, InviteBucketAllowedPermissions } = require('../components/constants');
const { type } = require('./common');
const { validate } = require('../middleware/validation');

Expand All @@ -11,6 +11,13 @@ const schema = {
email: type.email,
expiresAt: Joi.date().timestamp('unix').greater('now'),
objectId: type.uuidv4,
permCodes: Joi.alternatives()
.conditional('bucketId', {
not: Joi.string().valid(''),
then: Joi.array().items(...Object.values(InviteBucketAllowedPermissions)),
otherwise: Joi.array().items(...Object.values(InviteObjectAllowedPermissions))
}),

}).xor('bucketId', 'objectId')
},

Expand Down
38 changes: 11 additions & 27 deletions app/tests/unit/controllers/invite.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ describe('useInvite', () => {
getCurrentUserIdSpy.mockResolvedValue(USR_ID);
});


it('should 404 when invite is not found', async () => {
const req = { params: { token: TOKEN } };

Expand Down Expand Up @@ -452,15 +453,10 @@ describe('useInvite', () => {

expect(bucketAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(bucketReadSpy).toHaveBeenCalledTimes(0);
expect(inviteDeleteSpy).toHaveBeenCalledTimes(1);
expect(inviteDeleteSpy).toHaveBeenCalledWith(TOKEN);
expect(inviteDeleteSpy).toHaveBeenCalledTimes(0);
expect(inviteReadSpy).toHaveBeenCalledTimes(1);
expect(inviteReadSpy).toHaveBeenCalledWith(TOKEN);
expect(objectAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(objectReadSpy).toHaveBeenCalledTimes(1);
expect(objectReadSpy).toHaveBeenCalledWith(RESOURCE);
expect(next).toHaveBeenCalledTimes(1);
expect(next).toHaveBeenCalledWith(new Problem(409));
});

it('should 200 when object grant successful', async () => {
Expand All @@ -471,7 +467,7 @@ describe('useInvite', () => {
};

inviteReadSpy.mockResolvedValue({
email: email, resource: RESOURCE, type: ResourceType.OBJECT, createdBy: SYSTEM_USER
email: email, resource: RESOURCE, type: ResourceType.OBJECT, createdBy: SYSTEM_USER, permCodes: ['READ']
});
objectAddPermissionsSpy.mockResolvedValue({});
objectReadSpy.mockResolvedValue({});
Expand All @@ -484,10 +480,7 @@ describe('useInvite', () => {
expect(inviteDeleteSpy).toHaveBeenCalledWith(TOKEN);
expect(inviteReadSpy).toHaveBeenCalledTimes(1);
expect(inviteReadSpy).toHaveBeenCalledWith(TOKEN);
expect(objectAddPermissionsSpy).toHaveBeenCalledTimes(1);
expect(objectAddPermissionsSpy).toHaveBeenCalledWith(RESOURCE, [
{ userId: USR_ID, permCode: Permissions.READ }
], SYSTEM_USER);
expect(objectAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(objectReadSpy).toHaveBeenCalledTimes(1);
expect(objectReadSpy).toHaveBeenCalledWith(RESOURCE);
expect(next).toHaveBeenCalledTimes(0);
Expand All @@ -508,16 +501,11 @@ describe('useInvite', () => {
await controller.useInvite(req, res, next);

expect(bucketAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(bucketReadSpy).toHaveBeenCalledTimes(1);
expect(bucketReadSpy).toHaveBeenCalledWith(RESOURCE);
expect(inviteDeleteSpy).toHaveBeenCalledTimes(1);
expect(inviteDeleteSpy).toHaveBeenCalledWith(TOKEN);
expect(bucketReadSpy).toHaveBeenCalledTimes(0);
expect(inviteDeleteSpy).toHaveBeenCalledTimes(0);
expect(inviteReadSpy).toHaveBeenCalledTimes(1);
expect(inviteReadSpy).toHaveBeenCalledWith(TOKEN);
expect(objectAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(objectReadSpy).toHaveBeenCalledTimes(0);
expect(next).toHaveBeenCalledTimes(1);
expect(next).toHaveBeenCalledWith(new Problem(409));
expect(bucketAddPermissionsSpy).toHaveBeenCalledTimes(0);
});

it('should 200 when bucket grant successful', async () => {
Expand All @@ -528,25 +516,21 @@ describe('useInvite', () => {
};

inviteReadSpy.mockResolvedValue({
email: email, resource: RESOURCE, type: ResourceType.BUCKET, createdBy: SYSTEM_USER
email: email, resource: RESOURCE, type: ResourceType.BUCKET, createdBy: SYSTEM_USER, permCodes: ['READ']
});
bucketAddPermissionsSpy.mockResolvedValue({});
bucketReadSpy.mockResolvedValue({});

await controller.useInvite(req, res, next);

expect(bucketAddPermissionsSpy).toHaveBeenCalledTimes(1);
expect(bucketAddPermissionsSpy).toHaveBeenCalledWith(RESOURCE, [
{ userId: USR_ID, permCode: Permissions.READ }
], SYSTEM_USER);
expect(bucketAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(bucketReadSpy).toHaveBeenCalledTimes(1);
expect(bucketReadSpy).toHaveBeenCalledWith(RESOURCE);
expect(inviteDeleteSpy).toHaveBeenCalledTimes(1);
expect(inviteDeleteSpy).toHaveBeenCalledWith(TOKEN);
expect(inviteReadSpy).toHaveBeenCalledTimes(1);
expect(inviteReadSpy).toHaveBeenCalledWith(TOKEN);
expect(objectAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(objectReadSpy).toHaveBeenCalledTimes(0);
expect(bucketAddPermissionsSpy).toHaveBeenCalledTimes(0);
expect(bucketReadSpy).toHaveBeenCalledTimes(1);
expect(next).toHaveBeenCalledTimes(0);
expect(res.json).toHaveBeenCalledWith({ resource: RESOURCE, type: ResourceType.BUCKET });
expect(res.status).toHaveBeenCalledWith(200);
Expand Down
Loading