Commit 29ceb98b authored by Douwe Maan's avatar Douwe Maan Committed by Robert Speicher

Merge branch 'issue_25064' into 'security'

Ensure state param has a valid value when filtering issuables.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/25064

This fix makes sure we only call safe methods on issuable when filtering by state.

See merge request !2038
parent f0f514ac
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
# current_user - which user use # current_user - which user use
# params: # params:
# scope: 'created-by-me' or 'assigned-to-me' or 'all' # scope: 'created-by-me' or 'assigned-to-me' or 'all'
# state: 'open' or 'closed' or 'all' # state: 'opened' or 'closed' or 'all'
# group_id: integer # group_id: integer
# project_id: integer # project_id: integer
# milestone_title: string # milestone_title: string
...@@ -207,10 +207,13 @@ class IssuableFinder ...@@ -207,10 +207,13 @@ class IssuableFinder
end end
def by_state(items) def by_state(items)
params[:state] ||= 'all' case params[:state].to_s
when 'closed'
if items.respond_to?(params[:state]) items.closed
items.public_send(params[:state]) when 'merged'
items.respond_to?(:merged) ? items.merged : items.closed
when 'opened'
items.opened
else else
items items
end end
......
---
title: Validate state param when filtering issuables
merge_request:
author:
...@@ -10,6 +10,7 @@ describe IssuesFinder do ...@@ -10,6 +10,7 @@ describe IssuesFinder do
let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') } let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') }
let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') } let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') }
let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) } let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) }
let(:closed_issue) { create(:issue, author: user2, assignee: user2, project: project2, state: 'closed') }
let!(:label_link) { create(:label_link, label: label, target: issue2) } let!(:label_link) { create(:label_link, label: label, target: issue2) }
before do before do
...@@ -25,7 +26,7 @@ describe IssuesFinder do ...@@ -25,7 +26,7 @@ describe IssuesFinder do
describe '#execute' do describe '#execute' do
let(:search_user) { user } let(:search_user) { user }
let(:params) { {} } let(:params) { {} }
let(:issues) { IssuesFinder.new(search_user, params.merge(scope: scope, state: 'opened')).execute } let(:issues) { IssuesFinder.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute }
context 'scope: all' do context 'scope: all' do
let(:scope) { 'all' } let(:scope) { 'all' }
...@@ -143,6 +144,40 @@ describe IssuesFinder do ...@@ -143,6 +144,40 @@ describe IssuesFinder do
end end
end end
context 'filtering by state' do
context 'with opened' do
let(:params) { { state: 'opened' } }
it 'returns only opened issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3)
end
end
context 'with closed' do
let(:params) { { state: 'closed' } }
it 'returns only closed issues' do
expect(issues).to contain_exactly(closed_issue)
end
end
context 'with all' do
let(:params) { { state: 'all' } }
it 'returns all issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue)
end
end
context 'with invalid state' do
let(:params) { { state: 'invalid_state' } }
it 'returns all issues' do
expect(issues).to contain_exactly(issue1, issue2, issue3, closed_issue)
end
end
end
context 'when the user is unauthorized' do context 'when the user is unauthorized' do
let(:search_user) { nil } let(:search_user) { nil }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment