Skip to content

Remote hooks called before ACL rules are applied #1191

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

Closed
kortac opened this issue Mar 9, 2015 · 7 comments
Closed

Remote hooks called before ACL rules are applied #1191

kortac opened this issue Mar 9, 2015 · 7 comments
Assignees

Comments

@kortac
Copy link

kortac commented Mar 9, 2015

Right now ACLs are called after before remote hook. That means this stack of calls:

  1. Before remote Hook
  2. ACL
  3. Remote method
  4. After remote hook
    I'd expect ACLs to be checked prior calling before remote hooks and not the other way round.

Example:

Event.beforeRemote("prototype.__unlink__guests", function(ctx, modelInstance, next){
    console.log("before remote hook");
    next();
});

with this ACL

{
  "accessType": "*",
  "principalType": "ROLE",
  "principalId": "$everyone",
  "permission": "DENY",
  "property": "__unlink__guests"
}

Even though the Loopback explorer displays a 401 error, the console displays "before remote hook". In my opinion, this should not be the case.

@raymondfeng
Copy link
Member

@ritch ACL check is done from one of the remote hooks. Can we ensure it's called before any other remote hooks?

@ritch
Copy link
Member

ritch commented Mar 9, 2015

If you create the hook after boot it should run in the expected order.

@fabien
Copy link
Contributor

fabien commented Apr 25, 2015

@raymondfeng @ritch I think the expected behavior would be to have the ACL check be the first call in the stack. Or at least it should be configurable to work that way. It has some pretty serious implications, if you're not aware of this right now!

@fabien
Copy link
Contributor

fabien commented Apr 25, 2015

@raymondfeng @ritch I strongly believe this should be fixed ASAP. Any before remote hook defined at the models/model.js or mixin level will just execute the before hook without any regard to ACL checks. This is a security disaster waiting to happen.

@bajtos bajtos changed the title ACLs called after before hook Remote hooks called before ACL rules are applied Apr 28, 2015
@chandadharap chandadharap added #plan and removed #tob labels May 5, 2015
@ritch ritch mentioned this issue May 5, 2015
2 tasks
@chandadharap chandadharap changed the title Remote hooks called before ACL rules are applied 70 Remote hooks called before ACL rules are applied May 6, 2015
@chandadharap chandadharap changed the title 70 Remote hooks called before ACL rules are applied A Remote hooks called before ACL rules are applied May 6, 2015
@chandadharap chandadharap changed the title A Remote hooks called before ACL rules are applied 70 Remote hooks called before ACL rules are applied May 6, 2015
@chandadharap chandadharap changed the title 70 Remote hooks called before ACL rules are applied Remote hooks called before ACL rules are applied May 6, 2015
@ritch
Copy link
Member

ritch commented May 6, 2015

@fabien agreed. I've implemented this as both a new feature (a new authorization hook) and a fix (ensuring we use this hook instead of adding blindly as a remotes.before() hook).

See #1370

@fabien
Copy link
Contributor

fabien commented May 6, 2015

@ritch thanks!

@ritch
Copy link
Member

ritch commented May 12, 2015

@Toreader @fabien fixed @ 2.17.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants