Conflict resolution and Access control
Add end-to-end unit-tests verifying enforcement of access control during conflict resolution. Implement two facade methods providing REST API for Change methods used by conflict resolution: PersistedModel.findLastChange GET /api/{model.pluralName}/{id}/changes/last PersistedModel.updateLastChange PUT /api/{model.pluralName}/{id}/changes/last By providing these two methods on PersistedModel, replication users don't have to expose the Change model via the REST API. What's even more important, these two methods use the same set of ACL rules as other (regular) PersistedModel methods. Rework `Conflict.prototype.changes()` and `Conflict.prototype.resolve()` to use these new facade methods. Implement a new method `Conflict.prototype.swapParties()` that provides better API for the situation when a conflict detected in Remote->Local replication should be resolved locally (i.e. in the replication target).
This commit is contained in:
parent
28acffd7dd
commit
cf2acb3cd2
|
@ -617,9 +617,8 @@ module.exports = function(Change) {
|
|||
], done);
|
||||
|
||||
function getSourceChange(cb) {
|
||||
conflict.SourceChange.findOne({where: {
|
||||
modelId: conflict.modelId
|
||||
}}, function(err, change) {
|
||||
var SourceModel = conflict.SourceModel;
|
||||
SourceModel.findLastChange(conflict.modelId, function(err, change) {
|
||||
if (err) return cb(err);
|
||||
sourceChange = change;
|
||||
cb();
|
||||
|
@ -627,9 +626,8 @@ module.exports = function(Change) {
|
|||
}
|
||||
|
||||
function getTargetChange(cb) {
|
||||
conflict.TargetChange.findOne({where: {
|
||||
modelId: conflict.modelId
|
||||
}}, function(err, change) {
|
||||
var TargetModel = conflict.TargetModel;
|
||||
TargetModel.findLastChange(conflict.modelId, function(err, change) {
|
||||
if (err) return cb(err);
|
||||
targetChange = change;
|
||||
cb();
|
||||
|
@ -658,11 +656,15 @@ module.exports = function(Change) {
|
|||
|
||||
Conflict.prototype.resolve = function(cb) {
|
||||
var conflict = this;
|
||||
conflict.changes(function(err, sourceChange, targetChange) {
|
||||
if (err) return cb(err);
|
||||
sourceChange.prev = targetChange.rev;
|
||||
sourceChange.save(cb);
|
||||
});
|
||||
conflict.TargetModel.findLastChange(
|
||||
this.modelId,
|
||||
function(err, targetChange) {
|
||||
if (err) return cb(err);
|
||||
conflict.SourceModel.updateLastChange(
|
||||
conflict.modelId,
|
||||
{ prev: targetChange.rev },
|
||||
cb);
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
|
@ -692,7 +694,9 @@ module.exports = function(Change) {
|
|||
if (target === null) {
|
||||
return conflict.SourceModel.deleteById(conflict.modelId, done);
|
||||
}
|
||||
var inst = new conflict.SourceModel(target);
|
||||
var inst = new conflict.SourceModel(
|
||||
target.toObject(),
|
||||
{ persisted: true });
|
||||
inst.save(done);
|
||||
});
|
||||
|
||||
|
@ -702,6 +706,23 @@ module.exports = function(Change) {
|
|||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Return a new Conflict instance with swapped Source and Target models.
|
||||
*
|
||||
* This is useful when resolving a conflict in one-way
|
||||
* replication, where the source data must not be changed:
|
||||
*
|
||||
* ```js
|
||||
* conflict.swapParties().resolveUsingTarget(cb);
|
||||
* ```
|
||||
*
|
||||
* @returns {Conflict} A new Conflict instance.
|
||||
*/
|
||||
Conflict.prototype.swapParties = function() {
|
||||
var Ctor = this.constructor;
|
||||
return new Ctor(this.modelId, this.TargetModel, this.SourceModel);
|
||||
};
|
||||
|
||||
/**
|
||||
* Resolve the conflict using the supplied instance data.
|
||||
*
|
||||
|
|
|
@ -705,6 +705,37 @@ module.exports = function(registry) {
|
|||
accepts: {arg: 'updates', type: 'array'},
|
||||
http: {verb: 'post', path: '/bulk-update'}
|
||||
});
|
||||
|
||||
setRemoting(PersistedModel, 'findLastChange', {
|
||||
description: 'Get the most recent change record for this instance.',
|
||||
accessType: 'READ',
|
||||
accepts: {
|
||||
arg: 'id', type: 'any', required: true, http: { source: 'path' },
|
||||
description: 'Model id'
|
||||
},
|
||||
returns: { arg: 'result', type: this.Change.modelName, root: true },
|
||||
http: { verb: 'get', path: '/:id/changes/last' }
|
||||
});
|
||||
|
||||
setRemoting(PersistedModel, 'updateLastChange', {
|
||||
description: [
|
||||
'Update the properties of the most recent change record',
|
||||
'kept for this instance.'
|
||||
],
|
||||
accessType: 'WRITE',
|
||||
accepts: [
|
||||
{
|
||||
arg: 'id', type: 'any', required: true, http: { source: 'path' },
|
||||
description: 'Model id'
|
||||
},
|
||||
{
|
||||
arg: 'data', type: 'object', http: {source: 'body'},
|
||||
description: 'An object of Change property name/value pairs'
|
||||
},
|
||||
],
|
||||
returns: { arg: 'result', type: this.Change.modelName, root: true },
|
||||
http: { verb: 'put', path: '/:id/changes/last' }
|
||||
});
|
||||
}
|
||||
|
||||
if (options.trackChanges) {
|
||||
|
@ -1469,6 +1500,26 @@ module.exports = function(registry) {
|
|||
Change.rectifyModelChanges(this.modelName, [id], callback);
|
||||
};
|
||||
|
||||
PersistedModel.findLastChange = function(id, cb) {
|
||||
var Change = this.getChangeModel();
|
||||
Change.findOne({ where: { modelId: id } }, cb);
|
||||
};
|
||||
|
||||
PersistedModel.updateLastChange = function(id, data, cb) {
|
||||
var self = this;
|
||||
this.findLastChange(id, function(err, inst) {
|
||||
if (err) return cb(err);
|
||||
if (!inst) {
|
||||
err = new Error('No change record found for ' +
|
||||
self.modelName + ' with id ' + id);
|
||||
err.statusCode = 404;
|
||||
return cb(err);
|
||||
}
|
||||
|
||||
inst.updateAttributes(data, cb);
|
||||
});
|
||||
};
|
||||
|
||||
PersistedModel.setup();
|
||||
|
||||
return PersistedModel;
|
||||
|
|
|
@ -13,6 +13,7 @@ describe('Replication over REST', function() {
|
|||
var serverApp, serverUrl, ServerUser, ServerCar, serverCars;
|
||||
var aliceId, peterId, aliceToken, peterToken, emeryToken, request;
|
||||
var clientApp, LocalUser, LocalCar, RemoteUser, RemoteCar, clientCars;
|
||||
var conflictedCarId;
|
||||
|
||||
before(setupServer);
|
||||
before(setupClient);
|
||||
|
@ -154,9 +155,137 @@ describe('Replication over REST', function() {
|
|||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// TODO conflict resolution
|
||||
// TODO verify permissions of individual methods
|
||||
describe('conflict resolution with model-level permissions', function() {
|
||||
var LocalConflict, RemoteConflict;
|
||||
|
||||
before(function setupConflictModels() {
|
||||
LocalConflict = LocalCar.getChangeModel().Conflict;
|
||||
RemoteConflict = RemoteCar.getChangeModel().Conflict;
|
||||
});
|
||||
|
||||
beforeEach(seedConflict);
|
||||
|
||||
describe('as anonymous user', function() {
|
||||
it('rejects resolve() on the client', function(done) {
|
||||
// simulate replication Client->Server
|
||||
var conflict = new LocalConflict(
|
||||
conflictedCarId,
|
||||
LocalCar,
|
||||
RemoteCar);
|
||||
conflict.resolveUsingSource(expectHttpError(401, done));
|
||||
});
|
||||
|
||||
it('rejects resolve() on the server', function(done) {
|
||||
// simulate replication Server->Client
|
||||
var conflict = new RemoteConflict(
|
||||
conflictedCarId,
|
||||
RemoteCar,
|
||||
LocalCar);
|
||||
conflict.resolveUsingSource(expectHttpError(401, done));
|
||||
});
|
||||
});
|
||||
|
||||
describe('as user with READ-only permissions', function() {
|
||||
beforeEach(function() {
|
||||
setAccessToken(emeryToken);
|
||||
});
|
||||
|
||||
it('allows resolve() on the client', function(done) {
|
||||
// simulate replication Client->Server
|
||||
var conflict = new LocalConflict(
|
||||
conflictedCarId,
|
||||
LocalCar,
|
||||
RemoteCar);
|
||||
conflict.resolveUsingSource(done);
|
||||
});
|
||||
|
||||
it('rejects resolve() on the server', function(done) {
|
||||
// simulate replication Server->Client
|
||||
var conflict = new RemoteConflict(
|
||||
conflictedCarId,
|
||||
RemoteCar,
|
||||
LocalCar);
|
||||
conflict.resolveUsingSource(expectHttpError(401, done));
|
||||
});
|
||||
});
|
||||
|
||||
describe('as user with REPLICATE-only permissions', function() {
|
||||
beforeEach(function() {
|
||||
setAccessToken(aliceToken);
|
||||
});
|
||||
|
||||
it('allows reverse resolve() on the client', function(done) {
|
||||
RemoteCar.replicate(LocalCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
expect(conflicts, 'conflicts').to.have.length(1);
|
||||
|
||||
// By default, conflicts are always resolved by modifying
|
||||
// the source datasource, so that the next replication run
|
||||
// replicates the resolved data.
|
||||
// However, when replicating changes from a datasource that
|
||||
// users are not authorized to change, the users have to resolve
|
||||
// conflicts at the target, discarding any changes made in
|
||||
// the target datasource only.
|
||||
conflicts[0].swapParties().resolveUsingTarget(function(err) {
|
||||
if (err) return done(err);
|
||||
|
||||
RemoteCar.replicate(LocalCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
if (conflicts.length) return done(conflictError(conflicts));
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('rejects resolve() on the server', function(done) {
|
||||
RemoteCar.replicate(LocalCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
expect(conflicts, 'conflicts').to.have.length(1);
|
||||
conflicts[0].resolveUsingSource(expectHttpError(401, done));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('as user with READ and WRITE permissions', function() {
|
||||
beforeEach(function() {
|
||||
setAccessToken(peterToken);
|
||||
});
|
||||
|
||||
it('allows resolve() on the client', function(done) {
|
||||
LocalCar.replicate(RemoteCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
expect(conflicts).to.have.length(1);
|
||||
|
||||
conflicts[0].resolveUsingSource(function(err) {
|
||||
if (err) return done(err);
|
||||
LocalCar.replicate(RemoteCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
if (conflicts.length) return done(conflictError(conflicts));
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('allows resolve() on the server', function(done) {
|
||||
RemoteCar.replicate(LocalCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
expect(conflicts).to.have.length(1);
|
||||
|
||||
conflicts[0].resolveUsingSource(function(err) {
|
||||
if (err) return done(err);
|
||||
RemoteCar.replicate(LocalCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
if (conflicts.length) return done(conflictError(conflicts));
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe.skip('sync with instance-level permissions', function() {
|
||||
|
@ -222,7 +351,7 @@ describe('Replication over REST', function() {
|
|||
], done);
|
||||
});
|
||||
|
||||
// TODO verify conflict resolution
|
||||
// TODO(bajtos) verify conflict resolution
|
||||
|
||||
function setupModifiedLocalCopyOfAlice(done) {
|
||||
// Replicate directly, bypassing REST+AUTH layers
|
||||
|
@ -245,7 +374,7 @@ describe('Replication over REST', function() {
|
|||
base: 'User',
|
||||
plural: 'Users', // use the same REST path in all models
|
||||
trackChanges: true,
|
||||
strict: true,
|
||||
strict: 'throw',
|
||||
persistUndefinedAsNull: true
|
||||
};
|
||||
|
||||
|
@ -259,7 +388,7 @@ describe('Replication over REST', function() {
|
|||
base: 'PersistedModel',
|
||||
plural: 'Cars', // use the same REST path in all models
|
||||
trackChanges: true,
|
||||
strict: true,
|
||||
strict: 'throw',
|
||||
persistUndefinedAsNull: true,
|
||||
acls: [
|
||||
// disable anonymous access
|
||||
|
@ -390,9 +519,6 @@ describe('Replication over REST', function() {
|
|||
function(next) {
|
||||
serverApp.dataSources.db.automigrate(next);
|
||||
},
|
||||
function(next) {
|
||||
ServerUser.deleteAll(next);
|
||||
},
|
||||
function(next) {
|
||||
ServerUser.create([ALICE, PETER, EMERY], function(err, created) {
|
||||
if (err) return next(err);
|
||||
|
@ -421,8 +547,8 @@ describe('Replication over REST', function() {
|
|||
function(next) {
|
||||
ServerCar.create(
|
||||
[
|
||||
{ maker: 'Ford', model: 'Mustang' },
|
||||
{ maker: 'Audi', model: 'R8' }
|
||||
{ id: 'Ford-Mustang', maker: 'Ford', model: 'Mustang' },
|
||||
{ id: 'Audi-R8', maker: 'Audi', model: 'R8' }
|
||||
],
|
||||
function(err, cars) {
|
||||
if (err) return next(err);
|
||||
|
@ -434,16 +560,38 @@ describe('Replication over REST', function() {
|
|||
}
|
||||
|
||||
function seedClientData(done) {
|
||||
LocalUser.deleteAll(function(err) {
|
||||
if (err) return done(err);
|
||||
LocalCar.deleteAll(function(err) {
|
||||
if (err) return done(err);
|
||||
async.series([
|
||||
function(next) {
|
||||
clientApp.dataSources.db.automigrate(next);
|
||||
},
|
||||
function(next) {
|
||||
LocalCar.create(
|
||||
[{ maker: 'Local', model: 'Custom' }],
|
||||
function(err, cars) {
|
||||
if (err) return done(err);
|
||||
if (err) return next(err);
|
||||
clientCars = cars.map(carToString);
|
||||
done();
|
||||
next();
|
||||
});
|
||||
},
|
||||
], done);
|
||||
}
|
||||
|
||||
function seedConflict(done) {
|
||||
LocalCar.replicate(ServerCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
if (conflicts.length) return done(conflictError(conflicts));
|
||||
ServerCar.replicate(LocalCar, function(err, conflicts) {
|
||||
if (err) return done(err);
|
||||
if (conflicts.length) return done(conflictError(conflicts));
|
||||
|
||||
// Hard-coded, see the seed data above
|
||||
conflictedCarId = 'Ford-Mustang';
|
||||
|
||||
new LocalCar({ id: conflictedCarId })
|
||||
.updateAttributes({ model: 'Client' }, function(err, c) {
|
||||
if (err) return done(err);
|
||||
new ServerCar({ id: conflictedCarId })
|
||||
.updateAttributes({ model: 'Server' }, done);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue