Access restriction to model attributes

[obrienmd]

From: https://groups.google.com/forum/?fromgroups=#!topic/sailsjs/AM452zgLH-Y

As I understand policies allow restriction of viewing specific models. It would be important for larger apps to provide more granularity here, and allow restriction of viewing specific attributes/properties of a given model based on a policy.

[mikermcneil]

Hey @obrienmd, you can do this with custom controllers currently, but I agree being able to do this with policies and the blueprint controllers would be an awesome enhancement. What do you propose for how it would look?

I'd like to avoid putting ACL config directly in the model files, at least in the core, since we want auth/ACL to prevent access before ever hitting any controllers, so it would probably make sense for this to live in config/policies.js. What do you think?

[obrienmd]

Right - I'd considered custom controllers, but then we're losing a good amount of the magic that makes sails awesome. @kevinob11 and I have discussed this a bit, let me talk to him in person and we'll get some more concrete thoughts in here.

[mikermcneil]

@obrienmd Righto-- IMO, the first steps for Sails have been laying down the basics (hitting parity with the Rails core). Beyond that, there are some really neat options we can get into to automate the things we'd normally have to write custom code to do (e.g. using an SMTPAdapter with the blueprint API to send emails, or using the TwitterAdapter to fetch tweets, trends, or locations depending on the model) This fits perfectly into that roadmap :)

[kevinob11]

Just talked to @obrienmd. I'm thinking that most of the work would need to be done in the controllers. My use case is to allow access to the model still, but just prevent access to specific fields within that model. For example we want to return a list of users, but block access to just the password field. I think the config for this could fit into policies or model config, but the work to exclude the fields would need to be done in the controllers.

I haven't looked at all, but I'm also wondering if per field policy (while still configured in either policy or model files) could be handled in waterline.

I'm still just digging into to all this, so I'd love your thoughts on it, and @obrienmd and I will continue to discuss as we learn more.

[mikermcneil]

Waterline should not be in the business of access control, IMO-- in my experience, it's safest to run the ACL at the route/policy/controller level, because then you're explicitly blocking access to a public endpoint. As for partial access, I definitely see the limitations though, particularly if you're trying to rely mainly on blueprints.

Maybe what we could do is look at a nice clean way to do [select operations](http://en.wikipedia.org/wiki/Selection_(relational_algebra) via the Waterline query interface, and then consider the best way to handle that automatically in blueprints?

[particlebanana]

So I think I have a pretty elegant solution to hiding attributes in blueprints. I agree that access control shouldn't be at the data level and instead at the policy level but we also need a way to restrict certain attributes in an api. You don't want an encrypted password sent back in the api for instance.

In the new waterline you have model level instance methods. This poses a problem when serializing the attributes to JSON. So I can add a default .toJSON method on a model instance that gets returned from a query that the blueprints can call before returning the data in a response.

With instance methods, if you want to restrict an attribute you can just override this in the model. I'm not sure what the toJSON method would look like just yet but something along the lines of copying Mongoose like this:

Waterline.Model.extend({
  attributes: {

    // Some model attributes
    username: 'string',
    password: 'string',

    // An instance method that overrides the default
    toJSON: function() {
      var obj = this.toObject();
      delete obj.password;
      return obj;
    }
  }
});

Which would return an object with only the username and can then be used with req.json(model_instance.toJSON()) in the blueprint.

[obrienmd]

+1 on keeping Waterline out of the access control business to the extent possible.

Powerful selects makes sense, the 'best way to handle that automatically in blueprints' is going to be the hard part :)

[mphasize]

I am trying to keep the magic of blueprint controllers while at the same time protecting some model attributes from being changed by users on the default routes. I.e.: prevent access to the is_admin attribute on regular CRUD routes and implement a promote action or something similar on the UserController which makes the neccessary checks.

In order to do this, I came up with the following policy in combination with a small addition to the model definitions:

// file: api/policies/protectedAttributes.js

/**
 * protectedAttributes
 *
 * @module      :: Policy
 * @description :: Simple policy to protect certain attributes as returned by Model.protectedAttributes()
 * @docs        :: http://sailsjs.org/#!documentation/policies
 *
 */

var actionUtil = require( '../../node_modules/sails/lib/hooks/blueprints/actionUtil' );

module.exports = function ( req, res, next ) {

    var Model = actionUtil.parseModel( req );

    if ( Model.protectedAttributes ) {
        var attributes = Model.protectedAttributes();
        _.each( attributes, function ( attr ) {
            if ( req.params.hasOwnProperty( attr ) ) {
                delete req.params[ attr ];
            }
            if ( req.query.hasOwnProperty( attr ) ) {
                delete req.query[ attr ];
            }
            if ( req.body.hasOwnProperty( attr ) ) {
                delete req.body[ attr ];
            }

        } );
    }
    return next();
};

Inside a model I add the following function as a class method:

protectedAttributes: function () {
    return [ "is_admin", "email_verified" ];
},

So whenever I enable this policy on a route it reads the protected attributes and simply deletes all matching parameters from the request. The solution still feels a bit crude to me, but I get the behavior I want out of it. Any thoughts?

Cheers! Marcus

[mikermcneil]

@mphasize thanks a lot for posting, that looks awesome! feel free to continue this discussion in stackoverflow or the google groups, we're cleaning house up in here

[albertpeiro]

What about output policies @mikermcneil? Similar to the current policies but applied only after the controller pass, and before the response gets sent. Separating them would make sense as they're not the same and the workaround from @mphasize seems quite close to a solution, though it's just not clean.

To me it sounds like a powerful idea to have, it's one of the biggest things I'm missing at the moment. Being able to modify the response on a different layer than the actual controller and apply this policy on a controller basis (same as current).

It may seem like the use case is small, but I find when trying to do any medium size project this comes up as a requirement. It actually comes up as a requirement way way earlier when we need to build a backoffice and modify protected attrs only from certain controller.

Any thoughts?

[albertpeiro]

These seem related:

• https://github.com/balderdashy/sails/issues/2423
• https://github.com/balderdashy/sails/issues/352
• https://github.com/balderdashy/sails/issues/2729

[RWOverdijk]

This should be done upon select, not response. It's overhead per-row. It's not needed.

[albertpeiro]

@RWOverdijk not sure, sometimes I need to get all attributes in the controller from a query, like it works with current implementation of "protected". But I only want to send to the user specific ones based on the controller (but would be really awesome to even configure on a per action basis).

Protection as it is, it's great! What's missing is the ability to granularly attach protection to controllers <-> attributes.

[RWOverdijk]

@albertpeiro I understand what you're saying, but must advice against it for a couple of reasons.

The first reason is the fact that you're talking about needing all attributes, but wish to send back only a couple. If you need all of them, that probably means that you don't use the blueprints.So you might as well implement this yourself. But, reason two, is performance. Checking every attribute of every entity of every collection every time might become a heavy task to perform.

[tjwebb]

But I only want to send to the user specific ones based on the controller

I can understand the use case here, but we can't make everything infinitely configurable. sails.js gives you a lot out of the box, but at some point you have to start writing code :) I started sails-permissions as a starting point for dynamic policies, which will include attribute-level control of this kind, but which isn't yet implemented. I'd be interested in hearing more of your ideas on this over in sails-permissions.

But, reason two, is performance. Checking every attribute of every entity of every collection every time might become a heavy task to perform.

Yea this is really slow. Some enterprise-y applications do need this though, since many of them will have ridiculously granular access control. Like, people in the sales department can update create and update Invoice, but only the accounts receivable department can modify the field paid to true, and aren't allowed to read the line item details so those have to be stripped out when they are viewing Invoices. This is one potential class of problem that sails-permissions takes a shot at.

[albertpeiro]

I can understand the use case here, but we can't make everything infinitely configurable. sails.js gives you a lot out of the box, but at some point you have to start writing code :)

Absolutely. I didn't mean to be pushy, just trying to build a case and see if it's important enough and what benefits it could bring. And see what's the best way to tackle this problem, and if we can/should, add to sails/build other modules.

My case

The way I see it is we have this awesome MVC framework with a powerful mix of CoC sort of thing, but still easily extensible and presumably hyper-performant. One of its most powerful features is it has Models separated from Controllers. Now, one of the main advantages I see in using controllers is to map controllers to "perspectives" of the application, or points of view. For example: backoffice tool wants to be able to modify a User's email. On any other part of the application the User's email cannot be accessed because it's public (list of users, list of content with populated users). In the User's profile, the user needs to be able to see, and edit their email.

For me the whole point of having an MVC at all is to be able to modify/serve data from a different perspective (in a different way). From a Backoffice perspective, from a public user perspective, from a private users... and inside each of those I may need some attrs for a specific model in one action, or all of them in a different action. If you have perspectives you inevitably have different data needs, otherwise you most probably wouldn't have perspectives in first place. Considering all an API delivers is data. Data most of the times will be different. If data is different then why not build something to handle data delivery differently for each controller and action? To me that's the question we need to ask. Generalistic and tool-focused.

When I look at this case I can't help thinking this is way too common to not help solving it, and I'm like "damm if Sails had this built in it'd blew my mind and most importantly it'd relieve me from a huge pain", in the same way Sails blew my mind and relieved me from massive pain when I saw the CORS, blueprints, database & adapters,...

When you say we can't build everything you're right. But somehow Sailsjs manages to have an INSANE amount of things built in that required little config with a lot of extensibility. And that is the reason I use it. Hence, my proposal. This is not a minimalistic framework. It's a "it's built and configured by default, extend easily" framework.

About performance

I'm not biased towards any solution here. I just wanted to get my point across for the need of this on sails. But when it comes to performance we must not forget to look at the forest and not just the tree. So let's look at the system and not just the server. If you have different views that require different data (that's why you have different views), then chances are most of the times some attributes (or many!) are not going to be needed. You're sending that data on every request and it's not needed by the client. That is not high-performance either, and arguably, depending on the type of application, especially mobile, is even worse than having a server loop over a pages attributes (because at least you pay the bill, and not your users ;) )

Frameworks like GWT do some auto optimisations to ensure only the needed attributes are sent per view.

@tjwebb thank you for sharing sails-permissions I do like the idea of it. But I think tying data delivery granularity to an ACL is focusing only on part of the problem. I'd love to move Sails to production environments where I can apply performance optimisations outlined above.

[st3wart]

I am using this http://sailsjs.org/documentation/reference/waterline-orm/records/to-json to remove unwanted attributes

module.exports = {
  attributes: {
    name: 'string',
    phoneNumber: 'string',

    toJSON: function() {
      var obj = this.toObject();
      delete obj.phoneNumber;
      return obj;
    }
  }
}

[LouAdrien]

Hello, I know this topic is silent for a while now, but re-animating it : I think the proposed solutions are good, but not complete, as they forget to account for association data.

Many models might have associations setup, and the blueprint API will allow the setting of nested association data during update or create actions. In order to make sure all the protected attributes are well removed from the tree data structure, I made the following (modified) implementation. What do you guys think about it?

/**
 * removeProtectedAttributes policy
 *
 * @module      :: Policy
 * @description :: Simple policy to protect API calls from setting attributes mark as "private"
 *
 */

 var actionUtil = require( '../../node_modules/sails/lib/hooks/blueprints/actionUtil' );

 module.exports = function ( req, res, next ) {

     var model = actionUtil.parseModel( req );
     if ( model.protectedAttributes ) {
         var attributes = model.protectedAttributes;
         _.each( attributes, function ( attr ) {
           if ( req.params.hasOwnProperty( attr ) ) {
               delete req.params[ attr ];
           }
           if ( req.query.hasOwnProperty( attr ) ) {
               delete req.query[ attr ];
           }
           if ( req.body.hasOwnProperty( attr ) ) {
               delete req.body[ attr ];
           }
         });
     }
     cleanAssociationData(model,req.body)
     return next();
 };

 // association data can be given only via body parameters
 // This method will recursively check the model definition tree for possible association and clean the related request body data if present
 function cleanAssociationData(model, reqAssosData){
   if(!model || !reqAssosData) return;
   for(var attributeName in model.attributes){
      var attr = model.attributes[attributeName];
      if(attr.model){
        // if a simple 1 to 1 association
        var associationModelName = attr.model;
        var associationModel = sails.models[associationModelName];
        var associationAttributes = associationModel.attributes;
        var associationProtectedAttributes = associationModel.protectedAttributes;
        if(associationProtectedAttributes && reqAssosData[attributeName]){
          for (var assosDataKey in reqAssosData[attributeName]) {
            var associationAttrValue = reqAssosData[attributeName][assosDataKey]
            if(associationProtectedAttributes.indexOf(assosDataKey) != -1) delete reqAssosData[attributeName][assosDataKey];
            // Check if current data related to an association, if yes, recursively clean it too.
            var subAssociationKey = associationAttributes[assosDataKey] ? associationAttributes[assosDataKey].model || associationAttributes[assosDataKey].collection : false;
            if(subAssociationKey) cleanAssociationData(sails.models[subAssociationKey],reqAssosData[attributeName][assosDataKey]);
          }
        }
      }else if(attr.collection){
        // if a 1 to many associastion
        var associationModelName = attr.collection;
        var associationModel = sails.models[associationModelName];
        var associationAttributes = associationModel.attributes;
        var associationProtectedAttributes = associationModel.protectedAttributes;
        if(associationProtectedAttributes && reqAssosData[attributeName] && reqAssosData[attributeName].length){
          for (var i = 0; i < reqAssosData[attributeName].length; i++) {
            for (var assosDataKey in reqAssosData[attributeName][i]) {
              var associationAttrValue = reqAssosData[attributeName][i][assosDataKey]
              if(associationProtectedAttributes.indexOf(assosDataKey) != -1) delete reqAssosData[attributeName][i][assosDataKey];
              // Check if current data related to an association, if yes, recursively clean it too.
              var subAssociationKey = associationAttributes[assosDataKey] ? associationAttributes[assosDataKey].model || associationAttributes[assosDataKey].collection : false;
              if(subAssociationKey) cleanAssociationData(sails.models[subAssociationKey],reqAssosData[attributeName][i][assosDataKey]);
            }
          }
        }
      }
    }
  }

[NachtRitter]

@mikermcneil it would be awesome if Waterline could have something like JMSSerializerBundle for Symfony. In Entity I define property and define some serialize groups for which this property will be serialized, like:

/**
     * @var string
     *
     * @ORM\Column(name="slug", type="string", length=255, nullable=true)
     * @JMS\Expose
     * @JMS\Groups({"detailed"})
     */
    private $slug;

    /**
     * @var ArrayCollection|Product[]
     *
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\SearchTag", mappedBy="menu")
     * @JMS\Expose
     * @JMS\Groups({"list"})
     */
    private $searchTags;

In controller I can set either "detailed" or "list" serialize group and get entity with different serialized fields.

[LouAdrien]

As anyone check my previous fix? This is a working solution and could be integrated.