diff --git a/app/controllers/api/forward_auth_controller.rb b/app/controllers/api/forward_auth_controller.rb index 9422721..c06375b 100644 --- a/app/controllers/api/forward_auth_controller.rb +++ b/app/controllers/api/forward_auth_controller.rb @@ -1,6 +1,5 @@ module Api class ForwardAuthController < ApplicationController - # ForwardAuth endpoints need session storage for return URL allow_unauthenticated_access skip_before_action :verify_authenticity_token @@ -8,34 +7,23 @@ module Api after_action :track_failed_forward_auth_attempt # GET /api/verify - # This endpoint is called by reverse proxies (Traefik, Caddy, nginx) - # to verify if a user is authenticated and authorized to access a domain + # Called by reverse proxies (Traefik, Caddy, nginx) to verify authentication and authorization. def verify - # Note: app_slug parameter is no longer used - we match domains directly with Application (forward_auth type) - - # Check for bearer token first (API keys for server-to-server auth) bearer_result = authenticate_bearer_token return bearer_result if bearer_result - # Check for one-time forward auth token first (to handle race condition) session_id = check_forward_auth_token - - # If no token found, try to get session from cookie session_id ||= extract_session_id unless session_id - # No session cookie or token - user is not authenticated return render_unauthorized("No session cookie") end - # Find the session with user association (eager loading for performance) - session = Session.includes(:user).find_by(id: session_id) + session = Session.includes(user: :groups).find_by(id: session_id) unless session - # Invalid session return render_unauthorized("Invalid session") end - # Check if session is expired if session.expired? session.destroy return render_unauthorized("Session expired") @@ -46,41 +34,32 @@ module Api session.update_column(:last_activity_at, Time.current) end - # Get the user (already loaded via includes(:user)) user = session.user unless user.active? return render_unauthorized("User account is not active") end - # Check for forward auth application authorization - # Get the forwarded host for domain matching forwarded_host = request.headers["X-Forwarded-Host"] || request.headers["Host"] + app = nil if forwarded_host.present? - # 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 + apps = cached_forward_auth_apps - # Find matching forward auth application for this domain app = apps.find { |a| a.matches_domain?(forwarded_host) } if app - # Check if application is active unless app.active? Rails.logger.info "ForwardAuth: Access denied to #{forwarded_host} - application is inactive" return render_forbidden("No authentication rule configured for this domain") end - # Check if user is allowed by this application (with cached groups) - unless app_allows_user_cached?(app, user) + unless app.user_allowed?(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 Rails.logger.info "ForwardAuth: User #{user.email_address} granted access to #{forwarded_host} by app #{app.domain_pattern} (policy: #{app.policy_for_user(user)})" else - # No application found - DENY by default (fail-closed security) Rails.logger.info "ForwardAuth: Access denied to #{forwarded_host} - no authentication rule configured" return render_forbidden("No authentication rule configured for this domain") end @@ -88,10 +67,8 @@ module Api Rails.logger.info "ForwardAuth: User #{user.email_address} authenticated (no domain specified)" end - # User is authenticated and authorized - # Return 200 with user information headers using app-specific configuration headers = if app - headers_for_user_cached(app, user) + app.headers_for_user(user) else Application::DEFAULT_HEADERS.map { |key, header_name| case key @@ -100,7 +77,7 @@ module Api when :username [header_name, user.username] if user.username.present? when :groups - user.groups.any? ? [header_name, user.groups.pluck(:name).join(",")] : nil + user.groups.any? ? [header_name, user.groups.map(&:name).join(",")] : nil when :admin [header_name, user.admin? ? "true" : "false"] end @@ -108,15 +85,8 @@ module Api end headers.each { |key, value| response.headers[key] = value } + Rails.logger.debug "ForwardAuth: Headers sent: #{headers.keys.join(", ")}" if headers.any? - # Log what headers we're sending (helpful for debugging) - if headers.any? - Rails.logger.debug "ForwardAuth: Headers sent: #{headers.keys.join(", ")}" - else - Rails.logger.debug "ForwardAuth: No headers sent (access only)" - end - - # Return 200 OK with no body head :ok end @@ -126,7 +96,12 @@ module Api Rails.application.config.forward_auth_cache end - # Rate limiting: 50 failed attempts per minute per IP + def cached_forward_auth_apps + fa_cache.fetch("fa_apps", expires_in: 5.minutes) do + Application.forward_auth.includes(:allowed_groups).to_a + end + end + RATE_LIMIT_MAX_FAILURES = 50 RATE_LIMIT_WINDOW = 1.minute @@ -140,45 +115,13 @@ module Api 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 + # Use increment to avoid resetting TTL on each failure (fixed window) + unless fa_cache.increment(cache_key) + fa_cache.write(cache_key, 1, expires_in: RATE_LIMIT_WINDOW) end - - headers end def authenticate_bearer_token @@ -219,87 +162,50 @@ module Api end def check_forward_auth_token - # Check for one-time token in query parameters (for race condition handling) token = params[:fa_token] return nil unless token.present? - # Try to get session ID from cache session_id = Rails.cache.read("forward_auth_token:#{token}") return nil unless session_id - # Verify the session exists and is valid session = Session.find_by(id: session_id) return nil unless session && !session.expired? - # Delete the token immediately (one-time use) Rails.cache.delete("forward_auth_token:#{token}") - session_id end def extract_session_id - # Extract session ID from cookie - # Rails uses signed cookies by default cookies.signed[:session_id] end - def extract_app_from_headers - # This method is deprecated since we now use Application (forward_auth type) domain matching - # Keeping it for backward compatibility but it's no longer used - nil - end - def render_unauthorized(reason = nil) Rails.logger.info "ForwardAuth: Unauthorized - #{reason}" - - # Set auth reason header for debugging (like Authelia) response.headers["X-Auth-Reason"] = reason if reason.present? - # Get the redirect URL from query params or construct default redirect_url = validate_redirect_url(params[:rd]) base_url = determine_base_url(redirect_url) - # Set the original URL that user was trying to access - # This will be used after authentication original_host = request.headers["X-Forwarded-Host"] original_uri = request.headers["X-Forwarded-Uri"] || request.headers["X-Forwarded-Path"] || "/" - # Debug logging to see what headers we're getting - Rails.logger.info "ForwardAuth Headers: Host=#{request.headers["Host"]}, X-Forwarded-Host=#{original_host}, X-Forwarded-Uri=#{request.headers["X-Forwarded-Uri"]}, X-Forwarded-Path=#{request.headers["X-Forwarded-Path"]}" - original_url = if original_host - # Use the forwarded host and URI (original behavior) "https://#{original_host}#{original_uri}" else - # Fallback: use the validated redirect URL or default - redirect_url || "https://clinch.aapamilne.com" + redirect_url || base_url end - # Debug: log what we're redirecting to after login - Rails.logger.info "ForwardAuth: Will redirect to after login: #{original_url}" - session[:return_to_after_authenticating] = original_url - # Build login URL with redirect parameters like Authelia - login_params = { - rd: original_url, - rm: request.method - } + login_params = { rd: original_url, rm: request.method } login_url = "#{base_url}/signin?#{login_params.to_query}" - # Return 302 Found directly to login page (matching Authelia) - # This is the same as Authelia's StatusFound response - Rails.logger.info "Setting 302 redirect to: #{login_url}" redirect_to login_url, allow_other_host: true, status: :found end def render_forbidden(reason = nil) Rails.logger.info "ForwardAuth: Forbidden - #{reason}" - - # Set auth reason header for debugging (like Authelia) response.headers["X-Auth-Reason"] = reason if reason.present? - - # Return 403 Forbidden head :forbidden end @@ -308,19 +214,14 @@ module Api begin uri = URI.parse(url) - - # Only allow HTTP/HTTPS schemes return nil unless uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS) - - # Only allow HTTPS in production return nil unless Rails.env.development? || uri.scheme == "https" redirect_domain = uri.host.downcase return nil unless redirect_domain.present? - # Check against our ForwardAuth applications - matching_app = Application.forward_auth.active.find do |app| - app.matches_domain?(redirect_domain) + matching_app = cached_forward_auth_apps.find do |app| + app.active? && app.matches_domain?(redirect_domain) end matching_app ? url : nil @@ -329,32 +230,19 @@ module Api end end - def domain_has_forward_auth_rule?(domain) - return false if domain.blank? - - Application.forward_auth.active.any? do |app| - app.matches_domain?(domain.downcase) - end - end - def determine_base_url(redirect_url) - # If we have a valid redirect URL, use it return redirect_url if redirect_url.present? - # Try CLINCH_HOST environment variable first if ENV["CLINCH_HOST"].present? host = ENV["CLINCH_HOST"] - # Ensure URL has https:// protocol host.match?(/^https?:\/\//) ? host : "https://#{host}" else - # Fallback to the request host request_host = request.host || request.headers["X-Forwarded-Host"] if request_host.present? Rails.logger.warn "ForwardAuth: CLINCH_HOST not set, using request host: #{request_host}" "https://#{request_host}" else - # No host information available - raise exception to force proper configuration - raise StandardError, "ForwardAuth: CLINCH_HOST environment variable not set and no request host available. Please configure CLINCH_HOST properly." + raise StandardError, "ForwardAuth: CLINCH_HOST environment variable not set and no request host available." end end end diff --git a/app/models/application.rb b/app/models/application.rb index aba93de..b7e8f93 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -202,7 +202,7 @@ class Application < ApplicationRecord 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? + headers[header_name] = user.groups.map(&:name).join(",") if user.groups.any? when :admin headers[header_name] = user.admin? ? "true" : "false" end diff --git a/app/models/application_group.rb b/app/models/application_group.rb index 5eb95a0..4ee1f38 100644 --- a/app/models/application_group.rb +++ b/app/models/application_group.rb @@ -3,4 +3,12 @@ class ApplicationGroup < ApplicationRecord belongs_to :group validates :application_id, uniqueness: {scope: :group_id} + + after_commit :bust_forward_auth_cache + + private + + def bust_forward_auth_cache + Rails.application.config.forward_auth_cache&.delete("fa_apps") + end end diff --git a/test/controllers/api/forward_auth_controller_test.rb b/test/controllers/api/forward_auth_controller_test.rb index 2d330e9..1813e1d 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 diff --git a/test/integration/forward_auth_integration_test.rb b/test/integration/forward_auth_integration_test.rb index ad134e1..f72ab3c 100644 --- a/test/integration/forward_auth_integration_test.rb +++ b/test/integration/forward_auth_integration_test.rb @@ -130,7 +130,9 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest # Rails normalizes header keys to lowercase assert_equal @user.email_address, response.headers["x-remote-user"] assert response.headers.key?("x-remote-groups") - assert_equal "Group Two,Group One", response.headers["x-remote-groups"] + groups_in_header = response.headers["x-remote-groups"].split(",").sort + expected_groups = @user.groups.reload.map(&:name).sort + assert_equal expected_groups, groups_in_header # Test custom headers get "/api/verify", headers: {"X-Forwarded-Host" => "custom.example.com"} @@ -138,7 +140,7 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest # Custom headers are also normalized to lowercase assert_equal @user.email_address, response.headers["x-webauth-user"] assert response.headers.key?("x-webauth-roles") - assert_equal "Group Two,Group One", response.headers["x-webauth-roles"] + assert_equal expected_groups, response.headers["x-webauth-roles"].split(",").sort # Test no headers get "/api/verify", headers: {"X-Forwarded-Host" => "noheaders.example.com"}