Skip to content

Commit

Permalink
sql: owning a schema gives privilege to drop tables in schema
Browse files Browse the repository at this point in the history
Release note (sql change): Schema owners can drop tables inside
the schema without explicit DROP privilege on the table.
  • Loading branch information
RichardJCai committed Aug 21, 2020
1 parent 1f49884 commit 0bef16c
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 8 deletions.
38 changes: 38 additions & 0 deletions pkg/sql/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,3 +561,41 @@ func (p *planner) checkCanAlterToNewOwner(

return nil
}

// HasOwnershipOnSchema checks if the current user has ownership on the schema.
// For schemas, we cannot always use HasOwnership as not every schema has a
// descriptor.
func (p *planner) HasOwnershipOnSchema(ctx context.Context, schemaID descpb.ID) (bool, error) {
resolvedSchema, err := p.Descriptors().ResolveSchemaByID(
ctx, p.Txn(), schemaID,
)
if err != nil {
return false, err
}

hasOwnership := false
switch resolvedSchema.Kind {
case sqlbase.SchemaPublic:
// admin is the owner of the public schema.
hasOwnership, err = p.UserHasAdminRole(ctx, p.User())
if err != nil {
return false, err
}
case sqlbase.SchemaVirtual:
// Cannot drop on virtual schemas.
case sqlbase.SchemaTemporary:
// The user only owns the temporary schema that corresponds to the
// TemporarySchemaID in the sessionData.
hasOwnership = p.SessionData() != nil &&
p.SessionData().TemporarySchemaID == uint32(resolvedSchema.ID)
case sqlbase.SchemaUserDefined:
hasOwnership, err = p.HasOwnership(ctx, resolvedSchema.Desc)
if err != nil {
return false, err
}
default:
panic(errors.AssertionFailedf("unknown schema kind %d", resolvedSchema.Kind))
}

return hasOwnership, nil
}
1 change: 1 addition & 0 deletions pkg/sql/catalog/descs/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,7 @@ func (tc *Collection) ResolveSchemaByID(

// If this collection is attached to a session and the session has created
// a temporary schema, then check if the schema ID matches.
// This check doesn't work after switching databases. Issue #53163.
if tc.sessionData != nil && tc.sessionData.TemporarySchemaID == uint32(schemaID) {
return sqlbase.ResolvedSchema{
Kind: sqlbase.SchemaTemporary,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/drop_cascade.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (d *dropCascadeState) resolveCollectedObjects(
" dropped or made public before dropping database %s",
objName.FQString(), tree.AsString((*tree.Name)(&dbName)))
}
if err := p.prepareDropWithTableDesc(ctx, tbDesc); err != nil {
if err := p.canDropTable(ctx, tbDesc); err != nil {
return err
}
// Recursively check permissions on all dependent views, since some may
Expand Down
27 changes: 20 additions & 7 deletions pkg/sql/drop_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,33 @@ func (p *planner) prepareDrop(
if tableDesc == nil {
return nil, err
}
if err := p.prepareDropWithTableDesc(ctx, tableDesc); err != nil {
if err := p.canDropTable(ctx, tableDesc); err != nil {
return nil, err
}
return tableDesc, nil
}

// prepareDropWithTableDesc behaves as prepareDrop, except it assumes the
// table descriptor is already fetched. This is useful for DropDatabase,
// as prepareDrop requires resolving a TableName when DropDatabase already
// has it resolved.
func (p *planner) prepareDropWithTableDesc(
// canDropTable returns an error if the user cannot drop the table.
func (p *planner) canDropTable(
ctx context.Context, tableDesc *sqlbase.MutableTableDescriptor,
) error {
return p.CheckPrivilege(ctx, tableDesc, privilege.DROP)
// Don't check for ownership if the table is a temp table.
// ResolveSchema currently doesn't work for temp schemas.
// Hack until #53163 is fixed.
hasOwnership := false
var err error
if !tableDesc.Temporary {
// If the user owns the schema the table is part of, they can drop the table.
hasOwnership, err = p.HasOwnershipOnSchema(ctx, tableDesc.GetParentSchemaID())
if err != nil {
return err
}
}
if !hasOwnership {
return p.CheckPrivilege(ctx, tableDesc, privilege.DROP)
}

return nil
}

// canRemoveFKBackReference returns an error if the input backreference isn't
Expand Down
19 changes: 19 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/drop_table
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,22 @@ CREATE TABLE a (id INT PRIMARY KEY)
query I
SELECT * FROM a
----

user testuser

statement ok
SET experimental_enable_user_defined_schemas = true

statement ok
CREATE SCHEMA s

user root

statement ok
CREATE TABLE s.t()

user testuser

# Being the owner of schema s should allow testuser to drop table s.t.
statement ok
DROP TABLE s.t

0 comments on commit 0bef16c

Please sign in to comment.