syncbase: Enforce ACL spec on non-sync RPCs.
Non-sync RPCs now check permissions according to the Syncbase ACL
specification document, including recursively checking for Resolve
access.
Expanded tests to cover most updated RPCs.
MultiPart: 2/2
Change-Id: Ie6432c4dfc93ff8a0b26cf6573b0950f00db931d
diff --git a/services/debug/debug/browseserver/sbtree/colltree_test.go b/services/debug/debug/browseserver/sbtree/colltree_test.go
index 55c8793..5e5046d 100644
--- a/services/debug/debug/browseserver/sbtree/colltree_test.go
+++ b/services/debug/debug/browseserver/sbtree/colltree_test.go
@@ -20,7 +20,7 @@
defer cleanup()
var (
service = syncbase.NewService(serverName)
- db = tu.CreateDatabase(t, ctx, service, "the_db")
+ db = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, db, "the_collection")
)
@@ -67,7 +67,7 @@
defer cleanup()
var (
service = syncbase.NewService(serverName)
- db = tu.CreateDatabase(t, ctx, service, "the_db")
+ db = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, db, "the_collection")
wantTotKeySize = len("Bravo") + len("Alfa") + len("Delta") + len("Charlie")
)
@@ -129,7 +129,7 @@
defer cleanup()
var (
service = syncbase.NewService(serverName)
- db = tu.CreateDatabase(t, ctx, service, "the_db")
+ db = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, db, "the_collection")
)
// Ten keys, in pages of four, starting with the first key
@@ -187,7 +187,7 @@
defer cleanup()
var (
service = syncbase.NewService(serverName)
- db = tu.CreateDatabase(t, ctx, service, "the_db")
+ db = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, db, "the_collection")
)
// Ten keys, in pages of four, starting with fifth key
@@ -245,7 +245,7 @@
defer cleanup()
var (
service = syncbase.NewService(serverName)
- db = tu.CreateDatabase(t, ctx, service, "the_db")
+ db = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, db, "the_collection")
)
// Ten keys, in pages of four, starting with ninth key.
@@ -304,7 +304,7 @@
defer cleanup()
var (
service = syncbase.NewService(serverName)
- db = tu.CreateDatabase(t, ctx, service, "the_db")
+ db = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, db, "the_collection")
)
type childType struct {
diff --git a/services/debug/debug/browseserver/sbtree/sbtree_test.go b/services/debug/debug/browseserver/sbtree/sbtree_test.go
index 1d23928..46d2274 100644
--- a/services/debug/debug/browseserver/sbtree/sbtree_test.go
+++ b/services/debug/debug/browseserver/sbtree/sbtree_test.go
@@ -43,7 +43,7 @@
dbNames = []string{"db_a", "db_b", "db_c"}
)
for _, dbName := range dbNames {
- tu.CreateDatabase(t, ctx, service, dbName)
+ tu.CreateDatabase(t, ctx, service, dbName, nil)
}
dbIds, err := service.ListDatabases(ctx)
if err != nil {
@@ -80,7 +80,7 @@
var (
service = syncbase.NewService(serverName)
collNames = []string{"coll_a", "coll_b", "coll_c"}
- database = tu.CreateDatabase(t, ctx, service, "the_db")
+ database = tu.CreateDatabase(t, ctx, service, "the_db", nil)
)
for _, collName := range collNames {
tu.CreateCollection(t, ctx, database, collName)
@@ -119,7 +119,7 @@
service = syncbase.NewService(serverName)
sgNames = []string{"syncgroup_a", "syncgroup_b", "syncgroup_c"}
sgDescriptions = []string{"AAA", "BBB", "CCC"}
- database = tu.CreateDatabase(t, ctx, service, "the_db")
+ database = tu.CreateDatabase(t, ctx, service, "the_db", nil)
coll = tu.CreateCollection(t, ctx, database, "the_collection")
)
for i, sgName := range sgNames {
diff --git a/services/syncbase/common/access_util.go b/services/syncbase/common/access_util.go
index 2b8cf96..2bc7f1c 100644
--- a/services/syncbase/common/access_util.go
+++ b/services/syncbase/common/access_util.go
@@ -99,7 +99,10 @@
}
// Permser is an object in the hierarchy that supports retrieving perms and
-// authorizing access to existence checks.
+// authorizing access to existence checks. Access checks on Permser objects
+// using Get{Data,Perms}With*Auth functions below should be done in the same
+// transaction as any store modification to ensure that concurrent ACL changes
+// invalidate the modification.
type Permser interface {
// GetDataWithExistAuth must return a nil error only if the object exists and
// the caller is authorized to know it (Resolve access up to the parent and
diff --git a/services/syncbase/discovery/discovery_test.go b/services/syncbase/discovery/discovery_test.go
index ec2180d..68a977e 100644
--- a/services/syncbase/discovery/discovery_test.go
+++ b/services/syncbase/discovery/discovery_test.go
@@ -35,7 +35,7 @@
_, ctx, sName, rootp, cleanup := tu.SetupOrDieCustom("o:app1:client1", "server",
tu.DefaultPerms(access.AllTypicalTags(), "root:o:app1:client1"))
defer cleanup()
- d := tu.CreateDatabase(t, ctx, syncbase.NewService(sName), "d")
+ d := tu.CreateDatabase(t, ctx, syncbase.NewService(sName), "d", nil)
collection1 := tu.CreateCollection(t, ctx, d, "c1")
collection2 := tu.CreateCollection(t, ctx, d, "c2")
@@ -284,8 +284,8 @@
tu.DefaultPerms(access.AllTypicalTags(), "root:o:app1:client1"))
defer cleanup()
service := syncbase.NewService(sName)
- d1 := tu.CreateDatabase(t, ctx, service, "d1")
- d2 := tu.CreateDatabase(t, ctx, service, "d2")
+ d1 := tu.CreateDatabase(t, ctx, service, "d1", nil)
+ d2 := tu.CreateDatabase(t, ctx, service, "d2", nil)
collections := []wire.Id{}
for _, d := range []syncbase.Database{d1, d2} {
diff --git a/services/syncbase/server/collection.go b/services/syncbase/server/collection.go
index d497c97..5e27d1f 100644
--- a/services/syncbase/server/collection.go
+++ b/services/syncbase/server/collection.go
@@ -33,14 +33,15 @@
// RPC methods
func (c *collectionReq) Create(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle, perms access.Permissions) error {
+ allowCreate := []access.Tag{access.Write}
+
if err := common.ValidatePerms(ctx, perms, wire.AllCollectionTags); err != nil {
return err
}
impl := func(ts *transactionState) error {
tx := ts.tx
- // Check DatabaseData perms.
- dData := &DatabaseData{}
- if err := util.GetWithAuth(ctx, call, tx, c.d.stKey(), dData); err != nil {
+ // Check Database perms.
+ if _, err := common.GetPermsWithAuth(ctx, call, c.d, allowCreate, tx); err != nil {
return err
}
// Check implicit perms derived from blessing pattern in id.
@@ -70,14 +71,24 @@
// collection data deletion to be deferred, making deletion faster (reference
// removal).
func (c *collectionReq) Destroy(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) error {
+ allowDestroy := []access.Tag{access.Admin}
+
impl := func(ts *transactionState) error {
tx := ts.tx
- // Read CollectionPerms.
- if err := util.GetWithAuth(ctx, call, tx, c.permsKey(), &interfaces.CollectionPerms{}); err != nil {
- if verror.ErrorID(err) == verror.ErrNoExist.ID {
+ var authErr error
+ // Check permissions on Database.
+ if _, authErr = common.GetPermsWithAuth(ctx, call, c.d, allowDestroy, tx); authErr != nil {
+ // Caller has no Admin access on Database. Check CollectionPerms.
+ _, authErr = common.GetPermsWithAuth(ctx, call, c, allowDestroy, tx)
+ } else {
+ // Caller has Admin access on Database. Check if Collection exists.
+ authErr = store.Get(ctx, tx, c.permsKey(), &interfaces.CollectionPerms{})
+ }
+ if authErr != nil {
+ if verror.ErrorID(authErr) == verror.ErrNoExist.ID {
return nil // delete is idempotent
}
- return err
+ return authErr
}
// TODO(ivanpi): Check that no syncgroup includes the collection being
@@ -113,24 +124,29 @@
return common.ErrorToExists(c.d.runWithExistingBatchOrNewSnapshot(ctx, bh, impl))
}
-func (c *collectionReq) GetPermissions(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) (perms access.Permissions, err error) {
- var res interfaces.CollectionPerms
- impl := func(sntx store.SnapshotOrTransaction) error {
- return util.GetWithAuth(ctx, call, sntx, c.permsKey(), &res)
+func (c *collectionReq) GetPermissions(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) (access.Permissions, error) {
+ allowGetPermissions := []access.Tag{access.Admin}
+
+ var perms access.Permissions
+ impl := func(sntx store.SnapshotOrTransaction) (err error) {
+ perms, err = common.GetPermsWithAuth(ctx, call, c, allowGetPermissions, sntx)
+ return err
}
if err := c.d.runWithExistingBatchOrNewSnapshot(ctx, bh, impl); err != nil {
return nil, err
}
- return access.Permissions(res), nil
+ return perms, nil
}
func (c *collectionReq) SetPermissions(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle, newPerms access.Permissions) error {
+ allowSetPermissions := []access.Tag{access.Admin}
+
if err := common.ValidatePerms(ctx, newPerms, wire.AllCollectionTags); err != nil {
return err
}
impl := func(ts *transactionState) error {
tx := ts.tx
- currentPerms, err := c.checkAccess(ctx, call, tx)
+ currentPerms, err := common.GetPermsWithAuth(ctx, call, c, allowSetPermissions, tx)
if err != nil {
return err
}
@@ -142,10 +158,12 @@
}
func (c *collectionReq) DeleteRange(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle, start, limit []byte) error {
+ allowDeleteRange := []access.Tag{access.Write}
+
impl := func(ts *transactionState) error {
tx := ts.tx
// Check for collection-level access before doing a scan.
- currentPerms, err := c.checkAccess(ctx, call, tx)
+ currentPerms, err := common.GetPermsWithAuth(ctx, call, c, allowDeleteRange, tx)
if err != nil {
return err
}
@@ -168,9 +186,11 @@
}
func (c *collectionReq) Scan(ctx *context.T, call wire.CollectionScanServerCall, bh wire.BatchHandle, start, limit []byte) error {
+ allowScan := []access.Tag{access.Read}
+
impl := func(sntx store.SnapshotOrTransaction) error {
// Check for collection-level access before doing a scan.
- if _, err := c.checkAccess(ctx, call, sntx); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, c, allowScan, sntx); err != nil {
return err
}
it := sntx.Scan(common.ScanRangeArgs(common.JoinKeyParts(common.RowPrefix, c.stKeyPart()), string(start), string(limit)))
@@ -201,9 +221,11 @@
}
func (c *collectionReq) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
+ allowGlob := []access.Tag{access.Read}
+
impl := func(sntx store.SnapshotOrTransaction) error {
// Check perms.
- if _, err := c.checkAccess(ctx, call, sntx); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, c, allowGlob, sntx); err != nil {
return err
}
return util.GlobChildren(ctx, call, matcher, sntx, common.JoinKeyParts(common.RowPrefix, c.stKeyPart()))
@@ -243,19 +265,3 @@
func (c *collectionReq) stKeyPart() string {
return pubutil.EncodeId(c.id)
}
-
-// checkAccess checks that this collection exists in the database, and performs
-// an authorization check on the collection ACL. It should be called in the same
-// transaction as any store modification to ensure that concurrent ACL changes
-// invalidate the modification.
-// TODO(rogulenko): Revisit this behavior. Eventually we'll want the
-// collection-level access check to be a check for "Resolve", i.e. also check
-// access to service and database.
-// TODO(ivanpi): Remove once all callers are ported to explicit auth.
-func (c *collectionReq) checkAccess(ctx *context.T, call rpc.ServerCall, sntx store.SnapshotOrTransaction) (access.Permissions, error) {
- collectionPerms := &interfaces.CollectionPerms{}
- if err := util.GetWithAuth(ctx, call, sntx, c.permsKey(), collectionPerms); err != nil {
- return nil, err
- }
- return collectionPerms.GetPerms(), nil
-}
diff --git a/services/syncbase/server/database.go b/services/syncbase/server/database.go
index 16ec31c..6ab1d86 100644
--- a/services/syncbase/server/database.go
+++ b/services/syncbase/server/database.go
@@ -126,6 +126,7 @@
// permission change and returns false if any of the auth checks fail.
// TODO(ivanpi): This check should be done against signing blessings at signing time, in
// both batch and non-batch cases.
+// Note, service and database ACLs are not synced, so they don't need to be rechecked.
func (ts *transactionState) validatePermissionChanges(ctx *context.T, securityCall security.Call) bool {
for _, collectionState := range ts.permsChanges {
// This collection was modified, make sure that the write acl is either present at
@@ -166,9 +167,8 @@
// hasPermission returns true if the caller is authorized for the specific tag based on the
// passed in perms.
func hasPermission(ctx *context.T, securityCall security.Call, perms access.Permissions, tag access.Tag) bool {
- permForTag, ok := perms[string(tag)]
// Authorize returns either an error or nil, so nil means the caller is authorized.
- return ok && permForTag.Authorize(ctx, securityCall) == nil
+ return common.AnyOfTagsAuthorizer([]access.Tag{tag}, perms).Authorize(ctx, securityCall) == nil
}
// openDatabase opens a database and returns a *database for it. Designed for
@@ -228,6 +228,7 @@
// RPC methods
func (d *database) Create(ctx *context.T, call rpc.ServerCall, metadata *wire.SchemaMetadata, perms access.Permissions) error {
+ // Permissions checked in d.s.createDatabase.
if d.exists {
return verror.New(verror.ErrExist, ctx, d.id)
}
@@ -238,6 +239,7 @@
}
func (d *database) Destroy(ctx *context.T, call rpc.ServerCall) error {
+ // Permissions checked in d.s.destroyDatabase.
return d.s.destroyDatabase(ctx, call, d.id)
}
@@ -252,9 +254,14 @@
var rng *rand.Rand = rand.New(rand.NewSource(time.Now().UTC().UnixNano()))
func (d *database) BeginBatch(ctx *context.T, call rpc.ServerCall, opts wire.BatchOptions) (wire.BatchHandle, error) {
+ allowBeginBatch := wire.AllDatabaseTags
+
if !d.exists {
return "", verror.New(verror.ErrNoExist, ctx, d.id)
}
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowBeginBatch, d.st); err != nil {
+ return "", err
+ }
d.mu.Lock()
defer d.mu.Unlock()
var id uint64
@@ -279,12 +286,17 @@
}
func (d *database) Commit(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) error {
+ allowCommit := wire.AllDatabaseTags
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
if bh == "" {
return wire.NewErrNotBoundToBatch(ctx)
}
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowCommit, d.st); err != nil {
+ return err
+ }
_, ts, batchId, err := d.batchLookupInternal(ctx, bh)
if err != nil {
return err
@@ -309,12 +321,17 @@
}
func (d *database) Abort(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) error {
+ allowAbort := wire.AllDatabaseTags
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
if bh == "" {
return wire.NewErrNotBoundToBatch(ctx)
}
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowAbort, d.st); err != nil {
+ return err
+ }
sn, ts, batchId, err := d.batchLookupInternal(ctx, bh)
if err != nil {
return err
@@ -335,6 +352,7 @@
}
func (d *database) Exec(ctx *context.T, call wire.DatabaseExecServerCall, bh wire.BatchHandle, q string, params []*vom.RawBytes) error {
+ // Permissions checked in qdb.GetTable.
// RunInTransaction() cannot be used here because we may or may not be
// creating a transaction. qe.Exec must be called and the statement must be
// parsed before we know if a snapshot or a transaction should be created. To
@@ -417,6 +435,7 @@
}
func (d *database) SetPermissions(ctx *context.T, call rpc.ServerCall, perms access.Permissions, version string) error {
+ // Permissions checked in d.setPermsInternal.
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
@@ -424,23 +443,27 @@
}
func (d *database) GetPermissions(ctx *context.T, call rpc.ServerCall) (perms access.Permissions, version string, err error) {
+ allowGetPermissions := []access.Tag{access.Admin}
+
if !d.exists {
return nil, "", verror.New(verror.ErrNoExist, ctx, d.id)
}
- data := &DatabaseData{}
- if err := util.GetWithAuth(ctx, call, d.st, d.stKey(), data); err != nil {
+ var data DatabaseData
+ if _, err := common.GetDataWithAuth(ctx, call, d, allowGetPermissions, d.st, &data); err != nil {
return nil, "", err
}
return data.Perms, util.FormatVersion(data.Version), nil
}
func (d *database) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
+ allowGlob := []access.Tag{access.Read}
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
impl := func(sntx store.SnapshotOrTransaction) error {
// Check perms.
- if err := util.GetWithAuth(ctx, call, sntx, d.stKey(), &DatabaseData{}); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowGlob, sntx); err != nil {
return err
}
return util.GlobChildren(ctx, call, matcher, sntx, common.CollectionPermsPrefix)
@@ -451,13 +474,15 @@
// See comment in v.io/v23/services/syncbase/service.vdl for why we can't
// implement ListCollections using Glob.
func (d *database) ListCollections(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) ([]wire.Id, error) {
+ allowListCollections := []access.Tag{access.Read}
+
if !d.exists {
return nil, verror.New(verror.ErrNoExist, ctx, d.id)
}
var res []wire.Id
impl := func(sntx store.SnapshotOrTransaction) error {
// Check perms.
- if err := util.GetWithAuth(ctx, call, sntx, d.stKey(), &DatabaseData{}); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowListCollections, sntx); err != nil {
return err
}
it := sntx.Scan(common.ScanPrefixArgs(common.CollectionPermsPrefix, ""))
@@ -483,19 +508,31 @@
}
func (d *database) PauseSync(ctx *context.T, call rpc.ServerCall) error {
+ allowPauseSync := []access.Tag{access.Admin}
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
return d.runInTransaction(func(ts *transactionState) error {
+ // Check perms.
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowPauseSync, ts.tx); err != nil {
+ return err
+ }
return sbwatchable.AddDbStateChangeRequestOp(ctx, ts.tx, sbwatchable.StateChangePauseSync)
})
}
func (d *database) ResumeSync(ctx *context.T, call rpc.ServerCall) error {
+ allowResumeSync := []access.Tag{access.Admin}
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
return d.runInTransaction(func(ts *transactionState) error {
+ // Check perms.
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowResumeSync, ts.tx); err != nil {
+ return err
+ }
return sbwatchable.AddDbStateChangeRequestOp(ctx, ts.tx, sbwatchable.StateChangeResumeSync)
})
}
@@ -608,11 +645,16 @@
}
}
// Now that we have a collection, we need to check permissions.
- collectionPerms, err := qt.cReq.checkAccess(qdb.ctx, qdb.call, qdb.sntx)
+ // Always check for Read access.
+ collectionPerms, err := common.GetPermsWithAuth(qdb.ctx, qdb.call, qt.cReq, []access.Tag{access.Read}, qdb.sntx)
if err != nil {
return nil, err
}
if writeAccessReq {
+ // Also check for Write access if requested.
+ if _, err := common.GetPermsWithAuth(qdb.ctx, qdb.call, qt.cReq, []access.Tag{access.Write}, qdb.sntx); err != nil {
+ return nil, err
+ }
qt.qdb.ts.MarkDataChanged(qt.cReq.id, collectionPerms)
}
return qt, nil
@@ -768,37 +810,35 @@
}
func (d *database) runWithExistingBatchOrNewSnapshot(ctx *context.T, bh wire.BatchHandle, fn func(sntx store.SnapshotOrTransaction) error) error {
- if !d.exists {
- // TODO(ivanpi): Return fuzzy error if appropriate.
- return verror.New(verror.ErrNoExist, ctx, d.id)
- }
if bh != "" {
if sntx, err := d.batchReader(ctx, bh); err != nil {
// Batch does not exist.
- // TODO(ivanpi): Return fuzzy error if appropriate.
return err
} else {
return fn(sntx)
}
} else {
+ if !d.exists {
+ // TODO(ivanpi): Return fuzzy error if appropriate.
+ return verror.New(verror.ErrNoExist, ctx, d.id)
+ }
return store.RunWithSnapshot(d.st, fn)
}
}
func (d *database) runInExistingBatchOrNewTransaction(ctx *context.T, bh wire.BatchHandle, fn func(ts *transactionState) error) error {
- if !d.exists {
- // TODO(ivanpi): Return fuzzy error if appropriate.
- return verror.New(verror.ErrNoExist, ctx, d.id)
- }
if bh != "" {
if batch, err := d.batchTransaction(ctx, bh); err != nil {
// Batch does not exist or is readonly (snapshot).
- // TODO(ivanpi): Return fuzzy error if appropriate.
return err
} else {
return fn(batch)
}
} else {
+ if !d.exists {
+ // TODO(ivanpi): Return fuzzy error if appropriate.
+ return verror.New(verror.ErrNoExist, ctx, d.id)
+ }
return d.runInTransaction(fn)
}
}
@@ -836,6 +876,10 @@
if err != nil {
return nil, nil, 0, err
}
+ if !d.exists {
+ // TODO(ivanpi): Return fuzzy error if appropriate.
+ return nil, nil, 0, verror.New(verror.ErrNoExist, ctx, d.id)
+ }
d.mu.Lock()
defer d.mu.Unlock()
var found bool
@@ -852,6 +896,8 @@
}
func (d *database) setPermsInternal(ctx *context.T, call rpc.ServerCall, perms access.Permissions, version string) error {
+ allowSetPermissions := []access.Tag{access.Admin}
+
if !d.exists {
vlog.Fatalf("database %v does not exist", d.id)
}
@@ -859,15 +905,16 @@
return err
}
return store.RunInTransaction(d.st, func(tx store.Transaction) error {
- data := &DatabaseData{}
- return util.UpdateWithAuth(ctx, call, tx, d.stKey(), data, func() error {
- if err := util.CheckVersion(ctx, version, data.Version); err != nil {
- return err
- }
- data.Perms = perms
- data.Version++
- return nil
- })
+ var data DatabaseData
+ if _, err := common.GetDataWithAuth(ctx, call, d, allowSetPermissions, tx, &data); err != nil {
+ return err
+ }
+ if err := util.CheckVersion(ctx, version, data.Version); err != nil {
+ return err
+ }
+ data.Perms = perms
+ data.Version++
+ return store.Put(ctx, tx, d.stKey(), &data)
})
}
diff --git a/services/syncbase/server/database_bm.go b/services/syncbase/server/database_bm.go
index b18e1a7..a25a5a2 100644
--- a/services/syncbase/server/database_bm.go
+++ b/services/syncbase/server/database_bm.go
@@ -15,6 +15,8 @@
////////////////////////////////////////////////////////////////////////////////
// RPCs for managing blobs between Syncbase and its clients.
+// Note, access authorization is checked in SyncDatabase methods.
+
func (d *database) CreateBlob(ctx *context.T, call rpc.ServerCall) (wire.BlobRef, error) {
if !d.exists {
return wire.NullBlobRef, verror.New(verror.ErrNoExist, ctx, d.id)
diff --git a/services/syncbase/server/database_crm.go b/services/syncbase/server/database_crm.go
index 62993d1..d3c4d19 100644
--- a/services/syncbase/server/database_crm.go
+++ b/services/syncbase/server/database_crm.go
@@ -6,18 +6,26 @@
import (
"v.io/v23/context"
+ "v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
"v.io/v23/verror"
"v.io/x/lib/vlog"
+ "v.io/x/ref/services/syncbase/common"
)
////////////////////////////////////////
// ConflictManager RPC methods
func (d *database) StartConflictResolver(ctx *context.T, call wire.ConflictManagerStartConflictResolverServerCall) error {
+ allowStartConflictResolver := []access.Tag{access.Admin}
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowStartConflictResolver, d.st); err != nil {
+ return err
+ }
// Store the conflict resolver connection in the per-database singleton
// so that sync can access it.
diff --git a/services/syncbase/server/database_sgm.go b/services/syncbase/server/database_sgm.go
index f6a0af8..73605c2 100644
--- a/services/syncbase/server/database_sgm.go
+++ b/services/syncbase/server/database_sgm.go
@@ -15,6 +15,8 @@
////////////////////////////////////////
// Syncgroup RPC methods
+// Note, access authorization is checked in SyncDatabase methods.
+
func (d *database) ListSyncgroups(ctx *context.T, call rpc.ServerCall) ([]wire.Id, error) {
if !d.exists {
return nil, verror.New(verror.ErrNoExist, ctx, d.id)
diff --git a/services/syncbase/server/database_sm.go b/services/syncbase/server/database_sm.go
index 1061087..2319680 100644
--- a/services/syncbase/server/database_sm.go
+++ b/services/syncbase/server/database_sm.go
@@ -7,9 +7,10 @@
import (
"v.io/v23/context"
"v.io/v23/rpc"
+ "v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
"v.io/v23/verror"
- "v.io/x/ref/services/syncbase/server/util"
+ "v.io/x/ref/services/syncbase/common"
"v.io/x/ref/services/syncbase/store"
)
@@ -21,12 +22,14 @@
// SchemaManager RPC methods
func (d *database) GetSchemaMetadata(ctx *context.T, call rpc.ServerCall) (wire.SchemaMetadata, error) {
+ allowGetSchemaMetadata := []access.Tag{access.Read}
+
if !d.exists {
return wire.SchemaMetadata{}, verror.New(verror.ErrNoExist, ctx, d.id)
}
// Check permissions on Database and retrieve schema metadata.
- dbData := DatabaseData{}
- if err := util.GetWithAuth(ctx, call, d.st, d.stKey(), &dbData); err != nil {
+ var dbData DatabaseData
+ if _, err := common.GetDataWithAuth(ctx, call, d, allowGetSchemaMetadata, d.st, &dbData); err != nil {
return wire.SchemaMetadata{}, err
}
if dbData.SchemaMetadata == nil {
@@ -36,26 +39,32 @@
}
func (d *database) SetSchemaMetadata(ctx *context.T, call rpc.ServerCall, metadata wire.SchemaMetadata) error {
+ allowSetSchemaMetadata := []access.Tag{access.Admin}
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
// Check permissions on Database and store schema metadata.
return store.RunInTransaction(d.st, func(tx store.Transaction) error {
- dbData := DatabaseData{}
- return util.UpdateWithAuth(ctx, call, tx, d.stKey(), &dbData, func() error {
- // NOTE: For now we expect the client to not issue multiple
- // concurrent SetSchemaMetadata calls.
- dbData.SchemaMetadata = &metadata
- return nil
- })
+ var dbData DatabaseData
+ if _, err := common.GetDataWithAuth(ctx, call, d, allowSetSchemaMetadata, tx, &dbData); err != nil {
+ return err
+ }
+ // NOTE: For now we expect the client to not issue multiple
+ // concurrent SetSchemaMetadata calls.
+ dbData.SchemaMetadata = &metadata
+ return store.Put(ctx, tx, d.stKey(), &dbData)
})
}
+////////////////////////////////////////
+// interfaces.Database methods
+
func (d *database) GetSchemaMetadataInternal(ctx *context.T) (*wire.SchemaMetadata, error) {
if !d.exists {
return nil, verror.New(verror.ErrNoExist, ctx, d.id)
}
- dbData := DatabaseData{}
+ var dbData DatabaseData
if err := store.Get(ctx, d.st, d.stKey(), &dbData); err != nil {
return nil, err
}
diff --git a/services/syncbase/server/database_watch.go b/services/syncbase/server/database_watch.go
index 8df5782..b7f88aa 100644
--- a/services/syncbase/server/database_watch.go
+++ b/services/syncbase/server/database_watch.go
@@ -25,11 +25,17 @@
// GetResumeMarker implements the wire.DatabaseWatcher interface.
func (d *database) GetResumeMarker(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) (watch.ResumeMarker, error) {
+ allowGetResumeMarker := wire.AllDatabaseTags
+
if !d.exists {
return nil, verror.New(verror.ErrNoExist, ctx, d.id)
}
var res watch.ResumeMarker
impl := func(sntx store.SnapshotOrTransaction) (err error) {
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowGetResumeMarker, sntx); err != nil {
+ return err
+ }
res, err = watchable.GetResumeMarker(sntx)
return err
}
@@ -41,6 +47,7 @@
// WatchGlob implements the wire.DatabaseWatcher interface.
func (d *database) WatchGlob(ctx *context.T, call watch.GlobWatcherWatchGlobServerCall, req watch.GlobRequest) error {
+ // Database permissions checked in d.watchWithFilter and d.processLogBatch.
sender := &watchBatchSender{
send: call.SendStream().Send,
}
@@ -53,6 +60,7 @@
// WatchPatterns implements the wire.DatabaseWatcher interface.
func (d *database) WatchPatterns(ctx *context.T, call wire.DatabaseWatcherWatchPatternsServerCall, resumeMarker watch.ResumeMarker, patterns []wire.CollectionRowPattern) error {
+ // Database permissions checked in d.watchWithFilter and d.processLogBatch.
sender := &watchBatchSender{
send: call.SendStream().Send,
}
@@ -66,12 +74,16 @@
// watchWithFilter sends the initial state (if necessary) and watch events,
// filtered using watchFilter, to the caller using sender.
func (d *database) watchWithFilter(ctx *context.T, call rpc.ServerCall, sender *watchBatchSender, resumeMarker watch.ResumeMarker, watchFilter filter.CollectionRowFilter) error {
- // TODO(ivanpi): Check permissions here and in other methods.
+ allowWatchDbStart := []access.Tag{access.Read}
+
if !d.exists {
return verror.New(verror.ErrNoExist, ctx, d.id)
}
initImpl := func(sntx store.SnapshotOrTransaction) error {
- // TODO(ivanpi): Check permissions here.
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowWatchDbStart, sntx); err != nil {
+ return err
+ }
needInitialState := len(resumeMarker) == 0
needResumeMarker := needInitialState || bytes.Equal(resumeMarker, []byte("now"))
// Get the resume marker if necessary.
@@ -112,8 +124,8 @@
// scanInitialState sends the initial state of all matching and accessible
// collections and rows in the database. Checks access on collections, but
// not on database.
-// TODO(ivanpi): Assumes Read perms on database. Careful if supporting RPCs
-// requiring only Resolve (e.g. WatchGlob).
+// Note: Assumes Read perms on database. Careful if supporting RPCs requiring
+// only Resolve.
// TODO(ivanpi): Abstract out multi-scan for scan and possibly query support.
// TODO(ivanpi): Use watch pattern prefixes to optimize scan ranges.
func (d *database) scanInitialState(ctx *context.T, call rpc.ServerCall, sender *watchBatchSender, sntx store.SnapshotOrTransaction, watchFilter filter.CollectionRowFilter) error {
@@ -154,13 +166,14 @@
}); err != nil {
return err
}
- // Check permissions for row access.
+ // Filter out rows with no read access.
// TODO(ivanpi): Collection scan already gets perms, optimize?
+ // TODO(ivanpi): Check service and database resolve only once.
c := &collectionReq{
id: cxId,
d: d,
}
- if _, err := c.checkAccess(ctx, call, sntx); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, c, []access.Tag{access.Read}, sntx); err != nil {
if verror.ErrorID(err) == verror.ErrNoAccess.ID {
// Skip sending rows if the collection is inaccessible. Caller can see
// from collection info that they have no read access and may therefore
@@ -271,13 +284,17 @@
// Note: Since the governing ACL for each change is no longer tracked, the
// permissions check uses the ACLs in effect at the time processLogBatch is
// called.
-// TODO(ivanpi): Assumes Read perms on database. Careful if supporting RPCs
-// requiring only Resolve (e.g. WatchGlob).
+// Note: Assumes Read perms on database. Careful if supporting RPCs requiring
+// only Resolve.
func (d *database) processLogBatch(ctx *context.T, call rpc.ServerCall, sender *watchBatchSender, watchFilter filter.CollectionRowFilter, logs []*watchable.LogEntry) error {
+ allowWatchDbContinue := []access.Tag{access.Read}
+
sn := d.st.NewSnapshot()
defer sn.Abort()
- // TODO(ivanpi): Recheck database perms here and fail, or cache for collection
- // access checks.
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, d, allowWatchDbContinue, sn); err != nil {
+ return err
+ }
valueBytes := []byte{}
for _, logEntry := range logs {
var opKey string
@@ -305,13 +322,14 @@
if !watchFilter.RowMatches(cxId, row) {
continue
}
- // Filter out rows that we can't access.
+ // Filter out rows with no read access.
// TODO(ivanpi): Check only once per collection per batch.
+ // TODO(ivanpi): Check service and database resolve only once per batch.
c := &collectionReq{
id: cxId,
d: d,
}
- if _, err := c.checkAccess(ctx, call, sn); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, c, []access.Tag{access.Read}, sn); err != nil {
if verror.ErrorID(err) == verror.ErrNoAccess.ID || verror.ErrorID(err) == verror.ErrNoExist.ID {
// Skip sending rows if the collection is inaccessible. Caller can see
// from collection info that they have no read access and may therefore
diff --git a/services/syncbase/server/interfaces/database.go b/services/syncbase/server/interfaces/database.go
index 111c6d7..07089f3 100644
--- a/services/syncbase/server/interfaces/database.go
+++ b/services/syncbase/server/interfaces/database.go
@@ -8,6 +8,7 @@
"v.io/v23/context"
"v.io/v23/rpc"
wire "v.io/v23/services/syncbase"
+ "v.io/x/ref/services/syncbase/common"
"v.io/x/ref/services/syncbase/store"
"v.io/x/ref/services/syncbase/store/watchable"
)
@@ -42,4 +43,6 @@
// Note: Resetting a stream does not reconnect the stream. Its upto the
// client to reconnect.
ResetCrConnectionStream()
+
+ common.Permser
}
diff --git a/services/syncbase/server/interfaces/service.go b/services/syncbase/server/interfaces/service.go
index a0d2e29..9e9c6b7 100644
--- a/services/syncbase/server/interfaces/service.go
+++ b/services/syncbase/server/interfaces/service.go
@@ -8,6 +8,7 @@
"v.io/v23/context"
"v.io/v23/rpc"
wire "v.io/v23/services/syncbase"
+ "v.io/x/ref/services/syncbase/common"
"v.io/x/ref/services/syncbase/store"
)
@@ -24,4 +25,6 @@
// DatabaseIds returns ids for all databases.
DatabaseIds(ctx *context.T, call rpc.ServerCall) ([]wire.Id, error)
+
+ common.Permser
}
diff --git a/services/syncbase/server/row.go b/services/syncbase/server/row.go
index 6a0550c..e489673 100644
--- a/services/syncbase/server/row.go
+++ b/services/syncbase/server/row.go
@@ -29,6 +29,7 @@
func (r *rowReq) Exists(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) (bool, error) {
allowExists := []access.Tag{access.Read, access.Write}
+
impl := func(sntx store.SnapshotOrTransaction) (err error) {
if _, err := common.GetPermsWithAuth(ctx, call, r.c, allowExists, sntx); err != nil {
return err
@@ -39,6 +40,7 @@
}
func (r *rowReq) Get(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) (*vom.RawBytes, error) {
+ // Permissions checked in r.get.
var res *vom.RawBytes
impl := func(sntx store.SnapshotOrTransaction) (err error) {
res, err = r.get(ctx, call, sntx)
@@ -51,6 +53,7 @@
}
func (r *rowReq) Put(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle, value *vom.RawBytes) error {
+ // Permissions checked in r.put.
impl := func(ts *transactionState) error {
return r.put(ctx, call, ts, value)
}
@@ -58,6 +61,7 @@
}
func (r *rowReq) Delete(ctx *context.T, call rpc.ServerCall, bh wire.BatchHandle) error {
+ // Permissions checked in r.delete.
impl := func(ts *transactionState) error {
return r.delete(ctx, call, ts)
}
@@ -78,7 +82,9 @@
// get reads data from the storage engine.
// Performs authorization check.
func (r *rowReq) get(ctx *context.T, call rpc.ServerCall, sntx store.SnapshotOrTransaction) (*vom.RawBytes, error) {
- if _, err := r.c.checkAccess(ctx, call, sntx); err != nil {
+ allowGet := []access.Tag{access.Read}
+
+ if _, err := common.GetPermsWithAuth(ctx, call, r.c, allowGet, sntx); err != nil {
return nil, err
}
var valueAsRawBytes vom.RawBytes
@@ -91,7 +97,9 @@
// put writes data to the storage engine.
// Performs authorization check.
func (r *rowReq) put(ctx *context.T, call rpc.ServerCall, ts *transactionState, value *vom.RawBytes) error {
- currentPerms, err := r.c.checkAccess(ctx, call, ts.tx)
+ allowPut := []access.Tag{access.Write}
+
+ currentPerms, err := common.GetPermsWithAuth(ctx, call, r.c, allowPut, ts.tx)
if err != nil {
return err
}
@@ -102,7 +110,9 @@
// delete deletes data from the storage engine.
// Performs authorization check.
func (r *rowReq) delete(ctx *context.T, call rpc.ServerCall, ts *transactionState) error {
- currentPerms, err := r.c.checkAccess(ctx, call, ts.tx)
+ allowDelete := []access.Tag{access.Write}
+
+ currentPerms, err := common.GetPermsWithAuth(ctx, call, r.c, allowDelete, ts.tx)
if err != nil {
return err
}
diff --git a/services/syncbase/server/service.go b/services/syncbase/server/service.go
index 2693479..8db90a4 100644
--- a/services/syncbase/server/service.go
+++ b/services/syncbase/server/service.go
@@ -321,11 +321,13 @@
// TODO(sadovsky): Add test to demonstrate that these don't work unless Syncbase
// was started in dev mode.
func (s *service) DevModeUpdateVClock(ctx *context.T, call rpc.ServerCall, opts wire.DevModeUpdateVClockOpts) error {
+ allowUpdateVClock := []access.Tag{access.Admin}
+
if !s.opts.DevMode {
return wire.NewErrNotInDevMode(ctx)
}
// Check perms.
- if err := util.GetWithAuth(ctx, call, s.st, s.stKey(), &ServiceData{}); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, s, allowUpdateVClock, s.st); err != nil {
return err
}
if opts.NtpHost != "" {
@@ -348,47 +350,56 @@
}
func (s *service) DevModeGetTime(ctx *context.T, call rpc.ServerCall) (time.Time, error) {
+ allowGetTime := []access.Tag{access.Admin}
+
if !s.opts.DevMode {
return time.Time{}, wire.NewErrNotInDevMode(ctx)
}
// Check perms.
- if err := util.GetWithAuth(ctx, call, s.st, s.stKey(), &ServiceData{}); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, s, allowGetTime, s.st); err != nil {
return time.Time{}, err
}
return s.vclock.Now()
}
func (s *service) SetPermissions(ctx *context.T, call rpc.ServerCall, perms access.Permissions, version string) error {
+ allowSetPermissions := []access.Tag{access.Admin}
+
if err := common.ValidatePerms(ctx, perms, access.AllTypicalTags()); err != nil {
return err
}
return store.RunInTransaction(s.st, func(tx store.Transaction) error {
data := &ServiceData{}
- return util.UpdateWithAuth(ctx, call, tx, s.stKey(), data, func() error {
- if err := util.CheckVersion(ctx, version, data.Version); err != nil {
- return err
- }
- data.Perms = perms
- data.Version++
- return nil
- })
+ if _, err := common.GetDataWithAuth(ctx, call, s, allowSetPermissions, tx, data); err != nil {
+ return err
+ }
+ if err := util.CheckVersion(ctx, version, data.Version); err != nil {
+ return err
+ }
+ data.Perms = perms
+ data.Version++
+ return store.Put(ctx, tx, s.stKey(), data)
})
}
func (s *service) GetPermissions(ctx *context.T, call rpc.ServerCall) (perms access.Permissions, version string, err error) {
+ allowGetPermissions := []access.Tag{access.Admin}
+
data := &ServiceData{}
- if err := util.GetWithAuth(ctx, call, s.st, s.stKey(), data); err != nil {
+ if _, err := common.GetDataWithAuth(ctx, call, s, allowGetPermissions, s.st, data); err != nil {
return nil, "", err
}
return data.Perms, util.FormatVersion(data.Version), nil
}
func (s *service) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
+ allowGlob := []access.Tag{access.Read}
+
// Check perms.
sn := s.st.NewSnapshot()
defer sn.Abort()
- if err := util.GetWithAuth(ctx, call, sn, s.stKey(), &ServiceData{}); err != nil {
+ if _, err := common.GetPermsWithAuth(ctx, call, s, allowGlob, sn); err != nil {
return err
}
return util.GlobChildren(ctx, call, matcher, sn, common.DbInfoPrefix)
@@ -431,6 +442,8 @@
// Database management methods
func (s *service) createDatabase(ctx *context.T, call rpc.ServerCall, dbId wire.Id, perms access.Permissions, metadata *wire.SchemaMetadata) (reterr error) {
+ allowCreateDatabase := []access.Tag{access.Write}
+
if err := common.ValidatePerms(ctx, perms, wire.AllDatabaseTags); err != nil {
return err
}
@@ -457,7 +470,7 @@
}
} else {
// Check serviceData perms.
- if err := util.GetWithAuth(ctx, call, s.st, s.stKey(), sData); err != nil {
+ if _, err := common.GetDataWithAuth(ctx, call, s, allowCreateDatabase, s.st, sData); err != nil {
return err
}
if adminAcl, ok := sData.Perms[string(access.Admin)]; !ok || adminAcl.Authorize(ctx, call.Security()) != nil {
@@ -547,8 +560,10 @@
}
func (s *service) destroyDatabase(ctx *context.T, call rpc.ServerCall, dbId wire.Id) error {
+ allowDestroyDatabase := []access.Tag{access.Admin}
+
// Steps:
- // 1. Check databaseData perms.
+ // 1. Check serviceData and databaseData perms.
// 2. Move dbInfo from active dbs into GC log. <===== CHANGE BECOMES VISIBLE
// 3. Best effort database destroy. If it fails, it will be retried on
// syncbased restart.
@@ -559,12 +574,13 @@
return nil // destroy is idempotent
}
- // 1. Check databaseData perms.
- if err := d.CheckPermsInternal(ctx, call, d.St()); err != nil {
- if verror.ErrorID(err) == verror.ErrNoExist.ID {
- return nil // destroy is idempotent
+ // 1. Check serviceData and databaseData perms.
+ // Check permissions on Service.
+ if _, authErr := common.GetPermsWithAuth(ctx, call, d.s, allowDestroyDatabase, d.s.st); authErr != nil {
+ // Caller has no Admin access on Service. Check databaseData perms.
+ if _, authErr = common.GetPermsWithAuth(ctx, call, d, allowDestroyDatabase, d.st); authErr != nil {
+ return authErr
}
- return err
}
// 2. Move dbInfo from active dbs into GC log.
@@ -592,6 +608,7 @@
}
func (s *service) setDatabasePerms(ctx *context.T, call rpc.ServerCall, dbId wire.Id, perms access.Permissions, version string) error {
+ // Permissions checked in d.setPermsInternal.
s.mu.Lock()
defer s.mu.Unlock()
d, ok := s.dbs[dbId]
diff --git a/services/syncbase/server/util/store.go b/services/syncbase/server/util/store.go
index f44086f..ce3163e 100644
--- a/services/syncbase/server/util/store.go
+++ b/services/syncbase/server/util/store.go
@@ -30,6 +30,7 @@
// "c:" from the error messages they return.
// GetWithAuth does Get followed by an auth check.
+// TODO(ivanpi): Remove when all callers are gone.
func GetWithAuth(ctx *context.T, call rpc.ServerCall, st store.StoreReader, k string, v common.PermserData) error {
if err := store.Get(ctx, st, k, v); err != nil {
return err
@@ -40,16 +41,3 @@
}
return nil
}
-
-// UpdateWithAuth performs a read-modify-write.
-// Input v is populated by the "read" step. fn should "modify" v.
-// Performs an auth check as part of the "read" step.
-func UpdateWithAuth(ctx *context.T, call rpc.ServerCall, tx store.Transaction, k string, v common.PermserData, fn func() error) error {
- if err := GetWithAuth(ctx, call, tx, k, v); err != nil {
- return err
- }
- if err := fn(); err != nil {
- return err
- }
- return store.Put(ctx, tx, k, v)
-}
diff --git a/services/syncbase/server/watchlog_test.go b/services/syncbase/server/watchlog_test.go
index 8e3c96c..4484371 100644
--- a/services/syncbase/server/watchlog_test.go
+++ b/services/syncbase/server/watchlog_test.go
@@ -60,10 +60,11 @@
st, _ := watchable.Wrap(memstore.New(), clk, &watchable.Options{
ManagedPrefixes: []string{common.RowPrefix, common.CollectionPermsPrefix},
})
+ s := &service{st: st}
store.Put(ctx, st, common.ServicePrefix, &ServiceData{
Perms: access.Permissions{}.Add("root", access.TagStrings(access.AllTypicalTags()...)...),
})
- db := &database{id: wire.Id{Blessing: "a", Name: "d"}, exists: true, st: st}
+ db := &database{id: wire.Id{Blessing: "a", Name: "d"}, exists: true, st: st, s: s}
store.Put(ctx, st, db.stKey(), &DatabaseData{
Perms: access.Permissions{}.Add("root", access.TagStrings(wire.AllDatabaseTags...)...),
})
diff --git a/services/syncbase/store/leveldb/stats_blackbox_test.go b/services/syncbase/store/leveldb/stats_blackbox_test.go
index 89add07..92f6b8b 100644
--- a/services/syncbase/store/leveldb/stats_blackbox_test.go
+++ b/services/syncbase/store/leveldb/stats_blackbox_test.go
@@ -69,9 +69,9 @@
since := time.Now()
service := syncbase.NewService(serverName)
- tu.CreateDatabase(t, ctx, service, "empty_db_0")
- tu.CreateDatabase(t, ctx, service, "empty_db_1")
- tu.CreateDatabase(t, ctx, service, "empty_db_2")
+ tu.CreateDatabase(t, ctx, service, "empty_db_0", nil)
+ tu.CreateDatabase(t, ctx, service, "empty_db_1", nil)
+ tu.CreateDatabase(t, ctx, service, "empty_db_2", nil)
count := 0
for got := range statsValues("service/*/*", since) {
@@ -122,7 +122,7 @@
since := time.Now()
service := syncbase.NewService(serverName)
- db := tu.CreateDatabase(t, ctx, service, "big_db")
+ db := tu.CreateDatabase(t, ctx, service, "big_db", nil)
write100MB(ctx, db)
diff --git a/services/syncbase/testutil/layer.go b/services/syncbase/testutil/layer.go
index 1e846b8..de79cac 100644
--- a/services/syncbase/testutil/layer.go
+++ b/services/syncbase/testutil/layer.go
@@ -159,7 +159,7 @@
assertExists(t, ctx, self, "self", true)
assertExists(t, ctx, child, "child", false)
- // Test that destroy fails if the perms disallow access.
+ // Test that parent perms are sufficient for destroy.
self2 := parent.Child("self2")
if err := self2.Create(ctx, nil); err != nil {
t.Fatalf("self2.Create() failed: %v", err)
@@ -170,8 +170,15 @@
if err := self2.SetPermissions(ctx, perms, ""); err != nil {
t.Fatalf("self2.SetPermissions() failed: %v", err)
}
- if err := self2.Destroy(ctx); verror.ErrorID(err) != verror.ErrNoAccess.ID {
- t.Fatalf("self2.Destroy() should have failed: %v", err)
+ if err := self2.Destroy(ctx); err != nil {
+ t.Fatalf("self2.Destroy() failed: %v", err)
+ }
+
+ assertExists(t, ctx, self2, "self2", false)
+
+ // Recreate self2 for later.
+ if err := self2.Create(ctx, nil); err != nil {
+ t.Fatalf("self2.Create() failed: %v", err)
}
assertExists(t, ctx, self2, "self2", true)
@@ -209,6 +216,19 @@
}
assertExists(t, ctx, self, "self", false)
+
+ // Test that destroy fails if both parent and own perms disallow access.
+ perms = DefaultPerms(self2.AllowedTags(), "root:o:app:client")
+ perms.Clear("root:o:app:client", string(access.Write), string(access.Admin))
+ perms.Add("nobody", string(access.Admin))
+ if err := self2.SetPermissions(ctx, perms, ""); err != nil {
+ t.Fatalf("self2.SetPermissions() failed: %v", err)
+ }
+ if err := self2.Destroy(ctx); verror.ErrorID(err) != verror.ErrNoAccess.ID {
+ t.Fatalf("self2.Destroy() should have failed: %v", err)
+ }
+
+ assertExists(t, ctx, self2, "self2", true)
}
func copyAndSortStrings(strs []string) []string {
diff --git a/services/syncbase/testutil/util.go b/services/syncbase/testutil/util.go
index 22f9a6f..bdf484a 100644
--- a/services/syncbase/testutil/util.go
+++ b/services/syncbase/testutil/util.go
@@ -48,9 +48,9 @@
// TODO(sadovsky): Standardize on a small set of constants and helper functions
// to share across all Syncbase tests. Currently, our 'featuretests' tests use a
// different set of helpers from our other unit tests.
-func CreateDatabase(t testing.TB, ctx *context.T, s syncbase.Service, name string) syncbase.Database {
+func CreateDatabase(t testing.TB, ctx *context.T, s syncbase.Service, name string, perms access.Permissions) syncbase.Database {
d := s.Database(ctx, name, nil)
- if err := d.Create(ctx, nil); err != nil {
+ if err := d.Create(ctx, perms); err != nil {
Fatalf(t, "d.Create() failed: %v", err)
}
return d
diff --git a/services/syncbase/vsync/blob.go b/services/syncbase/vsync/blob.go
index ab50975..844ae53 100644
--- a/services/syncbase/vsync/blob.go
+++ b/services/syncbase/vsync/blob.go
@@ -192,9 +192,16 @@
// RPCs for managing blobs between Syncbase and its clients.
func (sd *syncDatabase) CreateBlob(ctx *context.T, call rpc.ServerCall) (wire.BlobRef, error) {
+ allowCreateBlob := []access.Tag{access.Write}
+
vlog.VI(2).Infof("sync: CreateBlob: begin")
defer vlog.VI(2).Infof("sync: CreateBlob: end")
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowCreateBlob, sd.db.St()); err != nil {
+ return wire.NullBlobRef, err
+ }
+
// Get this Syncbase's blob store handle.
ss := sd.sync.(*syncService)
bst := ss.bst
@@ -211,9 +218,16 @@
}
func (sd *syncDatabase) PutBlob(ctx *context.T, call wire.BlobManagerPutBlobServerCall, br wire.BlobRef) error {
+ allowPutBlob := []access.Tag{access.Write}
+
vlog.VI(2).Infof("sync: PutBlob: begin br %v", br)
defer vlog.VI(2).Infof("sync: PutBlob: end br %v", br)
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowPutBlob, sd.db.St()); err != nil {
+ return err
+ }
+
// Get this Syncbase's blob store handle.
ss := sd.sync.(*syncService)
bst := ss.bst
@@ -235,9 +249,16 @@
}
func (sd *syncDatabase) CommitBlob(ctx *context.T, call rpc.ServerCall, br wire.BlobRef) error {
+ allowCommitBlob := []access.Tag{access.Write}
+
vlog.VI(2).Infof("sync: CommitBlob: begin br %v", br)
defer vlog.VI(2).Infof("sync: CommitBlob: end br %v", br)
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowCommitBlob, sd.db.St()); err != nil {
+ return err
+ }
+
// Get this Syncbase's blob store handle.
ss := sd.sync.(*syncService)
bst := ss.bst
@@ -250,9 +271,16 @@
}
func (sd *syncDatabase) GetBlobSize(ctx *context.T, call rpc.ServerCall, br wire.BlobRef) (int64, error) {
+ allowGetBlobSize := wire.AllDatabaseTags
+
vlog.VI(2).Infof("sync: GetBlobSize: begin br %v", br)
defer vlog.VI(2).Infof("sync: GetBlobSize: end br %v", br)
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowGetBlobSize, sd.db.St()); err != nil {
+ return 0, err
+ }
+
// Get this Syncbase's blob store handle.
ss := sd.sync.(*syncService)
bst := ss.bst
@@ -267,13 +295,27 @@
}
func (sd *syncDatabase) DeleteBlob(ctx *context.T, call rpc.ServerCall, br wire.BlobRef) error {
+ allowDeleteBlob := wire.AllDatabaseTags
+
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowDeleteBlob, sd.db.St()); err != nil {
+ return err
+ }
+
return verror.NewErrNotImplemented(ctx)
}
func (sd *syncDatabase) GetBlob(ctx *context.T, call wire.BlobManagerGetBlobServerCall, br wire.BlobRef, offset int64) error {
+ allowGetBlob := wire.AllDatabaseTags
+
vlog.VI(2).Infof("sync: GetBlob: begin br %v", br)
defer vlog.VI(2).Infof("sync: GetBlob: end br %v", br)
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowGetBlob, sd.db.St()); err != nil {
+ return err
+ }
+
// First get the blob locally if available.
ss := sd.sync.(*syncService)
err := getLocalBlob(ctx, call.SendStream(), ss.bst, br, offset)
@@ -285,9 +327,16 @@
}
func (sd *syncDatabase) FetchBlob(ctx *context.T, call wire.BlobManagerFetchBlobServerCall, br wire.BlobRef, priority uint64) error {
+ allowFetchBlob := wire.AllDatabaseTags
+
vlog.VI(2).Infof("sync: FetchBlob: begin br %v", br)
defer vlog.VI(2).Infof("sync: FetchBlob: end br %v", br)
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowFetchBlob, sd.db.St()); err != nil {
+ return err
+ }
+
clientStream := call.SendStream()
// Check if BlobRef already exists locally.
@@ -314,14 +363,35 @@
}
func (sd *syncDatabase) PinBlob(ctx *context.T, call rpc.ServerCall, br wire.BlobRef) error {
+ allowPinBlob := []access.Tag{access.Write}
+
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowPinBlob, sd.db.St()); err != nil {
+ return err
+ }
+
return verror.NewErrNotImplemented(ctx)
}
func (sd *syncDatabase) UnpinBlob(ctx *context.T, call rpc.ServerCall, br wire.BlobRef) error {
+ allowUnpinBlob := wire.AllDatabaseTags
+
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowUnpinBlob, sd.db.St()); err != nil {
+ return err
+ }
+
return verror.NewErrNotImplemented(ctx)
}
func (sd *syncDatabase) KeepBlob(ctx *context.T, call rpc.ServerCall, br wire.BlobRef, rank uint64) error {
+ allowKeepBlob := wire.AllDatabaseTags
+
+ // Check permissions on Database.
+ if _, err := common.GetPermsWithAuth(ctx, call, sd.db, allowKeepBlob, sd.db.St()); err != nil {
+ return err
+ }
+
return verror.NewErrNotImplemented(ctx)
}
diff --git a/services/syncbase/vsync/testutil_test.go b/services/syncbase/vsync/testutil_test.go
index 8fb5a81..7b8e2e9 100644
--- a/services/syncbase/vsync/testutil_test.go
+++ b/services/syncbase/vsync/testutil_test.go
@@ -67,6 +67,14 @@
return []wire.Id{mockDbId}, nil
}
+func (s *mockService) GetDataWithExistAuth(ctx *context.T, call rpc.ServerCall, st store.StoreReader, v common.PermserData) (parentPerms, perms access.Permissions, existErr error) {
+ return nil, nil, nil
+}
+
+func (s *mockService) PermserData() common.PermserData {
+ return nil
+}
+
// mockDatabase emulates a Syncbase Database. It is used to test sync
// functionality.
type mockDatabase struct {
@@ -101,6 +109,14 @@
mockCRStream = nil
}
+func (d *mockDatabase) GetDataWithExistAuth(ctx *context.T, call rpc.ServerCall, st store.StoreReader, v common.PermserData) (parentPerms, perms access.Permissions, existErr error) {
+ return nil, nil, nil
+}
+
+func (d *mockDatabase) PermserData() common.PermserData {
+ return nil
+}
+
// createService creates a mock Syncbase service used for testing sync
// functionality.
func createService(t *testing.T) *mockService {
