diff --git a/CHANGELOG b/CHANGELOG index c633c4bb35f..ed59bc1b252 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -45,6 +45,7 @@ v 8.7.0 (unreleased) - Fix admin/projects when using visibility levels on search (PotHix) - Build status notifications - API: Expose user location (Robert Schilling) + - API: Do not leak group existence via return code (Robert Schilling) - ClosingIssueExtractor regex now also works with colons. e.g. "Fixes: #1234" !3591 - Update number of Todos in the sidebar when it's marked as "Done". !3600 - API: Expose 'updated_at' for issue, snippet, and merge request notes (Robert Schilling) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 4921ae99e78..96af7d7675c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -91,8 +91,7 @@ module API if can?(current_user, :read_group, group) group else - forbidden!("#{current_user.username} lacks sufficient "\ - "access to #{group.name}") + not_found!('Group') end end diff --git a/spec/requests/api/group_members_spec.rb b/spec/requests/api/group_members_spec.rb index 3e8b4aa1f88..96d89e69209 100644 --- a/spec/requests/api/group_members_spec.rb +++ b/spec/requests/api/group_members_spec.rb @@ -42,9 +42,10 @@ describe API::API, api: true do end end - it "users not part of the group should get access error" do + it 'users not part of the group should get access error' do get api("/groups/#{group_with_members.id}/members", stranger) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end end @@ -165,12 +166,13 @@ describe API::API, api: true do end end - describe "DELETE /groups/:id/members/:user_id" do - context "when not a member of the group" do + describe 'DELETE /groups/:id/members/:user_id' do + context 'when not a member of the group' do it "should not delete guest's membership of group_with_members" do random_user = create(:user) delete api("/groups/#{group_with_members.id}/members/#{owner.id}", random_user) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 7383c7d11aa..37ddab83c30 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -61,7 +61,8 @@ describe API::API, api: true do it "should not return a group not attached to user1" do get api("/groups/#{group2.id}", user1) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end @@ -92,7 +93,8 @@ describe API::API, api: true do it 'should not return a group not attached to user1' do get api("/groups/#{group2.path}", user1) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end end @@ -133,10 +135,10 @@ describe API::API, api: true do end context 'when authenticated as an user that cannot see the group' do - it 'returns 403 when trying to update the group' do + it 'returns 404 when trying to update the group' do put api("/groups/#{group2.id}", user1), name: new_group_name - expect(response.status).to eq(403) + expect(response.status).to eq(404) end end end @@ -157,7 +159,8 @@ describe API::API, api: true do it "should not return a group not attached to user1" do get api("/groups/#{group2.id}/projects", user1) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end @@ -189,7 +192,8 @@ describe API::API, api: true do it 'should not return a group not attached to user1' do get api("/groups/#{group2.path}/projects", user1) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end end @@ -247,7 +251,8 @@ describe API::API, api: true do it "should not remove a group not attached to user1" do delete api("/groups/#{group2.id}", user1) - expect(response.status).to eq(403) + + expect(response.status).to eq(404) end end