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

Does readOwn method include readAny inheritance? #14

Closed
miketdonahue opened this issue Sep 10, 2017 · 12 comments
Closed

Does readOwn method include readAny inheritance? #14

miketdonahue opened this issue Sep 10, 2017 · 12 comments
Labels
question A question rather than an issue.

Comments

@miketdonahue
Copy link

Hi Onury,

Really like what you did with this library 👍 .

I have a question for you. If I have a permission structure like this:

[
  { role: 'admin', resource: 'user', action: 'create:any', attributes: ['*'] },
  { role: 'admin', resource: 'user', action: 'read:any', attributes: ['*'] },
  { role: 'admin', resource: 'user', action: 'update:any', attributes: ['*'] },
  { role: 'admin', resource: 'user', action: 'delete:any', attributes: ['*'] },

  { role: 'user', resource: 'user', action: 'read:own', attributes: ['*'] },
  { role: 'user', resource: 'user', action: 'update:own', attributes: ['*'] },
  { role: 'user', resource: 'user', action: 'delete:own', attributes: ['*'] },
]

And in my controller, I do a check for can(role).readOwn(resource)... if the user I am checking on has a role of admin does the code assume that because admin's have access to read:any they can also read:own?

That is what I am seeing happen in my debugger, which makes sense to me. I just want to confirm that is happening?

For instance, if I am logged in as an admin and a run a check for can(role).readOwn(resource), I am returned TRUE from granted, even though I did not specifically state an admin can read:own in my permissions above.

I ask because in another question I see that you suggest the following:

var role = req.user.role;
// check if the request is for own photos or any
var permission = (req.user.name === req.params.username)
   ? ac.can(role).updateOwn('photo')
   : ac.can(role).updateAny('photo');

Is this necessary or can I just check ac.can(role).updateOwn('photo')?

Thanks!

@onury onury added the question A question rather than an issue. label Sep 10, 2017
@onury
Copy link
Owner

onury commented Sep 10, 2017

Does readOwn method include readAny inheritance?

No @sidewalksalsa. It's the other way around. any is omniscient. It includes own, which is logical. But don't let it fool you!

Is this necessary or can I just check ac.can(role).updateOwn('photo')?

ac.can(role).updateOwn('photo') has no meaning by itself.

Checking only for own is not sensible and will typically return true (if the resource is not explicitly denied). But then how do you know the resource is actually owned by that user (of that role)?

..Own requires you to also check for the actual possession. The "check" and <action>Own should always be validated in conjunction (&&).

..

So, you could write it like this:

const grantedIfOwn = req.user.name === req.params.username 
    && ac.can(role).updateOwn('photo').granted;

Now, although "admin" role is granted any photos; this would return false for "admin"; if the resource is not actually owned by this admin user.

That's why you should conditionally (not optionally) also check for ..Any:

const permission = (req.user.name === req.params.username)
   ? ac.can(role).updateOwn('photo')
   : ac.can(role).updateAny('photo');

console.log(permission.granted);

@miketdonahue
Copy link
Author

Cool, definitely makes sense. Thanks for the response @onury! Cheers.

@onury
Copy link
Owner

onury commented Sep 10, 2017

No problem at all. Thanks.

@pawangspandey
Copy link

pawangspandey commented Jan 23, 2018

How about using middleware for express.js?
https://github.com/pawangspandey/accesscontrol-middleware

@scandinave
Copy link

Maybe i'm wrong but with req.user.name === req.params.username
If the user update another user but in the request sets params.username to it's name, the authorisation will validate and it will be able to change any users on the system no?

@onury
Copy link
Owner

onury commented Feb 10, 2018

That's a pseudo express.js example. req.user.name cannot be set by the end-user. It's set by the server, when he/she is authenticated.

End-user can try and set params.username in the endpoint to anything. But it doesn't matter. The user can make a change only if req.user.name (the session username) matches the username in params... which indicates "own" username.

@scandinave
Copy link

scandinave commented Feb 10, 2018

I'm a a user foo. req.user.name will be set by server and equals to foo. I try to update a photo that is own by user bar. I set this object for example into the request

{
    id: 2,
    src: "path_to_photo"
    username: "foo" // Replace original bar by foo
}

The condition will validate and i will be able to update the path of the photo.

The only way i see is to retrieve the ressource from the server to get the original username

Something like that :

let photo = photoService.findPhotoById(req.params.id);
const permission = (req.user.name === photo.username)
   ? ac.can(role).updateOwn('photo')
   : ac.can(role).updateAny('photo');

@onury
Copy link
Owner

onury commented Feb 11, 2018

The example/idea was making a request to a REST endpoint like:
/users/:username/photos/:photoid.
e.g. POST /users/foo/photos/2

You could handle this request like below:
(This is a bit verbose to make it clear also for other readers)

app.post('/users/:username/photos/:photoid', (req, res) => {

    const realUsername = req.user.name;
    const targetUsername = req.params.username;
    const permission = (realUsername === targetUsername)
        ? ac.can(req.user.role).updateOwn('photo')
        : ac.can(req.user.role).updateAny('photo');

    if (permission.granted) {
        // get photo by the endpoint params — :username AND :photoid
        // so this does not return a photo object if username does not match.
        const photo = photoService.getUserPhoto(targetUsername, Number(req.params.photoid));

        // return Not Found if no photo 
        if (!photo) return res.status(404).end();

        const updateData = permission.filter(req.body);
        const result = photoService.updatePhoto(photo.id, updateData);
        return res.json(result);
    }

    // otherwise, return forbidden
    return res.status(403).end();
});

Your concern would be correct if the endpoint was: /photos/:photoid
e.g. POST /photos/2

app.post('/photos/:photoid', (req, res) => {
    const photo = photoService.getPhoto(Number(req.params.photoid));
    if (!photo) return res.status(404).end();

    const permission = (req.user.name === photo.username)
        ? ac.can(req.user.role).updateOwn('photo')
        : ac.can(req.user.role).updateAny('photo');

    if (permission.granted) {
        const result = photoService.updatePhoto(photo.id, permission.filter(req.body));
        return res.json(result);
    }

    return res.status(403).end();
});

@scandinave
Copy link

Ok thanks for you time :)

@tomislav12
Copy link

tomislav12 commented Feb 21, 2019

Is there an option to set-up accesscontrol that some group of users, lets say Moderators,
can only create resource for everybody else, but not for themselves? Currently I did it like this:
{ role: 'moderator', resource: 'thing', action: 'create:any', attributes: 'createAnyButNotOwn' },
and then when checking for own:

let permission = null;
if (owned) {
 permission = ac.can( user.role ... etc );
 if (permission.attributes.indexOf('createAnyButNotOwn') > -1) {
            permission = false;
          }
}

Is there a better way? Thanks

@ansarizafar
Copy link

Why don't we add another chain function to check possession like this

ac.can(role).updateOwn('photo').possession(() => req.user.name === photo.username)

@nicks78
Copy link

nicks78 commented Mar 30, 2020

Hi,
I am trying to find the best way to prevent a user to not update/read/create a data that is not the owner.
My problem is that I have a merchant with many user and many store.
When I want to perform an action (ex: update a store), I need to make sure that the user is belongsTo the merchant and the merchant is the owner of the store.

So if I use "updateOwn", I don't know where the check the permissions.

Thanks for your help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question A question rather than an issue.
Projects
None yet
Development

No branches or pull requests

7 participants