Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage): support PublicAccessPrevention #3608

Merged
merged 13 commits into from Jun 28, 2021
82 changes: 78 additions & 4 deletions storage/bucket.go
Expand Up @@ -244,6 +244,13 @@ type BucketAttrs struct {
// for more information.
UniformBucketLevelAccess UniformBucketLevelAccess

// PublicAccessPrevention is the setting for the bucket's
// PublicAccessPrevention policy, which can be used to prevent public access
// of data in the bucket. See
// https://cloud.google.com/storage/docs/public-access-prevention for more
// information.
PublicAccessPrevention PublicAccessPrevention

// DefaultObjectACL is the list of access controls to
// apply to new objects when no object ACL is provided.
DefaultObjectACL []ACLRule
Expand Down Expand Up @@ -353,6 +360,41 @@ type UniformBucketLevelAccess struct {
LockedTime time.Time
}

// PublicAccessPrevention configures the Public Access Prevention feature, which
// can be used to disallow public access to any data in a bucket. See
// https://cloud.google.com/storage/docs/public-access-prevention for more
// information.
type PublicAccessPrevention int

const (
// PublicAccessPreventionDefault is a zero value, used only if this field is
// not set in a call to GCS.
PublicAccessPreventionDefault PublicAccessPrevention = iota

// PublicAccessPreventionUnspecified corresponds to a value of "unspecified"
// and is the default for buckets.
PublicAccessPreventionUnspecified

// PublicAccessPreventionEnforced corresponds to a value of "enforced". This
// enforces Public Access Prevention on the bucket.
PublicAccessPreventionEnforced

publicAccessPreventionDefault string = ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the naming of this field might be confusing. Public Access Prevention defaults to 'unspecified'. It will never hold a blank value. I'm trying to think of another option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on this one-- I was trying to follow https://google.aip.dev/126 but it suggests "unspecified" for a zero value. 😆 Suggestions welcome!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went for "unknown" here instead.

publicAccessPreventionUnspecified = "unspecified"
publicAccessPreventionEnforced = "enforced"
)

func (p PublicAccessPrevention) String() string {
switch p {
case PublicAccessPreventionUnspecified:
return publicAccessPreventionUnspecified
case PublicAccessPreventionEnforced:
return publicAccessPreventionEnforced
default:
return publicAccessPreventionDefault
}
}

// Lifecycle is the lifecycle configuration for objects in the bucket.
type Lifecycle struct {
Rules []LifecycleRule
Expand Down Expand Up @@ -551,6 +593,7 @@ func newBucket(b *raw.Bucket) (*BucketAttrs, error) {
Website: toBucketWebsite(b.Website),
BucketPolicyOnly: toBucketPolicyOnly(b.IamConfiguration),
UniformBucketLevelAccess: toUniformBucketLevelAccess(b.IamConfiguration),
PublicAccessPrevention: toPublicAccessPrevention(b.IamConfiguration),
Etag: b.Etag,
LocationType: b.LocationType,
}, nil
Expand Down Expand Up @@ -578,11 +621,15 @@ func (b *BucketAttrs) toRawBucket() *raw.Bucket {
bb = &raw.BucketBilling{RequesterPays: true}
}
var bktIAM *raw.BucketIamConfiguration
if b.UniformBucketLevelAccess.Enabled || b.BucketPolicyOnly.Enabled {
bktIAM = &raw.BucketIamConfiguration{
UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{
if b.UniformBucketLevelAccess.Enabled || b.BucketPolicyOnly.Enabled || b.PublicAccessPrevention != PublicAccessPreventionDefault {
bktIAM = &raw.BucketIamConfiguration{}
if b.UniformBucketLevelAccess.Enabled || b.BucketPolicyOnly.Enabled {
bktIAM.UniformBucketLevelAccess = &raw.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: true,
},
}
}
if b.PublicAccessPrevention != PublicAccessPreventionDefault {
bktIAM.PublicAccessPrevention = b.PublicAccessPrevention.String()
}
}
return &raw.Bucket{
Expand Down Expand Up @@ -661,6 +708,13 @@ type BucketAttrsToUpdate struct {
// for more information.
UniformBucketLevelAccess *UniformBucketLevelAccess

// PublicAccessPrevention is the setting for the bucket's
// PublicAccessPrevention policy, which can be used to prevent public access
// of data in the bucket. See
// https://cloud.google.com/storage/docs/public-access-prevention for more
// information.
PublicAccessPrevention PublicAccessPrevention

// StorageClass is the default storage class of the bucket. This defines
// how objects in the bucket are stored and determines the SLA
// and the cost of storage. Typical values are "STANDARD", "NEARLINE",
Expand Down Expand Up @@ -771,6 +825,12 @@ func (ua *BucketAttrsToUpdate) toRawBucket() *raw.Bucket {
},
}
}
if ua.PublicAccessPrevention != PublicAccessPreventionDefault {
if rb.IamConfiguration == nil {
rb.IamConfiguration = &raw.BucketIamConfiguration{}
}
rb.IamConfiguration.PublicAccessPrevention = ua.PublicAccessPrevention.String()
}
if ua.Encryption != nil {
if ua.Encryption.DefaultKMSKeyName == "" {
rb.NullFields = append(rb.NullFields, "Encryption")
Expand Down Expand Up @@ -1139,6 +1199,20 @@ func toUniformBucketLevelAccess(b *raw.BucketIamConfiguration) UniformBucketLeve
}
}

func toPublicAccessPrevention(b *raw.BucketIamConfiguration) PublicAccessPrevention {
if b == nil {
return PublicAccessPreventionDefault
}
switch b.PublicAccessPrevention {
case publicAccessPreventionUnspecified:
return PublicAccessPreventionUnspecified
case publicAccessPreventionEnforced:
return PublicAccessPreventionEnforced
default:
return PublicAccessPreventionDefault
}
}

// Objects returns an iterator over the objects in the bucket that match the
// Query q. If q is nil, no filtering is done. Objects will be iterated over
// lexicographically by name.
Expand Down
41 changes: 41 additions & 0 deletions storage/bucket_test.go
Expand Up @@ -42,6 +42,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
},
BucketPolicyOnly: BucketPolicyOnly{Enabled: true},
UniformBucketLevelAccess: UniformBucketLevelAccess{Enabled: true},
PublicAccessPrevention: PublicAccessPreventionEnforced,
VersioningEnabled: false,
// should be ignored:
MetaGeneration: 39,
Expand Down Expand Up @@ -121,6 +122,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: true,
},
PublicAccessPrevention: "enforced",
shaffeeullah marked this conversation as resolved.
Show resolved Hide resolved
},
Versioning: nil, // ignore VersioningEnabled if false
Labels: map[string]string{"label": "value"},
Expand Down Expand Up @@ -205,6 +207,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: true,
},
PublicAccessPrevention: "enforced",
}
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
Expand All @@ -219,6 +222,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: true,
},
PublicAccessPrevention: "enforced",
}
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
Expand All @@ -234,6 +238,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: true,
},
PublicAccessPrevention: "enforced",
}
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
Expand All @@ -244,6 +249,42 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
attrs.BucketPolicyOnly = BucketPolicyOnly{}
attrs.UniformBucketLevelAccess = UniformBucketLevelAccess{}
got = attrs.toRawBucket()
want.IamConfiguration = &raw.BucketIamConfiguration{
PublicAccessPrevention: "enforced",
}
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
}

// Test that setting PublicAccessPrevention to "unspecified" leads to the
// setting being propagated in the proto.
attrs.PublicAccessPrevention = PublicAccessPreventionUnspecified
got = attrs.toRawBucket()
want.IamConfiguration = &raw.BucketIamConfiguration{
PublicAccessPrevention: "unspecified",
}
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
}

// Re-enable UBLA and confirm that it does not affect the PAP setting.
attrs.UniformBucketLevelAccess = UniformBucketLevelAccess{Enabled: true}
got = attrs.toRawBucket()
want.IamConfiguration = &raw.BucketIamConfiguration{
UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{
Enabled: true,
},
PublicAccessPrevention: "unspecified",
shaffeeullah marked this conversation as resolved.
Show resolved Hide resolved
}
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
}

// Disable UBLA and reset PAP to default. Confirm that the IAM config is set
// to nil in the proto.
attrs.UniformBucketLevelAccess = UniformBucketLevelAccess{Enabled: false}
attrs.PublicAccessPrevention = PublicAccessPreventionDefault
got = attrs.toRawBucket()
want.IamConfiguration = nil
if msg := testutil.Diff(got, want); msg != "" {
t.Errorf(msg)
Expand Down
82 changes: 82 additions & 0 deletions storage/integration_test.go
Expand Up @@ -52,6 +52,7 @@ import (
"google.golang.org/api/iterator"
itesting "google.golang.org/api/iterator/testing"
"google.golang.org/api/option"
iampb "google.golang.org/genproto/googleapis/iam/v1"
)

const (
Expand Down Expand Up @@ -575,6 +576,87 @@ func TestIntegration_UniformBucketLevelAccess(t *testing.T) {
}
}

func TestIntegration_PublicAccessPrevention(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
defer client.Close()
h := testHelper{t}

// Create a bucket with PublicAccessPrevention enforced.
bkt := client.Bucket(uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{PublicAccessPrevention: PublicAccessPreventionEnforced})
defer h.mustDeleteBucket(bkt)

// Making bucket public should fail.
policy, err := bkt.IAM().V3().Policy(ctx)
if err != nil {
t.Fatalf("fetching bucket IAM policy: %v", err)
}
policy.Bindings = append(policy.Bindings, &iampb.Binding{
Role: "roles/storage.objectViewer",
Members: []string{iam.AllUsers},
})
if err := bkt.IAM().V3().SetPolicy(ctx, policy); err == nil {
t.Error("SetPolicy: expected adding AllUsers policy to bucket should fail")
}

// Making object public via ACL should fail.
o := bkt.Object("publicAccessPrevention")
defer func() {
if err := o.Delete(ctx); err != nil {
log.Printf("failed to delete test object: %v", err)
}
}()
wc := o.NewWriter(ctx)
wc.ContentType = "text/plain"
h.mustWrite(wc, []byte("test"))
a := o.ACL()
if err := a.Set(ctx, AllUsers, RoleReader); err == nil {
t.Error("ACL.Set: expected adding AllUsers ACL to object should fail")
}

// Update PAP setting to unspecified should work and not affect UBLA setting.
attrs, err := bkt.Update(ctx, BucketAttrsToUpdate{PublicAccessPrevention: PublicAccessPreventionUnspecified})
if err != nil {
t.Fatalf("updating PublicAccessPrevention failed: %v", err)
}
if attrs.PublicAccessPrevention != PublicAccessPreventionUnspecified {
t.Errorf("updating PublicAccessPrevention: got %v, want %v", attrs.PublicAccessPrevention.String(), PublicAccessPreventionUnspecified.String())
tritone marked this conversation as resolved.
Show resolved Hide resolved
}
if attrs.UniformBucketLevelAccess.Enabled || attrs.BucketPolicyOnly.Enabled {
t.Error("updating PublicAccessPrevention changed UBLA setting")
}

// Now, making object public or making bucket public should succeed.
a = o.ACL()
if err := a.Set(ctx, AllUsers, RoleReader); err != nil {
t.Errorf("ACL.Set: making object public failed: %v", err)
}
policy, err = bkt.IAM().V3().Policy(ctx)
if err != nil {
t.Fatalf("fetching bucket IAM policy: %v", err)
}
policy.Bindings = append(policy.Bindings, &iampb.Binding{
Role: "roles/storage.objectViewer",
Members: []string{iam.AllUsers},
})
if err := bkt.IAM().V3().SetPolicy(ctx, policy); err != nil {
t.Errorf("SetPolicy: making bucket public failed: %v", err)
}

// Updating UBLA should not affect PAP setting.
attrs, err = bkt.Update(ctx, BucketAttrsToUpdate{UniformBucketLevelAccess: &UniformBucketLevelAccess{Enabled: true}})
if err != nil {
t.Fatalf("updating UBLA failed: %v", err)
}
if !attrs.UniformBucketLevelAccess.Enabled {
t.Error("updating UBLA: got UBLA not enabled, want enabled")
}
if attrs.PublicAccessPrevention != PublicAccessPreventionUnspecified {
t.Errorf("updating UBLA: got %v, want %v", attrs.PublicAccessPrevention.String(), PublicAccessPreventionUnspecified.String())
}
}

func TestIntegration_ConditionalDelete(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
Expand Down