Skip to content

Commit

Permalink
satellite/metainfo: Use project-level attribution at bucket level
Browse files Browse the repository at this point in the history 
If project.UserAgent is set, use this for bucket.UserAgent on bucket
creation. Otherwise, set bucket attribution as before (getting UserAgent
from request headers).

Tests were updated to create the bucket with a different user, added as
a project member. Otherwise, the tests do not catch the bug.

Change-Id: I7ecf79a8eac5957eed361cbea94823190f58b776
  • Loading branch information
@mobyvb
mobyvb committed Jun 7, 2022
1 parent 737d7c7 commit 5b79970
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 34 deletions.
13 changes: 7 additions & 6 deletions satellite/metainfo/attribution.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ import (
// MaxUserAgentLength is the maximum allowable length of the User Agent.
const MaxUserAgentLength = 500

// ensureAttribution ensures that the bucketName has the partner information specified by keyInfo partner ID or the header user agent.
// ensureAttribution ensures that the bucketName has the partner information specified by project-level user agent, header user agent, or keyInfo partner ID.
// PartnerID from keyInfo is a value associated with registered user and prevails over header user agent.
//
// Assumes that the user has permissions sufficient for authenticating.
func (endpoint *Endpoint) ensureAttribution(ctx context.Context, header *pb.RequestHeader, keyInfo *console.APIKeyInfo, bucketName []byte) (err error) {
func (endpoint *Endpoint) ensureAttribution(ctx context.Context, header *pb.RequestHeader, keyInfo *console.APIKeyInfo, bucketName, projectUserAgent []byte) (err error) {
defer mon.Task()(&ctx)(&err)

if header == nil {
return rpcstatus.Error(rpcstatus.InvalidArgument, "header is nil")
}
if len(header.UserAgent) == 0 && keyInfo.PartnerID.IsZero() && keyInfo.UserAgent == nil {
if keyInfo.PartnerID.IsZero() && len(header.UserAgent) == 0 && len(keyInfo.UserAgent) == 0 && len(projectUserAgent) == 0 {
return nil
}

Expand All @@ -49,13 +49,14 @@ func (endpoint *Endpoint) ensureAttribution(ctx context.Context, header *pb.Requ

partnerID := keyInfo.PartnerID
userAgent := keyInfo.UserAgent
if len(projectUserAgent) > 0 {
userAgent = projectUserAgent
}

// first check keyInfo (user) attribution
if partnerID.IsZero() && userAgent == nil {
// otherwise, use header (partner tool) as attribution
userAgent = header.UserAgent
if userAgent == nil {
return nil
}
}

userAgent, err = TrimUserAgent(userAgent)
Expand Down
63 changes: 40 additions & 23 deletions satellite/metainfo/attribution_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"storj.io/common/memory"
"storj.io/common/testcontext"
"storj.io/common/testrand"
"storj.io/common/uuid"
"storj.io/storj/private/testplanet"
"storj.io/storj/satellite/attribution"
"storj.io/storj/satellite/console"
Expand Down Expand Up @@ -68,7 +69,7 @@ func TestBucketAttribution(t *testing.T) {
expectedAttribution []byte
}{
{signupPartner: nil, userAgent: nil, expectedAttribution: nil},
{signupPartner: []byte(""), userAgent: []byte(""), expectedAttribution: []byte("")},
{signupPartner: []byte(""), userAgent: []byte(""), expectedAttribution: nil},
{signupPartner: []byte("Minio"), userAgent: nil, expectedAttribution: []byte("Minio")},
{signupPartner: []byte("Minio"), userAgent: []byte("Minio"), expectedAttribution: []byte("Minio")},
{signupPartner: []byte("Minio"), userAgent: []byte("Zenko"), expectedAttribution: []byte("Minio")},
Expand All @@ -79,46 +80,62 @@ func TestBucketAttribution(t *testing.T) {

satellite := planet.Satellites[0]

user, err := satellite.AddUser(ctx, console.CreateUser{
user1, err := satellite.AddUser(ctx, console.CreateUser{
FullName: "Test User " + strconv.Itoa(i),
Email: "user@test" + strconv.Itoa(i),
PartnerID: "",
UserAgent: tt.signupPartner,
}, 1)
require.NoError(t, err, errTag)

satProject, err := satellite.AddProject(ctx, user.ID, "test"+strconv.Itoa(i))
satProject, err := satellite.AddProject(ctx, user1.ID, "test"+strconv.Itoa(i))
require.NoError(t, err, errTag)

authCtx, err := satellite.AuthenticatedContext(ctx, user.ID)
// add a second user to the project, and create the api key with the new user to ensure that
// the project owner's attribution is used for a new bucket, even if someone else creates it.
user2, err := satellite.AddUser(ctx, console.CreateUser{
FullName: "Test User 2" + strconv.Itoa(i),
Email: "user2@test" + strconv.Itoa(i),
UserAgent: tt.signupPartner,
}, 1)
require.NoError(t, err, errTag)
_, err = satellite.DB.Console().ProjectMembers().Insert(ctx, user2.ID, satProject.ID)
require.NoError(t, err)

_, apiKeyInfo, err := satellite.API.Console.Service.CreateAPIKey(authCtx, satProject.ID, "root")
require.NoError(t, err, errTag)
createBucketAndCheckAttribution := func(userID uuid.UUID, apiKeyName, bucketName string) {
authCtx, err := satellite.AuthenticatedContext(ctx, userID)
require.NoError(t, err, errTag)

config := uplink.Config{
UserAgent: string(tt.userAgent),
}
access, err := config.RequestAccessWithPassphrase(ctx, satellite.NodeURL().String(), apiKeyInfo.Serialize(), "mypassphrase")
require.NoError(t, err, errTag)
_, apiKeyInfo, err := satellite.API.Console.Service.CreateAPIKey(authCtx, satProject.ID, apiKeyName)
require.NoError(t, err, errTag)

project, err := config.OpenProject(ctx, access)
require.NoError(t, err, errTag)
config := uplink.Config{
UserAgent: string(tt.userAgent),
}
access, err := config.RequestAccessWithPassphrase(ctx, satellite.NodeURL().String(), apiKeyInfo.Serialize(), "mypassphrase")
require.NoError(t, err, errTag)

_, err = project.CreateBucket(ctx, "bucket")
require.NoError(t, err, errTag)
project, err := config.OpenProject(ctx, access)
require.NoError(t, err, errTag)

bucketInfo, err := satellite.API.Buckets.Service.GetBucket(ctx, []byte("bucket"), satProject.ID)
require.NoError(t, err, errTag)
assert.Equal(t, tt.expectedAttribution, bucketInfo.UserAgent, errTag)
_, err = project.CreateBucket(ctx, bucketName)
require.NoError(t, err, errTag)

attributionInfo, err := planet.Satellites[0].DB.Attribution().Get(ctx, satProject.ID, []byte("bucket"))
if tt.expectedAttribution == nil {
assert.True(t, attribution.ErrBucketNotAttributed.Has(err), errTag)
} else {
bucketInfo, err := satellite.API.Buckets.Service.GetBucket(ctx, []byte(bucketName), satProject.ID)
require.NoError(t, err, errTag)
assert.Equal(t, tt.expectedAttribution, attributionInfo.UserAgent, errTag)
assert.Equal(t, tt.expectedAttribution, bucketInfo.UserAgent, errTag)

attributionInfo, err := planet.Satellites[0].DB.Attribution().Get(ctx, satProject.ID, []byte(bucketName))
if tt.expectedAttribution == nil {
assert.True(t, attribution.ErrBucketNotAttributed.Has(err), errTag)
} else {
require.NoError(t, err, errTag)
assert.Equal(t, tt.expectedAttribution, attributionInfo.UserAgent, errTag)
}
}

createBucketAndCheckAttribution(user1.ID, "apikey1", "bucket1")
createBucketAndCheckAttribution(user2.ID, "apikey2", "bucket2")
}
})
}
Expand Down
8 changes: 4 additions & 4 deletions satellite/metainfo/endpoint_bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,17 @@ func (endpoint *Endpoint) CreateBucket(ctx context.Context, req *pb.BucketCreate
return nil, rpcstatus.Error(rpcstatus.Internal, err.Error())
} else if exists {
// When the bucket exists, try to set the attribution.
if err := endpoint.ensureAttribution(ctx, req.Header, keyInfo, req.GetName()); err != nil {
if err := endpoint.ensureAttribution(ctx, req.Header, keyInfo, req.GetName(), nil); err != nil {
return nil, err
}
return nil, rpcstatus.Error(rpcstatus.AlreadyExists, "bucket already exists")
}

// check if project has exceeded its allocated bucket limit
maxBuckets, err := endpoint.projects.GetMaxBuckets(ctx, keyInfo.ProjectID)
project, err := endpoint.projects.Get(ctx, keyInfo.ProjectID)
if err != nil {
return nil, err
}
maxBuckets := project.MaxBuckets
if maxBuckets == nil {
defaultMaxBuckets := endpoint.config.ProjectLimits.MaxBuckets
maxBuckets = &defaultMaxBuckets
Expand All @@ -118,7 +118,7 @@ func (endpoint *Endpoint) CreateBucket(ctx context.Context, req *pb.BucketCreate
}

// Once we have created the bucket, we can try setting the attribution.
if err := endpoint.ensureAttribution(ctx, req.Header, keyInfo, req.GetName()); err != nil {
if err := endpoint.ensureAttribution(ctx, req.Header, keyInfo, req.GetName(), project.UserAgent); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion satellite/metainfo/endpoint_object.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (endpoint *Endpoint) BeginObject(ctx context.Context, req *pb.ObjectBeginRe
}
}

if err := endpoint.ensureAttribution(ctx, req.Header, keyInfo, req.Bucket); err != nil {
if err := endpoint.ensureAttribution(ctx, req.Header, keyInfo, req.Bucket, nil); err != nil {
return nil, err
}

Expand Down

1 comment on commit 5b79970

@storjrobot
Copy link

Choose a reason for hiding this comment

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

This commit has been mentioned on Storj Community Forum (official). There might be relevant details there:

https://forum.storj.io/t/release-preparation-v1-57/18810/1

Please sign in to comment.