diff --git a/config/initializers/rack-attack.rb b/config/initializers/rack-attack.rb index a4318ed4fb7..0dad1bf97fe 100644 --- a/config/initializers/rack-attack.rb +++ b/config/initializers/rack-attack.rb @@ -40,9 +40,15 @@ Rails.application.reloader.to_prepare do regex.any? { |i| i =~ req.path } end - Rack::Attack.blocklisted_responder = lambda do |_env| - # All blacklisted routes would return a 404. - [404, {}, ["Not found"]] + # Route blocklist returns 404. + # All other blocklists (for example, login ban) + # use the RateLimiting dispatcher set up by set_defaults! + Rack::Attack.blocklisted_responder = lambda do |request| + if request.env["rack.attack.matched"] == "block forbidden routes" + [404, {}, ["Not found"]] + else + OpenProject::RateLimiting.blocklisted_response(request) + end end end end diff --git a/docs/installation-and-operations/configuration/README.md b/docs/installation-and-operations/configuration/README.md index e028fb8e4d9..e56054fc631 100644 --- a/docs/installation-and-operations/configuration/README.md +++ b/docs/installation-and-operations/configuration/README.md @@ -652,13 +652,61 @@ OPENPROJECT_HIDDEN__MENU__ITEMS_ADMIN__MENU="roles types statuses workflows enum #### Rate limiting -OpenProject provides some rate limiting protections. The default configuration protects against repeated access to authentication credential resets (e.g., lost password functionality). +OpenProject includes HTTP-layer rate limiting via Rack::Attack. The rules below are configured through the `rate_limiting` setting and take effect without a restart when set via environment variable. -You can optionally enable additional rules on API rate limiting as follows: +In addition to these application-level rules, consider applying rate limiting at your load balancer or reverse proxy (e.g. `ngx_http_limit_req_module`, `mod_security`) for IP-level protection. -`OPENPROJECT_RATE_LIMITING_API__V3=true` +##### Login brute-force protection (enabled by default) -Additional application-level rate limiting rules will be added in the future. Additionally to these application level rules, use your load balancer / proxying web server to apply individual rate limiting rules using modules such as `ngx_http_limit_req_module` or `mod_security`. +OpenProject blocks repeated login attempts per account at the HTTP layer. After **20 POST `/login` requests for the same username within one minute**, that account is blocked for **30 minutes**. This works alongside the application-level lockout (`brute_force_block_after_failed_logins` / `brute_force_block_minutes` settings). + +This rule is **enabled by default**. To disable it: + +```shell +OPENPROJECT_RATE_LIMITING_LOGIN="false" +``` + +The thresholds can be tuned independently: + +| Variable | Default | Description | +|---|---|---| +| `OPENPROJECT_RATE_LIMITING_LOGIN_BURST__LIMIT` | `20` | Number of attempts allowed before the ban is triggered | +| `OPENPROJECT_RATE_LIMITING_LOGIN_BURST__PERIOD` | `60` | Detection window in seconds | +| `OPENPROJECT_RATE_LIMITING_LOGIN_BAN__PERIOD` | `1800` | Ban duration in seconds | + +Example: Stricter limits (10 attempts per minute, 1-hour ban): + +```shell +OPENPROJECT_RATE_LIMITING_LOGIN_BURST__LIMIT="10" +OPENPROJECT_RATE_LIMITING_LOGIN_BURST__PERIOD="60" +OPENPROJECT_RATE_LIMITING_LOGIN_BAN__PERIOD="3600" +``` + +> [!NOTE] +> This rule and the application-level brute-force protection (`brute_force_block_after_failed_logins` / +> `brute_force_block_minutes`) are independent controls that operate at different layers. The HTTP-layer +> rule counts **all** login attempts (including successful ones) within its burst window, while the +> application-level setting counts only **failed** attempts and operates over a longer rolling window. +> If you lower `brute_force_block_after_failed_logins` below `BURST_LIMIT` (default 20), the +> application-level lockout will fire before this rule does. Keep the two thresholds consistent to +> avoid surprising behaviour. For example, set `BURST_LIMIT` to match or be lower than +> `brute_force_block_after_failed_logins`. + +##### Lost password rate limiting (disabled by default) + +Limits password-reset requests per email address to 3 per hour. + +```shell +OPENPROJECT_RATE_LIMITING_LOST__PASSWORD="true" +``` + +##### API v3 rate limiting (disabled by default) + +Limits API form endpoint requests per session to 6 per 3 seconds. + +```shell +OPENPROJECT_RATE_LIMITING_API__V3="true" +``` #### Blacklisted routes diff --git a/lib/open_project/rate_limiting.rb b/lib/open_project/rate_limiting.rb index 7775b06d00e..ec0e21d33d9 100644 --- a/lib/open_project/rate_limiting.rb +++ b/lib/open_project/rate_limiting.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module OpenProject module RateLimiting module_function @@ -9,13 +11,15 @@ module OpenProject def default_rules @default_rules ||= [ LostPassword, - APIV3 + APIV3, + Login ] end def set_defaults! Rack::Attack.clear_configuration Rack::Attack.throttled_responder = ->(request) { OpenProject::RateLimiting.throttled_response(request) } + Rack::Attack.blocklisted_responder = ->(request) { OpenProject::RateLimiting.blocklisted_response(request) } @active_rules = [] default_rules.each do |rule| @@ -31,17 +35,20 @@ module OpenProject active_rules << rule.new.apply! if rule.enabled? end - ## - # Try to find a matching rule to respond with - # or use the default responder def throttled_response(request) - rule = active_rules.find { |r| r.rule_name == request.env["rack.attack.matched"] } + matched = request.env["rack.attack.matched"] + rule = find_rule(matched) + rule ? rule.response(request) : Base.new.response(request) + end - if rule - rule.response(request) - else - OpenProject::RateLimiting::Base.response(request) - end + def blocklisted_response(request) + matched = request.env["rack.attack.matched"] + rule = find_rule(matched) + rule ? rule.blocked_response : [403, {}, ["Forbidden\n"]] + end + + def find_rule(matched) + active_rules.find { |r| matched == r.rule_name || matched.start_with?("#{r.rule_name}/") } end end end diff --git a/lib/open_project/rate_limiting/base.rb b/lib/open_project/rate_limiting/base.rb index fc0b784244e..b19c570cfc4 100644 --- a/lib/open_project/rate_limiting/base.rb +++ b/lib/open_project/rate_limiting/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module OpenProject module RateLimiting class Base @@ -54,6 +56,14 @@ module OpenProject "Your request has been throttled. Try again at #{retry_after.seconds.from_now}.\n" end + def blocked_response + [429, { "Content-Type" => "text/plain" }, [blocked_response_body]] + end + + def blocked_response_body + "Your request has been blocked.\n" + end + protected # Provide a limit callback proc for the request, or use the default limit @@ -91,7 +101,7 @@ module OpenProject false end - def discriminator(request) + def discriminator(_request) raise SubclassResponsibilityError end diff --git a/lib/open_project/rate_limiting/login.rb b/lib/open_project/rate_limiting/login.rb new file mode 100644 index 00000000000..aca6f80b8cf --- /dev/null +++ b/lib/open_project/rate_limiting/login.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module OpenProject + module RateLimiting + # Per-account HTTP-layer brute-force protection for POST /login. + # + # Uses Rack::Attack::Allow2Ban: the first BURST_LIMIT attempts within + # BURST_PERIOD are allowed through; once the limit is exceeded a ban flag + # is written that blocks all subsequent attempts for BAN_PERIOD. + # + # Enabled by default. Disable or tune via configuration.yml: + # + # rate_limiting: + # login: + # enabled: false + class Login < Base + BURST_LIMIT = 20 + BURST_PERIOD = 1.minute.to_i + BAN_PERIOD = 30.minutes.to_i + + class << self + def enabled_by_default? + true + end + end + + def apply! + Rack::Attack.blocklist(rule_name) do |req| + next false unless req.post? && req.path == "/login" + + username = req.env.dig("rack.request.form_hash", "username").to_s.downcase.presence + next false unless username + + Rack::Attack::Allow2Ban.filter( + "login:#{username}", + maxretry: burst_limit, + findtime: burst_period, + bantime: ban_period + ) { true } + end + + self + end + + def blocked_response_body + "Too many login attempts. Please try again later.\n" + end + + protected + + def burst_limit + settings[:burst_limit].presence&.to_i || BURST_LIMIT + end + + def burst_period + settings[:burst_period].presence&.to_i || BURST_PERIOD + end + + def ban_period + settings[:ban_period].presence&.to_i || BAN_PERIOD + end + end + end +end diff --git a/spec/requests/rate_limiting/login_rate_limiting_spec.rb b/spec/requests/rate_limiting/login_rate_limiting_spec.rb new file mode 100644 index 00000000000..4e9dafd2c12 --- /dev/null +++ b/spec/requests/rate_limiting/login_rate_limiting_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Use a small burst_limit (3) so tests stay fast, and a large burst_period so +# the time-bucketed cache key never rolls over mid-test on a slow CI machine. +RSpec.describe "Rate limiting login", + :with_rack_attack, + with_config: { rate_limiting: { login: { burst_limit: 3, burst_period: 3600 } } }, + type: :rails_request do + before do + allow_any_instance_of(ActionController::Base) # rubocop:disable RSpec/AnyInstance + .to(receive(:protect_against_forgery?)) + .and_return(false) + end + + it "blocks after burst_limit attempts for the same username" do + freeze_time do + 3.times do + post signin_path, params: { username: "victim", password: "wrong" } + expect(response).not_to have_http_status(:too_many_requests) + end + + post signin_path, params: { username: "victim", password: "wrong" } + expect(response).to have_http_status(:too_many_requests) + expect(response.body).to include "Too many login attempts" + end + end + + it "does not affect a different username" do + freeze_time do + 3.times { post signin_path, params: { username: "victim", password: "wrong" } } + + post signin_path, params: { username: "other_user", password: "wrong" } + expect(response).not_to have_http_status(:too_many_requests) + end + end + + it "is case-insensitive on the username" do + freeze_time do + 2.times { post signin_path, params: { username: "Victim", password: "wrong" } } + post signin_path, params: { username: "VICTIM", password: "wrong" } + + post signin_path, params: { username: "victim", password: "wrong" } + expect(response).to have_http_status(:too_many_requests) + end + end + + it "does not throttle when no username is submitted" do + 4.times { post signin_path, params: {} } + expect(response).not_to have_http_status(:too_many_requests) + end + + context "when disabled", with_config: { rate_limiting: { login: false } } do + before { OpenProject::RateLimiting.set_defaults! } + + it "does not block repeated login attempts" do + 4.times do + post signin_path, params: { username: "victim", password: "wrong" } + expect(response).not_to have_http_status(:too_many_requests) + end + end + end +end