s3: list every bucket when auth is off (#12077)

* s3: list every bucket when auth is off

With no identities configured, every request may create, read and
write any bucket, but ListBuckets treated the caller as an
unauthenticated user and returned an empty list, so `aws s3 ls` showed
nothing. List all buckets in that mode, with prefix, max-buckets and
continuation tokens as before. Listing with auth on is unchanged.

* s3: cover the auth-on anonymous ListBuckets case

The no-auth listing change also moves the nil-identity gate; pin the
security-relevant direction that a request without an identity still
sees no buckets once auth enforcement is on.

* s3: verify ListBuckets continuation token reaches the next page

---------

Co-authored-by: user.email <chrislusf@users.noreply.github.com>

weed/s3api/s3api_bucket_handlers.go

@@ -78,19 +78,22 @@ func (s3a *S3ApiServer) ListBucketsHandler(w http.ResponseWriter, r *http.Reques
 
 	var buckets []ListAllMyBucketsEntry
 	var nextToken string
-	// Unauthenticated users should not see any buckets
+	var err error
+	// Unauthenticated users should not see any buckets, unless auth is off:
+	// then every request may use every bucket, so all of them are listed.
 	if identity != nil {
-		var err error
 		if fromIndex, granted := s3a.iam.canListBucketsFromOwnerIndex(r, identity); fromIndex && s3a.bucketOwnerIndexReady() {
 			buckets, nextToken, err = s3a.listBucketsFromOwnerIndex(r, identity, granted, prefix, startAfter, maxBuckets)
 		} else {
 			buckets, nextToken, err = s3a.scanVisibleBuckets(r, identity, prefix, startAfter, maxBuckets)
 		}
-		if err != nil {
-			glog.Errorf("ListBucketsHandler: %v", err)
-			s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
-			return
-		}
+	} else if !s3a.iam.isEnabled() {
+		buckets, nextToken, err = s3a.scanVisibleBuckets(r, nil, prefix, startAfter, maxBuckets)
+	}
+	if err != nil {
+		glog.Errorf("ListBucketsHandler: %v", err)
+		s3err.WriteErrorResponse(w, r, s3err.ErrInternalError)
+		return
 	}
 
 	response := ListAllMyBucketsResult{
@@ -151,6 +154,9 @@ func decodeContinuationToken(token string) (string, error) {
 // bucketVisibleToIdentity reports whether ListBuckets should include the bucket:
 // the identity owns it or has explicit permission to list it.
 func (s3a *S3ApiServer) bucketVisibleToIdentity(r *http.Request, bucket, owner string, identity *Identity) bool {
+	if !s3a.iam.isEnabled() {
+		return true
+	}
 	if isBucketOwnedByIdentity(owner, identity) {
 		return true
 	}

weed/s3api/s3api_bucket_handlers_list_test.go

@@ -1,14 +1,19 @@
 package s3api
 
 import (
+	"encoding/xml"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"strings"
 	"testing"
 
+	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
 	"github.com/seaweedfs/seaweedfs/weed/pb/iam_pb"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGetListBucketsArgs(t *testing.T) {
@@ -220,3 +225,72 @@ func TestContinuationTokenRoundTrip(t *testing.T) {
 		t.Error("non-base64 token should not decode")
 	}
 }
+
+// With auth off every request may use every bucket, so ListBuckets lists them
+// all instead of treating the caller as an unauthenticated user.
+func TestListBucketsWithoutAuthListsEveryBucket(t *testing.T) {
+	f := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{
+		"/buckets": {
+			{Name: ".system", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+			{Name: "alpha", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+			{Name: "beta", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+		},
+	}}
+	s3a := newBucketDeleteTestServer(t, f, false)
+
+	for _, tt := range []struct {
+		query string
+		want  []string
+	}{
+		{query: "", want: []string{"alpha", "beta"}},
+		{query: "?prefix=b", want: []string{"beta"}},
+	} {
+		rec := httptest.NewRecorder()
+		s3a.ListBucketsHandler(rec, httptest.NewRequest(http.MethodGet, "/"+tt.query, nil))
+		require.Equal(t, http.StatusOK, rec.Code, tt.query)
+		var result ListAllMyBucketsResult
+		require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &result), tt.query)
+		var names []string
+		for _, b := range result.Buckets.Bucket {
+			names = append(names, b.Name)
+		}
+		assert.Equal(t, tt.want, names, tt.query)
+	}
+
+	// A truncated page must hand back a token that reaches the next page.
+	rec := httptest.NewRecorder()
+	s3a.ListBucketsHandler(rec, httptest.NewRequest(http.MethodGet, "/?max-buckets=1", nil))
+	require.Equal(t, http.StatusOK, rec.Code)
+	var first ListAllMyBucketsResult
+	require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &first))
+	require.Len(t, first.Buckets.Bucket, 1)
+	assert.Equal(t, "alpha", first.Buckets.Bucket[0].Name)
+	require.NotEmpty(t, first.ContinuationToken)
+
+	rec = httptest.NewRecorder()
+	s3a.ListBucketsHandler(rec, httptest.NewRequest(http.MethodGet, "/?max-buckets=1&continuation-token="+url.QueryEscape(first.ContinuationToken), nil))
+	require.Equal(t, http.StatusOK, rec.Code)
+	var second ListAllMyBucketsResult
+	require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &second))
+	require.Len(t, second.Buckets.Bucket, 1)
+	assert.Equal(t, "beta", second.Buckets.Bucket[0].Name)
+	assert.Empty(t, second.ContinuationToken)
+}
+
+// With auth on, a request carrying no identity still lists no buckets.
+func TestListBucketsWithAuthListsNothingForAnonymous(t *testing.T) {
+	f := &fakeBucketDeleteFiler{entriesByDir: map[string][]*filer_pb.Entry{
+		"/buckets": {
+			{Name: "alpha", IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}},
+		},
+	}}
+	s3a := newBucketDeleteTestServer(t, f, false)
+	s3a.iam.isAuthEnabled = true
+
+	rec := httptest.NewRecorder()
+	s3a.ListBucketsHandler(rec, httptest.NewRequest(http.MethodGet, "/", nil))
+	require.Equal(t, http.StatusOK, rec.Code)
+	var result ListAllMyBucketsResult
+	require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &result))
+	assert.Empty(t, result.Buckets.Bucket)
+}