mickhansen / ssacl-attribute-roles

Simple attribute whitelisting/blacklisting with roles for Sequelize
MIT License
62 stars 10 forks source link

Use with update and sequelize 3.0 #6

Open gonzobrandon opened 9 years ago

gonzobrandon commented 9 years ago

Having a hard time using ssacl-attribute-roles with sequelize 2.0...

I want to update a record, but conform to aacl roles I set..e.g.:

var Devices = sequelize.define('devices', {});

    ssaclAttributeRoles(sequelize);
    ssaclAttributeRoles(Devices);

    Devices = sequelize.define("devices", {
        Name: {
            type: DataTypes.STRING,
            roles: {
                admin: true,
                public: false
            }
        }, 

// Then in a controller:

Devices.update(
        {
            Name: 'foobar'
        }
        {
            where: whereClause
            role: 'public'
        }
    ); //This should not work, right?

I have also tried:

Devices.findOne(
        {
            where: whereClause
        }).then(function(record) {
             record.set({Name: 'foobar'}, {role: 'public'});

             record.update(record.dataValues, {where:whereClause})
})

It still writes to the DB despite public being forbidden. Help anyone?

mickhansen commented 9 years ago

Appears to be an issue with Sequelize not passing options to set correctly: https://github.com/sequelize/sequelize/blob/master/lib/instance.js#L773

Btw you should only initialize ssaclAttributeRoles ONCE, on individual models OR on sequelize, not on both.

mickhansen commented 9 years ago

I've opened a PR here: https://github.com/sequelize/sequelize/pull/4138 But you'll have to upgrade to the latest 3.x.x when it's merged.

gonzobrandon commented 9 years ago

Thank you for looking into this and making the PR

mickhansen commented 9 years ago

@gonzobrandon You might want to go ahead and start upgrading to 3.x in any case so you've taken care of all the BC stuff :) I'll hopefully release a new version of Sequelize with the fix on monday.

gonzobrandon commented 9 years ago

@mickhansen, even after updating to ssacl-attribute-roles version 0.0.5 and sequelize 3.5.1, I still have the same issue with set.

e.g.

    Devices = sequelize.define("devices", {
        name: {
            type: DataTypes.STRING,
            roles: {
                admin: true,
                public: true
            },
        type: {
            type: DataTypes.STRING,
            roles: {
                admin: true,
                public: {set: false, get: true}
            },

        }, 

// Then in a controller:
// (assume there is a record in DB as: {name: 'foobar', type: 'television'} )

Devices.findOne({name: 'foobar'}).then(function(device) {
    device.set({type: 'computer'}, {role: 'public'});
    console.log(device.get({role: 'public'}));   //This should return {name: 'foobar', type: 'television'}  (no change), but it is actually making the change.

});

Set is working despite set: false in model/role definition.