sql: enforce privileges on the current descriptor for historical reads

[ajwerner]

Describe the problem

Cockroach stores its user privileges with string user names. This means that a privilege on an old table may refer to a different user entity than the current name (if you say delete the user and recreate a user with the same name). Furthermore, it's probably not great that if you revoke access to a table then the user can still access it historically.

There is quite a bit of discussion on this topic in this thread here:
https://groups.google.com/a/cockroachlabs.com/g/sql-schema-team/c/L4oUTiceGY8/m/srPRdGgCAgAJ

Expected behavior

Ideally we'd not allow access to a table unless the user currently has privileges on the table. For deleted tables this probably means that we'd require that the user had permissions when the table was deleted.

Proposed Solution

In the relatively short term we should address this. One complication is being able to find the appropriate privileges for tables which have been deleted. It seems better to me to only do the right thing for non-deleted tables than to do what we do today but obvious a complete solution would be better.

Jira issue: CRDB-3999

[ajwerner]

This shouldn't be too crazy to implement and we should try to get it into this release. My feeling is that this falls on the sql schema team. cc @thtruo

[ajwerner]

There has been some discussion of late in pushing permissions checking down into resolution. This would make solving this problem in the purvue of the descs.Collection easier. For now, I think a reasonable approach would be for the descs.Collection to recognize that a version of a table which has been acquired from the lease manager is historical, to request the latest version, and to copy the privileges off of that latest version.

[dt]

I wonder if we should update a table when it is truncated with a pointer to its replacement? right now we only have the links in the other direction?

[ajwerner]

I wonder if we should update a table when it is truncated with a pointer to its replacement? right now we only have the links in the other direction?

That's a good point. It begs the question about subsequent truncates. We could try to resolve the current privileges at the fully-qualified name but that leads to interesting questions about renames.

[ajwerner]

#2939 is the issue about making users not strings.

[ajwerner]

This one is legit and non-trivial

[rafiss]

CRDB-19134 is the new epic tracking the project to switch to use stable IDs for users.

[ajwerner]

The use of IDs makes this situation better, but it's not super tightly coupled.