Skip to content

Commit

Permalink
[api] do not allow to accept requests which are not in new state (or …
Browse files Browse the repository at this point in the history
…review with force parameter). Regression fix

Conflicts:
	src/api/app/controllers/request_controller.rb
  • Loading branch information
adrianschroeter committed Jun 13, 2013
1 parent c861ca1 commit bcf1be4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
1 change: 1 addition & 0 deletions ReleaseNotes-2.4.3
Expand Up @@ -27,6 +27,7 @@ Bugfixes:
* api: fix storage of Download on Demand path in API
(Note: this is still an experimental feature likely to change)
* api: crash fixed when request status includes repository pathes to remote instances
* api: allow request state change to approved only from new/review states (regression fix)
* backend: fix scanprjbinaries event handling
* backend: fixed caching for remote source handling in some cases
* backend: undelete of a project does restore the signkey now
Expand Down
12 changes: 9 additions & 3 deletions src/api/app/controllers/request_controller.rb
Expand Up @@ -922,12 +922,18 @@ def command_changestate
end

# Do not accept to skip the review, except force argument is given
if params[:newstate] == "accepted"
if params[:cmd] == "changestate" and req.state == :review and not params[:force]
if params[:cmd] == "changestate" and params[:newstate] == "accepted"
if req.state == :review
unless params[:force]
render_error :status => 403, :errorcode => "post_request_no_permission",
:message => "Request is in review state. You may use the force parameter to ignore this."
return
end
end
elsif req.state != :new
render_error :status => 403, :errorcode => "post_request_no_permission",
:message => "Request is not in new state. You may reopen it by setting it to new."
return
end
end

# valid users and groups ?
Expand Down
12 changes: 11 additions & 1 deletion src/api/test/functional/request_controller_test.rb
Expand Up @@ -1890,8 +1890,18 @@ def test_branch_version_update_and_submit_request_back
assert node.has_attribute?(:id)
id = node.value(:id)

# accept the request
# decline it and try to accept it
# must not work to avoid races between multiple users
prepare_request_with_user "king", "sunflower"
post "/request/#{id}?cmd=changestate&newstate=declined"
assert_response :success
post "/request/#{id}?cmd=changestate&newstate=accepted"
assert_response 403
assert_xml_tag(:tag => "status", :attributes => {:code => 'post_request_no_permission'})
assert_xml_tag(:tag => "summary", :content => "Request is not in new state. You may reopen it by setting it to new.")
# reopen and accept the request
post "/request/#{id}?cmd=changestate&newstate=new"
assert_response :success
post "/request/#{id}?cmd=changestate&newstate=accepted"
assert_response :success

Expand Down

0 comments on commit bcf1be4

Please sign in to comment.