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

How to handle rules with related models ? #31

Open
mdartic opened this issue May 24, 2021 · 7 comments
Open

How to handle rules with related models ? #31

mdartic opened this issue May 24, 2021 · 7 comments
Labels
discussion enhancement New feature or request

Comments

@mdartic
Copy link

mdartic commented May 24, 2021

Hello,

First, thanks for your effort to create this library, great to discover :-)

I'm asking myself how to do a use case by checking permissions against a related model.

I'm using feathers-objection, that allow use of joinRelated (by using $joinRelation keyword), and let me filter on properties of these relations.

This helps me a lot to check if a user have access to something, for example a Groups (table group) and with which role, eg ADMIN, MEMBER (in a relation based on a table usergroup between Group and Users).

I can do something like that with a filter applied on Group like this one :

{
  "query": {
    "$joinRelation": "users",
    "users.id": "${user.id}"
  }
}

This makes me knowing which groups the user have access, with which role,
according to these data, I can create rules to allow users see groups, or check that they can update groups.

What's the way of doing this with feathers-casl ? Could we handle it ?

Actually, I add a rule with condition { id: { $in: [] } } where ids are retrieved from the previous filter.

Same for limiting power depending on your role.

I'm not happy with this solution, because I think I make too much request on database, am I wrong ?

Maybe that's not the right use case, but I was thinking it could be great to allow keywords of DB adapters ?

What do you think about it ?

@fratzinger fratzinger added discussion enhancement New feature or request labels May 26, 2021
@fratzinger
Copy link
Owner

Hey, thanks for your issue and your thoughts on this. Relations is something I have in mind for some time now, but am not really far with my thoughts. It would be a really good addition I would love to see for feathers-casl!
Disclaimer: I use feathers-sequelize on my own projects, so I have not much knowledge about objection.

There are some questions, that come up:

  1. Is it about querying or about population? I would like to distinguish between those consequently.
  2. Would you like to have the $joinRelation in a casl rule or in query from client?

Since feathers-casl uses casl under the hood, the first question to answer would be, what casl can handle and what needs to be handled separately by feathers-casl? Quering for users.id should work with casl right away (as seen in https://casl.js.org/v5/en/guide/conditions-in-depth#mongo-db-and-its-query-language). The $joinRelation part is more problematic I guess. Maybe we need an option for 'blacklist'/'whitelist' operators, so $joinRelation will be passed to the service method but not to the check with casl.

Do you know this: https://gist.github.com/stalniy/b710d65ac8a6c15f37a435c910624ef7 ? Do you think this could help somehow?

@mdartic
Copy link
Author

mdartic commented May 28, 2021

Thanks for your reply,

  1. it's about querying, I want to check acls by filtering model's relations
  2. that's an idea, but I don't know if it's smart enough

I didn't knew the gist, that's interesting, but I didn't really read your codebase, so I don't know if it could fit ?

Or maybe have you some ideas ?

@robbyphillips
Copy link

I think the only way to do this is to fetch all the relevant data before building the ability.

I'm currently handling this with service level hooks that make use of

authorize({
  ability: async context => {
    const extraData =  await context.service('extra-data-').find() // whatever relation
    return createAbility(context.params.user, extraData)
  }
})

Currently, it's pretty simple, so we can tolerate the extra service call overhead, but I'm also looking into caching abilities per-service to help mitigate db trips.

@mdartic
Copy link
Author

mdartic commented Jul 22, 2021

@robbyphillips do you use also feathers-objection ?

I'm asking myself if I need to add feathers-casl rules "only" if I see a $join in the request.

But this would say that for each relation ($eager) configured in feathers-objection, and allowed, I would need to add some casl rule to protect these relations from retrieving unauthorized data ?

What do you think about it ? I find this a little hard to maintain.

@robbyphillips
Copy link

Yes, I'm also using feathers-objection.

As far as I know, the only way to limit fields on joined relations is to use a modifier. You can make these modifiers mandatory by using the eagerFilters service option.

For example, if we have "Comments" and "Authors" relationship setup, we could add a modifier to the User Model:

// user.model.ts

export class UserModel extends Model {
  // ...
  public static modifiers = {
    minimumSelect(query: QueryBuilder<UserMode>) {
      // select only the "public" fields of our user
      query.select('id', 'username')
  }
}

And enforce that modifier in the Comments service:

// comments.service.ts

export default function(app: Application) {
  const options: Partial<ObjectionServiceOptions> {
    // ... other service options
    allowedEager: 'author',
    eagerFilters: [
      {
        expression: 'author',
        filter: UserModel.modifiers.minimumSelect
      }
    ]
  }

  // ... other service setup
}

The biggest downside of this is that there's no way to conditionally apply these modifiers based on the outcome an ability check. In practice, I've not found this too hard to work with since most entities that I'm joining this way have pretty obvious "public" versions that any should be visible to any user who is allowed to see the parent entity.

A more "feathers way" of handling this would be to ignore $eager altogether and do all your joins at the service level so that each join is effectively just a service call that can use hooks like authorize. There's work being done over at batch-loader to make the general case more performant, and feathers-graph-populate is whole service-level join solution (that I haven't had a chance to fully check out yet).

@fratzinger
Copy link
Owner

I fully use feathers-graph-populate with authorize for sequelize. For population it's just awesome but it's not made for querying by relations. That's why I asked if this issue is about querying.

Do you know this? https://daddywarbucks.github.io/feathers-fletching/hooks.html#joinquery
It should help, but it's another definition of your models besides objection and shallow-populate. A wrapper/converter from dot.notation to objection-notation would be nice. I would also like to work on a wrapper/converter for sequelize, similar to https://daddywarbucks.github.io/feathers-fletching/hooks.html#sequelizejoinquery

I hope that @feathersjs/schema will become a replacement for feathers-graph-populate, feathers-fletching/joinQuery.

@fratzinger
Copy link
Owner

I've started a working example at: https://github.com/fratzinger/feathers-casl/tree/relations
It's using https://daddywarbucks.github.io/feathers-fletching/hooks.html#joinquery . It should fully support find and get but eventually will fail for the other methods.

Any thoughts on this are highly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants