Rework privilege storage.

Work towards #2005.

Credentials are now stored as a sorted list of {user, privilegeBitField}
The ALL logic makes grant/revoke a bit twisted, but is probably the most
expected behavior.

GRANT, REVOKE, and SHOW GRANTS are finalized.

The final step will be to fix all the CheckPrivilege calls to use the proper
privilege and finally remove READ,WRITE.

sql/create.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/security"
 	"github.com/cockroachdb/cockroach/sql/parser"
+	"github.com/cockroachdb/cockroach/structured"
 )
 
 // CreateDatabase creates a database.
@@ -58,17 +59,16 @@ func (p *planner) CreateTable(n *parser.CreateTable) (planNode, error) {
 		return nil, err
 	}
 
-	if !dbDesc.HasPrivilege(p.user, parser.PrivilegeWrite) {
-		return nil, fmt.Errorf("user %s does not have %s privilege on database %s",
-			p.user, parser.PrivilegeWrite, dbDesc.Name)
+	if err := p.checkPrivilege(dbDesc, structured.Privilege_WRITE); err != nil {
+		return nil, err
 	}
 
 	desc, err := makeTableDesc(n)
 	if err != nil {
 		return nil, err
 	}
 	// Inherit permissions from the database descriptor.
-	desc.PrivilegeDescriptor = dbDesc.PrivilegeDescriptor
+	desc.Privileges = dbDesc.GetPrivileges()
 
 	if err := desc.AllocateIDs(); err != nil {
 		return nil, err

sql/database.go

@@ -19,7 +19,6 @@ package sql
 
 import (
 	"github.com/cockroachdb/cockroach/proto"
-	"github.com/cockroachdb/cockroach/security"
 	"github.com/cockroachdb/cockroach/sql/parser"
 	"github.com/cockroachdb/cockroach/structured"
 )
@@ -39,11 +38,8 @@ func (dk databaseKey) Name() string {
 
 func makeDatabaseDesc(p *parser.CreateDatabase) structured.DatabaseDescriptor {
 	return structured.DatabaseDescriptor{
-		Name: p.Name.String(),
-		PrivilegeDescriptor: structured.PrivilegeDescriptor{
-			Read:  []string{security.RootUser},
-			Write: []string{security.RootUser},
-		},
+		Name:       p.Name.String(),
+		Privileges: structured.NewDefaultDatabasePrivilegeDescriptor(),
 	}
 }
 

sql/database_test.go

@@ -20,7 +20,6 @@ package sql
 import (
 	"testing"
 
-	"github.com/cockroachdb/cockroach/security"
 	"github.com/cockroachdb/cockroach/sql/parser"
 	"github.com/cockroachdb/cockroach/util/leaktest"
 )
@@ -40,10 +39,7 @@ func TestMakeDatabaseDesc(t *testing.T) {
 	if desc.ID != 0 {
 		t.Fatalf("expected ID == 0, got %d", desc.ID)
 	}
-	if len(desc.Read) != 1 || desc.Read[0] != security.RootUser {
-		t.Fatalf("expected read == [root], got: %v", desc.Read)
-	}
-	if len(desc.Write) != 1 || desc.Write[0] != security.RootUser {
-		t.Fatalf("expected write == [root], got: %v", desc.Write)
+	if len(desc.GetPrivileges().Users) != 1 {
+		t.Fatalf("wrong number of privilege users, expected 1, got: %d", len(desc.GetPrivileges().Users))
 	}
 }

sql/delete.go

@@ -19,7 +19,6 @@ package sql
 
 import (
 	"bytes"
-	"fmt"
 
 	"github.com/cockroachdb/cockroach/client"
 	"github.com/cockroachdb/cockroach/proto"
@@ -38,9 +37,8 @@ func (p *planner) Delete(n *parser.Delete) (planNode, error) {
 		return nil, err
 	}
 
-	if !tableDesc.HasPrivilege(p.user, parser.PrivilegeWrite) {
-		return nil, fmt.Errorf("user %s does not have %s privilege on table %s",
-			p.user, parser.PrivilegeWrite, tableDesc.Name)
+	if err := p.checkPrivilege(tableDesc, structured.Privilege_WRITE); err != nil {
+		return nil, err
 	}
 
 	// TODO(tamird,pmattis): avoid going through Select to avoid encoding

sql/descriptor.go

@@ -45,17 +45,23 @@ type descriptorKey interface {
 // TODO(marc): this is getting rather large.
 type descriptorProto interface {
 	gogoproto.Message
-	Grant(*parser.Grant) error
-	Revoke(*parser.Revoke) error
-	Show() (structured.UserPrivilegeList, error)
-	HasPrivilege(string, parser.PrivilegeType) bool
+	GetPrivileges() *structured.PrivilegeDescriptor
 	GetID() structured.ID
 	SetID(structured.ID)
 	TypeName() string
 	GetName() string
 	Validate() error
 }
 
+// checkPrivilege verifies that p.user has `privilege` on `descriptor`.
+func (p *planner) checkPrivilege(descriptor descriptorProto, privilege structured.Privilege) error {
+	if descriptor.GetPrivileges().CheckPrivilege(p.user, privilege) {
+		return nil
+	}
+	return fmt.Errorf("user %s does not have %s privilege on %s %s",
+		p.user, privilege, descriptor.TypeName(), descriptor.GetName())
+}
+
 // writeDescriptor takes a Table or Database descriptor and writes it
 // if needed, incrementing the descriptor counter.
 func (p *planner) writeDescriptor(plainKey descriptorKey, descriptor descriptorProto, ifNotExists bool) error {

sql/drop.go

@@ -66,9 +66,8 @@ func (p *planner) DropTable(n *parser.DropTable) (planNode, error) {
 			return nil, err
 		}
 
-		if !tableDesc.HasPrivilege(p.user, parser.PrivilegeWrite) {
-			return nil, fmt.Errorf("user %s does not have %s privilege on table %s",
-				p.user, parser.PrivilegeWrite, tableDesc.Name)
+		if err := p.checkPrivilege(&tableDesc, structured.Privilege_WRITE); err != nil {
+			return nil, err
 		}
 
 		if _, err = p.Truncate(&parser.Truncate{Tables: n.Names[i : i+1]}); err != nil {
@@ -123,9 +122,8 @@ func (p *planner) DropDatabase(n *parser.DropDatabase) (planNode, error) {
 		return nil, err
 	}
 
-	if !desc.HasPrivilege(p.user, parser.PrivilegeWrite) {
-		return nil, fmt.Errorf("user %s does not have %s privilege on database %s",
-			p.user, parser.PrivilegeWrite, desc.Name)
+	if err := p.checkPrivilege(&desc, structured.Privilege_WRITE); err != nil {
+		return nil, err
 	}
 
 	tbNames, err := p.getTableNames(&desc)

sql/grant.go

@@ -18,8 +18,6 @@
 package sql
 
 import (
-	"fmt"
-
 	"github.com/cockroachdb/cockroach/sql/parser"
 	"github.com/cockroachdb/cockroach/structured"
 )
@@ -40,12 +38,15 @@ func (p *planner) Grant(n *parser.Grant) (planNode, error) {
 		return nil, err
 	}
 
-	if !descriptor.HasPrivilege(p.user, parser.PrivilegeWrite) {
-		return nil, fmt.Errorf("user %s does not have %s privilege on %s %s",
-			p.user, parser.PrivilegeWrite, descriptor.TypeName(), descriptor.GetName())
+	if err := p.checkPrivilege(descriptor, structured.Privilege_WRITE); err != nil {
+		return nil, err
+	}
+
+	for _, grantee := range n.Grantees {
+		descriptor.GetPrivileges().Grant(grantee, n.Privileges)
 	}
 
-	if err := descriptor.Grant(n); err != nil {
+	if err := descriptor.Validate(); err != nil {
 		return nil, err
 	}
 

sql/insert.go

@@ -38,9 +38,8 @@ func (p *planner) Insert(n *parser.Insert) (planNode, error) {
 		return nil, err
 	}
 
-	if !tableDesc.HasPrivilege(p.user, parser.PrivilegeWrite) {
-		return nil, fmt.Errorf("user %s does not have %s privilege on table %s",
-			p.user, parser.PrivilegeWrite, tableDesc.Name)
+	if err := p.checkPrivilege(tableDesc, structured.Privilege_WRITE); err != nil {
+		return nil, err
 	}
 
 	// Determine which columns we're inserting into.

sql/parser/grant.go

@@ -24,13 +24,14 @@
 package parser
 
 import (
-	"bytes"
 	"fmt"
+
+	"github.com/cockroachdb/cockroach/structured"
 )
 
 // Grant represents a GRANT statement.
 type Grant struct {
-	Privileges PrivilegeList
+	Privileges structured.PrivilegeList
 	Targets    TargetList
 	Grantees   NameList
 }
@@ -42,53 +43,15 @@ func (tt TargetType) String() string {
 	return targetNames[tt]
 }
 
-// PrivilegeType represents the type of privilege.
-type PrivilegeType int
-
-func (pt PrivilegeType) String() string {
-	return privilegeNames[pt]
-}
-
-// PrivilegeList represents a list of privileges.
-type PrivilegeList []PrivilegeType
-
-func (pl PrivilegeList) String() string {
-	return pl.Join(", ")
-}
-
-// Join returns the list of privilege names joined using `sep`.
-// The default stringer uses `, `, but we want to be able to
-// get a tight representation of the string.
-func (pl PrivilegeList) Join(sep string) string {
-	var buf bytes.Buffer
-	for i, n := range pl {
-		if i > 0 {
-			_, _ = buf.WriteString(sep)
-		}
-		buf.WriteString(n.String())
-	}
-	return buf.String()
-}
-
 // Enums for target and privilege types.
 const (
 	TargetDatabase TargetType = iota
-
-	PrivilegeAll PrivilegeType = iota
-	PrivilegeRead
-	PrivilegeWrite
 )
 
 var (
 	targetNames = [...]string{
 		TargetDatabase: "DATABASE",
 	}
-
-	privilegeNames = [...]string{
-		PrivilegeAll:   "ALL",
-		PrivilegeRead:  "READ",
-		PrivilegeWrite: "WRITE",
-	}
 )
 
 // TargetList represents a list of targets.

sql/parser/revoke.go

@@ -23,12 +23,16 @@
 
 package parser
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/cockroachdb/cockroach/structured"
+)
 
 // Revoke represents a REVOKE statements.
 // PrivilegeList and TargetList are defined in grant.go
 type Revoke struct {
-	Privileges PrivilegeList
+	Privileges structured.PrivilegeList
 	Targets    TargetList
 	Grantees   NameList
 }