From 8bfb5cae8ad5aa2973349b128baa06d8b5ee5e7b Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Wed, 28 Mar 2012 18:45:16 +0100 Subject: [PATCH] Added relation redaction logic, same as node and way logic, plus tests --- app/controllers/old_relation_controller.rb | 55 +++++- app/models/old_relation.rb | 5 +- config/routes.rb | 1 + test/fixtures/current_relations.yml | 14 ++ test/fixtures/relations.yml | 58 +++++++ test/fixtures/ways.yml | 7 - .../old_relation_controller_test.rb | 162 ++++++++++++++++++ test/unit/relation_test.rb | 2 +- 8 files changed, 289 insertions(+), 15 deletions(-) diff --git a/app/controllers/old_relation_controller.rb b/app/controllers/old_relation_controller.rb index 19b4e5d34..85cac89e7 100644 --- a/app/controllers/old_relation_controller.rb +++ b/app/controllers/old_relation_controller.rb @@ -2,15 +2,28 @@ class OldRelationController < ApplicationController require 'xml/libxml' skip_before_filter :verify_authenticity_token + before_filter :setup_user_auth, :only => [ :history, :version ] + before_filter :authorize, :only => [ :redact ] + before_filter :authorize_moderator, :only => [ :redact ] + before_filter :require_allow_write_api, :only => [ :redact ] before_filter :check_api_readable + before_filter :check_api_writable, :only => [ :redact ] + before_filter :lookup_old_relation, :except => [ :history ] after_filter :compress_output around_filter :api_call_handle_error, :api_call_timeout def history - relation = Relation.find(params[:id]) + relation = Relation.find(params[:id].to_i) + doc = OSM::API.new.get_xml_doc - relation.old_relations.each do |old_relation| + visible_relations = if @user and @user.moderator? and params[:show_redactions] == "true" + relation.old_relations + else + relation.old_relations.unredacted + end + + visible_relations.each do |old_relation| doc.root << old_relation.to_xml_node end @@ -18,15 +31,45 @@ class OldRelationController < ApplicationController end def version - if old_relation = OldRelation.where(:relation_id => params[:id], :version => params[:version]).first - response.last_modified = old_relation.timestamp + if @old_relation.redacted? and not (@user and @user.moderator? and params[:show_redactions] == "true") + render :nothing => true, :status => :forbidden + else + response.last_modified = @old_relation.timestamp + doc = OSM::API.new.get_xml_doc - doc.root << old_relation.to_xml_node - + doc.root << @old_relation.to_xml_node + render :text => doc.to_s, :content_type => "text/xml" + end + end + + def redact + redaction_id = params['redaction'] + unless redaction_id.nil? + # if a redaction ID was specified, then set this relation to + # be redacted in that redaction. (TODO: check that the + # user doing the redaction owns the redaction object too) + redaction = Redaction.find(redaction_id.to_i) + @old_relation.redact!(redaction) + else + # if no redaction ID was provided, then this is an unredact + # operation. + @old_relation.redact!(nil) + end + + # just return an empty 200 OK for success + render :nothing => true + end + + private + + def lookup_old_relation + @old_relation = OldRelation.where(:relation_id => params[:id], :version => params[:version]).first + if @old_relation.nil? render :nothing => true, :status => :not_found + return false end end end diff --git a/app/models/old_relation.rb b/app/models/old_relation.rb index e4be8fc7f..20651f622 100644 --- a/app/models/old_relation.rb +++ b/app/models/old_relation.rb @@ -1,10 +1,13 @@ class OldRelation < ActiveRecord::Base include ConsistencyValidations - include Redactable self.table_name = "relations" self.primary_keys = "relation_id", "version" + # note this needs to be included after the table name changes, or + # the queries generated by Redactable will use the wrong table name. + include Redactable + belongs_to :changeset belongs_to :redaction belongs_to :current_relation, :class_name => "Relation", :foreign_key => "relation_id" diff --git a/config/routes.rb b/config/routes.rb index 3cfba03b9..73540ca7f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,6 +38,7 @@ OpenStreetMap::Application.routes.draw do match 'api/0.6/relation/:id/relations' => 'relation#relations_for_relation', :via => :get, :id => /\d+/ match 'api/0.6/relation/:id/history' => 'old_relation#history', :via => :get, :id => /\d+/ match 'api/0.6/relation/:id/full' => 'relation#full', :via => :get, :id => /\d+/ + match 'api/0.6/relation/:id/:version/redact' => 'old_relation#redact', :via => :post, :version => /\d+/, :id => /\d+/ match 'api/0.6/relation/:id/:version' => 'old_relation#version', :via => :get, :id => /\d+/, :version => /\d+/ match 'api/0.6/relation/:id' => 'relation#read', :via => :get, :id => /\d+/ match 'api/0.6/relation/:id' => 'relation#update', :via => :put, :id => /\d+/ diff --git a/test/fixtures/current_relations.yml b/test/fixtures/current_relations.yml index 1f4fb46fe..fd95f60ce 100644 --- a/test/fixtures/current_relations.yml +++ b/test/fixtures/current_relations.yml @@ -39,3 +39,17 @@ public_visible_relation: timestamp: 2009-04-22 00:34:12 visible: true version: 1 + +relation_with_redacted_versions: + id: 7 + changeset_id: 4 + timestamp: 2008-01-01 00:04:13 + visible: true + version: 4 + +relation_with_versions: + id: 8 + changeset_id: 4 + timestamp: 2008-01-01 00:04:00 + visible: true + version: 4 diff --git a/test/fixtures/relations.yml b/test/fixtures/relations.yml index 059528f62..0640bf2f3 100644 --- a/test/fixtures/relations.yml +++ b/test/fixtures/relations.yml @@ -39,3 +39,61 @@ public_visible_relation: timestamp: 2009-04-22 00:34:12 visible: true version: 1 + +relation_with_redacted_versions_v1: + relation_id: 7 + changeset_id: 4 + timestamp: 2008-01-01 00:04:10 + visible: true + version: 1 + +relation_with_redacted_versions_v2: + relation_id: 7 + changeset_id: 4 + timestamp: 2008-01-01 00:04:11 + visible: true + version: 2 + redaction_id: 1 + +relation_with_redacted_versions_v3: + relation_id: 7 + changeset_id: 4 + timestamp: 2008-01-01 00:04:12 + visible: true + version: 3 + redaction_id: 1 + +relation_with_redacted_versions_v4: + relation_id: 7 + changeset_id: 4 + timestamp: 2008-01-01 00:04:13 + visible: true + version: 4 + +relation_with_versions_v1: + relation_id: 8 + changeset_id: 4 + timestamp: 2008-01-01 00:01:00 + visible: true + version: 1 + +relation_with_versions_v2: + relation_id: 8 + changeset_id: 4 + timestamp: 2008-01-01 00:02:00 + visible: true + version: 2 + +relation_with_versions_v3: + relation_id: 8 + changeset_id: 4 + timestamp: 2008-01-01 00:03:00 + visible: true + version: 3 + +relation_with_versions_v4: + relation_id: 8 + changeset_id: 4 + timestamp: 2008-01-01 00:04:00 + visible: true + version: 4 diff --git a/test/fixtures/ways.yml b/test/fixtures/ways.yml index 9a79b977b..dfb48b450 100644 --- a/test/fixtures/ways.yml +++ b/test/fixtures/ways.yml @@ -61,13 +61,6 @@ way_with_redacted_versions_v1: visible: true version: 1 -way_with_redacted_versions_v1: - way_id: 6 - changeset_id: 4 - timestamp: 2008-01-01 00:04:10 - visible: true - version: 1 - way_with_redacted_versions_v2: way_id: 6 changeset_id: 4 diff --git a/test/functional/old_relation_controller_test.rb b/test/functional/old_relation_controller_test.rb index 89676a175..fa20e2fc9 100644 --- a/test/functional/old_relation_controller_test.rb +++ b/test/functional/old_relation_controller_test.rb @@ -29,4 +29,166 @@ class OldRelationControllerTest < ActionController::TestCase get :history, :id => 0 assert_response :not_found end + + ## + # test the redaction of an old version of a relation, while not being + # authorised. + def test_redact_relation_unauthorised + do_redact_relation(relations(:relation_with_versions_v3), + redactions(:example)) + assert_response :unauthorized, "should need to be authenticated to redact." + end + + ## + # test the redaction of an old version of a relation, while being + # authorised as a normal user. + def test_redact_relation_normal_user + basic_authorization(users(:public_user).email, "test") + + do_redact_relation(relations(:relation_with_versions_v3), + redactions(:example)) + assert_response :forbidden, "should need to be moderator to redact." + end + + ## + # test that, even as moderator, the current version of a relation + # can't be redacted. + def test_redact_relation_current_version + basic_authorization(users(:moderator_user).email, "test") + + do_redact_relation(relations(:relation_with_versions_v4), + redactions(:example)) + assert_response :bad_request, "shouldn't be OK to redact current version as moderator." + end + + ## + # test that redacted relations aren't visible, regardless of + # authorisation except as moderator... + def test_version_redacted + relation = relations(:relation_with_redacted_versions_v2) + + get :version, :id => relation.relation_id, :version => relation.version + assert_response :forbidden, "Redacted node shouldn't be visible via the version API." + + # not even to a logged-in user + basic_authorization(users(:public_user).email, "test") + get :version, :id => relation.relation_id, :version => relation.version + assert_response :forbidden, "Redacted node shouldn't be visible via the version API, even when logged in." + end + + ## + # test that redacted nodes aren't visible in the history + def test_history_redacted + relation = relations(:relation_with_redacted_versions_v2) + + get :history, :id => relation.relation_id + assert_response :success, "Redaction shouldn't have stopped history working." + assert_select "osm relation[id=#{relation.relation_id}][version=#{relation.version}]", 0, "redacted relation #{relation.relation_id} version #{relation.version} shouldn't be present in the history." + + # not even to a logged-in user + basic_authorization(users(:public_user).email, "test") + get :version, :id => relation.relation_id, :version => relation.version + get :history, :id => relation.relation_id + assert_response :success, "Redaction shouldn't have stopped history working." + assert_select "osm relation[id=#{relation.relation_id}][version=#{relation.version}]", 0, "redacted node #{relation.relation_id} version #{relation.version} shouldn't be present in the history, even when logged in." + end + + ## + # test the redaction of an old version of a relation, while being + # authorised as a moderator. + def test_redact_relation_moderator + relation = relations(:relation_with_versions_v3) + basic_authorization(users(:moderator_user).email, "test") + + do_redact_relation(relation, redactions(:example)) + assert_response :success, "should be OK to redact old version as moderator." + + # check moderator can still see the redacted data, when passing + # the appropriate flag + get :version, :id => relation.relation_id, :version => relation.version + assert_response :forbidden, "After redaction, node should be gone for moderator, when flag not passed." + get :version, :id => relation.relation_id, :version => relation.version, :show_redactions => 'true' + assert_response :success, "After redaction, node should not be gone for moderator, when flag passed." + + # and when accessed via history + get :history, :id => relation.relation_id + assert_response :success, "Redaction shouldn't have stopped history working." + assert_select "osm relation[id=#{relation.relation_id}][version=#{relation.version}]", 0, "relation #{relation.relation_id} version #{relation.version} should not be present in the history for moderators when not passing flag." + get :history, :id => relation.relation_id, :show_redactions => 'true' + assert_response :success, "Redaction shouldn't have stopped history working." + assert_select "osm relation[id=#{relation.relation_id}][version=#{relation.version}]", 1, "relation #{relation.relation_id} version #{relation.version} should still be present in the history for moderators when passing flag." + end + + # testing that if the moderator drops auth, he can't see the + # redacted stuff any more. + def test_redact_relation_is_redacted + relation = relations(:relation_with_versions_v3) + basic_authorization(users(:moderator_user).email, "test") + + do_redact_relation(relation, redactions(:example)) + assert_response :success, "should be OK to redact old version as moderator." + + # re-auth as non-moderator + basic_authorization(users(:public_user).email, "test") + + # check can't see the redacted data + get :version, :id => relation.relation_id, :version => relation.version + assert_response :forbidden, "Redacted node shouldn't be visible via the version API." + + # and when accessed via history + get :history, :id => relation.relation_id + assert_response :success, "Redaction shouldn't have stopped history working." + assert_select "osm relation[id=#{relation.relation_id}][version=#{relation.version}]", 0, "redacted relation #{relation.relation_id} version #{relation.version} shouldn't be present in the history." + end + + ## + # check that the current version of a relation is equivalent to the + # version which we're getting from the versions call. + def check_current_version(relation_id) + # get the current version + current_relation = with_controller(RelationController.new) do + get :read, :id => relation_id + assert_response :success, "can't get current relation #{relation_id}" + Relation.from_xml(@response.body) + end + assert_not_nil current_relation, "getting relation #{relation_id} returned nil" + + # get the "old" version of the relation from the version method + get :version, :id => relation_id, :version => current_relation.version + assert_response :success, "can't get old relation #{relation_id}, v#{current_relation.version}" + old_relation = Relation.from_xml(@response.body) + + # check that the relations are identical + assert_relations_are_equal current_relation, old_relation + end + + ## + # look at all the versions of the relation in the history and get each version from + # the versions call. check that they're the same. + def check_history_equals_versions(relation_id) + get :history, :id => relation_id + assert_response :success, "can't get relation #{relation_id} from API" + history_doc = XML::Parser.string(@response.body).parse + assert_not_nil history_doc, "parsing relation #{relation_id} history failed" + + history_doc.find("//osm/relation").each do |relation_doc| + history_relation = Relation.from_xml_node(relation_doc) + assert_not_nil history_relation, "parsing relation #{relation_id} version failed" + + get :version, :id => relation_id, :version => history_relation.version + assert_response :success, "couldn't get relation #{relation_id}, v#{history_relation.version}" + version_relation = Relation.from_xml(@response.body) + assert_not_nil version_relation, "failed to parse #{relation_id}, v#{history_relation.version}" + + assert_relations_are_equal history_relation, version_relation + end + end + + def do_redact_relation(relation, redaction) + get :version, :id => relation.relation_id, :version => relation.version + assert_response :success, "should be able to get version #{relation.version} of node #{relation.relation_id}." + + # now redact it + post :redact, :id => relation.relation_id, :version => relation.version, :redaction => redaction.id + end end diff --git a/test/unit/relation_test.rb b/test/unit/relation_test.rb index b1fbc0fcd..a62320cc7 100644 --- a/test/unit/relation_test.rb +++ b/test/unit/relation_test.rb @@ -4,7 +4,7 @@ class RelationTest < ActiveSupport::TestCase api_fixtures def test_relation_count - assert_equal 6, Relation.count + assert_equal 8, Relation.count end def test_from_xml_no_id -- 2.43.2