Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

Permissions: Dynamic Rules #511

Closed
benhaynes opened this issue Oct 25, 2018 · 17 comments
Closed

Permissions: Dynamic Rules #511

benhaynes opened this issue Oct 25, 2018 · 17 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@benhaynes
Copy link
Member

While full, role, user, mine, and none will satisfy most use-cases, I've spoken with many people who could use a more dynamic option. I imagine using a set of filters like we do in directus_collection_presets.filters... so there would be an option for a CRUDE permission to be set to:

This is how filters are saved in directus_collection_presets:

[
    {
        "field":"email",
        "operator":"contains",
        "value":"gmail"
    }, 
    {
        "field":"added_on",
        "operator":"contains",
        "value":"2018"
    }
]

Originally I was thinking that we could save the permissions as filter:[...] (where the array matches above), but I wonder if there's a cleaner way to handle this.

@benhaynes benhaynes added enhancement New feature or request question Further information is requested labels Oct 25, 2018
@benhaynes
Copy link
Member Author

benhaynes commented Oct 30, 2018

I just had a thought. If Read/Update/Delete are all controlled by filters (eg: a WHERE clause) then that also allows for the current mine and role options. So we could potentially simplify the whole way this is stored/set. Let me ignore the "breaking change" for a bit and illustrate one option:

directus_permissions.update could have the following values:

  • * — Full permissions (same as full) This could also use an empty array, but that might be confusing vs NULL
  • NULL — No permission (same as none)
  • [array of filters] — Filtered permission (same as mine)

Example of filter array

[
    {
        "field":"created_by",
        "operator":"eq",
        "value":"$CURRENT_USER" // Or something like that
    }
]

We would need a few variables, but these would be really useful to have in normal filters as well. Examples include:

  • current user
  • now()
  • current_role

@WoLfulus
Copy link
Contributor

This might be already answered, but just to be clear: this filter will be added/forced by directus api itself right? My main concern is just the public-facing api endpoints, in which someone can forge/modify the request to remove the filter.

@benhaynes
Copy link
Member Author

Correct. This will be the Core ACL for the API (and therefore also used by App). The only way to bypass this would be if you're connecting directly to the database (obviously).

@WoLfulus
Copy link
Contributor

Awesome!

@benhaynes
Copy link
Member Author

@wellingguzman — could I get your thoughts on this one?

@alvarocabanas
Copy link

This filtered Dynamic Permissions would be a great help also for our use case, because we could make composition of permissions for our collections by user.

@benhaynes
Copy link
Member Author

To achieve better clarity/visibility, we are now tracking feature requests within the Feature Request project board.

This issue being closed does not mean it's not being considered.

@Vadorequest
Copy link

This looks a bit like https://github.com/UnlyEd/conditions-matcher we built. It's simpler though.

Exemples: https://github.com/UnlyEd/conditions-matcher/blob/master/examples/medium-match/index.js

What's the status on this? Any ETA? I could also use it to more finely control permissions of my customer's users (being a SaaS B2B business managing big accounts with multiple people with different sets of permissions)

@benhaynes
Copy link
Member Author

The status is "in work". We're discussing how best to implement this, and when. While we'd like to include it in v10 (the API refactor in Laravel), we're also trying to strictly adhere to our feature freeze so that we focus on stability and timeliness first.

@Vadorequest
Copy link

Okay, I've seen many questions related to this on Slack this past week.

@directus directus deleted a comment from stale bot Apr 8, 2020
@benhaynes
Copy link
Member Author

Schema

Directus Permissions

  • id — UUID or auto-increment ID?
  • role — FK
  • collection
    • Store collection/table name, or FK?
  • operation — varchar(10)
    • create
    • read
    • update
    • validate
    • delete
    • comment
    • explain
  • permissions — JSON
  • limit — Only used on Read
    • Could be per request (including RUD)
  • presets — Only used by Create and Update
    • Static values
    • Dynamic values — NOW, CURRENT_USER, etc
  • fields — Only used by Create, Read and Update
    • Whitelist CSV of fields
    • Wildcard
  • conditional_fields (9.x)
    • JSON, save conditional permissions for each column/field READ
  • conditional_presets (9.x)
    • JSON, save conditional permissions for each column/field READ

Directus Roles

  • allow_app_access — toggles ability to login to app

Permissions Example JSON

{
  "_or": [
    {
      "_and": [
        {
          "owner": {
            "_eq": "CURRENT_USER"
          }
        },
        {
          "status": {
            "_in": ["published","draft"]
          }
        }
      ]
    },
    {
      "_and": [
        {
          "owner": {
            "_ne": "CURRENT_USER"
          }
        },
        {
          "status": {
            "_in": ["published"]
          }
        }
      ]
    }
  ]
}

@WoLfulus
Copy link
Contributor

@benhaynes

My 2 cents:

  • FK in collections
  • Fields should also be an FK to directus_fields
  • Unique between (collection_id and field_id) - one entry per field

I'd also split the permission JSON and operations into another table and have a FK point to that, should be easier to perform operations over the table to fetch specific contents like checking for (posts, title).

Also, "_ne": "CURRENT_USER" won't work, since there's no way to distinguish between a string value and a constant. {"_ne": {"type": "variable", "value": "CURRENT_USER"} and {"type": "constant", "value": "some_value"} is preferred.

About allow_app_access, there's no "bound" between app and api, so how we'd know it's getting accessed by the app? It's still the same data and API requests. Should that work by preventing all access to system collections?

@benhaynes
Copy link
Member Author

Thanks @WoLfulus! Some thoughts below... although I'm sure @rijkvanzanten will weigh in too.

Collection FKs

My main concern with using abstracted FKs in the system collections is that we strive to be a "database admin tool", but this really obfuscates the data. Looking at directus_permissions and seeing 7f373d06-b4c4-11ea-b3de-0242ac130004 doesn't help... and DBAs would have to do complex lookups, etc to administer the data directly. We've had this issue elsewhere, and it can be really annoying when you just want to work in the DB directly.

I know the main reason to use FKs is to make schema updates easier, but if we have a short list of places/columns that need to be updated, I don't think this should be that big of an issue. The same applies to column FKs vs Names.

Permissions Collections

I don't really understand the reasoning behind splitting permissions into two tables, perhaps you explain the benefits of this a bit more? What I've proposed is basically 5 columns (ignoring a few new ones like limit) to store permissions in one place. Again, unless there's a big performance gain, I'd like to maintain readability of the database records.

Are you saying you want to use columns instead of the JSON? That seems like a huge change, since these permissions could be significantly nested/complex. JSON can certainly handle that, but I imagine splitting all that into individual records would be far more complex. Enlighten me.

Dynamic vs Static Values

I agree that typing is an issue here, but it seems that we could simply come up with a format for reserved variables to solve this instead of introducing typing into the permissions. If ALL dynamic variables start with DIRECTUS_VAR_* (or something like that) we should be fine, if it's properly documented. I'll let Rijk give his thoughts on this, since I'm no expert.

App Access

This flag would not matter for the API. If a user tries to log in to the App, and they don't have app_access enabled, then the App simply wouldn't let them authenticate. They can still use the API like normal, but wouldn't be a "CMS User", as it were.

@tvld
Copy link

tvld commented Jul 3, 2020

Weighing my two cents in here, I am wondering if it is sufficient for most use cases to introduce User groups instead of too granular permissions? I remember we once went that road with a ModX, a cms, and there it became so bad that some installations had such mixed up dynamic permissions that we completely lost track at times... The principle is easy, the maintenance over time not.

With user groups a user group owner can invite users in a group and in that group give them a role (owner, editor,..). Then, each item should have a user_groups field that lists one or more user_groups that has access to that item.

@benhaynes
Copy link
Member Author

Thanks @tvld — we actually had that (or at least the architecture for it) back in v7. Roles were a M2M with Users. While that remains an option in the future, we're first focusing on roles themselves having more control (creating conditions, rather than simply configuring a status filter). This doesn't limit us from having multiple roles per user int he future... just makes Roles themselves more powerful.

@tvld
Copy link

tvld commented Jul 3, 2020

@benhaynes Point taken... and I can see your reasoning...

I suppose this discussion is very related to what we discuss here? Or was I hijacking by accident? >> directus/directus#2687

@benhaynes
Copy link
Member Author

Yeah, very similar... basically our goal is to improve the fundamental flexibility and power of Directus, while leaving as many doors open as possible for enhancements in the future that we don't have time to coordinate now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants