diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 727e0f09f..615082809 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -23,6 +23,10 @@ class AccountsController < ApplicationController rescue_from ActionController::ParameterMissing, with: :handle_parameter_missing + # Prevent clickjacking (CWE-1021): block account pages from being embedded in external iframes. + # Uses after_action (not content_security_policy DSL) so it runs even when a before_action redirects. + after_action :set_clickjacking_headers + def index @cbp_map = PeopleDecorator.new(@people).commits_by_project_map @positions_map = Position.where(id: @cbp_map.values.map(&:first).flatten) @@ -93,6 +97,15 @@ def settings; end private + def set_clickjacking_headers + response.headers['X-Frame-Options'] = 'SAMEORIGIN' + if (policy = request.content_security_policy) + modified = policy.clone + modified.frame_ancestors :self + request.content_security_policy = modified + end + end + def find_claimed_people total_entries = params[:query].blank? ? Person::Count.claimed : nil @people = Person.find_claimed(params[:query], params[:sort]) diff --git a/config/initializers/clearance_session_security.rb b/config/initializers/clearance_session_security.rb new file mode 100644 index 000000000..913e79c27 --- /dev/null +++ b/config/initializers/clearance_session_security.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# CWE-1021: Prevent clickjacking on session pages. +# +# GET /sessions/new and DELETE /sessions are routed directly to +# Clearance::SessionsController (bypassing our SessionsController subclass), +# so security headers are added here via to_prepare, which re-runs on every +# code reload in development and once at boot in production. +Rails.application.config.to_prepare do + Clearance::SessionsController.class_eval do + # Uses after_action (not content_security_policy DSL) so it runs even when a before_action redirects. + # Guard prevents duplicate registration on each code reload in development. + unless _process_action_callbacks.any? { |cb| cb.filter == :set_clickjacking_headers_on_session } + after_action :set_clickjacking_headers_on_session + end + + private + + def set_clickjacking_headers_on_session + response.headers['X-Frame-Options'] = 'SAMEORIGIN' + if (policy = request.content_security_policy) + modified = policy.clone + modified.frame_ancestors :self + request.content_security_policy = modified + end + end + end +end diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index 6e39f5171..7fc7a03f1 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -557,4 +557,12 @@ class AccountsControllerTest < ActionController::TestCase end end end + + describe 'clickjacking protection (CWE-1021)' do + it 'sets X-Frame-Options: SAMEORIGIN on account show' do + get :show, params: { id: admin.login } + + _(response.headers['X-Frame-Options']).must_equal 'SAMEORIGIN' + end + end end diff --git a/test/integration/clickjacking_protection_test.rb b/test/integration/clickjacking_protection_test.rb new file mode 100644 index 000000000..f47540b77 --- /dev/null +++ b/test/integration/clickjacking_protection_test.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'test_helper' + +# Regression tests for CWE-1021 (clickjacking). +# +# GET /sessions/new is routed to Clearance::SessionsController (not our SessionsController +# subclass), so an ActionDispatch::IntegrationTest is used — it runs the full middleware +# stack and exercises the to_prepare patch in clearance_session_security.rb. +class ClickjackingProtectionTest < ActionDispatch::IntegrationTest + describe 'GET /sessions/new (Clearance::SessionsController)' do + it 'sets X-Frame-Options: SAMEORIGIN' do + get new_session_path + + _(response.headers['X-Frame-Options']).must_equal 'SAMEORIGIN' + end + + it 'includes frame-ancestors \'self\' in Content-Security-Policy' do + get new_session_path + + _(response.headers['Content-Security-Policy']).must_include "frame-ancestors 'self'" + end + end + + describe 'GET /accounts/:id' do + it 'sets X-Frame-Options: SAMEORIGIN' do + account = create(:account) + get account_path(account) + + _(response.headers['X-Frame-Options']).must_equal 'SAMEORIGIN' + end + + it 'includes frame-ancestors \'self\' in Content-Security-Policy' do + account = create(:account) + get account_path(account) + + _(response.headers['Content-Security-Policy']).must_include "frame-ancestors 'self'" + end + end +end