Stop user without access permissions#30
Merged
Merged
Conversation
Coverage Report Results
1 empty file skipped. |
Prior to this change, Psycopack did not check for any schema privileges. It was assumed, that the user would have CREATE and USAGE privilege for the schema. - USAGE: for introspecting objects in the schema. - CREATE: for creating new objects in the schema. This change adds a check to validate whether the user has both of these privileges or not.
marcelofern
force-pushed
the
forbid-user-without-access
branch
from
55afd36 to
fb6acfd
Compare
marcelofern
changed the title
Prior to this change, there was no check to verify that the user was an OWNER of the table to be repacked. This is a problem because only the owner can rename and delete the table (needed once it has been swapped). This change adds an explicit check to ensure the user is the owner of the table before proceeding.
marcelofern
force-pushed
the
forbid-user-without-access
branch
from
fb6acfd to
d54dbf2
Compare
felcury
approved these changes
May 14, 2025
| is_validated: bool | ||
| referring_table: str | ||
| schema: str | ||
| is_owner: bool |
There was a problem hiding this comment.
Could I suggest calling these something along the lines of is_owned_by_user or something similar?
I appreciate it's a bit more verbose, but I think it's more consistent with the other properties of the FK (e.g. "the FK is validated, and the FK is owned by the user", instead of "the FK is validated, and the FK is owner(?)")
Collaborator
Author
| name: str | ||
| schema: str | ||
| privileges: list[str] | ||
| is_owner: bool |
Prior to this change, there was no check to verify that the user was an OWNER of the referring tables (tables that have fks pointing to the table to be repacked). This is a problem because Psycopack needs to create new foreign keys on that table that point to the shadow table, and that is not possible unless the user is the table owner. This change adds an explicit check to ensure the user is the owner of the referring tables before proceeding.
Prior to this change, there was no check to verify that the user had the REFERENCES privilege on the referred tables (tables the table to be repacked has a fk to). This is a problem because Psycopack needs to create new foreign keys on the shadow table that point to the referred tables, and that is not possible unless the user has REFERENCES privilege. This change adds an explicit check to ensure the user has this privilege for all referred tables before proceeding.
marcelofern
force-pushed
the
forbid-user-without-access
branch
from
0de4572 to
180dcd6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds some sanity checks to make sure the user has the permissions they
ought to have to Psycopack a table.