Copy the redaction code from nodes to ways

diff --git a/app/controllers/old_way_controller.rb b/app/controllers/old_way_controller.rb
index 3836d4ab7c5167f8675d7c11db003521bb69c2d8..fc7366bb6657e3745c279f8cd6c5c4cd19d5d462 100644 (file)
--- a/app/controllers/old_way_controller.rb
+++ b/app/controllers/old_way_controller.rb
@@ -2,17 +2,25 @@ class OldWayController < ApplicationController
   require 'xml/libxml'
 
   skip_before_filter :verify_authenticity_token
   require 'xml/libxml'
 
   skip_before_filter :verify_authenticity_token

+  before_filter :authorize, :only => [ :redact ]
+  before_filter :require_allow_write_api, :only => [ :redact ]

   before_filter :check_api_readable
   after_filter :compress_output
   around_filter :api_call_handle_error, :api_call_timeout
 
   def history
     way = Way.find(params[:id])
   before_filter :check_api_readable
   after_filter :compress_output
   around_filter :api_call_handle_error, :api_call_timeout
 
   def history
     way = Way.find(params[:id])

+
+    # TODO - maybe a bit heavyweight to do this on every
+    # call, perhaps try lazy auth.
+    setup_user_auth

     
     doc = OSM::API.new.get_xml_doc
     
     way.old_ways.each do |old_way|
     
     doc = OSM::API.new.get_xml_doc
     
     way.old_ways.each do |old_way|

-      doc.root << old_way.to_xml_node
+      unless old_way.redacted? and (@user.nil? or not @user.moderator?) and not params[:show_redactions] == "true"
+        doc.root << old_way.to_xml_node
+      end

     end
     
     render :text => doc.to_s, :content_type => "text/xml"
     end
     
     render :text => doc.to_s, :content_type => "text/xml"


@@ -20,14 +28,31 @@ class OldWayController < ApplicationController
   
   def version
     if old_way = OldWay.where(:way_id => params[:id], :version => params[:version]).first
   
   def version
     if old_way = OldWay.where(:way_id => params[:id], :version => params[:version]).first

-      response.last_modified = old_way.timestamp
-
-      doc = OSM::API.new.get_xml_doc
-      doc.root << old_way.to_xml_node
+      # TODO - maybe a bit heavyweight to do this on every
+      # call, perhaps try lazy auth.
+      setup_user_auth

 
 

-      render :text => doc.to_s, :content_type => "text/xml"
+      if old_way.redacted? and (@user.nil? or not @user.moderator?) and not params[:show_redactions] == "true"
+        render :nothing => true, :status => :forbidden
+      else
+        response.last_modified = old_way.timestamp
+        
+        doc = OSM::API.new.get_xml_doc
+        doc.root << old_way.to_xml_node
+        
+        render :text => doc.to_s, :content_type => "text/xml"
+      end

     else
       render :nothing => true, :status => :not_found
     end
   end
     else
       render :nothing => true, :status => :not_found
     end
   end

+
+  def redact
+    if @user && @user.moderator?
+      render :nothing => true
+
+    else
+      render :nothing => true, :status => :forbidden
+    end
+  end

 end
 end

diff --git a/app/models/old_way.rb b/app/models/old_way.rb
index 3df0c2d4f3b53bc99c0c27e4c276227e59a5d582..474c3e02be862d47890d16c11f46591e2ec9b15a 100644 (file)
--- a/app/models/old_way.rb
+++ b/app/models/old_way.rb
@@ -100,18 +100,26 @@ class OldWay < ActiveRecord::Base
     end
     el1['version'] = self.version.to_s
     el1['changeset'] = self.changeset.id.to_s
     end
     el1['version'] = self.version.to_s
     el1['changeset'] = self.changeset.id.to_s

-    
-    self.old_nodes.each do |nd| # FIXME need to make sure they come back in the right order
-      e = XML::Node.new 'nd'
-      e['ref'] = nd.node_id.to_s
-      el1 << e
+
+    if self.redacted?
+      el1['redacted'] = self.redaction.title

     end
     end

- 
-    self.old_tags.each do |tag|
-      e = XML::Node.new 'tag'
-      e['k'] = tag.k
-      e['v'] = tag.v
-      el1 << e
+    
+    unless self.redacted? and (@user.nil? or not @user.moderator?)
+      # If a way is redacted and the user isn't a moderator, only show
+      # meta-data from this revision, but no real data.
+      self.old_nodes.each do |nd| # FIXME need to make sure they come back in the right order
+        e = XML::Node.new 'nd'
+        e['ref'] = nd.node_id.to_s
+        el1 << e
+      end
+      
+      self.old_tags.each do |tag|
+        e = XML::Node.new 'tag'
+        e['k'] = tag.k
+        e['v'] = tag.v
+        el1 << e
+      end

     end
     return el1
   end
     end
     return el1
   end

diff --git a/config/routes.rb b/config/routes.rb
index 46be25c97e5a842eed477e40925685a6b1a9507b..c0a31b0742c41904268c2ebef8566f1ee305563e 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -28,6 +28,7 @@ OpenStreetMap::Application.routes.draw do
   match 'api/0.6/way/:id/full' => 'way#full', :via => :get, :id => /\d+/
   match 'api/0.6/way/:id/relations' => 'relation#relations_for_way', :via => :get, :id => /\d+/
   match 'api/0.6/way/:id/:version' => 'old_way#version', :via => :get, :id => /\d+/, :version => /\d+/
   match 'api/0.6/way/:id/full' => 'way#full', :via => :get, :id => /\d+/
   match 'api/0.6/way/:id/relations' => 'relation#relations_for_way', :via => :get, :id => /\d+/
   match 'api/0.6/way/:id/:version' => 'old_way#version', :via => :get, :id => /\d+/, :version => /\d+/

+  match 'api/0.6/way/:id/:version/redact' => 'old_way#redact', :via => :put, :version => /\d+/, :id => /\d+/

   match 'api/0.6/way/:id' => 'way#read', :via => :get, :id => /\d+/
   match 'api/0.6/way/:id' => 'way#update', :via => :put, :id => /\d+/
   match 'api/0.6/way/:id' => 'way#delete', :via => :delete, :id => /\d+/
   match 'api/0.6/way/:id' => 'way#read', :via => :get, :id => /\d+/
   match 'api/0.6/way/:id' => 'way#update', :via => :put, :id => /\d+/
   match 'api/0.6/way/:id' => 'way#delete', :via => :delete, :id => /\d+/

diff --git a/test/fixtures/current_ways.yml b/test/fixtures/current_ways.yml
index cd06d06d3a900d1643f510db2541eae779a3ffcf..255be4a97c079775b78829fdac0668286a159e76 100644 (file)
--- a/test/fixtures/current_ways.yml
+++ b/test/fixtures/current_ways.yml
@@ -32,3 +32,10 @@ way_with_duplicate_nodes:
   timestamp: 2007-01-01 00:00:00
   visible: true
   version: 1
   timestamp: 2007-01-01 00:00:00
   visible: true
   version: 1

+
+way_with_redacted_versions:
+  id: 6
+  changeset_id: 4
+  timestamp: 2008-01-01 00:04:13
+  visible: true
+  version: 4

diff --git a/test/fixtures/way_nodes.yml b/test/fixtures/way_nodes.yml
index 19541d19aac7e2bc080fe44f7f0fc3fdc3ef3117..262f29b3179cf3594ab2500808d67c6e6f08c77c 100644 (file)
--- a/test/fixtures/way_nodes.yml
+++ b/test/fixtures/way_nodes.yml
@@ -75,3 +75,61 @@ w5_n2:
   node_id: 4
   sequence_id: 2
   version: 1
   node_id: 4
   sequence_id: 2
   version: 1

+
+w6_v1_n1:
+  way_id: 6
+  node_id: 3
+  sequence_id: 1
+  version: 1
+
+w6_v1_n2:
+  way_id: 6
+  node_id: 2
+  sequence_id: 2
+  version: 1
+
+w6_v2_n1:
+  way_id: 6
+  node_id: 3
+  sequence_id: 1
+  version: 2
+
+w6_v2_n2:
+  way_id: 6
+  node_id: 2
+  sequence_id: 2
+  version: 2
+
+w6_v3_n1:
+  way_id: 6
+  node_id: 3
+  sequence_id: 1
+  version: 3
+
+w6_v3_n2:
+  way_id: 6
+  node_id: 2
+  sequence_id: 2
+  version: 3
+
+w6_v4_n1:
+  way_id: 6
+  node_id: 3
+  sequence_id: 1
+  version: 4
+
+w6_v4_n2:
+  way_id: 6
+  node_id: 2
+  sequence_id: 2
+  version: 4
+
+w6_v4_n3:
+  way_id: 6
+  node_id: 4
+  sequence_id: 3
+  version: 4
+
+
+
+

diff --git a/test/fixtures/way_tags.yml b/test/fixtures/way_tags.yml
index dc727b5f5cd8416c89bf750e6f9753d4ecc3c77b..6fb77d71b547be3d684af65e29ed64aeaa691a0c 100644 (file)
--- a/test/fixtures/way_tags.yml
+++ b/test/fixtures/way_tags.yml
@@ -15,3 +15,27 @@ t3:
   k: 'test'
   v: 'yes'
   version: 1
   k: 'test'
   v: 'yes'
   version: 1

+
+t6_v1:
+  way_id: 6
+  k: 'test'
+  v: 'yes'
+  version: 1
+
+t6_v2:
+  way_id: 6
+  k: 'test'
+  v: 'yes'
+  version: 2
+
+t6_v3:
+  way_id: 6
+  k: 'test'
+  v: 'yes'
+  version: 3
+
+t6_v4:
+  way_id: 6
+  k: 'test'
+  v: 'yes'
+  version: 4

diff --git a/test/fixtures/ways.yml b/test/fixtures/ways.yml
index 95c001c30fcaa6e34dd2298f94e27fcc42cb676b..56f382f885b64756ce0b925902f1f72f75e5569c 100644 (file)
--- a/test/fixtures/ways.yml
+++ b/test/fixtures/ways.yml
@@ -53,3 +53,40 @@ way_with_duplicate_nodes:
   timestamp: 2007-01-01 00:00:00
   visible: true
   version: 1
   timestamp: 2007-01-01 00:00:00
   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_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
+  timestamp: 2008-01-01 00:04:11
+  visible: true
+  version: 2
+  redaction_id: 1
+
+way_with_redacted_versions_v3:
+  way_id: 6
+  changeset_id: 4
+  timestamp: 2008-01-01 00:04:12
+  visible: true
+  version: 3
+  redaction_id: 1
+
+way_with_redacted_versions_v4:
+  way_id: 6
+  changeset_id: 4
+  timestamp: 2008-01-01 00:04:13
+  visible: true
+  version: 4
\ No newline at end of file

diff --git a/test/functional/old_way_controller_test.rb b/test/functional/old_way_controller_test.rb
index f0ab6bd859409fc0fb3452dd8af335a25bebea60..e79d7b480ca2d7af6bc1098413643eba3ffbebe1 100644 (file)
--- a/test/functional/old_way_controller_test.rb
+++ b/test/functional/old_way_controller_test.rb
@@ -56,6 +56,112 @@ class OldWayControllerTest < ActionController::TestCase
     check_history_equals_versions(current_ways(:way_with_versions).id)
   end
 
     check_history_equals_versions(current_ways(:way_with_versions).id)
   end
 

+  ##
+  # test the redaction of an old version of a way, while not being
+  # authorised.
+  def test_redact_way_unauthorised
+    do_redact_way(ways(:way_with_versions),
+                   redactions(:example))
+    assert_response :unauthorized, "should need to be authenticated to redact."
+  end
+
+    ##
+  # test the redaction of an old version of a way, while being 
+  # authorised as a normal user.
+  def test_redact_way_normal_user
+    basic_authorization(users(:public_user).email, "test")
+
+    do_redact_way(ways(:way_with_versions),
+                   redactions(:example))
+    assert_response :forbidden, "should need to be moderator to redact."
+  end
+
+  ##
+  # test that, even as moderator, the current version of a way
+  # can't be redacted.
+  def test_redact_way_current_version
+    basic_authorization(users(:moderator_user).email, "test")
+
+    do_redact_way(ways(:way_with_versions_v4),
+                   redactions(:example))
+    assert_response :forbidden, "shouldn't be OK to redact current version as moderator."
+  end    
+
+  ##
+  # test that redacted ways aren't visible, regardless of 
+  # authorisation except as moderator...
+  def test_version_redacted
+    way = ways(:way_with_redacted_versions_v2)
+
+    get :version, :id => way.way_id, :version => way.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 => way.way_id, :version => way.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
+    way = ways(:way_with_redacted_versions_v2)
+
+    get :history, :id => way.way_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm way[id=#{way.way_id}][version=#{way.version}]", 0, "redacted way #{way.way_id} version #{way.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 => way.way_id, :version => way.version
+    get :history, :id => way.way_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm way[id=#{way.way_id}][version=#{way.version}]", 0, "redacted node #{way.way_id} version #{way.version} shouldn't be present in the history, even when logged in."
+  end
+
+  ##
+  # test the redaction of an old version of a way, while being 
+  # authorised as a moderator.
+  def test_redact_way_moderator
+    way = ways(:way_with_versions)
+    basic_authorization(users(:moderator_user).email, "test")
+
+    do_redact_way(way, redactions(:example))
+    assert_response :success, "should be OK to redact old version as moderator."
+
+    # check moderator can still see the redacted data
+    get :version, :id => way.way_id, :version => way.version
+    assert_response :success, "After redaction, node should not be gone for moderator."
+    
+    # and when accessed via history
+    get :history, :id => way.way_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm way[id=#{way.way_id}][version=#{way.version}]", 1, "way #{way.way_id} version #{way.version} should still be present in the history for moderators."
+  end
+
+  # testing that if the moderator drops auth, he can't see the
+  # redacted stuff any more.
+  def test_redact_way_is_redacted
+    way = ways(:way_with_versions)
+    basic_authorization(users(:moderator_user).email, "test")
+
+    do_redact_way(way, 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 => way.way_id, :version => way.version
+    assert_response :forbidden, "Redacted node shouldn't be visible via the version API."
+    
+    # and when accessed via history
+    get :version, :id => way.node_id, :version => way.version
+    get :history, :id => way.node_id
+    assert_response :success, "Redaction shouldn't have stopped history working."
+    assert_select "osm way[id=#{way.way_id}][version=#{way.version}]", 0, "redacted way #{way.way_id} version #{way.version} shouldn't be present in the history."
+  end
+

   ##
   # check that the current version of a way is equivalent to the
   # version which we're getting from the versions call.
   ##
   # check that the current version of a way is equivalent to the
   # version which we're getting from the versions call.


@@ -99,4 +205,12 @@ class OldWayControllerTest < ActionController::TestCase
     end
   end
 
     end
   end
 

+  def do_redact_way(way, redaction)
+    get :version, :id => way.way_id, :version => way.version
+    assert_response :success, "should be able to get version #{way.version} of node #{way.way_id}."
+    
+    # now redact it
+    post :redact, :id => way.way_id, :version => way.version, :redaction => redaction.id
+  end
+

 end
 end

diff --git a/test/unit/old_way_tag_test.rb b/test/unit/old_way_tag_test.rb
index 334e7b86a4d6648391f3172bb75c4b2b0c3cc07d..38023bb525cb230513c7fdb012baab0c98884687 100644 (file)
--- a/test/unit/old_way_tag_test.rb
+++ b/test/unit/old_way_tag_test.rb
@@ -4,7 +4,7 @@ class WayTagTest < ActiveSupport::TestCase
   api_fixtures
   
   def test_tag_count
   api_fixtures
   
   def test_tag_count

-    assert_equal 3, OldWayTag.count
+    assert_equal 7, OldWayTag.count

   end
   
   def test_length_key_valid
   end
   
   def test_length_key_valid

diff --git a/test/unit/way_test.rb b/test/unit/way_test.rb
index ca5b751763e6cbe5deb5ea8bb9b0fb54616ef014..d2a447e408aad5e3db4ae8a3339d05bfe9546c5f 100644 (file)
--- a/test/unit/way_test.rb
+++ b/test/unit/way_test.rb
@@ -6,7 +6,7 @@ class WayTest < ActiveSupport::TestCase
   # Check that we have the correct number of currnet ways in the db
   # This will need to updated whenever the current_ways.yml is updated
   def test_db_count
   # Check that we have the correct number of currnet ways in the db
   # This will need to updated whenever the current_ways.yml is updated
   def test_db_count

-    assert_equal 5, Way.count
+    assert_equal 6, Way.count

   end
   
   def test_bbox
   end
   
   def test_bbox