diff --git a/app/controllers/api/forward_auth_controller.rb b/app/controllers/api/forward_auth_controller.rb index 1708e49..9422721 100644 --- a/app/controllers/api/forward_auth_controller.rb +++ b/app/controllers/api/forward_auth_controller.rb @@ -3,7 +3,9 @@ module Api # ForwardAuth endpoints need session storage for return URL allow_unauthenticated_access 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 # This endpoint is called by reverse proxies (Traefik, Caddy, nginx) @@ -39,8 +41,10 @@ module Api return render_unauthorized("Session expired") end - # Update last activity (skip validations for performance) - session.update_column(:last_activity_at, Time.current) + # 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) + end # Get the user (already loaded via includes(:user)) user = session.user @@ -53,9 +57,10 @@ module Api forwarded_host = request.headers["X-Forwarded-Host"] || request.headers["Host"] if forwarded_host.present? - # Load all forward auth applications (including inactive ones) for security checks - # Preload groups to avoid N+1 queries in user_allowed? checks - apps = Application.forward_auth.includes(:allowed_groups) + # Load all forward auth applications with cached lookup + apps = fa_cache.fetch("fa_apps", expires_in: 5.minutes) do + Application.forward_auth.includes(:allowed_groups).to_a + end # Find matching forward auth application for this domain 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") end - # Check if user is allowed by this application - unless app.user_allowed?(user) + # Check if user is allowed by this application (with cached groups) + 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}" return render_forbidden("You do not have permission to access this domain") end @@ -86,7 +91,7 @@ module Api # User is authenticated and authorized # Return 200 with user information headers using app-specific configuration headers = if app - app.headers_for_user(user) + headers_for_user_cached(app, user) else Application::DEFAULT_HEADERS.map { |key, header_name| case key @@ -117,6 +122,65 @@ module Api 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 auth_header = request.headers["Authorization"] return nil unless auth_header&.start_with?("Bearer ") diff --git a/app/models/application.rb b/app/models/application.rb index 059dcb8..aba93de 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -22,6 +22,8 @@ class Application < ApplicationRecord super(parsed) end + after_commit :bust_forward_auth_cache, if: :forward_auth? + has_one_attached :icon # Fix SVG content type after attachment @@ -268,6 +270,10 @@ class Application < ApplicationRecord private + def bust_forward_auth_cache + Rails.application.config.forward_auth_cache&.delete("fa_apps") + end + def fix_icon_content_type return unless icon.attached? diff --git a/config/initializers/forward_auth_cache.rb b/config/initializers/forward_auth_cache.rb new file mode 100644 index 0000000..49593df --- /dev/null +++ b/config/initializers/forward_auth_cache.rb @@ -0,0 +1,2 @@ +Rails.application.config.forward_auth_cache = + ActiveSupport::Cache::MemoryStore.new(size: 8.megabytes) diff --git a/test/controllers/api/forward_auth_controller_test.rb b/test/controllers/api/forward_auth_controller_test.rb index 8280a69..2d330e9 100644 --- a/test/controllers/api/forward_auth_controller_test.rb +++ b/test/controllers/api/forward_auth_controller_test.rb @@ -9,7 +9,7 @@ module Api @group = groups(:admin_group) @rule = Application.create!(name: "Test App", slug: "test-app", app_type: "forward_auth", domain_pattern: "test.example.com", active: true) @inactive_rule = Application.create!(name: "Inactive App", slug: "inactive-app", app_type: "forward_auth", domain_pattern: "inactive.example.com", active: false) - end +end # Authentication Tests test "should redirect to login when no session cookie" do @@ -516,6 +516,81 @@ module Api assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"] 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 test "should handle requests efficiently under load" do sign_in_as(@user) diff --git a/test/test_helper.rb b/test/test_helper.rb index 106257f..3077c2d 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -32,5 +32,10 @@ module ActiveSupport fixtures :all # 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