Create system.users table.

kv/dist_sender_server_test.go

@@ -258,10 +258,11 @@ func TestSingleRangeReverseScan(t *testing.T) {
 		t.Errorf("expected 2 rows; got %d", l)
 	}
 	// Case 3: Test proto.KeyMax
+	// TODO(marc): this depends on the sql system objects.
 	if rows, err := db.ReverseScan("g", proto.KeyMax, 0); err != nil {
 		t.Fatalf("unexpected error on ReverseScan: %s", err)
-	} else if l := len(rows); l != 8 {
-		t.Errorf("expected 2 rows; got %d", l)
+	} else if l := len(rows); l != 10 {
+		t.Errorf("expected 10 rows; got %d", l)
 	}
 	// Case 4: Test keys.SystemMax
 	if rows, err := db.ReverseScan(keys.SystemMax, "b", 0); err != nil {

server/node_test.go

@@ -121,6 +121,7 @@ func TestBootstrapCluster(t *testing.T) {
 	for _, kv := range rows {
 		keys = append(keys, kv.Key)
 	}
+	// TODO(marc): this depends on the sql system objects.
 	var expectedKeys = []proto.Key{
 		proto.MakeKey(proto.Key("\x00\x00meta1"), proto.KeyMax),
 		proto.MakeKey(proto.Key("\x00\x00meta2"), proto.KeyMax),
@@ -133,9 +134,11 @@ func TestBootstrapCluster(t *testing.T) {
 		proto.Key("\xff\n\x02\n\x01\tsystem\x00\x01\n\x03"),
 		proto.Key("\xff\n\x02\n\x01\n\x01descriptor\x00\x01\n\x03"),
 		proto.Key("\xff\n\x02\n\x01\n\x01namespace\x00\x01\n\x03"),
+		proto.Key("\xff\n\x02\n\x01\n\x01users\x00\x01\n\x03"),
 		proto.Key("\xff\n\x03\n\x01\n\x01\n\x02"),
 		proto.Key("\xff\n\x03\n\x01\n\x02\n\x02"),
 		proto.Key("\xff\n\x03\n\x01\n\x03\n\x02"),
+		proto.Key("\xff\n\x03\n\x01\n\x04\n\x02"),
 	}
 	if !reflect.DeepEqual(keys, expectedKeys) {
 		t.Errorf("expected keys mismatch:\n%s\n  -- vs. -- \n\n%s",

sql/create_test.go

@@ -72,7 +72,8 @@ func TestDatabaseDescriptor(t *testing.T) {
 	if kvs, err := kvDB.Scan(start, start.PrefixEnd(), 0); err != nil {
 		t.Fatal(err)
 	} else {
-		if a, e := len(kvs), 3; a != e {
+		// TODO(marc): this is the number of system tables + 1.
+		if a, e := len(kvs), 4; a != e {
 			t.Fatalf("expected %d keys to have been written, found %d keys", e, a)
 		}
 	}

sql/database.go

@@ -38,7 +38,7 @@ func (dk databaseKey) Name() string {
 func makeDatabaseDesc(p *parser.CreateDatabase) DatabaseDescriptor {
 	return DatabaseDescriptor{
 		Name:       p.Name.String(),
-		Privileges: NewDefaultDatabasePrivilegeDescriptor(),
+		Privileges: NewDefaultPrivilegeDescriptor(),
 	}
 }
 

sql/privilege.go

@@ -98,22 +98,22 @@ func (p *PrivilegeDescriptor) removeUser(user string) {
 	p.Users = p.Users[:len(p.Users)-1]
 }
 
-// NewSystemObjectPrivilegeDescriptor returns a privilege descriptor for system
-// databases and tables.
-func NewSystemObjectPrivilegeDescriptor() *PrivilegeDescriptor {
+// NewPrivilegeDescriptor returns a privilege descriptor for the given
+// user with the specified list of privileges.
+func NewPrivilegeDescriptor(user string, priv privilege.List) *PrivilegeDescriptor {
 	return &PrivilegeDescriptor{
 		Users: []*UserPrivileges{
 			{
-				User:       security.RootUser,
-				Privileges: privilege.SELECT.Mask() | privilege.GRANT.Mask(),
+				User:       user,
+				Privileges: priv.ToBitField(),
 			},
 		},
 	}
 }
 
-// NewDefaultDatabasePrivilegeDescriptor returns a privilege descriptor
+// NewDefaultPrivilegeDescriptor returns a privilege descriptor
 // with ALL privileges for the root user.
-func NewDefaultDatabasePrivilegeDescriptor() *PrivilegeDescriptor {
+func NewDefaultPrivilegeDescriptor() *PrivilegeDescriptor {
 	return &PrivilegeDescriptor{
 		Users: []*UserPrivileges{
 			{
@@ -181,21 +181,27 @@ func (p *PrivilegeDescriptor) Revoke(user string, privList privilege.List) {
 	}
 }
 
-// Validate is called when writing a descriptor.
-// If 'isSystem' is true, perform validation for system
-// database and tables.
-func (p *PrivilegeDescriptor) Validate(isSystem bool) error {
+// Validate is called when writing a database or table descriptor.
+// It takes the descriptor ID which is used to determine if
+// it belongs to a system descriptor, in which case the maximum
+// set of allowed privileges is looked up and applied.
+func (p *PrivilegeDescriptor) Validate(id ID) error {
 	if p == nil {
 		return fmt.Errorf("missing privilege descriptor")
 	}
 	userPriv, ok := p.findUser(security.RootUser)
 	if !ok {
 		return fmt.Errorf("user %s does not have privileges", security.RootUser)
 	}
-	if isSystem {
-		// System databases and tables must have read-only permissions
-		// only (SELECT and GRANT). These must remain set for the root user.
-		allowedPrivileges := privilege.SELECT.Mask() | privilege.GRANT.Mask()
+	if IsSystemID(id) {
+		// System databases and tables have custom maximum allowed privileges.
+		objectPrivileges, ok := SystemAllowedPrivileges[id]
+		if !ok {
+			return fmt.Errorf("no allowed privileges found for system object with ID=%d", id)
+		}
+
+		// The root user must have all the allowed privileges.
+		allowedPrivileges := objectPrivileges.ToBitField()
 		if userPriv.Privileges&allowedPrivileges != allowedPrivileges {
 			return fmt.Errorf("user %s must have %s privileges on system objects",
 				security.RootUser, privilege.ListFromBitField(allowedPrivileges))

sql/privilege/privilege.go

@@ -42,6 +42,13 @@ const (
 	UPDATE
 )
 
+// Predefined sets of privileges.
+var (
+	ReadData      = List{GRANT, SELECT}
+	WriteData     = List{INSERT, DELETE, UPDATE}
+	ReadWriteData = List{GRANT, SELECT, INSERT, DELETE, UPDATE}
+)
+
 // Mask returns the bitmask for a given privilege.
 func (k Kind) Mask() uint32 {
 	return 1 << k

sql/privilege_test.go

@@ -20,46 +20,47 @@ package sql_test
 import (
 	"testing"
 
+	"github.com/cockroachdb/cockroach/security"
 	"github.com/cockroachdb/cockroach/sql"
 	"github.com/cockroachdb/cockroach/sql/privilege"
 	"github.com/cockroachdb/cockroach/util/leaktest"
 )
 
 func TestPrivilege(t *testing.T) {
 	defer leaktest.AfterTest(t)
-	descriptor := sql.NewDefaultDatabasePrivilegeDescriptor()
+	descriptor := sql.NewDefaultPrivilegeDescriptor()
 
 	testCases := []struct {
 		grantee       string // User to grant/revoke privileges on.
 		grant, revoke privilege.List
 		show          []sql.UserPrivilegeString
 	}{
 		{"", nil, nil,
-			[]sql.UserPrivilegeString{{"root", "ALL"}},
+			[]sql.UserPrivilegeString{{security.RootUser, "ALL"}},
 		},
-		{"root", privilege.List{privilege.ALL}, nil,
-			[]sql.UserPrivilegeString{{"root", "ALL"}},
+		{security.RootUser, privilege.List{privilege.ALL}, nil,
+			[]sql.UserPrivilegeString{{security.RootUser, "ALL"}},
 		},
-		{"root", privilege.List{privilege.INSERT, privilege.DROP}, nil,
-			[]sql.UserPrivilegeString{{"root", "ALL"}},
+		{security.RootUser, privilege.List{privilege.INSERT, privilege.DROP}, nil,
+			[]sql.UserPrivilegeString{{security.RootUser, "ALL"}},
 		},
 		{"foo", privilege.List{privilege.INSERT, privilege.DROP}, nil,
-			[]sql.UserPrivilegeString{{"foo", "DROP,INSERT"}, {"root", "ALL"}},
+			[]sql.UserPrivilegeString{{"foo", "DROP,INSERT"}, {security.RootUser, "ALL"}},
 		},
 		{"bar", nil, privilege.List{privilege.INSERT, privilege.ALL},
-			[]sql.UserPrivilegeString{{"foo", "DROP,INSERT"}, {"root", "ALL"}},
+			[]sql.UserPrivilegeString{{"foo", "DROP,INSERT"}, {security.RootUser, "ALL"}},
 		},
 		{"foo", privilege.List{privilege.ALL}, nil,
-			[]sql.UserPrivilegeString{{"foo", "ALL"}, {"root", "ALL"}},
+			[]sql.UserPrivilegeString{{"foo", "ALL"}, {security.RootUser, "ALL"}},
 		},
 		{"foo", nil, privilege.List{privilege.SELECT, privilege.INSERT},
-			[]sql.UserPrivilegeString{{"foo", "CREATE,DELETE,DROP,GRANT,UPDATE"}, {"root", "ALL"}},
+			[]sql.UserPrivilegeString{{"foo", "CREATE,DELETE,DROP,GRANT,UPDATE"}, {security.RootUser, "ALL"}},
 		},
 		{"foo", nil, privilege.List{privilege.ALL},
-			[]sql.UserPrivilegeString{{"root", "ALL"}},
+			[]sql.UserPrivilegeString{{security.RootUser, "ALL"}},
 		},
 		// Validate checks that root still has ALL privileges, but we do not call it here.
-		{"root", nil, privilege.List{privilege.ALL},
+		{security.RootUser, nil, privilege.List{privilege.ALL},
 			[]sql.UserPrivilegeString{},
 		},
 	}
@@ -90,60 +91,119 @@ func TestPrivilege(t *testing.T) {
 	}
 }
 
+// TestPrivilegeValidate exercises validation for non-system descriptors.
 func TestPrivilegeValidate(t *testing.T) {
 	defer leaktest.AfterTest(t)
-	descriptor := sql.NewDefaultDatabasePrivilegeDescriptor()
-	if err := descriptor.Validate(false); err != nil {
+	id := sql.MaxReservedDescID + 1
+	descriptor := sql.NewDefaultPrivilegeDescriptor()
+	if err := descriptor.Validate(id); err != nil {
 		t.Fatal(err)
 	}
 	descriptor.Grant("foo", privilege.List{privilege.ALL})
-	if err := descriptor.Validate(false); err != nil {
+	if err := descriptor.Validate(id); err != nil {
 		t.Fatal(err)
 	}
-	descriptor.Grant("root", privilege.List{privilege.SELECT})
-	if err := descriptor.Validate(false); err != nil {
+	descriptor.Grant(security.RootUser, privilege.List{privilege.SELECT})
+	if err := descriptor.Validate(id); err != nil {
 		t.Fatal(err)
 	}
-	descriptor.Revoke("root", privilege.List{privilege.SELECT})
-	if err := descriptor.Validate(false); err == nil {
+	descriptor.Revoke(security.RootUser, privilege.List{privilege.SELECT})
+	if err := descriptor.Validate(id); err == nil {
 		t.Fatal("unexpected success")
 	}
 	// TODO(marc): validate fails here because we do not aggregate
 	// privileges into ALL when all are set.
-	descriptor.Grant("root", privilege.List{privilege.SELECT})
-	if err := descriptor.Validate(false); err == nil {
+	descriptor.Grant(security.RootUser, privilege.List{privilege.SELECT})
+	if err := descriptor.Validate(id); err == nil {
 		t.Fatal("unexpected success")
 	}
-	descriptor.Revoke("root", privilege.List{privilege.ALL})
-	if err := descriptor.Validate(false); err == nil {
+	descriptor.Revoke(security.RootUser, privilege.List{privilege.ALL})
+	if err := descriptor.Validate(id); err == nil {
 		t.Fatal("unexpected success")
 	}
 }
 
+// TestSystemPrivilegeValidate exercises validation for system descriptors.
+// We use 1 (the system database ID).
 func TestSystemPrivilegeValidate(t *testing.T) {
 	defer leaktest.AfterTest(t)
-	descriptor := sql.NewSystemObjectPrivilegeDescriptor()
-	if err := descriptor.Validate(true); err != nil {
-		t.Fatal(err)
-	}
-	descriptor.Grant("foo", privilege.List{privilege.SELECT, privilege.GRANT})
-	if err := descriptor.Validate(true); err != nil {
-		t.Fatal(err)
+	id := sql.ID(1)
+	allowedPrivileges := sql.SystemAllowedPrivileges[id]
+
+	hasPrivilege := func(pl privilege.List, p privilege.Kind) bool {
+		for _, i := range pl {
+			if i == p {
+				return true
+			}
+		}
+		return false
 	}
-	descriptor.Grant("root", privilege.List{privilege.SELECT})
-	if err := descriptor.Validate(true); err != nil {
+
+	// Exhaustively grant/revoke all privileges.
+	// Due to the way validation is done after Grant/Revoke,
+	// we need to revert the just-performed change after errors.
+	descriptor := sql.NewPrivilegeDescriptor(security.RootUser, allowedPrivileges)
+	if err := descriptor.Validate(id); err != nil {
 		t.Fatal(err)
 	}
-	descriptor.Revoke("root", privilege.List{privilege.SELECT})
-	if err := descriptor.Validate(true); err == nil {
-		t.Fatal("unexpected success")
-	}
-	descriptor.Grant("root", privilege.List{privilege.INSERT})
-	if err := descriptor.Validate(true); err == nil {
-		t.Fatal("unexpected success")
-	}
-	descriptor.Revoke("root", privilege.List{privilege.ALL})
-	if err := descriptor.Validate(true); err == nil {
-		t.Fatal("unexpected success")
+	for _, p := range privilege.ByValue {
+		if hasPrivilege(allowedPrivileges, p) {
+			// Grant allowed privileges. Either they are already
+			// on (noop), or they're accepted.
+			descriptor.Grant(security.RootUser, privilege.List{p})
+			if err := descriptor.Validate(id); err != nil {
+				t.Fatal(err)
+			}
+			descriptor.Grant("foo", privilege.List{p})
+			if err := descriptor.Validate(id); err != nil {
+				t.Fatal(err)
+			}
+
+			// Remove allowed privileges. This fails for root,
+			// but passes for other users.
+			descriptor.Revoke(security.RootUser, privilege.List{p})
+			if err := descriptor.Validate(id); err == nil {
+				t.Fatal("unexpected success")
+			}
+			descriptor.Grant(security.RootUser, privilege.List{p})
+		} else {
+			// Granting non-allowed privileges always.
+			descriptor.Grant(security.RootUser, privilege.List{p})
+			if err := descriptor.Validate(id); err == nil {
+				t.Fatal("unexpected success")
+			}
+			descriptor.Revoke(security.RootUser, privilege.List{p})
+			descriptor.Grant(security.RootUser, allowedPrivileges)
+
+			descriptor.Grant("foo", privilege.List{p})
+			if err := descriptor.Validate(id); err == nil {
+				t.Fatal("unexpected success")
+			}
+			descriptor.Revoke("foo", privilege.List{p})
+			descriptor.Grant("foo", allowedPrivileges)
+
+			// Revoking non-allowed privileges always succeeds,
+			// except when removing ALL for root.
+			if p == privilege.ALL {
+				// We need to reset privileges as Revoke(ALL) will clear
+				// all bits.
+				descriptor.Revoke(security.RootUser, privilege.List{p})
+				if err := descriptor.Validate(id); err == nil {
+					t.Fatal("unexpected success")
+				}
+				descriptor.Grant(security.RootUser, allowedPrivileges)
+			} else {
+				descriptor.Revoke(security.RootUser, privilege.List{p})
+				if err := descriptor.Validate(id); err != nil {
+					t.Fatal(err)
+				}
+			}
+		}
+
+		// We can always revoke anything from non-root users.
+		descriptor.Revoke("foo", privilege.List{p})
+		if err := descriptor.Validate(id); err != nil {
+			t.Fatal(err)
+		}
 	}
 }

sql/structured.go

@@ -256,7 +256,7 @@ func (desc *TableDescriptor) Validate() error {
 		}
 	}
 	// Validate the privilege descriptor.
-	return desc.Privileges.Validate(IsSystemID(desc.GetID()))
+	return desc.Privileges.Validate(desc.GetID())
 }
 
 // FindColumnByName finds the column with specified name.
@@ -337,5 +337,5 @@ func (desc *DatabaseDescriptor) Validate() error {
 		return fmt.Errorf("invalid database ID 0")
 	}
 	// Validate the privilege descriptor.
-	return desc.Privileges.Validate(IsSystemID(desc.GetID()))
+	return desc.Privileges.Validate(desc.GetID())
 }

sql/structured_test.go

@@ -42,7 +42,7 @@ func TestAllocateIDs(t *testing.T) {
 			{Name: "d", ColumnNames: []string{"b", "a"}},
 			{Name: "e", ColumnNames: []string{"b"}},
 		},
-		Privileges: sql.NewDefaultDatabasePrivilegeDescriptor(),
+		Privileges: sql.NewDefaultPrivilegeDescriptor(),
 	}
 	if err := desc.AllocateIDs(); err != nil {
 		t.Fatal(err)
@@ -63,7 +63,7 @@ func TestAllocateIDs(t *testing.T) {
 			{ID: 3, Name: "e", ColumnIDs: []sql.ColumnID{2}, ColumnNames: []string{"b"},
 				ImplicitColumnIDs: []sql.ColumnID{1}},
 		},
-		Privileges:   sql.NewDefaultDatabasePrivilegeDescriptor(),
+		Privileges:   sql.NewDefaultPrivilegeDescriptor(),
 		NextColumnID: 4,
 		NextIndexID:  4,
 	}