fix unauthorized fk change (#1538)

Downstream failures are unrelated.
This commit is contained in:
Taranveer Virk 2018-01-17 13:34:37 -05:00 committed by Kevin Delisle
parent 9189dba001
commit cc60ef8202
2 changed files with 83 additions and 3 deletions

View File

@ -107,6 +107,19 @@ function bindRelationMethods(relation, relationMethod, definition) {
});
};
function preventFkOverride(inst, data, fkProp) {
if (!fkProp) return undefined;
if (data[fkProp] !== undefined && !idEquals(data[fkProp], inst[fkProp])) {
var err = new Error(g.f(
'Cannot override foreign key %s from %s to %s',
fkProp,
inst[fkProp],
data[fkProp])
);
}
return err;
}
/**
* Relation definition class. Use to define relationships between models.
* @param {Object} definition
@ -920,10 +933,15 @@ HasMany.prototype.updateById = function(fkId, data, options, cb) {
options = {};
}
cb = cb || utils.createPromiseCallback();
var fk = this.definition.keyTo;
this.findById(fkId, options, function(err, inst) {
if (err) {
return cb && cb(err);
}
// Ensure Foreign Key cannot be changed!
var fkErr = preventFkOverride(inst, data, fk);
if (fkErr) return cb(fkErr);
inst.updateAttributes(data, options, cb);
});
return cb.promise;
@ -1420,8 +1438,13 @@ BelongsTo.prototype.update = function(targetModelData, options, cb) {
}
cb = cb || utils.createPromiseCallback();
var definition = this.definition;
var fk = definition.keyTo;
this.fetch(options, function(err, inst) {
if (inst instanceof ModelBaseClass) {
// Ensures Foreign Key cannot be changed!
var fkErr = preventFkOverride(inst, targetModelData, fk);
if (fkErr) return cb(fkErr);
inst.updateAttributes(targetModelData, options, cb);
} else {
cb(new Error(g.f('{{BelongsTo}} relation %s is empty', definition.name)));
@ -1819,7 +1842,9 @@ HasOne.prototype.update = function(targetModelData, options, cb) {
var fk = this.definition.keyTo;
this.fetch(function(err, targetModel) {
if (targetModel instanceof ModelBaseClass) {
delete targetModelData[fk];
// Ensures Foreign Key cannot be changed!
var fkErr = preventFkOverride(targetModel, targetModelData, fk);
if (fkErr) return cb(fkErr);
targetModel.updateAttributes(targetModelData, options, cb);
} else {
cb(new Error(g.f('{{HasOne}} relation %s is empty', definition.name)));

View File

@ -95,6 +95,22 @@ describe('relations', function() {
});
});
it('should not update FK', function(done) {
Book.create(function(err, book) {
book.chapters.create({name: 'chapter 1'}, function(err, c) {
if (err) return done(err);
should.exist(c);
c.bookId.should.eql(book.id);
c.name.should.eql('chapter 1');
book.chapters.updateById(c.id, {name: 'chapter 0', bookId: 10}, function(err, cc) {
should.exist(err);
err.message.should.startWith('Cannot override foreign key');
done();
});
});
});
});
it('should create record on scope with promises', function(done) {
Book.create()
.then(function(book) {
@ -3272,6 +3288,17 @@ describe('relations', function() {
});
});
it('should not update related item FK on scope', function(done) {
Item.findById(itemId, function(e, todo) {
if (e) return done(e);
todo.list.update({id: 10}, function(err, list) {
should.exist(err);
err.message.should.startWith('Cannot override foreign key');
done();
});
});
});
it('should get related item on scope', function(done) {
Item.findById(itemId, function(e, todo) {
todo.list(function(err, list) {
@ -3587,6 +3614,18 @@ describe('relations', function() {
});
});
it('should not update the related item FK on scope', function(done) {
Supplier.findById(supplierId, function(err, supplier) {
if (err) return done(err);
should.exist(supplier);
supplier.account.update({supplierName: 'Supplier A', supplierId: 10}, function(err, acct) {
should.exist(err);
err.message.should.containEql('Cannot override foreign key');
done();
});
});
});
it('should update the related item on scope with promises', function(done) {
Supplier.findById(supplierId)
.then(function(supplier) {
@ -3600,7 +3639,7 @@ describe('relations', function() {
.catch(done);
});
it('should ignore the foreign key in the update', function(done) {
it('should error trying to change the foreign key in the update', function(done) {
Supplier.create({name: 'Supplier 2'}, function(e, supplier) {
var sid = supplier.id;
Supplier.findById(supplierId, function(e, supplier) {
@ -3609,7 +3648,23 @@ describe('relations', function() {
supplier.account.update({supplierName: 'Supplier A',
supplierId: sid},
function(err, act) {
should.not.exist(e);
should.exist(err);
err.message.should.startWith('Cannot override foreign key');
done();
});
});
});
});
it('should update the related item on scope with same foreign key', function(done) {
Supplier.create({name: 'Supplier 2'}, function(err, supplier) {
Supplier.findById(supplierId, function(err, supplier) {
if (err) return done(err);
should.exist(supplier);
supplier.account.update({supplierName: 'Supplier A',
supplierId: supplierId},
function(err, act) {
if (err) return done(err);
act.supplierName.should.equal('Supplier A');
act.supplierId.toString().should.eql(supplierId.toString());
done();