diff --git a/Makefile b/Makefile index 110f9c5757..9a05121840 100644 --- a/Makefile +++ b/Makefile @@ -130,7 +130,7 @@ lint-js: yarn lint lint-go: - golangci-lint run --skip-dirs ./node_modules --timeout 10m + golangci-lint run --skip-dirs ./node_modules --timeout 15m lint: lint-go lint-js diff --git a/changes/16076-ping-endpoints-not-rate-limit b/changes/16076-ping-endpoints-not-rate-limit new file mode 100644 index 0000000000..112f796640 --- /dev/null +++ b/changes/16076-ping-endpoints-not-rate-limit @@ -0,0 +1 @@ +* Remove ineffective rate-limiting from `/api/fleet/orbit/ping` and `/api/fleet/device/ping` endpoints. diff --git a/server/service/handler.go b/server/service/handler.go index 230c2e468f..c2f026f8e7 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -748,13 +748,9 @@ func attachFleetAPIRoutes(r *mux.Router, svc fleet.Service, config config.FleetC ne.WithCustomMiddleware(limiter.Limit("login", throttled.RateQuota{MaxRate: loginRateLimit, MaxBurst: 9})). POST("/api/_version_/fleet/demologin", makeDemologinEndpoint(config.Server.URLPrefix), demologinRequest{}) - ne.WithCustomMiddleware( - errorLimiter.Limit("ping_device", desktopQuota), - ).HEAD("/api/fleet/device/ping", devicePingEndpoint, devicePingRequest{}) + ne.HEAD("/api/fleet/device/ping", devicePingEndpoint, devicePingRequest{}) - ne.WithCustomMiddleware( - errorLimiter.Limit("ping_orbit", desktopQuota), - ).HEAD("/api/fleet/orbit/ping", orbitPingEndpoint, orbitPingRequest{}) + ne.HEAD("/api/fleet/orbit/ping", orbitPingEndpoint, orbitPingRequest{}) neAppleMDM.WithCustomMiddleware(limiter.Limit("login", throttled.RateQuota{MaxRate: loginRateLimit, MaxBurst: 9})). POST("/api/_version_/fleet/mdm/sso", initiateMDMAppleSSOEndpoint, initiateMDMAppleSSORequest{}) diff --git a/server/service/middleware/ratelimit/ratelimit.go b/server/service/middleware/ratelimit/ratelimit.go index 383f5fab8e..f4d9f1beea 100644 --- a/server/service/middleware/ratelimit/ratelimit.go +++ b/server/service/middleware/ratelimit/ratelimit.go @@ -38,7 +38,14 @@ func (m *Middleware) Limit(keyName string, quota throttled.RateQuota) endpoint.M return func(ctx context.Context, req interface{}) (response interface{}, err error) { limited, result, err := limiter.RateLimit(keyName, 1) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "check rate limit") + // This can happen if the limit store (e.g. Redis) is unavailable. + // + // We need to set authentication as checked, otherwise we end up returning HTTP 500 + // errors. + if az, ok := authz_ctx.FromContext(ctx); ok { + az.SetChecked() + } + return nil, ctxerr.Wrap(ctx, err, "rate limit Middleware: failed to increase rate limit") } if limited { @@ -84,7 +91,13 @@ func (m *ErrorMiddleware) Limit(keyName string, quota throttled.RateQuota) endpo // RateLimit with quantity 0 will never get limited=true, so we check result.Remaining instead _, result, err := limiter.RateLimit(ipKeyName, 0) if err != nil { - return nil, ctxerr.Wrap(ctx, err, "check rate limit") + // This can happen if the limit store (e.g. Redis) is unavailable. + // + // We need to set authentication as checked, otherwise we end up returning HTTP 500 errors. + if az, ok := authz_ctx.FromContext(ctx); ok { + az.SetChecked() + } + return nil, ctxerr.Wrap(ctx, err, "rate limit ErrorMiddleware: failed to check rate limit") } if result.Remaining == 0 { // We need to set authentication as checked, otherwise we end up returning HTTP 500 errors. @@ -98,7 +111,7 @@ func (m *ErrorMiddleware) Limit(keyName string, quota throttled.RateQuota) endpo if err != nil { _, _, rateErr := limiter.RateLimit(ipKeyName, 1) if rateErr != nil { - return nil, ctxerr.Wrap(ctx, err, "check rate limit") + return nil, ctxerr.Wrap(ctx, err, "rate limit ErrorMiddleware: failed to increase rate limit") } } return resp, err