diff --git a/app/controllers/api/forward_auth_controller.rb b/app/controllers/api/forward_auth_controller.rb index e5f218c..39b678a 100644 --- a/app/controllers/api/forward_auth_controller.rb +++ b/app/controllers/api/forward_auth_controller.rb @@ -243,18 +243,13 @@ module Api def determine_base_url(redirect_url) return redirect_url if redirect_url.present? - if ENV["CLINCH_HOST"].present? - host = ENV["CLINCH_HOST"] - host.match?(/^https?:\/\//) ? host : "https://#{host}" - else - 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 - raise StandardError, "ForwardAuth: CLINCH_HOST environment variable not set and no request host available." - end - end + # CLINCH_HOST is the IdP's canonical origin and is mandatory in deployed + # environments (enforced at boot in config/initializers/clinch_host.rb). + # We never fall back to the request host: a spoofed X-Forwarded-Host would + # otherwise redirect the login flow to an attacker-controlled origin. The + # localhost default only applies to local dev/test. + host = ENV["CLINCH_HOST"].presence || "http://localhost:3000" + host.match?(%r{\Ahttps?://}) ? host : "https://#{host}" end end end diff --git a/config/initializers/clinch_host.rb b/config/initializers/clinch_host.rb new file mode 100644 index 0000000..d29ed84 --- /dev/null +++ b/config/initializers/clinch_host.rb @@ -0,0 +1,13 @@ +# CLINCH_HOST is this IdP's canonical external origin, e.g. https://auth.example.com. +# It anchors the OIDC issuer, the WebAuthn RP ID, and the forward-auth login +# redirect. In deployed (non-local) environments it MUST be set explicitly and +# never inferred from request headers — X-Forwarded-Host is attacker-influenceable, +# so inferring the origin from it would allow host-header phishing and open +# redirects. Fail fast at boot rather than start in an unsafe configuration. +unless Rails.env.local? + if ENV["CLINCH_HOST"].blank? + raise "CLINCH_HOST must be set (e.g. https://auth.example.com). It is the " \ + "canonical origin of this Clinch instance and must not be inferred " \ + "from request headers." + end +end diff --git a/test/integration/forward_auth_integration_test.rb b/test/integration/forward_auth_integration_test.rb index e23aa9b..8eeeff2 100644 --- a/test/integration/forward_auth_integration_test.rb +++ b/test/integration/forward_auth_integration_test.rb @@ -177,13 +177,10 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest end test "spoofed X-Forwarded-Host is not reflected as a redirect target" do - # CLINCH_HOST pins the IdP origin (as in production) so base_url cannot be - # influenced by the request; this isolates the return_to/redirect behaviour. - original_clinch_host = ENV["CLINCH_HOST"] - ENV["CLINCH_HOST"] = "https://auth.example.com" - # No forward-auth app exists for evil.com, and no valid rd is supplied. The - # attacker-controlled host must NOT be stored or reflected into the signin URL. + # attacker-controlled host must NOT be stored or reflected into the signin URL, + # and base_url must come from CLINCH_HOST (or the safe localhost default in + # test) rather than the request host. get "/api/verify", headers: { "X-Forwarded-Host" => "evil.com", "X-Forwarded-Uri" => "/steal" @@ -193,8 +190,6 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest assert_match %r{/signin}, response.location refute_includes response.location, "evil.com" refute_match(/evil\.com/, session[:return_to_after_authenticating].to_s) - ensure - ENV["CLINCH_HOST"] = original_clinch_host end test "return URL functionality after authentication" do