Merge pull request #13787 from opf/revert-implementation/49872-extend…

…-csp-frame-ancestors-with-active-nextcloud-storage-hosts

Rollback #13611.

modules/storages/lib/open_project/storages/append_storages_hosts_to_csp_hook.rb

@@ -43,31 +43,18 @@ class ::OpenProject::Storages::AppendStoragesHostsToCspHook < OpenProject::Hook:
   # places of OpenProject, and we want to be able to upload in all those places
   # (work packages module, BCF module, notification center, boards, ...).
   def application_controller_before_action(context)
-    controller = context[:controller]
-
-    active_nextcloud_storages_hosts = ::Storages::NextcloudStorage
-                                        .where(id: ::Storages::ProjectStorage.select(:storage_id))
-                                        .pluck(:host)
-                                        .map(&method(:remove_uri_path))
-
-    if active_nextcloud_storages_hosts.present?
-      controller.append_content_security_policy_directives(
-        frame_ancestors: active_nextcloud_storages_hosts
-      )
-    end
-
     storage_ids = ::Storages::ProjectStorage.where(
-      project_id: Project.allowed_to(User.current, :manage_file_links)
+      project_id: Project.allowed_to(User.current, :manage_file_links),
     ).select(:storage_id)
     storages_hosts = ::Storages::Storage
-                       .where(id: storage_ids)
-                       .where.not(host: nil)
-                       .pluck(:host)
-                       .map(&method(:remove_uri_path))
+      .where(id: storage_ids)
+      .where.not(host: nil)
+      .pluck(:host)
+      .map(&method(:remove_uri_path))
 
     if storages_hosts.present?
       # secure_headers gem provides this helper method to append to the current content security policy
-      controller.append_content_security_policy_directives(connect_src: storages_hosts)
+      context[:controller].append_content_security_policy_directives(connect_src: storages_hosts)
     end
   end
 

modules/storages/spec/lib/append_storages_hosts_to_csp_hook_spec.rb

@@ -53,11 +53,10 @@ def trigger_application_controller_before_action_hook
   end
 
   shared_examples 'content security policy directives' do
-    it 'adds CSP connect_src and frame_ancestors directives' do
+    it 'adds CSP connect_src directives' do
       trigger_application_controller_before_action_hook
 
-      expect(controller).to have_received(:append_content_security_policy_directives).with(frame_ancestors: [storage.host]).ordered
-      expect(controller).to have_received(:append_content_security_policy_directives).with(connect_src: [storage.host]).ordered
+      expect(controller).to have_received(:append_content_security_policy_directives).with(connect_src: [storage.host])
     end
   end
 
@@ -85,10 +84,9 @@ def trigger_application_controller_before_action_hook
     context 'when current user is a member of the project without permission to manage file links' do
       current_user { create(:user, member_in_project: project, member_with_permissions: []) }
 
-      it 'adds CSP frame_ancestors directive only' do
+      it 'does not add CSP connect_src directive' do
         trigger_application_controller_before_action_hook
 
-        expect(controller).to have_received(:append_content_security_policy_directives).with(frame_ancestors: [storage.host]).once
         expect(controller).not_to have_received(:append_content_security_policy_directives).with(connect_src: [storage.host])
       end
     end
@@ -100,10 +98,9 @@ def trigger_application_controller_before_action_hook
         project.update(active: false)
       end
 
-      it 'adds CSP frame_ancestors directive only' do
+      it 'does not add CSP connect_src directive' do
         trigger_application_controller_before_action_hook
 
-        expect(controller).to have_received(:append_content_security_policy_directives).with(frame_ancestors: [storage.host])
         expect(controller).not_to have_received(:append_content_security_policy_directives).with(connect_src: [storage.host])
       end
     end

modules/storages/spec/requests/append_content_security_policy_spec.rb

@@ -45,21 +45,19 @@ def parse_csp(csp_string)
     context 'when logged in' do
       current_user { create(:user, member_in_project: project, member_with_permissions: %i[manage_file_links]) }
 
-      it 'appends storage host to the connect-src and frame-ancestors CSP' do
+      it 'appends storage host to the connect-src CSP' do
         get '/'
 
         csp = parse_csp(last_response.headers['Content-Security-Policy'])
-        expect(csp['frame-ancestors']).to eq(["'self'", storage.host])
         expect(csp['connect-src']).to include(storage.host)
       end
     end
 
     context 'when not logged in' do
-      it 'appends storage host to frame-ancestors CSP only' do
+      it 'does not append host to connect-src CSP' do
         get '/'
 
         csp = parse_csp(last_response.headers['Content-Security-Policy'])
-        expect(csp['frame-ancestors']).to eq(["'self'", storage.host])
         expect(csp['connect-src']).not_to include(storage.host)
       end
     end