From 2bd4d580d17e91605b4f350eafb2e9a50d6181c7 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Wed, 28 Aug 2019 18:27:18 -0500 Subject: [PATCH] Add system hooks for project/group membership updates When updating group and project members, new system hooks `user_update_for_group` and `user_update_for_team` will be executed. Fixes https://gitlab.com/gitlab-org/gitlab-ee/issues/12252 --- app/models/member.rb | 2 +- app/services/system_hooks_service.rb | 2 + .../unreleased/change-role-system-hook.yml | 5 +++ doc/system_hooks/system_hooks.md | 40 +++++++++++++++++++ spec/models/hooks/system_hook_spec.rb | 23 ++++++++++- spec/services/system_hooks_service_spec.rb | 10 +++++ 6 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/change-role-system-hook.yml diff --git a/app/models/member.rb b/app/models/member.rb index dbae1076670..6457fe9ef0c 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -399,7 +399,7 @@ class Member < ApplicationRecord end def post_update_hook - # override in sub class + system_hook_service.execute_hooks_for(self, :update) end def post_destroy_hook diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 858e04f43b2..34260d12a62 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -74,9 +74,11 @@ class SystemHooksService when ProjectMember return "user_add_to_team" if event == :create return "user_remove_from_team" if event == :destroy + return "user_update_for_team" if event == :update when GroupMember return 'user_add_to_group' if event == :create return 'user_remove_from_group' if event == :destroy + return 'user_update_for_group' if event == :update else "#{model.class.name.downcase}_#{event}" end diff --git a/changelogs/unreleased/change-role-system-hook.yml b/changelogs/unreleased/change-role-system-hook.yml new file mode 100644 index 00000000000..adc9e43b1f2 --- /dev/null +++ b/changelogs/unreleased/change-role-system-hook.yml @@ -0,0 +1,5 @@ +--- +title: Add system hooks for project/group membership updates +merge_request: 32371 +author: Brandon Williams +type: added diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index 1e9eb15533a..24a334e7067 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -13,6 +13,7 @@ Your GitLab instance can perform HTTP POST requests on the following events: - `project_update` - `user_add_to_team` - `user_remove_from_team` +- `user_update_for_team` - `user_create` - `user_destroy` - `user_failed_login` @@ -24,6 +25,7 @@ Your GitLab instance can perform HTTP POST requests on the following events: - `group_rename` - `user_add_to_group` - `user_remove_from_group` +- `user_update_for_group` The triggers for most of these are self-explanatory, but `project_update` and `project_rename` deserve some clarification: `project_update` is fired any time an attribute of a project is changed (name, description, tags, etc.) *unless* the `path` attribute is also changed. In that case, a `project_rename` is triggered instead (so that, for instance, if all you care about is the repo URL, you can just listen for `project_rename`). @@ -173,6 +175,26 @@ Please refer to `group_rename` and `user_rename` for that case. } ``` +**Team Member Updated:** + +```json +{ + "created_at": "2012-07-21T07:30:56Z", + "updated_at": "2012-07-21T07:38:22Z", + "event_name": "user_update_for_team", + "access_level": "Maintainer", + "project_id": 74, + "project_name": "StoreCloud", + "project_path": "storecloud", + "project_path_with_namespace": "jsmith/storecloud", + "user_email": "johnsmith@gmail.com", + "user_name": "John Smith", + "user_username": "johnsmith", + "user_id": 41, + "project_visibility": "visibilitylevel|private" +} +``` + **User created:** ```json @@ -349,6 +371,24 @@ If the user is blocked via LDAP, `state` will be `ldap_blocked`. } ``` +**Group Member Updated:** + +```json +{ + "created_at": "2012-07-21T07:30:56Z", + "updated_at": "2012-07-21T07:38:22Z", + "event_name": "user_update_for_group", + "group_access": "Maintainer", + "group_id": 78, + "group_name": "StoreCloud", + "group_path": "storecloud", + "user_email": "johnsmith@gmail.com", + "user_name": "John Smith", + "user_username": "johnsmith", + "user_id": 41 +} +``` + ## Push events Triggered when you push to the repository, except when pushing tags. diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index e0d4d2e4858..a4d202dc4f8 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -64,7 +64,7 @@ describe SystemHook do ).once end - it "project_create hook" do + it "project member create hook" do project.add_maintainer(user) expect(WebMock).to have_requested(:post, system_hook.url).with( @@ -73,7 +73,7 @@ describe SystemHook do ).once end - it "project_destroy hook" do + it "project member destroy hook" do project.add_maintainer(user) project.project_members.destroy_all # rubocop: disable DestroyAll @@ -83,6 +83,15 @@ describe SystemHook do ).once end + it "project member update hook" do + project.add_guest(user) + + expect(WebMock).to have_requested(:post, system_hook.url).with( + body: /user_update_for_team/, + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } + ).once + end + it 'group create hook' do create(:group) @@ -119,6 +128,16 @@ describe SystemHook do headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } ).once end + + it 'group member update hook' do + group.add_guest(user) + group.add_maintainer(user) + + expect(WebMock).to have_requested(:post, system_hook.url).with( + body: /user_update_for_group/, + headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'System Hook' } + ).once + end end describe '.repository_update_hooks' do diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index f5c6e972953..d72e5cc2b16 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -19,6 +19,7 @@ describe SystemHooksService do it { expect(event_data(project, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :path, :project_id, :owner_name, :owner_email, :project_visibility) } it { expect(event_data(project_member, :create)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) } it { expect(event_data(project_member, :destroy)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) } + it { expect(event_data(project_member, :update)).to include(:event_name, :created_at, :updated_at, :project_name, :project_path, :project_path_with_namespace, :project_id, :user_name, :user_username, :user_email, :user_id, :access_level, :project_visibility) } it { expect(event_data(key, :create)).to include(:username, :key, :id) } it { expect(event_data(key, :destroy)).to include(:username, :key, :id) } it { expect(event_data(deploy_key, :create)).to include(:key, :id) } @@ -70,6 +71,13 @@ describe SystemHooksService do ) end + it do + expect(event_data(group_member, :update)).to include( + :event_name, :created_at, :updated_at, :group_name, :group_path, + :group_id, :user_id, :user_username, :user_name, :user_email, :group_access + ) + end + it 'includes the correct project visibility level' do data = event_data(project, :create) @@ -145,6 +153,7 @@ describe SystemHooksService do it { expect(event_name(project, :update)).to eq "project_update" } it { expect(event_name(project_member, :create)).to eq "user_add_to_team" } it { expect(event_name(project_member, :destroy)).to eq "user_remove_from_team" } + it { expect(event_name(project_member, :update)).to eq "user_update_for_team" } it { expect(event_name(key, :create)).to eq 'key_create' } it { expect(event_name(key, :destroy)).to eq 'key_destroy' } it { expect(event_name(group, :create)).to eq 'group_create' } @@ -152,6 +161,7 @@ describe SystemHooksService do it { expect(event_name(group, :rename)).to eq 'group_rename' } it { expect(event_name(group_member, :create)).to eq 'user_add_to_group' } it { expect(event_name(group_member, :destroy)).to eq 'user_remove_from_group' } + it { expect(event_name(group_member, :update)).to eq 'user_update_for_group' } end def event_data(*args)