diff --git a/app/controllers/projects/hook_logs_controller.rb b/app/controllers/projects/hook_logs_controller.rb index 3ab4c34737ddf7ed96f5567c0ccf0b6ba3ceffcf..756e86555ac1fac9d348e4a23f854713d8cecf50 100644 --- a/app/controllers/projects/hook_logs_controller.rb +++ b/app/controllers/projects/hook_logs_controller.rb @@ -1,7 +1,7 @@ # 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 diff --git a/ee/app/controllers/groups/hook_logs_controller.rb b/ee/app/controllers/groups/hook_logs_controller.rb index 2f88b1b1927be123efeea7a7c1f3a7c1f4895106..200d18e9178cb796734e46823350b7aedd1c4eb2 100644 --- a/ee/app/controllers/groups/hook_logs_controller.rb +++ b/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 diff --git a/ee/spec/requests/custom_roles/admin_web_hook/request_spec.rb b/ee/spec/requests/custom_roles/admin_web_hook/request_spec.rb index f976eb438e8f53c6f093968d4edb059715fda0b4..f141461b6baa7751d32a31f98f2f891f6a1a9181 100644 --- a/ee/spec/requests/custom_roles/admin_web_hook/request_spec.rb +++ b/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 diff --git a/ee/spec/requests/groups/hook_logs_controller_spec.rb b/ee/spec/requests/groups/hook_logs_controller_spec.rb index 929b5ee4fbbc6f39de326f86066b43273939d4be..dc8f638c8deb3bedc0a02f4d9edab04877ab0c1f 100644 --- a/ee/spec/requests/groups/hook_logs_controller_spec.rb +++ b/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