Skip to content

Commit e63be70

Browse files
craig[bot]souravcrl
andcommitted
Merge #149463
149463: sql: restrict ALTER user password for provisioned users r=rafiss a=souravcrl We need to restrict the `ALTER USER password` command for provisioned users to disallow password set/password change for self as login access for these users is strictly managed through IDP based authentication method. fixes #146061 Epic CRDB-21590 Release note (sql): Post the change the users with the role option `PROVISIONSRC` assigned to them will be unable to change their own password overriding any config set for sql.auth.change_own_password.enabled cluster setting. Changing other role options still has the same privilege requirements as before (either CREATEROLE or CREATELOGIN, depending on the option). Co-authored-by: souravcrl <[email protected]>
2 parents 06c6a78 + 847321d commit e63be70

File tree

5 files changed

+91
-15
lines changed

5 files changed

+91
-15
lines changed

pkg/ccl/logictestccl/testdata/logic_test/provisioning

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,24 @@ AND option = 'PROVISIONSRC'
2121
ldap:ldap.bar.com
2222

2323
statement ok
24-
ALTER ROLE role_with_provisioning PROVISIONSRC 'ldap:ldap.example.com'
24+
CREATE ROLE role_with_provisioning_2 PROVISIONSRC 'ldap:ldap.example.com'
2525

2626
query T
2727
SELECT value FROM system.role_options
28-
WHERE username = 'role_with_provisioning'
28+
WHERE username = 'role_with_provisioning_2'
2929
AND option = 'PROVISIONSRC'
3030
----
3131
ldap:ldap.example.com
3232

3333
statement error pq: provided IDP "\[\]!@#%#\^\$&\*" in PROVISIONSRC is non parseable: parse "\[\]!@#%#\^\$&\*": invalid URL escape "%#\^"
34-
ALTER ROLE role_with_provisioning PROVISIONSRC 'ldap:[]!@#%#^$&*'
34+
CREATE ROLE role_with_provisioning_3 PROVISIONSRC 'ldap:[]!@#%#^$&*'
3535

3636
statement ok
37-
ALTER ROLE role_with_provisioning PROVISIONSRC 'ldap:foo.bar'
37+
CREATE ROLE role_with_provisioning_3 PROVISIONSRC 'ldap:foo.bar'
3838

3939
query T
4040
SELECT value FROM system.role_options
41-
WHERE username = 'role_with_provisioning'
41+
WHERE username = 'role_with_provisioning_3'
4242
AND option = 'PROVISIONSRC'
4343
----
4444
ldap:foo.bar

pkg/sql/alter_role.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,15 @@ func (n *alterRoleNode) startExec(params runParams) error {
158158
"cannot edit admin role")
159159
}
160160

161+
// Restrict PASSWORD and PROVISIONSRC alterations for provisioned users.
162+
if provisioned, err := params.p.UserHasRoleOption(params.ctx, n.roleName, roleoption.PROVISIONSRC); err != nil {
163+
return err
164+
} else if provisioned &&
165+
(n.roleOptions.Contains(roleoption.PROVISIONSRC) || n.roleOptions.Contains(roleoption.PASSWORD)) {
166+
return pgerror.Newf(pgcode.ObjectNotInPrerequisiteState,
167+
"cannot alter PASSWORD/PROVISIONSRC option for provisioned user %s", n.roleName)
168+
}
169+
161170
needMoreChecks := true
162171
if n.roleName == params.p.SessionData().User() && len(n.roleOptions) == 1 && n.roleOptions.Contains(roleoption.
163172
PASSWORD) {

pkg/sql/authorization.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -647,17 +647,20 @@ func (p *planner) UserHasRoleOption(
647647
return false, errors.AssertionFailedf("cannot use HasRoleOption without a txn")
648648
}
649649

650-
if user.IsRootUser() || user.IsNodeUser() {
651-
return true, nil
652-
}
650+
// Skip non-admin inherited role options for validation.
651+
if !slices.Contains(roleoption.NonAdminInheritedOptions, roleOption) {
652+
if user.IsRootUser() || user.IsNodeUser() {
653+
return true, nil
654+
}
653655

654-
hasAdmin, err := p.UserHasAdminRole(ctx, user)
655-
if err != nil {
656-
return false, err
657-
}
658-
if hasAdmin {
659-
// Superusers have all role privileges.
660-
return true, nil
656+
hasAdmin, err := p.UserHasAdminRole(ctx, user)
657+
if err != nil {
658+
return false, err
659+
}
660+
if hasAdmin {
661+
// Superusers have all role privileges.
662+
return true, nil
663+
}
661664
}
662665

663666
hasRolePrivilege, err := p.InternalSQLTxn().QueryRowEx(

pkg/sql/logictest/testdata/logic_test/role

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,6 +1530,66 @@ RESET ROLE
15301530

15311531
subtest end
15321532

1533+
subtest validate_alter_provisioned_user_restrictions
1534+
1535+
# Changing PROVISIONSRC privilege for provisioned users is disallowed even for
1536+
# ADMIN users, they can only be dropped.
1537+
statement ok
1538+
DROP ROLE testuser
1539+
1540+
statement ok
1541+
CREATE USER testuser with PROVISIONSRC 'ldap:example.com'
1542+
1543+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1544+
ALTER ROLE testuser WITH PROVISIONSRC 'ldap:example.com'
1545+
1546+
statement ok
1547+
SET ROLE testuser
1548+
1549+
# Changing users with SET ROLE should now disallow self-password/provisionsrc
1550+
# change as role has a PROVISIONSRC privilege.
1551+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1552+
ALTER USER testuser PASSWORD 'cat'
1553+
1554+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1555+
ALTER USER testuser PROVISIONSRC 'ldap:example2.com'
1556+
1557+
statement ok
1558+
RESET ROLE
1559+
1560+
user testuser
1561+
1562+
# Verify that now provisioned testuser can't change its own password or provisionsrc.
1563+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1564+
ALTER USER testuser PASSWORD 'xyz'
1565+
1566+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1567+
ALTER USER CURRENT_USER PASSWORD '123'
1568+
1569+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1570+
ALTER USER testuser PROVISIONSRC 'ldap:example2.com'
1571+
1572+
# Verify that provisioned testuser still cannot *remove* its own password.
1573+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1574+
ALTER USER testuser PASSWORD NULL
1575+
1576+
# testuser still cannot change another user's password.
1577+
statement error pq: user testuser does not have CREATEROLE privilege
1578+
ALTER USER testuser2 WITH PASSWORD 'abc'
1579+
1580+
# Verify that testuser still cannot modify other role options of itself.
1581+
statement error pq: cannot alter PASSWORD/PROVISIONSRC option for provisioned user testuser
1582+
ALTER USER testuser PASSWORD 'abc' VALID UNTIL '4044-10-31'
1583+
1584+
user root
1585+
1586+
1587+
statement ok
1588+
DROP user testuser;
1589+
CREATE user testuser CREATEROLE;
1590+
1591+
subtest end
1592+
15331593
subtest replication
15341594

15351595
statement ok

pkg/sql/roleoption/role_option.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ const (
8181
PROVISIONSRC
8282
)
8383

84+
var NonAdminInheritedOptions = []Option{
85+
PROVISIONSRC,
86+
}
87+
8488
// ControlChangefeedDeprecationNoticeMsg is a user friendly notice which should be shown when CONTROLCHANGEFEED is used
8589
//
8690
// TODO(#94757): remove CONTROLCHANGEFEED entirely

0 commit comments

Comments
 (0)