<%= t(:webhook_disabled_info_html, :link => link_to(l(:label_integrations), settings_path(:tab => 'integrations'))) %>
+<% end %> + +<%= render :partial => 'webhooks/list', :locals => { :webhooks => @webhooks, :show_author => true, :back_url => admin_webhooks_path } %> diff --git a/app/views/webhooks/_form.html.erb b/app/views/webhooks/_form.html.erb index 2f99a6469..020fe19f6 100644 --- a/app/views/webhooks/_form.html.erb +++ b/app/views/webhooks/_form.html.erb @@ -3,12 +3,16 @@<%= text_field_tag 'webhook_user', @webhook.user.name, disabled: true %>
+ <% end %><%= f.text_field :url, required: true, size: 60 %> <%= l :webhook_url_info %>
- <%= f.text_field :secret %> + <%= f.text_field :secret, :value => (secret_hidden? ? '' : @webhook.secret) %> <%= raw l :webhook_secret_info_html %> + <% if secret_hidden? %><%= l :webhook_secret_keep_info %><% end %>
<%= f.check_box :active %>
| <%= l :field_user %> | <% end %> +<%= l :field_active %> | +<%= l :field_url %> | +<%= l :label_webhook_events %> | +<%= l :label_project_plural %> | ++ |
|---|---|---|---|---|---|
| <%= link_to_user webhook.user %> | <% end %> +<%= webhook.active ? l(:general_text_Yes) : l(:general_text_No) %> | +<%= truncate webhook.url, length: 40 %> | +<%= safe_join webhook.events.map{|e| content_tag :code, e }, ', ' %> | +<%= safe_join webhook.projects.visible.map{|p| link_to_project(p) }, ', ' %> | ++ <%= link_to sprite_icon('edit', l(:button_edit)), edit_webhook_path(webhook, link_params), class: 'icon icon-edit' %> + <%= link_to sprite_icon('del', l(:button_delete)), webhook_path(webhook, link_params), :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, :class => 'icon icon-del' %> + | +
<%= l(:label_no_data) %>
+<% end %> diff --git a/app/views/webhooks/edit.html.erb b/app/views/webhooks/edit.html.erb index 4afb7b0c3..244b26aa8 100644 --- a/app/views/webhooks/edit.html.erb +++ b/app/views/webhooks/edit.html.erb @@ -2,5 +2,7 @@ <%= labelled_form_for @webhook, html: { method: :patch } do |f| %> <%= render :partial => 'form', :locals => { :f => f } %> + <%= back_url_hidden_field_tag %> <%= submit_tag l(:button_save) %> + <%= cancel_button_tag webhooks_path %> <% end %> diff --git a/app/views/webhooks/index.html.erb b/app/views/webhooks/index.html.erb index 3ac2aef4c..810f97ce0 100644 --- a/app/views/webhooks/index.html.erb +++ b/app/views/webhooks/index.html.erb @@ -4,32 +4,4 @@ <%= title l :label_webhook_plural %> -<% if @webhooks.any? %> -| <%= l :field_active %> | -<%= l :field_url %> | -<%= l :label_webhook_events %> | -<%= l :label_project_plural %> | -- |
|---|---|---|---|---|
| <%= webhook.active ? l(:general_text_Yes) : l(:general_text_No) %> | -<%= truncate webhook.url, length: 40 %> | -<%= safe_join webhook.events.map{|e| content_tag :code, e }, ', ' %> | -<%= safe_join webhook.projects.visible.map{|p| link_to_project(p) }, ', ' %> | -- <%= link_to sprite_icon('edit', l(:button_edit)), edit_webhook_path(webhook), class: 'icon icon-edit' %> - <%= link_to sprite_icon('del', l(:button_delete)), webhook_path(webhook), :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, :class => 'icon icon-del' %> - | -
<%= l(:label_no_data) %>
-<% end %> +<%= render :partial => 'webhooks/list', :locals => { :webhooks => @webhooks, :show_author => false, :back_url => nil } %> diff --git a/app/views/webhooks/new.html.erb b/app/views/webhooks/new.html.erb index 50359f320..14d9dddf7 100644 --- a/app/views/webhooks/new.html.erb +++ b/app/views/webhooks/new.html.erb @@ -2,6 +2,7 @@ <%= labelled_form_for @webhook, url: webhooks_path do |f| %> <%= render :partial => 'webhooks/form', locals: { f: f } %> + <%= back_url_hidden_field_tag %> <%= submit_tag l(:button_create) %> - <%= link_to l(:button_cancel), webhooks_path %> + <%= cancel_button_tag webhooks_path %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 2a30a9d3a..274362a8c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1197,6 +1197,8 @@ en: webhook_event_deleted: "%{object_name} deleted" webhook_url_info: Redmine will send a POST request to this URL whenever one of the selected events occurs in one of the selected projects. webhook_secret_info_html: If provided, Redmine will use this to create a hash signature that is sent with each delivery as the value of the X-Redmine-Signature-256 header. + webhook_secret_keep_info: The secret of another user is not displayed. Leave this field blank to keep it unchanged. + webhook_disabled_info_html: Webhooks are disabled, so none of the webhooks below is triggered. They are still listed here so that they can be reviewed or removed. Webhooks can be enabled in the %{link} settings. button_login: Login button_submit: Submit diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 6addfcf30..10f50375d 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -1508,6 +1508,11 @@ fr: selected events occurs in one of the selected projects. webhook_secret_info_html: If provided, Redmine will use this to create a hash signature that is sent with each delivery as the value of the X-Redmine-Signature-256 header. + webhook_secret_keep_info: "Le secret d'un autre utilisateur n'est pas affiché. Laisser + ce champ vide pour le conserver." + webhook_disabled_info_html: "Les webhooks sont désactivés : aucun des webhooks ci-dessous + n'est déclenché. Ils restent listés ici afin de pouvoir être examinés ou supprimés. + Les webhooks peuvent être activés dans la configuration %{link}." setting_webhooks_enabled: Enable webhooks label_integrations: Integrations label_alert_note: Note diff --git a/config/routes.rb b/config/routes.rb index ce15da420..34bae573c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -380,6 +380,7 @@ Rails.application.routes.draw do get 'admin', :to => 'admin#index' get 'admin/projects', :to => 'admin#projects' get 'admin/plugins', :to => 'admin#plugins' + get 'admin/webhooks', :to => 'admin#webhooks' get 'admin/info', :to => 'admin#info' post 'admin/test_email', :to => 'admin#test_email', :as => 'test_email' post 'admin/default_configuration', :to => 'admin#default_configuration' diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index a1bc97e50..61363ac71 100644 --- a/lib/redmine/preparation.rb +++ b/lib/redmine/preparation.rb @@ -293,6 +293,10 @@ module Redmine :caption => :'doorkeeper.layouts.admin.nav.applications', :icon => 'apps', :html => {:class => 'icon icon-applications'} + menu.push :webhooks, {:controller => 'admin', :action => 'webhooks'}, + :caption => :label_webhook_plural, + :icon => 'webhook', + :html => {:class => 'icon icon-webhook'} menu.push :plugins, {:controller => 'admin', :action => 'plugins'}, :last => true, :icon => 'plugins', diff --git a/test/functional/admin_controller_test.rb b/test/functional/admin_controller_test.rb index 3c8f50fdf..3776b2747 100644 --- a/test/functional/admin_controller_test.rb +++ b/test/functional/admin_controller_test.rb @@ -168,6 +168,45 @@ class AdminControllerTest < Redmine::ControllerTest end end + def test_webhooks + hook = Webhook.create!(:url => 'https://example.com/dlopper/hook', :user => User.find_by_login('dlopper'), + :events => %w(issue.created), :projects => [Project.find(1)]) + other_hook = Webhook.create!(:url => 'https://example.com/admin/hook', :user => User.find(1), + :events => %w(issue.updated), :projects => [Project.find(1)]) + + with_settings :webhooks_enabled => '1' do + get :webhooks + end + assert_response :success + + assert_select "tr#webhook_#{hook.id}" do + assert_select 'td', :text => hook.user.name + assert_select 'td', :text => hook.url + assert_select "td.buttons a[href=?]", "/webhooks/#{hook.id}/edit?back_url=%2Fadmin%2Fwebhooks" + end + assert_select "tr#webhook_#{other_hook.id}" + assert_select 'div.contextual a[href=?]', '/webhooks/new?back_url=%2Fadmin%2Fwebhooks' + end + + def test_webhooks_should_remain_accessible_without_creation_link_when_disabled + Webhook.create!(:url => 'https://example.com/dlopper/hook', :user => User.find_by_login('dlopper'), + :events => %w(issue.created), :projects => [Project.find(1)]) + + with_settings :webhooks_enabled => '0' do + get :webhooks + assert_response :success + assert_select 'table.webhooks tr[id^=?]', 'webhook_' + assert_select 'div.contextual a', 0 + assert_select 'p.warning a[href=?]', '/settings?tab=integrations' + end + end + + def test_webhooks_should_be_denied_to_non_admin_users + @request.session[:user_id] = 2 + get :webhooks + assert_response :forbidden + end + def test_info get :info assert_response :success diff --git a/test/functional/webhooks_controller_test.rb b/test/functional/webhooks_controller_test.rb index 046be69ea..684c0b9c7 100644 --- a/test/functional/webhooks_controller_test.rb +++ b/test/functional/webhooks_controller_test.rb @@ -62,6 +62,7 @@ class WebhooksControllerTest < Redmine::ControllerTest test "should get new" do get :new assert_response :success + assert_select 'em.info', text: I18n.t(:webhook_secret_keep_info), count: 0 end test "should create webhook" do @@ -74,6 +75,15 @@ class WebhooksControllerTest < Redmine::ControllerTest test "should get edit" do get :edit, params: { id: @hook.id } assert_response :success end test "should update webhook" do @@ -87,8 +97,127 @@ class WebhooksControllerTest < Redmine::ControllerTest assert_response :not_found end + test 'index should not list hooks of other users to admins' do + admin_hook = @other_hook + dlopper_hook = @hook + login_as_admin + get :index + assert_response :success + assert_select 'td', text: admin_hook.url + assert_select 'td', text: dlopper_hook.url, count: 0 + end + + test 'admin should edit hook of other user' do + login_as_admin + get :edit, params: { id: @hook.id } + assert_response :success + assert_select 'input#webhook_user[disabled][value=?]', @dlopper.name + end + + test 'edit should not show the owner of ones own hook' do + get :edit, params: { id: @hook.id } + assert_response :success + assert_select 'input#webhook_user', count: 0 + end + + test 'admin should update hook of other user without becoming its owner' do + login_as_admin + patch :update, params: { id: @hook.id, webhook: { url: 'https://example.com/fixed/hook' } } + assert_redirected_to webhooks_path + @hook.reload + assert_equal 'https://example.com/fixed/hook', @hook.url + assert_equal @dlopper, @hook.user + end + + test 'admin should deactivate hook of other user' do + @hook.update_column :active, true + login_as_admin + patch :update, params: { id: @hook.id, webhook: { active: '0' } } + assert_not @hook.reload.active + end + + test 'admin should destroy hook of other user' do + login_as_admin + assert_difference 'Webhook.count', -1 do + delete :destroy, params: { id: @hook.id } + end + end + + test 'create should redirect to back_url' do + post :create, params: { webhook: { url: 'https://example.com/new/hook', events: %w(issue.created), project_ids: [@project.id] }, back_url: '/admin/webhooks' } + assert_redirected_to '/admin/webhooks' + end + + test 'update should redirect to back_url' do + login_as_admin + patch :update, params: { id: @hook.id, webhook: { url: 'https://example.com/fixed/hook' }, back_url: '/admin/webhooks' } + assert_redirected_to '/admin/webhooks' + end + + test 'edit should not disclose secret of other user' do + @hook.update_column :secret, 'v3rys3cret' + login_as_admin + get :edit, params: { id: @hook.id } + assert_response :success + assert_select 'input#webhook_secret' + assert_not_include 'v3rys3cret', response.body + end + + test 'update should keep secret of other user when submitted blank' do + @hook.update_column :secret, 'v3rys3cret' + login_as_admin + patch :update, params: { id: @hook.id, webhook: { url: @hook.url, secret: '' } } + assert_equal 'v3rys3cret', @hook.reload.secret + end + + test 'update should replace secret of other user when a new one is submitted' do + @hook.update_column :secret, 'v3rys3cret' + login_as_admin + patch :update, params: { id: @hook.id, webhook: { url: @hook.url, secret: 'newsecret' } } + assert_equal 'newsecret', @hook.reload.secret + end + + test 'owner should see and be able to clear their own secret' do + @hook.update_column :secret, 'v3rys3cret' + get :edit, params: { id: @hook.id } + assert_select 'input#webhook_secret[value=?]', 'v3rys3cret' + + patch :update, params: { id: @hook.id, webhook: { url: @hook.url, secret: '' } } + assert_equal '', @hook.reload.secret + end + + test 'admin should keep access to existing hooks when disabled' do + login_as_admin + with_settings webhooks_enabled: '0' do + get :edit, params: { id: @hook.id } + assert_response :success + + patch :update, params: { id: @hook.id, webhook: { url: 'https://example.com/fixed/hook' } } + assert_redirected_to webhooks_path + + assert_difference 'Webhook.count', -1 do + delete :destroy, params: { id: @hook.id } + end + end + end + + test 'admin should not create hooks when disabled' do + login_as_admin + with_settings webhooks_enabled: '0' do + get :index + assert_response :forbidden + + get :new + assert_response :forbidden + end + end + private + def login_as_admin + @request.session[:user_id] = User.find_by_login('admin').id + end + def create_hook(url: 'https://example.com/some/hook', user: User.find_by_login('dlopper'), events: %w(issue.created issue.updated), diff --git a/test/integration/routing/admin_test.rb b/test/integration/routing/admin_test.rb index 92d535f60..556110ba2 100644 --- a/test/integration/routing/admin_test.rb +++ b/test/integration/routing/admin_test.rb @@ -24,6 +24,7 @@ class RoutingAdminTest < Redmine::RoutingTest should_route 'GET /admin' => 'admin#index' should_route 'GET /admin/projects' => 'admin#projects' should_route 'GET /admin/plugins' => 'admin#plugins' + should_route 'GET /admin/webhooks' => 'admin#webhooks' should_route 'GET /admin/info' => 'admin#info' should_route 'POST /admin/test_email' => 'admin#test_email' should_route 'POST /admin/default_configuration' => 'admin#default_configuration'