OTWO-7713 CWE-1021: Clickjacking — X-Frame-Options and CSP frame-ance…

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])

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

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

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