Feature request: insert/update permissions without delete

[jpotter]

Hi Zach,

We're building an app where we have some audit/compliance needs that require rows never be deleted -- only updated. It'd be great if pgbedrock had the ability to have more fine-grain permissions other than read/write. I did see the note in documentation about being open to a pull request with that functionality. Before starting in on that, do you have any particular constraints around a pull request that you'd accept?

I also noticed on a quick skim of the read/write code that there's a comment at https://github.com/Squarespace/pgbedrock/blob/master/pgbedrock/privileges.py#L62: "If a write privilege is desired then read access is as well" -- we actually have this case too, where we have a 3rd party system that needs permission to insert incoming data into our system but cannot have read access to that table (just insert, no select). Updating the permissions to be more fine-grain may require revisiting that?

Thanks,
Jeff

[zcmarine]

Hey Jeff,

Thanks for offering a new use case! This is a really useful generalization of what pgbedrock currently provides. I could see us supporting this, though I'm not 100% decided on what that API should look like. I'll propose what I'm thinking below, but I'd be curious to hear your thoughts as well.

CLI Changes
I'm thinking is that we add a --simplified/--non-simplified flag to pgbedrock generate, which will have it either create a spec like it currently does or create one that doesn't collapse privileges down to read vs. write. I'd like to keep the existing API (--simplified) the default since the YAML spec that is generated and interacted with by end users is simple to reason about and meets most use cases (at least to the best of my knowledge).

pgbedrock configure wouldn't necessarily need to change so long as it can identify what kind of spec it's working with: a simplified or non-simplified one.

YAML Format
The first idea that comes to mind regarding the YAML spec is probably one you've had already: just removing the read vs. write thing and using the privilege type itself. I think that could make sense here. Additionally, if there's a myschema.* we still know what default privileges we need to grant based on the privilege name of that subdict. So we'd end up with something like:

    privileges:
        tables:
            select:
                - marketing.ad_spend
                - marketing.impressions
            insert:
                - marketing.impressions
            delete:
                - marketing.ad_spend

Code Path
One thing that would be nice if we go this route is that we could have the above be an intermediate output that ultimately gets collapsed into read vs. write in the --simplified use case. This would be nice since the above doesn't make as many assumptions, and we can then reuse a code path. The privilege stuff tends to be difficult to get right since there are so many edge cases and ways things interact, so if we can use the same code in both the simplified and non-simplified cases then there's less chance of weird inconsistencies.

Two simplifications / assumptions would still be present here though, and I'm curious if you think they make sense:

• Default privileges would still be assumed, i.e. if all tables in myschema currently have SELECT, then myschema.* in the select subdict would assume that all future tables in that schema should get SELECT. We could try to remove that assumption, but it feels like that does bring value to the end user. What do you think?
• Similarly, if someone has some access to all personal schemas, we currently collapse that down to personal_schemas.*, which is nice in case a new personal schema is added later since pgbedrock will go ahead and grant the expected permissions. Again, we could try to remove that assumption, but it feels like it brings value to keep it.

I guess this comes down to what we want this output to represent: the exact state of things with no assumptions about the future, or a more fine-grained permission mode that still makes things easier for the database administrator in the future. My vote would be for the second, but I'm open to counter-arguments.

Again, I'd love to hear your opinion on the above, and if you are interested in collaborating on the above I'd love to sync up and help you how I can!

• Zach

[lsowen]

Hi @zcmarine,

I've been looking for a while for a system to declaratively manage users, roles, and permissions for my postgres servers. My use case might be slightly different from yours (for example, I don't expect to use the generate functionality), so take everything I say with a grain of salt. 😁

File Format Version Support

First, I think it would be useful to have a config file format version in the yaml, something similar to how docker-compose versions theirs:

version: "2"
role-a:
  ....

This would allow an evolving format, while still supporting older versions. Files not having the version field could be implicitly understood as being version. Newer releases (even ones using version 1) could be updated to check the version number and return nice error messages. Only older (ie already released) versions would "ugly error" if presented with a newer file format.

Permission "Groups"

For my use case, it would be very useful to be able to specify arbitrary permissions (eg select, insert, update, but not delete). To make it slightly more readable (but would break the file format, thus using the above version field), you could do something like "permission groups". For example:

version: "1.1"
permissions:
  MyPermissionGroup:
    - SELECT
    - UPDATE
    - INSERT
  ReadOnly:
     - SELECT
roles:
  jdoe:
      can_login: yes
      is_superuser: no
      privileges:
          tables:
               MyPermissionGroup:
                - marketing.impressions
                - marketing.users
              ReadOnly:
                 - marketing.ad_spend
...

Explicit Default Privileges

Once again, because I'm not using the generate functionality, I think it would be useful to be able to manage default permissions explicitly, instead of relying on special handling of myschema.*. Maybe something like

roles:
  jdoe:
      can_login: yes
      is_superuser: no
      default_privileges:
          tables:
               MyPermissionGroup:
                  - myschema
...

Explicit "default" personal_schemas permission

Lastly, I also think it would be useful to be able to explicitly control whether a role does or does not have access to all the personal_schemas, and what that default access looks like. This would make it slightly "less magical", and also support the corner case where someone has an actual schema called personal_schemas (low chance, but still...)

Let me know what you think, and if I can provide any additional information. I'd love to collaborate, because like I said, this is very close to what I've been looking for.

Thanks!
Logan

[zcmarine]

Hey @lsowen, sorry for the month-long delay in responding; I was traveling for the last couple months. Those changes make a lot of sense to me and sound valuable, though they'd also entail a large amount of work and are a big enough departure from the existing functionality that it would probably be easier to simply fork the repo and build what you want out of it. To the degree that I can offer any useful context please let me know!