diff --git a/pkg/minio/api-router.go b/pkg/minio/api-router.go index 61b4dec3..620c83cb 100644 --- a/pkg/minio/api-router.go +++ b/pkg/minio/api-router.go @@ -22,8 +22,8 @@ func RegisterAPIRouter(router *mux.Router, layer *gw.MultiTenancyLayer, domainNa CacheAPI: func() cmd.CacheObjectLayer { return nil }, }, corsAllowedOrigins} - // limit the conccurrency of uploads and downloads per macaroon head - limit := middleware.NewMacaroonLimiter(concurrentAllowed, + // limit the conccurrency of uploads and downloads + limit := middleware.NewConcurrentRequestsLimiter(concurrentAllowed, func(w http.ResponseWriter, r *http.Request) { err := cmd.APIError{ Code: "SlowDown", // necessary to return a RetryAfter header diff --git a/pkg/server/middleware/limiter.go b/pkg/server/middleware/limiter.go index f9096952..55e0fb44 100644 --- a/pkg/server/middleware/limiter.go +++ b/pkg/server/middleware/limiter.go @@ -10,23 +10,30 @@ import ( "storj.io/common/grant" ) -// NewMacaroonLimiter constructs a Limiter that limits based on macaroon credentials. +// NewConcurrentRequestsLimiter constructs a Limiter that limits using a key from credentials. // It relies on the AccessKey middleware being run to append credentials to the request context. -func NewMacaroonLimiter(allowed uint, limitFunc func(w http.ResponseWriter, r *http.Request)) *Limiter { - return NewLimiter(allowed, getRequestMacaroonHead, limitFunc) +func NewConcurrentRequestsLimiter(allowed uint, limitFunc func(w http.ResponseWriter, r *http.Request)) *Limiter { + return NewLimiter(allowed, getLimitKey, limitFunc) } -// getRequestMacaroonHead gets the macaroon head corresponding to the current request. -// Macaroon head is the best available criteria for associating a request to a user. -func getRequestMacaroonHead(r *http.Request) (ip string, err error) { +// getLimitKey retrieves a key used to identify the user for limiting requests. +func getLimitKey(r *http.Request) (identifier string, err error) { credentials := GetAccess(r.Context()) if credentials == nil || credentials.AccessGrant == "" { return "", ParseV4CredentialError.New("missing access grant") } + if credentials.PublicProjectID != "" { + return credentials.PublicProjectID, nil + } + + // fallback to macaroon head if the credentials don't contain public project ID. This is likely because the + // authservice record is old and hasn't been backfilled with the ID yet, or the ID couldn't be resolved due + // to a connectivity issue between authservice and the satellite. access, err := grant.ParseAccess(credentials.AccessGrant) if err != nil { return "", err } + return string(access.APIKey.Head()), err } @@ -40,7 +47,7 @@ type Limiter struct { m sync.RWMutex } -// NewLimiter constructs a concurrency Limiter. Error and Limit functions are user defined +// NewLimiter constructs a concurrency Limiter. Error and Limit functions are user defined // in part because referencing the "minio" package here would cause an import loop. func NewLimiter(allowed uint, keyFunc func(*http.Request) (string, error), limitFunc func(w http.ResponseWriter, r *http.Request)) *Limiter { return &Limiter{ diff --git a/pkg/server/middleware/limiter_test.go b/pkg/server/middleware/limiter_test.go index b140b876..52aad2a9 100644 --- a/pkg/server/middleware/limiter_test.go +++ b/pkg/server/middleware/limiter_test.go @@ -17,16 +17,17 @@ import ( "storj.io/common/grant" "storj.io/common/macaroon" "storj.io/common/testcontext" + "storj.io/common/testrand" "storj.io/edge/pkg/authclient" ) const maxConncurrent = 3 -func TestNewMacaroonLimiter(t *testing.T) { +func TestNewConcurrentRequestsLimiter(t *testing.T) { var next = make(chan struct{}) - var done = make(chan struct{}, maxConncurrent*2*3) + var done = make(chan struct{}, maxConncurrent*2*4) var allTests = make(chan struct{}) - rateLimiter := NewMacaroonLimiter(maxConncurrent, + rateLimiter := NewConcurrentRequestsLimiter(maxConncurrent, func(w http.ResponseWriter, r *http.Request) { next <- struct{}{} // create in-order results http.Error(w, "", http.StatusTooManyRequests) @@ -45,27 +46,31 @@ func TestNewMacaroonLimiter(t *testing.T) { })) // expect burst number of successes - creds1 := getCredentials(t) - testWithMacaroon(ctx, t, creds1, handler, next, done) - // expect similar results for a different apiKey / macaroon - creds2 := getCredentials(t) - testWithMacaroon(ctx, t, creds2, handler, next, done) - // expect similar results for a different apiKey / macaroon - creds3 := getCredentials(t) - testWithMacaroon(ctx, t, creds3, handler, next, done) + creds1 := getCredentials(t, true) + testWithCredentials(ctx, t, creds1, handler, next, done) + // expect similar results for a different project + creds2 := getCredentials(t, true) + testWithCredentials(ctx, t, creds2, handler, next, done) + + creds3 := getCredentials(t, false) + testWithCredentials(ctx, t, creds3, handler, next, done) + // expect similar results for a different macaroon head + creds4 := getCredentials(t, false) + testWithCredentials(ctx, t, creds4, handler, next, done) close(allTests) // wait for previous responses to arrive so we can retest cred1 - for x := 0; x < maxConncurrent*2*3; x++ { + for x := 0; x < maxConncurrent*2*4; x++ { <-done } - // expect burst number of successes with cred1 again + // expect burst number of successes again allTests = make(chan struct{}) - testWithMacaroon(ctx, t, creds1, handler, next, done) + testWithCredentials(ctx, t, creds1, handler, next, done) + testWithCredentials(ctx, t, creds3, handler, next, done) close(allTests) } -func testWithMacaroon(ctx *testcontext.Context, t *testing.T, creds *Credentials, handler http.Handler, next, done chan struct{}) { +func testWithCredentials(ctx *testcontext.Context, t *testing.T, creds *Credentials, handler http.Handler, next, done chan struct{}) { // expect maxConncurrent HTTP 200s, then maxConncurrent HTTP 429s for x := 0; x < maxConncurrent*2; x++ { localX := x @@ -92,7 +97,7 @@ func doRequest(ctx context.Context, t *testing.T, creds *Credentials, handler ht return rr.Code } -func getCredentials(t *testing.T) *Credentials { +func getCredentials(t *testing.T, includePublicProjectID bool) *Credentials { apiKey, err := macaroon.NewAPIKey([]byte("secret")) require.NoError(t, err) ag := grant.Access{ @@ -102,7 +107,7 @@ func getCredentials(t *testing.T) *Credentials { } grant, err := ag.Serialize() require.NoError(t, err) - return &Credentials{ + credentials := Credentials{ AccessKey: "accessKey", AuthServiceResponse: authclient.AuthServiceResponse{ AccessGrant: grant, @@ -111,6 +116,10 @@ func getCredentials(t *testing.T) *Credentials { }, Error: nil, } + if includePublicProjectID { + credentials.PublicProjectID = testrand.UUID().String() + } + return &credentials } func simpleKeyFunc(r *http.Request) (string, error) {