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: ensure that CREATELOGIN is needed for CREATE USER #55368

Merged
merged 1 commit into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions pkg/sql/alter_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,13 @@ func (p *planner) checkPasswordOptionConstraints(

if roleOptions.Contains(roleoption.CREATELOGIN) ||
roleOptions.Contains(roleoption.NOCREATELOGIN) ||
roleOptions.Contains(roleoption.LOGIN) ||
(roleOptions.Contains(roleoption.NOLOGIN) && !newUser) || // CREATE ROLE NOLOGIN is valid without CREATELOGIN.
roleOptions.Contains(roleoption.PASSWORD) ||
roleOptions.Contains(roleoption.VALIDUNTIL) {
roleOptions.Contains(roleoption.VALIDUNTIL) ||
roleOptions.Contains(roleoption.LOGIN) ||
// CREATE ROLE NOLOGIN is valid without CREATELOGIN.
(roleOptions.Contains(roleoption.NOLOGIN) && !newUser) ||
// Disallow implicit LOGIN upon new user.
(newUser && !roleOptions.Contains(roleoption.NOLOGIN) && !roleOptions.Contains(roleoption.LOGIN)) {
// Only a role who has CREATELOGIN itself can grant CREATELOGIN or
// NOCREATELOGIN to another role, or set up a password for
// authentication, or set up password validity, or enable/disable
Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/create_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,18 @@ func (p *planner) CreateRoleNode(
return nil, err
}

// Check that the requested combination of password options is
// compatible with the user's own CREATELOGIN privilege.
if err := p.checkPasswordOptionConstraints(ctx, roleOptions, true /* newUser */); err != nil {
return nil, err
}

// Using CREATE ROLE syntax enables NOLOGIN by default.
if isRole && !roleOptions.Contains(roleoption.LOGIN) && !roleOptions.Contains(roleoption.NOLOGIN) {
roleOptions = append(roleOptions,
roleoption.RoleOption{Option: roleoption.NOLOGIN, HasValue: false})
}

// Check that the requested combination of password options is
// compatible with the user's own CREATELOGIN privilege.
if err := p.checkPasswordOptionConstraints(ctx, roleOptions, true /* newUser */); err != nil {
return nil, err
}

ua, err := p.getUserAuthInfo(ctx, nameE, opName)
if err != nil {
return nil, err
Expand Down
18 changes: 14 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/role
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ ALTER ROLE rolewithcreate WITH NOCREATEROLE
user root

statement ok
ALTER USER testuser CREATEROLE
ALTER USER testuser CREATEROLE CREATELOGIN

user testuser

Expand Down Expand Up @@ -1200,12 +1200,16 @@ CREATE USER testuser3 LOGIN
statement error user testuser does not have CREATELOGIN privilege
CREATE ROLE testrole3 LOGIN

# However it's possible to create a user/role with NOLOGIN.
statement error user testuser does not have CREATELOGIN privilege
CREATE USER testuser2

# CREATE ROLE implies NOLOGIN, which is OK
statement ok
CREATE USER testuser4 NOLOGIN
CREATE ROLE testuser4

# It's also possible to create a user/role with NOLOGIN.
statement ok
CREATE USER testuser2
CREATE USER testuser2 NOLOGIN

statement error user testuser does not have CREATELOGIN privilege
ALTER USER testuser2 WITH PASSWORD 'abc'
Expand Down Expand Up @@ -1308,6 +1312,12 @@ ALTER USER testuser NOCREATELOGIN

user testuser

statement error user testuser does not have CREATELOGIN privilege
CREATE USER testuser5

statement ok
CREATE ROLE testuser6

statement error user testuser does not have CREATELOGIN privilege
CREATE USER testuser5 WITH PASSWORD 'abc'

Expand Down
12 changes: 10 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/user
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,24 @@ GRANT SELECT ON system.role_options to testuser
user testuser

statement ok
CREATE USER user4 CREATEROLE
CREATE ROLE user4 CREATEROLE

statement ok
CREATE USER user5 NOLOGIN

query TTT
SELECT * FROM system.role_options
----
testuser CREATEROLE NULL
user4 CREATEROLE NULL
user4 NOLOGIN NULL
user5 NOLOGIN NULL

statement ok
DROP ROLE user4

statement ok
DROP USER user4
DROP ROLE user5

subtest min_password_length

Expand Down