Skip to content
This repository has been archived by the owner on Apr 16, 2021. It is now read-only.

Commit

Permalink
get rid of loopback.getCurrentContext() as it is not reliable, instea…
Browse files Browse the repository at this point in the history
…d override the checkAccess method and look at the request to identify the context and whether the current (token) user can access the target data

strongloop/loopback#1676
  • Loading branch information
Frederic Lavigne committed Jun 3, 2016
1 parent 23019a1 commit c314749
Show file tree
Hide file tree
Showing 9 changed files with 241 additions and 112 deletions.
124 changes: 75 additions & 49 deletions common/mixins/isolated.js
Original file line number Diff line number Diff line change
@@ -1,69 +1,95 @@
// Licensed under the Apache License. See footer for details.
var winston = require("winston");

/**
* Overrides check access to inject the demoId of the current user in create and queries
* in order to limit the visibility of objects between demo environments
*/
module.exports = function (Model, options) {

// assign demo id to the new instances of model objects
Model.observe('before save', function (ctx, next) {
winston.debug(ctx.Model.modelName, "before save");
// keep the old version of checkAccess
Model.__checkAccess = Model.checkAccess;

var inst = ctx.instance || ctx.currentInstance;
if (inst.demoId) {
next();
} else {
var loopbackContext = Model.app.loopback.getCurrentContext();
if (!loopbackContext) { // no context to find a demoId, we can't proceed
next(new Error("No demoId specified"));
} else {
var currentUser = loopbackContext.get('currentUser');
if (!currentUser) { // no current user to find a demoId, we can't proceed
next(new Error("No demoId specified"));
} else {
inst.demoId = currentUser.demoId;
next();
}
}
}
});
// define our own
Model.checkAccess = function (token, modelId, sharedMethod, ctx, callback) {
var model = this;

// take the demoId from the currentUser and
// inject it in the where query for this object
Model.observe('access', function (ctx, next) {
winston.debug(ctx.Model.modelName, "before access");
if (!token) {
return Model.__checkAccess(token, modelId, sharedMethod, ctx, callback);
}

var demoIdSpecified = false;
// find the user behind the token
Model.app.models.ERPUser.findById(token.userId, function (err, user) {
if (err) {
return callback(err);
}

// first infer the demoId from the currentUser
var loopbackContext = Model.app.loopback.getCurrentContext();
if (loopbackContext) {
var currentUser = loopbackContext.get('currentUser');
if (currentUser) {
if (ctx.query.where) {
ctx.query.where.demoId = currentUser.demoId;
} else {
ctx.query.where = {
demoId: currentUser.demoId
};
// if there is an instance behind this call,
// it should already be within the user demo environment
if (ctx.instance) {
// if it does not have a demoId, this is unexpected as
// this mixin should only be installed on objects specific to a demo environment
if (!ctx.instance.demoId) {
winston.error(model.modelName, "Instance has no demoId!", ctx.instance);
return callback(new Error("Object is not attached to a demo"));
} else if (ctx.instance.demoId != user.demoId) {
// if it has a different demoId then this user is not allowed to access it
var notFound = new Error("Object not found in this demo");
notFound.status = 404;
return callback(notFound);
}
demoIdSpecified = true;
}
}

// if we were not able to find a current user, this might be coming
// from an internal call or from the "Demo" API that does not require a user,
// just make sure a demoId was specified anyway in the query
if (!demoIdSpecified && ctx.query.where && ctx.query.where.demoId) {
demoIdSpecified = true;
// there is no instance but a modelId is specified
// this is typically the case in calls like /Model/id/relations
if (!ctx.instance && modelId) {
// find the instance
Model.findById(modelId, function (err, object) {
if (object && object.demoId != user.demoId) {
// if it has a different demoId then this user is not allowed to access it
var notFound = new Error("Object not found in this demo");
notFound.status = 404;
return callback(notFound);
} else {
// otherwise proceed to inject the demoId where needed
injectDemoId(user, token, modelId, sharedMethod, ctx, callback);
}
});
} else {
// otherwise proceed to inject the demoId where needed
injectDemoId(user, token, modelId, sharedMethod, ctx, callback);
}

});
};

function injectDemoId(user, token, modelId, sharedMethod, ctx, callback) {
// if we are create a new object
// then inject the demoId of the current user
if ("create" == sharedMethod.name) {
ctx.req.remotingContext.args.data.demoId = user.demoId;
}
// if we are create an object through a relation /Model/:id/relation
// then inject the demoId of the current user
else if (sharedMethod.name.indexOf("__create__") == 0) {
ctx.req.remotingContext.args.data.__data.demoId = user.demoId;
}

if (!demoIdSpecified) {
winston.warn(ctx.Model.modelName, "no demoId found");
// inject the demoId in the filter "where" clause
if (!ctx.args.filter) {
ctx.args.filter = {};
}
if (!ctx.args.filter.where) {
ctx.args.filter.where = {};
}

next();
});
ctx.args.filter.where.demoId = user.demoId;

// let the regular check happens,
// we ensured that the demoId will be added to new item and to any query
Model.__checkAccess(token, modelId, sharedMethod, ctx, callback);
}
}

//------------------------------------------------------------------------------
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down
3 changes: 3 additions & 0 deletions common/models/demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ module.exports = function (Demo) {
if (model.modelName == "Shipment") {
object.toId = object.toId + "-" + object.demoId;
}
if (model.modelName == "LineItem") {
object.shipmentId = object.shipmentId + "-" + object.demoId;
}
});
}
winston.info("Injecting", objects.length, model.definition.name);
Expand Down
10 changes: 7 additions & 3 deletions server/boot/create-roles.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed under the Apache License. See footer for details.
var winston = require("winston");

module.exports = function (app) {
module.exports = function (app, next) {

function createRoles() {
app.models.Role.find(function (err, roles) {
Expand All @@ -16,9 +16,10 @@ module.exports = function (app) {
});
} else {
winston.error("find:", err);
next();
}
} else if (roles.length == 0) {
winston.info("ERP roles not found. Creating...");
winston.warn("ERP roles not found. Creating...");
app.models.Role.create([{
name: app.models.ERPUser.SUPPLY_CHAIN_MANAGER_ROLE
}, {
Expand All @@ -27,18 +28,21 @@ module.exports = function (app) {
if (err) {
winston.error("create:", err);
} else {
winston.info("Created", roles.length, "roles");
winston.warn("Created", roles.length, "roles");
}
next(err);
});
} else {
winston.info("Existing ERP roles", roles.map(function (role) {
return role.name;
}));
next();
}
});
}

createRoles();

};
//------------------------------------------------------------------------------
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down
28 changes: 4 additions & 24 deletions server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,35 +20,15 @@ app.start = function () {
});
};

// inject the current user as a context parameter of requests
app.use(loopback.context());
app.use(loopback.token());
app.use(function setCurrentUser(req, res, next) {
if (!req.accessToken) {
return next();
}

app.models.ERPUser.findById(req.accessToken.userId, function (err, user) {
if (err) {
return next(err);
}
if (!user) {
return next(new Error('No user with this access token was found.'));
}
var loopbackContext = loopback.getCurrentContext();
if (loopbackContext) {
req.accessToken.currentUser = user;
loopbackContext.set('currentUser', user);
}
next();
});
});

// Bootstrap the application, configure models, datasources and middleware.
// Sub-apps like REST API are mounted via boot scripts.
boot(app, __dirname, function (err) {
if (err) throw err;

// notify that the app has booted, ready to be started
app.booted = true;
app.emit('booted');

// start the server if `$ node server.js`
if (require.main === module)
app.start();
Expand Down
54 changes: 50 additions & 4 deletions test/data-isolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ describe('Data Isolation', function () {
apiSupply1 = supertest(app);
apiSupply2 = supertest(app);

done();
if (!app.booted) {
app.once("booted", function () {
done();
});
} else {
done();
}
});

var demoEnvironment1,
Expand Down Expand Up @@ -86,6 +92,20 @@ describe('Data Isolation', function () {
});
});

it('can attach a line item to the shipment', function (done) {
apiSupply1.post("/Shipments/" + newShipment.id + "/items")
.set("Authorization", apiSupply1.loopbackAccessToken.id)
.send({
"productId": "I4",
"quantity": 100
})
.expect(200)
.end(function (err, res) {
assert.equal(newShipment.id, res.body.shipmentId);
done(err);
});
});

// this shipment is not visible in D2
it('can see the shipment it created in D1', function (done) {
apiSupply1.get("/Shipments")
Expand All @@ -112,6 +132,18 @@ describe('Data Isolation', function () {
});
});

it('can see shipment items from D1 by id', function (done) {
apiSupply1.get("/Shipments/" + newShipment.id + "/items")
.set("Authorization", apiSupply1.loopbackAccessToken.id)
.set('Content-Type', 'application/json')
.expect(200)
.end(function (err, res) {
assert.isAbove(res.body.length, 0);
done(err);
});
});


// log in as Supply Chain Manager in D2
it('can log in D2', function (done) {
apiSupply2.post("/Demos/" + demoEnvironment2.guid + "/loginAs")
Expand All @@ -126,6 +158,7 @@ describe('Data Isolation', function () {
});
});


// this shipment is not visible in D2
it('can not see shipment from D1 in D2 Shipments', function (done) {
apiSupply2.get("/Shipments")
Expand All @@ -146,7 +179,7 @@ describe('Data Isolation', function () {
apiSupply2.get("/Shipments/" + newShipment.id)
.set("Authorization", apiSupply2.loopbackAccessToken.id)
.set('Content-Type', 'application/json')
.expect(404)
.expect(404) // this shipment should not be visible so Not Found
.end(function (err, res) {
done(err);
});
Expand All @@ -157,14 +190,14 @@ describe('Data Isolation', function () {
apiSupply2.get("/Shipments/" + newShipment.id + "/items")
.set("Authorization", apiSupply2.loopbackAccessToken.id)
.set('Content-Type', 'application/json')
.expect(404)
.expect(404) // this shipment should not be visible so Not Found
.end(function (err, res) {
done(err);
});
});

it('can not update shipments from D1 in D2', function (done) {
newShipment.updatedAt = new Date();
newShipment.deliveredAt = new Date();
apiSupply2.put("/Shipments/" + newShipment.id)
.set("Authorization", apiSupply2.loopbackAccessToken.id)
.set('Content-Type', 'application/json')
Expand All @@ -175,6 +208,19 @@ describe('Data Isolation', function () {
});
});

it('can not add items to shipments from D1 in D2', function (done) {
apiSupply2.post("/Shipments/" + newShipment.id + "/items")
.set("Authorization", apiSupply2.loopbackAccessToken.id)
.send({
"productId": "I5",
"quantity": 100
})
.expect(404) // this shipment should not be visible so Not Found
.end(function (err, res) {
done(err);
});
});

it('can not delete shipments from D1 in D2', function (done) {
apiSupply2.delete("/Shipments/" + newShipment.id)
.set("Authorization", apiSupply2.loopbackAccessToken.id)
Expand Down
14 changes: 7 additions & 7 deletions test/demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ describe('Demos', function () {
apiSupply = supertest(app);
apiRetail = supertest(app);

done();
});

after(function (done) {
app.models.ERPUser.destroyAll(function (err, info) {
done(err);
});
if (!app.booted) {
app.once("booted", function () {
done();
});
} else {
done();
}
});

it('can populate the app with sample data', function (done) {
Expand Down
Loading

0 comments on commit c314749

Please sign in to comment.