Skip to content

Conversation

@timb07
Copy link
Contributor

@timb07 timb07 commented Jun 30, 2025

Prior to this change, the introspections that looked to see whether the current user owned a particular table only considered direct ownership. But in the case of a table owned by a role, any user that has that role also effectively owns the table, and has permission to operate on the table as if they were the direct owner.

This change updates the get_referring_fks and is_table_owner introspection functions to consider both direct ownership and also indirect ownership via role membership.

Prior to this change, the introspections that looked to see whether the
current user owned a particular table only considered direct ownership.
But in the case of a table owned by a role, any user that has that role
also effectively owns the table, and has permission to operate on the table
as if they were the direct owner.

This change updates the get_referring_fks and is_table_owner introspection
functions to consider both direct ownership and also indirect ownership
via role membership.
@github-actions
Copy link

Coverage Report Results

Name Stmts Miss Branch BrPart Cover
src/psycopack/__init__.py 6 0 0 0 100%
src/psycopack/_commands.py 111 0 8 0 100%
src/psycopack/_conn.py 5 0 0 0 100%
src/psycopack/_const.py 3 0 0 0 100%
src/psycopack/_cur.py 24 0 2 0 100%
src/psycopack/_identifiers.py 12 0 2 0 100%
src/psycopack/_introspect.py 160 0 16 0 100%
src/psycopack/_logging.py 2 0 0 0 100%
src/psycopack/_psycopg.py 5 0 0 0 100%
src/psycopack/_registry.py 58 0 6 0 100%
src/psycopack/_repack.py 355 0 120 0 100%
src/psycopack/_tracker.py 165 0 36 0 100%
tests/conftest.py 19 0 0 0 100%
tests/factories.py 40 0 8 0 100%
tests/test_cur.py 20 0 2 0 100%
tests/test_fixtures.py 5 0 0 0 100%
tests/test_package.py 3 0 0 0 100%
tests/test_repack.py 708 0 0 0 100%
TOTAL 1701 0 200 0 100%

1 empty file skipped.

Copy link
Collaborator

@marcelofern marcelofern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice finding (-:
This deserves some tests though

Copy link

@s-cooper18 s-cooper18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with tests being useful for this

Copy link

@s-cooper18 s-cooper18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested changes locally - adding more thorough test ideal in long term, but for short term this is okay.

@timb07 timb07 merged commit db7a419 into main Jul 1, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants