Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: Remove inheritance of role attributes #18779

Merged
merged 2 commits into from
Apr 17, 2023

Conversation

jkosh44
Copy link
Contributor

@jkosh44 jkosh44 commented Apr 15, 2023

Previously, roles inherited role attributes of all their member roles. This was incorrect and not has PostgreSQL works. This commit removes this inheritance.

Part of MaterializeInc/database-issues#3380

Motivation

This PR adds a known-desirable feature.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • This PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way) and therefore is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:

Previously, roles inherited role attributes of all their member roles.
This was incorrect and not has PostgreSQL works. This commit removes
this inheritance.

Part of #11579
@jkosh44 jkosh44 marked this pull request as ready for review April 15, 2023 18:49
@jkosh44 jkosh44 requested a review from a team as a code owner April 15, 2023 18:49
@jkosh44 jkosh44 requested a review from def- April 15, 2023 18:51
@@ -145,8 +145,9 @@ pub fn check_plan(
}

// Validate that the current session has the required attributes to execute the provided plan.
// Note: role attributes are not inherited by role membership.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait what? Is this because of NOINHERIT or something? I'm reading over https://www.postgresql.org/docs/current/role-membership.html but don't see what's going on here.

Copy link
Contributor Author

@jkosh44 jkosh44 Apr 17, 2023

Choose a reason for hiding this comment

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

No, I just originally misunderstood role attributes. They are never inherited, unlike role privileges and object ownership. If you scroll down to the end of those docs you'll see:

The role attributes LOGIN, SUPERUSER, CREATEDB, and CREATEROLE can be thought of as special privileges, but they are never inherited as ordinary privileges on database objects are. You must actually SET ROLE to a specific role having one of these attributes in order to make use of the attribute. Continuing the above example, we might choose to grant CREATEDB and CREATEROLE to the admin role. Then a session connecting as role joe would not have these privileges immediately, only after doing SET ROLE admin.

Copy link
Contributor

@def- def- left a comment

Choose a reason for hiding this comment

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

Thanks for test updates, lgtm

@jkosh44 jkosh44 merged commit 9ff08a8 into MaterializeInc:main Apr 17, 2023
@jkosh44 jkosh44 deleted the role-attribute-member-fix branch April 17, 2023 19:41
umanwizard pushed a commit to umanwizard/materialize-1 that referenced this pull request Apr 19, 2023
Previously, roles inherited role attributes of all their member roles.
This was incorrect and not has PostgreSQL works. This commit removes
this inheritance.

Part of #11579
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.

3 participants