services/mgmt/application/applicationd: rationalize permission list behaviour
Modify the implementation of permission lists in applicationd to
1. set a permission list associated with a specific application at the
time of uploading.
2. have the same permission list for all versions of an application
envelope.
Change-Id: Ie7a3875a11d639d16859f4b86316e4de0397b688
diff --git a/services/application/applicationd/acl_test.go b/services/application/applicationd/acl_test.go
index bfefc2f..86f5713 100644
--- a/services/application/applicationd/acl_test.go
+++ b/services/application/applicationd/acl_test.go
@@ -121,15 +121,32 @@
t.Fatalf("Put() failed: %v", err)
}
+ vlog.VI(2).Infof("Accessing the Permission Lists of the root returns a (simulated) list providing default authorization.")
acl, etag, err := repostub.GetPermissions(ctx)
- if verror.ErrorID(err) != verror.ErrNoExist.ID {
- t.Fatalf("GetPermissions should have failed with ErrNoExist but was: %v", err)
+ if err != nil {
+ t.Fatalf("GetPermissions should not have failed: %v", err)
}
if got, want := etag, ""; got != want {
t.Fatalf("GetPermissions got %v, want %v", got, want)
}
- if acl != nil {
- t.Fatalf("GetPermissions got %v, expected %v", acl, nil)
+ expected := access.Permissions{
+ "Admin": access.AccessList{
+ In: []security.BlessingPattern{"root/$", "root/self/$", "root/self/child"},
+ NotIn: []string(nil)},
+ "Read": access.AccessList{
+ In: []security.BlessingPattern{"root/$", "root/self/$", "root/self/child"},
+ NotIn: []string(nil)},
+ "Write": access.AccessList{
+ In: []security.BlessingPattern{"root/$", "root/self/$", "root/self/child"},
+ NotIn: []string(nil)},
+ "Debug": access.AccessList{
+ In: []security.BlessingPattern{"root/$", "root/self/$", "root/self/child"},
+ NotIn: []string(nil)},
+ "Resolve": access.AccessList{
+ In: []security.BlessingPattern{"root/$", "root/self/$", "root/self/child"},
+ NotIn: []string(nil)}}
+ if got := acl; !reflect.DeepEqual(expected.Normalize(), got.Normalize()) {
+ t.Errorf("got %#v, exected %#v ", got, expected)
}
vlog.VI(2).Infof("self attempting to give other permission to update application")
@@ -146,7 +163,7 @@
if err != nil {
t.Fatalf("GetPermissions should not have failed: %v", err)
}
- expected := newAccessList
+ expected = newAccessList
if got := acl; !reflect.DeepEqual(expected.Normalize(), got.Normalize()) {
t.Errorf("got %#v, exected %#v ", got, expected)
}
@@ -246,20 +263,37 @@
if err := v2stub.Put(ctx, []string{"base"}, envelopeV1); err != nil {
t.Fatalf("Put() failed: %v", err)
}
+ v3stub := repository.ApplicationClient("repo/naps/v1")
+ if err := v3stub.Put(ctx, []string{"base"}, envelopeV1); err != nil {
+ t.Fatalf("Put() failed: %v", err)
+ }
vlog.VI(2).Info("Self can access.AccessLists but other can't.")
- for _, path := range []string{"repo/search", "repo/search/v1", "repo/search/v2"} {
+ expectedSelfPermissions := access.Permissions{
+ "Admin": access.AccessList{
+ In: []security.BlessingPattern{"root/$", "root/self"},
+ NotIn: []string{}},
+ "Read": access.AccessList{In: []security.BlessingPattern{"root/$", "root/self"},
+ NotIn: []string{}},
+ "Write": access.AccessList{In: []security.BlessingPattern{"root/$", "root/self"},
+ NotIn: []string{}},
+ "Debug": access.AccessList{In: []security.BlessingPattern{"root/$", "root/self"},
+ NotIn: []string{}},
+ "Resolve": access.AccessList{In: []security.BlessingPattern{"root/$", "root/self"},
+ NotIn: []string{}}}
+
+ for _, path := range []string{"repo/search", "repo/search/v1", "repo/search/v2", "repo/naps", "repo/naps/v1"} {
stub := repository.ApplicationClient(path)
- acl, etag, err := stub.GetPermissions(ctx)
- if verror.ErrorID(err) != verror.ErrNoExist.ID {
- t.Fatalf("GetPermissions should have failed with ErrNoExist but was: %v", err)
+ acl, _, err := stub.GetPermissions(ctx)
+ if err != nil {
+ t.Fatalf("Newly uploaded envelopes failed to receive permission lists: %v", err)
}
- if got, want := etag, ""; got != want {
- t.Fatalf("GetPermissions got %v, want %v", got, want)
+
+ if got := acl; !reflect.DeepEqual(expectedSelfPermissions.Normalize(), got.Normalize()) {
+ t.Errorf("got %#v, expected %#v ", got, expectedSelfPermissions)
}
- if acl != nil {
- t.Fatalf("GetPermissions got %v, expected %v", acl, nil)
- }
+
+ // But otherCtx doesn't have admin permissions so has no access.
if _, _, err := stub.GetPermissions(otherCtx); err == nil {
t.Fatalf("GetPermissions didn't fail for other when it should have.")
}
@@ -280,50 +314,72 @@
t.Fatalf("GetPermissions should have failed")
}
- vlog.VI(2).Infof("Self gives other full access only to repo/search/v1.")
- newAccessList = make(access.Permissions)
- for _, tag := range access.AllTypicalTags() {
- newAccessList.Add("root/other", string(tag))
- }
- if err := v1stub.SetPermissions(ctx, newAccessList, ""); err != nil {
- t.Fatalf("SetPermissions failed: %v", err)
- }
-
- vlog.VI(2).Infof("Other can now access this location.")
- acl, _, err := v1stub.GetPermissions(otherCtx)
+ vlog.VI(2).Infof("Self gives other full access to repo/search/...")
+ newAccessList, etag, err := v1stub.GetPermissions(ctx)
if err != nil {
t.Fatalf("GetPermissions should not have failed: %v", err)
}
- expected := access.Permissions{
- "Admin": access.AccessList{
- In: []security.BlessingPattern{"root/other"},
- NotIn: []string{}},
- "Read": access.AccessList{In: []security.BlessingPattern{"root/other"},
- NotIn: []string{}},
- "Write": access.AccessList{In: []security.BlessingPattern{"root/other"},
- NotIn: []string{}},
- "Debug": access.AccessList{In: []security.BlessingPattern{"root/other"},
- NotIn: []string{}},
- "Resolve": access.AccessList{In: []security.BlessingPattern{"root/other"},
- NotIn: []string{}}}
- if got := acl; !reflect.DeepEqual(expected.Normalize(), got.Normalize()) {
- t.Errorf("got %#v, exected %#v ", got, expected)
+ for _, tag := range access.AllTypicalTags() {
+ newAccessList.Add("root/other", string(tag))
}
- vlog.VI(2).Infof("Self can too thanks to hierarchical auth.")
- if _, _, err = v1stub.GetPermissions(ctx); err != nil {
- t.Fatalf("GetPermissions should not have failed: %v", err)
+ if err := v1stub.SetPermissions(ctx, newAccessList, etag); err != nil {
+ t.Fatalf("SetPermissions failed: %v", err)
}
- // But other locations should be unaffected and other cannot access.
- for _, path := range []string{"repo/search", "repo/search/v2"} {
+ expected := access.Permissions{
+ "Resolve": access.AccessList{In: []security.BlessingPattern{
+ "root/$",
+ "root/other",
+ "root/self"},
+ NotIn: []string(nil)},
+ "Admin": access.AccessList{In: []security.BlessingPattern{
+ "root/$",
+ "root/other",
+ "root/self"},
+ NotIn: []string(nil)},
+ "Read": access.AccessList{In: []security.BlessingPattern{
+ "root/$",
+ "root/other",
+ "root/self"},
+ NotIn: []string(nil)},
+ "Write": access.AccessList{In: []security.BlessingPattern{
+ "root/$",
+ "root/other",
+ "root/self"},
+ NotIn: []string(nil)},
+ "Debug": access.AccessList{In: []security.BlessingPattern{
+ "root/$",
+ "root/other", "root/self"},
+ NotIn: []string(nil)},
+ }
+
+ for _, path := range []string{"repo/search", "repo/search/v1", "repo/search/v2"} {
+ stub := repository.ApplicationClient(path)
+ vlog.VI(2).Infof("Other can now access this app independent of version.")
+ acl, _, err := stub.GetPermissions(otherCtx)
+ if err != nil {
+ t.Fatalf("GetPermissions should not have failed: %v", err)
+ }
+
+ if got := acl; !reflect.DeepEqual(expected.Normalize(), got.Normalize()) {
+ t.Errorf("got %#v, expected %#v ", got, expected)
+ }
+ vlog.VI(2).Infof("Self can also access thanks to hierarchical auth.")
+ if _, _, err = stub.GetPermissions(ctx); err != nil {
+ t.Fatalf("GetPermissions should not have failed: %v", err)
+ }
+ }
+
+ vlog.VI(2).Infof("But other locations are unaffected and other cannot access.")
+ for _, path := range []string{"repo/naps", "repo/naps/v1"} {
stub := repository.ApplicationClient(path)
if _, _, err := stub.GetPermissions(otherCtx); err == nil {
- t.Fatalf("GetPermissions didn't fail for other when it should have.")
+ t.Fatalf("GetPermissions didn't fail when it should have.")
}
}
// Self gives other write perms on base.
- acl, etag, err := repostub.GetPermissions(ctx)
+ newAccessList, etag, err = repostub.GetPermissions(ctx)
if err != nil {
t.Fatalf("GetPermissions should not have failed: %v", err)
}
@@ -339,11 +395,23 @@
}
}
+ // But because application search already exists, the ACLs do not change.
+ for _, path := range []string{"repo/search", "repo/search/v1", "repo/search/v2"} {
+ stub := repository.ApplicationClient(path)
+ acl, _, err := stub.GetPermissions(otherCtx)
+ if err != nil {
+ t.Fatalf("GetPermissions should not have failed: %v", err)
+ }
+ if got := acl; !reflect.DeepEqual(expected.Normalize(), got.Normalize()) {
+ t.Errorf("got %#v, expected %#v ", got, expected)
+ }
+ }
+
// But self didn't give other AccessList modification permissions.
for _, path := range []string{"repo/search", "repo/search/v2"} {
stub := repository.ApplicationClient(path)
- if _, _, err := stub.GetPermissions(otherCtx); err == nil {
- t.Fatalf("GetPermissions didn't fail for other when it should have.")
+ if _, _, err := stub.GetPermissions(otherCtx); err != nil {
+ t.Fatalf("GetPermissions failed when it should not have for same application: %v", err)
}
}
}
diff --git a/services/application/applicationd/dispatcher.go b/services/application/applicationd/dispatcher.go
index 4c5aa1c..6a9becd 100644
--- a/services/application/applicationd/dispatcher.go
+++ b/services/application/applicationd/dispatcher.go
@@ -35,9 +35,14 @@
}
func (d *dispatcher) Lookup(suffix string) (interface{}, security.Authorizer, error) {
+ name, _, err := parse(nil, suffix)
+ if err != nil {
+ return nil, nil, err
+ }
+
auth, err := acls.NewHierarchicalAuthorizer(
naming.Join("/acls", "data"),
- naming.Join("/acls", suffix, "data"),
+ naming.Join("/acls", name, "data"),
(*applicationAccessListStore)(d.store))
if err != nil {
return nil, nil, err
diff --git a/services/application/applicationd/service.go b/services/application/applicationd/service.go
index 9632cdf..07313b5 100644
--- a/services/application/applicationd/service.go
+++ b/services/application/applicationd/service.go
@@ -11,8 +11,10 @@
"v.io/x/ref/services/mgmt/lib/fs"
"v.io/x/ref/services/repository"
+ "v.io/v23/context"
"v.io/v23/naming"
"v.io/v23/rpc"
+ "v.io/v23/security"
"v.io/v23/security/access"
"v.io/v23/services/application"
"v.io/v23/verror"
@@ -37,7 +39,7 @@
var (
ErrInvalidSuffix = verror.Register(pkgPath+".InvalidSuffix", verror.NoRetry, "{1:}{2:} invalid suffix{:_}")
ErrOperationFailed = verror.Register(pkgPath+".OperationFailed", verror.NoRetry, "{1:}{2:} operation failed{:_}")
- ErrInvalidBlessing = verror.Register(pkgPath+".InvalidBlessing", verror.NoRetry, "{1:}{2:} invalid blessing{:_}")
+ ErrNotAuthorized = verror.Register(pkgPath+".errNotAuthorized", verror.NoRetry, "{1:}{2:} none of the client's blessings are valid {:_}")
)
// NewApplicationService returns a new Application service implementation.
@@ -45,7 +47,7 @@
return &appRepoService{store: store, storeRoot: storeRoot, suffix: suffix}
}
-func parse(call rpc.ServerCall, suffix string) (string, string, error) {
+func parse(ctx *context.T, suffix string) (string, string, error) {
tokens := strings.Split(suffix, "/")
switch len(tokens) {
case 2:
@@ -53,14 +55,14 @@
case 1:
return tokens[0], "", nil
default:
- return "", "", verror.New(ErrInvalidSuffix, call.Context())
+ return "", "", verror.New(ErrInvalidSuffix, ctx)
}
}
func (i *appRepoService) Match(call rpc.ServerCall, profiles []string) (application.Envelope, error) {
vlog.VI(0).Infof("%v.Match(%v)", i.suffix, profiles)
empty := application.Envelope{}
- name, version, err := parse(call, i.suffix)
+ name, version, err := parse(call.Context(), i.suffix)
if err != nil {
return empty, err
}
@@ -88,7 +90,7 @@
func (i *appRepoService) Put(call rpc.ServerCall, profiles []string, envelope application.Envelope) error {
vlog.VI(0).Infof("%v.Put(%v, %v)", i.suffix, profiles, envelope)
- name, version, err := parse(call, i.suffix)
+ name, version, err := parse(call.Context(), i.suffix)
if err != nil {
return err
}
@@ -103,6 +105,22 @@
return err
}
+ // Only add a Permission list value if there is not already one
+ // present.
+ apath := naming.Join("/acls", name, "data")
+ aobj := i.store.BindObject(apath)
+ if _, err := aobj.Get(call); verror.ErrorID(err) == fs.ErrNotInMemStore.ID {
+ rb, _ := security.RemoteBlessingNames(call.Context())
+ if len(rb) == 0 {
+ // None of the client's blessings are valid.
+ return verror.New(ErrNotAuthorized, call.Context())
+ }
+ newacls := acls.PermissionsForBlessings(rb)
+ if _, err := aobj.Put(nil, newacls); err != nil {
+ return err
+ }
+ }
+
for _, profile := range profiles {
path := naming.Join(tname, "/applications", name, profile, version)
@@ -120,7 +138,7 @@
func (i *appRepoService) Remove(call rpc.ServerCall, profile string) error {
vlog.VI(0).Infof("%v.Remove(%v)", i.suffix, profile)
- name, version, err := parse(call, i.suffix)
+ name, version, err := parse(call.Context(), i.suffix)
if err != nil {
return err
}
@@ -231,16 +249,30 @@
}
func (i *appRepoService) GetPermissions(call rpc.ServerCall) (acl access.Permissions, etag string, err error) {
+ name, _, err := parse(call.Context(), i.suffix)
+ if err != nil {
+ return nil, "", err
+ }
i.store.Lock()
defer i.store.Unlock()
- path := naming.Join("/acls", i.suffix, "data")
- return getAccessList(i.store, path)
+ path := naming.Join("/acls", name, "data")
+
+ acl, etag, err = getAccessList(i.store, path)
+ if verror.ErrorID(err) == verror.ErrNoExist.ID {
+ return acls.NilAuthPermissions(call), "", nil
+ }
+
+ return acl, etag, err
}
func (i *appRepoService) SetPermissions(call rpc.ServerCall, acl access.Permissions, etag string) error {
+ name, _, err := parse(call.Context(), i.suffix)
+ if err != nil {
+ return err
+ }
i.store.Lock()
defer i.store.Unlock()
- path := naming.Join("/acls", i.suffix, "data")
+ path := naming.Join("/acls", name, "data")
return setAccessList(i.store, path, acl, etag)
}
diff --git a/services/binary/binarylib/service.go b/services/binary/binarylib/service.go
index f0b2b7a..c73eb0d 100644
--- a/services/binary/binarylib/service.go
+++ b/services/binary/binarylib/service.go
@@ -376,18 +376,7 @@
if os.IsNotExist(err) {
// No AccessList file found which implies a nil authorizer. This results in default authorization.
- // Therefore we return an AccessList that mimics the default authorization policy (i.e., the AccessList
- // is matched by all blessings that are either extensions of one of the local blessings or
- // can be extended to form one of the local blessings.)
- tam := make(access.Permissions)
-
- lb := security.LocalBlessingNames(call.Context())
- for _, p := range acls.PrefixPatterns(lb) {
- for _, tag := range access.AllTypicalTags() {
- tam.Add(p, string(tag))
- }
- }
- return tam, "", nil
+ return acls.NilAuthPermissions(call), "", nil
}
return acl, etag, err
}
diff --git a/services/mgmt/lib/acls/aclaccess.go b/services/mgmt/lib/acls/aclaccess.go
index 62b0896..42d4154 100644
--- a/services/mgmt/lib/acls/aclaccess.go
+++ b/services/mgmt/lib/acls/aclaccess.go
@@ -14,6 +14,7 @@
"path/filepath"
"sync"
+ "v.io/v23/rpc"
"v.io/v23/security"
"v.io/v23/security/access"
"v.io/v23/verror"
@@ -198,3 +199,18 @@
}
return tam
}
+
+// NilAuthPermissions creates an AccessList that mimics the default
+// authorization policy (i.e., the AccessList is matched by all blessings
+// that are either extensions of one of the local blessings or can be
+// extended to form one of the local blessings.)
+func NilAuthPermissions(call rpc.ServerCall) access.Permissions {
+ tam := make(access.Permissions)
+ lb := security.LocalBlessingNames(call.Context())
+ for _, p := range PrefixPatterns(lb) {
+ for _, tag := range access.AllTypicalTags() {
+ tam.Add(p, string(tag))
+ }
+ }
+ return tam
+}
