diff --git a/app/controllers/api/forward_auth_controller.rb b/app/controllers/api/forward_auth_controller.rb index 0cd2902..3cf659a 100644 --- a/app/controllers/api/forward_auth_controller.rb +++ b/app/controllers/api/forward_auth_controller.rb @@ -64,26 +64,16 @@ module Api return render_forbidden("No authentication rule configured for this domain") end else - Rails.logger.info "ForwardAuth: User #{user.email_address} authenticated (no domain specified)" - end - - headers = if app - app.headers_for_user(user) - else - Application::DEFAULT_HEADERS.map { |key, header_name| - case key - when :user, :email, :name - [header_name, user.email_address] - when :username - [header_name, user.username] if user.username.present? - when :groups - user.groups.any? ? [header_name, user.groups.map(&:name).join(",")] : nil - when :admin - [header_name, user.admin? ? "true" : "false"] - end - }.compact.to_h + # Fail closed: with no host we cannot resolve an application or evaluate its + # group policy. Emitting identity headers here would bypass all per-domain + # access control, so reject instead. + Rails.logger.info "ForwardAuth: Access denied - no host header present" + return render_forbidden("No host header present") end + # Reaching here implies a matching, active application was resolved above + # (every other path returns forbidden), so headers are always scoped to it. + headers = app.headers_for_user(user) headers.each { |key, value| response.headers[key] = value } Rails.logger.debug "ForwardAuth: Headers sent: #{headers.keys.join(", ")}" if headers.any? diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index 412747e..4625b0c 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -4,7 +4,11 @@ class OidcController < ApplicationController # Discovery and JWKS endpoints are public # authorize is also unauthenticated to handle prompt=none and prompt=login specially allow_unauthenticated_access only: [:discovery, :jwks, :token, :revoke, :userinfo, :logout, :authorize] - skip_before_action :verify_authenticity_token, only: [:token, :revoke, :userinfo, :logout, :authorize, :consent] + # Machine-to-machine endpoints (token/revoke/userinfo) and pure redirect handlers + # (logout/authorize) legitimately skip CSRF. The consent endpoint is browser-facing + # and state-changing (it grants OAuth scopes), so it MUST keep CSRF protection — the + # consent form already embeds the token via form_with. + skip_before_action :verify_authenticity_token, only: [:token, :revoke, :userinfo, :logout, :authorize] # RFC 6749 §4.1.2.1: client_id and redirect_uri must be validated *before* any # other error can be reported via redirect. Failures here render a plain page. diff --git a/test/controllers/api/forward_auth_controller_test.rb b/test/controllers/api/forward_auth_controller_test.rb index 8526231..822b8aa 100644 --- a/test/controllers/api/forward_auth_controller_test.rb +++ b/test/controllers/api/forward_auth_controller_test.rb @@ -242,6 +242,20 @@ module Api assert_equal "No authentication rule configured for this domain", response.headers["x-auth-reason"] end + # Fail closed when no host can be determined: emitting identity headers without + # an application would bypass all per-domain group access control. + test "should fail closed and emit no identity headers when host is absent" do + sign_in_as(@user) + + # Blank both host sources so forwarded_host is not present. + get "/api/verify", headers: {"X-Forwarded-Host" => "", "Host" => ""} + + assert_response 403 + assert_equal "No host header present", response.headers["x-auth-reason"] + assert_nil response.headers["X-Remote-User"] + assert_nil response.headers["X-Remote-Groups"] + end + # Security Tests test "should handle very long domain names" do long_domain = "a" * 250 + ".example.com" diff --git a/test/controllers/oidc_authorization_code_security_test.rb b/test/controllers/oidc_authorization_code_security_test.rb index b158d02..5e15a0e 100644 --- a/test/controllers/oidc_authorization_code_security_test.rb +++ b/test/controllers/oidc_authorization_code_security_test.rb @@ -34,6 +34,25 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest # CRITICAL SECURITY TESTS # ==================== + test "consent endpoint rejects cross-site POST without a CSRF token" do + sign_in_as(@user) + + # Forgery protection is disabled in the test env by default; enable it so the + # before_action actually runs, mirroring production behaviour. + original = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = true + begin + # No authenticity_token param: a forged cross-site submission. Because + # :consent is NOT in the verify_authenticity_token skip list, this must be + # rejected before the action can grant any OAuth scopes. + post "/oauth/authorize/consent", params: {approve: "true"} + + assert_response :unprocessable_entity + ensure + ActionController::Base.allow_forgery_protection = original + end + end + test "prevents authorization code reuse - sequential attempts" do # Create consent OidcUserConsent.create!(