Commit Graph

46 Commits

Author SHA1 Message Date
Osama Sayegh
b86127ad12
FEATURE: Apply rate limits per user instead of IP for trusted users (#14706)
Currently, Discourse rate limits all incoming requests by the IP address they
originate from regardless of the user making the request. This can be
frustrating if there are multiple users using Discourse simultaneously while
sharing the same IP address (e.g. employees in an office).

This commit implements a new feature to make Discourse apply rate limits by
user id rather than IP address for users at or higher than the configured trust
level (1 is the default).

For example, let's say a Discourse instance is configured to allow 200 requests
per minute per IP address, and we have 10 users at trust level 4 using
Discourse simultaneously from the same IP address. Before this feature, the 10
users could only make a total of 200 requests per minute before they got rate
limited. But with the new feature, each user is allowed to make 200 requests
per minute because the rate limits are applied on user id rather than the IP
address.

The minimum trust level for applying user-id-based rate limits can be
configured by the `skip_per_ip_rate_limit_trust_level` global setting. The
default is 1, but it can be changed by either adding the
`DISCOURSE_SKIP_PER_IP_RATE_LIMIT_TRUST_LEVEL` environment variable with the
desired value to your `app.yml`, or changing the setting's value in the
`discourse.conf` file.

Requests made with API keys are still rate limited by IP address and the
relevant global settings that control API keys rate limits.

Before this commit, Discourse's auth cookie (`_t`) was simply a 32 characters
string that Discourse used to lookup the current user from the database and the
cookie contained no additional information about the user. However, we had to
change the cookie content in this commit so we could identify the user from the
cookie without making a database query before the rate limits logic and avoid
introducing a bottleneck on busy sites.

Besides the 32 characters auth token, the cookie now includes the user id,
trust level and the cookie's generation date, and we encrypt/sign the cookie to
prevent tampering.

Internal ticket number: t54739.
2021-11-17 23:27:30 +03:00
Rafael dos Santos Silva
b136375582
FEATURE: Rate limit exceptions via ENV (#14033)
Allow admins to configure exceptions to our Rails rate limiter.

Configuration happens in the environment variables, and work with both
IPs and CIDR blocks.

Example:

```
env:
  DISCOURSE_MAX_REQS_PER_IP_EXCEPTIONS: >-
    14.15.16.32/27
    216.148.1.2
```
2021-08-13 12:00:23 -03:00
Andrei Prigorshnev
075cd07a07
No need to disable rate limiter after running tests (#13093)
We disable rate limiter before running every test here 90ab3b1c75/spec/rails_helper.rb (L109-L109)
2021-05-19 16:04:35 +04:00
Bianca Nenciu
765ba1ab2d
FEATURE: Ignore anonymous page views on private sites (#12800)
For sites with login_required set to true, counting anonymous pageviews is
confusing. Requests to /login and other pages would make it look like
anonymous users have access to site's content.
2021-04-26 14:19:47 +03:00
Jarek Radosz
6ff888bd2c
DEV: Retry-after header values should be strings (#12475)
Fixes `Rack::Lint::LintError: a header value must be a String, but the value of 'Retry-After' is a Integer`. (see: 14a236b4f0/lib/rack/lint.rb (L676))

I found it when I got flooded by those warning a while back in a test-related accident 😉 (ember CLI tests were hitting a local rails server at a fast rate)
2021-03-23 20:32:36 +01:00
Martin Brennan
6eb0d0c38d
SECURITY: Fix is_private_ip for RateLimiter to cover all cases (#12464)
The regular expression to detect private IP addresses did not always detect them successfully.
Changed to use ruby's in-built IPAddr.new(ip_address).private? method instead
which does the same thing but covers all cases.
2021-03-22 13:56:32 +10:00
Sam
9fb9a2c098
DEV: freeze time when running rate limiter tests (#12315)
This avoids issues around clock skew making retry-after return 9 instead of
10
2021-03-11 10:47:23 +11:00
Dan Ungureanu
1f2f84a6df
FIX: Add Retry-Header to rate limited responses (#11736)
It returned a 429 error code with a 'Retry-After' header if a
RateLimiter::LimitExceeded was raised and unhandled, but the header was
missing if the request was limited in the 'RequestTracker' middleware.
2021-01-19 11:35:46 +02:00
Krzysztof Kotlarek
e0d9232259
FIX: use allowlist and blocklist terminology (#10209)
This is a PR of the renaming whitelist to allowlist and blacklist to the blocklist.
2020-07-27 10:23:54 +10:00
Guo Xiang Tan
d01c336899
DEV: Clean up some Redis leaks in test env. 2020-05-18 17:27:37 +08:00
Kane York
58ae0d4bd9
DEV: Add test case for /srv/status probers (#9259) 2020-03-24 16:28:07 +11:00
Sam Saffron
e440ec2519 FIX: crawler requests not tracked for non UTF-8 user agents
Non UTF-8 user_agent requests were bypassing logging due to PG always
wanting UTF-8 strings.

This adds some conversion to ensure we are always dealing with UTF-8
2019-12-09 17:43:51 +11:00
Joffrey JAFFEUX
0d3d2c43a0
DEV: s/\$redis/Discourse\.redis (#8431)
This commit also adds a rubocop rule to prevent global variables.
2019-12-03 10:05:53 +01:00
Sam Saffron
7d389df5e7 DEV: correct spec to allow for new default
b4bfc27b changes the default so the spec should be changed as well.
2019-11-18 16:05:58 +11:00
Penar Musaraj
74869b8a7f FIX: Do not consider mobile app traffic as crawler visits
Followup to a4eb523a
2019-11-04 09:16:50 -05:00
Krzysztof Kotlarek
427d54b2b0 DEV: Upgrading Discourse to Zeitwerk (#8098)
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains. 

We no longer need to use Rails "require_dependency" anywhere and instead can just use standard 
Ruby patterns to require files.

This is a far reaching change and we expect some followups here.
2019-10-02 14:01:53 +10:00
Sam Saffron
ed00f35306 FEATURE: improve performance of anonymous cache
This commit introduces 2 features:

1. DISCOURSE_COMPRESS_ANON_CACHE (true|false, default false): this allows
you to optionally compress the anon cache body entries in Redis, can be
useful for high load sites with Redis that lives on a separate server to
to webs

2. DISCOURSE_ANON_CACHE_STORE_THRESHOLD (default 2), only pop entries into
redis if we observe them more than N times. This avoids situations where
a crawler can walk a big pile of topics and store them all in Redis never
to be used. Our default anon cache time for topics is only 60 seconds. Anon
cache is in place to avoid the "slashdot" effect where a single topic is
hit by 100s of people in one minute.
2019-09-04 17:18:32 +10:00
Sam Saffron
b9954b53bb FIX: report cached controller and action to loggers
Previously we would treat all cached hits in anon cache as "other"

This hinders analysis of cache performance and makes logging inaccurate
2019-09-03 10:55:16 +10:00
Sam Saffron
08743e8ac0 FEATURE: anon cache reports data to loggers
This allows custom plugins such as prometheus exporter to log how many
requests are stored in the anon cache vs used by the anon cache.

This metric allows us to fine tune cache behaviors
2019-09-02 18:45:35 +10:00
Sam Saffron
62141b6316 FEATURE: enable_performance_http_headers for performance diagnostics
This adds support for DISCOURSE_ENABLE_PERFORMANCE_HTTP_HEADERS
when set to `true` this will turn on performance related headers

```text
X-Redis-Calls: 10     # number of redis calls
X-Redis-Time: 1.02    # redis time in seconds
X-Sql-Commands: 102   # number of SQL commands
X-Sql-Time: 1.02      # duration in SQL in seconds
X-Queue-Time: 1.01    # time the request sat in queue (depends on NGINX)
```

To get queue time NGINX must provide: HTTP_X_REQUEST_START

We do not recommend you enable this without thinking, it exposes information
about what your page is doing, usually you would only enable this if you
intend to strip off the headers further down the stream in a proxy
2019-06-05 16:08:11 +10:00
Penar Musaraj
a4eb523af6 Track Discourse user agent pageviews as crawler
Since 5bfe051e, Discourse user agents are marked as non-crawlers (to avoid accidental blacklisting). This makes sure pageviews for these agents are tracked as crawler hits.
2019-05-08 10:38:55 -04:00
Sam Saffron
4ea21fa2d0 DEV: use #frozen_string_literal: true on all spec
This change both speeds up specs (less strings to allocate) and helps catch
cases where methods in Discourse are mutating inputs.

Overall we will be migrating everything to use #frozen_string_literal: true
it will take a while, but this is the first and safest move in this direction
2019-04-30 10:27:42 +10:00
Neil Lalonde
e8a6323bea remove crawler blocking until multisite support 2018-07-03 17:54:45 -04:00
Neil Lalonde
b87fa6d749 FIX: blacklisted crawlers could get through by omitting the accept header 2018-04-17 12:39:30 -04:00
Sam
9980f18d86 FEATURE: track request queueing as early as possible 2018-04-17 18:06:17 +10:00
Neil Lalonde
7311023a52
Merge pull request #5700 from discourse/crawl-block
FEATURE: control web crawlers access with white/blacklist
2018-03-27 15:06:03 -04:00
Neil Lalonde
4d12ff2e8a when writing cache, remove elements from the user agents list. also return a message and content type when blocking a crawler. 2018-03-27 13:44:14 -04:00
Sam
31dea5d5fc correct flaky spec 2018-03-27 17:57:19 +11:00
Neil Lalonde
a84bb81ab5 only applies to get html requests 2018-03-22 17:57:44 -04:00
Neil Lalonde
ced7e9a691 FEATURE: control which web crawlers can access using a whitelist or blacklist 2018-03-22 15:41:02 -04:00
Sam
f0d5f83424 FEATURE: limit assets less that non asset paths
By default assets can be requested up to 200 times per 10 seconds
from the app, this includes CSS and avatars
2018-03-06 15:20:39 +11:00
Sam Saffron
df8e43abdd use lazy & instead of try
unregister ip skipper in test
raise if called when a skipper is in play
2018-02-06 10:38:15 +11:00
Robin Ward
eefd226611 Add extensibility point to request_tracker to skip IP addresses
This is useful if you want to run a per IP rate limiter but want to be
able to skip some IPs with custom logic.
2018-02-05 17:49:40 -05:00
Sam
f26ff290c3 FEATURE: Shorten setting name to max_reqs
So it is consistent with other settings
2018-01-22 13:18:30 +11:00
Sam
d7657d8e47 correct specs, ensure crawler layout only applies to html 2018-01-16 16:28:11 +11:00
Sam
cecd7d0d07 FEATURE: global rate limiter can bypass local IPs 2018-01-08 08:39:17 +11:00
Sam
4986ebcf24 FEATURE: optional default off global per ip rate limiter 2017-12-11 17:52:57 +11:00
Sam
a4c539bade FEATURE: Allow registration of detailed request logger
Detailed request loggers can be used to gather rich timing info
from all requests (which in turn can be forwarded to monitoring solution)

Middleware::RequestTracker.detailed_request_logger(->|env, data| do
   # do stuff with env and data
end
2017-10-18 12:10:30 +11:00
Guo Xiang Tan
5012d46cbd Add rubocop to our build. (#5004) 2017-07-28 10:20:09 +09:00
Andy Waite
3e50313fdc Prepare for separation of RSpec helper files
Since rspec-rails 3, the default installation creates two helper files:
* `spec_helper.rb`
* `rails_helper.rb`

`spec_helper.rb` is intended as a way of running specs that do not
require Rails, whereas `rails_helper.rb` loads Rails (as Discourse's
current `spec_helper.rb` does).

For more information:

https://www.relishapp.com/rspec/rspec-rails/docs/upgrade#default-helper-files

In this commit, I've simply replaced all instances of `spec_helper` with
`rails_helper`, and renamed the original `spec_helper.rb`.

This brings the Discourse project closer to the standard usage of RSpec
in a Rails app.

At present, every spec relies on loading Rails, but there are likely
many that don't need to. In a future pull request, I hope to introduce a
separate, minimal `spec_helper.rb` which can be used in tests which
don't rely on Rails.
2015-12-01 20:39:42 +00:00
Neil Lalonde
86cd1a19cc FEATURE: page view stats for mobile view 2015-07-03 17:19:33 -04:00
Arthur Neves
b8cbe51026
Convert specs to RSpec 2.99.2 syntax with Transpec
This conversion is done by Transpec 3.1.0 with the following command:
    transpec

* 424 conversions
    from: obj.should
      to: expect(obj).to

* 325 conversions
    from: == expected
      to: eq(expected)

* 38 conversions
    from: obj.should_not
      to: expect(obj).not_to

* 15 conversions
    from: =~ /pattern/
      to: match(/pattern/)

* 9 conversions
    from: it { should ... }
      to: it { is_expected.to ... }

* 5 conversions
    from: lambda { }.should_not
      to: expect { }.not_to

* 4 conversions
    from: lambda { }.should
      to: expect { }.to

* 2 conversions
    from: -> { }.should
      to: expect { }.to

* 2 conversions
    from: -> { }.should_not
      to: expect { }.not_to

* 1 conversion
    from: === expected
      to: be === expected

* 1 conversion
    from: =~ [1, 2]
      to: match_array([1, 2])

For more details: https://github.com/yujinakayama/transpec#supported-conversions
2015-04-25 11:18:35 -04:00
Sam
cbe18eb0df FEATURE: allow view exclusion using custom header
Set Discourse-Track-View to either "0" or "false" to exclude request
2015-02-26 11:41:11 +11:00
Sam
acda6ebd60 FIX: view tracking needs to release data earlier
retaining data during queuing was causing huge memory spikes
2015-02-10 17:03:33 +11:00
Sam
820ce8765e refactor traffic report
split traffic report in 2, page view vs raw traffic
hide raw traffic report by default
improve flushing logic for application reqs
2015-02-06 14:39:16 +11:00
Sam
08b790b3c2 improve metrics gathered using in our traffic section
this also pulls out the middleware into its own home and inserts in front
2015-02-05 16:08:52 +11:00