syncbase: improve test blessing setup & use AllowEveryone dispatcher
Previously in tests we used server blessing "server" and
client blessing "server/client". This CL changes things so
that the server uses blessing "root/server" and the client
uses blessing "root/client".
The previous setup was flawed in the following way:
- Our Dispatcher.Lookup was returning a nil authorizer; the
RPC framework thus defaults to using a
security.DefaultAuthorizer(), and uses that authorizer to
perform an auth check on every request.
- The default authorizer implements a conservative policy,
where the client must present a blessing that extends the
server's blessing, or vice versa. In general, this is not
expected to be the case, and in particular with my JS
todos example app, the blessings I was using did not
satisfy this property, so all RPCs will failing with "no
access" errors.
- Our previous test setup satisfied the above property, so
the DefaultAuthorizer returned by the dispatcher had the
same effect as an AllowEveryone authorizer would've had,
and the tests ended up testing what they were intended to
test.
Returning an AllowEveryone authorizer from the dispatcher
seems like the right thing to do in our case, because the
authorization decision is more complicated than looking up
an ACL based on the tag for the called method. E.g. in some
cases we'll need to consult multiple ACLs, and in other
cases the method tag actually refers to an ACL on a
different object than the RPC receiver (e.g. Database.Create
consults the app's ACL).
Finally, I just want to note that before switching the
dispatcher to return an AllowEveryone authorizer, I verified
that our tests fail with the "root/client" and "root/server"
blessings, as expected.
Change-Id: Idfc895813bbae62c518b728d4d569c04e7a4f753
diff --git a/services/syncbase/server/dispatcher.go b/services/syncbase/server/dispatcher.go
index 46f7f7e..a39f8bb 100644
--- a/services/syncbase/server/dispatcher.go
+++ b/services/syncbase/server/dispatcher.go
@@ -27,17 +27,20 @@
return &dispatcher{s: s}
}
-// TODO(sadovsky): Return a real authorizer in various places below.
+// We always return an AllowEveryone authorizer from Lookup(), and rely on our
+// RPC method implementations to perform proper authorization.
+var auth security.Authorizer = security.AllowEveryone()
+
func (disp *dispatcher) Lookup(suffix string) (interface{}, security.Authorizer, error) {
suffix = strings.TrimPrefix(suffix, "/")
parts := strings.SplitN(suffix, "/", 2)
if len(suffix) == 0 {
- return wire.ServiceServer(disp.s), nil, nil
+ return wire.ServiceServer(disp.s), auth, nil
}
if parts[0] == util.SyncbaseSuffix {
- return interfaces.SyncServer(disp.s.sync), nil, nil
+ return interfaces.SyncServer(disp.s.sync), auth, nil
}
// Validate all key atoms up front, so that we can avoid doing so in all our
@@ -64,7 +67,7 @@
}
if len(parts) == 1 {
- return wire.AppServer(a), nil, nil
+ return wire.AppServer(a), auth, nil
}
// All database, table, and row methods require the app to exist. If it
diff --git a/services/syncbase/server/nosql/dispatcher.go b/services/syncbase/server/nosql/dispatcher.go
index 3bbea26..1b151c4 100644
--- a/services/syncbase/server/nosql/dispatcher.go
+++ b/services/syncbase/server/nosql/dispatcher.go
@@ -29,6 +29,10 @@
return &dispatcher{a: a}
}
+// We always return an AllowEveryone authorizer from Lookup(), and rely on our
+// RPC method implementations to perform proper authorization.
+var auth security.Authorizer = security.AllowEveryone()
+
func (disp *dispatcher) Lookup(suffix string) (interface{}, security.Authorizer, error) {
suffix = strings.TrimPrefix(suffix, "/")
parts := strings.Split(suffix, "/")
@@ -74,7 +78,7 @@
return nil, nil, wire.NewErrInvalidName(nil, suffix)
}
if len(parts) == 1 {
- return nosqlWire.DatabaseServer(dReq), nil, nil
+ return nosqlWire.DatabaseServer(dReq), auth, nil
}
// All table and row methods require the database to exist. If it doesn't,
@@ -92,7 +96,7 @@
d: dReq,
}
if len(parts) == 2 {
- return nosqlWire.TableServer(tReq), nil, nil
+ return nosqlWire.TableServer(tReq), auth, nil
}
rReq := &rowReq{
@@ -100,7 +104,7 @@
t: tReq,
}
if len(parts) == 3 {
- return nosqlWire.RowServer(rReq), nil, nil
+ return nosqlWire.RowServer(rReq), auth, nil
}
return nil, nil, verror.NewErrNoExist(nil)
diff --git a/services/syncbase/syncbased/main.go b/services/syncbase/syncbased/main.go
index a75b471..d158752 100644
--- a/services/syncbase/syncbased/main.go
+++ b/services/syncbase/syncbased/main.go
@@ -50,11 +50,12 @@
vlog.Fatal("securityflag.PermissionsFromFlag() failed: ", err)
}
if perms != nil {
- vlog.Info("Using permissions from command line flag.")
+ vlog.Info("Using perms from command line flag.")
} else {
- vlog.Info("No permissions flag provided. Giving local principal all permissions.")
+ vlog.Info("Perms flag not set. Giving local principal all perms.")
perms = defaultPerms(security.DefaultBlessingPatterns(v23.GetPrincipal(ctx)))
}
+ vlog.Infof("Perms: %v", perms)
service, err := server.NewService(nil, nil, server.ServiceOptions{
Perms: perms,
RootDir: *rootDir,
