Skip to content

Commit

Permalink
sql: dropping user with default privilege's error message is more clear
Browse files Browse the repository at this point in the history 
Release note (sql change): When dropping a user that has default privileges,
the error message now includes which database and schema the default privileges
are defined in.

Additionally a hint is given to show exactly how to remove the default privileges.
Example:
pq: role testuser4 cannot be dropped because some objects depend on it
owner of default privileges on new sequences belonging to role testuser4 in database testdb2 in schema s
privileges for default privileges on new sequences belonging to role testuser3 in database testdb2 in schema s
privileges for default privileges on new sequences for all roles in database testdb2 in schema public
privileges for default privileges on new sequences for all roles in database testdb2 in schema s
HINT: USE testdb2; ALTER DEFAULT PRIVILEGES FOR ROLE testuser4 IN SCHEMA S REVOKE ALL ON SEQUENCES FROM testuser3;
USE testdb2; ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 IN SCHEMA S REVOKE ALL ON SEQUENCES FROM testuser4;
USE testdb2; ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA PUBLIC REVOKE ALL ON SEQUENCES FROM testuser4;
USE testdb2; ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA S REVOKE ALL ON SEQUENCES FROM testuser4;
  • Loading branch information
@RichardJCai
RichardJCai committed Feb 25, 2022
1 parent 920464f commit 586f44c
Show file tree
Hide file tree
Showing 2 changed files with 181 additions and 65 deletions.
78 changes: 61 additions & 17 deletions pkg/sql/drop_role.go
View file
Expand Up @@ -14,6 +14,7 @@ import (
"context"
"fmt"
"sort"
"strings"

"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand Down Expand Up @@ -144,7 +145,7 @@ func (n *DropRoleNode) startExec(params runParams) error {
break
}
}
return accumulateDependentDefaultPrivileges(db.GetDefaultPrivilegeDescriptor(), userNames)
return accumulateDependentDefaultPrivileges(db.GetDefaultPrivilegeDescriptor(), userNames, db.GetName(), "" /* schemaName */)
}); err != nil {
return err
}
Expand Down Expand Up @@ -208,7 +209,12 @@ func (n *DropRoleNode) startExec(params runParams) error {
})
}

if err := accumulateDependentDefaultPrivileges(schemaDesc.GetDefaultPrivilegeDescriptor(), userNames); err != nil {
dbDesc, err := lCtx.getDatabaseByID(schemaDesc.GetParentID())
if err != nil {
return err
}

if err := accumulateDependentDefaultPrivileges(schemaDesc.GetDefaultPrivilegeDescriptor(), userNames, dbDesc.GetName(), schemaDesc.GetName()); err != nil {
return err
}
}
Expand Down Expand Up @@ -251,6 +257,20 @@ func (n *DropRoleNode) startExec(params runParams) error {
for _, name := range n.roleNames {
// Did the user own any objects?
dependentObjects := userNames[name]

// Sort the slice so we're guaranteed the same ordering on errors.
sort.SliceStable(dependentObjects, func(i int, j int) bool {
if dependentObjects[i].ObjectType != dependentObjects[j].ObjectType {
return dependentObjects[i].ObjectType < dependentObjects[j].ObjectType
}

if dependentObjects[i].ObjectName != dependentObjects[j].ObjectName {
return dependentObjects[i].ObjectName < dependentObjects[j].ObjectName
}

return dependentObjects[i].ErrorMessage.Error() < dependentObjects[j].ErrorMessage.Error()
})
var hints []string
if len(dependentObjects) > 0 {
objectsMsg := tree.NewFmtCtx(tree.FmtSimple)
for _, obj := range dependentObjects {
Expand All @@ -260,6 +280,7 @@ func (n *DropRoleNode) startExec(params runParams) error {
case defaultPrivilege:
hasDependentDefaultPrivilege = true
objectsMsg.WriteString(fmt.Sprintf("\n%s", obj.ErrorMessage))
hints = append(hints, errors.GetAllHints(obj.ErrorMessage)...)
}
}
objects := objectsMsg.CloseAndGetString()
Expand All @@ -268,10 +289,7 @@ func (n *DropRoleNode) startExec(params runParams) error {
name, objects)
if hasDependentDefaultPrivilege {
err = errors.WithHint(err,
"use SHOW DEFAULT PRIVILEGES FOR ROLE to find existing default privileges"+
" and execute ALTER DEFAULT PRIVILEGES {FOR ROLE ... / FOR ALL ROLES} "+
"REVOKE ... ON ... FROM ... to remove them"+
"\nsee: SHOW DEFAULT PRIVILEGES and ALTER DEFAULT PRIVILEGES",
strings.Join(hints, "\n"),
)
}
return err
Expand Down Expand Up @@ -422,6 +440,8 @@ func (*DropRoleNode) Close(context.Context) {}
func accumulateDependentDefaultPrivileges(
defaultPrivilegeDescriptor catalog.DefaultPrivilegeDescriptor,
userNames map[security.SQLUsername][]objectAndType,
dbName string,
schemaName string,
) error {
// The func we pass into ForEachDefaultPrivilegeForRole will never
// err so no err will be returned.
Expand All @@ -434,7 +454,7 @@ func accumulateDependentDefaultPrivileges(
role.ForAllRoles = true
}
for object, defaultPrivs := range defaultPrivilegesForRole.DefaultPrivilegesPerObject {
addDependentPrivileges(object, defaultPrivs, role, userNames)
addDependentPrivileges(object, defaultPrivs, role, userNames, dbName, schemaName)
}
return nil
})
Expand All @@ -445,6 +465,8 @@ func addDependentPrivileges(
defaultPrivs catpb.PrivilegeDescriptor,
role catpb.DefaultPrivilegesRole,
userNames map[security.SQLUsername][]objectAndType,
dbName string,
schemaName string,
) {
var objectType string
switch object {
Expand All @@ -458,37 +480,59 @@ func addDependentPrivileges(
objectType = "schemas"
}

inSchemaMsg := ""
if schemaName != "" {
inSchemaMsg = fmt.Sprintf(" in schema %s", schemaName)
}

createHint := func(
role catpb.DefaultPrivilegesRole,
grantee security.SQLUsername,
) string {

roleString := "ALL ROLES"
if !role.ForAllRoles {
roleString = fmt.Sprintf("ROLE %s", role.Role.SQLIdentifier())
}

return fmt.Sprintf("USE %s; ALTER DEFAULT PRIVILEGES FOR %s%s REVOKE ALL ON %s FROM %s;",
dbName, roleString, strings.ToUpper(inSchemaMsg), strings.ToUpper(object.String()), grantee.SQLIdentifier())
}

for _, privs := range defaultPrivs.Users {
grantee := privs.User()
if !role.ForAllRoles {
if _, ok := userNames[role.Role]; ok {
hint := createHint(role, grantee)
userNames[role.Role] = append(userNames[role.Role],
objectAndType{
ObjectType: defaultPrivilege,
ErrorMessage: errors.Newf(
"owner of default privileges on new %s belonging to role %s",
objectType, role.Role,
),
ErrorMessage: errors.WithHint(
errors.Newf(
"owner of default privileges on new %s belonging to role %s in database %s%s",
objectType, role.Role, dbName, inSchemaMsg,
), hint),
})
}
}
grantee := privs.User()
if _, ok := userNames[grantee]; ok {
hint := createHint(role, grantee)
var err error
if role.ForAllRoles {
err = errors.Newf(
"privileges for default privileges on new %s for all roles",
objectType,
"privileges for default privileges on new %s for all roles in database %s%s",
objectType, dbName, inSchemaMsg,
)
} else {
err = errors.Newf(
"privileges for default privileges on new %s belonging to role %s",
objectType, role.Role,
"privileges for default privileges on new %s belonging to role %s in database %s%s",
objectType, role.Role, dbName, inSchemaMsg,
)
}
userNames[grantee] = append(userNames[grantee],
objectAndType{
ObjectType: defaultPrivilege,
ErrorMessage: err,
ErrorMessage: errors.WithHint(err, hint),
})
}
}
Expand Down
168 changes: 120 additions & 48 deletions pkg/sql/logictest/testdata/logic_test/drop_role_with_default_privileges
View file
@@ -1,85 +1,157 @@
statement ok
CREATE USER test1;
CREATE USER test2;
GRANT test1 TO ROOT;
CREATE USER testuser1;
CREATE USER testuser2;
GRANT testuser1 TO ROOT;

statement ok
ALTER DEFAULT PRIVILEGES FOR ROLE test1 GRANT SELECT ON TABLES TO test2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 GRANT SELECT ON TABLES TO testuser2;

statement error pq: role test1 cannot be dropped because some objects depend on it\nowner of default privileges on new relations belonging to role test1
DROP ROLE test1
statement error pq: role testuser1 cannot be dropped because some objects depend on it\nowner of default privileges on new relations belonging to role testuser1 in database test
DROP ROLE testuser1

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new relations belonging to role test1
DROP ROLE test2;
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new relations belonging to role testuser1 in database test
DROP ROLE testuser2;

statement ok
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE SELECT ON TABLES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE test1 GRANT USAGE ON SCHEMAS TO test2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE SELECT ON TABLES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 GRANT USAGE ON SCHEMAS TO testuser2;

statement error pq: role test1 cannot be dropped because some objects depend on it\nowner of default privileges on new schemas belonging to role test1
DROP ROLE test1
statement error pq: role testuser1 cannot be dropped because some objects depend on it\nowner of default privileges on new schemas belonging to role testuser1 in database test
DROP ROLE testuser1

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new schemas belonging to role test1
DROP ROLE test2;
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new schemas belonging to role testuser1 in database test
DROP ROLE testuser2;

statement ok
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE USAGE ON SCHEMAS FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE test1 GRANT USAGE ON TYPES TO test2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE USAGE ON SCHEMAS FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 GRANT USAGE ON TYPES TO testuser2;

statement error pq: role test1 cannot be dropped because some objects depend on it\nowner of default privileges on new types belonging to role test1
DROP ROLE test1
statement error pq: role testuser1 cannot be dropped because some objects depend on it\nowner of default privileges on new types belonging to role testuser1 in database test
DROP ROLE testuser1

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new types belonging to role test1
DROP ROLE test2;
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new types belonging to role testuser1 in database test
DROP ROLE testuser2;

statement ok
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE USAGE ON TYPES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE test1 GRANT SELECT ON SEQUENCES TO test2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE USAGE ON TYPES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 GRANT SELECT ON SEQUENCES TO testuser2;

statement error pq: role test1 cannot be dropped because some objects depend on it\nowner of default privileges on new sequences belonging to role test1
DROP ROLE test1
statement error pq: role testuser1 cannot be dropped because some objects depend on it\nowner of default privileges on new sequences belonging to role testuser1 in database test
DROP ROLE testuser1

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new sequences belonging to role test1
DROP ROLE test2;
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new sequences belonging to role testuser1 in database test
DROP ROLE testuser2;

statement ok
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE SELECT ON TABLES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE USAGE ON SCHEMAS FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE USAGE ON TYPES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE test1 REVOKE SELECT ON SEQUENCES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE SELECT ON TABLES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE USAGE ON SCHEMAS FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE USAGE ON TYPES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser1 REVOKE SELECT ON SEQUENCES FROM testuser2;

statement ok
DROP ROLE test1;
DROP ROLE testuser1;

statement ok
DROP ROLE test2;
DROP ROLE testuser2;

statement ok
CREATE USER test2
CREATE USER testuser2

statement ok
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT SELECT ON TABLES TO test2
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT SELECT ON TABLES TO testuser2

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new relations for all roles
DROP ROLE test2;
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new relations for all roles in database test
DROP ROLE testuser2;

statement ok
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE SELECT ON TABLES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT USAGE ON SCHEMAS TO test2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE SELECT ON TABLES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT USAGE ON SCHEMAS TO testuser2;

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new schemas for all roles
DROP ROLE test2;
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new schemas for all roles in database test
DROP ROLE testuser2;

statement ok
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE USAGE ON SCHEMAS FROM test2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT USAGE ON TYPES TO test2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE USAGE ON SCHEMAS FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT USAGE ON TYPES TO testuser2;

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new types for all roles
DROP ROLE test2
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new types for all roles in database test
DROP ROLE testuser2

statement ok
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE USAGE ON TYPES FROM test2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT SELECT ON SEQUENCES TO test2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE USAGE ON TYPES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT SELECT ON SEQUENCES TO testuser2;

statement error pq: role test2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new sequences for all roles
DROP ROLE test2
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nprivileges for default privileges on new sequences for all roles in database test
DROP ROLE testuser2

# Grant default privileges to testuser2 in a second database.
statement ok
CREATE ROLE testuser3;
GRANT testuser2 TO root;
GRANT testuser3 TO root;
CREATE DATABASE testdb2;
USE testdb2;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT SELECT ON SEQUENCES TO testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 GRANT SELECT ON SEQUENCES TO testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser2 GRANT SELECT ON SEQUENCES TO testuser3;

statement error pq: role testuser2 cannot be dropped because some objects depend on it\nowner of default privileges on new sequences belonging to role testuser2 in database testdb2\nprivileges for default privileges on new sequences belonging to role testuser3 in database testdb2\nprivileges for default privileges on new sequences for all roles in database test\nprivileges for default privileges on new sequences for all roles in database testdb2
DROP ROLE testuser2

# Check the hint output.
statement error pq: role testuser2 cannot be dropped because some objects depend on it\nowner of default privileges on new sequences belonging to role testuser2 in database testdb2\nprivileges for default privileges on new sequences belonging to role testuser3 in database testdb2\nprivileges for default privileges on new sequences for all roles in database test\nprivileges for default privileges on new sequences for all roles in database testdb2\nHINT: USE testdb2; ALTER DEFAULT PRIVILEGES FOR ROLE testuser2 REVOKE ALL ON SEQUENCES FROM testuser3;\nUSE testdb2; ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 REVOKE ALL ON SEQUENCES FROM testuser2;\nUSE test; ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE ALL ON SEQUENCES FROM testuser2;\nUSE testdb2; ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE ALL ON SEQUENCES FROM testuser2;
DROP ROLE testuser2

# Tests where default privileges are defined on the schema.
statement ok
CREATE ROLE testuser4;
GRANT testuser4 TO root;

statement ok
ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA PUBLIC GRANT SELECT ON SEQUENCES TO testuser4;

statement error pq: role testuser4 cannot be dropped because some objects depend on it\nprivileges for default privileges on new sequences for all roles in database testdb2 in schema public
DROP ROLE testuser4

statement ok
CREATE SCHEMA s;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA s GRANT SELECT ON SEQUENCES TO testuser4;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 IN SCHEMA s GRANT SELECT ON SEQUENCES TO testuser4;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser4 IN SCHEMA s GRANT SELECT ON SEQUENCES TO testuser3;

statement error pq: role testuser4 cannot be dropped because some objects depend on it\nowner of default privileges on new sequences belonging to role testuser4 in database testdb2 in schema s\nprivileges for default privileges on new sequences belonging to role testuser3 in database testdb2 in schema s\nprivileges for default privileges on new sequences for all roles in database testdb2 in schema public\nprivileges for default privileges on new sequences for all roles in database testdb2 in schema s
DROP ROLE testuser4

statement error pq: role testuser4 cannot be dropped because some objects depend on it\nowner of default privileges on new sequences belonging to role testuser4 in database testdb2 in schema s\nprivileges for default privileges on new sequences belonging to role testuser3 in database testdb2 in schema s\nprivileges for default privileges on new sequences for all roles in database testdb2 in schema public\nprivileges for default privileges on new sequences for all roles in database testdb2 in schema s\nHINT: USE testdb2; ALTER DEFAULT PRIVILEGES FOR ROLE testuser4 IN SCHEMA S REVOKE ALL ON SEQUENCES FROM testuser3;\nUSE testdb2; ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 IN SCHEMA S REVOKE ALL ON SEQUENCES FROM testuser4;\nUSE testdb2; ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA PUBLIC REVOKE ALL ON SEQUENCES FROM testuser4;\nUSE testdb2; ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA S REVOKE ALL ON SEQUENCES FROM testuser4;
DROP ROLE testuser4

# Now let's ensure we can revoke all the privileges and drop testuser2/3/4.

# Prepare to drop testuser2.
statement ok
USE test;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE ALL ON SEQUENCES FROM testuser2;

statement ok
USE testdb2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 REVOKE ALL ON SEQUENCES FROM testuser2;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser2 REVOKE ALL ON SEQUENCES FROM testuser3;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES REVOKE ALL ON SEQUENCES FROM testuser2;

statement ok
DROP ROLE testuser2;

# Prepare to drop testuser3.
statement ok
ALTER DEFAULT PRIVILEGES FOR ROLE testuser3 IN SCHEMA s REVOKE ALL ON SEQUENCES FROM testuser4;
ALTER DEFAULT PRIVILEGES FOR ROLE testuser4 IN SCHEMA s REVOKE ALL ON SEQUENCES FROM testuser3;

statement ok
DROP ROLE testuser3;

statement ok
ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA s REVOKE ALL ON SEQUENCES FROM testuser4;
ALTER DEFAULT PRIVILEGES FOR ALL ROLES IN SCHEMA public REVOKE ALL ON SEQUENCES FROM testuser4;

statement ok
DROP ROLE testuser4;

0 comments on commit 586f44c

Please sign in to comment.