Skip to content

Commit

Permalink
added permset in the cache layer - upsert/ bulk upsert endpoints
Browse files Browse the repository at this point in the history
  • Loading branch information
shriramshankar committed Mar 17, 2017
1 parent cce90eb commit cbbf0d8
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 33 deletions.
26 changes: 11 additions & 15 deletions api/v1/controllers/samples.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,21 +166,16 @@ module.exports = {
u.getUserNameFromToken(req,
featureToggles.isFeatureEnabled('enforceWritePermission'))
.then((userName) => {
let upsertSamplePromise;
if (featureToggles.isFeatureEnabled(constants.featureName)) {
if (sampleQueryBody.relatedLinks) {
u.checkDuplicateRLinks(sampleQueryBody.relatedLinks);
}

upsertSamplePromise = redisModelSample.upsertSample(
sampleQueryBody, resultObj, res.method
);
} else {
upsertSamplePromise = helper.model.upsertByName(
sampleQueryBody, userName
);
if (sampleQueryBody.relatedLinks) {
u.checkDuplicateRLinks(sampleQueryBody.relatedLinks);
}

const upsertSamplePromise =
featureToggles.isFeatureEnabled(constants.featureName) ?
redisModelSample.upsertSample(
sampleQueryBody, resultObj, res.method, userName) :
helper.model.upsertByName(sampleQueryBody, userName);

return upsertSamplePromise;
})
.then((samp) => {
Expand Down Expand Up @@ -230,9 +225,10 @@ module.exports = {

const j = jobWrapper.createJob(jobType.BULKUPSERTSAMPLES,
wrappedBulkUpsertData, req);
} else if (featureToggles.isFeatureEnabled(constants.featureName)) {
redisModelSample.bulkUpsertSample(value);
} else {
const bulkUpsertPromise =
featureToggles.isFeatureEnabled(constants.featureName) ?
redisModelSample.bulkUpsertSample(value, userName) :
helper.model.bulkUpsertByName(value, userName);
}
});
Expand Down
14 changes: 13 additions & 1 deletion api/v1/errorHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const util = require('util');
const apiErrors = require('./apiErrors');
const constants = require('./constants');
const dbErrors = require('../../db/dbErrors');
const cacheErrors = require('../../cache/redisErrors');

/**
* Indicates whether the error is an API error as defined by our apiErrors
Expand Down Expand Up @@ -53,6 +54,17 @@ function isDbError(err) {
err instanceof dbErrors.CreateConstraintError;
}

/**
* Indicates whether the error is a Cache error as defined by our cacheErrors
* module.
*
* @param {Error} err - The error to test
* @returns {Boolean} true if err is defined as one of our Cache errors
*/
function isCacheError(err) {
return err instanceof cacheErrors.UpdateDeleteForbidden;
}

/**
* Construct error response based on various errors
*
Expand Down Expand Up @@ -106,7 +118,7 @@ module.exports = function errorHandler(err, req, res, next) {

try {
const errResponse = constructError(err);
if (!isApiError(err) && !isDbError(err)) {
if (!isApiError(err) && !isDbError(err) && !isCacheError(err)) {
if (/Route defined in Swagger specification.*/.test(err.message)) {
err.status = constants.httpStatus.NOT_ALLOWED;
} else if (err.name === 'SequelizeUniqueConstraintError') {
Expand Down
58 changes: 44 additions & 14 deletions cache/models/samples.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
'use strict'; // eslint-disable-line strict

const helper = require('../../api/v1/helpers/nouns/samples');
const aspectHelper = require('../../api/v1/helpers/nouns/aspects');
const u = require('../../api/v1/helpers/verbs/utils');
const sampleStore = require('../sampleStore');
const redisClient = require('../redisCache').client.sampleStore;
Expand All @@ -26,6 +27,8 @@ const db = require('../../db/index');
const aspectType = redisOps.aspectType;
const sampleType = redisOps.sampleType;
const subjectType = redisOps.subjectType;
const featureToggles = require('feature-toggles');

const sampFields = {
MSG_BODY: 'messageBody',
MSG_CODE: 'messageCode',
Expand Down Expand Up @@ -373,9 +376,10 @@ function handleUpsertError(objectType, isBulk) {
* @param {Object} sampleQueryBodyObj - Query Body Object for a sample
* @param {String} method - Type of request method
* @param {Boolean} isBulk - If the caller method is bulk upsert
* @param {String} userName - The user performing the write operation
* @returns {Object} - Updated sample
*/
function upsertOneSample(sampleQueryBodyObj, method, isBulk) {
function upsertOneSample(sampleQueryBodyObj, method, isBulk, userName) {
const sampleName = sampleQueryBodyObj.name;
const subjAspArr = sampleName.toLowerCase().split('|');

Expand All @@ -391,26 +395,50 @@ function upsertOneSample(sampleQueryBodyObj, method, isBulk) {
return Promise.reject(err);
}

const aspectName = subjAspArr[ONE];
const hasWritePerm = featureToggles
.isFeatureEnabled('enforceWritePermission') ?
sampleStore.isSampleWritable(aspectHelper.model,
aspectName, userName) : Promise.resolve(true);

const subjKey = sampleStore.toKey(
constants.objectType.subject, subjAspArr[ZERO]
);
const sampleKey = sampleStore.toKey(
constants.objectType.sample, sampleName
);
const aspectName = subjAspArr[ONE];
let aspectObj;

// if any of the promise errors, the subsequent promise does not process and
// error is returned, else sample is returned
return Promise.all([
redisClient.sismemberAsync(constants.indexKey.subject, subjKey),
redisClient.hgetallAsync(
return hasWritePerm
.then((ok) => {
// reject the promise if the user does not have write permission
if (!ok) {
const err = new redisErrors.UpdateDeleteForbidden({
explanation: `The user: ${userName}, does not have write permission` +
` on the sample: ${sampleName}`,
});
if (isBulk) {
return Promise.reject({ isFailed: true, explanation: err });
}

return Promise.reject(err);
}

/*
* if any of the promise errors, the subsequent promise does not process and
* error is returned, else sample is returned
*/
return Promise.all([
redisClient.sismemberAsync(constants.indexKey.subject, subjKey),
redisClient.hgetallAsync(
sampleStore.toKey(constants.objectType.aspect, aspectName)
),
redisClient.hgetallAsync(sampleKey),
])
),
redisClient.hgetallAsync(sampleKey),
]);
})
.then((responses) => {
const [isSubjPresent, aspect, sample] = responses;

if (!isSubjPresent) {
handleUpsertError(constants.objectType.subject, isBulk);
}
Expand Down Expand Up @@ -871,20 +899,22 @@ module.exports = {
* @param {Object} qbObj - Query body object
* @param {Object} logObject - Log object
* @param {String} method - Type of request method
* @param {String} userName - The user performing the write operation
* @returns {Promise} - Resolves to upserted sample
*/
upsertSample(qbObj, logObject, method) {
return upsertOneSample(qbObj, method);
upsertSample(qbObj, logObject, method, userName) {
return upsertOneSample(qbObj, method, false, userName);
},

/**
* Upsert multiple samples in Redis.
* @param {Object} sampleQueryBody - Query body object
* @param {String} userName - The user performing the write operation
* @returns {Array} - Resolves to an array of resolved promises
*/
bulkUpsertSample(sampleQueryBody) {
bulkUpsertSample(sampleQueryBody, userName) {
const promises = sampleQueryBody.map(
(sampleReq) => upsertOneSample(sampleReq, null, true)
(sampleReq) => upsertOneSample(sampleReq, null, true, userName)
);

return Promise.all(promises);
Expand Down
11 changes: 11 additions & 0 deletions cache/redisErrors.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ redisErrors.create({
resourceKey: '',
});

// ---------------------------------------------------------------------------
// Permission Errors
// ----------------------------------------------------------------------------
redisErrors.create({
code: 11300,
status: 403,
name: 'UpdateDeleteForbidden',
parent: redisErrors.RefocusRedisError,
fields: [],
});

// ----------------------------------------------------------------------------
// Forbidden Error
// ----------------------------------------------------------------------------
Expand Down
16 changes: 16 additions & 0 deletions cache/sampleStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,27 @@ function cleanSample(s) {
return retval;
} // cleanSample

/**
* Checks if the aspect corresponding to a sample is writable by the given user.
*
* @param {Model} aspectModel - The model object
* @param {String} aspectName - The name of the aspect
* @param {String} userName - The user name
* @returns {Boolean} - returns true if the sample is writable by the given user
*/
function isSampleWritable(aspectModel, aspectName, userName) {
const options = {};
options.where = { name: aspectName };
return aspectModel.findOne(options)
.then((aspect) => Promise.resolve(aspect.isWritableBy(userName)));
} // isSampleWritable

module.exports = {
cleanAspect,
cleanSample,
constants,
toKey,
arrayStringsToJson,
getNameFromKey,
isSampleWritable,
};
7 changes: 4 additions & 3 deletions tests/api/v1/samples/upsert.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ describe(`api: POST ${path}`, () => {
done(err);
}

const { message, source } = res.body.errors[0];
expect(message).to.equal('Name of the relatedlinks should be unique');
expect(source).to.equal('relatedLinks');
const { source, description } = res.body.errors[0];
expect(description).to
.contain('Name of the relatedlinks should be unique');
expect(source).to.equal('Sample');
done();
});
});
Expand Down
1 change: 1 addition & 0 deletions tests/cache/models/samples/upsert.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe(`api::redisEnabled::POST::upsert ${path}`, () => {

before((done) => {
tu.toggleOverride('enableRedisSampleStore', true);
tu.toggleOverride('enforceWritePermission', false);
tu.createToken()
.then((returnedToken) => {
token = returnedToken;
Expand Down
1 change: 1 addition & 0 deletions tests/cache/models/samples/upsertBulk.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('api::redisEnabled::POST::bulkUpsert ' + path, () => {

before((done) => {
tu.toggleOverride('enableRedisSampleStore', true);
tu.toggleOverride('enforceWritePermission', false);
tu.createToken()
.then((returnedToken) => {
token = returnedToken;
Expand Down
107 changes: 107 additions & 0 deletions tests/cache/models/samples/upsertWithoutPerms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* Copyright (c) 2016, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or
* https://opensource.org/licenses/BSD-3-Clause
*/

/**
* tests/cache/models/samples/upsertWithoutPerms.js
*/
'use strict';

const supertest = require('supertest');
const api = supertest(require('../../../../index').app);
const constants = require('../../../../api/v1/constants');
const samstoinit = require('../../../../cache/sampleStoreInit');
const tu = require('../../../testUtils');
const rtu = require('../redisTestUtil');
const Subject = tu.db.Subject;
const Aspect = tu.db.Aspect;
const User = tu.db.User;

const upsertPath = '/v1/samples/upsert';
const bulkUpsertPath = '/v1/samples/upsert/bulk';
describe('api: upsert samples without perms', () => {
const sub = { name: `${tu.namePrefix}NorthAmerica`, isPublished: true };
const asp = {
name: 'temperature',
timeout: '30s',
isPublished: true,
rank: 10,
};
let aspect;
let subject;
let otherValidToken;

before((done) => {
tu.toggleOverride('enforceWritePermission', true);
tu.toggleOverride('enableRedisSampleStore', true);
tu.createToken()
.then(() => {
done();
})
.catch(done);
});

beforeEach((done) => {
Aspect.create(asp)
.then((a) => {
aspect = a;
return Subject.create(sub);
})
.then((s) => {
subject = s;
})
.then(() => {
return User.findOne({ where: { name: tu.userName } });
})
.then((usr) => {
return aspect.addWriter(usr);
})
.then(() => tu.createUser('myUNiqueUser'))
.then((_usr) => tu.createTokenFromUserName(_usr.name))
.then((tkn) => {
otherValidToken = tkn;
return samstoinit.populate();
})
.then(() => done())
.catch(done);
});

afterEach(rtu.forceDelete);
after(() => tu.toggleOverride('enableRedisSampleStore', false));
after(() => tu.toggleOverride('enforceWritePermission', false));

after(tu.forceDeleteUser);


it('upsert should fail when upserting a sample ' +
'for an aspect without permission', (done) => {
api.post(upsertPath)
.set('Authorization', otherValidToken)
.send({
name: `${subject.absolutePath}|${aspect.name}`,
value: '2',
})
.expect(constants.httpStatus.FORBIDDEN)
.end((err /* , res*/) => {
return err ? done(err) : done();
});
});

it('bulk upsert should return OK for operations with any' +
' permission', (done) => {
api.post(bulkUpsertPath)
.set('Authorization', otherValidToken)
.send([{
name: `${subject.absolutePath}|${aspect.name}`,
value: '2',
}])
.expect(constants.httpStatus.OK)
.end((err /* , res*/) => {
return err ? done(err) : done();
});
});
});

0 comments on commit cbbf0d8

Please sign in to comment.