allow reading bucket metadata with restricted keys (#2321)

Change-Id: I47d3a2f5f02744ae6c51d54963cdf2dff24134e2

pkg/macaroon/apikey.go

@@ -151,6 +151,30 @@ func (a *APIKey) Serialize() string {
 
 // Allows returns true if the provided action is allowed by the caveat.
 func (c *Caveat) Allows(action Action) bool {
+	// if the action is after the caveat's "not after" field, then it is invalid
+	if c.NotAfter != nil && action.Time.After(*c.NotAfter) {
+		return false
+	}
+	// if the caveat's "not before" field is *after* the action, then the action
+	// is before the "not before" field and it is invalid
+	if c.NotBefore != nil && c.NotBefore.After(action.Time) {
+		return false
+	}
+
+	// we want to always allow reads for bucket metadata, perhaps filtered by the
+	// buckets in the allowed paths.
+	if action.Op == ActionRead && len(action.EncryptedPath) == 0 {
+		if len(c.AllowedPaths) == 0 {
+			return true
+		}
+		for _, path := range c.AllowedPaths {
+			if bytes.Equal(path.Bucket, action.Bucket) {
+				return true
+			}
+		}
+		return false
+	}
+
 	switch action.Op {
 	case ActionRead:
 		if c.DisallowReads {
@@ -174,17 +198,7 @@ func (c *Caveat) Allows(action Action) bool {
 		return false
 	}
 
-	// if the action is after the caveat's "not after" field, then it is invalid
+	if len(c.AllowedPaths) > 0 && action.Op != ActionProjectInfo {
-	if c.NotAfter != nil && action.Time.After(*c.NotAfter) {
-		return false
-	}
-	// if the caveat's "not before" field is *after* the action, then the action
-	// is before the "not before" field and it is invalid
-	if c.NotBefore != nil && c.NotBefore.After(action.Time) {
-		return false
-	}
-
-	if len(c.AllowedPaths) > 0 {
 		found := false
 		for _, path := range c.AllowedPaths {
 			if bytes.Equal(action.Bucket, path.Bucket) &&

pkg/storage/streams/path.go

@@ -4,10 +4,10 @@
 package streams
 
 import (
-	"bytes"
+	"strings"
-	"context"
 
 	"storj.io/storj/pkg/paths"
+	"storj.io/storj/pkg/storj"
 )
 
 // Path is a representation of an object path within a bucket
@@ -30,19 +30,15 @@ func (p Path) Raw() []byte { return append([]byte(nil), p.raw...) }
 func (p Path) String() string { return string(p.raw) }
 
 // ParsePath returns a new Path with the given raw bytes.
-func ParsePath(ctx context.Context, raw []byte) (path Path, err error) {
+func ParsePath(raw storj.Path) (path Path) {
-	defer mon.Task()(&ctx)(&err)
+	// A path may contain a bucket and an unencrypted path.
-
+	parts := strings.SplitN(raw, "/", 2)
-	// A path must contain a bucket and maybe an unencrypted path.
+	path.bucket = parts[0]
-	parts := bytes.SplitN(raw, []byte("/"), 2)
-
-	path.raw = raw
-	path.bucket = string(parts[0])
 	if len(parts) > 1 {
-		path.unencPath = paths.NewUnencrypted(string(parts[1]))
+		path.unencPath = paths.NewUnencrypted(parts[1])
 	}
-
+	path.raw = []byte(raw)
-	return path, nil
+	return path
 }
 
 // CreatePath will create a Path for the provided information.

pkg/storage/streams/shim.go

@@ -41,56 +41,35 @@ func NewStreamStore(segments segments.Store, segmentSize int64, encStore *encryp
 func (s *shimStore) Meta(ctx context.Context, path storj.Path, pathCipher storj.Cipher) (_ Meta, err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	streamsPath, err := ParsePath(ctx, []byte(path))
+	return s.store.Meta(ctx, ParsePath(path), pathCipher)
-	if err != nil {
-		return Meta{}, err
-	}
-	return s.store.Meta(ctx, streamsPath, pathCipher)
 }
 
 // Get parses the passed in path and dispatches to the typed store.
 func (s *shimStore) Get(ctx context.Context, path storj.Path, pathCipher storj.Cipher) (_ ranger.Ranger, _ Meta, err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	streamsPath, err := ParsePath(ctx, []byte(path))
+	return s.store.Get(ctx, ParsePath(path), pathCipher)
-	if err != nil {
-		return nil, Meta{}, err
-	}
-	return s.store.Get(ctx, streamsPath, pathCipher)
 }
 
 // Put parses the passed in path and dispatches to the typed store.
 func (s *shimStore) Put(ctx context.Context, path storj.Path, pathCipher storj.Cipher, data io.Reader, metadata []byte, expiration time.Time) (_ Meta, err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	streamsPath, err := ParsePath(ctx, []byte(path))
+	return s.store.Put(ctx, ParsePath(path), pathCipher, data, metadata, expiration)
-	if err != nil {
-		return Meta{}, err
-	}
-	return s.store.Put(ctx, streamsPath, pathCipher, data, metadata, expiration)
 }
 
 // Delete parses the passed in path and dispatches to the typed store.
 func (s *shimStore) Delete(ctx context.Context, path storj.Path, pathCipher storj.Cipher) (err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	streamsPath, err := ParsePath(ctx, []byte(path))
+	return s.store.Delete(ctx, ParsePath(path), pathCipher)
-	if err != nil {
-		return err
-	}
-	return s.store.Delete(ctx, streamsPath, pathCipher)
 }
 
 // List parses the passed in path and dispatches to the typed store.
 func (s *shimStore) List(ctx context.Context, prefix storj.Path, startAfter storj.Path, endBefore storj.Path, pathCipher storj.Cipher, recursive bool, limit int, metaFlags uint32) (items []ListItem, more bool, err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	// TODO: list is maybe wrong?
+	return s.store.List(ctx, ParsePath(prefix), startAfter, endBefore, pathCipher, recursive, limit, metaFlags)
-	streamsPrefix, err := ParsePath(ctx, []byte(prefix))
-	if err != nil {
-		return nil, false, err
-	}
-	return s.store.List(ctx, streamsPrefix, startAfter, endBefore, pathCipher, recursive, limit, metaFlags)
 }
 
 // EncryptAfterBucket encrypts a path without encrypting its first element. This is a legacy function
@@ -118,13 +97,8 @@ func DecryptStreamInfo(ctx context.Context, streamMetaBytes []byte, path storj.P
 	streamInfo []byte, streamMeta pb.StreamMeta, err error) {
 	defer mon.Task()(&ctx)(&err)
 
-	streamsPath, err := ParsePath(ctx, []byte(path))
-	if err != nil {
-		return nil, pb.StreamMeta{}, err
-	}
-
 	store := encryption.NewStore()
 	store.SetDefaultKey(rootKey)
 
-	return TypedDecryptStreamInfo(ctx, streamMetaBytes, streamsPath, store)
+	return TypedDecryptStreamInfo(ctx, streamMetaBytes, ParsePath(path), store)
 }

satellite/metainfo/metainfo_test.go

@@ -96,6 +96,7 @@ func TestRestrictedAPIKey(t *testing.T) {
 		ReadSegmentAllowed   bool
 		DeleteSegmentAllowed bool
 		ListSegmentsAllowed  bool
+		ReadBucketAllowed    bool
 	}{
 		{ // Everything disallowed
 			Caveat: macaroon.Caveat{
@@ -104,6 +105,7 @@ func TestRestrictedAPIKey(t *testing.T) {
 				DisallowLists:   true,
 				DisallowDeletes: true,
 			},
+			ReadBucketAllowed: true,
 		},
 
 		{ // Read only
@@ -114,6 +116,7 @@ func TestRestrictedAPIKey(t *testing.T) {
 			SegmentInfoAllowed:  true,
 			ReadSegmentAllowed:  true,
 			ListSegmentsAllowed: true,
+			ReadBucketAllowed:   true,
 		},
 
 		{ // Write only
@@ -124,6 +127,7 @@ func TestRestrictedAPIKey(t *testing.T) {
 			CreateSegmentAllowed: true,
 			CommitSegmentAllowed: true,
 			DeleteSegmentAllowed: true,
+			ReadBucketAllowed:    true,
 		},
 
 		{ // Bucket restriction
@@ -141,6 +145,7 @@ func TestRestrictedAPIKey(t *testing.T) {
 					EncryptedPathPrefix: []byte("otherpath"),
 				}},
 			},
+			ReadBucketAllowed: true,
 		},
 
 		{ // Time restriction after
@@ -181,6 +186,8 @@ func TestRestrictedAPIKey(t *testing.T) {
 		_, _, err = client.ListSegments(ctx, "testbucket", "testpath", "", "", true, 1, 0)
 		assertUnauthenticated(t, err, test.ListSegmentsAllowed)
 
+		_, _, err = client.ReadSegment(ctx, "testbucket", "", -1)
+		assertUnauthenticated(t, err, test.ReadBucketAllowed)
 	}
 }