JaneJeon / objection-authorize

isomorphic, "magical" authorization integration with Objection.js 🎉
https://janejeon.dev/objection-authorize-v4-and-the-future/
GNU Lesser General Public License v3.0
76 stars 6 forks source link

Relation support is undocumented or missing #154

Closed davisml closed 3 years ago

davisml commented 3 years ago

According to the readme the library should support applying ACL to related models (experimentally? is there an option?).

Looking at the authorizeRead function on the Model extension (master branch) – it appears to simply be applying rules to the Model instance itself and not traversing relationships.

PS. Thank you for creating this project – let me know if you need another contributor!

davisml commented 3 years ago

Here's the behaviour I'm looking for:

I have two models Book & Library

Library has "books" relationship with N Book instances Book has "library" relationship with 1 Library instance

Calling library.authorizeRead() would traverse the books relationship applying authorizeRead() to each Book instance.

Similarly, calling book.authorizeRead() would traverse the library relationship applying authorizeRead() to it

JaneJeon commented 3 years ago

https://github.com/JaneJeon/objection-authorize#relation-support

JaneJeon commented 3 years ago

Also it sounds like you’re fundamentally misunderstanding Objection’s model class vs. instance difference. I suggest you read up on Objection’s relation queries carefully again - I am very specific in my documentation where you just add a method in the .query() chain vs. when you need to call it on a specific instance.

Finally, you’re trying to do 1-N “filtering”: for that I suggest you read up on the QueryBuilder.fetchResourceContextFromDB() section. It is the most relevant for what you’re trying to do (it actually supports a full N x M but most people will use it for 1 x N matrix filters anyway)

davisml commented 3 years ago

Let me add more detail here – I'm not looking to apply changes to the QueryBuilder but simply wondering if the library supports a "deep" authorizeRead() on an instance of Model.

For example:

Library model has a "privateId" property

In CASL abilities we deny anonymous users from seeing the privateId for a Library

forbid('read', 'Library', ['privateId'])

As an anonymous user I fetch a Book with it's library relationship and apply authorizeRead()

const data = await Book.query().authorize(ctx.user).findById(1).withGraphJoined('library').authorizeRead(ctx.user)

The library object in the response object does not have the CASL rules for the Library read action applied and exposes the privateId attribute.

Is it possible to apply the rules to nested relationships on a Model?

JaneJeon commented 3 years ago

The library does support “deep filtering” (in fact I went to great lengths to ensure it can be as ‘deep” as you want) but it all depends on the structure of your model instance/query result. Furthermore, please note that relation support is experimental - so if a library does not support a relation feature, you can either make a PR to cover your use case, or simply fall back to checking ACL on an individual model/model instance(s) level, as clunky as it may be.

In your above query (minus .authorizeRead(), please read the documentation again before asking), you’re doing a read query (and if you want to modify the “name” of the query to identify it with, you can simply append .action(‘fetchrelatedlibrary’) or something so the library checks whether the user can read the book w/ ID 1 (that is the “main” object that is being fetched here).

(Note: it is possible to check if the user is allowed to “read” the library associated with the book as well, but that doesn’t seem to be what you want to do).

Then, the result of that query is something akin to bookInstance with an additional “field” called “library” which is in fact a reference to its related parent library.

Now, think - all of the information needed to check “can I read the library’s privateId” is already there. All you need to do is to configure it so (that’s the beauty of this library).

Now, I’m not as familiar with the graph queries in objection (as I prefer to join tables manually myself using relatedQuery/$relatedQuery), but even then I can still see that it is possible.

First, I’m not sure if the .library property is exposed as a plain javascript object or an instance of the Library model, but if it’s not, you can just simply wrap the book.library in a Library model (check .fromJson() method in Objection.js documentation). So from now on we’ll assume that data.library is in fact, in the form of a Library model instance.

Next, you want to selectively filter out the privateId key from the Library instance depending on the user during serialization. Now, this may be one area where the library lacks support for relations (since the relation support was added based on my personal usage and how I think people would use relation queries with objection-authorize) - and please feel free to add a PR once you’re done here to add support for this - but .authorizeRead() is meant for serializing a model instance (or two or three), and not its related model instances, too.

What this means for us is that when you do data.authorizeRead(user), because the data is of instance Book (model classes run EVERYTHING in objection-authorize land, so please be mindful of it at all times), the result will be a filtered book instance, as in, the .authorizeRead() call will only filter based on the user’s Book reading privileges (i.e. stuff like forbid(‘read’, ‘Book’, [‘superDuperPrivateField’])).

But you want to also check the .library’s read access too, correct? In that case, you’d have to check the read privileges “manually” - by running something like filteredLibraryJSON = data.library.authorizeRead(user). Then, you can simply overwrite the value of data.library with the filteredLibraryJSON. One more additional step, but still doesn’t mean you can’t do it!

So this answers your question, but again, I highly suggest that you:

  1. Really try to understand the model class/instance-driven approach of this library and carefully peruse the documentation of this library with that in mind. Thoroughly. This library handles great amount of complexities and while it makes it easy to handle and abstract it, it doesn’t mean you can’t just be unaware of said complexities entirely (e.g. how it figures out what/how to check and what the query is about, auto-wrapping model instances and how it knows what the object being passed is, how it knows what to filter and how, etc), just like everything else in SQL-land xD. (I’m not saying this just because I’m rather wary of doing “tech support” for people not reading the documentation before asking questions that are easily addressed by the documentation, but it IS a part, so please allow me to help you better and more efficiently by understanding the core model more comprehensively - I only have so much brainpower every day to write 3000 word replies like this)
  2. Dig deeper into Objection.js’ source code and documentation to fully understand what the return types are (i.e. when things are returned as a model instance - such as the Book - and when things might be returned as a plain object - possibly in the case of book.library in your quer, for example). The Objection.js author is VERY much not clear on it, and often times you just have to run the code to see what it actually returns 🤷‍♀️
  3. Once you’re done, please feel free to add further support for relation queries. As I’ve said, it’s currently experimentational (but again note that that doesn’t mean there’s nothing it CAN’T do; it just doesn’t pick it up “automatically” such as in the case of book.library) and I’d greatly appreciate any help in not only covering more and more use cases, but also adding/displaying said use cases in the documentation and guiding people like you who may have questions about their use cases. Seriously, don’t be afraid to make a PR.
JaneJeon commented 3 years ago

That being said, I’m tapped out for today from writing the above monstrocity... so please excuse me if you have any further questions and I don’t reply in a timely manner for the rest of today. zzZ

JaneJeon commented 3 years ago

@davisml hey I wanted to follow up with you, did my above reply manage to resolve your problem?