Remove ineffective rate limit to /api/fleet/device/ping and api/fleet/orbit/ping endpoints (#16334)

#16076

This change removes ineffective rate limit to `/api/fleet/device/ping`
and `api/fleet/orbit/ping`.
Currently these endpoints are not rate limited, because the rate
limiting used in these was the `errorLimiter` which only takes effect if
the request fails and the ping endpoints never fail. So... we were
making ineffective Redis accesses on every `/api/fleet/device/ping` and
`api/fleet/orbit/ping` requests (we use Redis as the limiter store).

- [X] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Manual QA for all new/changed functionality

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
 

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.

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{})

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