The Wayback Machine - https://web.archive.org/web/20201029114823/https://github.com/directus/api/issues/511
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

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

Comments

@benhaynes
Copy link
Member

@benhaynes benhaynes commented Oct 25, 2018

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
Copy link
Member Author

@benhaynes 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
Member

@WoLfulus WoLfulus commented Oct 31, 2018

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

@benhaynes benhaynes commented Oct 31, 2018

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
Member

@WoLfulus WoLfulus commented Oct 31, 2018

Awesome!

@stale stale bot added the stale label Dec 30, 2018
@benhaynes benhaynes removed the stale label Dec 30, 2018
@benhaynes benhaynes added this to the Priority Features milestone Jan 30, 2019
@benhaynes
Copy link
Member Author

@benhaynes benhaynes commented Jan 30, 2019

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

@alvarocabanas
Copy link

@alvarocabanas alvarocabanas commented Feb 4, 2019

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

@benhaynes benhaynes commented May 20, 2019

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

@Vadorequest Vadorequest commented Apr 7, 2020

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

@benhaynes benhaynes commented Apr 8, 2020

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

@Vadorequest Vadorequest commented Apr 8, 2020

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

@benhaynes benhaynes commented Jun 21, 2020

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
Member

@WoLfulus WoLfulus commented Jun 22, 2020

@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

@benhaynes benhaynes commented Jun 22, 2020

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 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

@benhaynes benhaynes commented Jul 3, 2020

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 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

@benhaynes benhaynes commented Jul 3, 2020

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 join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.