Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions about authentication & ACL #222

Closed
Cadrach opened this issue Apr 2, 2014 · 33 comments
Closed

Questions about authentication & ACL #222

Cadrach opened this issue Apr 2, 2014 · 33 comments

Comments

@Cadrach
Copy link

Cadrach commented Apr 2, 2014

Hi,

I have a few questions about authentication:

I have model A and A hasMany B. Also, A has a property "users" which holds links to users that should be able to access it. How can I configure B, so that it can be read & written only by users allowed to A? Example high level code:

A.hasMany('b', {model: B});
A.hasMany('users', {model: User});

I am a bit lost for this: should I DENY all in B, and work only with hooks? But it looks like hooks only work with remote methods, so how can I protect B so that it can be listed with A only if User is in A.users?

@Cadrach
Copy link
Author

Cadrach commented Apr 2, 2014

Just saw that hooks work with pre-defined method. I will work from here but input still appreciated

@Cadrach
Copy link
Author

Cadrach commented Apr 2, 2014

I am trying to add a remote method to the User, but since I am extending the base User model, I cannot access it, I have tried adding the access through ACL.create, but to no avail.

var db = require('../data-sources/db');
var loopback = require('loopback');
var ACL = loopback.ACL;

// define a User model
var User = loopback.User.extend('User',{

});

// attach to the memory connector
User.attachTo(db);

User.findByUsername = function(username, callback){
    console.log('FIND BY USERNAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAME');
}

//Link remote methods
loopback.remoteMethod(
    User.findByUsername,
    {
        accepts: {arg: 'username', type: 'string'},
        returns: {arg: 'users', type: 'object'},
        http: {path: '/findByUsername', verb: 'get'}
    }
);

ACL.create( {
    accessType: ACL.ALL,
    permission: ACL.ALLOW,
    principalType: ACL.ROLE,
    principalId: '$everyone',
    model: User,
    property: User.findByUsername
});

What am I doing wrong?

@bajtos
Copy link
Member

bajtos commented Apr 2, 2014

Hi @Cadrach, the code in your last comment describes only the setup, not the actual call of the findByUsername method. How did you called it and what error did you received?

@bajtos
Copy link
Member

bajtos commented Apr 2, 2014

I don't know enough about ACLs to answer you original question, we'll have to wait for @ritch.

@raymondfeng
Copy link
Member

The ACL create should be:

ACL.create( {
    accessType: ACL.ALL,
    permission: ACL.ALLOW,
    principalType: ACL.ROLE,
    principalId: '$everyone',
    model: 'User', // Name of the model
    property: 'findByUsername' // Name of the property/method
}, callback);

@Cadrach
Copy link
Author

Cadrach commented Apr 2, 2014

@bajtos Thanks anyways :)
@raymondfeng It did the trick, thanks! I will leave the issue open a bit and post again if I get stuck.

For anyone having the same kind of issue: I ran the request through the built in explorer, and also through the generated angular JS. In fact since it is a GET I can run it though the browser and get the same return, here it is :

{
  "error": {
    "name": "Error",
    "status": 401,
    "message": "Access Denied",
    "statusCode": 401,
    "stack": "Error: Access Denied
    at node_modules/loopback/lib/application.js:298:21
    at node_modules/loopback/lib/models/acl.js:463:17
    at node_modules/loopback/lib/models/acl.js:426:19
    at node_modules/async/lib/async.js:232:13
    at node_modules/async/lib/async.js:119:25
    at node_modules/async/lib/async.js:24:16
    at node_modules/async/lib/async.js:229:17
    at node_modules/async/lib/async.js:516:34
    at node_modules/loopback/lib/models/acl.js:410:15
    at node_modules/loopback/lib/models/role.js:184:19"
  }
}

@Cadrach
Copy link
Author

Cadrach commented Apr 2, 2014

Ok, so new question :), how can I change the "public" property of a Model from the JS?

var db = require('../data-sources/db');
var config = require('./message.json');
var Message = module.exports = db.createModel(
    'Message',
    config.properties,
    config.options
);
//Now I would like to do something like
Message.setPublic(false);

@raymondfeng
Copy link
Member

By default, models are not public unless you (or app.boot()) calls app.model(model).

@Cadrach
Copy link
Author

Cadrach commented Apr 5, 2014

Now that you say it, it seems logical :)

New questions if you do not mind:

  • I do not like the fact that I need to expose the User service to take advantage of "login" and "logout", but then the "findById" request returns the hashed password, as you can imagine it does not sit well with me :). How would you go about that: we need the hashed password server side, but we do not want to send it ever, should I do this with hooks too? If so how can the hook get the difference between server & client calls?
  • Related to this, I have created a model A that has a "belongsTo" relations with User (to store User registered to this entity). Doing this means that any request which "include" the relations will get the full User information, again with hashed password and email. Do I need a post-hook in "A" to remove the sensible information? (This is quite common in my setup)

@bajtos
Copy link
Member

bajtos commented Apr 7, 2014

Hi Cadrach, regarding the JSON representation of the User containing password hash: the issue has been already fixed in v1.7.4. See #160 and #217.

@bajtos bajtos added the question label Apr 7, 2014
@Cadrach
Copy link
Author

Cadrach commented Apr 7, 2014

I updated to 1.7.4 but I still have the issue (I double-checked and the options.hidden modification are indeed in the code):

Participant.belongsTo(User, {
    as: 'user',
    foreignKey: 'userId'
});

I call

http://localhost:3000/api/Participants?filter={"include":"user"}
// Encoded this gives: api/Participants?filter=%7B%22include%22%3A%22user%22%7D

And I get the following (I leave the full id & hash since it is test data):

[
  {
    "id": "533ead93d6d8c95cdcda247e",
    "userId": "53144d4e9e79b6f057ffb9c7",
    "user": {
      "realm": null,
      "username": "a",
      "password": "$2a$10$bh0t7SJCxBkpEkdTh2gvDe4.PDbAxNeWDHZTPRW1Pht17sBEmCYPm",
      "email": "[email protected]",
      "emailVerified": null,
      "verificationToken": null,
      "credentials": [],
      "challenges": [],
      "status": null,
      "created": null,
      "lastUpdated": null,
      "id": "53144d4e9e79b6f057ffb9c7"
    }
  }
]

As a side question, do you know where I can look to get an advanced example of Scope usage, I think this is what I need in my application (Member creates a model instance and has specifics rights on it and its related models)

@bajtos
Copy link
Member

bajtos commented Apr 7, 2014

@ritch It seems that your fix does not work for nested object, is that possible?

As a side question, do you know where I can look to get an advanced example of Scope usage, I think this is what I need in my application (Member creates a model instance and has specifics rights on it and its related models)

That's a question @raymondfeng can answer best.

@Cadrach
Copy link
Author

Cadrach commented Apr 9, 2014

@raymondfeng I can see you are under heavy load, could you just confirm if the Scope is the right way to look for? As a reminder what I would like to achieve is:
Assign a Role to a User only for a specific model instance, and allow him some higher rights for this instance and its related models instances (For example a forum moderator with moderation rights only on some sub-forums, and their posts).

@raymondfeng
Copy link
Member

Scope is basically a predefined query. The filter can be preset by the server code. It can be used to constrain find with ACL.

You can also use custom methods to define high order functions.

Sorry for the late reply as I'm sick out.

@raymondfeng
Copy link
Member

Reopen as it's closed by mistake.

@raymondfeng raymondfeng reopened this Apr 9, 2014
@Cadrach
Copy link
Author

Cadrach commented Apr 10, 2014

@raymondfeng Thanks for your answer, sorry to bother you while sick, take care! You can answer the following only when you feel better :)

Ok, I guess Scope is a kind of a "view" like in MySQL. So if I want to solve my problem nicely, and we keep the example of Moderator/Forum/Post, do you think I could create a new Role Moderator_ForumId for each forum, where ForumId would be replaced by the Forum model instance id?

Would this make the ACL system overweight? I then would have to add this new Role to each Forum and related Post for this to work, and I could grant this Role to User that would be Moderator of this particular Forum.

@ritch
Copy link
Member

ritch commented Apr 10, 2014

@Cadrach can you open a new issue for the nested object containing a hidden property?

@Cadrach
Copy link
Author

Cadrach commented Apr 17, 2014

If anyone can answer my previous question @raymondfeng it would be much appreciated. In the meantime could someone provide an example for the following:
http://docs.strongloop.com/display/DOC/Controlling+data+access#Controllingdataaccess-Staticpermissions

Protected Resource: A model, a model instance, or a model property, method, or relation. Protected resources can have principals as owners. 

How do you define a model instance as a protected resource? An example using ACL.create() or something similar would be welcome.

Thanks all for your time on this great app :)

@raymondfeng
Copy link
Member

Do you have chance to check out http://docs.strongloop.com/display/DOC/Model+definition+reference#Modeldefinitionreference-ACLs?

The static ACLs can be defined at the models.json level.

@Cadrach
Copy link
Author

Cadrach commented Apr 17, 2014

Thanks for your reply @raymondfeng, but sadly I do not see what I need here. I know and already did add ACL to a Model, but what I need here is to add a specific access rights to a Model Instance, not to all instances of the Model.

All I can find as of now is how to setup ACL for:

  • All instances of the Model (but again, I need to set it for one instance in particular)
  • A method/property/relation of the Model

In the part I highlighted, there is mention of specifying model instance as the protected resource (not as the principal)

Finally, I cannot explain it better than with the example of a project with 3 models: Forum / Post / User. If you want to grant Moderator rights to only one Forum (and not all Forums of your website) to a User, how would you do it?

@raymondfeng
Copy link
Member

I think this is what you are after:

  1. Define a dynamic role, such as ‘$mediator’
  2. Create a role resolver (check if a requestor is in the role within the model instance context), for example:
    https://github.com/strongloop/loopback/blob/master/lib/models/role.js#L224
  3. Register the role resolver
    https://github.com/strongloop/loopback/blob/master/lib/models/role.js#L169

Thanks,


Raymond Feng
Co-Founder and Architect @ StrongLoop, Inc.

StrongLoop makes it easy to develop APIs in Node, plus get DevOps capabilities like monitoring, debugging and clustering.

On Apr 17, 2014, at 1:58 PM, Cadrach [email protected] wrote:

Thanks for your reply @raymondfeng, but sadly I do not see what I need here. I know and already did add ACL to a Model, but what I need here is to add a specific access rights to a Model Instance, not to all instances of the Model.

All I can find as of now is how to setup ACL for:

All instances of the Model (but again, I need to set it for one instance in particular)
A method/property/relation of the Model
In the part I highlighted, there is mention of specifying model instance as the protected resource (not as the principal)

Finally, I cannot explain it better than with the example of a project with 3 models: Forum / Post / User. If you want to grant Moderator rights to only one Forum (and not all Forums of your website) to a User, how would you do it?


Reply to this email directly or view it on GitHub.

@Cadrach
Copy link
Author

Cadrach commented Apr 17, 2014

This has got my fingers tingling, I will try it asap, thanks a lot!

@Cadrach
Copy link
Author

Cadrach commented Apr 17, 2014

Ok, almost there, I am using the Role.registerResolver to add my role (I kept the structure with process.nextTick). Now I have a small issue:
When I use "findById", the "modelId" property is correctly set in the context parameter:

//Parts of context parameter received
{
  modelName: 'Model',
  modelId: '530a4696808f657c13e50282',
  property: 'findById',
  method: 'findById'
}

However, when I run "findOne" (which I use the most since it allows to add the "include" parameter to fetch related data), the modelId property of the context is set to undefined. Is this the expected behavior? How can I get the model instance and test for access rights if so?

@raymondfeng
Copy link
Member

It seems that a post check is needed in these cases. The results of findOne or find are model instances. Before such values are sent back to the client, they need to be checked against the ACL.

I'm debating how far we want to go here with ACL. Typically, ACLs are enforced before the requested operations are performed. You can probably set up a 'after' hook to do the check by code once the results are produced.

We should take the following cases into consideration too:

  1. find with a filter
  2. findOne with a filter
  3. delete with a filter
  4. update with a filter

It would be difficult to enforce 'only owner is allowed' rule for the operations above.

@Cadrach
Copy link
Author

Cadrach commented Apr 18, 2014

It is true, I could probably set an "after" hook, but I would need to add a hook to every model and its related models that I want to use the same feature.

Currently, the Role.isOwner function already performs a findById on the model class to find the model instance in most case (in fact, every case but when testing the User model and its child models).

I understand that a request returning many model instances would require as many findById to fetch the information about the ownership. However, if the feature is needed, the findById will have to take place, be it already in the provided loopback code or in an after hook made by the developer.

Finally, I found the resolver system really interesting, it allows me to develop my own complex roles, but I can keep the declarative ACLs system to use the new dynamic roles. So even if you remove the $owner role (because you probably should since the current behavior is not coherent on all method of the model), you should keep the resolver system (and somehow add the modelId in every case).

If the way you want to go is removal of $owner, you should probably think about providing a proper way to use ACL to deny/allow specific model instances, in most project I come accross, there is always some kind of specific resource accesses.

PS: I hope I dont come accross too harsh, I really loved the glimpse the resolver gave me and would really like to use it in full ^_^

@karlmikko
Copy link

@raymondfeng @Cadrach I have setup a post boot function to loop all the models and overload the find/findOne/delete/etc methods.

I then look for the filter/where and change it to be {and:[oldwhere, newwhere]}.

In order to work out what newwhere should be is to look at who is logged in via ctx.req.accessToken.user.

In effect I am creating a 'default scope' for the model based on the user.

I also have some extra code so i can configure more filters based on application settings.

@karlmikko
Copy link

I also use this post boot to filter on tables that have a property is_deleted so we can soft delete.

@coodoo
Copy link

coodoo commented Aug 8, 2014

For a upsert operation, probably a beforeRemote check is necessary?

@weiner
Copy link

weiner commented Dec 2, 2014

@karlmikko: I like your suggestion to add a filter based on the on the ctx.req.accessToken.user with a boot function.
But how can I read the ctx in the overloaded method, e.g. the find method.
Hier is what I try, I don't know how to get the ctx

module.exports = function(app) {
  var DemoModel, find;
  DemoModel = app.models.demo;
  find = DemoModel.find;
  return DemoModel.find = function(filter, cb) {
   //ctx?
    console.log("demo.find filter", filter, this);

    return find.call(DemoModel, cb);
  };
};

@bajtos
Copy link
Member

bajtos commented Dec 3, 2014

@weiner Until recently, it was not possible to access the context from your model methods.

#337 added a concept of loopback.getCurrentContext(), unfortunately it is not documented yet (see #761). You can find some related information in #766 and #809, especially see this comment).

@bajtos
Copy link
Member

bajtos commented Dec 3, 2014

@Cadrach @karlmikko @raymondfeng is there anything left to discuss/answer in this issue, or can it be closed as resolved?

@Amir-61
Copy link
Contributor

Amir-61 commented Dec 3, 2015

I followed all of the conversation here and not only isn't there any activities for a long time, but also it seems all of the user's questions have been already answered; I will be closing this ticket soon...

Thanks!

@Amir-61
Copy link
Contributor

Amir-61 commented Dec 3, 2015

I'm closing this ticket now for the user's problems seem to be resolved.

@Amir-61 Amir-61 closed this as completed Dec 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants