From e56e3cdc62f96541b9bd8b7814204e92f1909253 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 9 May 2016 19:35:37 -0300 Subject: Fix api leaking notes when user is not authorized to read noteable --- spec/requests/api/notes_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'spec/requests/api/notes_spec.rb') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 49091fc0f49..f5b31be1ba3 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -57,6 +57,15 @@ describe API::API, api: true do expect(json_response).to be_empty end + context "and issue is confidential" do + before { ext_issue.update_attributes(confidential: true) } + + it "returns 404" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(404) + end + end + context "and current user can view the note" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user) @@ -80,6 +89,11 @@ describe API::API, api: true do get api("/projects/#{project.id}/snippets/42/notes", user) expect(response.status).to eq(404) end + + it "returns 404 when not authorized" do + get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user) + expect(response.status).to eq(404) + end end context "when noteable is a Merge Request" do @@ -94,6 +108,11 @@ describe API::API, api: true do get api("/projects/#{project.id}/merge_requests/4444/notes", user) expect(response.status).to eq(404) end + + it "returns 404 when not authorized" do + get api("/projects/#{project.id}/merge_requests/4444/notes", private_user) + expect(response.status).to eq(404) + end end end -- cgit v1.2.1 From 93ca5c9964a26fbf31fcc794348b30193f4dff9f Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Tue, 10 May 2016 16:06:02 -0300 Subject: Fix notes API calls symbol convertions --- spec/requests/api/notes_spec.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'spec/requests/api/notes_spec.rb') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index f5b31be1ba3..f9bfee94424 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -39,6 +39,7 @@ describe API::API, api: true do context "when noteable is an Issue" do it "should return an array of issue notes" do get api("/projects/#{project.id}/issues/#{issue.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(issue_note.note) @@ -46,12 +47,14 @@ describe API::API, api: true do it "should return a 404 error when issue id not found" do get api("/projects/#{project.id}/issues/12345/notes", user) + expect(response.status).to eq(404) end context "that references a private issue" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response).to be_empty @@ -62,6 +65,7 @@ describe API::API, api: true do it "returns 404" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) + expect(response.status).to eq(404) end end @@ -69,6 +73,7 @@ describe API::API, api: true do context "and current user can view the note" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", private_user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(cross_reference_note.note) @@ -80,6 +85,7 @@ describe API::API, api: true do context "when noteable is a Snippet" do it "should return an array of snippet notes" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(snippet_note.note) @@ -87,11 +93,13 @@ describe API::API, api: true do it "should return a 404 error when snippet id not found" do get api("/projects/#{project.id}/snippets/42/notes", user) + expect(response.status).to eq(404) end it "returns 404 when not authorized" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user) + expect(response.status).to eq(404) end end @@ -99,6 +107,7 @@ describe API::API, api: true do context "when noteable is a Merge Request" do it "should return an array of merge_requests notes" do get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes", user) + expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.first['body']).to eq(merge_request_note.note) @@ -106,11 +115,13 @@ describe API::API, api: true do it "should return a 404 error if merge request id not found" do get api("/projects/#{project.id}/merge_requests/4444/notes", user) + expect(response.status).to eq(404) end it "returns 404 when not authorized" do get api("/projects/#{project.id}/merge_requests/4444/notes", private_user) + expect(response.status).to eq(404) end end @@ -120,24 +131,28 @@ describe API::API, api: true do context "when noteable is an Issue" do it "should return an issue note by id" do get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{issue_note.id}", user) + expect(response.status).to eq(200) expect(json_response['body']).to eq(issue_note.note) end it "should return a 404 error if issue note not found" do get api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user) + expect(response.status).to eq(404) end context "that references a private issue" do it "should return a 404 error" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user) + expect(response.status).to eq(404) end context "and current user can view the note" do it "should return an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user) + expect(response.status).to eq(200) expect(json_response['body']).to eq(cross_reference_note.note) end @@ -148,12 +163,14 @@ describe API::API, api: true do context "when noteable is a Snippet" do it "should return a snippet note by id" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) + expect(response.status).to eq(200) expect(json_response['body']).to eq(snippet_note.note) end it "should return a 404 error if snippet note not found" do get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/12345", user) + expect(response.status).to eq(404) end end @@ -163,6 +180,7 @@ describe API::API, api: true do context "when noteable is an Issue" do it "should create a new issue note" do post api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!' + expect(response.status).to eq(201) expect(json_response['body']).to eq('hi!') expect(json_response['author']['username']).to eq(user.username) @@ -170,11 +188,13 @@ describe API::API, api: true do it "should return a 400 bad request error if body not given" do post api("/projects/#{project.id}/issues/#{issue.id}/notes", user) + expect(response.status).to eq(400) end it "should return a 401 unauthorized error if user not authenticated" do post api("/projects/#{project.id}/issues/#{issue.id}/notes"), body: 'hi!' + expect(response.status).to eq(401) end @@ -183,6 +203,7 @@ describe API::API, api: true do creation_time = 2.weeks.ago post api("/projects/#{project.id}/issues/#{issue.id}/notes", user), body: 'hi!', created_at: creation_time + expect(response.status).to eq(201) expect(json_response['body']).to eq('hi!') expect(json_response['author']['username']).to eq(user.username) @@ -195,6 +216,7 @@ describe API::API, api: true do context "when noteable is a Snippet" do it "should create a new snippet note" do post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user), body: 'hi!' + expect(response.status).to eq(201) expect(json_response['body']).to eq('hi!') expect(json_response['author']['username']).to eq(user.username) @@ -202,11 +224,13 @@ describe API::API, api: true do it "should return a 400 bad request error if body not given" do post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) + expect(response.status).to eq(400) end it "should return a 401 unauthorized error if user not authenticated" do post api("/projects/#{project.id}/snippets/#{snippet.id}/notes"), body: 'hi!' + expect(response.status).to eq(401) end end @@ -246,6 +270,7 @@ describe API::API, api: true do it 'should return modified note' do put api("/projects/#{project.id}/issues/#{issue.id}/"\ "notes/#{issue_note.id}", user), body: 'Hello!' + expect(response.status).to eq(200) expect(json_response['body']).to eq('Hello!') end @@ -253,12 +278,14 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/issues/#{issue.id}/notes/12345", user), body: 'Hello!' + expect(response.status).to eq(404) end it 'should return a 400 bad request error if body not given' do put api("/projects/#{project.id}/issues/#{issue.id}/"\ "notes/#{issue_note.id}", user) + expect(response.status).to eq(400) end end @@ -267,6 +294,7 @@ describe API::API, api: true do it 'should return modified note' do put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ "notes/#{snippet_note.id}", user), body: 'Hello!' + expect(response.status).to eq(200) expect(json_response['body']).to eq('Hello!') end @@ -274,6 +302,7 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ "notes/12345", user), body: "Hello!" + expect(response.status).to eq(404) end end @@ -282,6 +311,7 @@ describe API::API, api: true do it 'should return modified note' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\ "notes/#{merge_request_note.id}", user), body: 'Hello!' + expect(response.status).to eq(200) expect(json_response['body']).to eq('Hello!') end @@ -289,6 +319,7 @@ describe API::API, api: true do it 'should return a 404 error when note id not found' do put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/"\ "notes/12345", user), body: "Hello!" + expect(response.status).to eq(404) end end -- cgit v1.2.1 From c9be74e24797c1dab5b443728349bb0c5ce969c3 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 16 May 2016 16:43:19 -0300 Subject: Fix single note api request --- spec/requests/api/notes_spec.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'spec/requests/api/notes_spec.rb') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index f9bfee94424..ed1ed5aeb95 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers let(:user) { create(:user) } - let!(:project) { create(:project, namespace: user.namespace) } + let!(:project) { create(:project, :public, namespace: user.namespace) } let!(:issue) { create(:issue, project: project, author: user) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } let!(:snippet) { create(:project_snippet, project: project, author: user) } @@ -51,7 +51,7 @@ describe API::API, api: true do expect(response.status).to eq(404) end - context "that references a private issue" do + context "and current user cannot view the notes" do it "should return an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes", user) @@ -142,13 +142,24 @@ describe API::API, api: true do expect(response.status).to eq(404) end - context "that references a private issue" do + context "and current user cannot view the note" do it "should return a 404 error" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", user) expect(response.status).to eq(404) end + context "when issue is confidential" do + before { issue.update_attributes(confidential: true) } + + it "returns 404" do + get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{issue_note.id}", private_user) + + expect(response.status).to eq(404) + end + end + + context "and current user can view the note" do it "should return an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.id}/notes/#{cross_reference_note.id}", private_user) -- cgit v1.2.1 From 1bcb1ef85e63478c8b48524c8a761f9819673f50 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 25 Apr 2016 13:19:57 +0200 Subject: Update specs for creating new note without access --- spec/requests/api/notes_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/requests/api/notes_spec.rb') diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index ed1ed5aeb95..beb29a68692 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -258,8 +258,8 @@ describe API::API, api: true do body: 'Hi!' end - it 'responds with 500' do - expect(response.status).to eq 500 + it 'responds with resource not found error' do + expect(response.status).to eq 404 end it 'does not create new note' do -- cgit v1.2.1