From 7fcc004beb9f6ef022f755e8e2f2a958c976c637 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Mon, 26 Sep 2016 12:56:12 -0400 Subject: 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 +- webapp/components/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 c043fe2ae..9345d3fc4 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 b16ad6e8e..979c8f5ef 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 98d6f6687..63aa5006a 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 13135f9ae..514260d09 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 d592bac98..88a409869 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 92eb8613a..bb4f9d83c 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 c93ad1c60..2bf4248a8 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.", -- cgit v1.2.3-1-g7c22