Merge branch 'kkloss-hook-logs-with-admin_hook-permission' into 'master'

Allow viewing webhook logs with admin_hook permission

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/153806



Merged-by: Brian Williams <bwilliams@gitlab.com>
Approved-by: Lee Tickett <ltickett@gitlab.com>
Approved-by: Brian Williams <bwilliams@gitlab.com>
Approved-by: Alex Buijs <abuijs@gitlab.com>
Reviewed-by: Alex Buijs <abuijs@gitlab.com>
Reviewed-by: Lee Tickett <ltickett@gitlab.com>
Co-authored-by: Kev Kloss <kkloss@gitlab.com>

app/controllers/projects/hook_logs_controller.rb

 # frozen_string_literal: true
 
 class Projects::HookLogsController < Projects::ApplicationController
-  before_action :authorize_admin_project!
+  before_action :authorize_admin_hook!
 
   include WebHooks::HookLogActions
 
@@ -16,4 +16,8 @@ def hook
   def after_retry_redirect_path
     edit_project_hook_path(@project, hook)
   end
+
+  def authorize_admin_hook!
+    render_404 unless can?(current_user, :admin_web_hook, project)
+  end
 end

ee/app/controllers/groups/hook_logs_controller.rb

@@ -2,7 +2,7 @@
 
 module Groups
   class HookLogsController < Groups::ApplicationController
-    before_action :authorize_admin_group!
+    before_action :authorize_admin_hook!
 
     include ::WebHooks::HookLogActions
 
@@ -17,5 +17,9 @@ def hook
     def after_retry_redirect_path
       edit_group_hook_path(@group, hook)
     end
+
+    def authorize_admin_hook!
+      render_404 unless can?(current_user, :admin_web_hook, group)
+    end
   end
 end

ee/spec/requests/custom_roles/admin_web_hook/request_spec.rb

@@ -6,7 +6,8 @@
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
   let_it_be(:project) { create(:project, group: group) }
-  let_it_be(:role) { create(:member_role, :guest, :admin_web_hook, namespace: group) }
+  let(:can_admin_web_hook) { true }
+  let(:role) { create(:member_role, :guest, admin_web_hook: can_admin_web_hook, namespace: group) }
 
   before do
     stub_licensed_features(custom_roles: true)
@@ -24,7 +25,7 @@
 
     describe '#edit' do
       it 'allows access' do
-        get edit_path
+        get edit_hook_path
 
         expect(response).to have_gitlab_http_status(:ok)
       end
@@ -65,35 +66,94 @@
     end
   end
 
-  describe Projects::HooksController do
-    let_it_be(:membership) { create(:project_member, :guest, member_role: role, user: user, project: project) }
+  shared_examples 'HookLogsController' do
+    describe '#show' do
+      it 'allows access' do
+        get show_path
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+
+      context 'without admin_web_hook permission' do
+        let(:can_admin_web_hook) { false }
+
+        it 'does not allow access' do
+          get show_path
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+
+    describe '#retry' do
+      it 'allows access' do
+        stub_request(:post, hook.interpolated_url)
+
+        post retry_path
+
+        expect(response).to have_gitlab_http_status(:redirect)
+        expect(response).to redirect_to(edit_hook_path)
+      end
+    end
+  end
+
+  context 'in a project' do
     let_it_be(:project_hook) { create(:project_hook, project: project, url: 'http://example.test/') }
 
     let(:hook) { create(:project_hook, project: project) }
+    let(:edit_hook_path) { edit_project_hook_path(project, hook) }
+
+    before do
+      create(:project_member, :guest, member_role: role, user: user, project: project)
+    end
+
+    describe Projects::HooksController do
+      let(:index_path) { project_hooks_path(project) }
+      let(:create_path) { project_hooks_path(project) }
+      let(:update_path) { project_hook_path(project, project_hook) }
+      let(:destroy_path) { project_hook_path(project, project_hook) }
+      let(:test_path) { test_project_hook_path(project, project_hook) }
+
+      it_behaves_like 'HooksController'
+    end
+
+    describe Projects::HookLogsController do
+      let(:hook_log) { create(:web_hook_log, web_hook: hook, internal_error_message: 'get error') }
 
-    let(:index_path) { project_hooks_path(project) }
-    let(:edit_path) { edit_project_hook_path(project, project_hook) }
-    let(:create_path) { project_hooks_path(project) }
-    let(:update_path) { project_hook_path(project, project_hook) }
-    let(:destroy_path) { project_hook_path(project, hook) }
-    let(:test_path) { test_project_hook_path(project, project_hook) }
+      let(:show_path) { hook_log.present.details_path }
+      let(:retry_path) { hook_log.present.retry_path }
 
-    it_behaves_like 'HooksController'
+      it_behaves_like 'HookLogsController'
+    end
   end
 
-  describe Groups::HooksController do
-    let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) }
+  context 'in a group' do
     let_it_be(:group_hook) { create(:group_hook, group: group, url: 'http://example.test/') }
 
     let(:hook) { create(:group_hook, group: group) }
+    let(:edit_hook_path) { edit_group_hook_path(group, hook) }
+
+    before do
+      create(:group_member, :guest, member_role: role, user: user, group: group)
+    end
+
+    describe Groups::HooksController do
+      let(:index_path) { group_hooks_path(group) }
+      let(:create_path) { group_hooks_path(group) }
+      let(:update_path) { group_hook_path(group, group_hook) }
+      let(:destroy_path) { group_hook_path(group, hook) }
+      let(:test_path) { test_group_hook_path(group, group_hook) }
 
-    let(:index_path) { group_hooks_path(group) }
-    let(:edit_path) { edit_group_hook_path(group, group_hook) }
-    let(:create_path) { group_hooks_path(group) }
-    let(:update_path) { group_hook_path(group, group_hook) }
-    let(:destroy_path) { group_hook_path(group, hook) }
-    let(:test_path) { test_group_hook_path(group, group_hook) }
+      it_behaves_like 'HooksController'
+    end
+
+    describe Groups::HookLogsController do
+      let(:hook_log) { create(:web_hook_log, web_hook: hook, internal_error_message: 'get error') }
 
-    it_behaves_like 'HooksController'
+      let(:show_path) { hook_log.present.details_path }
+      let(:retry_path) { hook_log.present.retry_path }
+
+      it_behaves_like 'HookLogsController'
+    end
   end
 end

ee/spec/requests/groups/hook_logs_controller_spec.rb

@@ -7,13 +7,26 @@
   let_it_be_with_refind(:web_hook) { create(:group_hook) }
   let_it_be_with_refind(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) }
 
-  let(:group) { web_hook.group }
+  let_it_be(:group) { web_hook.group }
 
   it_behaves_like WebHooks::HookLogActions do
     let(:edit_hook_path) { edit_group_hook_url(group, web_hook) }
 
-    before do
+    before_all do
       group.add_owner(user)
     end
   end
+
+  context 'with a custom role' do
+    let_it_be(:role) { create(:member_role, :guest, :admin_web_hook) }
+    let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) }
+
+    before do
+      stub_licensed_features(custom_roles: true)
+    end
+
+    it_behaves_like WebHooks::HookLogActions do
+      let(:edit_hook_path) { edit_group_hook_url(group, web_hook) }
+    end
+  end
 end