Error: Permission denied for sequence

[seancolsen]

In this user testing session, starting at 46:13, we were trying to insert a row into a table and we hit an error that we didn't expect.

InsufficientPrivilege: permission denied for sequence...

This was after we had granted a role all privileges on a table which the role didn't own. To me it smelled of a problem with privileges on sequences.

Here's my hypothesis: Mathesar may not be granting the necessary privileges on PK sequences for users to insert rows. I wasn't able to fully validate this hypothesis though.

I've spent the past 30 minutes educating myself about ownership and granted privileges work for sequences, then then trying to cleanly replicate this same error. But I've been running into what feels like a mess of other behaviors within Mathesar I didn't expect. Unfortunately at this point I need to cut my losses and come back to this another time so that I can move on with higher-priority work. But suffice to say: I think it's rather important to spend some focused time hammering at this situation to pull out specific bugs/improvements.

[mathemancer]

Neat, and a bit subtle. This is due to a few things coming together:

• The Library demo dataset uses a deprecated SERIAL integer-sequence instead of an Identity sequence for its id column.
• PostgreSQL doesn't automatically grant USAGE on a sequence just because you granted INSERT on a table.

We could solve this in the Library Management example case simply by updating to use a generated identity value rather than an old-style sequence. But solving it more broadly is more difficult.

It's correct that granting INSERT on a table doesn't automatically grant permissions to use the associated sequence. This is because (with old-style sequences) you burn sequence values whenever you use them. This sequence could, in turn, be called for other purposes, etc., and it may be that you want to allow a user to INSERT into a table (with a hand-chosen id value in our example), but disallow them from burning sequence values.

PostgreSQL, for their part, strongly recommend using GENERATED BY DEFAULT AS IDENTITY, since the behavior of that style of auto-generated id column has behavior that aligns better with typical user expectations.

Note that Newly-created tables in Mathesar won't have this problem, since they use the recommended identity generator style.

[mathemancer]

After thinking about it a bit, I suggest we:

1. update our demo data sets to use the modern sequence version.
2. Provide specific guidance for this situation if it occurs on preexisting DBs.

I don't really think we should introduce the concept of "sequence permissions". If we update our data sets, this will only happen if they connect to preexisting DBs. The recommendation from the PG docs has been to use the new identity sequence style since PostgreSQL 10. The only reason we had these legacy sequences in our data sets is because they were constructed back when we were still using SQLAlchemy to create and modify tables, and that tool defaults to the legacy sequence style. So far, we haven't seen any feedback indicating that our users are wiring up to preexisting DBs with these legacy sequences, and if they did, I'd expect them to already have some ability to grant permissions on the DB using another client. Perhaps we could offer some guidance in our docs about that.

[seancolsen]

@mathemancer

Provide specific guidance

• What kind of guidance would you want to provide? Do we instruct users on how to modernize their sequences? Do we instruct them on how to change their privileges to account for legacy sequences? Both? Something else?
• Where would the guidance be shown? In the app? In the docs guide?
• And how would the guidance be be discoverable (especially in the event that someone bumps into this problem through the steps that Zack and I took)?

[mathemancer]

@mathemancer

Provide specific guidance

• What kind of guidance would you want to provide? Do we instruct users on how to modernize their sequences? Do we instruct them on how to change their privileges to account for legacy sequences? Both? Something else?

I think both. We need to make sure they understand what happened, and give them options. The reason is that this is a really specific setup and we can't necessarily guarantee which solution would work for their use case. They may, in some edge cases, want to allow some users to insert into a table as long as they provide their own ID values, while others can pull from the sequence. This would mean they need to have guidance about what privileges are needed for the sequence (honestly, I would imagine a user who has this need would already know about those privileges). For the more common case, we would want to provide guidance about updating their tables to use the newer sequence style.

• Where would the guidance be shown? In the app? In the docs guide?

Perhaps a link from the error message to the docs? We could also just have this in the docs and allow users to find it via searching or google. I think this is likely to be an exceedingly rare problem.

• And how would the guidance be be discoverable (especially in the event that someone bumps into this problem through the steps that Zack and I took)?

They won't bump into it through the steps Zack and you took after the merge of #4075. See above for ideas about how they'd get help if they do bump into it.

Overall, I think "Mathesar doesn't have great support for managing permissions on a sequence style which has been deprecated for 7 years" is a reasonable compromise for the moment. Eventually, of course, we should fully support working with all privileges on all objects in PostgreSQL, but I'd put this legacy sequence style below a number of other things in priority.

Edit: This would be a case where it might make sense to encourage error reports from the app as described in the analytics writeup on HackMD. I'd be interested in whether this actually causes a problem for a single user (now that our demo data sets are fixed)

[seancolsen]

Ok this gives me a clearer picture of what you're after. Thanks!

I think this is likely to be an exceedingly rare problem.

I hope you're correct with this prediction.

My initial impression was that this problem might not necessarily be rare. It was mostly a hunch, but informed by the following anecdotes and logic:

• I've been writing PostgreSQL CREATE TABLE statements for over three years now (in my role on this team), and I'm only just now learning about GENERATED BY DEFAULT AS IDENTITY. Coming from MySQL, I remember wrestling a bit a first when learning to create a primary key column because it's different in PostgreSQL. I recall finding some resource that steered me towards SERIAL. I don't know what it was. But since then, nothing has ever steered me away from using SERIAL. I've been under the impression that SERIAL was idiomatic this whole time. TIL! Thanks!

• These days I write PostgreSQL CREATE TABLE statements somewhat regularly in order try out different scenarios in Mathesar. I usually lean on LLMs to speed up the process (either ChatGPT or Gemini). And I don't think I've ever seen an LLM recommend using IDENTITY. For example:

  

• I would hope that it's relatively common for Mathesar users to have data that began 10 or more years ago — especially if we're aiming for users with pre-existing PostgreSQL data sets. Perhaps I'm wrong, but I imagine that if they began with SERIAL, they'd still have SERIAL. Right?

• I do wonder how other ORMs establish primary keys, e.g. Active Record, Prisma, Django ORM. If they switched to IDENTITY, I wonder when. The fact that SQLAlchemy still uses SERIAL contributes to my hunch that there could be ORMs which didn't make the switch very long ago. For example, pretend that Active Record only just began using IDENTITY two years ago. (I don't know whether this is true, but I'd say it's plausible, given what you say about SQLAlchemy.) In that case I'd expect a substantial portion of Rails apps to have tables using SERIAL.

Anyway... if, after reading my thoughts above, you still feel confident that this problem will, in fact, be exceedingly rare, then I'm keen to trust you on that!

Perhaps a link from the error message to the docs?

This is a cool idea. I can see some technical challenges in implementing it though.

• In many places across the front end, we have code that assumes error messages are strings, not HTML. Changing that assumption in a secure manner would require some very careful thought and refactoring. As a compromise we could include the URL in plain text and leave it to the user to copy-paste.
• We'd need some logic somewhere (probably in the DB layer) which identifies this scenario and adjusts the error message accordingly. I wonder exactly what logic we'd employ.

If we take up this implementation, it would be interesting to talk through these details on a call to hash out an approach.

But: instead of trying to recover gracefully from a failed INSERT, my intuition would be to try fixing this problem during the GRANT step. To me that seems like a more opportune time to fix the problem because we'd be targeting the user with the proper privileges. It wouldn't help in scenarios where GRANT has happened outside Mathesar. But I'm less worried about that.

Anyway, I'm probably getting too far into the weeds now, so I'll cut myself off. If this is rare, then fine, let's sleep on this. But if not, I'd be keen to bounce some more ideas around for how to approach it.

[zackkrida]

A UI observation worth mentioning, regarding errors:

It should be visible in the user testing session video, but during the session I was unable to select the error text as hovering my mouse over the error pop-up/tooltip resulted in the pop-up/tooltip disappearing.

[mathemancer]

You raise some good points, @seancolsen . Thanks for spending the time and putting in so much apparent legwork.

It seems Django has been using GENERATED ... AS IDENTITY for 2.5 years. Not really long enough for total comfort in a vacuum, but my contention isn't that DBs including SERIAL columns are super rare, it's that the combination:

1. their DB includes SERIAL columns (this means it's preexisting for a while, and tables with auto-generated columns have been set up through some means,
2. they now want to use Mathesar to create roles and manage privileges of those roles on DB objects, and
3. they're going to be confused and unable to proceed if a role has INSERT on the table and not UPDATE on a sequence owned by that table

is rare.

The way they'd end up at point (3) is if they've had these sequences, but haven't run into the problem before. I.e., they haven't actually granted INSERT on a table with an owned sequence to a less-privileged user before, and dealt with the ensuing confusion. That's the part I think will be most rare. I.e., they were previously using the DB in a way that avoided the problem, but after installing Mathesar they'll be introducing this problem by creating and using roles in a way they otherwise wouldn't. It's possible I'm wrong, e.g., if they find creating roles easier and therefore more enticing after installing Mathesar, but I'd like to wait till we hear complaints to upgrade the priority.

Regarding Django, Rails, SQLAlchemy, etc. Many (maybe most) of the ORM or otherwise multi-dialect SQL-speaking tools tend to run your whole app as a single user. In that case, permissions aren't an issue. For example, if a user wires Mathesar up to a Django DB for some kind of lower-level DB admin interface, it seems most likely to me they're going to just log in with the same user that Django uses to connect to the DB. Same for the other tools. It's just my intuition, though, I'm not exactly 100% confident in that assessment.

Finally, to reiterate, I do think we should completely solve this sometime (say before the next year is out), since I want to reach something like "feature parity" with PostgreSQL itself w.r.t. privilege wrangling. I just think I have a different assessment of the priority level compared to other concerns (even other privilege-related concerns).

[seancolsen]

Ok right on. Makes sense. Thanks @mathemancer! I'm always happy when we identify things to deprioritize!