Merge #55368

55368: sql: ensure that CREATELOGIN is needed for CREATE USER r=solongordon a=knz

Fixes #55367

Release note (bug fix): The `CREATE USER` statement without explicit
NOLOGIN option implicitly grants LOGIN, and so requires the
CREATELOGIN privilege. This was not checked properly, and is now
enforced. This bug was introduced earlier in the v20.2 development
cycle.

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>

pkg/sql/alter_role.go

@@ -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

pkg/sql/create_role.go

@@ -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

pkg/sql/logictest/testdata/logic_test/role

@@ -891,7 +891,7 @@ ALTER ROLE rolewithcreate WITH NOCREATEROLE
 user root
 
 statement ok
-ALTER USER testuser CREATEROLE
+ALTER USER testuser CREATEROLE CREATELOGIN
 
 user testuser
 
@@ -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'
@@ -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'
 

pkg/sql/logictest/testdata/logic_test/user

@@ -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