Skip to content

Commit

Permalink
Merge pull request #736 from SiaFoundation/chris/fix-bucket-policy
Browse files Browse the repository at this point in the history
Fix an issue where bucket policy couldn't be set from "public" back to private anymore
  • Loading branch information
ChrisSchinnerl committed Nov 15, 2023
2 parents 641aae3 + c2a0fca commit 37e4acb
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 16 deletions.
37 changes: 24 additions & 13 deletions internal/testing/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,24 +177,21 @@ func TestS3Authentication(t *testing.T) {
defer cluster.Shutdown()
tt := cluster.tt

assertAuth := func(c *minio.Client, shouldWork bool) {
assertAuth := func(c *minio.Core, shouldWork bool) {
t.Helper()
resp := c.ListObjects(context.Background(), api.DefaultBucketName, minio.ListObjectsOptions{})
for obj := range resp {
err := obj.Err
if shouldWork && err != nil {
t.Fatal(err)
} else if !shouldWork && err == nil {
t.Fatal("expected error")
} else if !shouldWork && err != nil && !strings.Contains(err.Error(), "AccessDenied") {
t.Fatal("wrong error")
}
_, err := c.ListObjectsV2(api.DefaultBucketName, "/", "", "", "", 100)
if shouldWork && err != nil {
t.Fatal(err)
} else if !shouldWork && err == nil {
t.Fatal("expected error")
} else if !shouldWork && err != nil && !strings.Contains(err.Error(), "AccessDenied") {
t.Fatal("wrong error")
}
}

// Create client.
url := cluster.S3.EndpointURL().Host
s3Unauthenticated, err := minio.New(url, &minio.Options{
s3Unauthenticated, err := minio.NewCore(url, &minio.Options{
Creds: nil, // no authentication
})
tt.OK(err)
Expand All @@ -203,7 +200,7 @@ func TestS3Authentication(t *testing.T) {
assertAuth(s3Unauthenticated, false)

// Create client with credentials and try again..
s3Authenticated, err := minio.New(url, &minio.Options{
s3Authenticated, err := minio.NewCore(url, &minio.Options{
Creds: testS3Credentials,
})
tt.OK(err)
Expand All @@ -216,6 +213,13 @@ func TestS3Authentication(t *testing.T) {
PublicReadAccess: true,
}))

// Check that the policy was updated.
b, err := cluster.Bus.Bucket(context.Background(), api.DefaultBucketName)
tt.OK(err)
if b.Policy.PublicReadAccess != true {
t.Fatal("expected public read access")
}

// Listing should work now.
assertAuth(s3Unauthenticated, true)

Expand All @@ -224,6 +228,13 @@ func TestS3Authentication(t *testing.T) {
PublicReadAccess: false,
}))

// Check that the policy was updated.
b, err = cluster.Bus.Bucket(context.Background(), api.DefaultBucketName)
tt.OK(err)
if b.Policy.PublicReadAccess == true {
t.Fatal("expected no public read access")
}

// Listing should not work now.
assertAuth(s3Unauthenticated, false)
}
Expand Down
12 changes: 9 additions & 3 deletions stores/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package stores

import (
"context"
"encoding/json"
"errors"
"fmt"
"math"
Expand Down Expand Up @@ -553,13 +554,18 @@ func (s *SQLStore) CreateBucket(ctx context.Context, bucket string, policy api.B
}

func (s *SQLStore) UpdateBucketPolicy(ctx context.Context, bucket string, policy api.BucketPolicy) error {
b, err := json.Marshal(policy)
if err != nil {
return err
}
return s.retryTransaction(func(tx *gorm.DB) error {
return tx.
Model(&dbBucket{}).
Where("name", bucket).
Updates(dbBucket{
Policy: policy,
}).
Updates(map[string]interface{}{
"policy": string(b),
},
).
Error
})
}
Expand Down

0 comments on commit 37e4acb

Please sign in to comment.