From 0c7c950149888777e186e2eb7525b37f340bc6f0 Mon Sep 17 00:00:00 2001 From: Milan Cvetkovic Date: Mon, 18 Mar 2024 15:32:00 +0000 Subject: [PATCH] Add social signin buttons to signup screen, avoid repeating round trip to auth provider. --- app/controllers/users_controller.rb | 41 +- app/views/users/new.html.erb | 79 +-- config/locales/en.yml | 8 +- test/integration/user_creation_test.rb | 647 ++++++++++++++++--------- 4 files changed, 485 insertions(+), 290 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 4974d99f1..74ec5c0ec 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -69,12 +69,17 @@ class UsersController < ApplicationController # page, instead send them to the home page redirect_to @referer || { :controller => "site", :action => "index" } elsif params.key?(:auth_provider) && params.key?(:auth_uid) + @verified_email = params[:verified_email] + self.current_user = User.new(:email => params[:email], :display_name => params[:nickname], :auth_provider => params[:auth_provider], :auth_uid => params[:auth_uid]) flash.now[:notice] = render_to_string :partial => "auth_association" + + # validate before displaying to show email field if it conflicts + flash.now[:notice] = t ".duplicate_social_email" unless current_user.valid? && current_user.errors[:email].empty? else check_signup_allowed @@ -90,7 +95,7 @@ class UsersController < ApplicationController Rails.logger.info "create: #{session[:referer]}" - if current_user.auth_provider.present? && current_user.pass_crypt.empty? + if current_user.auth_uid.present? # We are creating an account with external authentication and # no password was specified so create a random one current_user.pass_crypt = SecureRandom.base64(16) @@ -100,14 +105,9 @@ class UsersController < ApplicationController if current_user.invalid? # Something is wrong with a new user, so rerender the form render :action => "new" - elsif current_user.auth_provider.present? - # Verify external authenticator before moving on - session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation", "consider_pd") - redirect_to auth_url(current_user.auth_provider, current_user.auth_uid), :status => :temporary_redirect else # Save the user record - session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation", "consider_pd") - save_new_user + save_new_user params[:verified_email] end end end @@ -131,7 +131,7 @@ class UsersController < ApplicationController if current_user&.terms_agreed? # Already agreed to terms, so just show settings redirect_to edit_account_path - elsif current_user.nil? && session[:new_user].nil? + elsif current_user.nil? redirect_to login_path(:referer => request.fullpath) end end @@ -220,12 +220,6 @@ class UsersController < ApplicationController session[:user_errors] = current_user.errors.as_json redirect_to edit_account_path - elsif session[:new_user] - session[:new_user]["auth_provider"] = provider - session[:new_user]["auth_uid"] = uid - session[:new_user]["verified_email"] = email if email_verified - - save_new_user else user = User.find_by(:auth_provider => provider, :auth_uid => uid) @@ -247,7 +241,8 @@ class UsersController < ApplicationController failed_login t("sessions.new.auth failure") end else - redirect_to :action => "new", :nickname => name, :email => email, + verified_email = UsersController.message_hmac(email) if email_verified && email + redirect_to :action => "new", :nickname => name, :email => email, :verified_email => verified_email, :auth_provider => provider, :auth_uid => uid end end @@ -263,14 +258,16 @@ class UsersController < ApplicationController redirect_to origin || login_url end - private - - def save_new_user - new_user = session.delete(:new_user) - verified_email = new_user.delete("verified_email") + def self.message_hmac(text) + sha256 = Digest::SHA256.new + sha256 << Rails.application.key_generator.generate_key("openstreetmap/verified_email") + sha256 << text + Base64.urlsafe_encode64(sha256.digest) + end - self.current_user = User.new(new_user) + private + def save_new_user(email_hmac) if check_signup_allowed(current_user.email) current_user.data_public = true current_user.description = "" if current_user.description.nil? @@ -283,7 +280,7 @@ class UsersController < ApplicationController if current_user.auth_uid.blank? current_user.auth_provider = nil current_user.auth_uid = nil - elsif current_user.email == verified_email + elsif email_hmac && ActiveSupport::SecurityUtils.secure_compare(email_hmac, UsersController.message_hmac(current_user.email)) current_user.activate end diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 0d3160a8c..918b315e8 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -1,5 +1,6 @@ <% content_for :head do %> <%= javascript_include_tag "user" %> + <%= javascript_include_tag "auth_providers" %> <% end %> <% content_for :heading_class, "p-0 mw-100" %> @@ -11,59 +12,73 @@ <% end %>
-
-

<%= t ".about.header" %> <%= t ".about.paragraph_1" %>

-

<%= t ".about.paragraph_2" %>

-
+ <% if current_user.auth_uid.nil? %> +
+

<%= t ".about.header" %> <%= t ".about.paragraph_1" %>

+

<%= t ".about.paragraph_2" %>

+
+ <% else %> +

<%= t ".about.welcome" %>

+ <% end %> <%= bootstrap_form_for current_user, :url => { :action => "create" } do |f| %> <%= hidden_field_tag("referer", h(@referer)) unless @referer.nil? %> + <%= hidden_field_tag("verified_email", h(@verified_email)) unless @verified_email.nil? %> + <%= f.hidden_field :auth_provider unless current_user.auth_provider.nil? %> + <%= f.hidden_field :auth_uid unless current_user.auth_uid.nil? %> - <%= f.email_field :email, :help => t(".email_help_html", - :privacy_policy_link => link_to(t(".privacy_policy"), - t(".privacy_policy_url"), - :title => t(".privacy_policy_title"))), - :tabindex => 1 %> + <% if current_user.auth_uid.nil? or not current_user.errors[:email].empty? %> + <%= f.email_field :email, :help => t(".email_help_html", + :privacy_policy_link => link_to(t(".privacy_policy"), + t(".privacy_policy_url"), + :title => t(".privacy_policy_title"), + :target => :new)), + :tabindex => 1 %> + <% else %> + <%= f.hidden_field :email %> + <% end %> <%= f.text_field :display_name, :help => t(".display name description"), :tabindex => 2 %> -
- + <% if current_user.auth_uid.nil? %>
- <%= f.select(:auth_provider, Auth.providers, :default => "", :hide_label => true, :wrapper => { :class => "col-auto mb-0" }, :tabindex => 3) %> - <%= f.text_field(:auth_uid, :hide_label => true, :wrapper => { :class => "col mb-0" }, :tabindex => 4) %> +
+ <%= f.password_field :pass_crypt, :tabindex => 3 %> +
+
+ <%= f.password_field :pass_crypt_confirmation, :tabindex => 4 %> +
- <%= t ".auth no password" %> -
- - <%= f.password_field :pass_crypt, :tabindex => 5 %> - <%= f.password_field :pass_crypt_confirmation, :tabindex => 6 %> + <% end %>

<%= t(".by_signing_up_html", :tou_link => link_to(t("layouts.tou"), "https://wiki.osmfoundation.org/wiki/Terms_of_Use", :target => :new), + :privacy_policy_link => link_to(t(".privacy_policy"), + t(".privacy_policy_url"), + :title => t(".privacy_policy_title"), + :target => :new), :contributor_terms_link => link_to(t(".contributor_terms"), t(".contributor_terms_url"), :target => :new)) %>

-
- <%= submit_tag(t(".continue"), :name => "continue", :id => "continue", :class => "btn btn-primary", :tabindex => 7) %> -
+ <%= f.form_group do %> + <%= f.check_box :consider_pd, + :tabindex => 5, + :label => t(".consider_pd_html", + :consider_pd_link => link_to(t(".consider_pd"), + t(".consider_pd_url"), + :target => :new)) %> + <% end %>
-
- <%= check_box("user", "consider_pd", :class => "form-check-input") %> - - (<%= link_to(t(".consider_pd_why"), t(".consider_pd_why_url"), :target => :new) %>) -
-
- -
-

<%= link_to t(".use external auth"), "#", :id => "auth_enable" %>

+ <%= submit_tag(t(".continue"), :name => "continue", :id => "continue", :class => "btn btn-primary", :tabindex => 6) %>
+ <% end %> + <% if current_user.auth_uid.nil? %> +
+ <%= render :partial => "auth_providers" %> <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index f6d79369b..4046b8068 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2735,6 +2735,8 @@ en: header: Free and editable. paragraph_1: Unlike other maps, OpenStreetMap is completely created by people like you, and it's free for anyone to fix, update, download and use. paragraph_2: Sign up to get started contributing. We'll send an email to confirm your account. + welcome: "Welcome to OpenStreetMap" + duplicate_social_email: "If you already have an OpenStreetMap account and wish to use a 3rd party identity provider, please log in using your password and modify the settings of your account." display name description: "Your publicly displayed username. You can change this later in the preferences." by_signing_up_html: "By signing up, you agree to our %{contributor_terms_link} and %{tou_link}." contributor_terms_url: "https://wiki.osmfoundation.org/wiki/Licence/Contributor_Terms" @@ -2748,9 +2750,9 @@ en: privacy_policy: privacy policy privacy_policy_url: https://wiki.osmfoundation.org/wiki/Privacy_Policy privacy_policy_title: OSMF privacy policy including section on email addresses - 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://wiki.osmfoundation.org/wiki/Licence_and_Legal_FAQ/Why_would_I_want_my_contributions_to_be_public_domain + consider_pd_html: "I consider my contributions to be in the %{consider_pd_link}." + consider_pd: "public domain" + consider_pd_url: https://wiki.osmfoundation.org/wiki/Licence_and_Legal_FAQ/Why_would_I_want_my_contributions_to_be_public_domain terms: title: "Terms" heading: "Terms" diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index 370886e5d..beb05a98a 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -45,6 +45,29 @@ class UserCreationTest < ActionDispatch::IntegrationTest assert_select "form > div > input.is-invalid#user_email" end + def test_user_create_association_submit_duplicate_email + dup_email = create(:user).email + display_name = "new_tester" + assert_difference("User.count", 0) do + assert_no_difference("ActionMailer::Base.deliveries.size") do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => dup_email, + :display_name => display_name, + :pass_crypt => "testtest", + :pass_crypt_confirmation => "testtest", + :auth_provider => "auth_provider", + :auth_uid => "123454321", + :consider_pd => "1" } } + end + end + end + assert_response :success + assert_template "users/new" + assert_select "form" + assert_select "form > div > input.is-invalid#user_email" + end + def test_user_create_submit_duplicate_username dup_display_name = create(:user).display_name email = "new_tester" @@ -74,13 +97,34 @@ class UserCreationTest < ActionDispatch::IntegrationTest :params => { :user => { :email => email, :display_name => display_name, :pass_crypt => "testtest", - :pass_crypt_confirmation => "blahblah" } } + :pass_crypt_confirmation => "blahblah", + :consider_pd => "1" } } end end end assert_response :success assert_template "users/new" - assert_select "form > div > input.is-invalid#user_pass_crypt_confirmation" + assert_select "form > div > div > div > input.is-invalid#user_pass_crypt_confirmation" + end + + def test_user_create_association_submit_duplicate_username + dup_display_name = create(:user).display_name + email = "new_tester" + assert_difference("User.count", 0) do + assert_no_difference("ActionMailer::Base.deliveries.size") do + perform_enqueued_jobs do + post "/user/new", + :params => { :user => { :email => email, + :display_name => dup_display_name, + :auth_provider => "provider", + :auth_uid => "123454321", + :consider_pd => "1" } } + end + end + end + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_display_name" end def test_user_create_success @@ -96,7 +140,6 @@ class UserCreationTest < ActionDispatch::IntegrationTest :pass_crypt => "testtest", :pass_crypt_confirmation => "testtest", :consider_pd => "1" } } - assert_response :redirect assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name follow_redirect! end @@ -189,35 +232,58 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_openid_success - OmniAuth.config.add_mock(:openid, :uid => "http://localhost:1123/new.tester") - new_email = "newtester-openid@osm.org" display_name = "new_tester-openid" - password = "testtest" + auth_uid = "http://localhost:1123/new.tester" + + OmniAuth.config.add_mock(:openid, + :uid => auth_uid, + :info => { :email => new_email, :name => display_name }) + assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, :email => new_email, + :auth_provider => "openid", :auth_uid => auth_uid + follow_redirect! post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "openid", :auth_uid => "http://localhost:1123/new.tester", - :pass_crypt => password, - :pass_crypt_confirmation => password, :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name - follow_redirect! end end end # Check the page + assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + + ActionMailer::Base.deliveries.clear + end + + def test_user_create_openid_duplicate_email + dup_user = create(:user) + display_name = "new_tester-openid" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:openid, + :uid => auth_uid, + :info => { :email => dup_user.email, :name => display_name }) + + post auth_path(:provider => "openid", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "openid", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, :email => dup_user.email, + :auth_provider => "openid", :auth_uid => auth_uid + follow_redirect! + assert_response :success - assert_template "confirmations/confirm" + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_email" ActionMailer::Base.deliveries.clear end @@ -225,29 +291,14 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_openid_failure OmniAuth.config.mock_auth[:openid] = :connection_failed - new_email = "newtester-openid2@osm.org" - display_name = "new_tester-openid2" 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, - :display_name => display_name, - :auth_provider => "openid", - :auth_uid => "http://localhost:1123/new.tester", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + post auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "openid", :message => "connection_failed", :origin => "/user/new") follow_redirect! - assert_response :redirect - follow_redirect! - assert_response :success - assert_template "users/new" + assert_redirected_to "/user/new" end end end @@ -256,29 +307,29 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_openid_redirect - OmniAuth.config.add_mock(:openid, :uid => "http://localhost:1123/new.tester") - + auth_uid = "12345654321" new_email = "redirect_tester_openid@osm.org" display_name = "redirect_tester_openid" - # nothing special about this page, just need a protected page to redirect back to. - referer = "/traces/mine" + + OmniAuth.config.add_mock(:openid, + :uid => auth_uid, + :info => { :email => new_email, :name => display_name }) + assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, :email => new_email, + :auth_provider => "openid", :auth_uid => auth_uid + follow_redirect! post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "openid", - :auth_uid => "http://localhost:1123/new.tester", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" }, - :referer => referer } - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + :auth_uid => auth_uid, + :consider_pd => "1" } } follow_redirect! end end @@ -315,25 +366,33 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_google_success new_email = "newtester-google@osm.org" + verified_email = UsersController.message_hmac(new_email) display_name = "new_tester-google" - password = "testtest" + auth_uid = "123454321" - OmniAuth.config.add_mock(:google, :uid => "123454321", :info => { "email" => new_email }) + OmniAuth.config.add_mock(:google, + :uid => auth_uid, + :extra => { :id_info => { :openid_id => "http://localhost:1123/new.tester" } }, + :info => { :email => new_email, :name => display_name }) assert_difference("User.count") do assert_no_difference("ActionMailer::Base.deliveries.size") do perform_enqueued_jobs do + post auth_path(:provider => "google", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "google") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => new_email, :verified_email => verified_email, + :auth_provider => "google", :auth_uid => auth_uid + follow_redirect! + post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "google", - :pass_crypt => password, - :pass_crypt_confirmation => password, - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "google") - follow_redirect! + :auth_uid => auth_uid, + :consider_pd => "1" }, + :verified_email => verified_email } assert_redirected_to welcome_path follow_redirect! end @@ -347,31 +406,43 @@ class UserCreationTest < ActionDispatch::IntegrationTest ActionMailer::Base.deliveries.clear end + def test_user_create_google_duplicate_email + dup_user = create(:user) + display_name = "new_tester-google" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:google, + :uid => auth_uid, + :extra => { :id_info => { :openid_id => "http://localhost:1123/new.tester" } }, + :info => { :email => dup_user.email, :name => display_name }) + + post auth_path(:provider => "google", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "google") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, :email => dup_user.email, + :verified_email => UsersController.message_hmac(dup_user.email), + :auth_provider => "google", :auth_uid => auth_uid + follow_redirect! + + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_email" + + ActionMailer::Base.deliveries.clear + end + def test_user_create_google_failure OmniAuth.config.mock_auth[:google] = :connection_failed - new_email = "newtester-google2@osm.org" - display_name = "new_tester-google2" 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, - :display_name => display_name, - :auth_provider => "google", - :pass_crypt => "", - :pass_crypt_confirmation => "", - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "google") + post auth_path(:provider => "google", :origin => "/user/new") + assert_response :redirect follow_redirect! assert_redirected_to auth_failure_path(:strategy => "google", :message => "connection_failed", :origin => "/user/new") follow_redirect! - assert_response :redirect - follow_redirect! - assert_response :success - assert_template "users/new" + assert_redirected_to "/user/new" end end end @@ -380,30 +451,35 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_google_redirect - OmniAuth.config.add_mock(:google, :uid => "123454321", :extra => { - :id_info => { "openid_id" => "http://localhost:1123/new.tester" } - }) - - new_email = "redirect_tester_google@osm.org" + orig_email = "redirect_tester_google_orig@google.com" + verified_email = UsersController.message_hmac(orig_email) + new_email = "redirect_tester_google@osm.org" display_name = "redirect_tester_google" - # nothing special about this page, just need a protected page to redirect back to. - referer = "/traces/mine" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:google, + :uid => auth_uid, + :extra => { :id_info => { :openid_id => "http://localhost:1123/new.tester" } }, + :info => { :email => orig_email, :name => display_name }) + assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "google", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "google") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => orig_email, :verified_email => verified_email, + :auth_provider => "google", :auth_uid => auth_uid + follow_redirect! post "/user/new", :params => { :user => { :email => new_email, + :verified_email => verified_email, :display_name => display_name, :auth_provider => "google", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" }, - :referer => referer } - assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "google") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + :auth_uid => auth_uid, + :consider_pd => "1" } } + assert_response :redirect follow_redirect! end end @@ -440,25 +516,32 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_facebook_success new_email = "newtester-facebook@osm.org" + verified_email = UsersController.message_hmac(new_email) display_name = "new_tester-facebook" - password = "testtest" + auth_uid = "123454321" - OmniAuth.config.add_mock(:facebook, :uid => "123454321", :info => { "email" => new_email }) + OmniAuth.config.add_mock(:facebook, + :uid => auth_uid, + :info => { "email" => new_email, :name => display_name }) assert_difference("User.count") do assert_no_difference("ActionMailer::Base.deliveries.size") do perform_enqueued_jobs do + post auth_path(:provider => "facebook", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "facebook") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => new_email, :verified_email => verified_email, + :auth_provider => "facebook", :auth_uid => auth_uid + follow_redirect! + post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "facebook", - :pass_crypt => password, - :pass_crypt_confirmation => password, - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "facebook") - follow_redirect! + :auth_uid => auth_uid, + :consider_pd => "1" }, + :verified_email => verified_email } assert_redirected_to welcome_path follow_redirect! end @@ -472,31 +555,42 @@ class UserCreationTest < ActionDispatch::IntegrationTest ActionMailer::Base.deliveries.clear end + def test_user_create_facebook_duplicate_email + dup_user = create(:user) + display_name = "new_tester-facebook" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:facebook, + :uid => auth_uid, + :info => { :email => dup_user.email, :name => display_name }) + + post auth_path(:provider => "facebook", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "facebook") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, :email => dup_user.email, + :verified_email => UsersController.message_hmac(dup_user.email), + :auth_provider => "facebook", :auth_uid => auth_uid + follow_redirect! + + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_email" + + ActionMailer::Base.deliveries.clear + end + def test_user_create_facebook_failure OmniAuth.config.mock_auth[:facebook] = :connection_failed - new_email = "newtester-facebook2@osm.org" - display_name = "new_tester-facebook2" 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, - :display_name => display_name, - :auth_provider => "facebook", - :pass_crypt => "", - :pass_crypt_confirmation => "", - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "facebook") + post auth_path(:provider => "facebook", :origin => "/user/new") + assert_response :redirect follow_redirect! assert_redirected_to auth_failure_path(:strategy => "facebook", :message => "connection_failed", :origin => "/user/new") follow_redirect! - assert_response :redirect - follow_redirect! - assert_response :success - assert_template "users/new" + assert_redirected_to "/user/new" end end end @@ -505,28 +599,36 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_facebook_redirect - OmniAuth.config.add_mock(:facebook, :uid => "123454321") - + orig_email = "redirect_tester_facebook_orig@osm.org" + verified_email = UsersController.message_hmac(orig_email) new_email = "redirect_tester_facebook@osm.org" display_name = "redirect_tester_facebook" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:facebook, + :uid => auth_uid, + :info => { :email => orig_email, :name => display_name }) + # nothing special about this page, just need a protected page to redirect back to. - referer = "/traces/mine" assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "facebook", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "facebook") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => orig_email, :verified_email => verified_email, + :auth_provider => "facebook", :auth_uid => auth_uid + follow_redirect! + post "/user/new", :params => { :user => { :email => new_email, + :verified_email => verified_email, :display_name => display_name, :auth_provider => "facebook", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" }, - :referer => referer } - assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "facebook") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + :auth_uid => auth_uid, + :consider_pd => "1" } } + assert_response :redirect follow_redirect! end end @@ -563,25 +665,31 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_microsoft_success new_email = "newtester-microsoft@osm.org" + verified_email = UsersController.message_hmac(new_email) display_name = "new_tester-microsoft" - password = "testtest" + auth_uid = "123454321" - OmniAuth.config.add_mock(:microsoft, :uid => "123454321", :info => { "email" => new_email }) + OmniAuth.config.add_mock(:microsoft, + :uid => auth_uid, + :info => { "email" => new_email, :name => display_name }) assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 0) do perform_enqueued_jobs do + post auth_path(:provider => "microsoft", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "microsoft") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => new_email, :verified_email => verified_email, + :auth_provider => "microsoft", :auth_uid => auth_uid + follow_redirect! post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "microsoft", - :pass_crypt => password, - :pass_crypt_confirmation => password, - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "microsoft") - follow_redirect! + :auth_uid => auth_uid, + :consider_pd => "1" }, + :verified_email => verified_email } assert_redirected_to welcome_path follow_redirect! end @@ -595,31 +703,42 @@ class UserCreationTest < ActionDispatch::IntegrationTest ActionMailer::Base.deliveries.clear end + def test_user_create_microsoft_duplicate_email + dup_user = create(:user) + display_name = "new_tester-microsoft" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:microsoft, + :uid => auth_uid, + :info => { :email => dup_user.email, :name => display_name }) + + post auth_path(:provider => "microsoft", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "microsoft") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, :email => dup_user.email, + :verified_email => UsersController.message_hmac(dup_user.email), + :auth_provider => "microsoft", :auth_uid => auth_uid + follow_redirect! + + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_email" + + ActionMailer::Base.deliveries.clear + end + def test_user_create_microsoft_failure OmniAuth.config.mock_auth[:microsoft] = :connection_failed - new_email = "newtester-microsoft2@osm.org" - display_name = "new_tester-microsoft2" 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, - :display_name => display_name, - :auth_provider => "microsoft", - :pass_crypt => "", - :pass_crypt_confirmation => "", - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "microsoft") + post auth_path(:provider => "microsoft", :origin => "/user/new") + assert_response :redirect follow_redirect! assert_redirected_to auth_failure_path(:strategy => "microsoft", :message => "connection_failed", :origin => "/user/new") follow_redirect! - assert_response :redirect - follow_redirect! - assert_response :success - assert_template "users/new" + assert_redirected_to "/user/new" end end end @@ -628,28 +747,35 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_microsoft_redirect - OmniAuth.config.add_mock(:microsoft, :uid => "123454321") - + orig_email = "redirect_tester_microsoft_orig@osm.org" + verified_email = UsersController.message_hmac(orig_email) new_email = "redirect_tester_microsoft@osm.org" display_name = "redirect_tester_microsoft" - # nothing special about this page, just need a protected page to redirect back to. - referer = "/traces/mine" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:microsoft, + :uid => auth_uid, + :info => { :email => orig_email, :name => display_name }) + assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "microsoft", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "microsoft") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => orig_email, :verified_email => verified_email, + :auth_provider => "microsoft", :auth_uid => auth_uid + follow_redirect! + post "/user/new", :params => { :user => { :email => new_email, + :verified_email => verified_email, :display_name => display_name, :auth_provider => "microsoft", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" }, - :referer => referer } - assert_redirected_to auth_path(:provider => "microsoft", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "microsoft") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + :auth_uid => auth_uid, + :consider_pd => "1" } } + assert_response :redirect follow_redirect! end end @@ -686,25 +812,36 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_github_success new_email = "newtester-github@osm.org" + verified_email = UsersController.message_hmac(new_email) display_name = "new_tester-github" password = "testtest" + auth_uid = "123454321" - OmniAuth.config.add_mock(:github, :uid => "123454321", :info => { "email" => new_email }) + OmniAuth.config.add_mock(:github, + :uid => auth_uid, + :info => { "email" => new_email, :name => display_name }) assert_difference("User.count") do assert_no_difference("ActionMailer::Base.deliveries.size") do perform_enqueued_jobs do + post auth_path(:provider => "github", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "github") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => new_email, :verified_email => verified_email, + :auth_provider => "github", :auth_uid => auth_uid + follow_redirect! + post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "github", + :auth_uid => "123454321", :pass_crypt => password, - :pass_crypt_confirmation => password, - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "github") - follow_redirect! + :pass_crypt_confirmation => password }, + :read_ct => 1, + :read_tou => 1, + :verified_email => verified_email } assert_redirected_to welcome_path follow_redirect! end @@ -718,31 +855,42 @@ class UserCreationTest < ActionDispatch::IntegrationTest ActionMailer::Base.deliveries.clear end + def test_user_create_github_duplicate_email + dup_user = create(:user) + display_name = "new_tester-github" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:github, + :uid => auth_uid, + :extra => { :id_info => { :openid_id => "http://localhost:1123/new.tester" } }, + :info => { :email => dup_user.email, :name => display_name }) + + post auth_path(:provider => "github", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "github") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => dup_user.email, :verified_email => UsersController.message_hmac(dup_user.email), + :auth_provider => "github", :auth_uid => auth_uid + follow_redirect! + + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_email" + + ActionMailer::Base.deliveries.clear + end + def test_user_create_github_failure OmniAuth.config.mock_auth[:github] = :connection_failed - new_email = "newtester-github2@osm.org" - display_name = "new_tester-github2" 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, - :display_name => display_name, - :auth_provider => "github", - :pass_crypt => "", - :pass_crypt_confirmation => "", - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "github") + post auth_path(:provider => "github", :origin => "/user/new") follow_redirect! assert_redirected_to auth_failure_path(:strategy => "github", :message => "connection_failed", :origin => "/user/new") follow_redirect! - assert_response :redirect - follow_redirect! - assert_response :success - assert_template "users/new" + assert_redirected_to "/user/new" end end end @@ -751,28 +899,34 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_github_redirect - OmniAuth.config.add_mock(:github, :uid => "123454321") - + orig_email = "redirect_tester_github_orig@osm.org" + verified_email = UsersController.message_hmac(orig_email) new_email = "redirect_tester_github@osm.org" display_name = "redirect_tester_github" - # nothing special about this page, just need a protected page to redirect back to. - referer = "/traces/mine" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:github, + :uid => auth_uid, + :info => { :email => orig_email, :name => display_name }) + assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "github", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "github") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => orig_email, :verified_email => verified_email, + :auth_provider => "github", :auth_uid => auth_uid + follow_redirect! post "/user/new", :params => { :user => { :email => new_email, + :verified_email => verified_email, :display_name => display_name, :auth_provider => "github", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" }, - :referer => referer } - assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "github") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + :auth_uid => auth_uid, + :consider_pd => "1" } } + assert_response :redirect follow_redirect! end end @@ -809,25 +963,35 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_wikipedia_success new_email = "newtester-wikipedia@osm.org" + verified_email = UsersController.message_hmac(new_email) display_name = "new_tester-wikipedia" password = "testtest" + auth_uid = "123454321" - OmniAuth.config.add_mock(:wikipedia, :uid => "123454321", :info => { "email" => new_email }) + OmniAuth.config.add_mock(:wikipedia, + :uid => auth_uid, + :info => { :email => new_email, :name => display_name }) assert_difference("User.count") do assert_no_difference("ActionMailer::Base.deliveries.size") do perform_enqueued_jobs do + post auth_path(:provider => "wikipedia", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => new_email, :verified_email => verified_email, + :auth_provider => "wikipedia", :auth_uid => auth_uid + follow_redirect! post "/user/new", :params => { :user => { :email => new_email, :display_name => display_name, :auth_provider => "wikipedia", + :auth_uid => "123454321", :pass_crypt => password, - :pass_crypt_confirmation => password, - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") - follow_redirect! + :pass_crypt_confirmation => password }, + :read_ct => 1, + :read_tou => 1, + :verified_email => verified_email } assert_redirected_to welcome_path follow_redirect! end @@ -837,6 +1001,28 @@ class UserCreationTest < ActionDispatch::IntegrationTest # Check the page assert_response :success assert_template "site/welcome" + end + + def test_user_create_wikipedia_duplicate_email + dup_user = create(:user) + display_name = "new_tester-wikipedia" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:wikipedia, + :uid => auth_uid, + :info => { "email" => dup_user.email, :name => display_name }) + + post auth_path(:provider => "wikipedia", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => dup_user.email, :verified_email => UsersController.message_hmac(dup_user.email), + :auth_provider => "wikipedia", :auth_uid => auth_uid + follow_redirect! + + assert_response :success + assert_template "users/new" + assert_select "form > div > input.is-invalid#user_email" ActionMailer::Base.deliveries.clear end @@ -844,28 +1030,15 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_wikipedia_failure OmniAuth.config.mock_auth[:wikipedia] = :connection_failed - new_email = "newtester-wikipedia2@osm.org" - display_name = "new_tester-wikipedia2" 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, - :display_name => display_name, - :auth_provider => "wikipedia", - :pass_crypt => "", - :pass_crypt_confirmation => "", - :consider_pd => "1" } } - assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") + post auth_path(:provider => "wikipedia", :origin => "/user/new") + assert_response :redirect follow_redirect! assert_redirected_to auth_failure_path(:strategy => "wikipedia", :message => "connection_failed", :origin => "/user/new") follow_redirect! - assert_response :redirect - follow_redirect! - assert_response :success - assert_template "users/new" + assert_redirected_to "/user/new" end end end @@ -874,28 +1047,36 @@ class UserCreationTest < ActionDispatch::IntegrationTest end def test_user_create_wikipedia_redirect - OmniAuth.config.add_mock(:wikipedia, :uid => "123454321") - + orig_email = "redirect_tester_wikipedia_orig@osm.org" + verified_email = UsersController.message_hmac(orig_email) new_email = "redirect_tester_wikipedia@osm.org" display_name = "redirect_tester_wikipedia" + auth_uid = "123454321" + + OmniAuth.config.add_mock(:wikipedia, + :uid => auth_uid, + :info => { :email => orig_email, :name => display_name }) + # nothing special about this page, just need a protected page to redirect back to. - referer = "/traces/mine" assert_difference("User.count") do assert_difference("ActionMailer::Base.deliveries.size", 1) do perform_enqueued_jobs do + post auth_path(:provider => "wikipedia", :origin => "/user/new") + assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") + follow_redirect! + assert_redirected_to :controller => :users, :action => "new", :nickname => display_name, + :email => orig_email, :verified_email => verified_email, + :auth_provider => "wikipedia", :auth_uid => auth_uid + follow_redirect! + post "/user/new", :params => { :user => { :email => new_email, + :verified_email => verified_email, :display_name => display_name, :auth_provider => "wikipedia", - :pass_crypt => "testtest", - :pass_crypt_confirmation => "testtest", - :consider_pd => "1" }, - :referer => referer } - assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") - post response.location - assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") - follow_redirect! - assert_redirected_to :controller => :confirmations, :action => :confirm, :display_name => display_name + :auth_uid => auth_uid, + :consider_pd => "1" } } + assert_response :redirect follow_redirect! end end -- 2.39.5