Added flash notice for CTs decline

Also ensured that CTs are either accepted or declined and cannot
be inadvertently bypassed.

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d715c618df35f0697dac19fc5399df4191c1891b..8062c9fe3e4e16058cd1113b6b19303b906bb1ce 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -15,6 +15,16 @@ class ApplicationController < ActionController::Base
         session_expires_automatically
 
         redirect_to :controller => "user", :action => "suspended"
+
+        # don't allow access to any auth-requiring part of the site unless
+        # the new CTs have been seen (and accept/decline chosen).
+      elsif !@user.terms_seen and flash[:showing_terms].nil?
+        flash[:notice] = t 'user.terms.you need to accept or decline'
+        if params[:referer]
+          redirect_to :controller => "user", :action => "terms", :referer => params[:referer]
+        else
+          redirect_to :controller => "user", :action => "terms", :referer => request.request_uri
+        end
       end
     elsif session[:token]
       @user = User.authenticate(:token => session[:token])

diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb
index 72d60a307eda63e0cd9e947bf326000afb7061d6..f37d4f394336e38228a18ed122263e1dae0d83cf 100644 (file)
--- a/app/controllers/user_controller.rb
+++ b/app/controllers/user_controller.rb
@@ -1,6 +1,7 @@
 class UserController < ApplicationController
   layout :choose_layout
 
+  before_filter :disable_terms_redirect, :only => [:terms, :save]
   before_filter :authorize, :only => [:api_details, :api_gpx_files]
   before_filter :authorize_web, :except => [:api_details, :api_gpx_files]
   before_filter :set_locale, :except => [:api_details, :api_gpx_files]
@@ -55,7 +56,10 @@ class UserController < ApplicationController
     elsif params[:decline]
       if @user
         @user.terms_seen = true
-        @user.save
+
+        if @user.save
+          flash[:notice] = t 'user.new.terms declined', :url => t('user.new.terms declined url')
+        end
 
         if params[:referer]
           redirect_to params[:referer]
@@ -511,4 +515,13 @@ private
       'site'
     end
   end
+
+  ##
+  #
+  def disable_terms_redirect
+    # this is necessary otherwise going to the user terms page, when 
+    # having not agreed already would cause an infinite redirect loop.
+    # it's .now so that this doesn't propagate to other pages.
+    flash.now[:showing_terms] = true
+  end
 end

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7641d5532ca218b828102fbae474faab101a29f7..e2ca59fb659ab79a2f50ebb203e8610df6b296c4 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1567,6 +1567,8 @@ en:
       continue: Continue
       flash create success message: "Thanks for signing up. We've sent a confirmation note to {{email}} and as soon as you confirm your account you'll be able to get mapping.<br /><br />If you use an antispam system which sends confirmation requests then please make sure you whitelist webmaster@openstreetmap.org as we are unable to reply to any confirmation requests."
       terms accepted: "Thanks for accepting the new contributor terms!"
+      terms declined: "We are sorry that you have decided to not accept the new Contributor Terms. For more information, please see <a href=\"{{url}}\">this wiki page</a>."
+      terms declined url: http://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined
     terms:
       title: "Contributor terms"
       heading: "Contributor terms"
@@ -1577,6 +1579,7 @@ en:
       agree: Agree
       declined: "http://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined"
       decline: "Decline"
+      you need to accept or decline: "Please read and then either accept or decline the new Contributor Terms to continue."
       legale_select: "Please select your country of residence:"
       legale_names:
         france: "France"

diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb
index f9c266ba0092a4bc853d8980c60268029d4fa9cd..f30c5d98e700b47ee19f8d9d0d62037af1d04693 100644 (file)
--- a/test/integration/user_terms_seen_test.rb
+++ b/test/integration/user_terms_seen_test.rb
@@ -39,8 +39,35 @@ class UserTermsSeenTest < ActionController::IntegrationTest
       assert_response :success
 
       # don't agree to the terms, but hit decline
+      post "/user/#{user.display_name}/save", {'decline' => 'decline', 'referer' => '/'}
+      assert_redirected_to "/"
+      follow_redirect!
+      
+      # should be carried through to a normal login with a message
+      assert_response :success
+      assert !flash[:notice].nil?
+    end
+  end
+
+  def test_terms_cant_be_circumvented
+    if REQUIRE_TERMS_SEEN
+      user = users(:terms_not_seen_user)
+
+      # try to log in
+      get_via_redirect "/login"
+      assert_response :success
+      assert_template 'user/login'
+      post "/login", {'user[email]' => user.email, 'user[password]' => 'test', :referer => "/"}
+      assert_response :redirect
+      # but now we need to look at the terms
+      assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/"
+      follow_redirect!
+      assert_response :success
 
-      # should be carried through to a normal login
+      # check that if we go somewhere else now, it redirects
+      # back to the terms page.
+      get "/traces/mine"
+      assert_redirected_to "controller" => "user", "action" => "terms", :referer => "/traces/mine"
     end
   end