feat: add flag to disaable all rate limits (#5570)

This commit is contained in:
Dean Sheather
2023-01-05 12:05:20 -06:00
committed by GitHub
parent ab7e676b54
commit 5a968e2f93
16 changed files with 292 additions and 110 deletions

View File

@ -2325,9 +2325,6 @@ const docTemplate = `{
"agent_stat_refresh_interval": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
},
"api_rate_limit": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
},
"audit_logging": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
},
@ -2385,6 +2382,9 @@ const docTemplate = `{
"proxy_trusted_origins": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-array_string"
},
"rate_limit": {
"$ref": "#/definitions/codersdk.RateLimitConfig"
},
"scim_api_key": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-string"
},
@ -2950,6 +2950,17 @@ const docTemplate = `{
}
}
},
"codersdk.RateLimitConfig": {
"type": "object",
"properties": {
"api": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
},
"disable_all": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
}
}
},
"codersdk.Response": {
"type": "object",
"properties": {

View File

@ -2058,9 +2058,6 @@
"agent_stat_refresh_interval": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
},
"api_rate_limit": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
},
"audit_logging": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
},
@ -2118,6 +2115,9 @@
"proxy_trusted_origins": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-array_string"
},
"rate_limit": {
"$ref": "#/definitions/codersdk.RateLimitConfig"
},
"scim_api_key": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-string"
},
@ -2662,6 +2662,17 @@
}
}
},
"codersdk.RateLimitConfig": {
"type": "object",
"properties": {
"api": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-int"
},
"disable_all": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
}
}
},
"codersdk.Response": {
"type": "object",
"properties": {

View File

@ -82,24 +82,20 @@ type Options struct {
Auditor audit.Auditor
AgentConnectionUpdateFrequency time.Duration
AgentInactiveDisconnectTimeout time.Duration
// APIRateLimit is the minutely throughput rate limit per user or ip.
// Setting a rate limit <0 will disable the rate limiter across the entire
// app. Specific routes may have their own limiters.
APIRateLimit int
AWSCertificates awsidentity.Certificates
Authorizer rbac.Authorizer
AzureCertificates x509.VerifyOptions
GoogleTokenValidator *idtoken.Validator
GithubOAuth2Config *GithubOAuth2Config
OIDCConfig *OIDCConfig
PrometheusRegistry *prometheus.Registry
SecureAuthCookie bool
SSHKeygenAlgorithm gitsshkey.Algorithm
Telemetry telemetry.Reporter
TracerProvider trace.TracerProvider
GitAuthConfigs []*gitauth.Config
RealIPConfig *httpmw.RealIPConfig
TrialGenerator func(ctx context.Context, email string) error
AWSCertificates awsidentity.Certificates
Authorizer rbac.Authorizer
AzureCertificates x509.VerifyOptions
GoogleTokenValidator *idtoken.Validator
GithubOAuth2Config *GithubOAuth2Config
OIDCConfig *OIDCConfig
PrometheusRegistry *prometheus.Registry
SecureAuthCookie bool
SSHKeygenAlgorithm gitsshkey.Algorithm
Telemetry telemetry.Reporter
TracerProvider trace.TracerProvider
GitAuthConfigs []*gitauth.Config
RealIPConfig *httpmw.RealIPConfig
TrialGenerator func(ctx context.Context, email string) error
// TLSCertificates is used to mesh DERP servers securely.
TLSCertificates []tls.Certificate
TailnetCoordinator tailnet.Coordinator
@ -107,6 +103,13 @@ type Options struct {
DERPMap *tailcfg.DERPMap
SwaggerEndpoint bool
// APIRateLimit is the minutely throughput rate limit per user or ip.
// Setting a rate limit <0 will disable the rate limiter across the entire
// app. Some specific routes have their own configurable rate limits.
APIRateLimit int
LoginRateLimit int
FilesRateLimit int
MetricsCacheRefreshInterval time.Duration
AgentStatsRefreshInterval time.Duration
Experimental bool
@ -157,6 +160,12 @@ func New(options *Options) *API {
if options.APIRateLimit == 0 {
options.APIRateLimit = 512
}
if options.LoginRateLimit == 0 {
options.LoginRateLimit = 60
}
if options.FilesRateLimit == 0 {
options.FilesRateLimit = 12
}
if options.Authorizer == nil {
options.Authorizer = rbac.NewAuthorizer()
}
@ -230,6 +239,10 @@ func New(options *Options) *API {
Optional: false,
})
// API rate limit middleware. The counter is local and not shared between
// replicas or instances of this middleware.
apiRateLimiter := httpmw.RateLimit(options.APIRateLimit, time.Minute)
r.Use(
httpmw.Recover(api.Logger),
tracing.StatusWriterMiddleware,
@ -241,8 +254,8 @@ func New(options *Options) *API {
// handleSubdomainApplications checks if the first subdomain is a valid
// app URL. If it is, it will serve that application.
api.handleSubdomainApplications(
apiRateLimiter,
// Middleware to impose on the served application.
httpmw.RateLimit(options.APIRateLimit, time.Minute),
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
DB: options.Database,
OAuth2Configs: oauthConfigs,
@ -268,7 +281,7 @@ func New(options *Options) *API {
apps := func(r chi.Router) {
r.Use(
httpmw.RateLimit(options.APIRateLimit, time.Minute),
apiRateLimiter,
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
DB: options.Database,
OAuth2Configs: oauthConfigs,
@ -315,8 +328,9 @@ func New(options *Options) *API {
r.NotFound(func(rw http.ResponseWriter, r *http.Request) { httpapi.RouteNotFound(rw) })
r.Use(
// Specific routes can specify smaller limits.
httpmw.RateLimit(options.APIRateLimit, time.Minute),
// Specific routes can specify different limits, but every rate
// limit must be configurable by the admin.
apiRateLimiter,
)
r.Get("/", apiRoot)
// All CSP errors will be logged
@ -339,9 +353,7 @@ func New(options *Options) *API {
r.Route("/files", func(r chi.Router) {
r.Use(
apiKeyMiddleware,
// This number is arbitrary, but reading/writing
// file content is expensive so it should be small.
httpmw.RateLimit(12, time.Minute),
httpmw.RateLimit(options.FilesRateLimit, time.Minute),
)
r.Get("/{fileID}", api.fileByID)
r.Post("/", api.postFile)
@ -426,25 +438,25 @@ func New(options *Options) *API {
r.Route("/users", func(r chi.Router) {
r.Get("/first", api.firstUser)
r.Post("/first", api.postFirstUser)
r.Group(func(r chi.Router) {
// We use a tight limit for password login to protect
// against audit-log write DoS, pbkdf2 DoS, and simple
// brute-force attacks.
//
// Making this too small can break tests.
r.Use(httpmw.RateLimit(60, time.Minute))
r.Post("/login", api.postLogin)
})
r.Get("/authmethods", api.userAuthMethods)
r.Route("/oauth2", func(r chi.Router) {
r.Route("/github", func(r chi.Router) {
r.Use(httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient))
r.Get("/callback", api.userOAuth2Github)
r.Group(func(r chi.Router) {
// We use a tight limit for password login to protect against
// audit-log write DoS, pbkdf2 DoS, and simple brute-force
// attacks.
//
// This value is intentionally increased during tests.
r.Use(httpmw.RateLimit(options.LoginRateLimit, time.Minute))
r.Post("/login", api.postLogin)
r.Route("/oauth2", func(r chi.Router) {
r.Route("/github", func(r chi.Router) {
r.Use(httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient))
r.Get("/callback", api.userOAuth2Github)
})
})
r.Route("/oidc/callback", func(r chi.Router) {
r.Use(httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient))
r.Get("/", api.userOIDC)
})
})
r.Route("/oidc/callback", func(r chi.Router) {
r.Use(httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient))
r.Get("/", api.userOIDC)
})
r.Group(func(r chi.Router) {
r.Use(

View File

@ -92,7 +92,6 @@ type Options struct {
OIDCConfig *coderd.OIDCConfig
GoogleTokenValidator *idtoken.Validator
SSHKeygenAlgorithm gitsshkey.Algorithm
APIRateLimit int
AutobuildTicker <-chan time.Time
AutobuildStats chan<- executor.Stats
Auditor audit.Auditor
@ -100,6 +99,11 @@ type Options struct {
GitAuthConfigs []*gitauth.Config
TrialGenerator func(context.Context, string) error
// All rate limits default to -1 (unlimited) in tests if not set.
APIRateLimit int
LoginRateLimit int
FilesRateLimit int
// IncludeProvisionerDaemon when true means to start an in-memory provisionerD
IncludeProvisionerDaemon bool
MetricsCacheRefreshInterval time.Duration
@ -177,6 +181,17 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
options.DeploymentConfig = DeploymentConfig(t)
}
// If no ratelimits are set, disable all rate limiting for tests.
if options.APIRateLimit == 0 {
options.APIRateLimit = -1
}
if options.LoginRateLimit == 0 {
options.LoginRateLimit = -1
}
if options.FilesRateLimit == 0 {
options.FilesRateLimit = -1
}
ctx, cancelFunc := context.WithCancel(context.Background())
lifecycleExecutor := executor.New(
ctx,
@ -270,6 +285,8 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
SSHKeygenAlgorithm: options.SSHKeygenAlgorithm,
DERPServer: derpServer,
APIRateLimit: options.APIRateLimit,
LoginRateLimit: options.LoginRateLimit,
FilesRateLimit: options.FilesRateLimit,
Authorizer: options.Authorizer,
Telemetry: telemetry.NewNoop(),
TLSCertificates: options.TLSCertificates,

View File

@ -661,7 +661,7 @@ func TestTemplateVersionDryRun(t *testing.T) {
Type: "cool_resource_type",
}
client := coderdtest.New(t, &coderdtest.Options{APIRateLimit: -1, IncludeProvisionerDaemon: true})
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
@ -882,7 +882,7 @@ func TestTemplateVersionDryRun(t *testing.T) {
func TestPaginatedTemplateVersions(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{APIRateLimit: -1})
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)

View File

@ -1325,7 +1325,7 @@ func TestWorkspacesByUser(t *testing.T) {
func TestSuspendedPagination(t *testing.T) {
t.Parallel()
t.Skip("This fails when two users are created at the exact same time. The reason is unknown... See: https://github.com/coder/coder/actions/runs/3057047622/jobs/4931863163")
client := coderdtest.New(t, &coderdtest.Options{APIRateLimit: -1})
client := coderdtest.New(t, nil)
coderdtest.CreateFirstUser(t, client)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
@ -1370,7 +1370,7 @@ func TestSuspendedPagination(t *testing.T) {
// them using different page sizes.
func TestPaginatedUsers(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{APIRateLimit: -1})
client := coderdtest.New(t, nil)
coderdtest.CreateFirstUser(t, client)
// This test takes longer than a long time.