From 3b688389f60edfac524ab5314fd9039d36468617 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Sat, 5 Jul 2025 05:05:17 +0300 Subject: [PATCH] Split update node tests --- test/controllers/api/nodes_controller_test.rb | 281 ++++++++++++------ 1 file changed, 188 insertions(+), 93 deletions(-) diff --git a/test/controllers/api/nodes_controller_test.rb b/test/controllers/api/nodes_controller_test.rb index 0af116266..a3c544e77 100644 --- a/test/controllers/api/nodes_controller_test.rb +++ b/test/controllers/api/nodes_controller_test.rb @@ -463,129 +463,224 @@ module Api end end - ## - # tests whether the API works and prevents incorrect use while trying - # to update nodes. - def test_update - invalid_attr_values = [["lat", 91.0], ["lat", -91.0], ["lon", 181.0], ["lon", -181.0]] + def test_update_when_unauthorized + with_unchanging(:node) do |node| + osm_xml = xml_for_node node - ## First test with no user credentials - # try and update a node without authorisation - # first try to delete node without auth - private_user = create(:user, :data_public => false) - private_node = create(:node, :changeset => create(:changeset, :user => private_user)) - user = create(:user) - node = create(:node, :changeset => create(:changeset, :user => user)) + put api_node_path(node), :params => osm_xml.to_s - xml = xml_for_node(node) - put api_node_path(node), :params => xml.to_s - assert_response :unauthorized + assert_response :unauthorized + end + end - ## Second test with the private user + def test_update_in_changeset_of_other_user_by_private_user + with_unchanging(:node) do |node| + other_user = create(:user) - # setup auth - auth_header = bearer_authorization_header private_user + with_unchanging_request([:data_public => false], [:user => other_user]) do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, changeset.id - ## trying to break changesets + put api_node_path(node), :params => osm_xml.to_s, :headers => headers - # try and update in someone else's changeset - xml = update_changeset(xml_for_node(private_node), - create(:changeset).id) - put api_node_path(private_node), :params => xml.to_s, :headers => auth_header - assert_require_public_data "update with other user's changeset should be forbidden when data isn't public" + assert_require_public_data "update with other user's changeset should be forbidden when data isn't public" + end + end + end - # try and update in a closed changeset - xml = update_changeset(xml_for_node(private_node), - create(:changeset, :closed, :user => private_user).id) - put api_node_path(private_node), :params => xml.to_s, :headers => auth_header - assert_require_public_data "update with closed changeset should be forbidden, when data isn't public" + def test_update_in_closed_changeset_by_private_user + with_unchanging(:node) do |node| + with_unchanging_request([:data_public => false], [:closed]) do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, changeset.id - # try and update in a non-existant changeset - xml = update_changeset(xml_for_node(private_node), 0) - put api_node_path(private_node), :params => xml.to_s, :headers => auth_header - assert_require_public_data "update with changeset=0 should be forbidden, when data isn't public" + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_require_public_data "update with closed changeset should be forbidden, when data isn't public" + end + end + end + + def test_update_in_missing_changeset_by_private_user + with_unchanging(:node) do |node| + with_unchanging_request([:data_public => false]) do |headers| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, 0 + + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_require_public_data "update with changeset=0 should be forbidden, when data isn't public" + end + end + end + + def test_update_with_invalid_attr_values_by_private_user + node = create(:node) + user = create(:user, :data_public => false) + changeset = create(:changeset, :user => user) + invalid_attr_values = [["lat", 91.0], ["lat", -91.0], ["lon", 181.0], ["lon", -181.0]] - ## try and submit invalid updates invalid_attr_values.each do |name, value| - xml = xml_attr_rewrite(xml_for_node(private_node), name, value) - put api_node_path(private_node), :params => xml.to_s, :headers => auth_header + xml = xml_for_node node + xml = xml_attr_rewrite xml, name, value + xml = update_changeset xml, changeset.id + + put api_node_path(node), :params => xml.to_s, :headers => bearer_authorization_header(user) + assert_require_public_data "node at #{name}=#{value} should be forbidden, when data isn't public" end + end - ## finally, produce a good request which still won't work - xml = xml_for_node(private_node) - put api_node_path(private_node), :params => xml.to_s, :headers => auth_header - assert_require_public_data "should have failed with a forbidden when data isn't public" + def test_update_by_private_user + with_unchanging(:node) do |node| + with_unchanging_request([:data_public => false]) do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, changeset.id - ## Finally test with the public user + put api_node_path(node), :params => osm_xml.to_s, :headers => headers - # setup auth - auth_header = bearer_authorization_header user + assert_require_public_data "should have failed with a forbidden when data isn't public" + end + end + end - ## trying to break changesets + def test_update_in_changeset_of_other_user + with_unchanging(:node) do |node| + other_user = create(:user) - # try and update in someone else's changeset - xml = update_changeset(xml_for_node(node), - create(:changeset).id) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :conflict, "update with other user's changeset should be rejected" + with_unchanging_request([], [:user => other_user]) do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, changeset.id - # try and update in a closed changeset - xml = update_changeset(xml_for_node(node), - create(:changeset, :closed, :user => user).id) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :conflict, "update with closed changeset should be rejected" + put api_node_path(node), :params => osm_xml.to_s, :headers => headers - # try and update in a non-existant changeset - xml = update_changeset(xml_for_node(node), 0) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :conflict, "update with changeset=0 should be rejected" + assert_response :conflict, "update with other user's changeset should be rejected" + end + end + end + + def test_update_in_closed_changeset + with_unchanging(:node) do |node| + with_unchanging_request([], [:closed]) do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, changeset.id + + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_response :conflict, "update with closed changeset should be rejected" + end + end + end + + def test_update_in_missing_changeset + with_unchanging(:node) do |node| + with_unchanging_request do |headers| + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, 0 + + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_response :conflict, "update with changeset=0 should be rejected" + end + end + end + + def test_update_with_invalid_attr_values + user = create(:user) + node = create(:node) + changeset = create(:changeset, :user => user) + invalid_attr_values = [["lat", 91.0], ["lat", -91.0], ["lon", 181.0], ["lon", -181.0]] - ## try and submit invalid updates invalid_attr_values.each do |name, value| - xml = xml_attr_rewrite(xml_for_node(node), name, value) - put api_node_path(node), :params => xml.to_s, :headers => auth_header + xml = xml_for_node node + xml = xml_attr_rewrite xml, name, value + xml = update_changeset xml, changeset.id + + put api_node_path(node), :params => xml.to_s, :headers => bearer_authorization_header(user) + assert_response :bad_request, "node at #{name}=#{value} should be rejected" end + end + + def test_update_with_version_behind + with_unchanging(:node, :version => 2) do |node| + with_unchanging_request do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = xml_attr_rewrite osm_xml, "version", node.version - 1 + osm_xml = update_changeset osm_xml, changeset.id - ## next, attack the versioning - current_node_version = node.version + put api_node_path(node), :params => osm_xml.to_s, :headers => headers - # try and submit a version behind - xml = xml_attr_rewrite(xml_for_node(node), - "version", current_node_version - 1) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :conflict, "should have failed on old version number" + assert_response :conflict, "should have failed on old version number" + end + end + end - # try and submit a version ahead - xml = xml_attr_rewrite(xml_for_node(node), - "version", current_node_version + 1) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :conflict, "should have failed on skipped version number" + def test_update_with_version_ahead + with_unchanging(:node, :version => 2) do |node| + with_unchanging_request do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = xml_attr_rewrite osm_xml, "version", node.version + 1 + osm_xml = update_changeset osm_xml, changeset.id - # try and submit total crap in the version field - xml = xml_attr_rewrite(xml_for_node(node), - "version", "p1r4t3s!") - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :conflict, - "should not be able to put 'p1r4at3s!' in the version field" + put api_node_path(node), :params => osm_xml.to_s, :headers => headers - ## try an update with the wrong ID - xml = xml_for_node(create(:node)) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :bad_request, - "should not be able to update a node with a different ID from the XML" + assert_response :conflict, "should have failed on skipped version number" + end + end + end - ## try an update with a minimal valid XML doc which isn't a well-formed OSM doc. - xml = "" - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :bad_request, - "should not be able to update a node with non-OSM XML doc." + def test_update_with_invalid_version + with_unchanging(:node) do |node| + with_unchanging_request do |headers, changeset| + osm_xml = xml_for_node node + osm_xml = xml_attr_rewrite osm_xml, "version", "p1r4t3s!" + osm_xml = update_changeset osm_xml, changeset.id + + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_response :conflict, "should not be able to put 'p1r4at3s!' in the version field" + end + end + end + + def test_update_other_node + with_unchanging(:node) do |node| + with_unchanging(:node) do |other_node| + with_unchanging_request do |headers, changeset| + osm_xml = xml_for_node other_node + osm_xml = update_changeset osm_xml, changeset.id + + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_response :bad_request, "should not be able to update a node with a different ID from the XML" + end + end + end + end + + def test_update_with_invalid_osm_structure + with_unchanging(:node) do |node| + with_unchanging_request do |headers| + osm = "" + + put api_node_path(node), :params => osm, :headers => headers + + assert_response :bad_request, "should not be able to update a node with non-OSM XML doc." + end + end + end - ## finally, produce a good request which should work - xml = xml_for_node(node) - put api_node_path(node), :params => xml.to_s, :headers => auth_header - assert_response :success, "a valid update request failed" + def test_update + with_request do |headers, changeset| + node = create(:node) + osm_xml = xml_for_node node + osm_xml = update_changeset osm_xml, changeset.id + + put api_node_path(node), :params => osm_xml.to_s, :headers => headers + + assert_response :success, "a valid update request failed" + end end ## -- 2.39.5