diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b2cc901b09..f989d393d2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -73,7 +73,7 @@ Metrics/ParameterLists: # Offense count: 72 Metrics/PerceivedComplexity: - Max: 23 + Max: 25 # Offense count: 6 Naming/AccessorMethodName: diff --git a/app/assets/javascripts/leaflet.map.js b/app/assets/javascripts/leaflet.map.js index 1f161ceeaf..fe8d74af54 100644 --- a/app/assets/javascripts/leaflet.map.js +++ b/app/assets/javascripts/leaflet.map.js @@ -15,11 +15,12 @@ L.OSM.Map = L.Map.extend({ var copyright = I18n.t("javascripts.map.copyright", { copyright_url: "/copyright" }); var donate = I18n.t("javascripts.map.donate_link_text", { donate_url: "https://donate.openstreetmap.org" }); + var terms = I18n.t("javascripts.map.terms", { terms_url: "https://wiki.osmfoundation.org/wiki/Terms_of_Use" }); this.baseLayers = []; this.baseLayers.push(new L.OSM.Mapnik({ - attribution: copyright + " ♥ " + donate, + attribution: copyright + " ♥ " + donate + ". " + terms, code: "M", keyid: "mapnik", name: I18n.t("javascripts.map.base.standard") @@ -27,7 +28,7 @@ L.OSM.Map = L.Map.extend({ if (OSM.THUNDERFOREST_KEY) { this.baseLayers.push(new L.OSM.CycleMap({ - attribution: copyright + ". Tiles courtesy of Andy Allan", + attribution: copyright + ". Tiles courtesy of Andy Allan. " + terms, apikey: OSM.THUNDERFOREST_KEY, code: "C", keyid: "cyclemap", @@ -35,7 +36,7 @@ L.OSM.Map = L.Map.extend({ })); this.baseLayers.push(new L.OSM.TransportMap({ - attribution: copyright + ". Tiles courtesy of Andy Allan", + attribution: copyright + ". Tiles courtesy of Andy Allan. " + terms, apikey: OSM.THUNDERFOREST_KEY, code: "T", keyid: "transportmap", @@ -44,7 +45,7 @@ L.OSM.Map = L.Map.extend({ } this.baseLayers.push(new L.OSM.HOT({ - attribution: copyright + ". Tiles style by Humanitarian OpenStreetMap Team hosted by OpenStreetMap France", + attribution: copyright + ". Tiles style by Humanitarian OpenStreetMap Team hosted by OpenStreetMap France. " + terms, code: "H", keyid: "hot", name: I18n.t("javascripts.map.base.hot") diff --git a/app/assets/javascripts/user.js b/app/assets/javascripts/user.js index f898455b58..69cc259f0c 100644 --- a/app/assets/javascripts/user.js +++ b/app/assets/javascripts/user.js @@ -124,4 +124,12 @@ $(document).ready(function () { $("#contributorTerms").html(""); $("#contributorTerms").load(url); }); + + $("#read_ct").on("click", function () { + $("#continue").prop("disabled", !($(this).prop("checked") && $("#read_tou").prop("checked"))); + }); + + $("#read_tou").on("click", function () { + $("#continue").prop("disabled", !($(this).prop("checked") && $("#read_ct").prop("checked"))); + }); }); diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9b97fa0a1f..e41330c4b4 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -36,7 +36,7 @@ def terms def save @title = t "users.new.title" - if params[:decline] + if params[:decline] || !(params[:read_tou] && params[:read_ct]) if current_user current_user.terms_seen = true @@ -47,12 +47,15 @@ def save else redirect_to :action => :account, :display_name => current_user.display_name end - else + elsif params[:decline] redirect_to t("users.terms.declined") + else + redirect_to :action => :terms end elsif current_user unless current_user.terms_agreed? current_user.consider_pd = params[:user][:consider_pd] + current_user.tou_agreed = Time.now.getutc current_user.terms_agreed = Time.now.getutc current_user.terms_seen = true @@ -73,6 +76,7 @@ def save current_user.creation_ip = request.remote_ip current_user.languages = http_accept_language.user_preferred_languages current_user.terms_agreed = Time.now.getutc + current_user.tou_agreed = Time.now.getutc current_user.terms_seen = true if current_user.auth_uid.blank? diff --git a/app/models/user.rb b/app/models/user.rb index 9f13108434..dbe91ab0f9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -34,6 +34,7 @@ # image_content_type :string # auth_provider :string # home_tile :integer +# tou_agreed :datetime # # Indexes # diff --git a/app/views/users/terms.html.erb b/app/views/users/terms.html.erb index a93ecb0a13..d4a9fa0bd4 100644 --- a/app/views/users/terms.html.erb +++ b/app/views/users/terms.html.erb @@ -9,7 +9,12 @@ <%= form_tag({ :action => "save" }, { :class => " inner22 standard-form fillL" }) do %> +

<%= t ".read and accept with tou" %>

+
+

<%= t ".contributor_terms_explain" %>

@@ -28,27 +33,42 @@ <%= render :partial => "terms" %>
+
+

+ <%= raw t ".guidance", + :summary => "https://www.osmfoundation.org/wiki/License/Contributor_Terms_Summary", + :translations => "https://www.osmfoundation.org/wiki/License/Contributor_Terms/Informal_Translations" %> +

+
-
+ + +

<%= t ".tou_explain_html", :tou_link => link_to(t("layouts.tou"), "https://wiki.osmfoundation.org/wiki/Terms_of_Use", :target => :new) %>

+
+ - (<%= link_to(t(".consider_pd_why"), t(".consider_pd_why_url"), :target => :new) %>) <%= hidden_field_tag("referer", h(params[:referer])) unless params[:referer].nil? %>
-

<%= t ".read and accept" %>

- <%= submit_tag(t(".agree"), :name => "agree", :id => "agree") %> - <%= submit_tag(t(".decline"), :name => "decline", :id => "decline") %> + <%= submit_tag("Continue", :name => "continue", :id => "continue", :disabled => true) %> + <%= submit_tag("Cancel", :name => "decline", :id => "decline") %>
-
-

- <%= raw t ".guidance", - :summary => "https://www.osmfoundation.org/wiki/License/Contributor_Terms_Summary", - :translations => "https://www.osmfoundation.org/wiki/License/Contributor_Terms/Informal_Translations" %> -

-
+ + (<%= link_to(t(".consider_pd_why"), t(".consider_pd_why_url"), :target => :new) %>) +
<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 5699925999..575c361bb1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1089,6 +1089,7 @@ en: partners_ucl: "UCL" partners_bytemark: "Bytemark Hosting" partners_partners: "partners" + tou: "Terms of Use" osm_offline: "The OpenStreetMap database is currently offline while essential database maintenance work is carried out." osm_read_only: "The OpenStreetMap database is currently in read-only mode while essential database maintenance work is carried out." donate: "Support OpenStreetMap by %{link} to the Hardware Upgrade Fund." @@ -1299,7 +1300,7 @@ en: This site and many other related services are formally operated by the OpenStreetMap Foundation (OSMF) on behalf of the community. Use of all OSMF operated services is subject - to our + to our Terms of Use, Acceptable Use Policies and our Privacy Policy
Please contact the OSMF @@ -2025,14 +2026,19 @@ en: terms declined: "We are sorry that you have decided to not accept the new Contributor Terms. For more information, please see this wiki page." terms declined url: https://wiki.openstreetmap.org/wiki/Contributor_Terms_Declined terms: - title: "Contributor terms" - heading: "Contributor terms" - read and accept: "Please read the agreement and press the agree button to confirm that you accept the terms of this agreement for your existing and future contributions." - consider_pd: "In addition to the above agreement, I consider my contributions to be in the Public Domain" + title: "Terms" + heading: "Terms" + heading_ct: "Contributor terms" + read and accept with tou: "Please read the contributor agreement and the terms of use, check both checkboxes when done and then press the continue button." + contributor_terms_explain: "This agreement governs the terms for your existing and future contributions." + read_ct: "I have read and agree to the above contributor terms" + tou_explain_html: "These %{tou_link} govern the use of the website and other infrastructure provided by the OSMF. Please click on the link, read and aggree to the text." + read_tou: "I have read and agree to the Terms of Use" + consider_pd: "In addition to the above, I consider my contributions to be in the Public Domain" consider_pd_why: "what's this?" consider_pd_why_url: https://www.osmfoundation.org/wiki/License/Why_would_I_want_my_contributions_to_be_public_domain guidance: 'Information to help understand these terms: a human readable summary and some informal translations' - agree: Agree + continue: Continue declined: "https://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." @@ -2417,6 +2423,7 @@ en: title: "Layers" copyright: "© OpenStreetMap contributors" donate_link_text: "" + terms: "Website and API terms" site: edit_tooltip: Edit the map edit_disabled_tooltip: Zoom in to edit the map diff --git a/db/migrate/20181020114000_add_user_tou_agreed.rb b/db/migrate/20181020114000_add_user_tou_agreed.rb new file mode 100644 index 0000000000..ea89c6c09a --- /dev/null +++ b/db/migrate/20181020114000_add_user_tou_agreed.rb @@ -0,0 +1,5 @@ +class AddUserTouAgreed < ActiveRecord::Migration[5.1] + def change + add_column :users, :tou_agreed, :datetime + end +end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 1b875ca97b..df2e7d8f52 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -221,7 +221,7 @@ def test_new_success assert_difference "User.count", 1 do assert_difference "ActionMailer::Base.deliveries.size", 1 do perform_enqueued_jobs do - post :save, :session => { :new_user => user } + post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -245,7 +245,7 @@ def test_new_duplicate_email assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post :save, :session => { :new_user => user } + post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -262,7 +262,7 @@ def test_new_duplicate_email_uppercase assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post :save, :session => { :new_user => user } + post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -279,7 +279,7 @@ def test_new_duplicate_name assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post :save, :session => { :new_user => user } + post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -296,7 +296,7 @@ def test_new_duplicate_name_uppercase assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post :save, :session => { :new_user => user } + post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -313,7 +313,7 @@ def test_new_blocked_domain assert_no_difference "User.count" do assert_no_difference "ActionMailer::Base.deliveries.size" do perform_enqueued_jobs do - post :save, :session => { :new_user => user } + post :save, :session => { :new_user => user }, :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -329,7 +329,8 @@ def test_save_referer_params assert_difference "ActionMailer::Base.deliveries.size", 1 do perform_enqueued_jobs do post :save, :session => { :new_user => user, - :referer => "/edit?editor=id#map=1/2/3" } + :referer => "/edit?editor=id#map=1/2/3" }, + :params => { :read_ct => 1, :read_tou => 1 } end end end @@ -637,7 +638,7 @@ def test_terms_not_seen_without_referer assert_response :success assert_template :terms - post :save, :params => { :user => { :consider_pd => true } } + post :save, :params => { :user => { :consider_pd => true }, :read_ct => 1, :read_tou => 1 } assert_response :redirect assert_redirected_to :action => :account, :display_name => user.display_name assert_equal "Thanks for accepting the new contributor terms!", flash[:notice] @@ -658,7 +659,7 @@ def test_terms_not_seen_with_referer assert_response :success assert_template :terms - post :save, :params => { :user => { :consider_pd => true }, :referer => "/test" } + post :save, :params => { :user => { :consider_pd => true }, :referer => "/test", :read_ct => 1, :read_tou => 1 } assert_response :redirect assert_redirected_to "/test" assert_equal "Thanks for accepting the new contributor terms!", flash[:notice] diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index 144a45a45a..ba4c12b8cd 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -7,6 +7,7 @@ def setup OmniAuth.config.test_mode = true stub_request(:get, /.*gravatar.com.*d=404/).to_return(:status => 404) + stub_hostip_requests end def teardown @@ -92,7 +93,8 @@ def test_user_create_success assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do post "/user/save", - :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } + :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s }, + :params => { :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -113,6 +115,32 @@ def test_user_create_success end end + def test_user_create_no_tou_failure + I18n.available_locales.each do |locale| + new_email = "#{locale}newtester@osm.org" + display_name = "#{locale}_new_tester" + + assert_difference("User.count", 0) do + assert_difference("ActionMailer::Base.deliveries.size", 0) do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + end + end + end + + assert_redirected_to "/user/terms" + + perform_enqueued_jobs do + post "/user/save", + :headers => { "HTTP_ACCEPT_LANGUAGE" => locale.to_s } + assert_redirected_to "/user/terms" + end + + ActionMailer::Base.deliveries.clear + end + end + # Check that the user can successfully recover their password def lost_password_recovery_success # Open the lost password form @@ -135,7 +163,7 @@ def test_user_create_redirect :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => password, :pass_crypt_confirmation => password }, :referer => referer } assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -190,7 +218,7 @@ def test_user_create_openid_success assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "openid", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "openid", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } assert_response :redirect follow_redirect! end @@ -255,7 +283,7 @@ def test_user_create_openid_redirect assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "openid", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "openid", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -312,7 +340,7 @@ def test_user_create_google_success assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "google", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "google", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } assert_response :redirect follow_redirect! end @@ -379,7 +407,7 @@ def test_user_create_google_redirect assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "google", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "google", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -434,7 +462,7 @@ def test_user_create_facebook_success assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "facebook", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "facebook", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } assert_response :redirect follow_redirect! end @@ -499,7 +527,7 @@ def test_user_create_facebook_redirect assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "facebook", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "facebook", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -554,7 +582,7 @@ def test_user_create_windowslive_success assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } assert_response :redirect follow_redirect! end @@ -619,7 +647,7 @@ def test_user_create_windowslive_redirect assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "windowslive", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -674,7 +702,7 @@ def test_user_create_github_success assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "github", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "github", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } assert_response :redirect follow_redirect! end @@ -739,7 +767,7 @@ def test_user_create_github_redirect assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "github", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "github", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end @@ -794,7 +822,7 @@ def test_user_create_wikipedia_success assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "wikipedia", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "wikipedia", :auth_uid => "123454321", :pass_crypt => password, :pass_crypt_confirmation => password }, :read_ct => 1, :read_tou => 1 } assert_response :redirect follow_redirect! end @@ -859,7 +887,7 @@ def test_user_create_wikipedia_redirect assert_response :redirect assert_redirected_to "/user/terms" post "/user/save", - :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "wikipedia", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" } } + :params => { :user => { :email => new_email, :email_confirmation => new_email, :display_name => display_name, :auth_provider => "wikipedia", :auth_uid => "http://localhost:1123/new.tester", :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest" }, :read_ct => 1, :read_tou => 1 } follow_redirect! end end