From 7fcc004beb9f6ef022f755e8e2f2a958c976c637 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Mon, 26 Sep 2016 12:56:12 -0400 Subject: [PATCH] Modifications to rate limiting settings. (#4091) --- api/apitestlib.go | 4 +- api/server.go | 11 +++--- i18n/en.json | 4 ++ model/config.go | 25 ++++++++++--- utils/diagnostic.go | 2 +- .../admin_console/rate_settings.jsx | 37 +++++++++++++++---- webapp/i18n/en.json | 3 ++ 7 files changed, 65 insertions(+), 21 deletions(-) diff --git a/api/apitestlib.go b/api/apitestlib.go index c043fe2aeb..9345d3fc42 100644 --- a/api/apitestlib.go +++ b/api/apitestlib.go @@ -33,7 +33,7 @@ func SetupEnterprise() *TestHelper { utils.LoadConfig("config.json") utils.InitTranslations(utils.Cfg.LocalizationSettings) utils.Cfg.TeamSettings.MaxUsersPerTeam = 50 - utils.Cfg.RateLimitSettings.EnableRateLimiter = false + *utils.Cfg.RateLimitSettings.Enable = false utils.DisableDebugLogForTest() utils.License.Features.SetDefaults() NewServer() @@ -55,7 +55,7 @@ func Setup() *TestHelper { utils.LoadConfig("config.json") utils.InitTranslations(utils.Cfg.LocalizationSettings) utils.Cfg.TeamSettings.MaxUsersPerTeam = 50 - utils.Cfg.RateLimitSettings.EnableRateLimiter = false + *utils.Cfg.RateLimitSettings.Enable = false utils.DisableDebugLogForTest() NewServer() StartServer() diff --git a/api/server.go b/api/server.go index b16ad6e8e7..979c8f5efb 100644 --- a/api/server.go +++ b/api/server.go @@ -4,6 +4,10 @@ package api import ( + "net/http" + "strings" + "time" + l4g "github.com/alecthomas/log4go" "github.com/braintree/manners" "github.com/gorilla/handlers" @@ -12,9 +16,6 @@ import ( "github.com/mattermost/platform/utils" "gopkg.in/throttled/throttled.v2" "gopkg.in/throttled/throttled.v2/store/memstore" - "net/http" - "strings" - "time" ) type Server struct { @@ -70,7 +71,7 @@ func StartServer() { var handler http.Handler = &CorsWrapper{Srv.Router} - if utils.Cfg.RateLimitSettings.EnableRateLimiter { + if *utils.Cfg.RateLimitSettings.Enable { l4g.Info(utils.T("api.server.start_server.rate.info")) store, err := memstore.New(utils.Cfg.RateLimitSettings.MemoryStoreSize) @@ -81,7 +82,7 @@ func StartServer() { quota := throttled.RateQuota{ MaxRate: throttled.PerSec(utils.Cfg.RateLimitSettings.PerSec), - MaxBurst: 100, + MaxBurst: *utils.Cfg.RateLimitSettings.MaxBurst, } rateLimiter, err := throttled.NewGCRARateLimiter(store, quota) diff --git a/i18n/en.json b/i18n/en.json index 98d6f66870..63aa5006a6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3043,6 +3043,10 @@ "id": "model.config.is_valid.email_security.app_error", "translation": "Invalid connection security for email settings. Must be '', 'TLS', or 'STARTTLS'" }, + { + "id": "model.config.is_valid.max_burst.app_error", + "translation": "Max burst must be greater than zero." + }, { "id": "model.config.is_valid.encrypt_sql.app_error", "translation": "Invalid at rest encrypt key for SQL settings. Must be 32 chars or more." diff --git a/model/config.go b/model/config.go index 13135f9aef..514260d09d 100644 --- a/model/config.go +++ b/model/config.go @@ -177,11 +177,12 @@ type EmailSettings struct { } type RateLimitSettings struct { - EnableRateLimiter bool - PerSec int - MemoryStoreSize int - VaryByRemoteAddr bool - VaryByHeader string + Enable *bool + PerSec int + MaxBurst *int + MemoryStoreSize int + VaryByRemoteAddr bool + VaryByHeader string } type PrivacySettings struct { @@ -894,6 +895,16 @@ func (o *Config) SetDefaults() { *o.NativeAppSettings.IosAppDownloadLink = "https://about.mattermost.com/mattermost-ios-app/" } + if o.RateLimitSettings.Enable == nil { + o.RateLimitSettings.Enable = new(bool) + *o.RateLimitSettings.Enable = false + } + + if o.RateLimitSettings.MaxBurst == nil { + o.RateLimitSettings.MaxBurst = new(int) + *o.RateLimitSettings.MaxBurst = 100 + } + o.defaultWebrtcSettings() } @@ -1097,6 +1108,10 @@ func (o *Config) IsValid() *AppError { return NewLocAppError("Config.IsValid", "model.config.is_valid.sitename_length.app_error", map[string]interface{}{"MaxLength": SITENAME_MAX_LENGTH}, "") } + if *o.RateLimitSettings.MaxBurst <= 0 { + return NewLocAppError("Config.IsValid", "model.config.is_valid.max_burst.app_error", nil, "") + } + if err := o.isValidWebrtcSettings(); err != nil { return err } diff --git a/utils/diagnostic.go b/utils/diagnostic.go index d592bac98f..88a4098698 100644 --- a/utils/diagnostic.go +++ b/utils/diagnostic.go @@ -112,7 +112,7 @@ func trackConfig() { }) SendDiagnostic(TRACK_CONFIG_RATE, map[string]interface{}{ - "enable_rate_limiter": Cfg.RateLimitSettings.EnableRateLimiter, + "enable_rate_limiter": *Cfg.RateLimitSettings.Enable, "vary_by_remote_address": Cfg.RateLimitSettings.VaryByRemoteAddr, }) diff --git a/webapp/components/admin_console/rate_settings.jsx b/webapp/components/admin_console/rate_settings.jsx index 92eb8613a6..bb4f9d83c2 100644 --- a/webapp/components/admin_console/rate_settings.jsx +++ b/webapp/components/admin_console/rate_settings.jsx @@ -21,8 +21,9 @@ export default class RateSettings extends AdminSettings { } getConfigFromState(config) { - config.RateLimitSettings.EnableRateLimiter = this.state.enableRateLimiter; + config.RateLimitSettings.Enable = this.state.enable; config.RateLimitSettings.PerSec = this.parseIntNonZero(this.state.perSec); + config.RateLimitSettings.MaxBurst = this.parseIntNonZero(this.state.maxBurst); config.RateLimitSettings.MemoryStoreSize = this.parseIntNonZero(this.state.memoryStoreSize); config.RateLimitSettings.VaryByRemoteAddr = this.state.varyByRemoteAddr; config.RateLimitSettings.VaryByHeader = this.state.varyByHeader; @@ -32,8 +33,9 @@ export default class RateSettings extends AdminSettings { getStateFromConfig(config) { return { - enableRateLimiter: config.RateLimitSettings.EnableRateLimiter, + enable: config.RateLimitSettings.Enable, perSec: config.RateLimitSettings.PerSec, + maxBurst: config.RateLimitSettings.MaxBurst, memoryStoreSize: config.RateLimitSettings.MemoryStoreSize, varyByRemoteAddr: config.RateLimitSettings.VaryByRemoteAddr, varyByHeader: config.RateLimitSettings.VaryByHeader @@ -63,7 +65,7 @@ export default class RateSettings extends AdminSettings { } - value={this.state.enableRateLimiter} + value={this.state.enable} onChange={this.handleChange} /> + + } + placeholder={Utils.localizeMessage('admin.rate.maxBurstExample', 'Ex "100"')} + helpText={ + + } + value={this.state.maxBurst} + onChange={this.handleChange} + disabled={!this.state.enable} /> ); diff --git a/webapp/i18n/en.json b/webapp/i18n/en.json index c93ad1c60d..2bf4248a86 100644 --- a/webapp/i18n/en.json +++ b/webapp/i18n/en.json @@ -543,6 +543,9 @@ "admin.rate.remoteDescription": "When true, rate limit API access by IP address.", "admin.rate.remoteTitle": "Vary rate limit by remote address: ", "admin.rate.title": "Rate Limit Settings", + "admin.rate.maxBurst": "Max Burst:", + "admin.rate.maxBurstExample": "Ex \"100\"", + "admin.rate.maxBurstDescription": "Maxumum number of requests allowed beyond the per second query limit.", "admin.recycle.button": "Recycle Database Connections", "admin.recycle.loading": " Recycling...", "admin.recycle.recycleDescription": "Deployments using multiple databases can switch from one master database to another without restarting the Mattermost server by updating \"config.json\" to the new desired configuration and using the Configuration > Reload Configuration from Disk feature to load the new settings while the server is running. The administrator should then use Recycle Database Connections feature to recycle the database connections based on the new settings.",