Skip to content

Commit

Permalink
Merge pull request cockroachdb#126862 from rafiss/backport23.1-125590
Browse files Browse the repository at this point in the history
release-23.1: sql: add cluster setting for grant option inheritance
  • Loading branch information
rafiss authored Jul 10, 2024
2 parents 0c096d6 + 61932a8 commit 4dc8d04
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 1 deletion.
1 change: 1 addition & 0 deletions docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ server.web_session.purge.ttl duration 1h0m0s if nonzero, entries in system.web_s
server.web_session_timeout duration 168h0m0s the duration that a newly created web session will be valid
sql.auth.change_own_password.enabled boolean false controls whether a user is allowed to change their own password, even if they have no other privileges
sql.auth.createrole_allows_grant_role_membership.enabled boolean false if set, users with CREATEROLE privilege can grant/revoke membership in roles
sql.auth.grant_option_inheritance.enabled boolean true determines whether the GRANT OPTION for privileges is inherited through role membership
sql.auth.modify_cluster_setting_applies_to_all.enabled boolean true a bool which indicates whether MODIFYCLUSTERSETTING is able to set all cluster settings or only settings with the sql.defaults prefix (deprecated)
sql.auth.resolve_membership_single_scan.enabled boolean true determines whether to populate the role membership cache with a single scan
sql.closed_session_cache.capacity integer 1000 the maximum number of sessions in the cache
Expand Down
1 change: 1 addition & 0 deletions docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
<tr><td><div id="setting-server-web-session-timeout" class="anchored"><code>server.web_session_timeout</code></div></td><td>duration</td><td><code>168h0m0s</code></td><td>the duration that a newly created web session will be valid</td></tr>
<tr><td><div id="setting-sql-auth-change-own-password-enabled" class="anchored"><code>sql.auth.change_own_password.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>controls whether a user is allowed to change their own password, even if they have no other privileges</td></tr>
<tr><td><div id="setting-sql-auth-createrole-allows-grant-role-membership-enabled" class="anchored"><code>sql.auth.createrole_allows_grant_role_membership.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>if set, users with CREATEROLE privilege can grant/revoke membership in roles</td></tr>
<tr><td><div id="setting-sql-auth-grant-option-inheritance-enabled" class="anchored"><code>sql.auth.grant_option_inheritance.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether the GRANT OPTION for privileges is inherited through role membership</td></tr>
<tr><td><div id="setting-sql-auth-modify-cluster-setting-applies-to-all-enabled" class="anchored"><code>sql.auth.modify_cluster_setting_applies_to_all.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>a bool which indicates whether MODIFYCLUSTERSETTING is able to set all cluster settings or only settings with the sql.defaults prefix (deprecated)</td></tr>
<tr><td><div id="setting-sql-auth-resolve-membership-single-scan-enabled" class="anchored"><code>sql.auth.resolve_membership_single_scan.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether to populate the role membership cache with a single scan</td></tr>
<tr><td><div id="setting-sql-closed-session-cache-capacity" class="anchored"><code>sql.closed_session_cache.capacity</code></div></td><td>integer</td><td><code>1000</code></td><td>the maximum number of sessions in the cache</td></tr>
Expand Down
19 changes: 18 additions & 1 deletion pkg/sql/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,17 @@ func (p *planner) CheckGrantOptionsForUser(
if isAdmin {
return true, nil
}
return p.checkRolePredicate(ctx, user, func(role username.SQLUsername) (bool, error) {

// Normally, we check the user and its ancestors. But if
// enableGrantOptionInheritance is false, then we only check the user.
runPredicateFn := p.checkRolePredicate
if !enableGrantOptionInheritance.Get(&p.ExecCfg().Settings.SV) {
runPredicateFn = func(ctx context.Context, user username.SQLUsername, predicate func(role username.SQLUsername) (bool, error)) (bool, error) {
return predicate(user)
}
}

return runPredicateFn(ctx, user, func(role username.SQLUsername) (bool, error) {
isOwner, err := isOwner(ctx, p, privilegeObject, role)
return privs.CheckGrantOptions(role, privList) || isOwner, err
})
Expand Down Expand Up @@ -666,6 +676,13 @@ var useSingleQueryForRoleMembershipCache = settings.RegisterBoolSetting(
defaultSingleQueryForRoleMembershipCache,
).WithPublic()

var enableGrantOptionInheritance = settings.RegisterBoolSetting(
settings.TenantWritable,
"sql.auth.grant_option_inheritance.enabled",
"determines whether the GRANT OPTION for privileges is inherited through role membership",
true,
).WithPublic()

// resolveMemberOfWithAdminOption performs the actual recursive role membership lookup.
func resolveMemberOfWithAdminOption(
ctx context.Context, member username.SQLUsername, txn isql.Txn, singleQuery bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,3 +671,79 @@ test public owner_grant_option admin ALL
test public owner_grant_option other_owner ALL true
test public owner_grant_option owner_grant_option_child SELECT false
test public owner_grant_option root ALL true

subtest disable_grant_option_inheritance

statement ok
SET CLUSTER SETTING sql.auth.grant_option_inheritance.enabled = false

statement ok
CREATE USER parent;
CREATE USER child;
CREATE USER other;

statement ok
GRANT parent TO child

statement ok
CREATE TABLE tbl_owned_by_root (a INT PRIMARY KEY)

statement ok
SET ROLE parent

statement ok
CREATE TABLE tbl_owned_by_parent (a INT PRIMARY KEY)

statement ok
RESET role

statement ok
INSERT INTO tbl_owned_by_root VALUES (1);
INSERT INTO tbl_owned_by_parent VALUES (1);

statement ok
GRANT SELECT ON TABLE tbl_owned_by_root TO parent WITH GRANT OPTION

statement ok
SET ROLE child

# child should inherit the SELECT privilege itself.
query I
SELECT a FROM tbl_owned_by_root LIMIT 1
----
1

# child can also inherit SELECT via parent's ownership.
query I
SELECT a FROM tbl_owned_by_parent LIMIT 1
----
1

# child should not inherit the grant option.
statement error user child missing WITH GRANT OPTION privilege on SELECT
GRANT SELECT ON TABLE tbl_owned_by_root TO other

# Nor should child inherit the grant option from parent's ownership.
statement error user child missing WITH GRANT OPTION privilege on SELECT
GRANT SELECT ON TABLE tbl_owned_by_parent TO other

statement ok
RESET ROLE

statement ok
RESET CLUSTER SETTING sql.auth.grant_option_inheritance.enabled

statement ok
SET ROLE child

# Now the GRANT command should work since child inherits the grant option.
statement ok
GRANT SELECT ON TABLE tbl_owned_by_root TO other

statement ok
GRANT SELECT ON TABLE tbl_owned_by_parent TO other

statement ok
RESET role

subtest end

0 comments on commit 4dc8d04

Please sign in to comment.