Add rate limiting and in-memory caching for forward auth endpoint
Rate limit failed attempts (50/min per IP) with 429 + Retry-After. Cache forward auth applications in a dedicated MemoryStore (8MB LRU) to avoid loading all apps from SQLite on every request. Debounce last_activity_at writes to at most once per minute per session. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,9 @@ module Api
|
|||||||
# ForwardAuth endpoints need session storage for return URL
|
# ForwardAuth endpoints need session storage for return URL
|
||||||
allow_unauthenticated_access
|
allow_unauthenticated_access
|
||||||
skip_before_action :verify_authenticity_token
|
skip_before_action :verify_authenticity_token
|
||||||
# No rate limiting on forward_auth endpoint - proxy middleware hits this frequently
|
|
||||||
|
before_action :check_forward_auth_rate_limit
|
||||||
|
after_action :track_failed_forward_auth_attempt
|
||||||
|
|
||||||
# GET /api/verify
|
# GET /api/verify
|
||||||
# This endpoint is called by reverse proxies (Traefik, Caddy, nginx)
|
# This endpoint is called by reverse proxies (Traefik, Caddy, nginx)
|
||||||
@@ -39,8 +41,10 @@ module Api
|
|||||||
return render_unauthorized("Session expired")
|
return render_unauthorized("Session expired")
|
||||||
end
|
end
|
||||||
|
|
||||||
# Update last activity (skip validations for performance)
|
# Debounce last_activity_at updates (at most once per minute)
|
||||||
|
if session.last_activity_at.nil? || session.last_activity_at < 1.minute.ago
|
||||||
session.update_column(:last_activity_at, Time.current)
|
session.update_column(:last_activity_at, Time.current)
|
||||||
|
end
|
||||||
|
|
||||||
# Get the user (already loaded via includes(:user))
|
# Get the user (already loaded via includes(:user))
|
||||||
user = session.user
|
user = session.user
|
||||||
@@ -53,9 +57,10 @@ module Api
|
|||||||
forwarded_host = request.headers["X-Forwarded-Host"] || request.headers["Host"]
|
forwarded_host = request.headers["X-Forwarded-Host"] || request.headers["Host"]
|
||||||
|
|
||||||
if forwarded_host.present?
|
if forwarded_host.present?
|
||||||
# Load all forward auth applications (including inactive ones) for security checks
|
# Load all forward auth applications with cached lookup
|
||||||
# Preload groups to avoid N+1 queries in user_allowed? checks
|
apps = fa_cache.fetch("fa_apps", expires_in: 5.minutes) do
|
||||||
apps = Application.forward_auth.includes(:allowed_groups)
|
Application.forward_auth.includes(:allowed_groups).to_a
|
||||||
|
end
|
||||||
|
|
||||||
# Find matching forward auth application for this domain
|
# Find matching forward auth application for this domain
|
||||||
app = apps.find { |a| a.matches_domain?(forwarded_host) }
|
app = apps.find { |a| a.matches_domain?(forwarded_host) }
|
||||||
@@ -67,8 +72,8 @@ module Api
|
|||||||
return render_forbidden("No authentication rule configured for this domain")
|
return render_forbidden("No authentication rule configured for this domain")
|
||||||
end
|
end
|
||||||
|
|
||||||
# Check if user is allowed by this application
|
# Check if user is allowed by this application (with cached groups)
|
||||||
unless app.user_allowed?(user)
|
unless app_allows_user_cached?(app, user)
|
||||||
Rails.logger.info "ForwardAuth: User #{user.email_address} denied access to #{forwarded_host} by app #{app.domain_pattern}"
|
Rails.logger.info "ForwardAuth: User #{user.email_address} denied access to #{forwarded_host} by app #{app.domain_pattern}"
|
||||||
return render_forbidden("You do not have permission to access this domain")
|
return render_forbidden("You do not have permission to access this domain")
|
||||||
end
|
end
|
||||||
@@ -86,7 +91,7 @@ module Api
|
|||||||
# User is authenticated and authorized
|
# User is authenticated and authorized
|
||||||
# Return 200 with user information headers using app-specific configuration
|
# Return 200 with user information headers using app-specific configuration
|
||||||
headers = if app
|
headers = if app
|
||||||
app.headers_for_user(user)
|
headers_for_user_cached(app, user)
|
||||||
else
|
else
|
||||||
Application::DEFAULT_HEADERS.map { |key, header_name|
|
Application::DEFAULT_HEADERS.map { |key, header_name|
|
||||||
case key
|
case key
|
||||||
@@ -117,6 +122,65 @@ module Api
|
|||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def fa_cache
|
||||||
|
Rails.application.config.forward_auth_cache
|
||||||
|
end
|
||||||
|
|
||||||
|
# Rate limiting: 50 failed attempts per minute per IP
|
||||||
|
RATE_LIMIT_MAX_FAILURES = 50
|
||||||
|
RATE_LIMIT_WINDOW = 1.minute
|
||||||
|
|
||||||
|
def check_forward_auth_rate_limit
|
||||||
|
count = fa_cache.read("fa_fail:#{request.remote_ip}")
|
||||||
|
return unless count && count >= RATE_LIMIT_MAX_FAILURES
|
||||||
|
|
||||||
|
response.headers["Retry-After"] = "60"
|
||||||
|
head :too_many_requests
|
||||||
|
end
|
||||||
|
|
||||||
|
def track_failed_forward_auth_attempt
|
||||||
|
return unless response.status.in?([401, 403, 302])
|
||||||
|
# 302 in this controller means unauthorized redirect to login
|
||||||
|
# Don't track 200 (success) or 429 (already rate limited)
|
||||||
|
return if response.status == 302 && !response.headers["X-Auth-Reason"]
|
||||||
|
|
||||||
|
cache_key = "fa_fail:#{request.remote_ip}"
|
||||||
|
count = fa_cache.read(cache_key) || 0
|
||||||
|
fa_cache.write(cache_key, count + 1, expires_in: RATE_LIMIT_WINDOW)
|
||||||
|
end
|
||||||
|
|
||||||
|
def app_allows_user_cached?(app, user)
|
||||||
|
return false unless app.active?
|
||||||
|
return false unless user.active?
|
||||||
|
return true if app.allowed_groups.empty?
|
||||||
|
|
||||||
|
(user.groups & app.allowed_groups).any?
|
||||||
|
end
|
||||||
|
|
||||||
|
def headers_for_user_cached(app, user)
|
||||||
|
headers = {}
|
||||||
|
effective = app.effective_headers
|
||||||
|
|
||||||
|
effective.each do |key, header_name|
|
||||||
|
next unless header_name.present?
|
||||||
|
|
||||||
|
case key
|
||||||
|
when :user, :email
|
||||||
|
headers[header_name] = user.email_address
|
||||||
|
when :name
|
||||||
|
headers[header_name] = user.name.presence || user.email_address
|
||||||
|
when :username
|
||||||
|
headers[header_name] = user.username if user.username.present?
|
||||||
|
when :groups
|
||||||
|
headers[header_name] = user.groups.pluck(:name).join(",") if user.groups.any?
|
||||||
|
when :admin
|
||||||
|
headers[header_name] = user.admin? ? "true" : "false"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
headers
|
||||||
|
end
|
||||||
|
|
||||||
def authenticate_bearer_token
|
def authenticate_bearer_token
|
||||||
auth_header = request.headers["Authorization"]
|
auth_header = request.headers["Authorization"]
|
||||||
return nil unless auth_header&.start_with?("Bearer ")
|
return nil unless auth_header&.start_with?("Bearer ")
|
||||||
|
|||||||
@@ -22,6 +22,8 @@ class Application < ApplicationRecord
|
|||||||
super(parsed)
|
super(parsed)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
after_commit :bust_forward_auth_cache, if: :forward_auth?
|
||||||
|
|
||||||
has_one_attached :icon
|
has_one_attached :icon
|
||||||
|
|
||||||
# Fix SVG content type after attachment
|
# Fix SVG content type after attachment
|
||||||
@@ -268,6 +270,10 @@ class Application < ApplicationRecord
|
|||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def bust_forward_auth_cache
|
||||||
|
Rails.application.config.forward_auth_cache&.delete("fa_apps")
|
||||||
|
end
|
||||||
|
|
||||||
def fix_icon_content_type
|
def fix_icon_content_type
|
||||||
return unless icon.attached?
|
return unless icon.attached?
|
||||||
|
|
||||||
|
|||||||
2
config/initializers/forward_auth_cache.rb
Normal file
2
config/initializers/forward_auth_cache.rb
Normal file
@@ -0,0 +1,2 @@
|
|||||||
|
Rails.application.config.forward_auth_cache =
|
||||||
|
ActiveSupport::Cache::MemoryStore.new(size: 8.megabytes)
|
||||||
@@ -516,6 +516,81 @@ module Api
|
|||||||
assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"]
|
assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Rate Limiting Tests
|
||||||
|
test "should return 429 when rate limit exceeded" do
|
||||||
|
cache = Rails.application.config.forward_auth_cache
|
||||||
|
cache.write("fa_fail:127.0.0.1", 50, expires_in: 1.minute)
|
||||||
|
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
|
||||||
|
assert_response 429
|
||||||
|
assert_equal "60", response.headers["Retry-After"]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "should allow requests below rate limit" do
|
||||||
|
cache = Rails.application.config.forward_auth_cache
|
||||||
|
cache.write("fa_fail:127.0.0.1", 49, expires_in: 1.minute)
|
||||||
|
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
|
||||||
|
assert_response 302 # unauthorized redirect, but not rate limited
|
||||||
|
end
|
||||||
|
|
||||||
|
test "should track failed attempts and eventually rate limit" do
|
||||||
|
cache = Rails.application.config.forward_auth_cache
|
||||||
|
|
||||||
|
# Make 50 failed requests (no session = unauthorized)
|
||||||
|
50.times do
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
end
|
||||||
|
|
||||||
|
# The 51st should be rate limited
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
assert_response 429
|
||||||
|
end
|
||||||
|
|
||||||
|
test "should not track successful requests as failures" do
|
||||||
|
cache = Rails.application.config.forward_auth_cache
|
||||||
|
sign_in_as(@user)
|
||||||
|
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
assert_response 200
|
||||||
|
|
||||||
|
count = cache.read("fa_fail:127.0.0.1")
|
||||||
|
assert_nil count, "Successful requests should not increment failure counter"
|
||||||
|
end
|
||||||
|
|
||||||
|
# Caching Tests
|
||||||
|
test "should debounce last_activity_at updates" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
session = Session.last
|
||||||
|
|
||||||
|
# First request should update last_activity_at
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
assert_response 200
|
||||||
|
first_activity = session.reload.last_activity_at
|
||||||
|
|
||||||
|
# Second request within 1 minute should NOT update
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
assert_response 200
|
||||||
|
assert_equal first_activity, session.reload.last_activity_at
|
||||||
|
end
|
||||||
|
|
||||||
|
test "should bust app cache when forward auth application is saved" do
|
||||||
|
cache = Rails.application.config.forward_auth_cache
|
||||||
|
sign_in_as(@user)
|
||||||
|
|
||||||
|
# Prime the cache
|
||||||
|
get "/api/verify", headers: {"X-Forwarded-Host" => "test.example.com"}
|
||||||
|
assert_response 200
|
||||||
|
assert cache.read("fa_apps"), "Cache should be populated after request"
|
||||||
|
|
||||||
|
# Update the application
|
||||||
|
@rule.update!(name: "Updated App")
|
||||||
|
|
||||||
|
assert_nil cache.read("fa_apps"), "Cache should be busted after application update"
|
||||||
|
end
|
||||||
|
|
||||||
# Performance and Load Tests
|
# Performance and Load Tests
|
||||||
test "should handle requests efficiently under load" do
|
test "should handle requests efficiently under load" do
|
||||||
sign_in_as(@user)
|
sign_in_as(@user)
|
||||||
|
|||||||
@@ -32,5 +32,10 @@ module ActiveSupport
|
|||||||
fixtures :all
|
fixtures :all
|
||||||
|
|
||||||
# Add more helper methods to be used by all tests here...
|
# Add more helper methods to be used by all tests here...
|
||||||
|
|
||||||
|
# Clear in-memory forward auth cache before each test to prevent cross-test pollution
|
||||||
|
setup do
|
||||||
|
Rails.application.config.forward_auth_cache&.clear
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user