Fixes #9 include all attributes when no roles are specified to get

[hendrul]

What has changed?

• Inverted polarity from whitelisting to blacklisting by default ("raw" option is implicit when calling get without a role to apply, so it was removed.)
• Added support for "default" role, applied when no role given or matched

Why this change?

I think the metaphor of roles in this plugin is taken too literally, as a security mecanism but this is not the real use case, what we want is to explicitly pick the attributes to "filter" depending on the situation or data destination (send to client? or use internally?), and not a super strong role base access mecanism, that prevent us from get/set sensible data by mistake. We choose!.
This way this plugin would cover the following use cases:

//Having...
User = sequelize.define('user', {
  email: {
    type: Sequelize.STRING,
    roles: {
      default: {get: true}, //default role definition
      system: {get: true},
      client: true
    }
  },
  password: {
    type: Sequelize.STRING,
    roles: {
       default: false, 
       client: false
    }
  },
  rank: {
    type: Sequelize.STRING,
    roles: {
      default: 'client', //we could also reference other role as default
      client: {set: false, get: true}
      system: true
    }
  }
});

//UC1: Sending data to client.  
let data = yield User.findById(1).get({plain: true, role: 'client'});
res.json(data); 

//UC2: Preparing raw data before send to service layer
let user = yield User.findById(1).get({plain: true}); //we need all attributes
user.properties = user.cascadeProperties;
delete user.cascadeProperties;

//UC3: Setting data coming from client
user.set('rank', req.body.rank) //Ignored
user.update(req.body) //rank and password are ignored by default roles
user.save(req.body) //rank and password are ignored by default roles
User.create(req.body) //rank and password are ignored by default roles

[mickhansen]

io.js failing, go ahead and remove it from .travis.yml

[mickhansen]

I think this case should rather throw, i don't like silently failing.

[hendrul]

What do you mean by silently failing?. Not returning boolean but rising an error instead?

[mickhansen]

@i don't think that applying the default role for a "bad" role is a good default.

[hendrul]

Do you mean that if I try to save some attribute defaulted to set: false, I'll get an error?. In my particular case the client could send all attributes and I need just omit those protected ones. I think throwing is obtrusive (at least in my case) I would be forced to enclose every operation in try-catch block when I just need to be careful when assigning roles. Maybe this should be configurable don't you think?

[mickhansen]

We should avoid configuration on call-site at all costs in favour of reasonable convention.

What i mean is that if you call get('attr', {role: 'roleThatDoesNotExist'}) it should throw, not default to using the default role.

[mickhansen]

I agree, the original default is almost unworkable and hard to implement unless you do so from the beginning.

[hendrul]

Maybe its a good idea to make this behavior configurable. I mean, being able to toggle it from blacklist to whitelist as needed. For backward compatibility.

[hendrul]

Although it's cool to have "default" roles, sometimes we may also want to bypass them and just get/set everything, This could be like: instance.update(..., { role: null })
One annoyance I see is when building instances, we always want to set all, bypassing the default roles, but this means we would have to pass { role: null } every time we build an instance!.

What if from get/set we don't apply default roles if the caller is Instance.build. Is that a horrible idea?

[mickhansen]

@hendrul I prefer this behaviour so fine with a major bump.
If people want to bypass roles they should define an omnipotent role.

Sequelize uses raw with build internally so it shouldn't interfere.
As for build via create i still think it should be protected by roles.

[hendrul]

For this common use case the problem I see with an omnipotent role is that now using blacklisting you should not forget to add that role for every attribute you have, while using just null as role means you want to disable roles

[mickhansen]

@hendrul I see your point, {role: null} is just too easy to mess up i think. Maybe {role: DISABLED} with DISABLED being some library supported constant.