coder · deansheather · Jan 5, 2023 · Jan 4, 2023 · Jan 4, 2023 · Jan 5, 2023
diff --git a/cli/deployment/config.go b/cli/deployment/config.go
@@ -429,11 +429,19 @@ func newConfig() *codersdk.DeploymentConfig {
 				Default: 10 * time.Minute,
 			},
 		},
-		APIRateLimit: &codersdk.DeploymentConfigField[int]{
-			Name:    "API Rate Limit",
-			Usage:   "Maximum number of requests per minute allowed to the API per user, or per IP address for unauthenticated users. Negative values mean no rate limit. Some API endpoints are always rate limited regardless of this value to prevent denial-of-service attacks.",
-			Flag:    "api-rate-limit",
-			Default: 512,
+		RateLimit: &codersdk.RateLimitConfig{
+			DisableAll: &codersdk.DeploymentConfigField[bool]{
+				Name:    "Disable All Rate Limits",
+				Usage:   "Disables all rate limits. This is not recommended in production.",
+				Flag:    "dangerous-disable-rate-limits",
+				Default: false,
+			},
+			API: &codersdk.DeploymentConfigField[int]{
+				Name:    "API Rate Limit",
+				Usage:   "Maximum number of requests per minute allowed to the API per user, or per IP address for unauthenticated users. Negative values mean no rate limit. Some API endpoints have separate flags to control rate limits to help prevent denial-of-service attacks.",
+				Flag:    "api-rate-limit",
+				Default: 512,
+			},
 		},
 		Experimental: &codersdk.DeploymentConfigField[bool]{
 			Name:  "Experimental",

diff --git a/cli/scaletest.go b/cli/scaletest.go
@@ -265,7 +265,7 @@ func requireAdmin(ctx context.Context, client *codersdk.Client) (codersdk.User,
 	// Only owners can do scaletests. This isn't a very strong check but there's
 	// not much else we can do. Ratelimits are enforced for non-owners so
 	// hopefully that limits the damage if someone disables this check and runs
-	// it against a non-owner account.
+	// it against a non-owner account on a production deployment.
 	ok := false
 	for _, role := range me.Roles {
 		if role.Name == "owner" {
@@ -488,7 +488,9 @@ func scaletestCreateWorkspaces() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "create-workspaces",
 		Short: "Creates many workspaces and waits for them to be ready",
-		Long:  "Creates many users, then creates a workspace for each user and waits for them finish building and fully come online. Optionally runs a command inside each workspace, and connects to the workspace over WireGuard.",
+		Long: `Creates many users, then creates a workspace for each user and waits for them finish building and fully come online. Optionally runs a command inside each workspace, and connects to the workspace over WireGuard.
+
+It is recommended that all rate limits are disabled on the server before running this scaletest. This test generates many login events which will be rate limited against the (most likely single) IP.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
 			client, err := CreateClient(cmd)

diff --git a/cli/server.go b/cli/server.go
@@ -104,6 +104,16 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 				return xerrors.Errorf("either HTTP or TLS must be enabled")
 			}
 
+			// Disable rate limits if the `--dangerous-disable-rate-limits` flag
+			// was specified.
+			loginRateLimit := 60
+			filesRateLimit := 12
+			if cfg.RateLimit.DisableAll.Value {
+				cfg.RateLimit.API.Value = -1
+				loginRateLimit = -1
+				filesRateLimit = -1
+			}
+
 			printLogo(cmd)
 			logger := slog.Make(sloghuman.Sink(cmd.ErrOrStderr()))
 			if ok, _ := cmd.Flags().GetBool(varVerbose); ok {
@@ -432,7 +442,9 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 				AgentStatsRefreshInterval:   cfg.AgentStatRefreshInterval.Value,
 				DeploymentConfig:            cfg,
 				PrometheusRegistry:          prometheus.NewRegistry(),
-				APIRateLimit:                cfg.APIRateLimit.Value,
+				APIRateLimit:                cfg.RateLimit.API.Value,
+				LoginRateLimit:              loginRateLimit,
+				FilesRateLimit:              filesRateLimit,
 				HTTPClient:                  httpClient,
 			}
 			if tlsConfig != nil {

diff --git a/cli/testdata/coder_scaletest_create-workspaces_--help.golden b/cli/testdata/coder_scaletest_create-workspaces_--help.golden
@@ -1,5 +1,7 @@
 Creates many users, then creates a workspace for each user and waits for them finish building and fully come online. Optionally runs a command inside each workspace, and connects to the workspace over WireGuard.
 
+It is recommended that all rate limits are disabled on the server before running this scaletest. This test generates many login events which will be rate limited against the (most likely single) IP.
+
 Usage:
   coder scaletest create-workspaces [flags]
 

diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -18,16 +18,19 @@ Flags:
                                                      allowed to the API per user, or per IP
                                                      address for unauthenticated users.
                                                      Negative values mean no rate limit. Some
-                                                     API endpoints are always rate limited
-                                                     regardless of this value to prevent
+                                                     API endpoints have separate flags to
+                                                     control rate limits to help prevent
                                                      denial-of-service attacks.
-                                                     Consumes $CODER_API_RATE_LIMIT (default 512)
+                                                     Consumes $CODER_RATE_LIMIT_API (default 512)
       --cache-dir string                             The directory to cache temporary files.
                                                      If unspecified and $CACHE_DIRECTORY is
                                                      set, it will be used for compatibility
                                                      with systemd.
                                                      Consumes $CODER_CACHE_DIRECTORY (default
                                                      "/tmp/coder-cli-test-cache")
+      --dangerous-disable-rate-limits                Disables all rate limits. This is not
+                                                     recommended in production.
+                                                     Consumes $CODER_RATE_LIMIT_DISABLE_ALL
       --derp-config-path string                      Path to read a DERP mapping from. See:
                                                      https://tailscale.com/kb/1118/custom-derp-servers/
                                                      Consumes $CODER_DERP_CONFIG_PATH

diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
@@ -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": {

diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
@@ -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": {

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -82,31 +82,34 @@ 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
 	DERPServer         *derp.Server
 	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.Get("/authmethods", api.userAuthMethods)
 			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.
+				// 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))
+				// This value is intentionally increased during tests.
+				r.Use(httpmw.RateLimit(options.LoginRateLimit, 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.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(

diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -92,14 +92,18 @@ 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
 	TLSCertificates      []tls.Certificate
 	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,