diff --git a/go.mod b/go.mod index be90c9c42..d5ce6854b 100644 --- a/go.mod +++ b/go.mod @@ -94,7 +94,7 @@ require ( github.com/hashicorp/cap/ldap v0.0.0-20231012003312-273118a6e3b8 github.com/hashicorp/dbassert v0.0.0-20231012105025-1bc1bd88e22b github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20231124110655-4315424d22d0 - github.com/hashicorp/go-rate v0.0.0-20231106220535-0b8f59b56792 + github.com/hashicorp/go-rate v0.0.0-20231129140553-d912bfc00ee2 github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/nodeenrollment v0.2.9 github.com/jackc/pgx/v5 v5.4.3 diff --git a/go.sum b/go.sum index d2f133013..c04601408 100644 --- a/go.sum +++ b/go.sum @@ -260,8 +260,8 @@ github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9 github.com/hashicorp/go-plugin v1.4.3/go.mod h1:5fGEH17QVwTTcR0zV7yhDPLLmFX9YSZ38b18Udy6vYQ= github.com/hashicorp/go-plugin v1.5.2 h1:aWv8eimFqWlsEiMrYZdPYl+FdHaBJSN4AWwGWfT1G2Y= github.com/hashicorp/go-plugin v1.5.2/go.mod h1:w1sAEES3g3PuV/RzUrgow20W2uErMly84hhD3um1WL4= -github.com/hashicorp/go-rate v0.0.0-20231106220535-0b8f59b56792 h1:YS6tr7P9pfMqfomjrquQEHEj+tEk3aCj1OuYp6u87aQ= -github.com/hashicorp/go-rate v0.0.0-20231106220535-0b8f59b56792/go.mod h1:uTQC66EFZuSKunHYx0hHIfdH3nhRrv7UOOE8Ug0yI0A= +github.com/hashicorp/go-rate v0.0.0-20231129140553-d912bfc00ee2 h1:vNHBERW+GdaqHG0a+McDV/ExpdFshheEzie41wb8nts= +github.com/hashicorp/go-rate v0.0.0-20231129140553-d912bfc00ee2/go.mod h1:uTQC66EFZuSKunHYx0hHIfdH3nhRrv7UOOE8Ug0yI0A= github.com/hashicorp/go-retryablehttp v0.5.3/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es7NDiJgTc6Er0maI1Xs= github.com/hashicorp/go-retryablehttp v0.7.4 h1:ZQgVdpTdAL7WpMIwLzCfbalOcSUdkDZnpUv3/+BxzFA= github.com/hashicorp/go-retryablehttp v0.7.4/go.mod h1:Jy/gPYAdjqffZ/yFGCFV2doI5wjtH1ewM9u8iYVjtX8= diff --git a/internal/daemon/controller/handler.go b/internal/daemon/controller/handler.go index 9731befd5..d390cf1b6 100644 --- a/internal/daemon/controller/handler.go +++ b/internal/daemon/controller/handler.go @@ -74,7 +74,7 @@ func createMuxWithEndpoints(c *Controller, props HandlerProperties) (http.Handle } mux := http.NewServeMux() - mux.Handle("/v1/", ratelimit.Handler(c.rateLimiter, grpcGwMux)) + mux.Handle("/v1/", ratelimit.Handler(c.baseContext, c.rateLimiter, grpcGwMux)) mux.Handle(uiPath, handleUi(c)) isUiRequest := func(req *http.Request) bool { diff --git a/internal/ratelimit/config.go b/internal/ratelimit/config.go index dd8cc1162..a4ea23138 100644 --- a/internal/ratelimit/config.go +++ b/internal/ratelimit/config.go @@ -39,10 +39,14 @@ import ( // Defaults used when creating default rate.Limits. const ( - DefaultRequestLimit = 3000 - DefaultPeriod = time.Second * 30 - DefaultListRequestLimit = 60 - DefaultListPeriod = time.Second * 30 + DefaultInTotalRequestLimit = 3000 + DefaultIpAddressRequestLimit = 3000 + DefaultAuthTokenRequestLimit = 3000 + DefaultPeriod = time.Second * 30 + DefaultInTotalListRequestLimit = 60 + DefaultIpAddressListRequestLimit = 60 + DefaultAuthTokenListRequestLimit = 60 + DefaultListPeriod = time.Second * 30 ) // defaultLimiterMaxEntries is the default maximum number of quotas that @@ -130,24 +134,58 @@ func (c Configs) Limits(ctx context.Context) ([]*rate.Limit, error) { } for a := range validActions { - key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerTotal) + inTotalKey := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerTotal) + authTokenKey := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerAuthToken) + ipAddressKey := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerIPAddress) switch a { case action.List: - defaults[key] = &rate.Limit{ + defaults[inTotalKey] = &rate.Limit{ Resource: res.String(), Action: a.String(), Per: rate.LimitPerTotal, Unlimited: false, - MaxRequests: DefaultListRequestLimit, + MaxRequests: DefaultInTotalListRequestLimit, + Period: DefaultListPeriod, + } + defaults[authTokenKey] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenListRequestLimit, + Period: DefaultListPeriod, + } + defaults[ipAddressKey] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressListRequestLimit, Period: DefaultListPeriod, } default: - defaults[key] = &rate.Limit{ + defaults[inTotalKey] = &rate.Limit{ Resource: res.String(), Action: a.String(), Per: rate.LimitPerTotal, Unlimited: false, - MaxRequests: DefaultRequestLimit, + MaxRequests: DefaultInTotalRequestLimit, + Period: DefaultPeriod, + } + defaults[authTokenKey] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenRequestLimit, + Period: DefaultPeriod, + } + defaults[ipAddressKey] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressRequestLimit, Period: DefaultPeriod, } } @@ -177,7 +215,7 @@ func (c Configs) Limits(ctx context.Context) ([]*rate.Limit, error) { return nil, err } for a := range validActions { - key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerTotal) + key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPer(cc.Per)) defaults[key] = &rate.Limit{ Resource: res.String(), @@ -205,7 +243,7 @@ func (c Configs) Limits(ctx context.Context) ([]*rate.Limit, error) { if !ok { return nil, errors.New(ctx, errors.InvalidConfiguration, op, "", errors.WithMsg("action %s not valid for resource %s", aStr, res.String())) } - key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerTotal) + key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPer(cc.Per)) defaults[key] = &rate.Limit{ Resource: res.String(), diff --git a/internal/ratelimit/config_test.go b/internal/ratelimit/config_test.go index d5374f6bb..6c0d83193 100644 --- a/internal/ratelimit/config_test.go +++ b/internal/ratelimit/config_test.go @@ -41,23 +41,61 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue case action.List: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultListRequestLimit, - Period: DefaultListPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenListRequestLimit, + Period: DefaultListPeriod, + }, + ) default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultRequestLimit, - Period: DefaultPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenRequestLimit, + Period: DefaultPeriod, + }, + ) } } } @@ -75,6 +113,20 @@ func TestConfigsLimits(t *testing.T) { Limit: 20, Period: time.Minute * 5, }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "ip-address", + Limit: 10, + Period: time.Minute * 5, + }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "auth-token", + Limit: 5, + Period: time.Minute * 5, + }, }, func() []*rate.Limit { limits := make([]*rate.Limit, 0, len(resource.Map)*len(action.Map)) @@ -90,14 +142,33 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: 20, - Period: time.Minute * 5, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute * 5, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 5, + Period: time.Minute * 5, + }, + ) } } } @@ -115,6 +186,20 @@ func TestConfigsLimits(t *testing.T) { Limit: 20, Period: time.Minute * 5, }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "ip-address", + Limit: 20, + Period: time.Minute * 5, + }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "auth-token", + Limit: 20, + Period: time.Minute * 5, + }, { Resources: []string{"target"}, Actions: []string{"list"}, @@ -122,6 +207,20 @@ func TestConfigsLimits(t *testing.T) { Limit: 10, Period: time.Minute, }, + { + Resources: []string{"target"}, + Actions: []string{"list"}, + Per: "ip-address", + Limit: 5, + Period: time.Minute, + }, + { + Resources: []string{"target"}, + Actions: []string{"list"}, + Per: "auth-token", + Limit: 1, + Period: time.Minute, + }, }, func() []*rate.Limit { limits := make([]*rate.Limit, 0, len(resource.Map)*len(action.Map)) @@ -138,26 +237,64 @@ func TestConfigsLimits(t *testing.T) { continue case action.List: if res == resource.Target { - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: 10, - Period: time.Minute, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 5, + Period: time.Minute, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 1, + Period: time.Minute, + }, + ) continue } fallthrough default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: 20, - Period: time.Minute * 5, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }, + ) } } } @@ -175,6 +312,20 @@ func TestConfigsLimits(t *testing.T) { Limit: 10, Period: time.Minute, }, + { + Resources: []string{"target", "session"}, + Actions: []string{"*"}, + Per: "ip-address", + Limit: 5, + Period: time.Minute, + }, + { + Resources: []string{"target", "session"}, + Actions: []string{"*"}, + Per: "auth-token", + Limit: 1, + Period: time.Minute, + }, }, func() []*rate.Limit { limits := make([]*rate.Limit, 0, len(resource.Map)*len(action.Map)) @@ -190,14 +341,33 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: 10, - Period: time.Minute, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 5, + Period: time.Minute, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 1, + Period: time.Minute, + }, + ) } } default: @@ -208,23 +378,61 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue case action.List: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultListRequestLimit, - Period: DefaultListPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenListRequestLimit, + Period: DefaultListPeriod, + }, + ) default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultRequestLimit, - Period: DefaultPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenRequestLimit, + Period: DefaultPeriod, + }, + ) } } } @@ -243,6 +451,20 @@ func TestConfigsLimits(t *testing.T) { Limit: 10, Period: time.Minute, }, + { + Resources: []string{"target", "session"}, + Actions: []string{"list", "read"}, + Per: "ip-address", + Limit: 5, + Period: time.Minute, + }, + { + Resources: []string{"target", "session"}, + Actions: []string{"list", "read"}, + Per: "auth-token", + Limit: 1, + Period: time.Minute, + }, }, func() []*rate.Limit { limits := make([]*rate.Limit, 0, len(resource.Map)*len(action.Map)) @@ -258,23 +480,61 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue case action.List, action.Read: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: 10, - Period: time.Minute, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 5, + Period: time.Minute, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 1, + Period: time.Minute, + }, + ) default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultRequestLimit, - Period: DefaultPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenRequestLimit, + Period: DefaultPeriod, + }, + ) } } default: @@ -285,23 +545,61 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue case action.List: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultListRequestLimit, - Period: DefaultListPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenListRequestLimit, + Period: DefaultListPeriod, + }, + ) default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: DefaultRequestLimit, - Period: DefaultPeriod, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultInTotalRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenRequestLimit, + Period: DefaultPeriod, + }, + ) } } } @@ -342,15 +640,62 @@ func TestConfigsLimits(t *testing.T) { switch a { case action.Unknown, action.All: continue + case action.List: + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressListRequestLimit, + Period: DefaultListPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenListRequestLimit, + Period: DefaultListPeriod, + }, + ) default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: false, - MaxRequests: 20, - Period: time.Minute * 5, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: DefaultIpAddressRequestLimit, + Period: DefaultPeriod, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: DefaultAuthTokenRequestLimit, + Period: DefaultPeriod, + }, + ) } } } @@ -367,6 +712,18 @@ func TestConfigsLimits(t *testing.T) { Per: "total", Unlimited: true, }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "ip-address", + Unlimited: true, + }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "auth-token", + Unlimited: true, + }, }, func() []*rate.Limit { limits := make([]*rate.Limit, 0, len(resource.Map)*len(action.Map)) @@ -382,12 +739,27 @@ func TestConfigsLimits(t *testing.T) { case action.Unknown, action.All: continue default: - limits = append(limits, &rate.Limit{ - Resource: res.String(), - Action: a.String(), - Per: rate.LimitPerTotal, - Unlimited: true, - }) + limits = append( + limits, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: true, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerIPAddress, + Unlimited: true, + }, + &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerAuthToken, + Unlimited: true, + }, + ) } } } diff --git a/internal/ratelimit/handler.go b/internal/ratelimit/handler.go index 38bc9fb79..dec2a69de 100644 --- a/internal/ratelimit/handler.go +++ b/internal/ratelimit/handler.go @@ -4,11 +4,14 @@ package ratelimit import ( + "context" "fmt" "math" "net/http" "regexp" + "github.com/hashicorp/boundary/globals" + "github.com/hashicorp/boundary/internal/event" "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" "github.com/hashicorp/go-rate" @@ -112,12 +115,23 @@ func extractResourceAction(path, method string) (res, act string, err error) { // using the provided rate limiter. If the request is allowed, the next handler // is called. Otherwise a 429 is returned with the Retry-After response header // set to the number of seconds the client should wait to make it's next request. -func Handler(l *rate.Limiter, next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - path := req.URL.Path - method := req.Method +func Handler(ctx context.Context, l *rate.Limiter, next http.Handler) http.Handler { + const op = "ratelimit.Handler" - res, a, err := extractResourceAction(path, method) + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + reqInfo, ok := event.RequestInfoFromContext(req.Context()) + if !ok || reqInfo == nil || reqInfo.ClientIp == "" { + rw.WriteHeader(http.StatusInternalServerError) + return + } + + authtoken, ok := req.Context().Value(globals.ContextAuthTokenPublicIdKey).(string) + if !ok { + rw.WriteHeader(http.StatusInternalServerError) + return + } + + res, a, err := extractResourceAction(req.URL.Path, req.Method) if err != nil { if extractErr, ok := err.(*extractResourceActionErr); ok { rw.WriteHeader(extractErr.statusCode) @@ -127,14 +141,13 @@ func Handler(l *rate.Limiter, next http.Handler) http.Handler { return } - allowed, quota, err := l.Allow(res, a) + allowed, quota, err := l.Allow(res, a, reqInfo.ClientIp, authtoken) if err != nil { if errFull, ok := err.(*rate.ErrLimiterFull); ok { rw.Header().Add("Retry-After", fmt.Sprintf("%.0f", math.Ceil(errFull.RetryIn.Seconds()))) rw.WriteHeader(http.StatusServiceUnavailable) return } - // The only other error here should be rate.ErrLimitNotFound, which // shouldn't be possible given how we initialize the limiter and // the checks done by extractResourceAction. diff --git a/internal/ratelimit/handler_test.go b/internal/ratelimit/handler_test.go index e0ab17ec1..013cabf1c 100644 --- a/internal/ratelimit/handler_test.go +++ b/internal/ratelimit/handler_test.go @@ -4,11 +4,15 @@ package ratelimit_test import ( + "context" "net/http" "net/http/httptest" "testing" "time" + "github.com/hashicorp/boundary/globals" + "github.com/hashicorp/boundary/internal/daemon/common" + "github.com/hashicorp/boundary/internal/event" "github.com/hashicorp/boundary/internal/ratelimit" "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" @@ -18,10 +22,14 @@ import ( ) func TestHandler(t *testing.T) { + ctx := context.Background() + cases := []struct { name string limiter *rate.Limiter req func(uri string) *http.Request + ip string + authtoken string expectCode int expectHeaders http.Header }{ @@ -37,6 +45,22 @@ func TestHandler(t *testing.T) { MaxRequests: 10, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -46,6 +70,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusOK, http.Header{}, }, @@ -61,6 +87,22 @@ func TestHandler(t *testing.T) { MaxRequests: 10, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.Create.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.Create.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -70,6 +112,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusOK, http.Header{}, }, @@ -85,6 +129,22 @@ func TestHandler(t *testing.T) { MaxRequests: 10, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.Read.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.Read.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -94,6 +154,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusOK, http.Header{}, }, @@ -109,6 +171,22 @@ func TestHandler(t *testing.T) { MaxRequests: 10, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.Update.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.Update.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -118,6 +196,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusOK, http.Header{}, }, @@ -133,6 +213,22 @@ func TestHandler(t *testing.T) { MaxRequests: 10, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.Delete.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.Delete.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -142,6 +238,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusOK, http.Header{}, }, @@ -157,6 +255,22 @@ func TestHandler(t *testing.T) { MaxRequests: 10, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.AuthorizeSession.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.AuthorizeSession.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -166,6 +280,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusOK, http.Header{}, }, @@ -181,9 +297,25 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) - _, q, err := r.Allow("target", "list") + _, q, err := r.Allow("target", "list", "", "") require.NoError(t, err) // consume all of the quota @@ -198,6 +330,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusTooManyRequests, http.Header{ "Retry-After": []string{"60"}, @@ -223,10 +357,42 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, - }, 1) + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.Read.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.Read.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + }, 3) require.NoError(t, err) // use up all of the capacity for the limiter - _, _, err = r.Allow("target", "read") + _, _, err = r.Allow("target", "read", "", "") require.NoError(t, err) return r }(), @@ -235,6 +401,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusServiceUnavailable, http.Header{ "Retry-After": []string{"1"}, @@ -252,6 +420,22 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -261,6 +445,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusInternalServerError, http.Header{}, }, @@ -276,6 +462,22 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -285,6 +487,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusNotFound, http.Header{}, }, @@ -300,6 +504,22 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -309,6 +529,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusBadRequest, http.Header{}, }, @@ -324,6 +546,22 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -333,6 +571,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusMethodNotAllowed, http.Header{}, }, @@ -348,6 +588,22 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -357,6 +613,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusMethodNotAllowed, http.Header{}, }, @@ -372,6 +630,22 @@ func TestHandler(t *testing.T) { MaxRequests: 2, Period: time.Minute, }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 2, + Period: time.Minute, + }, }, 10) require.NoError(t, err) return r @@ -381,6 +655,8 @@ func TestHandler(t *testing.T) { require.NoError(t, err) return r }, + "127.0.0.1", + "authtoken", http.StatusMethodNotAllowed, http.Header{}, }, @@ -388,10 +664,29 @@ func TestHandler(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - server := httptest.NewServer(ratelimit.Handler(tc.limiter, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - return - }))) + server := httptest.NewServer( + func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + id, err := event.NewId(event.IdPrefix) + require.NoError(t, err) + ctx, err = event.NewRequestInfoContext(ctx, &event.RequestInfo{ + Id: id, + EventId: common.GeneratedTraceId(ctx), + ClientIp: tc.ip, + }) + require.NoError(t, err) + ctx = context.WithValue(ctx, globals.ContextAuthTokenPublicIdKey, tc.authtoken) + + req = req.Clone(ctx) + + next.ServeHTTP(rw, req) + }) + }(ratelimit.Handler(ctx, tc.limiter, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + return + }))), + ) req := tc.req(server.URL) client := &http.Client{} res, err := client.Do(req) @@ -403,3 +698,104 @@ func TestHandler(t *testing.T) { }) } } + +func TestHandlerErrors(t *testing.T) { + ctx := context.Background() + + cases := []struct { + name string + ctxFn func(context.Context) context.Context + expectCode int + }{ + { + "NoRequestInfo", + func(ctx context.Context) context.Context { + ctx = context.WithValue(ctx, globals.ContextAuthTokenPublicIdKey, "auth-token") + return ctx + }, + http.StatusInternalServerError, + }, + { + "ClientIpEmptyString", + func(ctx context.Context) context.Context { + id, err := event.NewId(event.IdPrefix) + require.NoError(t, err) + ctx, err = event.NewRequestInfoContext(ctx, &event.RequestInfo{ + Id: id, + EventId: common.GeneratedTraceId(ctx), + ClientIp: "", + }) + require.NoError(t, err) + ctx = context.WithValue(ctx, globals.ContextAuthTokenPublicIdKey, "auth-token") + return ctx + }, + http.StatusInternalServerError, + }, + { + "NoAuthToken", + func(ctx context.Context) context.Context { + id, err := event.NewId(event.IdPrefix) + require.NoError(t, err) + ctx, err = event.NewRequestInfoContext(ctx, &event.RequestInfo{ + Id: id, + EventId: common.GeneratedTraceId(ctx), + ClientIp: "127.0.0.1", + }) + require.NoError(t, err) + return ctx + }, + http.StatusInternalServerError, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r, err := rate.NewLimiter([]*rate.Limit{ + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerIPAddress, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + { + Resource: resource.Target.String(), + Action: action.List.String(), + Per: rate.LimitPerAuthToken, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }, + }, 10) + require.NoError(t, err) + server := httptest.NewServer( + func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + ctx := tc.ctxFn(req.Context()) + req = req.Clone(ctx) + + next.ServeHTTP(rw, req) + }) + }(ratelimit.Handler(ctx, r, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + return + }))), + ) + req, err := http.NewRequest(http.MethodGet, server.URL+"/v1/targets", nil) + require.NoError(t, err) + client := &http.Client{} + res, err := client.Do(req) + require.NoError(t, err) + assert.Equal(t, tc.expectCode, res.StatusCode) + }) + } +}