From c85c146aa2042710caddc6666ce8f9e07b2fe5ca Mon Sep 17 00:00:00 2001 From: Gauvain Pocentek Date: Sun, 23 Oct 2016 09:46:39 +0200 Subject: [PATCH 1/2] Add support for token attr in project hooks API The UI allows to define a token to validate payload on the target URL, this patch adds the feature to the API. --- CHANGELOG.md | 1 + doc/api/projects.md | 2 ++ lib/api/entities.rb | 2 +- lib/api/project_hooks.rb | 6 ++++-- spec/requests/api/project_hooks_spec.rb | 4 ++++ 5 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14907e1546e..4e0417885d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - Trim leading and trailing whitespace on project_path (Linus Thiel) - Prevent award emoji via notes for issues/MRs authored by user (barthc) +- Adds support for the `token` attribute in project hooks API (Gauvain Pocentek) - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) - Fix extra space on Build sidebar on Firefox !7060 - Fix mobile layout issues in admin user overview page !7087 diff --git a/doc/api/projects.md b/doc/api/projects.md index b69db90e70d..96145c0cafa 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1139,6 +1139,7 @@ Parameters: | `pipeline_events` | boolean | no | Trigger hook on pipeline events | | `wiki_events` | boolean | no | Trigger hook on wiki events | | `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook | +| `token` | string | no | Secret token to validate received payloads | ### Edit project hook @@ -1164,6 +1165,7 @@ Parameters: | `pipeline_events` | boolean | no | Trigger hook on pipeline events | | `wiki_events` | boolean | no | Trigger hook on wiki events | | `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook | +| `token` | string | no | Secret token to validate received payloads | ### Delete project hook diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ab9d2d54f4b..94586040fa4 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -50,7 +50,7 @@ module API expose :project_id, :push_events expose :issues_events, :merge_requests_events, :tag_push_events expose :note_events, :build_events, :pipeline_events, :wiki_page_events - expose :enable_ssl_verification + expose :enable_ssl_verification, :token end class BasicProjectDetails < Grape::Entity diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 14f5be3b5f6..dd93a85dc54 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -47,7 +47,8 @@ module API :build_events, :pipeline_events, :wiki_page_events, - :enable_ssl_verification + :enable_ssl_verification, + :token ] @hook = user_project.hooks.new(attrs) @@ -82,7 +83,8 @@ module API :build_events, :pipeline_events, :wiki_page_events, - :enable_ssl_verification + :enable_ssl_verification, + :token ] if @hook.update_attributes attrs diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index cfcdcad74cd..53113c62996 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -36,6 +36,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response.first['pipeline_events']).to eq(true) expect(json_response.first['wiki_page_events']).to eq(true) expect(json_response.first['enable_ssl_verification']).to eq(true) + expect(json_response.first['token']).to eq('S3cr3t') end end @@ -62,6 +63,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) + expect(json_response['token']).to eq(hook.token) end it "returns a 404 error if hook id is not available" do @@ -99,6 +101,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['pipeline_events']).to eq(false) expect(json_response['wiki_page_events']).to eq(false) expect(json_response['enable_ssl_verification']).to eq(true) + expect(json_response['token']).to eq('S3cr3t') end it "returns a 400 error if url not given" do @@ -127,6 +130,7 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) + expect(json_response['token']).to eq(hook.token) end it "returns 404 error if hook id not found" do From f77be11cb9caa62cdd4690a53c73b6d34e102148 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 1 Nov 2016 11:40:06 +0000 Subject: [PATCH 2/2] Ensure hook tokens are write-only in the API --- doc/api/projects.md | 4 +-- lib/api/entities.rb | 2 +- spec/requests/api/project_hooks_spec.rb | 36 ++++++++++++++++++++++--- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/doc/api/projects.md b/doc/api/projects.md index 96145c0cafa..d7b99c35b33 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1139,7 +1139,7 @@ Parameters: | `pipeline_events` | boolean | no | Trigger hook on pipeline events | | `wiki_events` | boolean | no | Trigger hook on wiki events | | `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook | -| `token` | string | no | Secret token to validate received payloads | +| `token` | string | no | Secret token to validate received payloads; this will not be returned in the response | ### Edit project hook @@ -1165,7 +1165,7 @@ Parameters: | `pipeline_events` | boolean | no | Trigger hook on pipeline events | | `wiki_events` | boolean | no | Trigger hook on wiki events | | `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook | -| `token` | string | no | Secret token to validate received payloads | +| `token` | string | no | Secret token to validate received payloads; this will not be returned in the response | ### Delete project hook diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 94586040fa4..ab9d2d54f4b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -50,7 +50,7 @@ module API expose :project_id, :push_events expose :issues_events, :merge_requests_events, :tag_push_events expose :note_events, :build_events, :pipeline_events, :wiki_page_events - expose :enable_ssl_verification, :token + expose :enable_ssl_verification end class BasicProjectDetails < Grape::Entity diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 53113c62996..5f39329a1b8 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -36,7 +36,6 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response.first['pipeline_events']).to eq(true) expect(json_response.first['wiki_page_events']).to eq(true) expect(json_response.first['enable_ssl_verification']).to eq(true) - expect(json_response.first['token']).to eq('S3cr3t') end end @@ -63,7 +62,6 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) - expect(json_response['token']).to eq(hook.token) end it "returns a 404 error if hook id is not available" do @@ -90,6 +88,7 @@ describe API::API, 'ProjectHooks', api: true do expect do post api("/projects/#{project.id}/hooks", user), url: "http://example.com", issues_events: true end.to change {project.hooks.count}.by(1) + expect(response).to have_http_status(201) expect(json_response['url']).to eq('http://example.com') expect(json_response['issues_events']).to eq(true) @@ -101,7 +100,24 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['pipeline_events']).to eq(false) expect(json_response['wiki_page_events']).to eq(false) expect(json_response['enable_ssl_verification']).to eq(true) - expect(json_response['token']).to eq('S3cr3t') + expect(json_response).not_to include('token') + end + + it "adds the token without including it in the response" do + token = "secret token" + + expect do + post api("/projects/#{project.id}/hooks", user), url: "http://example.com", token: token + end.to change {project.hooks.count}.by(1) + + expect(response).to have_http_status(201) + expect(json_response["url"]).to eq("http://example.com") + expect(json_response).not_to include("token") + + hook = project.hooks.find(json_response["id"]) + + expect(hook.url).to eq("http://example.com") + expect(hook.token).to eq(token) end it "returns a 400 error if url not given" do @@ -130,7 +146,19 @@ describe API::API, 'ProjectHooks', api: true do expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) - expect(json_response['token']).to eq(hook.token) + end + + it "adds the token without including it in the response" do + token = "secret token" + + put api("/projects/#{project.id}/hooks/#{hook.id}", user), url: "http://example.org", token: token + + expect(response).to have_http_status(200) + expect(json_response["url"]).to eq("http://example.org") + expect(json_response).not_to include("token") + + expect(hook.reload.url).to eq("http://example.org") + expect(hook.reload.token).to eq(token) end it "returns 404 error if hook id not found" do